From 60eeba2cd8622abd567dcbd05dd6d38b2cecd3d8 Mon Sep 17 00:00:00 2001 From: Hunter B Date: Wed, 30 Sep 2026 07:22:54 -0700 Subject: [PATCH] base: measure a state shared by several windows in one window at a time A host can draw the same entity in two windows of different sizes (Codewhale opens a second window onto one workspace). Two kit states keep geometry measured at draw time, and each window wrote its own into the one shared entity, so every draw invalidated the other window and GPUI's test-mode flush_effects redrew the pair forever (100% CPU in the app's differently-sized-windows regression; in the app it is endless frames). - ResizableState (dock splits): ResizablePanelGroup::measure(bool) keeps the group/panel prepaint and handle drags out of non-measuring windows. DockArea::set_measurement_window names the window whose layout the shared split sizes follow (None, the default, measures everywhere as before). Dock-edge drags now size against the area bounds of the window the pointer is in; bounds() reports the measuring window. - ResizableState::update_panel_size notifies only when something changed (it runs on every prepaint). - InputBaseState: a paint writes geometry, soft-wrap width and scroll only from the window that last measured, until another window is active or it closes; other windows draw with that geometry and do not rewrap the shared display map. Root cause was found by sampling the hung test and then attaching lldb to break on WindowInvalidator::invalidate_view: every captured invalidation came from TextElement::paint on the shared textarea, after the split fix removed the ResizableState ping-pong. Evidence (local, macOS arm64, cargowhale governor): - cargo test -p gpui-base --lib -- resizable:: dock:: : 178 passed, 0 failed, including a_state_shared_by_two_windows_settles_on_the_measuring_window - cargo test -p gpui-base --lib -- input:: : 274 passed, 0 failed, including test_textarea_shared_by_two_windows_settles - cargo clippy -p gpui-base --locked -- --deny warnings: clean - cargo fmt --check -p gpui-base: clean (only these files changed) - Codewhale app differently_sized_windows_follow_the_active_layout_owner_ and_transfer_on_close, built against this tree via a cargo paths override: 1 passed in 0.66s (previously hung indefinitely) Hosted CI for this head is pending. Co-Authored-By: Claude Opus 5.5 --- crates/base/src/dock/dock_area.rs | 76 ++++++++++++++-- crates/base/src/input/base/element.rs | 14 ++- crates/base/src/input/base/state.rs | 120 +++++++++++++++++++++++++- crates/base/src/resizable/mod.rs | 93 +++++++++++++++++++- crates/base/src/resizable/panel.rs | 40 +++++++-- 5 files changed, 323 insertions(+), 20 deletions(-) diff --git a/crates/base/src/dock/dock_area.rs b/crates/base/src/dock/dock_area.rs index 4a4fded1ce..e591bb11bd 100644 --- a/crates/base/src/dock/dock_area.rs +++ b/crates/base/src/dock/dock_area.rs @@ -14,7 +14,8 @@ use gpui::{ AnyElement, AnyView, App, AppContext as _, Axis, Bounds, Context, Div, Empty, Entity, EventEmitter, FocusHandle, Focusable, Hsla, InteractiveElement as _, IntoElement, ParentElement, Pixels, Point, Render, SharedString, Stateful, Styled as _, Subscription, - WeakEntity, Window, WindowHandle, WindowOptions, div, prelude::FluentBuilder as _, px, + WeakEntity, Window, WindowHandle, WindowId, WindowOptions, div, prelude::FluentBuilder as _, + px, }; use crate::{ @@ -101,7 +102,15 @@ struct CachedSplit { pub struct DockArea { id: SharedString, version: Option, + /// Bounds in the measurement window, or in the last window painted when + /// none is named. bounds: Bounds, + /// Bounds in each window the area is drawn in. A dock resize measures + /// against the window the pointer is in. + window_bounds: HashMap>, + /// The one window whose layout writes measured split geometry. See + /// [`Self::set_measurement_window`]. + measurement_window: Option, this: WeakEntity, center: PaneTree, @@ -140,6 +149,8 @@ impl DockArea { id: id.into(), version, bounds: Bounds::default(), + window_bounds: HashMap::new(), + measurement_window: None, this: cx.weak_entity(), center: PaneTree::new(RootKind::Split), docks: HashMap::new(), @@ -186,12 +197,48 @@ impl DockArea { cx.notify(); } - /// The area's own bounds, recorded each frame. Dock resizing measures - /// against it. + /// The area's own bounds, recorded each frame in the measurement window + /// (in whichever window painted last when none is named). pub fn bounds(&self) -> Bounds { self.bounds } + /// Name the window whose layout the area's shared split sizes follow. + /// + /// The splits' measured sizes live in one state per split, shared by every + /// window the area is drawn in. When two differently sized windows both + /// write their measurements into it, each makes the other's layout stale + /// and they redraw each other forever. A host that draws one area in + /// several windows names the one that measures; the others draw the + /// splits at the sizes it measured. `None`, the default, measures in + /// every window, which is right for an area drawn in only one. + /// + /// Takes effect at the next draw of each window and schedules nothing, + /// so a host may call it while rendering. + pub fn set_measurement_window(&mut self, window: Option) { + self.measurement_window = window; + } + + /// The window named by [`Self::set_measurement_window`]. + pub fn measurement_window(&self) -> Option { + self.measurement_window + } + + fn measures_in(&self, window: WindowId) -> bool { + self.measurement_window.is_none_or(|owner| owner == window) + } + + fn record_bounds(&mut self, window: WindowId, bounds: Bounds, cx: &App) { + if self.measures_in(window) { + self.bounds = bounds; + } + if self.window_bounds.insert(window, bounds).is_none() { + // A window is new here: forget any that have since closed. + let live: HashSet = cx.windows().iter().map(|w| w.window_id()).collect(); + self.window_bounds.retain(|id, _| live.contains(id)); + } + } + /// The tree for one region, or `None` for a dock that does not exist. /// /// The `Option` is in the signature rather than hidden behind a panic @@ -1484,6 +1531,7 @@ impl DockArea { &mut self, placement: DockPlacement, pointer: Point, + window: WindowId, cx: &mut Context, ) { let opposite = match placement { @@ -1491,8 +1539,15 @@ impl DockArea { DockPlacement::Right => self.dock_size(DockPlacement::Left), _ => None, }; + // The pointer is in `window`'s coordinates, so it is measured against + // the area as that window laid it out. + let area_bounds = self + .window_bounds + .get(&window) + .copied() + .unwrap_or(self.bounds); let sizing = DockSizing::new(placement) - .with_area_bounds(self.bounds) + .with_area_bounds(area_bounds) .with_opposite_dock_size(opposite.unwrap_or(px(0.))); let size = sizing.clamp(sizing.size_from_pointer(pointer)); @@ -1561,6 +1616,7 @@ impl DockArea { .when_some(self.splits.get(&node.id()), |group, cached| { group.with_state(&cached.entity) }) + .measure(self.measures_in(window.window_handle().window_id())) .with_handle_appearance({ let renderer = self.renderer.clone(); Rc::new(move |handle, window, cx| { @@ -1652,8 +1708,11 @@ impl DockArea { _ = area.update(cx, |area, cx| area.toggle_dock(placement, window, cx)); }) }, - on_resize: Rc::new(move |pointer, _, cx| { - _ = area.update(cx, |area, cx| area.resize_dock(placement, pointer, cx)); + on_resize: Rc::new(move |pointer, window, cx| { + let window = window.window_handle().window_id(); + _ = area.update(cx, |area, cx| { + area.resize_dock(placement, pointer, window, cx) + }); }), } } @@ -1686,8 +1745,9 @@ impl Render for DockArea { .overflow_hidden() .flex() .flex_row() - .on_prepaint(move |bounds, _, cx| { - area.update(cx, |area, _| area.bounds = bounds); + .on_prepaint(move |bounds, window, cx| { + let window = window.window_handle().window_id(); + area.update(cx, |area, cx| area.record_bounds(window, bounds, cx)); }) .track_focus(&self.focus_handle) .map(|frame| match self.zoomed_view() { diff --git a/crates/base/src/input/base/element.rs b/crates/base/src/input/base/element.rs index 6d2be4acc0..f512d951ed 100644 --- a/crates/base/src/input/base/element.rs +++ b/crates/base/src/input/base/element.rs @@ -1685,6 +1685,9 @@ pub(super) struct PrepaintState { /// First line of inline completion (painted after cursor on same line) ghost_first_line: Option, ghost_lines_height: Pixels, + /// Whether this window writes the state's geometry back at paint. See + /// `InputBaseState::measures_in`. + measures: bool, } impl PrepaintState { @@ -1810,6 +1813,7 @@ impl Element for TextElement { }); let state = self.state.read(cx); + let measures = state.measures_in(window, cx); let multi_line = state.is_multi_line(); let text = state.text.clone(); let is_empty = text.len() == 0; @@ -1857,7 +1861,9 @@ impl Element for TextElement { .map(|l| l.wrapping_indent != wrapping_indent) .unwrap_or(true); - if wrap_width_changed || wrapping_indent_changed { + // Another window's wrap stays: rewrapping here would change the lines + // that window laid out and draws from. + if measures && (wrap_width_changed || wrapping_indent_changed) { self.state.update(cx, |state, cx| { state.display_map.on_layout_changed(wrap_width, cx); state.display_map.set_wrapping_indent(wrapping_indent, cx); @@ -2195,6 +2201,7 @@ impl Element for TextElement { ghost_first_line, ghost_lines, ghost_lines_height, + measures, } } @@ -2485,7 +2492,12 @@ impl Element for TextElement { cx, ); + let window_id = window.window_handle().window_id(); self.state.update(cx, |state, cx| { + if !prepaint.measures { + return; + } + state.geometry_window = Some(window_id); let geometry_changed = state.last_bounds != Some(bounds) || state.input_bounds != input_bounds || state.scroll_size != prepaint.scroll_size diff --git a/crates/base/src/input/base/state.rs b/crates/base/src/input/base/state.rs index 01d13c3120..72634634b5 100644 --- a/crates/base/src/input/base/state.rs +++ b/crates/base/src/input/base/state.rs @@ -8,7 +8,7 @@ use gpui::{ EventEmitter, FocusHandle, Focusable, InteractiveElement as _, IntoElement, KeyBinding, MouseButton, MouseDownEvent, MouseMoveEvent, MouseUpEvent, ParentElement as _, Pixels, Point, Render, ScrollHandle, ScrollWheelEvent, SharedString, Styled as _, Subscription, - UTF16Selection, Window, actions, div, point, prelude::FluentBuilder as _, px, + UTF16Selection, Window, WindowId, actions, div, point, prelude::FluentBuilder as _, px, }; use ropey::{Rope, RopeSlice}; use serde::Deserialize; @@ -368,6 +368,9 @@ pub struct InputBaseState { pub(super) input_bounds: Bounds, /// The text bounds pub(super) last_bounds: Option>, + /// The window whose layout last wrote the geometry above. See + /// [`Self::measures_in`]. + pub(super) geometry_window: Option, pub(super) last_selected_range: Option, pub(super) selecting: bool, /// Anchor point of an in-progress columnar (block) selection. @@ -714,6 +717,7 @@ impl InputBaseState { mode: LayoutMode::default(), last_layout: None, last_bounds: None, + geometry_window: None, last_selected_range: None, column_select_start: None, last_cursor: None, @@ -3382,6 +3386,26 @@ impl InputBaseState { }; } + /// Whether drawing in `window` may write this input's geometry: its + /// soft-wrap width, laid-out lines, bounds and scroll. + /// + /// A host can draw one state in several windows at different sizes. It + /// holds one geometry, and each window writing its own would leave the + /// other's stale, so the windows would redraw each other forever. The + /// window that last measured keeps measuring until another window is + /// active or it closes; the rest draw with the geometry it measured. + pub(super) fn measures_in(&self, window: &Window, cx: &App) -> bool { + let Some(owner) = self.geometry_window else { + return true; + }; + owner == window.window_handle().window_id() + || window.is_window_active() + || !cx + .windows() + .iter() + .any(|handle| handle.window_id() == owner) + } + pub(super) fn set_input_bounds(&mut self, new_bounds: Bounds, cx: &mut Context) { let wrap_width_changed = self.input_bounds.size.width != new_bounds.size.width; self.input_bounds = new_bounds; @@ -4392,6 +4416,100 @@ mod tests { } } + /// One textarea drawn in two differently sized windows holds one geometry. + /// When both windows wrote theirs at paint, each paint changed the + /// geometry the other had written, notified, and dirtied the other + /// window, so the pair redrew each other forever. The active window + /// measures; the other draws with its geometry, and both settle. + #[gpui::test] + fn test_textarea_shared_by_two_windows_settles(cx: &mut TestAppContext) { + use std::{cell::Cell, rc::Rc}; + + cx.update(|cx| { + cx.set_global(Theme::default()); + super::super::init(cx); + }); + let mut input = None; + let wide = cx.open_window(size(px(900.), px(200.)), |window, cx| { + let state = cx.new(|cx| crate::input::TextareaState::new(window, cx)); + input = Some(state.clone()); + TestRoot(state) + }); + let input = input.unwrap(); + let narrow = cx.open_window(size(px(500.), px(200.)), { + let input = input.clone(); + move |_, _| TestRoot(input) + }); + let notifications = Rc::new(Cell::new(0usize)); + let _subscription = cx.update(|cx| { + let notifications = notifications.clone(); + cx.observe(&input, move |_, _| { + let n = notifications.get() + 1; + notifications.set(n); + assert!( + n < 64, + "a shared textarea keeps notifying: its windows never settle" + ); + }) + }); + wide.update(cx, |_, window, cx| { + window.activate_window(); + input.update(cx, |state, cx| { + state.set_value( + "one textarea shown in two windows at two widths ".repeat(8), + window, + cx, + ) + }); + }) + .unwrap(); + let handles = [*wide, *narrow]; + let draw_both = |cx: &mut TestAppContext| { + for handle in handles { + cx.update_window(handle, |_, window, cx| window.draw(cx).clear(cx)) + .unwrap(); + } + cx.run_until_parked(); + }; + let measured = |cx: &mut TestAppContext| { + input.read_with(cx, |state, _| { + ( + state.geometry_window, + state.last_bounds.map(|b| b.size.width), + ) + }) + }; + for _ in 0..3 { + draw_both(cx); + } + let settled = notifications.get(); + for _ in 0..3 { + draw_both(cx); + } + assert_eq!( + notifications.get(), + settled, + "drawing both windows again must not notify" + ); + let (owner, width) = measured(cx); + assert_eq!(owner, Some(wide.window_id())); + assert!( + width.is_some_and(|w| w > px(500.)), + "the active wide window measures" + ); + + // Activating the other window hands measurement over to it. + narrow + .update(cx, |_, window, _| window.activate_window()) + .unwrap(); + for _ in 0..3 { + draw_both(cx); + } + let (owner, width) = measured(cx); + assert_eq!(owner, Some(narrow.window_id())); + assert!(width.is_some_and(|w| w <= px(500.))); + } + #[gpui::test] fn test_noop_scroll_notifies_diagnostic_dismissal(cx: &mut TestAppContext) { use std::{cell::Cell, rc::Rc}; diff --git a/crates/base/src/resizable/mod.rs b/crates/base/src/resizable/mod.rs index 1f3f9ed8a7..fd1c49235c 100644 --- a/crates/base/src/resizable/mod.rs +++ b/crates/base/src/resizable/mod.rs @@ -205,16 +205,23 @@ impl ResizableState { cx: &mut Context, ) { let size = bounds.size.along(self.axis); + let panel = &mut self.panels[panel_ix]; + let mut changed = panel.bounds != bounds || panel.size_range != size_range; // This check is only necessary to stop the very first panel from resizing on its own // it needs to be passed when the panel is freshly created so we get the initial size, // but its also fine when it sometimes passes later. if self.sizes[panel_ix].as_f32() == PANEL_MIN_SIZE.as_f32() { + changed |= self.sizes[panel_ix] != size || panel.size != Some(size); self.sizes[panel_ix] = size; - self.panels[panel_ix].size = Some(size); + panel.size = Some(size); + } + panel.bounds = bounds; + panel.size_range = size_range; + // Runs in every prepaint. Notifying an unchanged measurement would + // schedule another frame for each window showing this state, forever. + if changed { + cx.notify(); } - self.panels[panel_ix].bounds = bounds; - self.panels[panel_ix].size_range = size_range; - cx.notify(); } /// Remove the panel at `panel_ix` and redistribute the remaining space. @@ -505,6 +512,84 @@ mod tests { assert_eq!(followup, settled, "settling frame must not be pending"); } + struct SharedStateHarness { + state: gpui::Entity, + measure: bool, + } + + impl Render for SharedStateHarness { + fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { + div().size_full().child( + h_resizable("shared-state") + .with_state(&self.state) + .measure(self.measure) + .child(resizable_panel().size(px(240.)).child(div().size_full())) + .child(resizable_panel().child(div().size_full())), + ) + } + } + + /// One state drawn in two differently sized windows holds one + /// measurement. With both windows measuring, each draw made the other + /// window's layout stale and the pair redrew each other without end. + /// Only the measuring window writes geometry now, and both settle. + #[gpui::test] + fn a_state_shared_by_two_windows_settles_on_the_measuring_window(cx: &mut TestAppContext) { + let state = cx.update(|cx| cx.new(|_| ResizableState::default())); + let notifications = Rc::new(Cell::new(0usize)); + let _observer = cx.update(|cx| { + let notifications = notifications.clone(); + cx.observe(&state, move |_, _| { + let n = notifications.get() + 1; + notifications.set(n); + assert!( + n < 64, + "a shared split state keeps notifying: its windows never settle" + ); + }) + }); + let wide = cx.open_window(size(px(1200.), px(300.)), { + let state = state.clone(); + move |_, _| SharedStateHarness { + state, + measure: true, + } + }); + let narrow = cx.open_window(size(px(700.), px(300.)), { + let state = state.clone(); + move |_, _| SharedStateHarness { + state, + measure: false, + } + }); + let handles = [*wide, *narrow]; + let draw_both = |cx: &mut TestAppContext| { + for handle in handles { + cx.update_window(handle, |_, window, cx| window.draw(cx).clear(cx)) + .unwrap(); + } + cx.run_until_parked(); + }; + for _ in 0..3 { + draw_both(cx); + } + let settled = notifications.get(); + for _ in 0..3 { + draw_both(cx); + } + + assert_eq!( + notifications.get(), + settled, + "drawing both windows again must not notify" + ); + assert_eq!( + state.read_with(cx, |state, _| state.container_size()), + px(1200.), + "the measuring window's width is the one the state holds" + ); + } + struct ResizableHarness { state: gpui::Entity, resizes: Rc>, diff --git a/crates/base/src/resizable/panel.rs b/crates/base/src/resizable/panel.rs index d4032077f7..77a0511793 100644 --- a/crates/base/src/resizable/panel.rs +++ b/crates/base/src/resizable/panel.rs @@ -36,6 +36,7 @@ pub struct ResizablePanelGroup { children: Vec, on_resize: Rc, &mut Window, &mut App)>, handle_appearance: Option, + measure: bool, } impl ResizablePanelGroup { @@ -49,6 +50,7 @@ impl ResizablePanelGroup { size: None, on_resize: Rc::new(|_, _, _| {}), handle_appearance: None, + measure: true, } } @@ -69,6 +71,21 @@ impl ResizablePanelGroup { self } + /// Whether this group writes the geometry it lays out at into its state, + /// default true. + /// + /// A state bound with [`Self::with_state`] and drawn in several windows + /// holds one measurement. Every window writing its own size into it makes + /// the other window's layout stale, and the two then redraw each other + /// without end. Pass `false` in every window but the one whose size the + /// shared layout follows: those windows draw the panels at the sizes the + /// state already has, and do not drive handle drags against geometry + /// they did not measure. + pub fn measure(mut self, measure: bool) -> Self { + self.measure = measure; + self + } + /// Set the axis of the resizable panel group, default is horizontal. pub fn axis(mut self, axis: Axis) -> Self { self.axis = axis; @@ -169,12 +186,13 @@ impl RenderOnce for ResizablePanelGroup { panel.axis = self.axis; panel.state = Some(state.clone()); panel.handle_appearance = self.handle_appearance.clone(); + panel.measure = self.measure; panel }), ) - .on_prepaint({ + .when(self.measure, |this| { let state = state.clone(); - move |bounds, window, cx| { + this.on_prepaint(move |bounds, window, cx| { state.update(cx, |state, cx| { let size_changed = state.bounds.size.along(self.axis) != bounds.size.along(self.axis); @@ -197,12 +215,13 @@ impl RenderOnce for ResizablePanelGroup { }); } }) - } + }) }) .child(ResizePanelGroupElement { state: state.clone(), axis: self.axis, on_resize: self.on_resize.clone(), + measure: self.measure, }) } } @@ -247,6 +266,8 @@ pub struct ResizablePanel { visible: bool, style: StyleRefinement, handle_appearance: Option, + /// Set by the group: see [`ResizablePanelGroup::measure`]. + measure: bool, } impl ResizablePanel { @@ -262,6 +283,7 @@ impl ResizablePanel { visible: true, style: StyleRefinement::default(), handle_appearance: None, + measure: true, } } @@ -351,13 +373,13 @@ impl RenderOnce for ResizablePanel { Some(size) => this.flex_basis(size.min(size_range.end).max(size_range.start)), None => this, }) - .on_prepaint({ + .when(self.measure, |this| { let state = state.clone(); - move |bounds, _, cx| { + this.on_prepaint(move |bounds, _, cx| { state.update(cx, |state, cx| { state.update_panel_size(self.panel_ix, bounds, self.size_range, cx) }) - } + }) }) .children(self.children) .when(self.panel_ix > 0, |this| { @@ -384,6 +406,7 @@ struct ResizePanelGroupElement { state: Entity, on_resize: Rc, &mut Window, &mut App)>, axis: Axis, + measure: bool, } impl IntoElement for ResizePanelGroupElement { @@ -438,6 +461,11 @@ impl Element for ResizePanelGroupElement { window: &mut Window, cx: &mut App, ) { + // A drag resizes against the panel bounds the measuring window + // recorded; pointer positions from any other window do not match them. + if !self.measure { + return; + } window.on_mouse_event({ let state = self.state.clone(); let axis = self.axis;