Skip to content

fix: redraw only the views whose moving pictures are due, at most 30 times a second - #49

Open
itz4blitz wants to merge 1 commit into
mainfrom
fix/anim-targeted-redraw
Open

itz4blitz wants to merge 1 commit into
mainfrom
fix/anim-targeted-redraw

Conversation

@itz4blitz

Copy link
Copy Markdown
Owner

Fixes #39

Cause

anim::frame_now scheduled each playing picture's next frame through anim::wake, and the wake's timer called window.refresh(). That dirties the whole window and sets GPUI's refreshing flag, which bypasses view caching. The only floor on frame time was frames::shown_for, which only slows frames of 10 ms or less, so a GIF with 16–20 ms frames redrew the whole window at about the display rate. The hover handler for hover-to-play pictures also called window.refresh().

Fix (crates/tinyclient/src/gui/anim.rs)

  • Targeted notify. frame_now records the view drawing the picture (window.current_view()). The window's single timer now keeps a per-view due time (Due<EntityId>). When it fires it calls cx.notify(view) for only the views that are due, then sets itself for the earliest of the rest. Views that are not due, and any view embedded with .cached, are not drawn again. The hover handler notifies its own view the same way.
  • A floor on draw rate. A playing picture is drawn again no sooner than FASTEST (33 ms, about 30 fps). The floor is applied when the wake is scheduled, not in frames::shown_for. The frame shown is still worked out from elapsed time (frame_at), so a 20 ms GIF keeps its real speed and skips the frames in between. Raising the delays in shown_for would have slowed it down to about 60% speed instead.

Not in this PR: .cached views

No GUI view uses .cached(...) yet. Until one does, GPUI still re-renders every uncached view when any view is dirty, and notifying a view also dirties its ancestors (the root Shell). So for now most of the CPU saving comes from the frame-rate floor: about 1.6x fewer full draws for 20 ms GIFs and about 2x for 16 ms ones. The targeted notify is the groundwork that will let caching pay off later.

I did not add .cached because it isn't safe yet. Several child views get state pushed into them during Shell::render without a notify. For example, MemberList::see assigns self.scene without notifying, so a cached view would show stale content. Caching MemberList, Composer and Pane needs those paths checked first, so it belongs in a follow-up.

Validation

  • cargo build -p tinyclient --features gui: builds; the only warning is the existing one in call/permission.rs.
  • cargo test -p tinyclient --features gui: 350 passed, 0 failed, 1 ignored.
  • cargo test -p tinyclient (no gui): 12 passed, 0 failed.
  • cargo clippy -p tinyclient --features gui: no warnings in anim.rs.
  • New unit tests:
    • a_fast_animation_is_drawn_at_most_thirty_times_a_second_and_keeps_its_time: the floor, and a 20 ms GIF skipping frames on the clock.
    • a_wake_draws_only_the_views_that_are_due: per-view due times, keeping the sooner request, and the next fire time.

Not verified: I did not run the app with animations playing, and did not profile CPU or frame rate live. The ~57 fps → ≤30 fps figure comes from the code, not a measurement. TINYCLIENT_FRAMES=1 with the gui-shot feature can confirm it on a live session.

🤖 Generated with Claude Code

…times a second

An animation's wake called window.refresh(), which dirties the whole window and
bypasses view caching, and a GIF of 20 ms frames asked for a draw every 20 ms.
The wake now notifies the views that drew the due pictures, and a playing
picture is drawn again no sooner than 33 ms later; the frame shown still
follows the clock, so fast files keep their speed and skip frames between.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@itz4blitz itz4blitz self-assigned this Oct 18, 2026
@itz4blitz

Copy link
Copy Markdown
Owner Author

@vbmithr thanks for the profile in #39. It made the cause easy to find. Here's what this PR does with it, and how to check it on your machine.

What changed (crates/tinyclient/src/gui/anim.rs):

  • Only the drawing view is notified. Each window has one timer. It records which view drew each picture and when that picture's next frame is due. When it fires, it calls cx.notify(view) for just the views that are due, instead of window.refresh(). The hover handler does the same.
  • Frame-rate floor of 33 ms (about 30 fps). I put this where the timer is scheduled, not in frames::shown_for. The frame shown still follows the clock, so a 20 ms GIF plays at its real speed and skips frames in between. Raising the delays in shown_for would have played it at about 60% speed.
  • No .cached(...) yet. Some child views get state set by the shell's render without being notified. For example, MemberList::see assigns self.scene with no notify, so a cached member list would show stale content. That needs going through first, so I left it for a follow-up.

What to expect: without caching, notifying one view still redraws the whole window, because the root view gets marked dirty and nothing is cached. For now the saving comes from the floor: by the arithmetic, about 2x fewer full redraws for 16 ms GIFs and about 1.6x for 20 ms ones. I haven't profiled it, so your uprobe numbers before and after would be the real measurement. They'd also tell us whether the caching pass is worth doing.

To try it:

git fetch origin pull/49/head:pr-49 && git checkout pr-49
cargo test -p tinyclient --features gui gui::anim          # the two new tests
cargo run -p tinyclient --features gui-shot                # window, with a frame counter
TINYCLIENT_FRAMES=1 cargo run -p tinyclient --features gui-shot 2>&1 | grep 'tinyclient: frame'

TINYCLIENT_FRAMES=1 prints a line for each frame the shell renders. With one GIF or animated avatar playing, you should see at most about 30 lines a second, where before it was about 57. For the CPU numbers, run the same set_dirty / Window::draw uprobes as before against this build.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Animated pictures redraw the whole window every frame (~57 fps, 25–30% CPU)

1 participant