gpui: Make the Wayland render loop demand-driven - #60690
Conversation
99b0ef6 to
dd16384
Compare
|
Thank you, works perfectly! |
|
@NeelChotai So I didn't see this PR and developed the same demand-driven wayland render loop independently. I have a bunch of regression tests and some other stuff your pr doesn't quite handle. I'm a bit unsure how to best collaborate if you're interested in that. I'll put up a draft PR that combines yours and mine and you can see if there's anything in there that you're interested in picking up. Let me know if you prefer something else. |
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>
|
@NeelChotai We should talk about this PR because #60308 already implements a fix and it's almost mergeable. I believe it solves the same problems |
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>
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>
dd16384 to
61ee549
Compare
|
Thank you! I'll close my PR in favor of this one. |
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>
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>
3445155 to
6c3760a
Compare
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>
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>
af4fa05 to
65a1afa
Compare
|
Pardon me for disturbing you, but is there any ETA for this to be merged? I am mainly just looking forward to this happening so that I can stop using Cursor. And, likely, will be contributing to zed in the future, too. But the absence of a properly working full-screen mode is a blocker for me. Just having my fingers crossed and hoping for it to be merged asap, as well as released :-) |
I should be able to look at it this weekend. Sorry I have been traveling for the past month and haven't had access to my Linux computer to test this |
19ac35f to
f4cf229
Compare
Closes #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
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>
f4cf229 to
6aeccb6
Compare
Co-authored-by: Daan De Meyer <daan@amutable.com>
6aeccb6 to
a63fdd7
Compare
|
@daandemeyer I pulled in some of the changes from your PR and I've been testing this morning, I'm happy with the state of this branch now so we're good to merge! |
|
@NeelChotai Thank you! It's slightly big but #61458 also helped me to reduce the CPU usage of zed quite a bit on wayland. |
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 committing 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 --------- 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>
Upstream at ce48461 (373 commits since the 2026-08-04 merge point). Thirteen conflicts; every fork patch in FORK_CHANGES.md survives. The four that took real work: * **`gpui_apple` crate split.** Upstream moved `metal_renderer.rs`, `metal_atlas.rs` and `shaders.metal` out of `gpui_macos` into a new `gpui_apple`. Git followed the renames, so the fork's backdrop-blur Metal patches came along; `metal_custom_shader.rs` did not, because upstream never knew about it. It moves by hand — `metal_renderer.rs` is its only caller — and the `naga` dependency moves with it. * **`completed_frame` is gone** (zed-industries#60690 made the Wayland render loop demand-driven). Every backend loses it; the trait gains `schedule_frame`. The fork's Wayland patches are re-seated on the new loop: the `closed` guard in `frame()` and the deferral of `pending_drawable_size` / `pending_viewport_dest` to `draw()` are kept as they were, the fork's `force_render_after_recovery` latch becomes upstream's `redraw_requested` (same semantics, and the field itself was dropped by the struct merge), and the wlroots empty-commit workaround is dropped — `complete_frame`'s presentation state machine now commits on the `RetryAfterPresent` path. One behaviour change the new loop forced: the throttled-configure early return (the 1px Mutter "wabbern" fix) now calls `request_redraw()` before returning. Nothing ticks on its own any more, and `resize_throttle` is cleared at the top of `frame()` — parking with it still set would skip every further resizing configure and freeze the window for the rest of the drag. * **`WM_USER + 9` collided.** Upstream's new `WM_GPUI_END_SESSION` took the number `WM_GPUI_NATIVE_DRAG` was using. Upstream keeps it; this fork's own window messages move to `+100`, far enough clear that the next upstream addition cannot alias one again. * **Windows `render_to_image` existed twice** after the merge — upstream grew its own while the fork carried one. Upstream's is the better half (device-lost guard, `background_appearance`, goes through `render` instead of duplicating the batch loop), so the fork's copy is dropped and upstream's ungated instead, which is all the fork ever needed of it. Smaller ones, all "keep both": the `dmabuf` / `d3d11_surface` modules beside upstream's `debug_overlay`, `hovers_suspended` beside `debug_frame_overlay`, `GlowParams` and `blur_alpha_mask` in the text-system imports, the ungated `image::RgbaImage` imports, and the `ash` / `image` dependencies upstream dropped for its own reasons. The `log_err_with_backtrace` patch is extended to the three update sites zed-industries#60690 added inside the `request_frame` callback, which is exactly the diagnostic that patch exists for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011GAy2t2gSzKkFYNRVrtNpb
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 committing 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 --------- 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>
Closes #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 committing empty frames on every tick as the artificial heartbeat from the compositor.
Release Notes: