Port medium-priority GPUI animation, LineLayout, and Windows Restart Manager APIs - #9
Merged
Merged
Conversation
审阅者指南移植了三项相互独立的 GPUI 功能：支持解析、可重新定向且可选限制重绘速率的弹簧动画；可复用的 LineLayout 拆分与绘制 API；以及支持 Windows Restart Manager 关机处理和借用安全的延迟退出。窗口级非活动帧节流现已支持配置，同时保留现有默认值；每个功能区域都包含针对性的测试或示例覆盖。 可重新定向弹簧动画渲染的时序图sequenceDiagram
participant View as AnimatedElement
participant State as SpringElementState
participant Config as SpringConfig
participant Window
View->>State: with_spring(id, SpringAnimation)
View->>State: request_layout()
State->>Config: step(current_state, target, elapsed)
Config-->>State: position and velocity
State->>View: animator(element, position)
alt spring not settled
State->>Window: request_animation_frame()
end
Note over State: Target changes preserve existing velocity
受计时器限制的动画重绘时序图sequenceDiagram
participant Animation as AnimationElement
participant Window
participant Timer
participant View
Animation->>Animation: with_max_fps(max_fps)
Animation->>Window: spawn(timer)
Window->>Timer: timer(interval)
Timer-->>Window: elapsed
Window->>View: notify(current_view)
View->>Animation: request_layout()
Animation->>Window: spawn(timer)
Windows Restart Manager 关机时序图sequenceDiagram
participant Windows
participant Window as WindowsWindowInner
participant Platform as WindowsPlatformInner
participant AppCell
Windows->>Window: WM_QUERYENDSESSION
Window-->>Windows: 1
Windows->>Window: WM_ENDSESSION
Window->>Window: SendMessageW(WM_GPUI_END_SESSION)
Window->>Platform: handle_end_session()
Platform->>AppCell: quit callback()
alt AppCell available
AppCell-->>Platform: true
Platform->>Platform: flush logger
Platform->>Platform: exit(0)
else AppCell borrowed
AppCell-->>Platform: false
Platform->>Platform: PostQuitMessage(0)
end
文件级变更
提示和命令与 Sourcery 交互
自定义你的体验访问你的控制面板以：
获取帮助Original review guide in EnglishReviewer's GuidePorts three independent GPUI capabilities: analytic, retargetable spring animations with optional redraw-rate limits; reusable LineLayout split and painting APIs; and Windows Restart Manager shutdown handling with borrow-safe deferred quitting. Window-level inactive-frame throttling is configurable while retaining the existing default, and each area includes focused tests or example coverage. Sequence diagram for retargetable spring animation renderingsequenceDiagram
participant View as AnimatedElement
participant State as SpringElementState
participant Config as SpringConfig
participant Window
View->>State: with_spring(id, SpringAnimation)
View->>State: request_layout()
State->>Config: step(current_state, target, elapsed)
Config-->>State: position and velocity
State->>View: animator(element, position)
alt spring not settled
State->>Window: request_animation_frame()
end
Note over State: Target changes preserve existing velocity
Sequence diagram for timer-limited animation redrawssequenceDiagram
participant Animation as AnimationElement
participant Window
participant Timer
participant View
Animation->>Animation: with_max_fps(max_fps)
Animation->>Window: spawn(timer)
Window->>Timer: timer(interval)
Timer-->>Window: elapsed
Window->>View: notify(current_view)
View->>Animation: request_layout()
Animation->>Window: spawn(timer)
Sequence diagram for Windows Restart Manager shutdownsequenceDiagram
participant Windows
participant Window as WindowsWindowInner
participant Platform as WindowsPlatformInner
participant AppCell
Windows->>Window: WM_QUERYENDSESSION
Window-->>Windows: 1
Windows->>Window: WM_ENDSESSION
Window->>Window: SendMessageW(WM_GPUI_END_SESSION)
Window->>Platform: handle_end_session()
Platform->>AppCell: quit callback()
alt AppCell available
AppCell-->>Platform: true
Platform->>Platform: flush logger
Platform->>Platform: exit(0)
else AppCell borrowed
AppCell-->>Platform: false
Platform->>Platform: PostQuitMessage(0)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
嘿——我发现了 3 个问题
面向 AI Agent 的提示
请处理此次代码审查中的评论：
## 单独评论
### 评论 1
<location path="crates/gpui/src/elements/animation.rs" line_range="324-333" />
<code_context>
+ let elapsed = now.duration_since(state.updated_at).as_secs_f32();
+ match state.playback {
+ SpringPlayback::Running => {
+ state.spring = state.config.step(state.spring, state.target, elapsed);
+ }
+ SpringPlayback::Paused
+ | SpringPlayback::Stopped
+ | SpringPlayback::Completed
+ | SpringPlayback::Cancelled => {}
+ }
+
+ state.config = self.config;
+ state.target = self.target;
+
+ let done = match self.playback {
</code_context>
<issue_to_address>
**issue (bug_risk)：** 当弹簧目标、配置或播放模式发生变化时，该元素会先使用旧目标、旧配置和旧播放模式推进之前的状态，然后才赋予新值。因此，重新设定目标的弹簧会先朝着过时状态渲染一帧，播放模式的变化也会延迟一帧生效。
**触发条件：** 已存在的弹簧元素在两次布局之间被重新设定目标，或其播放模式/配置发生变化时。
**建议修复：** 在推进状态之前，将新的目标、配置和播放模式赋给状态，同时在运行中的弹簧重新设定目标时保留现有的位置和速度。
</issue_to_address>
### 评论 2
<location path="crates/gpui-windows/src/windows/platform.rs" line_range="1030-1043" />
<code_context>
}
+ fn handle_end_session(&self) -> Option<isize> {
+ let mut shutdown_completed = false;
+ if let Some(ref mut callback) = self.state.borrow_mut().callbacks.quit {
+ shutdown_completed = callback();
+ }
+ log::logger().flush();
+ if shutdown_completed {
+ std::process::exit(0);
+ }
+
+ // Shutdown couldn't run synchronously, since the AppCell is already borrowed.
+ // Windows may terminate the application as soon as we return from this handler,
+ // but posting WM_QUIT now may still let the message loop shut down first.
+ unsafe { PostQuitMessage(0) };
+ Some(0)
+ }
+
</code_context>
<issue_to_address>
**issue (bug_risk)：** 当退出回调无法借用 `AppCell` 时，`handle_end_session` 会发布 `WM_QUIT`，但不会重试该回调或运行 `App::shutdown`。这样消息循环可能在窗口、退出观察者和关闭清理仍处于活动状态时终止，因此在此更改要处理的这种确切借用状态下，Restart Manager 的关闭过程并不干净。
**触发条件：** `WM_ENDSESSION` 到达时，`AppCell` 已经被可变借用。
**建议修复：** 在允许进程退出之前，将退出回调推迟到主循环中并重试（或以其他方式调度 `App::shutdown`），而不是仅使用 `PostQuitMessage` 作为后备方案。
</issue_to_address>
### 评论 3
<location path="crates/gpui/src/text_system/line_layout.rs" line_range="138" />
<code_context>
+ /// - `suffix` contains glyphs for bytes `[byte_index, len)` with positions
+ /// shifted left so the first glyph starts at x=0, and byte indices rebased to 0.
+ /// - `font_size`, `ascent`, and `descent` are copied to both halves.
+ pub fn split_at(&self, byte_index: usize) -> (LineLayout, LineLayout) {
+ let x_offset = self.x_for_index(byte_index);
+
+ // Partition glyph runs. A single run may contribute glyphs to both halves.
+ let mut left_runs = Vec::new();
</code_context>
<issue_to_address>
**issue (bug_risk)：** 传入大于 `self.len` 的字节索引会导致 `self.len - byte_index` 下溢，而放置在后缀中的任何字形也可能导致 `g.index - byte_index` 下溢。因此，该公共方法会直接 panic，而不是拒绝或安全处理超出范围的拆分。
**触发条件：** 调用方传入超出范围的字节索引时。
**建议修复：** 在计算偏移量之前，对 `byte_index > self.len` 进行断言或返回错误，使其行为符合标准拆分 API 的边界处理方式。
```suggestion
pub fn split_at(&self, byte_index: usize) -> (LineLayout, LineLayout) {
assert!(byte_index <= self.len);
```
</issue_to_address>
帮我变得更有用！请在每条评论上点击 👍 或 👎，我会利用反馈来改进审查结果。
Original comment in English
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="crates/gpui/src/elements/animation.rs" line_range="324-333" />
<code_context>
+ let elapsed = now.duration_since(state.updated_at).as_secs_f32();
+ match state.playback {
+ SpringPlayback::Running => {
+ state.spring = state.config.step(state.spring, state.target, elapsed);
+ }
+ SpringPlayback::Paused
+ | SpringPlayback::Stopped
+ | SpringPlayback::Completed
+ | SpringPlayback::Cancelled => {}
+ }
+
+ state.config = self.config;
+ state.target = self.target;
+
+ let done = match self.playback {
</code_context>
<issue_to_address>
**issue (bug_risk):** When a spring target, configuration, or playback mode changes, the element advances the previous state using the previous target/configuration/playback before assigning the new values. The retargeted spring therefore renders one frame toward stale state and a playback change takes effect one frame late.
**Triggers:** When an existing spring element is retargeted or its playback/configuration is changed between layout passes.
**Suggested fix:** Assign the new target, configuration, and playback to the state before advancing it, while preserving the existing position and velocity for running retargets.
</issue_to_address>
### Comment 2
<location path="crates/gpui-windows/src/windows/platform.rs" line_range="1030-1043" />
<code_context>
}
+ fn handle_end_session(&self) -> Option<isize> {
+ let mut shutdown_completed = false;
+ if let Some(ref mut callback) = self.state.borrow_mut().callbacks.quit {
+ shutdown_completed = callback();
+ }
+ log::logger().flush();
+ if shutdown_completed {
+ std::process::exit(0);
+ }
+
+ // Shutdown couldn't run synchronously, since the AppCell is already borrowed.
+ // Windows may terminate the application as soon as we return from this handler,
+ // but posting WM_QUIT now may still let the message loop shut down first.
+ unsafe { PostQuitMessage(0) };
+ Some(0)
+ }
+
</code_context>
<issue_to_address>
**issue (bug_risk):** When the quit callback cannot borrow `AppCell`, `handle_end_session` posts `WM_QUIT` without retrying the callback or running `App::shutdown`. The message loop can then terminate with windows, quit observers, and shutdown cleanup still active, so Restart Manager shutdown is not clean in the exact borrowed-state case this change is intended to handle.
**Triggers:** When `WM_ENDSESSION` arrives while `AppCell` is already mutably borrowed.
**Suggested fix:** Defer and retry the quit callback on the main loop (or otherwise schedule `App::shutdown`) before allowing the process to exit, rather than using `PostQuitMessage` as the only fallback.
</issue_to_address>
### Comment 3
<location path="crates/gpui/src/text_system/line_layout.rs" line_range="138" />
<code_context>
+ /// - `suffix` contains glyphs for bytes `[byte_index, len)` with positions
+ /// shifted left so the first glyph starts at x=0, and byte indices rebased to 0.
+ /// - `font_size`, `ascent`, and `descent` are copied to both halves.
+ pub fn split_at(&self, byte_index: usize) -> (LineLayout, LineLayout) {
+ let x_offset = self.x_for_index(byte_index);
+
+ // Partition glyph runs. A single run may contribute glyphs to both halves.
+ let mut left_runs = Vec::new();
</code_context>
<issue_to_address>
**issue (bug_risk):** Passing a byte index greater than `self.len` makes `self.len - byte_index` underflow, and any glyph placed in the suffix can also make `g.index - byte_index` underflow. The public method consequently panics instead of rejecting or safely handling an out-of-range split.
**Triggers:** When a caller passes an out-of-range byte index.
**Suggested fix:** Assert or return an error for `byte_index > self.len` before calculating offsets, matching the bounds behavior of standard split APIs.
```suggestion
pub fn split_at(&self, byte_index: usize) -> (LineLayout, LineLayout) {
assert!(byte_index <= self.len);
```
</issue_to_address>
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+324
to
+333
| state.spring = state.config.step(state.spring, state.target, elapsed); | ||
| } | ||
| SpringPlayback::Paused | ||
| | SpringPlayback::Stopped | ||
| | SpringPlayback::Completed | ||
| | SpringPlayback::Cancelled => {} | ||
| } | ||
|
|
||
| state.config = self.config; | ||
| state.target = self.target; |
There was a problem hiding this comment.
issue (bug_risk)： 当弹簧目标、配置或播放模式发生变化时，该元素会先使用旧目标、旧配置和旧播放模式推进之前的状态，然后才赋予新值。因此，重新设定目标的弹簧会先朝着过时状态渲染一帧，播放模式的变化也会延迟一帧生效。
触发条件： 已存在的弹簧元素在两次布局之间被重新设定目标，或其播放模式/配置发生变化时。
建议修复： 在推进状态之前，将新的目标、配置和播放模式赋给状态，同时在运行中的弹簧重新设定目标时保留现有的位置和速度。
Original comment in English
issue (bug_risk): When a spring target, configuration, or playback mode changes, the element advances the previous state using the previous target/configuration/playback before assigning the new values. The retargeted spring therefore renders one frame toward stale state and a playback change takes effect one frame late.
Triggers: When an existing spring element is retargeted or its playback/configuration is changed between layout passes.
Suggested fix: Assign the new target, configuration, and playback to the state before advancing it, while preserving the existing position and velocity for running retargets.
Comment on lines
+1030
to
+1043
| let mut shutdown_completed = false; | ||
| if let Some(ref mut callback) = self.state.borrow_mut().callbacks.quit { | ||
| shutdown_completed = callback(); | ||
| } | ||
| log::logger().flush(); | ||
| if shutdown_completed { | ||
| std::process::exit(0); | ||
| } | ||
|
|
||
| // Shutdown couldn't run synchronously, since the AppCell is already borrowed. | ||
| // Windows may terminate the application as soon as we return from this handler, | ||
| // but posting WM_QUIT now may still let the message loop shut down first. | ||
| unsafe { PostQuitMessage(0) }; | ||
| Some(0) |
There was a problem hiding this comment.
issue (bug_risk)： 当退出回调无法借用 AppCell 时，handle_end_session 会发布 WM_QUIT，但不会重试该回调或运行 App::shutdown。这样消息循环可能在窗口、退出观察者和关闭清理仍处于活动状态时终止，因此在此更改要处理的这种确切借用状态下，Restart Manager 的关闭过程并不干净。
触发条件： WM_ENDSESSION 到达时，AppCell 已经被可变借用。
建议修复： 在允许进程退出之前，将退出回调推迟到主循环中并重试（或以其他方式调度 App::shutdown），而不是仅使用 PostQuitMessage 作为后备方案。
Original comment in English
issue (bug_risk): When the quit callback cannot borrow AppCell, handle_end_session posts WM_QUIT without retrying the callback or running App::shutdown. The message loop can then terminate with windows, quit observers, and shutdown cleanup still active, so Restart Manager shutdown is not clean in the exact borrowed-state case this change is intended to handle.
Triggers: When WM_ENDSESSION arrives while AppCell is already mutably borrowed.
Suggested fix: Defer and retry the quit callback on the main loop (or otherwise schedule App::shutdown) before allowing the process to exit, rather than using PostQuitMessage as the only fallback.
| /// - `suffix` contains glyphs for bytes `[byte_index, len)` with positions | ||
| /// shifted left so the first glyph starts at x=0, and byte indices rebased to 0. | ||
| /// - `font_size`, `ascent`, and `descent` are copied to both halves. | ||
| pub fn split_at(&self, byte_index: usize) -> (LineLayout, LineLayout) { |
There was a problem hiding this comment.
issue (bug_risk)： 传入大于 self.len 的字节索引会导致 self.len - byte_index 下溢，而放置在后缀中的任何字形也可能导致 g.index - byte_index 下溢。因此，该公共方法会直接 panic，而不是拒绝或安全处理超出范围的拆分。
触发条件： 调用方传入超出范围的字节索引时。
建议修复： 在计算偏移量之前，对 byte_index > self.len 进行断言或返回错误，使其行为符合标准拆分 API 的边界处理方式。
Suggested change
| pub fn split_at(&self, byte_index: usize) -> (LineLayout, LineLayout) { | |
| pub fn split_at(&self, byte_index: usize) -> (LineLayout, LineLayout) { | |
| assert!(byte_index <= self.len); |
Original comment in English
issue (bug_risk): Passing a byte index greater than self.len makes self.len - byte_index underflow, and any glyph placed in the suffix can also make g.index - byte_index underflow. The public method consequently panics instead of rejecting or safely handling an out-of-range split.
Triggers: When a caller passes an out-of-range byte index.
Suggested fix: Assert or return an error for byte_index > self.len before calculating offsets, matching the bounds behavior of standard split APIs.
Suggested change
| pub fn split_at(&self, byte_index: usize) -> (LineLayout, LineLayout) { | |
| pub fn split_at(&self, byte_index: usize) -> (LineLayout, LineLayout) { | |
| assert!(byte_index <= self.len); |
…-window throttle Port spring simulations (SpringConfig/SpringAnimation/with_spring), Animation::with_max_fps timer-driven redraws, and WindowOptions::inactive_frame_interval so overlay/HUD/daemon windows can keep a chosen inactive frame rate. Adapt the animation example. Skip synced repeating animations (no local consumer) and Zed inspector/telemetry. Zed-Origin: 8b1497dbd22fb06f5838a7c0b84a1e54fafa71bc Zed-Origin: dd04a229dd22700ff43815b8514453e197c333b7 Zed-Origin: 511ac170363776319b38cc0e9c047a06aa2e7541 Co-authored-by: freefcw <freefcw@gmail.com>
Move glyph partitioning onto LineLayout::split_at and expose paint/paint_background that take explicit decoration runs so callers can hold Arc<LineLayout> without a large ShapedLine. Zed-Origin: 2936989f1b7a15aaf7131b0a3c17961d706fdbf5 Co-authored-by: freefcw <freefcw@gmail.com>
Honor WM_QUERYENDSESSION and WM_ENDSESSION so installers can close the app cleanly. Platform::on_quit now reports whether shutdown ran synchronously; if the AppCell is borrowed, Windows posts WM_QUIT instead of exiting. Skip Zed crash-handler sidecar changes. Leave single-instance, autostart, and tray behavior unchanged. Zed-Origin: 7f2a2c3c3ee2f23f28772dee7661fb98d3910990 Co-authored-by: freefcw <freefcw@gmail.com>
cursor
Bot
force-pushed
the
cursor/zed-gpui-medium-ports-81de
branch
from
August 29, 2026 12:06
09e7cde to
ccb3a7f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three independent medium-priority Zed GPUI ports. Not a wholesale sync. One commit per topic.
Rebased onto
develop/0.9at2ce2baf01d0efea99b0efea1471248539cc8a232(merged #3).Topics
1. Spring animations + animation max FPS + configurable inactive-window FPS
Local commit:
23f861f1c6ad2bf4fac462da23c12c02f1377b9cSpringConfig/SpringAnimation/AnimationExt::with_spring(analytic step, retarget-preserving velocity).Animation::with_max_fpsschedules timer-driven redraws instead of every vsync.WindowOptions::inactive_frame_interval(default still ~30 FPS;Nonedisables the throttle). Useful for overlay/HUD/daemon windows.Zed-Origin:
8b1497dbd22fb06f5838a7c0b84a1e54fafa71bc(#62778)dd04a229dd22700ff43815b8514453e197c333b7(#62579)511ac170363776319b38cc0e9c047a06aa2e7541(#62628)Skipped hunks:
4ed3738/ #62332) — still no local consumer.debug_selectorkept only as a no-op in release).reduce_motionspring snap — no localApp::reduce_motion.scheduler::Instant/simulate_next_frameintegration tests (local animations usestd::time::Instant).2.
LineLayout::{split_at, paint, paint_background}Local commit:
04d80ead7776696126a080f624c4d26884112d86Callers can hold
Arc<LineLayout>plus their own decoration runs without a largeShapedLine. LocalShapedLinenever hadsplit_at, so there is no wrapper to delegate.Zed-Origin:
2936989f1b7a15aaf7131b0a3c17961d706fdbf5(#60831)3. Windows Restart Manager (
WM_QUERYENDSESSION/WM_ENDSESSION)Local commit:
ccb3a7fa0041b9d27d7ca3b978562443cb2c103aInstallers can close the app cleanly.
Platform::on_quitnow reports whether shutdown ran synchronously; ifAppCellis borrowed, Windows postsWM_QUITinstead of exiting.WM_GPUI_END_SESSIONuses the unusedWM_USER + 9slot (tray is+8, network is+10).Zed-Origin:
7f2a2c3c3ee2f23f28772dee7661fb98d3910990(#62987)Skipped hunks:
crates/crashes) — not present here.Protected files
Surgical edits only to
app.rs(on_quittry-borrow),platform.rs(WindowOptionsfield +on_quitsignature), andwindow.rs(inactive_frame_intervalplumbing). No wholesale overwrite of platform window/client orwgpu_renderer.rs. Local notifications/attention APIs unchanged.Verification
cargo test --locked -p adabraka-gpui-core --lib --features test-support— 234 passedcargo check --locked -p adabraka-gpui --example animationcargo check --locked -p adabraka-gpui-linux --features wayland,x11cargo check --locked -p adabraka-gpui-downstream-compatadabraka-gpui --testslink failed in this environment (missinglibstdc++/libxkbcommon); not a code regressioncfg(target_os = "windows"); not compiled on this Linux hostSourcery 摘要
在保留现有平台行为的同时，移植中优先级的 GPUI 动画、文本布局和 Windows 关闭功能。
新功能：
错误修复：
增强功能：
文档：
测试：
Original summary in English
Sourcery 摘要
移植中等优先级的 GPUI 动画、文本布局和 Windows 关闭功能，同时保留现有的平台行为。
新功能：
错误修复：
增强功能：
文档：
测试：
Original summary in English
Sourcery 摘要
移植中优先级的 GPUI 动画、文本布局和 Windows 关机功能，同时保留现有的平台行为。
新功能：
错误修复：
增强功能：
文档：
测试：
Original summary in English
Summary by Sourcery
Port medium-priority GPUI animation, text layout, and Windows shutdown capabilities while preserving existing platform behavior.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: