[Feature] Sync GPUI with current Zed upstream#4
Merged
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (1)
⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe PR synchronizes GPUI workspace dependencies, UI and accessibility APIs, profiling and benchmarking infrastructure, GPU recovery, native platform behavior, Linux Wayland support, examples, upstream documentation, and CI validation. ChangesGPUI synchronization
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/gpui_wgpu/src/wgpu_renderer.rs (1)
1400-1414: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winResetting
failed_frame_countmakes the panic path unreachable. After the 6th consecutive GPU error this branch zeros the counter, so it never reaches> 20and persistent failures will keep looping through recovery forever. Keep a separate recovery counter or stop clearing this one so the hard-failure escalation can still fire.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/gpui_wgpu/src/wgpu_renderer.rs` around lines 1400 - 1414, Update the GPU error handling around failed_frame_count so recovery after the sixth error does not reset the counter used by the >20 panic threshold. Preserve a separate recovery/reset mechanism if needed, while ensuring consecutive persistent failures eventually reach the panic path in the renderer’s error-handling logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 29-51: Consolidate the duplicated Linux apt dependency
installation used by the quality and linux-backend jobs into a shared composite
action or reusable workflow step. Move the package list from the install steps
into that shared definition, then update both jobs to invoke it so dependency
changes remain synchronized.
- Around line 140-141: Replace direct `${{ matrix.backend }}` interpolation in
the `cargo check` and `cargo test` run commands with a step-level environment
variable, then reference that variable in each command’s `--features` argument.
Preserve the existing matrix-driven behavior and feature combinations.
In `@crates/gpui_linux/src/linux.rs`:
- Around line 30-63: Update current_platform() to use the take-and-clear startup
activation token path instead of startup_activation_token_from_environment(),
ensuring XDG_ACTIVATION_TOKEN is removed before platform construction. Handle
the unsafe call according to the existing safety contract, and preserve passing
the retrieved token to current_platform_with_startup_activation_token.
In `@crates/gpui_util/src/lib.rs`:
- Around line 488-508: The TypeIdHasher::write implementation incorrectly
truncates 16-byte TypeId input to 8 bytes. Update it to validate the input is
exactly 16 bytes and fold both 64-bit halves into the resulting hash, or
otherwise explicitly handle the full 128-bit value while retaining a debug guard
for unexpected lengths.
In `@crates/gpui_wgpu/src/wgpu_context.rs`:
- Around line 313-338: Duplicate device-descriptor setup exists between
create_device and new_web. Extract the shared feature detection and
DeviceDescriptor construction into a helper such as build_device_descriptor,
then call it from both paths while preserving the dual_source_blending result
and existing limits, memory hints, tracing, and experimental feature settings.
In `@crates/gpui_wgpu/src/wgpu_renderer.rs`:
- Around line 2867-2869: Move the 350ms device-loss recovery delay out of the
draw callback in the needs_new_context path, preserving the retry behavior
without blocking draw or input processing. Update the recovery logic around
gpu_context and the draw flow to schedule the sleep/retry asynchronously, then
trigger redraw or context recreation when the backoff completes.
In `@crates/gpui/examples/view_example/editor_text.rs`:
- Around line 115-129: Clamp the offset returned by cursor_line_and_offset
before passing it to ShapedLine::x_for_index in both cursor rendering branches.
After clamping cursor_line to the available lines, clamp offset_in_line to
lines[cursor_line].len (or the equivalent line-length accessor) so inconsistent
cursor and content state cannot produce an out-of-range index.
In `@crates/gpui/examples/view_example/example_editor.rs`:
- Around line 192-203: Clamp or otherwise normalize self.cursor against the
current value length and UTF-8 character boundaries before calling
String::insert in insert_newline. Use the validated cursor for insertion and
update self.cursor and selected_range from that same position, preserving safe
behavior when external updates leave the stored cursor stale.
In `@crates/gpui/src/style.rs`:
- Line 1578: In the struct update using `..shadow`, avoid moving the `BoxShadow`
before its later uses: clone it with `..shadow.clone()`, or alternatively derive
`Copy` for `BoxShadow` if appropriate. Update the surrounding shadow
construction code accordingly.
---
Outside diff comments:
In `@crates/gpui_wgpu/src/wgpu_renderer.rs`:
- Around line 1400-1414: Update the GPU error handling around failed_frame_count
so recovery after the sixth error does not reset the counter used by the >20
panic threshold. Preserve a separate recovery/reset mechanism if needed, while
ensuring consecutive persistent failures eventually reach the panic path in the
renderer’s error-handling logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b81148d3-7930-4617-a843-cbb671aca0b8
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockcrates/gpui_windows/src/shaders.hlslis excluded by!**/*.hlsl
📒 Files selected for processing (131)
.gitattributes.github/workflows/ci.yml.tasque/.gitignore.tasque/config.json.tasque/events.jsonlCargo.tomlREADME.mdUPSTREAM.mdcrates/collections/Cargo.tomlcrates/collections/src/collections.rscrates/gpui/Cargo.tomlcrates/gpui/benches/scene_construction.rscrates/gpui/build.rscrates/gpui/examples/a11y.rscrates/gpui/examples/opacity.rscrates/gpui/examples/shadow.rscrates/gpui/examples/view_example/editor_text.rscrates/gpui/examples/view_example/example_editor.rscrates/gpui/examples/view_example/example_input.rscrates/gpui/examples/view_example/example_tests.rscrates/gpui/examples/view_example/example_text_area.rscrates/gpui/examples/view_example/view_example_main.rscrates/gpui/examples/window_shadow.rscrates/gpui/resources/windows/gpui.manifest.xmlcrates/gpui/src/_accessibility.rscrates/gpui/src/action.rscrates/gpui/src/app.rscrates/gpui/src/app/async_context.rscrates/gpui/src/app/bench_context.rscrates/gpui/src/app/test_context.rscrates/gpui/src/asset_cache.rscrates/gpui/src/color.rscrates/gpui/src/element.rscrates/gpui/src/elements/animation.rscrates/gpui/src/elements/div.rscrates/gpui/src/elements/list.rscrates/gpui/src/elements/text.rscrates/gpui/src/elements/uniform_list.rscrates/gpui/src/geometry.rscrates/gpui/src/gpui.rscrates/gpui/src/inspector.rscrates/gpui/src/keymap.rscrates/gpui/src/platform.rscrates/gpui/src/platform/bench_dispatcher.rscrates/gpui/src/platform/popup.rscrates/gpui/src/platform/test/dispatcher.rscrates/gpui/src/platform/test/platform.rscrates/gpui/src/platform/test/window.rscrates/gpui/src/platform_scheduler.rscrates/gpui/src/profiler.rscrates/gpui/src/profiler/actions.rscrates/gpui/src/queue.rscrates/gpui/src/scene.rscrates/gpui/src/style.rscrates/gpui/src/styled.rscrates/gpui/src/svg_renderer.rscrates/gpui/src/text_system.rscrates/gpui/src/text_system/line_wrapper.rscrates/gpui/src/view.rscrates/gpui/src/view_tests.rscrates/gpui/src/window.rscrates/gpui/src/window/a11y.rscrates/gpui/src/window/a11y/builder.rscrates/gpui/src/window/a11y/builder_tests.rscrates/gpui_linux/Cargo.tomlcrates/gpui_linux/src/gpui_linux.rscrates/gpui_linux/src/linux.rscrates/gpui_linux/src/linux/dispatcher.rscrates/gpui_linux/src/linux/headless.rscrates/gpui_linux/src/linux/headless/client.rscrates/gpui_linux/src/linux/headless/window.rscrates/gpui_linux/src/linux/platform.rscrates/gpui_linux/src/linux/wayland.rscrates/gpui_linux/src/linux/wayland/client.rscrates/gpui_linux/src/linux/wayland/clipboard.rscrates/gpui_linux/src/linux/wayland/cursor.rscrates/gpui_linux/src/linux/wayland/popup.rscrates/gpui_linux/src/linux/wayland/window.rscrates/gpui_linux/src/linux/x11/client.rscrates/gpui_linux/src/linux/x11/clipboard.rscrates/gpui_linux/src/linux/x11/window.rscrates/gpui_macos/Cargo.tomlcrates/gpui_macos/src/dispatcher.rscrates/gpui_macos/src/display_link.rscrates/gpui_macos/src/events.rscrates/gpui_macos/src/metal_atlas.rscrates/gpui_macos/src/metal_renderer.rscrates/gpui_macos/src/open_type.rscrates/gpui_macos/src/platform.rscrates/gpui_macos/src/text_system.rscrates/gpui_macos/src/window.rscrates/gpui_macros/src/bench.rscrates/gpui_macros/src/derive_into_element.rscrates/gpui_macros/src/gpui_macros.rscrates/gpui_macros/src/styles.rscrates/gpui_platform/src/gpui_platform.rscrates/gpui_tokio/Cargo.tomlcrates/gpui_tokio/src/gpui_tokio.rscrates/gpui_util/Cargo.tomlcrates/gpui_util/src/lib.rscrates/gpui_web/Cargo.tomlcrates/gpui_web/assets/fonts/ibm-plex-sans/license.txtcrates/gpui_web/assets/fonts/lilex/OFL.txtcrates/gpui_web/src/dispatcher.rscrates/gpui_web/src/platform.rscrates/gpui_wgpu/Cargo.tomlcrates/gpui_wgpu/benches/layout_line.rscrates/gpui_wgpu/src/cosmic_text_system.rscrates/gpui_wgpu/src/shaders.wgslcrates/gpui_wgpu/src/wgpu_atlas.rscrates/gpui_wgpu/src/wgpu_context.rscrates/gpui_wgpu/src/wgpu_renderer.rscrates/gpui_windows/Cargo.tomlcrates/gpui_windows/src/direct_manipulation.rscrates/gpui_windows/src/direct_write.rscrates/gpui_windows/src/directx_atlas.rscrates/gpui_windows/src/directx_devices.rscrates/gpui_windows/src/directx_renderer.rscrates/gpui_windows/src/dispatcher.rscrates/gpui_windows/src/display.rscrates/gpui_windows/src/events.rscrates/gpui_windows/src/platform.rscrates/gpui_windows/src/system_settings.rscrates/gpui_windows/src/util.rscrates/gpui_windows/src/vsync.rscrates/gpui_windows/src/window.rscrates/scheduler/src/executor.rscrates/scheduler/src/scheduler.rscrates/scheduler/src/test_scheduler.rscrates/scheduler/src/tests.rsrust-toolchain.toml
💤 Files with no reviewable changes (2)
- crates/gpui/src/platform/test/dispatcher.rs
- crates/gpui_web/src/dispatcher.rs
Owner
Author
|
Follow-up on the review-body-only WGPU finding:
All nine inline review threads have received finding-specific replies and are resolved. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/gpui_wgpu/src/wgpu_renderer.rs (1)
2929-2940: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winPreserve the failure streak when replacing the renderer.
from_surface_internalinitializes a newFrameFailureStreak, and*self = recovereddiscards the prior count. Repeated device-loss recoveries therefore restart at zero and never reach the >20 failure escalation.Proposed fix
let mut recovered = Self::from_surface_internal( context, surface, config, Some(Rc::clone(&gpu_context)), self.compositor_gpu, Some(atlas), Some(handles), )?; recovered.desired.is_bgr = is_bgr; + recovered.frame_failure_streak = std::mem::take(&mut self.frame_failure_streak); recovered.needs_redraw = true; *self = recovered;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/gpui_wgpu/src/wgpu_renderer.rs` around lines 2929 - 2940, Preserve the existing failure streak when replacing the renderer in the recovery path around `from_surface_internal` and `*self = recovered`. Save the current `FrameFailureStreak` before constructing `recovered`, then restore or assign it to the recovered renderer after initialization so repeated device-loss recoveries retain the accumulated count and escalation threshold.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crates/gpui_wgpu/src/wgpu_renderer.rs`:
- Around line 2929-2940: Preserve the existing failure streak when replacing the
renderer in the recovery path around `from_surface_internal` and `*self =
recovered`. Save the current `FrameFailureStreak` before constructing
`recovered`, then restore or assign it to the recovered renderer after
initialization so repeated device-loss recoveries retain the accumulated count
and escalation threshold.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7f59e25e-6eca-4e8a-ab24-080a7ba22f4a
📒 Files selected for processing (7)
.github/actions/install-linux-dependencies/action.yml.github/workflows/ci.ymlcrates/gpui/examples/view_example/editor_text.rscrates/gpui/examples/view_example/example_editor.rscrates/gpui/examples/view_example/example_tests.rscrates/gpui_linux/src/linux.rscrates/gpui_wgpu/src/wgpu_renderer.rs
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
Owner
Author
|
Addressed review-body finding
Validation: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/gpui_windows/src/events.rs`:
- Around line 1748-1753: Improve the table-driven assertions in the test loop
over `cases`: replace the boolean `assert!` in `window_control_hit_target` with
`assert_eq!`, and include `area`, capability parameters, and the computed versus
expected values in the failure diagnostics so the failing case is identifiable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: db459d49-6920-4493-b878-525cf7e4575e
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
Cargo.tomlcrates/gpui_wgpu/src/wgpu_renderer.rscrates/gpui_windows/src/events.rs
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.
Summary
Sync the standalone GPUI fork with the current GPUI implementation from
zed-industries/zed, while preserving fork-specific overlay windows, native background blur, rounded blur, animated blur cards, and standalone packaging behavior.This also applies the validated CodeRabbit and independent Sol review fixes discovered after the sync.
Changes
README.mdandUPSTREAM.md.Testing
cargo fmt --all -- --checkcargo check --locked --workspace --all-targetscargo clippy --locked --workspace --all-targets --all-features -- --deny warningscargo test --locked -p scheduler— 36 passedcargo test --locked -p gpui --features bench,profiler --lib— 263 passedcargo test --locked -p gpui_wgpu --lib— 29 passedcargo test --locked --release -p gpui --features test-support duplicate_id— 3 passedReview notes
gpui_linux::current_platformno longer mutates the process environment. Embedders that can guarantee single-threaded startup may explicitly take and pass the Wayland activation token through the documented API.