fix(gpui-linux): demand-driven Wayland loop and font-match prewarm - #7
Conversation
审查者指南移植了两项针对 Linux GPUI 的修复:Wayland 现在使用需求驱动的渲染循环,使空闲窗口进入休眠状态,并在有待处理工作时显式唤醒或重试;文本系统提供了字体缓存预热功能,Linux cosmic-text 会在字形整形之前预热主字体和回退字体匹配。核心行为已通过单元测试覆盖,但由于当前环境限制,实时合成器行为仍未经测试。 Sequence diagram for demand-driven Wayland frame schedulingsequenceDiagram
participant App
participant Window
participant Wayland
participant Compositor
App->>Window: schedule_frame()
Window->>Wayland: ping()
Wayland->>Window: scheduled_frame_fired()
Window->>App: request_frame_callback
App->>Window: draw(scene)
alt presentation succeeds
Window->>Compositor: surface.frame()
Window->>Compositor: surface.commit()
Compositor-->>Window: frame_callback_fired()
else presentation throttled or fails
Window->>Wayland: schedule_frame_retry()
Wayland->>Window: retry_timer_fired()
end
Window->>Window: Park when no work remains
Sequence diagram for Linux font-match cache prewarmingsequenceDiagram
participant Caller
participant TextSystem
participant CosmicText
Caller->>TextSystem: prewarm_fonts(fonts)
TextSystem->>TextSystem: resolve_font(font)
TextSystem->>CosmicText: prewarm_fonts(font_ids)
loop each primary and fallback font
CosmicText->>CosmicText: font_match_properties(font_id)
CosmicText->>CosmicText: get_font_matches(attributes)
end
Caller->>TextSystem: shape text without cold font-match lookup
State diagram for the demand-driven Wayland frame loopstateDiagram-v2
[*] --> Unconfigured
Unconfigured --> Scheduled: schedule_frame()
Scheduled --> Ticking: scheduled_frame_fired()
Ticking --> AwaitingCallback: draw() presents
Ticking --> PresentationFailed: draw() fails
Ticking --> RetryScheduled: schedule_frame()
AwaitingCallback --> Ticking: frame_callback_fired()
PresentationFailed --> RetryScheduled: retry_timer_fired()
RetryScheduled --> Ticking: retry_timer_fired()
Ticking --> Parked: no redraw or callback demand
Parked --> Scheduled: schedule_frame()
PresentationFailed --> PresentationFailed: draw() fails
AwaitingCallback --> Parked: callback completes without work
文件级变更
提示和命令与 Sourcery 交互
自定义使用体验访问你的控制面板:
获取帮助Original review guide in EnglishReviewer's GuidePorts two targeted Linux GPUI fixes: Wayland now uses a demand-driven render loop that parks idle windows and explicitly wakes or retries them when work is pending, while the text system exposes font-cache prewarming and Linux cosmic-text warms primary and fallback matches before shaping. Core behavior is covered by unit tests, but live compositor behavior remains untested in this environment. Sequence diagram for demand-driven Wayland frame schedulingsequenceDiagram
participant App
participant Window
participant Wayland
participant Compositor
App->>Window: schedule_frame()
Window->>Wayland: ping()
Wayland->>Window: scheduled_frame_fired()
Window->>App: request_frame_callback
App->>Window: draw(scene)
alt presentation succeeds
Window->>Compositor: surface.frame()
Window->>Compositor: surface.commit()
Compositor-->>Window: frame_callback_fired()
else presentation throttled or fails
Window->>Wayland: schedule_frame_retry()
Wayland->>Window: retry_timer_fired()
end
Window->>Window: Park when no work remains
Sequence diagram for Linux font-match cache prewarmingsequenceDiagram
participant Caller
participant TextSystem
participant CosmicText
Caller->>TextSystem: prewarm_fonts(fonts)
TextSystem->>TextSystem: resolve_font(font)
TextSystem->>CosmicText: prewarm_fonts(font_ids)
loop each primary and fallback font
CosmicText->>CosmicText: font_match_properties(font_id)
CosmicText->>CosmicText: get_font_matches(attributes)
end
Caller->>TextSystem: shape text without cold font-match lookup
State diagram for the demand-driven Wayland frame loopstateDiagram-v2
[*] --> Unconfigured
Unconfigured --> Scheduled: schedule_frame()
Scheduled --> Ticking: scheduled_frame_fired()
Ticking --> AwaitingCallback: draw() presents
Ticking --> PresentationFailed: draw() fails
Ticking --> RetryScheduled: schedule_frame()
AwaitingCallback --> Ticking: frame_callback_fired()
PresentationFailed --> RetryScheduled: retry_timer_fired()
RetryScheduled --> Ticking: retry_timer_fired()
Ticking --> Parked: no redraw or callback demand
Parked --> Scheduled: schedule_frame()
PresentationFailed --> PresentationFailed: draw() fails
AwaitingCallback --> Parked: callback completes without work
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
7d45621 to
94e6008
Compare
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
嗨——我发现了 1 个问题
面向 AI Agent 的提示
请处理此次代码审查中的评论:
## 单独评论
### 评论 1
<location path="crates/gpui-linux/src/linux/wayland/window.rs" line_range="728-737" />
<code_context>
+ // Before the first present, or when throttling skipped draw, a
+ // callback may never arrive. Otherwise let the compositor pace
+ // retries so an occluded window does not keep polling.
+ if frame_loop == FrameLoop::PresentationFailed
+ && state.presentation == PresentationState::RetryAfterPresent
+ {
+ if state.pending_frame_callback.is_none() {
+ let callback = state.surface.frame(&state.globals.qh, state.surface.id());
+ state.pending_frame_callback = Some(callback);
+ }
+ state.surface.commit();
+ self.frame_loop.set(FrameLoop::AwaitingCallback);
+ return;
+ }
+
</code_context>
<issue_to_address>
**问题(bug_risk):** `FrameLoop::PresentationFailed && RetryAfterPresent` 分支无法到达:每次重试都会进入 `frame()`,而该函数首先将循环状态更改为 `Ticking`,因此 `complete_frame()` 永远不会观察到 `FrameLoop::PresentationFailed`。结果是,首次成功呈现之后发生呈现失败时,系统不会为合成器节奏控制提交帧回调,而是不断安排固定的 60 Hz 重试计时器,即使表面被遮挡时也是如此。
**触发条件:** 窗口已经呈现至少一帧后,`renderer.draw(scene)` 返回 false。
**建议修复:** 在将状态更改为 `Ticking` 之前检查之前的帧循环状态,或者让 `complete_frame` 的条件仅依赖呈现状态,以便执行由合成器控制节奏的重试路径。
</issue_to_address>帮我变得更有用!请在每条评论上点击 👍 或 👎,我会利用这些反馈来改进审查结果。
Original comment in English
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="crates/gpui-linux/src/linux/wayland/window.rs" line_range="728-737" />
<code_context>
+ // Before the first present, or when throttling skipped draw, a
+ // callback may never arrive. Otherwise let the compositor pace
+ // retries so an occluded window does not keep polling.
+ if frame_loop == FrameLoop::PresentationFailed
+ && state.presentation == PresentationState::RetryAfterPresent
+ {
+ if state.pending_frame_callback.is_none() {
+ let callback = state.surface.frame(&state.globals.qh, state.surface.id());
+ state.pending_frame_callback = Some(callback);
+ }
+ state.surface.commit();
+ self.frame_loop.set(FrameLoop::AwaitingCallback);
+ return;
+ }
+
</code_context>
<issue_to_address>
**issue (bug_risk):** The `FrameLoop::PresentationFailed && RetryAfterPresent` branch is unreachable: every retry enters `frame()`, which first changes the loop state to `Ticking`, so `complete_frame()` never observes `FrameLoop::PresentationFailed`. As a result, a failed presentation after the first successful presentation never commits a frame callback for compositor pacing and instead repeatedly schedules the fixed 60 Hz retry timer, including while the surface is occluded.
**Triggers:** When `renderer.draw(scene)` returns false after the window has already presented at least one frame.
**Suggested fix:** Check the previous frame-loop state before changing it to `Ticking`, or make the `complete_frame` condition depend only on the presentation state so the compositor-paced retry path can execute.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 94e6008579
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if is_unconfigured_layer_shell(&state.role) { | ||
| state.redraw_requested = true; | ||
| return; | ||
| } |
There was a problem hiding this comment.
Prevent hidden Wayland windows from drawing
When hide() has set visible = false and detached the surface buffer, a normal entity or application refresh can now schedule this window again. Because this guard only excludes unconfigured layer-shell surfaces, both hidden XDG windows and configured layer-shell windows proceed to renderer.draw, whose presentation attaches a buffer and maps the supposedly hidden window again. Check state.visible here or suppress frame scheduling while the window is hidden.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 07fda8e.
draw() now returns without attaching a buffer when !state.visible (XDG and configured layer-shell), latching redraw_requested so a later show() can paint. frame() / complete_frame() / schedule_frame() also refuse to present or arm a wakeup while hidden; hide() parks the loop after setting visible = false. Overlay and click-through (set_input_region / set_mouse_passthrough) still commit independently of the present path.
c8d86d6 to
ee15530
Compare
Wayland frame callbacks only fire after a commit the compositor paints. Park the loop when idle instead of committing empty heartbeat frames, and wake from GPUI via schedule_frame when a window still needs work. Zed-Origin: eb354c8d504071bdb79110a7a5c9d374c2864113 Co-authored-by: freefcw <freefcw@gmail.com>
Expose TextSystem::prewarm_fonts and warm cosmic-text get_font_matches for the requested fonts so shaping does not pay that cost on the hot path. Zed-Origin: f1d27d545e79f98ffde7ebf3229f8eb8bad791aa Co-authored-by: freefcw <freefcw@gmail.com>
Key compositor-paced retries on PresentationState::RetryAfterPresent so frame() setting Ticking no longer skips the occluded-surface path. Skip buffer attach and frame scheduling while visible is false so a refresh cannot remap a hidden XDG or configured layer-shell window. Co-authored-by: freefcw <freefcw@gmail.com>
shellcheck without -x cannot follow verify-common.sh, and the first release-archive loop never uses manifest_path. Point shellcheck at scripts/ and ignore the unused TSV column. Co-authored-by: freefcw <freefcw@gmail.com>
ee15530 to
c209002
Compare
Selective ports of two Zed Linux GPUI fixes onto
develop/0.9. Not a wholesale Zed sync.Rebased onto
97bc23905fb9c54ad4a766ad748fa3bd4e8c829eafter #4 (Taffy), #5 (container_query+ axis-locked scroll), and #6 (appearance override, deferred appearance callbacks, const HSLA, macOS NSAppearance unsafe) merged. One conflict incrates/gpui/src/app.rstest imports: kept this PR’s refresh-test types (Context,Empty,Render,Window) and #6’sWindowAppearance.platform.rsandwindow.rsauto-merged. #3’squit_requestedskip and #6’sset_window_appearance/ deferred appearance observers are unchanged.1. Wayland demand-driven render loop
Upstream: zed-industries/zed
eb354c8d504071bdb79110a7a5c9d374c2864113(#60690)Wayland
wl_surfaceframe callbacks only arrive after a commit the compositor actually paints. The old loop requested a callback on every tick and committed empty frames as a heartbeat, which froze idle fullscreen windows and burned CPU.This port parks when idle and wakes via
schedule_frame()/ a calloop ping / a 60Hz retry timer. LayerShell, overlay, click-through,Application::new(),QuitMode, and GPU resource budgets are unchanged.Local commit:
554eff6885aa29252a8c35dad3f0f6121b16665cAuthor/committer: Jun He freefcw@gmail.com
2. Prewarm Linux font-match caches
Upstream:
f1d27d545e79f98ffde7ebf3229f8eb8bad791aa(#63158)Adds
PlatformTextSystem::prewarm_fonts/TextSystem::prewarm_fontsand implements it on the Linux cosmic-text backend soget_font_matchesis warmed off the shaping hot path. The text stack is not rewritten.Local commit:
ab2fb46c245f4552f2c5f7df70e70edcdc70801fAuthor/committer: Jun He freefcw@gmail.com
Review fixes (
ec811a9688a54b95b8d9d364ec7e0965f52a04c2)Surgical follow-up on the same branch. Author/committer: Jun He freefcw@gmail.com. File:
crates/gpui-linux/src/linux/wayland/window.rsonly.Sourcery — compositor-paced retry is now reachable
frame()still setsFrameLoop::Tickingimmediately, socomplete_frame()never observesFrameLoop::PresentationFailed. The retry policy now keys on presentation state viaPresentationState::compositor_paced_retry()(== RetryAfterPresent), not on the frame-loop enum.renderer.drawstays onRetryAfterPresentand commits a compositor frame callback (AwaitingCallback).RetryBeforeFirstPresent), retries still use the 60 Hz timer.draw()still recordsFrameLoop::PresentationFailedon a failed present; that is no longer thecomplete_framegate.Reachability is covered by
failed_present_after_a_successful_frame_uses_compositor_pacinginpresentation_state_tests:Presented.failed()isRetryAfterPresent, and that state is the only one for whichcompositor_paced_retry()is true.Codex P1 — hidden windows stay unmapped
draw()now returns without attaching a buffer when!state.visible(XDG and configured layer-shell), latchingredraw_requestedso a latershow()can paint.frame()/complete_frame()/schedule_frame()also refuse to present or arm a wakeup while hidden;hide()parks the loop after settingvisible = false. Overlay and click-through (set_input_region/set_mouse_passthrough) still commit independently of the present path.CI hygiene (
c209002548c4622800c0dad2268d7c3eda6190d2)shellcheck scripts/*.shwithout-xcannot followverify-common.sh(SC1091). CI now runsshellcheck --source-path=scripts -x scripts/*.sh.Files
Demand-driven loop
crates/gpui/src/platform.rs—completed_frame→schedule_frame(kept alongside Port application appearance override and const HSLA constructors #6set_window_appearance)crates/gpui/src/window.rs— wake demand-driven platforms from next-frame / throttle / leftover dirty work (kept alongside Port application appearance override and const HSLA constructors #6 deferred appearance callbacks)crates/gpui/src/app.rs—flush_effectsschedules frames for dirty / present / callback demand; refresh-effect test (kept alongside Port application appearance override and const HSLA constructors #6App::set_window_appearance)crates/gpui/src/app/async_context.rs—AsyncApp::refreshflushes viaupdatecrates/gpui-linux/src/linux/wayland/client.rs— frame ping + retry timercrates/gpui-linux/src/linux/wayland/window.rs—FrameLoop/PresentationState, plus the review-fix hide/retry guardsFont prewarm
crates/gpui/src/platform.rs—prewarm_fontstrait defaultcrates/gpui/src/text_system.rs—TextSystem::prewarm_fontscrates/gpui-linux/src/linux/text_system.rs— cosmic-text cache prewarm + sharedFontMatchPropertiesCI hygiene
.github/workflows/gpui-feature-matrix.yml— follow sourced scripts in shellcheckSkipped hunks (intentional)
crates/gpui_web/src/window.rs— no web backend herecrates/zed/src/main.rstheme-font prewarm caller — this is a library; apps callTextSystem::prewarm_fontsevent_arena.clear()— this tree has no event arenaWindowInvalidator::wake_platform()— not present hereTestWindowrewrite (simulate_scheduled_frame,frame_callback_pending) — this repo’s test window is a different, simpler harness#[gpui::test]s that require that harnessupdate_ime_enabled— keep this fork’srequired_ime_state_changeWaylandSurfaceState::Popup/ popup reposition viais_configured()— this fork uses LayerShell +WindowKind::Overlaywgpu_renderer.rs; wholesale overwrite ofapp.rs/window.rs/platform.rs/client.rsTests run
cargo fmt --all -- --checkcargo test -p adabraka-gpui-core --lib --features test-support -- --test-threads=1— 217 passed (pre-rebase)cargo test -p adabraka-gpui-linux --lib --features test-support -- --test-threads=1— 40 passed (includesfailed_present_after_a_successful_frame_uses_compositor_pacing)cargo clippy -p adabraka-gpui-core -p adabraka-gpui-linux --lib --tests --features test-support -- -D warningsshellcheck --source-path=scripts -x scripts/*.sh— cleanCould not exercise a live Wayland compositor in this environment; the demand-driven loop and hide/show present guards are covered by unit tests and compile, not by a fullscreen compositor repro.
Sourcery 摘要
采用需求驱动的 Wayland 渲染和 Linux 字体匹配缓存预热,以改善空闲窗口行为和文本整形响应速度。
新功能:
错误修复:
增强功能:
测试:
Original summary in English
Sourcery 摘要
通过让 Wayland 帧交付按需进行,并预热字体匹配缓存,提升 Linux 渲染响应速度和文本整形性能。
新功能:
错误修复:
增强功能:
测试:
Original summary in English
Sourcery 总结
通过让 Wayland 帧传递采用需求驱动,并预热字体匹配缓存,改善 Linux 渲染和文本响应能力。
新功能:
错误修复:
增强功能:
测试:
Original summary in English
Sourcery 总结
采用按需驱动的 Wayland 渲染和 Linux 字体匹配缓存预热,以改善空闲窗口行为和文本塑形响应速度。
新功能:
错误修复:
增强功能:
测试:
Original summary in English
Sourcery 摘要
通过按需驱动的 Wayland 调度和主动预热字体匹配缓存,提升 Linux 渲染响应速度和空闲行为。
新功能:
错误修复:
增强功能:
CI:
测试:
杂项:
Original summary in English
Sourcery 摘要
通过按需驱动的 Wayland 调度和预热字体匹配,提升 Linux 渲染响应速度和空闲行为。
新功能:
错误修复:
增强功能:
CI:
测试:
Original summary in English
Summary by Sourcery
Improve Linux rendering responsiveness and idle behavior with demand-driven Wayland scheduling and prewarmed font matching.
New Features:
Bug Fixes:
Enhancements:
CI:
Tests: