[BREAKING] Sync upstream GPUI changes#2
Merged
Conversation
added 2 commits
May 14, 2026 15:31
BREAKING CHANGE: CursorStyle::None and cursor_none() were removed; use CursorHideMode/platform cursor hide APIs instead.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds AccessKit-based accessibility, platform cursor hide/restore APIs and DisplayId u64 migration; implements per-window a11y manager and node builder; updates window/input flows, list remeasurement and scrollbar queries, scheduler/executor LocalExecutor and dedicated sessions, inset shadow support (data, paint, shaders), and supporting tests/examples/Cargo edits. ChangesPlatform Cursor Visibility & DisplayId migration
List Scrolling Enhancements
Accessibility / AccessKit integration
Rendering / Shadow inset support
Scheduler / Executor refactor and LocalExecutor
Supporting Changes
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~45 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Pull request overview
This PR syncs the GPUI crate and its platform backends with upstream (Zed) changes, including a breaking cursor-hiding API migration (removing CursorStyle::None / cursor_none() in favor of CursorHideMode + platform cursor hide APIs) and substantial list scrolling/remeasurement behavior updates.
Changes:
- Introduces
CursorHideModeand platform APIs to hide/restore the cursor based on typing/action consumption; removesCursorStyle::Nonesupport. - Updates list and uniform list logic to better preserve scroll position during remeasurement, improve follow-tail behavior, and stabilize scrollbar dragging semantics (plus adds/extends tests).
- Adjusts cross-platform display identifiers (
DisplayId) tou64and refreshes Windows/macOS/Linux/Web implementations accordingly.
Reviewed changes
Copilot reviewed 27 out of 28 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| crates/gpui/src/window.rs | Adds forced refresh on recovery render, fixes input-handler slot preservation, and integrates cursor auto-hide triggers. |
| crates/gpui/src/prelude.rs | Re-exports TaskExt and reorganizes prelude exports. |
| crates/gpui/src/platform/visual_test.rs | Implements new cursor hide/visibility platform APIs for visual tests. |
| crates/gpui/src/platform/test/platform.rs | Stubs new cursor hide/visibility platform APIs for unit tests. |
| crates/gpui/src/platform.rs | Extends Platform with cursor hide/visibility APIs; changes DisplayId to u64; removes CursorStyle::None. |
| crates/gpui/src/executor.rs | Switches to re-exporting scheduler Task and moves detach logging helpers to a TaskExt trait. |
| crates/gpui/src/elements/uniform_list.rs | Adds is_scrolled_to_end helper to the uniform list scroll handle. |
| crates/gpui/src/elements/list.rs | Major scroll anchoring + remeasurement improvements, scrollbar drag behavior fixes, follow-tail behavior changes, and extensive new tests. |
| crates/gpui/src/app.rs | Adds CursorHideMode policy + app APIs, and exposes platform cursor visibility. |
| crates/gpui/Cargo.toml | Feature/dependency cleanup and test-support dependency feature adjustments. |
| crates/gpui_windows/src/window.rs | Plumbs shared cursor visibility state into Windows window state creation. |
| crates/gpui_windows/src/util.rs | Fixes ResultExt import and removes handling for CursorStyle::None. |
| crates/gpui_windows/src/platform.rs | Implements platform cursor hide/visibility behavior and wires cursor visibility into window creation. |
| crates/gpui_windows/src/events.rs | Restores cursor on mouse move/non-client move, clears hidden state on leave/deactivate, and respects cursor visibility in WM_SETCURSOR. |
| crates/gpui_windows/src/display.rs | Reworks WindowsDisplay to use DisplayId as an HMONITOR-backed identifier and simplifies display enumeration. |
| crates/gpui_web/src/platform.rs | Adds cursor hide/restore behavior via CSS and event listeners; tracks cursor visibility and last cursor style. |
| crates/gpui_macros/src/styles.rs | Removes the cursor_none() style helper in line with the breaking API change. |
| crates/gpui_macos/src/window.rs | Mirrors AppKit cursor visibility restoration on mouse/activation events via a shared atomic flag. |
| crates/gpui_macos/src/platform.rs | Implements platform cursor hide/visibility and shares cursor visibility state with windows. |
| crates/gpui_macos/src/display.rs | Updates macOS display IDs to u64. |
| crates/gpui_linux/src/linux/x11/window.rs | Updates X11 display ID conversions for u64 DisplayId. |
| crates/gpui_linux/src/linux/x11/display.rs | Updates X11 display IDs to u64. |
| crates/gpui_linux/src/linux/x11/client.rs | Implements X11 cursor hide/restore using an invisible cursor and tracks hidden window state. |
| crates/gpui_linux/src/linux/wayland/display.rs | Updates Wayland display IDs to u64. |
| crates/gpui_linux/src/linux/wayland/client.rs | Implements Wayland cursor hide/restore and updates display ID matching for u64. |
| crates/gpui_linux/src/linux/wayland.rs | Removes CursorStyle::None handling in cursor-shape mapping. |
| crates/gpui_linux/src/linux/platform.rs | Adds default LinuxClient cursor hide/visibility methods and forwards them through LinuxPlatform. |
| .gitignore | Ignores .pi-lens/. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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_linux/src/linux/wayland/client.rs`:
- Around line 432-457: The method restore_cursor_after_hide currently clears the
hidden marker via cursor_hidden_window.take() before verifying it can actually
restore the cursor; change the flow so the marker is only removed after a
successful restore. Concretely, do not call cursor_hidden_window.take() at the
top — first check cursor_style, serial_tracker, cursor_shape_device or
mouse_focused_window and wl_pointer as currently done, and only call
cursor_hidden_window.take() (or set it None) after calling
cursor_shape_device.set_shape(...) or after successfully issuing the
wl_pointer.set_cursor(...) path; if any precondition fails, return without
clearing cursor_hidden_window so later enter/motion events can retry. Ensure you
update restore_cursor_after_hide to reference the same symbols
(cursor_hidden_window, cursor_style, cursor_shape_device, mouse_focused_window,
wl_pointer, serial_tracker, to_shape) when implementing this change.
In `@crates/gpui_linux/src/linux/x11/window.rs`:
- Around line 426-430: The code converts params.display_id into an X11 screen
index and calls find_visuals without validating that index, which can
out-of-bounds setup().roots and crash; update the logic around display_id /
x_main_screen_index to first query the XCB setup (setup().roots.len()) and
validate that the converted index (from DisplayId via u64::from(did) as usize)
is within bounds, returning an Err (or propagating a meaningful error) when
invalid instead of indexing, then pass the validated index into find_visuals;
reference symbols: params.display_id, x_main_screen_index, find_visuals, and
setup().roots[screen_index].
In `@crates/gpui/src/elements/list.rs`:
- Around line 395-413: The current proportional remeasure code falls back
unconditionally to state.pending_scroll.clone() when item.size() is None, which
can reapply a stale PendingScroll::Absolute using the wrong anchor; change the
or_else fallback so it only returns the existing pending scroll if it is a
PendingScroll::Proportional (and ideally matches the same item_ix), otherwise
return None; locate the closure around cursor.item().and_then(...) and adjust
the or_else to filter state.pending_scroll to only Proportional variants (or
None) instead of cloning any pending scroll.
In `@crates/gpui/src/executor.rs`:
- Around line 37-51: The trait docs claim TaskExt is "automatically implemented
for all Task<Result<T, E>>" but the impl for TaskExt<T, E> on Task<Result<T, E>>
has type bounds (T: 'static, E: 'static + Display + Debug), so update the
documentation to reflect those bounds; specifically, change the doc on the
TaskExt trait (and/or the method docs for detach_and_log_err and
detach_and_log_err_with_backtrace) to state that the blanket impl applies only
when T: 'static and E: 'static + std::fmt::Display + std::fmt::Debug,
referencing the TaskExt trait and the impl block to keep the docs consistent
with the actual implementation.
🪄 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: 580d8bf7-82a8-444a-99f5-40ad5c3880d9
📒 Files selected for processing (28)
.gitignorecrates/gpui/Cargo.tomlcrates/gpui/src/app.rscrates/gpui/src/elements/list.rscrates/gpui/src/elements/uniform_list.rscrates/gpui/src/executor.rscrates/gpui/src/platform.rscrates/gpui/src/platform/test/platform.rscrates/gpui/src/platform/visual_test.rscrates/gpui/src/prelude.rscrates/gpui/src/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/display.rscrates/gpui_linux/src/linux/x11/client.rscrates/gpui_linux/src/linux/x11/display.rscrates/gpui_linux/src/linux/x11/window.rscrates/gpui_macos/src/display.rscrates/gpui_macos/src/platform.rscrates/gpui_macos/src/window.rscrates/gpui_macros/src/styles.rscrates/gpui_web/src/platform.rscrates/gpui_windows/src/display.rscrates/gpui_windows/src/events.rscrates/gpui_windows/src/platform.rscrates/gpui_windows/src/util.rscrates/gpui_windows/src/window.rs
💤 Files with no reviewable changes (2)
- crates/gpui_linux/src/linux/wayland.rs
- crates/gpui_macros/src/styles.rs
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_linux/src/linux/x11/client.rs`:
- Around line 2088-2116: The function restore_cursor_after_hide currently calls
cursor_hidden_window.take() immediately which clears the retry state before
restoration is confirmed; change the logic in restore_cursor_after_hide to
read/clone/borrow the Option (e.g., use as_ref() or clone the Window ID) instead
of taking it, perform get_cursor_icon(style) and the
check_reply(self.xcb_connection.change_window_attributes(...)) and flush(), and
only call self.cursor_hidden_window.take() or set it to None after those
operations succeed; reference restore_cursor_after_hide, cursor_hidden_window,
get_cursor_icon, change_window_attributes, and check_reply when making the
change so the hidden-cursor state is preserved until the restore is proven
successful.
🪄 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: 6a0db94c-4498-4d76-8ff7-1b7b35bafae2
📒 Files selected for processing (6)
crates/gpui/src/elements/list.rscrates/gpui/src/executor.rscrates/gpui/src/prelude.rscrates/gpui_linux/src/linux/wayland/client.rscrates/gpui_linux/src/linux/x11/client.rscrates/gpui_linux/src/linux/x11/window.rs
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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_linux/src/linux/x11/client.rs`:
- Around line 1688-1690: is_cursor_visible currently returns true iff
cursor_hidden_window is None, which can be stale after XinputLeave; update
is_cursor_visible to derive visibility from whether the pointer is still over
the window that was given the invisible cursor by comparing mouse_focused_window
== cursor_hidden_window (i.e., return true when either cursor_hidden_window is
None or mouse_focused_window != cursor_hidden_window), or alternatively clear
cursor_hidden_window when handling XinputLeave; locate and modify the
is_cursor_visible method and/or the XinputLeave handler so the visibility state
reflects mouse_focused_window and cursor_hidden_window equality rather than
relying only on cursor_hidden_window being None.
- Around line 2062-2085: hide_cursor_until_mouse_moves currently sets
cursor_hidden_window even if the X11 change_window_attributes or subsequent
flush fails; update hide_cursor_until_mouse_moves so it only assigns
self.cursor_hidden_window = Some(focused_window) after the
change_window_attributes request (via check_reply) and the
self.xcb_connection.flush() both succeed — e.g., inspect the Result returned by
check_reply(...) and the flush call (or propagate errors) and only set
cursor_hidden_window when both are Ok, so is_cursor_visible() and
set_cursor_style() reflect the true server state.
🪄 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: 858553cb-fcda-4aa4-a9fa-3ee3a226670b
📒 Files selected for processing (2)
crates/gpui_linux/src/linux/x11/client.rscrates/gpui_linux/src/linux/x11/window.rs
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
crates/gpui/src/platform.rs (1)
208-214:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBreaking change: trait methods lack default implementations.
The
hide_cursor_until_mouse_moves()andis_cursor_visible()methods are added to thePlatformtrait without default implementations, requiring all platform backends to implement them. This is a breaking change for any custom platform implementations.Consider adding no-op defaults to ease migration:
🔧 Suggested defaults
+ /// Hides the mouse cursor until the user moves the mouse over one of + /// this application's windows. + fn hide_cursor_until_mouse_moves(&self) {} + + /// Returns whether the mouse cursor is currently visible. + fn is_cursor_visible(&self) -> bool { + true + }🤖 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/src/platform.rs` around lines 208 - 214, The new trait methods hide_cursor_until_mouse_moves() and is_cursor_visible() are breaking existing Platform implementations; add default implementations on the Platform trait so backends aren’t forced to implement them: make hide_cursor_until_mouse_moves() a no-op default and is_cursor_visible() return a sensible default (e.g., true or false as appropriate for your app), by providing default method bodies on the Platform trait for hide_cursor_until_mouse_moves and is_cursor_visible so existing platform backends continue to compile.crates/gpui_windows/src/window.rs (1)
472-478:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't fall back to the primary monitor when
display_idis invalid.This path treats “unknown display id” the same as “no display requested”, so a bad/stale
DisplayIdsilently opens the window on the primary monitor. That hides the error and breaks explicit monitor placement.Suggested fix
- let display = if let Some(display_id) = params.display_id { - WindowsDisplay::new(display_id) - } else { - None - } - .or_else(WindowsDisplay::primary_monitor) - .context("failed to find any monitor")?; + let display = match params.display_id { + Some(display_id) => WindowsDisplay::new(display_id).with_context(|| { + format!("failed to find monitor for display id {}", u64::from(display_id)) + })?, + None => WindowsDisplay::primary_monitor().context("failed to find any monitor")?, + };🤖 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_windows/src/window.rs` around lines 472 - 478, The code currently treats an invalid/stale params.display_id the same as "no display requested" because WindowsDisplay::new(display_id) returning None falls through to WindowsDisplay::primary_monitor; instead, if params.display_id is Some but WindowsDisplay::new returns None you must return an error. Change the logic around params.display_id and WindowsDisplay::new so that: when params.display_id is Some, attempt WindowsDisplay::new(display_id) and if it yields None return an error (use the same .context(...) or a specific message), and only call WindowsDisplay::primary_monitor when params.display_id is None; adjust the code that builds `display` (the match/if-let that currently uses .or_else) to implement this explicit branching.crates/gpui_linux/src/linux/wayland/client.rs (1)
791-798:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReturn an error when the requested Wayland display is missing.
This lookup silently degrades an unknown
DisplayIdtoNone, so the window opens on the compositor-default output instead of the caller-selected one. That makes restored/stale monitor ids land on the wrong screen rather than failing fast, unlike the X11 path in this PR.Suggested fix
- let target_output = params.display_id.and_then(|display_id| { - let target_protocol_id: u64 = display_id.into(); - state - .wl_outputs - .iter() - .find(|(id, _)| id.protocol_id() as u64 == target_protocol_id) - .map(|(_, output)| output.clone()) - }); + let target_output = match params.display_id { + Some(display_id) => { + let target_protocol_id: u64 = display_id.into(); + Some( + state + .wl_outputs + .iter() + .find(|(id, _)| id.protocol_id() as u64 == target_protocol_id) + .map(|(_, output)| output.clone()) + .ok_or_else(|| { + anyhow::anyhow!( + "no Wayland output found for display id {}", + target_protocol_id + ) + })?, + ) + } + None => None, + };🤖 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_linux/src/linux/wayland/client.rs` around lines 791 - 798, The code currently silently treats an unknown params.display_id as None; change the target_output computation to explicitly handle Some(display_id) by converting it to target_protocol_id, searching state.wl_outputs (same id.protocol_id() as u64 comparison) and if no matching output is found return an Err describing the missing Wayland display (include the DisplayId value) instead of mapping to None; keep the existing behavior for params.display_id == None (i.e., leave target_output as None). Use the enclosing function's Result error path (add or reuse an appropriate error variant) so callers fail fast when a requested display is absent.crates/gpui_windows/src/events.rs (1)
1096-1120:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep
handle_cursor_changedconsistent with the hidden-cursor gate.This
WM_SETCURSORchange hides the cursor correctly, but the window can still unhide it throughhandle_cursor_changed, which callsSetCursorwithout checkingstate.cursor_visible. That breaks the new “hide until mouse moves” behavior whenever the cursor handle changes while the cursor is supposed to stay hidden.Proposed fix
fn handle_cursor_changed(&self, lparam: LPARAM) -> Option<isize> { let had_cursor = self.state.current_cursor.get().is_some(); self.state.current_cursor.set(if lparam.0 == 0 { None } else { Some(HCURSOR(lparam.0 as _)) }); if had_cursor != self.state.current_cursor.get().is_some() { - unsafe { SetCursor(self.state.current_cursor.get()) }; + let cursor = if self.state.cursor_visible.load(Ordering::Relaxed) { + self.state.current_cursor.get() + } else { + None + }; + unsafe { SetCursor(cursor) }; } Some(0) }🤖 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_windows/src/events.rs` around lines 1096 - 1120, handle_cursor_changed currently calls SetCursor unconditionally which can re-show the cursor despite the new "hide until mouse moves" behavior; update handle_cursor_changed to mirror the gate in handle_set_cursor by checking state.cursor_visible (and using state.current_cursor when visible, otherwise using None) before calling SetCursor so SetCursor is only invoked when cursor_visible is true. Reference: handle_cursor_changed, handle_set_cursor, state.cursor_visible, state.current_cursor, SetCursor.
♻️ Duplicate comments (1)
crates/gpui_linux/src/linux/wayland/client.rs (1)
443-457:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't clear the hidden-cursor marker on restore failures.
Lines 448 and 456 reintroduce the earlier regression: when focus or
wl_pointeris temporarily unavailable, later enter/motion events stop retrying and the cursor can remain invisible for the rest of the session.Suggested fix
let Some(focused_window) = self.mouse_focused_window.clone() else { log::warn!( "wayland: no focused surface to restore cursor style {:?} after hide; cursor may stay invisible", style ); - self.cursor_hidden_window = None; return; }; let Some(wl_pointer) = self.wl_pointer.clone() else { log::warn!( "wayland: no wl_pointer to restore cursor style {:?} after hide; cursor may stay invisible", style ); - self.cursor_hidden_window = None; return; };🤖 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_linux/src/linux/wayland/client.rs` around lines 443 - 457, The function that restores cursor style currently clears self.cursor_hidden_window in the early-return branches when mouse_focused_window or wl_pointer is missing (the blocks referencing self.mouse_focused_window and self.wl_pointer), which stops future enter/motion events from retrying and can leave the cursor invisible; remove the lines that set self.cursor_hidden_window = None from those early-return branches so the hidden-cursor marker remains until a successful restore, and ensure you still clear self.cursor_hidden_window only after the cursor style has been successfully restored.
🤖 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_linux/src/linux/wayland/window.rs`:
- Around line 1478-1490: When initializing the AccessKit adapter in a11y_init,
seed it with the current window focus so it doesn't stay inactive if the window
is already focused; after creating and storing the Adapter
(self.borrow_mut().accesskit_adapter = Some(adapter)), query the current focus
state (the same source used by set_focused) and call
adapter.update_window_focus_state(...) with that boolean before returning so
subsequent a11y_tree_update calls see the correct active state.
In `@crates/gpui_linux/src/linux/x11/window.rs`:
- Around line 1872-1885: a11y_init creates and stores the
accesskit_unix::Adapter but never syncs its initial focus; after constructing
the adapter in a11y_init, read the current X11WindowState.active (via
self.0.state.borrow().active) and call the adapter's focus method (e.g.,
set_active or equivalent) to set the adapter's initial focus to that boolean
before storing it in self.0.state.borrow_mut().accesskit_adapter; this ensures
the AccessKit adapter starts with the correct focus state and matches set_active
behavior on subsequent changes.
In `@crates/gpui_wgpu/benches/layout_line.rs`:
- Around line 79-81: The benchmark "fallback_chain_ascii_text" currently uses
ASCII-only `text` so the fallback chain in `runs_with_fallback` is never
actually hit; add a new benchmark (e.g., `fallback_chain_mixed_text`) using a
non-ASCII/mixed string (Latin+CJK or other characters not covered by Lilex) —
create a `text_mixed` variable with those characters and add a
`group.bench_function("fallback_chain_mixed_text", |b| { b.iter(||
system.layout_line(&text_mixed, px(14.0), &runs_with_fallback)) });` alongside
the existing `fallback_chain_ascii_text` to measure real fallback behavior when
`system.layout_line` must use the fallback fonts.
In `@crates/gpui_wgpu/src/cosmic_text_system.rs`:
- Around line 709-763: compute_run_spans currently does expensive grapheme
iteration for every run when fallback_chain is non-empty; add cheap early checks
and fast paths to avoid grapheme segmentation when unnecessary: first, before
grapheme_indices, scan run_text by char (or bytes for ASCII fast-path) to see if
every character is covered by the primary font using the provided covers
callback—if so, push a single RunSpan and return; second, add an early-exit
check that scans run_text to detect whether any fallback in fallback_chain would
ever be needed (i.e., any char not covered by primary but covered by some
fallback via pick_covering_slot) and if none are needed, return a single span;
third, implement an ASCII fast-path by checking bytes (e.g., all bytes < 128)
and using byte-wise iteration to avoid grapheme segmentation; keep fallbacks to
full grapheme iteration only when those cheap checks indicate it’s necessary,
and keep references to compute_run_spans, pick_covering_slot, and slot_font_id
to locate the changes.
- Around line 243-275: The recursive calls from load_family when building
user_fallback_chain can recurse unbounded via fallback_list and cause stack
overflow; modify load_family to accept a depth parameter and a visited set
(e.g., HashSet<SharedString> or FontKey) and enforce a max depth constant (e.g.,
MAX_FALLBACK_DEPTH) and cycle detection by checking/adding the current family to
visited before recursing, returning early (or skipping) when depth exceeds the
limit or a cycle is detected; update callers that currently call load_family
(the code that builds user_fallback_chain, and any other call sites) to pass
depth = 0 and an empty visited set, and ensure font_ids_by_family_cache and
FontKey logic remains used to short-circuit loads when cached.
In `@crates/gpui_wgpu/src/wgpu_renderer.rs`:
- Around line 314-317: The present_mode selection currently filters
preferred_present_mode against surface_caps.present_modes which wrongly excludes
the convenience variants wgpu::PresentMode::AutoVsync and AutoNoVsync; update
the filter logic so that preferred_present_mode is accepted if it is either
AutoVsync or AutoNoVsync OR if surface_caps.present_modes.contains(mode),
otherwise fall back to wgpu::PresentMode::Fifo. Modify the code that references
preferred_present_mode and SurfaceCapabilities::present_modes to include this OR
check so callers requesting AutoVsync/AutoNoVsync are honored.
In `@crates/gpui/src/elements/text.rs`:
- Around line 92-100: The macro text! currently auto-generates a constant
ElementId using file!/line!/column! and
__hash_text_macro_location_unstable_do_not_use, which produces identical IDs
when the macro is expanded repeatedly (e.g. in loops); change the default arm
($text:expr) so it does NOT emit a constant id: replace the current default
expansion that computes const ID/HASH and calls
Text::new(ElementId::Integer(HASH), ...) with a safer alternative—either remove
the default and force callers to use the explicit (id = $id, $text) form or make
the default arm emit a compile_error! instructing the user to supply an explicit
id; update the macro_rules! text accordingly so only the explicit id path
constructs a Text via $crate::Text::new and eliminate use of file!/line!/column!
and __hash_text_macro_location_unstable_do_not_use in the default case.
In `@crates/gpui/src/text_system.rs`:
- Around line 701-721: TextSystem and WindowTextSystem both implement
layout_width and em_layout_width which duplicates logic but call different
backends (platform_text_system.layout_line vs line_layout_cache.layout_line);
pick the preferred implementation (suggest keeping WindowTextSystem's use of
line_layout_cache), remove or deprecate the duplicate in TextSystem, and either
(a) have TextSystem.delegate to WindowTextSystem by adding a small wrapper that
calls the WindowTextSystem methods, or (b) extract a shared helper (e.g., a
private fn compute_layout_width(buffer, font_size, &[FontRun], backend)) and
have both TextSystem and WindowTextSystem call it; update all callers to use the
single canonical implementation and add a comment documenting why the other path
was removed or deprecated.
In `@crates/gpui/src/text_system/line_wrapper.rs`:
- Around line 461-495: The binary search in truncate_wrapped_line_start
repeatedly allocates formatted strings (format!("{truncation_affix}{}", ...))
inside the loop; change it to avoid allocations by caching
truncation_affix.len() and calling wrapped_line_count on the slice plus a
virtual prefix instead of formatting each candidate. Concretely, add or overload
a helper (e.g., wrapped_line_count_with_prefix or extend wrapped_line_count to
accept an optional prefix_len/&str) and use &text[candidate..] with the cached
prefix length during the binary search, then only build the final SharedString
once (as you already do at the end) and call update_runs_after_truncation and
TruncateFrom::Start unchanged.
In `@crates/gpui/src/window.rs`:
- Around line 5411-5429: The Click accessibility fallback currently synthesizes
mouse events at bounds.center() (in the accesskit::Action::Click branch) which
relies on geometry/hit-testing; instead locate the target node via
request.target_node in self.a11y and invoke the node's semantic/action handler
directly (do not create PlatformInput::MouseDown/MouseUp and do not use
self.dispatch_event for a pointer click). Replace the synthetic-pointer logic
with a direct call to the node's accessibility activation API or internal action
handler (e.g., a method like perform_action_on_node / activate_node /
run_node_action) so the action is dispatched to the semantic element represented
by the node rather than to whatever is under the center point.
- Around line 5394-5405: The current code removes the listeners vector for
request.target_node, calls callbacks, then reinserts the original vector which
overwrites any registrations/removals that happened during callback execution;
instead, after taking the vector and invoking matching listeners (symbols:
self.a11y.action_listeners, request.target_node, request.action, listener),
retrieve or create the current entry in self.a11y.action_listeners (e.g., via
entry(request.target_node).or_insert(...)) and merge the remaining listeners
from the taken vector into that current vector rather than inserting/overwriting
— this preserves any additions or removals performed by callbacks while avoiding
lost or duplicated handlers.
In `@crates/util/src/command/darwin.rs`:
- Line 548: The comment incorrectly says "pipe" even though the code opens
/dev/null; update the comment near the /dev/null open (the block that sets
close-on-exec after opening the fd) to reference "file descriptor for /dev/null"
or "the opened file descriptor" instead of "pipe" so the wording accurately
describes that we're setting CLOEXEC on the /dev/null fd in the function
handling darwin FD setup.
---
Outside diff comments:
In `@crates/gpui_linux/src/linux/wayland/client.rs`:
- Around line 791-798: The code currently silently treats an unknown
params.display_id as None; change the target_output computation to explicitly
handle Some(display_id) by converting it to target_protocol_id, searching
state.wl_outputs (same id.protocol_id() as u64 comparison) and if no matching
output is found return an Err describing the missing Wayland display (include
the DisplayId value) instead of mapping to None; keep the existing behavior for
params.display_id == None (i.e., leave target_output as None). Use the enclosing
function's Result error path (add or reuse an appropriate error variant) so
callers fail fast when a requested display is absent.
In `@crates/gpui_windows/src/events.rs`:
- Around line 1096-1120: handle_cursor_changed currently calls SetCursor
unconditionally which can re-show the cursor despite the new "hide until mouse
moves" behavior; update handle_cursor_changed to mirror the gate in
handle_set_cursor by checking state.cursor_visible (and using
state.current_cursor when visible, otherwise using None) before calling
SetCursor so SetCursor is only invoked when cursor_visible is true. Reference:
handle_cursor_changed, handle_set_cursor, state.cursor_visible,
state.current_cursor, SetCursor.
In `@crates/gpui_windows/src/window.rs`:
- Around line 472-478: The code currently treats an invalid/stale
params.display_id the same as "no display requested" because
WindowsDisplay::new(display_id) returning None falls through to
WindowsDisplay::primary_monitor; instead, if params.display_id is Some but
WindowsDisplay::new returns None you must return an error. Change the logic
around params.display_id and WindowsDisplay::new so that: when params.display_id
is Some, attempt WindowsDisplay::new(display_id) and if it yields None return an
error (use the same .context(...) or a specific message), and only call
WindowsDisplay::primary_monitor when params.display_id is None; adjust the code
that builds `display` (the match/if-let that currently uses .or_else) to
implement this explicit branching.
In `@crates/gpui/src/platform.rs`:
- Around line 208-214: The new trait methods hide_cursor_until_mouse_moves() and
is_cursor_visible() are breaking existing Platform implementations; add default
implementations on the Platform trait so backends aren’t forced to implement
them: make hide_cursor_until_mouse_moves() a no-op default and
is_cursor_visible() return a sensible default (e.g., true or false as
appropriate for your app), by providing default method bodies on the Platform
trait for hide_cursor_until_mouse_moves and is_cursor_visible so existing
platform backends continue to compile.
---
Duplicate comments:
In `@crates/gpui_linux/src/linux/wayland/client.rs`:
- Around line 443-457: The function that restores cursor style currently clears
self.cursor_hidden_window in the early-return branches when mouse_focused_window
or wl_pointer is missing (the blocks referencing self.mouse_focused_window and
self.wl_pointer), which stops future enter/motion events from retrying and can
leave the cursor invisible; remove the lines that set self.cursor_hidden_window
= None from those early-return branches so the hidden-cursor marker remains
until a successful restore, and ensure you still clear self.cursor_hidden_window
only after the cursor style has been successfully restored.
🪄 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: 54dd69fe-f664-4e52-a505-e72f63aa09c0
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockcrates/gpui_windows/src/shaders.hlslis excluded by!**/*.hlsl
📒 Files selected for processing (54)
Cargo.tomlcrates/gpui/Cargo.tomlcrates/gpui/README.mdcrates/gpui/examples/a11y.rscrates/gpui/examples/opacity.rscrates/gpui/examples/shadow.rscrates/gpui/examples/window_shadow.rscrates/gpui/src/_accessibility.rscrates/gpui/src/app.rscrates/gpui/src/app/async_context.rscrates/gpui/src/element.rscrates/gpui/src/elements/div.rscrates/gpui/src/elements/list.rscrates/gpui/src/elements/retained_layer.rscrates/gpui/src/elements/retained_layer_tests.rscrates/gpui/src/elements/text.rscrates/gpui/src/executor.rscrates/gpui/src/gpui.rscrates/gpui/src/platform.rscrates/gpui/src/platform/test/dispatcher.rscrates/gpui/src/platform_scheduler.rscrates/gpui/src/scene.rscrates/gpui/src/style.rscrates/gpui/src/text_system.rscrates/gpui/src/text_system/line_wrapper.rscrates/gpui/src/view.rscrates/gpui/src/window.rscrates/gpui/src/window/a11y.rscrates/gpui_linux/Cargo.tomlcrates/gpui_linux/src/linux/wayland/client.rscrates/gpui_linux/src/linux/wayland/serial.rscrates/gpui_linux/src/linux/wayland/window.rscrates/gpui_linux/src/linux/x11/window.rscrates/gpui_macos/Cargo.tomlcrates/gpui_macos/src/platform.rscrates/gpui_macos/src/screen_capture.rscrates/gpui_macos/src/shaders.metalcrates/gpui_macos/src/window.rscrates/gpui_macros/src/styles.rscrates/gpui_web/src/window.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_renderer.rscrates/gpui_windows/Cargo.tomlcrates/gpui_windows/src/events.rscrates/gpui_windows/src/window.rscrates/scheduler/src/executor.rscrates/scheduler/src/scheduler.rscrates/scheduler/src/test_scheduler.rscrates/scheduler/src/tests.rscrates/sum_tree/src/sum_tree.rscrates/util/src/command/darwin.rs
There was a problem hiding this comment.
♻️ Duplicate comments (1)
crates/gpui_linux/src/linux/wayland/window.rs (1)
1481-1493:⚠️ Potential issue | 🟠 MajorSeed the AccessKit adapter with the current focus state.
If accessibility is initialized after the window is already focused, this adapter stays inactive until the next
set_focusedcall. That meansa11y_tree_update()can drop all updates for an already-active window.Suggested fix
fn a11y_init(&self, callbacks: gpui::A11yCallbacks) { let activation_handler = TrivialActivationHandler { callback: callbacks.activation, }; let action_handler = TrivialActionHandler(callbacks.action); let deactivation_handler = TrivialDeactivationHandler { callback: callbacks.deactivation, }; - let adapter = - accesskit_unix::Adapter::new(activation_handler, action_handler, deactivation_handler); - - self.borrow_mut().accesskit_adapter = Some(adapter); + let mut state = self.borrow_mut(); + let mut adapter = + accesskit_unix::Adapter::new(activation_handler, action_handler, deactivation_handler); + adapter.update_window_focus_state(state.active); + state.accesskit_adapter = Some(adapter); }🤖 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_linux/src/linux/wayland/window.rs` around lines 1481 - 1493, Create and store the accesskit_unix::Adapter as you do in a11y_init, then immediately seed it with the window's current focus state so it won't remain inactive if a11y is initialized after focus. After assigning self.borrow_mut().accesskit_adapter = Some(adapter), read the current focus state from the window (e.g. via the existing focus field/getter used by set_focused) and if focused call the adapter's focus/activation API (or invoke the provided activation callback) to mark the adapter active; ensure you reference the same TrivialActivationHandler/callbacks.activation and accesskit_unix::Adapter instance when doing this so future a11y_tree_update calls are not dropped.
🤖 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.
Duplicate comments:
In `@crates/gpui_linux/src/linux/wayland/window.rs`:
- Around line 1481-1493: Create and store the accesskit_unix::Adapter as you do
in a11y_init, then immediately seed it with the window's current focus state so
it won't remain inactive if a11y is initialized after focus. After assigning
self.borrow_mut().accesskit_adapter = Some(adapter), read the current focus
state from the window (e.g. via the existing focus field/getter used by
set_focused) and if focused call the adapter's focus/activation API (or invoke
the provided activation callback) to mark the adapter active; ensure you
reference the same TrivialActivationHandler/callbacks.activation and
accesskit_unix::Adapter instance when doing this so future a11y_tree_update
calls are not dropped.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7b259c96-8616-48ef-afb4-1d3e6aba0d00
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
Cargo.tomlcrates/gpui/Cargo.tomlcrates/gpui/src/elements/div.rscrates/gpui/src/text_system/line_wrapper.rscrates/gpui_linux/Cargo.tomlcrates/gpui_linux/src/linux.rscrates/gpui_linux/src/linux/accesskit_shims.rscrates/gpui_linux/src/linux/wayland/window.rscrates/gpui_linux/src/linux/x11/window.rscrates/gpui_macos/src/platform.rscrates/gpui_macos/src/window.rs
Consolidate duplicate poll logic in LogErrorFuture/LogErrorWithBacktraceFuture into a shared helper, and remove dead tilde-fallback in hyperfine lookup.
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
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
Syncs GPUI with upstream Zed changes on top of
main.BREAKING CHANGE:
CursorStyle::Noneandcursor_none()were removed. UseCursorHideMode/ platform cursor hide APIs instead.Changes
.pi-lens/to.gitignore.Testing
Review Notes
mainis still running separately and has not produced findings yet.