gpui: Fix wakeups in the demand-driven Wayland render loop - #61063
daandemeyer wants to merge 2 commits into
Conversation
|
We require contributors to sign our Contributor License Agreement, and we don't have @daandemeyer on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'. |
|
@cla-bot check |
|
We require contributors to sign our Contributor License Agreement, and we don't have @daandemeyer on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'. |
|
The cla-bot has been summoned, and re-checked this pull request! |
|
@cla-bot check |
|
The cla-bot has been summoned, and re-checked this pull request! |
The explicit `completed_frame` was a code smell and only existed to execute Wayland's frame-end bookkeeping, it's now inlined in `schedule_frame` so that becomes the entire render loop contract. Takes some elements from #60308 and test instrumentation from #61063. Co-authored-by: Philipp Schaffrath <philipp.schaffrath@gmail.com> Co-authored-by: Daan De Meyer <daan@amutable.com>
|
Firstly, thank you for taking the time to investigate this. I pulled some of your changes in to my branch (and added you as co-author) but I would like to merge some components of this PR separately - I can rebase this branch on main once mine is merged in, but your branch has some useful fixes for |
b414a77 to
e734f98
Compare
The explicit `completed_frame` was a code smell and only existed to execute Wayland's frame-end bookkeeping, it's now inlined in `schedule_frame` so that becomes the entire render loop contract. Takes some elements from #60308 and test instrumentation from #61063. Co-authored-by: Philipp Schaffrath <philipp.schaffrath@gmail.com> Co-authored-by: Daan De Meyer <daan@amutable.com>
e734f98 to
bdf69de
Compare
Closes zed-industries#55345 Wayland is the only platform whose frame ticks are conditional: a wl_surface frame callback only arrives after a commit the compositor goes on to repaint. GPUI assumed unconditional ticks (any previously our Wayland backend faked them), so an idle fullscreen window stopped receiving callbacks and froze until external damage arrived. This PR makes the Wayland render loop demand-driven, instead parking when there's nothing to draw, and stops Zed commiting empty frames on every tick as the artificial heartbeat from the compositor. Release Notes: - Fixed the UI freezing in fullscreen on some Wayland compositors - Fixed idle windows waking at the display's refresh rate on Wayland Signed-off-by: Daan De Meyer <daan@amutable.com>
zed-industries#60690 fixes the main idle repaint bug by parking the Wayland render loop when there is no work to do. While testing that change on KWin/RADV, I found a few places where the old always-running loop had been hiding missing wakeups. AsyncApp::refresh queued a refresh effect without flushing it, and the app only woke a parked window when its invalidator was dirty. That misses two other reasons to draw: a scene that still needs to be presented and a next-frame callback queued during the previous frame. Wayland configures, scale changes, and decoration changes can also alter the surface without dirtying GPUI. Flush async refreshes through App::update, consider all three kinds of work when waking a parked window, and keep Wayland redraw requests latched until a draw actually consumes them. Presentation failures need slightly different pacing before and after the first successful frame. Before the surface is mapped, keep the timer fallback because a compositor may not send it a frame callback. Afterwards, commit at most one callback and let the compositor wake us. This keeps an occluded window from polling on a timer and also avoids losing track of a callback committed by another surface update. This is a follow-up to zed-industries#60690 rather than a replacement for it. On the affected KWin/RADV system, the original code issued 7,544 frame requests and 7,545 commits in 52.749 seconds (143.02 per second), including 7,290 empty commits. Forcing FIFO was effectively unchanged at 143.09 requests per second. With zed-industries#60690 and this change applied, a 30-second idle trace after a 15-second settle contained no frame requests, commits, buffer attaches, damage, or callbacks. Add regression coverage for async refresh, parked windows with pending presentation work, and callbacks queued during a frame. Signed-off-by: Daan De Meyer <daan@amutable.com>
bdf69de to
24d37c2
Compare
The explicit `completed_frame` was a code smell and only existed to execute Wayland's frame-end bookkeeping, it's now inlined in `schedule_frame` so that becomes the entire render loop contract. Takes some elements from #60308 and test instrumentation from #61063. Co-authored-by: Philipp Schaffrath <philipp.schaffrath@gmail.com> Co-authored-by: Daan De Meyer <daan@amutable.com>
The explicit `completed_frame` was a code smell and only existed to execute Wayland's frame-end bookkeeping, it's now inlined in `schedule_frame` so that becomes the entire render loop contract. Takes some elements from #60308 and test instrumentation from #61063. Co-authored-by: Philipp Schaffrath <philipp.schaffrath@gmail.com> Co-authored-by: Daan De Meyer <daan@amutable.com> Co-authored-by: Christopher Biscardi <chris@christopherbiscardi.com>
The explicit `completed_frame` was a code smell and only existed to execute Wayland's frame-end bookkeeping, it's now inlined in `schedule_frame` so that becomes the entire render loop contract. Takes some elements from #60308 and test instrumentation from #61063. Co-authored-by: Philipp Schaffrath <philipp.schaffrath@gmail.com> Co-authored-by: Daan De Meyer <daan@amutable.com> Co-authored-by: Christopher Biscardi <chris@christopherbiscardi.com>
The explicit `completed_frame` was a code smell and only existed to execute Wayland's frame-end bookkeeping, it's now inlined in `schedule_frame` so that becomes the entire render loop contract. Takes some elements from #60308 and test instrumentation from #61063. Co-authored-by: Philipp Schaffrath <philipp.schaffrath@gmail.com> Co-authored-by: Daan De Meyer <daan@amutable.com> Co-authored-by: Christopher Biscardi <chris@christopherbiscardi.com>
The explicit `completed_frame` was a code smell and only existed to execute Wayland's frame-end bookkeeping, it's now inlined in `schedule_frame` so that becomes the entire render loop contract. Takes some elements from #60308 and test instrumentation from #61063. Co-authored-by: Philipp Schaffrath <philipp.schaffrath@gmail.com> Co-authored-by: Daan De Meyer <daan@amutable.com> Co-authored-by: Christopher Biscardi <chris@christopherbiscardi.com>

#60690 fixes the main idle repaint bug by parking the Wayland render loop when
there is no work to do. While testing that change on KWin/RADV, I found a few
places where the old always-running loop had been hiding missing wakeups.
AsyncApp::refresh queued a refresh effect without flushing it, and the app only
woke a parked window when its invalidator was dirty. That misses two other
reasons to draw: a scene that still needs to be presented and a next-frame
callback queued during the previous frame. Wayland configures, scale changes,
and decoration changes can also alter the surface without dirtying GPUI.
Flush async refreshes through App::update, consider all three kinds of work
when waking a parked window, and keep Wayland redraw requests latched until a
draw actually consumes them.
Presentation failures need slightly different pacing before and after the
first successful frame. Before the surface is mapped, keep the timer fallback
because a compositor may not send it a frame callback. Afterwards, commit at
most one callback and let the compositor wake us. This keeps an occluded
window from polling on a timer and also avoids losing track of a callback
committed by another surface update.
This is a follow-up to #60690 rather than a replacement for it. On the affected
KWin/RADV system, the original code issued 7,544 frame requests and 7,545
commits in 52.749 seconds (143.02 per second), including 7,290 empty commits.
Forcing FIFO was effectively unchanged at 143.09 requests per second. With
#60690 and this change applied, a 30-second idle trace after a 15-second settle
contained no frame requests, commits, buffer attaches, damage, or callbacks.
Add regression coverage for async refresh, parked windows with pending
presentation work, and callbacks queued during a frame.
Signed-off-by: Daan De Meyer daan@amutable.com