From 7f345f8bf5310a8c6c9673a54bcdb71a3f363131 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 3 May 2023 12:18:16 +0200 Subject: [PATCH 01/16] Separate `Window::build_scene` into `layout` and `paint` --- crates/collab_ui/src/collab_titlebar_item.rs | 6 +- crates/collab_ui/src/face_pile.rs | 4 +- crates/editor/src/element.rs | 18 ++- crates/gpui/src/app.rs | 133 +++++++++++++----- crates/gpui/src/app/window.rs | 46 ++++-- crates/gpui/src/elements.rs | 44 ++++-- crates/gpui/src/elements/align.rs | 4 +- crates/gpui/src/elements/canvas.rs | 2 +- crates/gpui/src/elements/clipped.rs | 6 +- crates/gpui/src/elements/constrained_box.rs | 11 +- crates/gpui/src/elements/container.rs | 4 +- crates/gpui/src/elements/empty.rs | 4 +- crates/gpui/src/elements/expanded.rs | 4 +- crates/gpui/src/elements/flex.rs | 12 +- crates/gpui/src/elements/hook.rs | 4 +- crates/gpui/src/elements/image.rs | 5 +- crates/gpui/src/elements/keystroke_label.rs | 2 +- crates/gpui/src/elements/label.rs | 4 +- crates/gpui/src/elements/list.rs | 45 ++++-- .../gpui/src/elements/mouse_event_handler.rs | 6 +- crates/gpui/src/elements/overlay.rs | 5 +- crates/gpui/src/elements/resizable.rs | 5 +- crates/gpui/src/elements/stack.rs | 4 +- crates/gpui/src/elements/svg.rs | 4 +- crates/gpui/src/elements/text.rs | 16 ++- crates/gpui/src/elements/tooltip.rs | 5 +- crates/gpui/src/elements/uniform_list.rs | 4 +- crates/terminal_view/src/terminal_element.rs | 6 +- crates/workspace/src/pane.rs | 6 +- crates/workspace/src/status_bar.rs | 6 +- 30 files changed, 289 insertions(+), 136 deletions(-) diff --git a/crates/collab_ui/src/collab_titlebar_item.rs b/crates/collab_ui/src/collab_titlebar_item.rs index 97fb82a5d6b7cbce8ab7373c9dcb78e6a7b8d3e9..0942c47910856a8c09102e7bfe7f2cf49b3518c4 100644 --- a/crates/collab_ui/src/collab_titlebar_item.rs +++ b/crates/collab_ui/src/collab_titlebar_item.rs @@ -14,8 +14,8 @@ use gpui::{ geometry::{rect::RectF, vector::vec2f, PathBuilder}, json::{self, ToJson}, platform::{CursorStyle, MouseButton}, - AppContext, Entity, ImageData, ModelHandle, SceneBuilder, Subscription, View, ViewContext, - ViewHandle, WeakViewHandle, + AppContext, Entity, ImageData, LayoutContext, ModelHandle, SceneBuilder, Subscription, View, + ViewContext, ViewHandle, WeakViewHandle, }; use project::Project; use settings::Settings; @@ -865,7 +865,7 @@ impl Element for AvatarRibbon { &mut self, constraint: gpui::SizeConstraint, _: &mut CollabTitlebarItem, - _: &mut ViewContext, + _: &mut LayoutContext, ) -> (gpui::geometry::vector::Vector2F, Self::LayoutState) { (constraint.max, ()) } diff --git a/crates/collab_ui/src/face_pile.rs b/crates/collab_ui/src/face_pile.rs index 839ea7c7353fd2b6d65bae5d58e9013b1f4cfc3a..1bbceee9af1bec406bf9b1398fde94dd230ac73d 100644 --- a/crates/collab_ui/src/face_pile.rs +++ b/crates/collab_ui/src/face_pile.rs @@ -7,7 +7,7 @@ use gpui::{ }, json::ToJson, serde_json::{self, json}, - AnyElement, Axis, Element, SceneBuilder, ViewContext, + AnyElement, Axis, Element, LayoutContext, SceneBuilder, ViewContext, }; use crate::CollabTitlebarItem; @@ -34,7 +34,7 @@ impl Element for FacePile { &mut self, constraint: gpui::SizeConstraint, view: &mut CollabTitlebarItem, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> (Vector2F, Self::LayoutState) { debug_assert!(constraint.max_along(Axis::Horizontal) == f32::INFINITY); diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 7c43885763e09286c5916b0d89883491050bc086..d1ad1f26259da3874d46352e7033e5829026bd39 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -30,8 +30,8 @@ use gpui::{ json::{self, ToJson}, platform::{CursorStyle, Modifiers, MouseButton, MouseButtonEvent, MouseMovedEvent}, text_layout::{self, Line, RunStyle, TextLayoutCache}, - AnyElement, Axis, Border, CursorRegion, Element, EventContext, MouseRegion, Quad, SceneBuilder, - SizeConstraint, ViewContext, WindowContext, + AnyElement, Axis, Border, CursorRegion, Element, EventContext, LayoutContext, MouseRegion, + Quad, SceneBuilder, SizeConstraint, ViewContext, WindowContext, }; use itertools::Itertools; use json::json; @@ -1388,7 +1388,7 @@ impl EditorElement { line_layouts: &[text_layout::Line], include_root: bool, editor: &mut Editor, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> (f32, Vec) { let tooltip_style = cx.global::().theme.tooltip.clone(); let scroll_x = snapshot.scroll_anchor.offset.x(); @@ -1594,7 +1594,7 @@ impl Element for EditorElement { &mut self, constraint: SizeConstraint, editor: &mut Editor, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> (Vector2F, Self::LayoutState) { let mut size = constraint.max; if size.x().is_infinite() { @@ -2565,10 +2565,18 @@ mod tests { let mut element = EditorElement::new(editor.read_with(cx, |editor, cx| editor.style(cx))); let (size, mut state) = editor.update(cx, |editor, cx| { + let mut new_parents = Default::default(); + let mut notify_views_if_parents_change = Default::default(); + let mut layout_cx = LayoutContext::new( + cx, + &mut new_parents, + &mut notify_views_if_parents_change, + false, + ); element.layout( SizeConstraint::new(vec2f(500., 500.), vec2f(500., 500.)), editor, - cx, + &mut layout_cx, ) }); diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 4d84f7c070fd179ec3220054b1f3fff2041ed3e5..f8d6459cf4fc1e6fca69d95ac86ab2b79681e8cc 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -1361,9 +1361,9 @@ impl AppContext { })); let mut window = Window::new(window_id, platform_window, self, build_root_view); - let scene = WindowContext::mutable(self, &mut window, window_id) - .build_scene() - .expect("initial scene should not error"); + let mut cx = WindowContext::mutable(self, &mut window, window_id); + cx.layout(false).expect("initial layout should not error"); + let scene = cx.paint().expect("initial paint should not error"); window.platform_window.present_scene(scene); window } @@ -1486,6 +1486,7 @@ impl AppContext { self.flushing_effects = true; let mut refreshing = false; + let mut updated_windows = HashSet::default(); loop { if let Some(effect) = self.pending_effects.pop_front() { match effect { @@ -1664,10 +1665,27 @@ impl AppContext { } else { self.remove_dropped_entities(); - if refreshing { - self.perform_window_refresh(); - } else { - self.update_windows(); + for window_id in self.windows.keys().cloned().collect::>() { + self.update_window(window_id, |cx| { + let invalidation = if refreshing { + let mut invalidation = + cx.window.invalidation.take().unwrap_or_default(); + invalidation + .updated + .extend(cx.window.rendered_views.keys().copied()); + Some(invalidation) + } else { + cx.window.invalidation.take() + }; + + if let Some(invalidation) = invalidation { + let appearance = cx.window.platform_window.appearance(); + cx.invalidate(invalidation, appearance); + if cx.layout(refreshing).log_err().is_some() { + updated_windows.insert(window_id); + } + } + }); } if self.pending_effects.is_empty() { @@ -1675,6 +1693,14 @@ impl AppContext { callback(self); } + for window_id in updated_windows.drain() { + self.update_window(window_id, |cx| { + if let Some(scene) = cx.paint().log_err() { + cx.window.platform_window.present_scene(scene); + } + }); + } + if self.pending_effects.is_empty() { self.flushing_effects = false; self.pending_notifications.clear(); @@ -1689,21 +1715,6 @@ impl AppContext { } } - fn update_windows(&mut self) { - let window_ids = self.windows.keys().cloned().collect::>(); - for window_id in window_ids { - self.update_window(window_id, |cx| { - if let Some(mut invalidation) = cx.window.invalidation.take() { - let appearance = cx.window.platform_window.appearance(); - cx.invalidate(&mut invalidation, appearance); - if let Some(scene) = cx.build_scene().log_err() { - cx.window.platform_window.present_scene(scene); - } - } - }); - } - } - fn window_was_resized(&mut self, window_id: usize) { self.pending_effects .push_back(Effect::ResizeWindow { window_id }); @@ -1747,23 +1758,6 @@ impl AppContext { self.pending_effects.push_back(Effect::RefreshWindows); } - fn perform_window_refresh(&mut self) { - let window_ids = self.windows.keys().cloned().collect::>(); - for window_id in window_ids { - self.update_window(window_id, |cx| { - let mut invalidation = cx.window.invalidation.take().unwrap_or_default(); - invalidation - .updated - .extend(cx.window.rendered_views.keys().copied()); - cx.invalidate(&mut invalidation, cx.window.platform_window.appearance()); - cx.refreshing = true; - if let Some(scene) = cx.build_scene().log_err() { - cx.window.platform_window.present_scene(scene); - } - }); - } - } - fn emit_global_event(&mut self, payload: Box) { let type_id = (&*payload).type_id(); @@ -3255,6 +3249,67 @@ impl BorrowWindowContext for ViewContext<'_, '_, V> { } } +pub struct LayoutContext<'a, 'b, 'c, V: View> { + view_context: &'c mut ViewContext<'a, 'b, V>, + new_parents: &'c mut HashMap, + views_to_notify_if_ancestors_change: &'c mut HashSet, + pub refreshing: bool, +} + +impl<'a, 'b, 'c, V: View> LayoutContext<'a, 'b, 'c, V> { + pub fn new( + view_context: &'c mut ViewContext<'a, 'b, V>, + new_parents: &'c mut HashMap, + views_to_notify_if_ancestors_change: &'c mut HashSet, + refreshing: bool, + ) -> Self { + Self { + view_context, + new_parents, + views_to_notify_if_ancestors_change, + refreshing, + } + } + + pub fn view_context(&mut self) -> &mut ViewContext<'a, 'b, V> { + self.view_context + } +} + +impl<'a, 'b, 'c, V: View> Deref for LayoutContext<'a, 'b, 'c, V> { + type Target = ViewContext<'a, 'b, V>; + + fn deref(&self) -> &Self::Target { + &self.view_context + } +} + +impl DerefMut for LayoutContext<'_, '_, '_, V> { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.view_context + } +} + +impl BorrowAppContext for LayoutContext<'_, '_, '_, V> { + fn read_with T>(&self, f: F) -> T { + BorrowAppContext::read_with(&*self.view_context, f) + } + + fn update T>(&mut self, f: F) -> T { + BorrowAppContext::update(&mut *self.view_context, f) + } +} + +impl BorrowWindowContext for LayoutContext<'_, '_, '_, V> { + fn read_with T>(&self, window_id: usize, f: F) -> T { + BorrowWindowContext::read_with(&*self.view_context, window_id, f) + } + + fn update T>(&mut self, window_id: usize, f: F) -> T { + BorrowWindowContext::update(&mut *self.view_context, window_id, f) + } +} + pub struct EventContext<'a, 'b, 'c, V: View> { view_context: &'c mut ViewContext<'a, 'b, V>, pub(crate) handled: bool, diff --git a/crates/gpui/src/app/window.rs b/crates/gpui/src/app/window.rs index 49befafbec4bfca2fac1782f2d5c8d249b24a951..f9ccef7ae077268e5f17ae9ae09a8c8b1e53ef6e 100644 --- a/crates/gpui/src/app/window.rs +++ b/crates/gpui/src/app/window.rs @@ -14,8 +14,8 @@ use crate::{ text_layout::TextLayoutCache, util::post_inc, Action, AnyView, AnyViewHandle, AppContext, BorrowAppContext, BorrowWindowContext, Effect, - Element, Entity, Handle, MouseRegion, MouseRegionId, ParentId, SceneBuilder, Subscription, - View, ViewContext, ViewHandle, WindowInvalidation, + Element, Entity, Handle, LayoutContext, MouseRegion, MouseRegionId, ParentId, SceneBuilder, + Subscription, View, ViewContext, ViewHandle, WindowInvalidation, }; use anyhow::{anyhow, bail, Result}; use collections::{HashMap, HashSet}; @@ -93,8 +93,8 @@ impl Window { let root_view = window_context .build_and_insert_view(ParentId::Root, |cx| Some(build_view(cx))) .unwrap(); - if let Some(mut invalidation) = window_context.window.invalidation.take() { - window_context.invalidate(&mut invalidation, appearance); + if let Some(invalidation) = window_context.window.invalidation.take() { + window_context.invalidate(invalidation, appearance); } window.focused_view_id = Some(root_view.id()); window.root_view = Some(root_view.into_any()); @@ -113,7 +113,6 @@ pub struct WindowContext<'a> { pub(crate) app_context: Reference<'a, AppContext>, pub(crate) window: Reference<'a, Window>, pub(crate) window_id: usize, - pub(crate) refreshing: bool, pub(crate) removed: bool, } @@ -169,7 +168,6 @@ impl<'a> WindowContext<'a> { app_context: Reference::Mutable(app_context), window: Reference::Mutable(window), window_id, - refreshing: false, removed: false, } } @@ -179,7 +177,6 @@ impl<'a> WindowContext<'a> { app_context: Reference::Immutable(app_context), window: Reference::Immutable(window), window_id, - refreshing: false, removed: false, } } @@ -891,7 +888,7 @@ impl<'a> WindowContext<'a> { false } - pub fn invalidate(&mut self, invalidation: &mut WindowInvalidation, appearance: Appearance) { + pub fn invalidate(&mut self, mut invalidation: WindowInvalidation, appearance: Appearance) { self.start_frame(); self.window.appearance = appearance; for view_id in &invalidation.removed { @@ -932,13 +929,32 @@ impl<'a> WindowContext<'a> { Ok(element) } - pub fn build_scene(&mut self) -> Result { + pub(crate) fn layout(&mut self, refreshing: bool) -> Result<()> { + let window_size = self.window.platform_window.content_size(); + let root_view_id = self.window.root_view().id(); + let mut rendered_root = self.window.rendered_views.remove(&root_view_id).unwrap(); + let mut new_parents = HashMap::default(); + let mut views_to_notify_if_ancestors_change = HashSet::default(); + rendered_root.layout( + SizeConstraint::strict(window_size), + &mut new_parents, + &mut views_to_notify_if_ancestors_change, + refreshing, + self, + )?; + + self.window + .rendered_views + .insert(root_view_id, rendered_root); + Ok(()) + } + + pub(crate) fn paint(&mut self) -> Result { let window_size = self.window.platform_window.content_size(); let scale_factor = self.window.platform_window.scale_factor(); let root_view_id = self.window.root_view().id(); let mut rendered_root = self.window.rendered_views.remove(&root_view_id).unwrap(); - rendered_root.layout(SizeConstraint::strict(window_size), self)?; let mut scene_builder = SceneBuilder::new(scale_factor); rendered_root.paint( @@ -1366,11 +1382,17 @@ impl Element for ChildView { &mut self, constraint: SizeConstraint, _: &mut V, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> (Vector2F, Self::LayoutState) { if let Some(mut rendered_view) = cx.window.rendered_views.remove(&self.view_id) { let size = rendered_view - .layout(constraint, cx) + .layout( + constraint, + cx.new_parents, + cx.views_to_notify_if_ancestors_change, + cx.refreshing, + cx.view_context, + ) .log_err() .unwrap_or(Vector2F::zero()); cx.window.rendered_views.insert(self.view_id, rendered_view); diff --git a/crates/gpui/src/elements.rs b/crates/gpui/src/elements.rs index 7de0bc10f59ec0619b596b3a9a52d619a9104943..073d12c329255edba4bfc90921359094365f915a 100644 --- a/crates/gpui/src/elements.rs +++ b/crates/gpui/src/elements.rs @@ -33,9 +33,11 @@ use crate::{ rect::RectF, vector::{vec2f, Vector2F}, }, - json, Action, SceneBuilder, SizeConstraint, View, ViewContext, WeakViewHandle, WindowContext, + json, Action, LayoutContext, SceneBuilder, SizeConstraint, View, ViewContext, WeakViewHandle, + WindowContext, }; use anyhow::{anyhow, Result}; +use collections::{HashMap, HashSet}; use core::panic; use json::ToJson; use std::{ @@ -54,7 +56,7 @@ pub trait Element: 'static { &mut self, constraint: SizeConstraint, view: &mut V, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> (Vector2F, Self::LayoutState); fn paint( @@ -211,7 +213,7 @@ trait AnyElementState { &mut self, constraint: SizeConstraint, view: &mut V, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> Vector2F; fn paint( @@ -263,7 +265,7 @@ impl> AnyElementState for ElementState { &mut self, constraint: SizeConstraint, view: &mut V, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> Vector2F { let result; *self = match mem::take(self) { @@ -444,7 +446,7 @@ impl AnyElement { &mut self, constraint: SizeConstraint, view: &mut V, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> Vector2F { self.state.layout(constraint, view, cx) } @@ -505,7 +507,7 @@ impl Element for AnyElement { &mut self, constraint: SizeConstraint, view: &mut V, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> (Vector2F, Self::LayoutState) { let size = self.layout(constraint, view, cx); (size, ()) @@ -597,7 +599,7 @@ impl> Element for ComponentHost { &mut self, constraint: SizeConstraint, view: &mut V, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> (Vector2F, AnyElement) { let mut element = self.component.render(view, cx); let size = element.layout(constraint, view, cx); @@ -642,7 +644,14 @@ impl> Element for ComponentHost { } pub trait AnyRootElement { - fn layout(&mut self, constraint: SizeConstraint, cx: &mut WindowContext) -> Result; + fn layout( + &mut self, + constraint: SizeConstraint, + new_parents: &mut HashMap, + views_to_notify_if_ancestors_change: &mut HashSet, + refreshing: bool, + cx: &mut WindowContext, + ) -> Result; fn paint( &mut self, scene: &mut SceneBuilder, @@ -660,12 +669,27 @@ pub trait AnyRootElement { } impl AnyRootElement for RootElement { - fn layout(&mut self, constraint: SizeConstraint, cx: &mut WindowContext) -> Result { + fn layout( + &mut self, + constraint: SizeConstraint, + new_parents: &mut HashMap, + views_to_notify_if_ancestors_change: &mut HashSet, + refreshing: bool, + cx: &mut WindowContext, + ) -> Result { let view = self .view .upgrade(cx) .ok_or_else(|| anyhow!("layout called on a root element for a dropped view"))?; - view.update(cx, |view, cx| Ok(self.element.layout(constraint, view, cx))) + view.update(cx, |view, cx| { + let mut cx = LayoutContext::new( + cx, + new_parents, + views_to_notify_if_ancestors_change, + refreshing, + ); + Ok(self.element.layout(constraint, view, &mut cx)) + }) } fn paint( diff --git a/crates/gpui/src/elements/align.rs b/crates/gpui/src/elements/align.rs index b3724c923f0704c5ac9e15920e6db1e5f334d00e..165cfcf190c7d69304c424f0bbed34b048905c44 100644 --- a/crates/gpui/src/elements/align.rs +++ b/crates/gpui/src/elements/align.rs @@ -1,6 +1,6 @@ use crate::{ geometry::{rect::RectF, vector::Vector2F}, - json, AnyElement, Element, SceneBuilder, SizeConstraint, View, ViewContext, + json, AnyElement, Element, LayoutContext, SceneBuilder, SizeConstraint, View, ViewContext, }; use json::ToJson; @@ -48,7 +48,7 @@ impl Element for Align { &mut self, mut constraint: SizeConstraint, view: &mut V, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> (Vector2F, Self::LayoutState) { let mut size = constraint.max; constraint.min = Vector2F::zero(); diff --git a/crates/gpui/src/elements/canvas.rs b/crates/gpui/src/elements/canvas.rs index 36ff4e2cf409222de491694d88ccb3e4aabbe087..bbd8d0393cd038b3e1bde28aa4811280c0dcd768 100644 --- a/crates/gpui/src/elements/canvas.rs +++ b/crates/gpui/src/elements/canvas.rs @@ -34,7 +34,7 @@ where &mut self, constraint: crate::SizeConstraint, _: &mut V, - _: &mut crate::ViewContext, + _: &mut crate::LayoutContext, ) -> (Vector2F, Self::LayoutState) { let x = if constraint.max.x().is_finite() { constraint.max.x() diff --git a/crates/gpui/src/elements/clipped.rs b/crates/gpui/src/elements/clipped.rs index 85466972d39e4957bd12b81d1c0ad556b776b6ce..a87dc3e7735c181a8716a61acf8fc7720c62532c 100644 --- a/crates/gpui/src/elements/clipped.rs +++ b/crates/gpui/src/elements/clipped.rs @@ -3,7 +3,9 @@ use std::ops::Range; use pathfinder_geometry::{rect::RectF, vector::Vector2F}; use serde_json::json; -use crate::{json, AnyElement, Element, SceneBuilder, SizeConstraint, View, ViewContext}; +use crate::{ + json, AnyElement, Element, LayoutContext, SceneBuilder, SizeConstraint, View, ViewContext, +}; pub struct Clipped { child: AnyElement, @@ -23,7 +25,7 @@ impl Element for Clipped { &mut self, constraint: SizeConstraint, view: &mut V, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> (Vector2F, Self::LayoutState) { (self.child.layout(constraint, view, cx), ()) } diff --git a/crates/gpui/src/elements/constrained_box.rs b/crates/gpui/src/elements/constrained_box.rs index fc033c9b433360df8a8e958dd9f75e1b84c3317e..46916c74f1054d442e190a809d69a5fa945ad631 100644 --- a/crates/gpui/src/elements/constrained_box.rs +++ b/crates/gpui/src/elements/constrained_box.rs @@ -5,7 +5,7 @@ use serde_json::json; use crate::{ geometry::{rect::RectF, vector::Vector2F}, - json, AnyElement, Element, SceneBuilder, SizeConstraint, View, ViewContext, + json, AnyElement, Element, LayoutContext, SceneBuilder, SizeConstraint, View, ViewContext, }; pub struct ConstrainedBox { @@ -15,7 +15,7 @@ pub struct ConstrainedBox { pub enum Constraint { Static(SizeConstraint), - Dynamic(Box) -> SizeConstraint>), + Dynamic(Box) -> SizeConstraint>), } impl ToJson for Constraint { @@ -37,7 +37,8 @@ impl ConstrainedBox { pub fn dynamically( mut self, - constraint: impl 'static + FnMut(SizeConstraint, &mut V, &mut ViewContext) -> SizeConstraint, + constraint: impl 'static + + FnMut(SizeConstraint, &mut V, &mut LayoutContext) -> SizeConstraint, ) -> Self { self.constraint = Constraint::Dynamic(Box::new(constraint)); self @@ -119,7 +120,7 @@ impl ConstrainedBox { &mut self, input_constraint: SizeConstraint, view: &mut V, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> SizeConstraint { match &mut self.constraint { Constraint::Static(constraint) => *constraint, @@ -138,7 +139,7 @@ impl Element for ConstrainedBox { &mut self, mut parent_constraint: SizeConstraint, view: &mut V, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> (Vector2F, Self::LayoutState) { let constraint = self.constraint(parent_constraint, view, cx); parent_constraint.min = parent_constraint.min.max(constraint.min); diff --git a/crates/gpui/src/elements/container.rs b/crates/gpui/src/elements/container.rs index 1cbef9a52e5319fbf1b5130e503e504bfc63bd03..3ce323db09775086ba1eb0897d71c6a51a0dd9ac 100644 --- a/crates/gpui/src/elements/container.rs +++ b/crates/gpui/src/elements/container.rs @@ -10,7 +10,7 @@ use crate::{ json::ToJson, platform::CursorStyle, scene::{self, Border, CursorRegion, Quad}, - AnyElement, Element, SceneBuilder, SizeConstraint, View, ViewContext, + AnyElement, Element, LayoutContext, SceneBuilder, SizeConstraint, View, ViewContext, }; use serde::Deserialize; use serde_json::json; @@ -192,7 +192,7 @@ impl Element for Container { &mut self, constraint: SizeConstraint, view: &mut V, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> (Vector2F, Self::LayoutState) { let mut size_buffer = self.margin_size() + self.padding_size(); if !self.style.border.overlay { diff --git a/crates/gpui/src/elements/empty.rs b/crates/gpui/src/elements/empty.rs index d1a3cbafdb1d7b6df56f249d9176219b5ef9e581..42a3824bfcb7f610b715424af6297363a469751b 100644 --- a/crates/gpui/src/elements/empty.rs +++ b/crates/gpui/src/elements/empty.rs @@ -6,7 +6,7 @@ use crate::{ vector::{vec2f, Vector2F}, }, json::{json, ToJson}, - SceneBuilder, View, ViewContext, + LayoutContext, SceneBuilder, View, ViewContext, }; use crate::{Element, SizeConstraint}; @@ -34,7 +34,7 @@ impl Element for Empty { &mut self, constraint: SizeConstraint, _: &mut V, - _: &mut ViewContext, + _: &mut LayoutContext, ) -> (Vector2F, Self::LayoutState) { let x = if constraint.max.x().is_finite() && !self.collapsed { constraint.max.x() diff --git a/crates/gpui/src/elements/expanded.rs b/crates/gpui/src/elements/expanded.rs index 8a59191b7a541e0cc88d82d231bbf04b16b32e8b..1fb935b2b8fa272413698fb14d7cfa8ab9cdf51b 100644 --- a/crates/gpui/src/elements/expanded.rs +++ b/crates/gpui/src/elements/expanded.rs @@ -2,7 +2,7 @@ use std::ops::Range; use crate::{ geometry::{rect::RectF, vector::Vector2F}, - json, AnyElement, Element, SceneBuilder, SizeConstraint, View, ViewContext, + json, AnyElement, Element, LayoutContext, SceneBuilder, SizeConstraint, View, ViewContext, }; use serde_json::json; @@ -42,7 +42,7 @@ impl Element for Expanded { &mut self, mut constraint: SizeConstraint, view: &mut V, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> (Vector2F, Self::LayoutState) { if self.full_width { constraint.min.set_x(constraint.max.x()); diff --git a/crates/gpui/src/elements/flex.rs b/crates/gpui/src/elements/flex.rs index 7fee0006f7c3b4acdc2c97c674acfb39683704c7..e0e8dfc215069b892fbfc1cd25d640fbdb7f18e2 100644 --- a/crates/gpui/src/elements/flex.rs +++ b/crates/gpui/src/elements/flex.rs @@ -2,8 +2,8 @@ use std::{any::Any, cell::Cell, f32::INFINITY, ops::Range, rc::Rc}; use crate::{ json::{self, ToJson, Value}, - AnyElement, Axis, Element, ElementStateHandle, SceneBuilder, SizeConstraint, Vector2FExt, View, - ViewContext, + AnyElement, Axis, Element, ElementStateHandle, LayoutContext, SceneBuilder, SizeConstraint, + Vector2FExt, View, ViewContext, }; use pathfinder_geometry::{ rect::RectF, @@ -74,7 +74,7 @@ impl Flex { remaining_flex: &mut f32, cross_axis_max: &mut f32, view: &mut V, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) { let cross_axis = self.axis.invert(); for child in &mut self.children { @@ -125,7 +125,7 @@ impl Element for Flex { &mut self, constraint: SizeConstraint, view: &mut V, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> (Vector2F, Self::LayoutState) { let mut total_flex = None; let mut fixed_space = 0.0; @@ -214,7 +214,7 @@ impl Element for Flex { } if let Some(scroll_state) = self.scroll_state.as_ref() { - scroll_state.0.update(cx, |scroll_state, _| { + scroll_state.0.update(cx.view_context(), |scroll_state, _| { if let Some(scroll_to) = scroll_state.scroll_to.take() { let visible_start = scroll_state.scroll_position.get(); let visible_end = visible_start + size.along(self.axis); @@ -432,7 +432,7 @@ impl Element for FlexItem { &mut self, constraint: SizeConstraint, view: &mut V, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> (Vector2F, Self::LayoutState) { let size = self.child.layout(constraint, view, cx); (size, ()) diff --git a/crates/gpui/src/elements/hook.rs b/crates/gpui/src/elements/hook.rs index cb22760285ef0a745657b48803490b7dc9f862ab..310b3c25ebeda1778daf3fd060bf24af6be10822 100644 --- a/crates/gpui/src/elements/hook.rs +++ b/crates/gpui/src/elements/hook.rs @@ -3,7 +3,7 @@ use std::ops::Range; use crate::{ geometry::{rect::RectF, vector::Vector2F}, json::json, - AnyElement, Element, SceneBuilder, SizeConstraint, View, ViewContext, + AnyElement, Element, LayoutContext, SceneBuilder, SizeConstraint, View, ViewContext, }; pub struct Hook { @@ -36,7 +36,7 @@ impl Element for Hook { &mut self, constraint: SizeConstraint, view: &mut V, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> (Vector2F, Self::LayoutState) { let size = self.child.layout(constraint, view, cx); if let Some(handler) = self.after_layout.as_mut() { diff --git a/crates/gpui/src/elements/image.rs b/crates/gpui/src/elements/image.rs index 162446c1f022added77887c8f69dc6dbb02196ce..98c5ae6226940946542313021e658243c843b6ea 100644 --- a/crates/gpui/src/elements/image.rs +++ b/crates/gpui/src/elements/image.rs @@ -5,7 +5,8 @@ use crate::{ vector::{vec2f, Vector2F}, }, json::{json, ToJson}, - scene, Border, Element, ImageData, SceneBuilder, SizeConstraint, View, ViewContext, + scene, Border, Element, ImageData, LayoutContext, SceneBuilder, SizeConstraint, View, + ViewContext, }; use serde::Deserialize; use std::{ops::Range, sync::Arc}; @@ -63,7 +64,7 @@ impl Element for Image { &mut self, constraint: SizeConstraint, _: &mut V, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> (Vector2F, Self::LayoutState) { let data = match &self.source { ImageSource::Path(path) => match cx.asset_cache.png(path) { diff --git a/crates/gpui/src/elements/keystroke_label.rs b/crates/gpui/src/elements/keystroke_label.rs index b179d781275542fcfc4e037a590859dc27876a64..c011649b2e946f980cd5b3eb2e7ab448bde8e2e1 100644 --- a/crates/gpui/src/elements/keystroke_label.rs +++ b/crates/gpui/src/elements/keystroke_label.rs @@ -39,7 +39,7 @@ impl Element for KeystrokeLabel { &mut self, constraint: SizeConstraint, view: &mut V, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> (Vector2F, AnyElement) { let mut element = if let Some(keystrokes) = cx.keystrokes_for_action(self.view_id, self.action.as_ref()) diff --git a/crates/gpui/src/elements/label.rs b/crates/gpui/src/elements/label.rs index 2669f4d5f2f1ba058845cef44ce816feb28f8339..9499841b3d46a0d8c77da4e741bd1c623eeb0de4 100644 --- a/crates/gpui/src/elements/label.rs +++ b/crates/gpui/src/elements/label.rs @@ -8,7 +8,7 @@ use crate::{ }, json::{ToJson, Value}, text_layout::{Line, RunStyle}, - Element, SceneBuilder, SizeConstraint, View, ViewContext, + Element, LayoutContext, SceneBuilder, SizeConstraint, View, ViewContext, }; use serde::Deserialize; use serde_json::json; @@ -135,7 +135,7 @@ impl Element for Label { &mut self, constraint: SizeConstraint, _: &mut V, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> (Vector2F, Self::LayoutState) { let runs = self.compute_runs(); let line = cx.text_layout_cache().layout_str( diff --git a/crates/gpui/src/elements/list.rs b/crates/gpui/src/elements/list.rs index 84043f5e1ff135df9596ffe15029d700f96a6458..ca73196c8bb04a1e3dba879c23e667ecba1b7e22 100644 --- a/crates/gpui/src/elements/list.rs +++ b/crates/gpui/src/elements/list.rs @@ -4,7 +4,8 @@ use crate::{ vector::{vec2f, Vector2F}, }, json::json, - AnyElement, Element, MouseRegion, SceneBuilder, SizeConstraint, View, ViewContext, + AnyElement, Element, LayoutContext, MouseRegion, SceneBuilder, SizeConstraint, View, + ViewContext, }; use std::{cell::RefCell, collections::VecDeque, fmt::Debug, ops::Range, rc::Rc}; use sum_tree::{Bias, SumTree}; @@ -99,7 +100,7 @@ impl Element for List { &mut self, constraint: SizeConstraint, view: &mut V, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> (Vector2F, Self::LayoutState) { let state = &mut *self.state.0.borrow_mut(); let size = constraint.max; @@ -452,7 +453,7 @@ impl StateInner { existing_element: Option<&ListItem>, constraint: SizeConstraint, view: &mut V, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> Option>>> { if let Some(ListItem::Rendered(element)) = existing_element { Some(element.clone()) @@ -665,7 +666,15 @@ mod tests { }); let mut list = List::new(state.clone()); - let (size, _) = list.layout(constraint, &mut view, cx); + let mut new_parents = Default::default(); + let mut notify_views_if_parents_change = Default::default(); + let mut layout_cx = LayoutContext::new( + cx, + &mut new_parents, + &mut notify_views_if_parents_change, + false, + ); + let (size, _) = list.layout(constraint, &mut view, &mut layout_cx); assert_eq!(size, vec2f(100., 40.)); assert_eq!( state.0.borrow().items.summary().clone(), @@ -689,7 +698,13 @@ mod tests { cx, ); - let (_, logical_scroll_top) = list.layout(constraint, &mut view, cx); + let mut layout_cx = LayoutContext::new( + cx, + &mut new_parents, + &mut notify_views_if_parents_change, + false, + ); + let (_, logical_scroll_top) = list.layout(constraint, &mut view, &mut layout_cx); assert_eq!( logical_scroll_top, ListOffset { @@ -713,7 +728,13 @@ mod tests { } ); - let (size, logical_scroll_top) = list.layout(constraint, &mut view, cx); + let mut layout_cx = LayoutContext::new( + cx, + &mut new_parents, + &mut notify_views_if_parents_change, + false, + ); + let (size, logical_scroll_top) = list.layout(constraint, &mut view, &mut layout_cx); assert_eq!(size, vec2f(100., 40.)); assert_eq!( state.0.borrow().items.summary().clone(), @@ -831,10 +852,18 @@ mod tests { let mut list = List::new(state.clone()); let window_size = vec2f(width, height); + let mut new_parents = Default::default(); + let mut notify_views_if_parents_change = Default::default(); + let mut layout_cx = LayoutContext::new( + cx, + &mut new_parents, + &mut notify_views_if_parents_change, + false, + ); let (size, logical_scroll_top) = list.layout( SizeConstraint::new(vec2f(0., 0.), window_size), &mut view, - cx, + &mut layout_cx, ); assert_eq!(size, window_size); last_logical_scroll_top = Some(logical_scroll_top); @@ -947,7 +976,7 @@ mod tests { &mut self, _: SizeConstraint, _: &mut V, - _: &mut ViewContext, + _: &mut LayoutContext, ) -> (Vector2F, ()) { (self.size, ()) } diff --git a/crates/gpui/src/elements/mouse_event_handler.rs b/crates/gpui/src/elements/mouse_event_handler.rs index 8abcf5f8354b4ef98d77492468146a6502cc13a0..ed624922d57362d0040a1d7e28a54ae9b3f28e9c 100644 --- a/crates/gpui/src/elements/mouse_event_handler.rs +++ b/crates/gpui/src/elements/mouse_event_handler.rs @@ -10,8 +10,8 @@ use crate::{ CursorRegion, HandlerSet, MouseClick, MouseDown, MouseDownOut, MouseDrag, MouseHover, MouseMove, MouseMoveOut, MouseScrollWheel, MouseUp, MouseUpOut, }, - AnyElement, Element, EventContext, MouseRegion, MouseState, SceneBuilder, SizeConstraint, View, - ViewContext, + AnyElement, Element, EventContext, LayoutContext, MouseRegion, MouseState, SceneBuilder, + SizeConstraint, View, ViewContext, }; use serde_json::json; use std::{marker::PhantomData, ops::Range}; @@ -220,7 +220,7 @@ impl Element for MouseEventHandler { &mut self, constraint: SizeConstraint, view: &mut V, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> (Vector2F, Self::LayoutState) { (self.child.layout(constraint, view, cx), ()) } diff --git a/crates/gpui/src/elements/overlay.rs b/crates/gpui/src/elements/overlay.rs index 0abd1231e2ec2dab8acf70017583fb12848b0df7..0f7e4a35c673ec78b5001420957670f5943ffe0f 100644 --- a/crates/gpui/src/elements/overlay.rs +++ b/crates/gpui/src/elements/overlay.rs @@ -3,7 +3,8 @@ use std::ops::Range; use crate::{ geometry::{rect::RectF, vector::Vector2F}, json::ToJson, - AnyElement, Axis, Element, MouseRegion, SceneBuilder, SizeConstraint, View, ViewContext, + AnyElement, Axis, Element, LayoutContext, MouseRegion, SceneBuilder, SizeConstraint, View, + ViewContext, }; use serde_json::json; @@ -124,7 +125,7 @@ impl Element for Overlay { &mut self, constraint: SizeConstraint, view: &mut V, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> (Vector2F, Self::LayoutState) { let constraint = if self.anchor_position.is_some() { SizeConstraint::new(Vector2F::zero(), cx.window_size()) diff --git a/crates/gpui/src/elements/resizable.rs b/crates/gpui/src/elements/resizable.rs index 37e15541f40ad565a23c14c68876fe6a3e8ad87f..0e78cc07fbce76fa73f6c5106ea218b774042281 100644 --- a/crates/gpui/src/elements/resizable.rs +++ b/crates/gpui/src/elements/resizable.rs @@ -7,7 +7,8 @@ use crate::{ geometry::rect::RectF, platform::{CursorStyle, MouseButton}, scene::MouseDrag, - AnyElement, Axis, Element, ElementStateHandle, MouseRegion, SceneBuilder, View, ViewContext, + AnyElement, Axis, Element, ElementStateHandle, LayoutContext, MouseRegion, SceneBuilder, View, + ViewContext, }; use super::{ConstrainedBox, Hook}; @@ -139,7 +140,7 @@ impl Element for Resizable { &mut self, constraint: crate::SizeConstraint, view: &mut V, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> (Vector2F, Self::LayoutState) { (self.child.layout(constraint, view, cx), ()) } diff --git a/crates/gpui/src/elements/stack.rs b/crates/gpui/src/elements/stack.rs index 348a9b4a3bbaa4ca859d9ef8e394543a4afe087a..196c04d2034a0efa796d9d0ac47dcda6c6f6ab51 100644 --- a/crates/gpui/src/elements/stack.rs +++ b/crates/gpui/src/elements/stack.rs @@ -3,7 +3,7 @@ use std::ops::Range; use crate::{ geometry::{rect::RectF, vector::Vector2F}, json::{self, json, ToJson}, - AnyElement, Element, SceneBuilder, SizeConstraint, View, ViewContext, + AnyElement, Element, LayoutContext, SceneBuilder, SizeConstraint, View, ViewContext, }; /// Element which renders it's children in a stack on top of each other. @@ -34,7 +34,7 @@ impl Element for Stack { &mut self, mut constraint: SizeConstraint, view: &mut V, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> (Vector2F, Self::LayoutState) { let mut size = constraint.min; let mut children = self.children.iter_mut(); diff --git a/crates/gpui/src/elements/svg.rs b/crates/gpui/src/elements/svg.rs index 0f75be82fd1e7894f4c9864d71edcfee1de80a8a..544422132298be738e97a29a0cd9f7e4a56eba38 100644 --- a/crates/gpui/src/elements/svg.rs +++ b/crates/gpui/src/elements/svg.rs @@ -8,7 +8,7 @@ use crate::{ rect::RectF, vector::{vec2f, Vector2F}, }, - scene, Element, SceneBuilder, SizeConstraint, View, ViewContext, + scene, Element, LayoutContext, SceneBuilder, SizeConstraint, View, ViewContext, }; pub struct Svg { @@ -38,7 +38,7 @@ impl Element for Svg { &mut self, constraint: SizeConstraint, _: &mut V, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> (Vector2F, Self::LayoutState) { match cx.asset_cache.svg(&self.path) { Ok(tree) => { diff --git a/crates/gpui/src/elements/text.rs b/crates/gpui/src/elements/text.rs index a92581f6c875e38a0acbf0854c683a0e5fb68a3c..829511c62f0978e58b9c1f8c9e5c5bc486a9a54e 100644 --- a/crates/gpui/src/elements/text.rs +++ b/crates/gpui/src/elements/text.rs @@ -7,8 +7,8 @@ use crate::{ }, json::{ToJson, Value}, text_layout::{Line, RunStyle, ShapedBoundary}, - AppContext, Element, FontCache, SceneBuilder, SizeConstraint, TextLayoutCache, View, - ViewContext, + AppContext, Element, FontCache, LayoutContext, SceneBuilder, SizeConstraint, TextLayoutCache, + View, ViewContext, }; use log::warn; use serde_json::json; @@ -78,7 +78,7 @@ impl Element for Text { &mut self, constraint: SizeConstraint, _: &mut V, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> (Vector2F, Self::LayoutState) { // Convert the string and highlight ranges into an iterator of highlighted chunks. @@ -411,10 +411,18 @@ mod tests { let mut view = TestView; fonts::with_font_cache(cx.font_cache().clone(), || { let mut text = Text::new("Hello\r\n", Default::default()).with_soft_wrap(true); + let mut new_parents = Default::default(); + let mut notify_views_if_parents_change = Default::default(); + let mut layout_cx = LayoutContext::new( + cx, + &mut new_parents, + &mut notify_views_if_parents_change, + false, + ); let (_, state) = text.layout( SizeConstraint::new(Default::default(), vec2f(f32::INFINITY, f32::INFINITY)), &mut view, - cx, + &mut layout_cx, ); assert_eq!(state.shaped_lines.len(), 2); assert_eq!(state.wrap_boundaries.len(), 2); diff --git a/crates/gpui/src/elements/tooltip.rs b/crates/gpui/src/elements/tooltip.rs index a879b996d5780f13555ae4e48a9af283c84e83b7..7b4892fc1c2f37012f0124113c5888c964d0232e 100644 --- a/crates/gpui/src/elements/tooltip.rs +++ b/crates/gpui/src/elements/tooltip.rs @@ -6,7 +6,8 @@ use crate::{ fonts::TextStyle, geometry::{rect::RectF, vector::Vector2F}, json::json, - Action, Axis, ElementStateHandle, SceneBuilder, SizeConstraint, Task, View, ViewContext, + Action, Axis, ElementStateHandle, LayoutContext, SceneBuilder, SizeConstraint, Task, View, + ViewContext, }; use serde::Deserialize; use std::{ @@ -172,7 +173,7 @@ impl Element for Tooltip { &mut self, constraint: SizeConstraint, view: &mut V, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> (Vector2F, Self::LayoutState) { let size = self.child.layout(constraint, view, cx); if let Some(tooltip) = self.tooltip.as_mut() { diff --git a/crates/gpui/src/elements/uniform_list.rs b/crates/gpui/src/elements/uniform_list.rs index e75425dc3f0b5b817762e0cc2bf268288c4433d0..9ccd57b2919727ec1c60e8af525d8b6de73e8f77 100644 --- a/crates/gpui/src/elements/uniform_list.rs +++ b/crates/gpui/src/elements/uniform_list.rs @@ -6,7 +6,7 @@ use crate::{ }, json::{self, json}, platform::ScrollWheelEvent, - AnyElement, MouseRegion, SceneBuilder, View, ViewContext, + AnyElement, LayoutContext, MouseRegion, SceneBuilder, View, ViewContext, }; use json::ToJson; use std::{cell::RefCell, cmp, ops::Range, rc::Rc}; @@ -159,7 +159,7 @@ impl Element for UniformList { &mut self, constraint: SizeConstraint, view: &mut V, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> (Vector2F, Self::LayoutState) { if constraint.max.y().is_infinite() { unimplemented!( diff --git a/crates/terminal_view/src/terminal_element.rs b/crates/terminal_view/src/terminal_element.rs index 392b050f434beb28316eb874fd2fee2c4f0a8c22..366db99622c7f0efe76169ce0d67d6ff6821ddc8 100644 --- a/crates/terminal_view/src/terminal_element.rs +++ b/crates/terminal_view/src/terminal_element.rs @@ -10,8 +10,8 @@ use gpui::{ platform::{CursorStyle, MouseButton}, serde_json::json, text_layout::{Line, RunStyle}, - AnyElement, Element, EventContext, FontCache, ModelContext, MouseRegion, Quad, SceneBuilder, - SizeConstraint, TextLayoutCache, ViewContext, WeakModelHandle, + AnyElement, Element, EventContext, FontCache, LayoutContext, ModelContext, MouseRegion, Quad, + SceneBuilder, SizeConstraint, TextLayoutCache, ViewContext, WeakModelHandle, }; use itertools::Itertools; use language::CursorShape; @@ -561,7 +561,7 @@ impl Element for TerminalElement { &mut self, constraint: gpui::SizeConstraint, view: &mut TerminalView, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> (gpui::geometry::vector::Vector2F, Self::LayoutState) { let settings = cx.global::(); let font_cache = cx.font_cache(); diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 8bd42fed045ec80f51ea7b9432f588dcef6ca2a6..599f04166a0bc03b6455173d747f83a00797fed0 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -24,8 +24,8 @@ use gpui::{ keymap_matcher::KeymapContext, platform::{CursorStyle, MouseButton, NavigationDirection, PromptLevel}, Action, AnyViewHandle, AnyWeakViewHandle, AppContext, AsyncAppContext, Entity, EventContext, - ModelHandle, MouseRegion, Quad, Task, View, ViewContext, ViewHandle, WeakViewHandle, - WindowContext, + LayoutContext, ModelHandle, MouseRegion, Quad, Task, View, ViewContext, ViewHandle, + WeakViewHandle, WindowContext, }; use project::{Project, ProjectEntryId, ProjectPath}; use serde::Deserialize; @@ -1999,7 +1999,7 @@ impl Element for PaneBackdrop { &mut self, constraint: gpui::SizeConstraint, view: &mut V, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> (Vector2F, Self::LayoutState) { let size = self.child.layout(constraint, view, cx); (size, ()) diff --git a/crates/workspace/src/status_bar.rs b/crates/workspace/src/status_bar.rs index 60bc1f81f5a7470af06380c00be595c8bacfec57..c31acc2f999333e27ca3d1b2a388df91796edba8 100644 --- a/crates/workspace/src/status_bar.rs +++ b/crates/workspace/src/status_bar.rs @@ -8,8 +8,8 @@ use gpui::{ vector::{vec2f, Vector2F}, }, json::{json, ToJson}, - AnyElement, AnyViewHandle, Entity, SceneBuilder, SizeConstraint, Subscription, View, - ViewContext, ViewHandle, WindowContext, + AnyElement, AnyViewHandle, Entity, LayoutContext, SceneBuilder, SizeConstraint, Subscription, + View, ViewContext, ViewHandle, WindowContext, }; use settings::Settings; @@ -157,7 +157,7 @@ impl Element for StatusBarElement { &mut self, mut constraint: SizeConstraint, view: &mut StatusBar, - cx: &mut ViewContext, + cx: &mut LayoutContext, ) -> (Vector2F, Self::LayoutState) { let max_width = constraint.max.x(); constraint.min = vec2f(0., constraint.min.y()); From 7f137ed3ddb90de232ec32eb3fef8fbffc738644 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 3 May 2023 16:36:14 +0200 Subject: [PATCH 02/16] Compute view ancestry at layout time --- crates/collab/src/tests.rs | 4 +- crates/collab/src/tests/integration_tests.rs | 20 +- crates/command_palette/src/command_palette.rs | 4 +- crates/diagnostics/src/diagnostics.rs | 8 +- crates/editor/src/editor_tests.rs | 4 +- crates/editor/src/items.rs | 11 +- crates/gpui/src/app.rs | 281 ++++++++++-------- crates/gpui/src/app/menu.rs | 2 +- crates/gpui/src/app/test_app_context.rs | 13 +- crates/gpui/src/app/window.rs | 49 ++- crates/project_symbols/src/project_symbols.rs | 4 +- crates/search/src/buffer_search.rs | 8 +- crates/search/src/project_search.rs | 2 - crates/terminal_view/src/terminal_view.rs | 17 +- crates/workspace/src/pane.rs | 2 - crates/workspace/src/sidebar.rs | 2 +- crates/workspace/src/status_bar.rs | 2 - crates/workspace/src/workspace.rs | 38 +-- 18 files changed, 246 insertions(+), 225 deletions(-) diff --git a/crates/collab/src/tests.rs b/crates/collab/src/tests.rs index 10014932428aa272701e49a971e724da1679714a..81a505cc4e79b403070873bb9529db8ba42b6ce1 100644 --- a/crates/collab/src/tests.rs +++ b/crates/collab/src/tests.rs @@ -462,8 +462,8 @@ impl TestClient { project: &ModelHandle, cx: &mut TestAppContext, ) -> ViewHandle { - let (_, root_view) = cx.add_window(|_| EmptyView); - cx.add_view(&root_view, |cx| Workspace::test_new(project.clone(), cx)) + let (window_id, _) = cx.add_window(|_| EmptyView); + cx.add_view(window_id, |cx| Workspace::test_new(project.clone(), cx)) } } diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index 7d6199b1095b1ecf78568f981b1e9adb9926c4d0..c6f51a9dc4a8e37ca6ad3d5456d8071e51d00546 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -1202,7 +1202,7 @@ async fn test_share_project( cx_c: &mut TestAppContext, ) { deterministic.forbid_parking(); - let (_, window_b) = cx_b.add_window(|_| EmptyView); + let (window_b, _) = cx_b.add_window(|_| EmptyView); let mut server = TestServer::start(&deterministic).await; let client_a = server.create_client(cx_a, "user_a").await; let client_b = server.create_client(cx_b, "user_b").await; @@ -1289,7 +1289,7 @@ async fn test_share_project( .await .unwrap(); - let editor_b = cx_b.add_view(&window_b, |cx| Editor::for_buffer(buffer_b, None, cx)); + let editor_b = cx_b.add_view(window_b, |cx| Editor::for_buffer(buffer_b, None, cx)); // Client A sees client B's selection deterministic.run_until_parked(); @@ -3076,13 +3076,13 @@ async fn test_newline_above_or_below_does_not_move_guest_cursor( .update(cx_a, |p, cx| p.open_buffer((worktree_id, "a.txt"), cx)) .await .unwrap(); - let (_, window_a) = cx_a.add_window(|_| EmptyView); - let editor_a = cx_a.add_view(&window_a, |cx| { + let (window_a, _) = cx_a.add_window(|_| EmptyView); + let editor_a = cx_a.add_view(window_a, |cx| { Editor::for_buffer(buffer_a, Some(project_a), cx) }); let mut editor_cx_a = EditorTestContext { cx: cx_a, - window_id: window_a.id(), + window_id: window_a, editor: editor_a, }; @@ -3091,13 +3091,13 @@ async fn test_newline_above_or_below_does_not_move_guest_cursor( .update(cx_b, |p, cx| p.open_buffer((worktree_id, "a.txt"), cx)) .await .unwrap(); - let (_, window_b) = cx_b.add_window(|_| EmptyView); - let editor_b = cx_b.add_view(&window_b, |cx| { + let (window_b, _) = cx_b.add_window(|_| EmptyView); + let editor_b = cx_b.add_view(window_b, |cx| { Editor::for_buffer(buffer_b, Some(project_b), cx) }); let mut editor_cx_b = EditorTestContext { cx: cx_b, - window_id: window_b.id(), + window_id: window_b, editor: editor_b, }; @@ -3832,8 +3832,8 @@ async fn test_collaborating_with_completion( .update(cx_b, |p, cx| p.open_buffer((worktree_id, "main.rs"), cx)) .await .unwrap(); - let (_, window_b) = cx_b.add_window(|_| EmptyView); - let editor_b = cx_b.add_view(&window_b, |cx| { + let (window_b, _) = cx_b.add_window(|_| EmptyView); + let editor_b = cx_b.add_view(window_b, |cx| { Editor::for_buffer(buffer_b.clone(), Some(project_b.clone()), cx) }); diff --git a/crates/command_palette/src/command_palette.rs b/crates/command_palette/src/command_palette.rs index 441fbb84a6812cc7b7952ab514e9f3f51fbbc253..123c33cba1e8b81d1485b931c79c8786e454028e 100644 --- a/crates/command_palette/src/command_palette.rs +++ b/crates/command_palette/src/command_palette.rs @@ -304,8 +304,8 @@ mod tests { }); let project = Project::test(app_state.fs.clone(), [], cx).await; - let (_, workspace) = cx.add_window(|cx| Workspace::test_new(project.clone(), cx)); - let editor = cx.add_view(&workspace, |cx| { + let (window_id, workspace) = cx.add_window(|cx| Workspace::test_new(project.clone(), cx)); + let editor = cx.add_view(window_id, |cx| { let mut editor = Editor::single_line(None, cx); editor.set_text("abc", cx); editor diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index 17d142ba4b1bf869021475178db439e76c12a510..d82c653a0924dfd8fc12e3f795f99be7a975756d 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/crates/diagnostics/src/diagnostics.rs @@ -852,7 +852,7 @@ mod tests { let language_server_id = LanguageServerId(0); let project = Project::test(fs.clone(), ["/test".as_ref()], cx).await; - let (_, workspace) = cx.add_window(|cx| Workspace::test_new(project.clone(), cx)); + let (window_id, workspace) = cx.add_window(|cx| Workspace::test_new(project.clone(), cx)); // Create some diagnostics project.update(cx, |project, cx| { @@ -939,7 +939,7 @@ mod tests { }); // Open the project diagnostics view while there are already diagnostics. - let view = cx.add_view(&workspace, |cx| { + let view = cx.add_view(window_id, |cx| { ProjectDiagnosticsEditor::new(project.clone(), workspace.downgrade(), cx) }); @@ -1244,9 +1244,9 @@ mod tests { let server_id_1 = LanguageServerId(100); let server_id_2 = LanguageServerId(101); let project = Project::test(fs.clone(), ["/test".as_ref()], cx).await; - let (_, workspace) = cx.add_window(|cx| Workspace::test_new(project.clone(), cx)); + let (window_id, workspace) = cx.add_window(|cx| Workspace::test_new(project.clone(), cx)); - let view = cx.add_view(&workspace, |cx| { + let view = cx.add_view(window_id, |cx| { ProjectDiagnosticsEditor::new(project.clone(), workspace.downgrade(), cx) }); diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index af4bc1674b8e010b1f2e357122b63b07b9d8ff8f..76c34c63d54e9adbb066cebd28f66507392eb7a8 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -493,9 +493,9 @@ async fn test_navigation_history(cx: &mut TestAppContext) { let fs = FakeFs::new(cx.background()); let project = Project::test(fs, [], cx).await; - let (_, workspace) = cx.add_window(|cx| Workspace::test_new(project, cx)); + let (window_id, workspace) = cx.add_window(|cx| Workspace::test_new(project, cx)); let pane = workspace.read_with(cx, |workspace, _| workspace.active_pane().clone()); - cx.add_view(&pane, |cx| { + cx.add_view(window_id, |cx| { let buffer = MultiBuffer::build_simple(&sample_text(300, 5, 'a'), cx); let mut editor = build_editor(buffer.clone(), cx); let handle = cx.handle(); diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index dcd49607fb95186caf302e8a29b7fb720b202451..342967136bec476e98ada623a59b4b65af3cce74 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -3,7 +3,7 @@ use crate::{ movement::surrounding_word, persistence::DB, scroll::ScrollAnchor, Anchor, Autoscroll, Editor, Event, ExcerptId, ExcerptRange, MultiBuffer, MultiBufferSnapshot, NavigationData, ToPoint as _, }; -use anyhow::{anyhow, Context, Result}; +use anyhow::{Context, Result}; use collections::HashSet; use futures::future::try_join_all; use gpui::{ @@ -864,16 +864,13 @@ impl Item for Editor { let buffer = project_item .downcast::() .context("Project item at stored path was not a buffer")?; - let pane = pane - .upgrade(&cx) - .ok_or_else(|| anyhow!("pane was dropped"))?; - Ok(cx.update(|cx| { - cx.add_view(&pane, |cx| { + Ok(pane.update(&mut cx, |_, cx| { + cx.add_view(|cx| { let mut editor = Editor::for_buffer(buffer, Some(project), cx); editor.read_scroll_position_from_db(item_id, workspace_id, cx); editor }) - })) + })?) }) }) .unwrap_or_else(|error| Task::ready(Err(error))) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index f8d6459cf4fc1e6fca69d95ac86ab2b79681e8cc..32ac815232da75181b4787e10d0d7e3155db19dd 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -335,7 +335,7 @@ impl AsyncAppContext { self.0 .borrow_mut() .update_window(window_id, |window| { - window.handle_dispatch_action_from_effect(Some(view_id), action); + window.dispatch_action(Some(view_id), action); }) .ok_or_else(|| anyhow!("window not found")) } @@ -440,7 +440,6 @@ type WindowShouldCloseSubscriptionCallback = Box b pub struct AppContext { models: HashMap>, views: HashMap<(usize, usize), Box>, - pub(crate) parents: HashMap<(usize, usize), ParentId>, windows: HashMap, globals: HashMap>, element_states: HashMap>, @@ -502,7 +501,6 @@ impl AppContext { Self { models: Default::default(), views: Default::default(), - parents: Default::default(), windows: Default::default(), globals: Default::default(), element_states: Default::default(), @@ -1380,18 +1378,6 @@ impl AppContext { self.update_window(window_id, |cx| cx.replace_root_view(build_root_view)) } - pub fn add_view(&mut self, parent: &AnyViewHandle, build_view: F) -> ViewHandle - where - S: View, - F: FnOnce(&mut ViewContext) -> S, - { - self.update_window(parent.window_id, |cx| { - cx.build_and_insert_view(ParentId::View(parent.view_id), |cx| Some(build_view(cx))) - .unwrap() - }) - .unwrap() - } - pub fn read_view(&self, handle: &ViewHandle) -> &T { if let Some(view) = self.views.get(&(handle.window_id, handle.view_id)) { view.as_any().downcast_ref().expect("downcast is type safe") @@ -1451,6 +1437,7 @@ impl AppContext { let mut view = self.views.remove(&(window_id, view_id)).unwrap(); view.release(self); let change_focus_to = self.windows.get_mut(&window_id).and_then(|window| { + window.parents.remove(&view_id); window .invalidation .get_or_insert_with(Default::default) @@ -1462,10 +1449,12 @@ impl AppContext { None } }); - self.parents.remove(&(window_id, view_id)); if let Some(view_id) = change_focus_to { - self.handle_focus_effect(window_id, Some(view_id)); + self.pending_effects.push_back(Effect::Focus { + window_id, + view_id: Some(view_id), + }); } self.pending_effects @@ -1487,7 +1476,9 @@ impl AppContext { let mut refreshing = false; let mut updated_windows = HashSet::default(); + let mut focused_views = HashMap::default(); loop { + self.remove_dropped_entities(); if let Some(effect) = self.pending_effects.pop_front() { match effect { Effect::Subscription { @@ -1561,7 +1552,7 @@ impl AppContext { } Effect::Focus { window_id, view_id } => { - self.handle_focus_effect(window_id, view_id); + focused_views.insert(window_id, view_id); } Effect::FocusObservation { @@ -1661,10 +1652,7 @@ impl AppContext { ), } self.pending_notifications.clear(); - self.remove_dropped_entities(); } else { - self.remove_dropped_entities(); - for window_id in self.windows.keys().cloned().collect::>() { self.update_window(window_id, |cx| { let invalidation = if refreshing { @@ -1688,6 +1676,10 @@ impl AppContext { }); } + for (window_id, view_id) in focused_views.drain() { + self.handle_focus_effect(window_id, view_id); + } + if self.pending_effects.is_empty() { for callback in after_window_update_callbacks.drain(..) { callback(self); @@ -2882,38 +2874,6 @@ impl<'a, 'b, V: View> ViewContext<'a, 'b, V> { }); } - pub fn add_view(&mut self, build_view: F) -> ViewHandle - where - S: View, - F: FnOnce(&mut ViewContext) -> S, - { - self.window_context - .build_and_insert_view(ParentId::View(self.view_id), |cx| Some(build_view(cx))) - .unwrap() - } - - pub fn add_option_view(&mut self, build_view: F) -> Option> - where - S: View, - F: FnOnce(&mut ViewContext) -> Option, - { - self.window_context - .build_and_insert_view(ParentId::View(self.view_id), build_view) - } - - pub fn reparent(&mut self, view_handle: &AnyViewHandle) { - if self.window_id != view_handle.window_id { - panic!("Can't reparent view to a view from a different window"); - } - self.parents - .remove(&(view_handle.window_id, view_handle.view_id)); - let new_parent_id = self.view_id; - self.parents.insert( - (view_handle.window_id, view_handle.view_id), - ParentId::View(new_parent_id), - ); - } - pub fn subscribe(&mut self, handle: &H, mut callback: F) -> Subscription where E: Entity, @@ -4562,9 +4522,9 @@ mod tests { } } - let (_, root_view) = cx.add_window(|cx| View::new(None, cx)); - let handle_1 = cx.add_view(&root_view, |cx| View::new(None, cx)); - let handle_2 = cx.add_view(&root_view, |cx| View::new(Some(handle_1.clone()), cx)); + let (window_id, _root_view) = cx.add_window(|cx| View::new(None, cx)); + let handle_1 = cx.add_view(window_id, |cx| View::new(None, cx)); + let handle_2 = cx.add_view(window_id, |cx| View::new(Some(handle_1.clone()), cx)); assert_eq!(cx.read(|cx| cx.views.len()), 3); handle_1.update(cx, |view, cx| { @@ -4724,8 +4684,8 @@ mod tests { type Event = String; } - let (_, handle_1) = cx.add_window(|_| TestView::default()); - let handle_2 = cx.add_view(&handle_1, |_| TestView::default()); + let (window_id, handle_1) = cx.add_window(|_| TestView::default()); + let handle_2 = cx.add_view(window_id, |_| TestView::default()); let handle_3 = cx.add_model(|_| Model); handle_1.update(cx, |_, cx| { @@ -4951,9 +4911,9 @@ mod tests { type Event = (); } - let (_, root_view) = cx.add_window(|_| TestView::default()); - let observing_view = cx.add_view(&root_view, |_| TestView::default()); - let emitting_view = cx.add_view(&root_view, |_| TestView::default()); + let (window_id, _root_view) = cx.add_window(|_| TestView::default()); + let observing_view = cx.add_view(window_id, |_| TestView::default()); + let emitting_view = cx.add_view(window_id, |_| TestView::default()); let observing_model = cx.add_model(|_| Model); let observed_model = cx.add_model(|_| Model); @@ -5078,8 +5038,8 @@ mod tests { type Event = (); } - let (_, root_view) = cx.add_window(|_| TestView::default()); - let observing_view = cx.add_view(&root_view, |_| TestView::default()); + let (window_id, _root_view) = cx.add_window(|_| TestView::default()); + let observing_view = cx.add_view(window_id, |_| TestView::default()); let observing_model = cx.add_model(|_| Model); let observed_model = cx.add_model(|_| Model); @@ -5201,9 +5161,9 @@ mod tests { } } - let (_, root_view) = cx.add_window(|_| View); - let observing_view = cx.add_view(&root_view, |_| View); - let observed_view = cx.add_view(&root_view, |_| View); + let (window_id, _root_view) = cx.add_window(|_| View); + let observing_view = cx.add_view(window_id, |_| View); + let observed_view = cx.add_view(window_id, |_| View); let observation_count = Rc::new(RefCell::new(0)); observing_view.update(cx, |_, cx| { @@ -5252,6 +5212,7 @@ mod tests { struct View { name: String, events: Arc>>, + child: Option, } impl Entity for View { @@ -5259,8 +5220,11 @@ mod tests { } impl super::View for View { - fn render(&mut self, _: &mut ViewContext) -> AnyElement { - Empty::new().into_any() + fn render(&mut self, cx: &mut ViewContext) -> AnyElement { + self.child + .as_ref() + .map(|child| ChildView::new(child, cx).into_any()) + .unwrap_or(Empty::new().into_any()) } fn ui_name() -> &'static str { @@ -5284,11 +5248,22 @@ mod tests { let (window_id, view_1) = cx.add_window(|_| View { events: view_events.clone(), name: "view 1".to_string(), + child: None, }); - let view_2 = cx.add_view(&view_1, |_| View { - events: view_events.clone(), - name: "view 2".to_string(), - }); + let view_2 = cx + .update_window(window_id, |cx| { + let view_2 = cx.add_view(|_| View { + events: view_events.clone(), + name: "view 2".to_string(), + child: None, + }); + view_1.update(cx, |view_1, cx| { + view_1.child = Some(view_2.clone().into_any()); + cx.notify(); + }); + view_2 + }) + .unwrap(); let observed_events: Arc>> = Default::default(); view_1.update(cx, |_, cx| { @@ -5325,7 +5300,7 @@ mod tests { assert_eq!(mem::take(&mut *observed_events.lock()), Vec::<&str>::new()); view_1.update(cx, |_, cx| { - // Ensure focus events are sent for all intermediate focuses + // Ensure only the last focus event is honored. cx.focus(&view_2); cx.focus(&view_1); cx.focus(&view_2); @@ -5337,22 +5312,11 @@ mod tests { }); assert_eq!( mem::take(&mut *view_events.lock()), - [ - "view 1 blurred", - "view 2 focused", - "view 2 blurred", - "view 1 focused", - "view 1 blurred", - "view 2 focused" - ], + ["view 1 blurred", "view 2 focused"], ); assert_eq!( mem::take(&mut *observed_events.lock()), [ - "view 2 observed view 1's blur", - "view 1 observed view 2's focus", - "view 1 observed view 2's blur", - "view 2 observed view 1's focus", "view 2 observed view 1's blur", "view 1 observed view 2's focus" ] @@ -5388,7 +5352,11 @@ mod tests { ] ); - view_1.update(cx, |_, _| drop(view_2)); + println!("====================="); + view_1.update(cx, |view, _| { + drop(view_2); + view.child = None; + }); assert_eq!(mem::take(&mut *view_events.lock()), ["view 1 focused"]); assert_eq!(mem::take(&mut *observed_events.lock()), Vec::<&str>::new()); } @@ -5433,6 +5401,7 @@ mod tests { fn test_dispatch_action(cx: &mut AppContext) { struct ViewA { id: usize, + child: Option, } impl Entity for ViewA { @@ -5440,8 +5409,11 @@ mod tests { } impl View for ViewA { - fn render(&mut self, _: &mut ViewContext) -> AnyElement { - Empty::new().into_any() + fn render(&mut self, cx: &mut ViewContext) -> AnyElement { + self.child + .as_ref() + .map(|child| ChildView::new(child, cx).into_any()) + .unwrap_or(Empty::new().into_any()) } fn ui_name() -> &'static str { @@ -5451,6 +5423,7 @@ mod tests { struct ViewB { id: usize, + child: Option, } impl Entity for ViewB { @@ -5458,8 +5431,11 @@ mod tests { } impl View for ViewB { - fn render(&mut self, _: &mut ViewContext) -> AnyElement { - Empty::new().into_any() + fn render(&mut self, cx: &mut ViewContext) -> AnyElement { + self.child + .as_ref() + .map(|child| ChildView::new(child, cx).into_any()) + .unwrap_or(Empty::new().into_any()) } fn ui_name() -> &'static str { @@ -5496,7 +5472,7 @@ mod tests { if view.id != 1 { cx.add_view(|cx| { cx.propagate_action(); // Still works on a nested ViewContext - ViewB { id: 5 } + ViewB { id: 5, child: None } }); } actions.borrow_mut().push(format!("{} b", view.id)); @@ -5534,13 +5510,41 @@ mod tests { }) .detach(); - let (window_id, view_1) = cx.add_window(Default::default(), |_| ViewA { id: 1 }); - let view_2 = cx.add_view(&view_1, |_| ViewB { id: 2 }); - let view_3 = cx.add_view(&view_2, |_| ViewA { id: 3 }); - let view_4 = cx.add_view(&view_3, |_| ViewB { id: 4 }); + let (window_id, view_1) = + cx.add_window(Default::default(), |_| ViewA { id: 1, child: None }); + let view_2 = cx + .update_window(window_id, |cx| { + let child = cx.add_view(|_| ViewB { id: 2, child: None }); + view_1.update(cx, |view, cx| { + view.child = Some(child.clone().into_any()); + cx.notify(); + }); + child + }) + .unwrap(); + let view_3 = cx + .update_window(window_id, |cx| { + let child = cx.add_view(|_| ViewA { id: 3, child: None }); + view_2.update(cx, |view, cx| { + view.child = Some(child.clone().into_any()); + cx.notify(); + }); + child + }) + .unwrap(); + let view_4 = cx + .update_window(window_id, |cx| { + let child = cx.add_view(|_| ViewB { id: 4, child: None }); + view_3.update(cx, |view, cx| { + view.child = Some(child.clone().into_any()); + cx.notify(); + }); + child + }) + .unwrap(); cx.update_window(window_id, |cx| { - cx.handle_dispatch_action_from_effect(Some(view_4.id()), &Action("bar".to_string())) + cx.dispatch_action(Some(view_4.id()), &Action("bar".to_string())) }); assert_eq!( @@ -5561,13 +5565,32 @@ mod tests { // Remove view_1, which doesn't propagate the action - let (window_id, view_2) = cx.add_window(Default::default(), |_| ViewB { id: 2 }); - let view_3 = cx.add_view(&view_2, |_| ViewA { id: 3 }); - let view_4 = cx.add_view(&view_3, |_| ViewB { id: 4 }); + let (window_id, view_2) = + cx.add_window(Default::default(), |_| ViewB { id: 2, child: None }); + let view_3 = cx + .update_window(window_id, |cx| { + let child = cx.add_view(|_| ViewA { id: 3, child: None }); + view_2.update(cx, |view, cx| { + view.child = Some(child.clone().into_any()); + cx.notify(); + }); + child + }) + .unwrap(); + let view_4 = cx + .update_window(window_id, |cx| { + let child = cx.add_view(|_| ViewB { id: 4, child: None }); + view_3.update(cx, |view, cx| { + view.child = Some(child.clone().into_any()); + cx.notify(); + }); + child + }) + .unwrap(); actions.borrow_mut().clear(); cx.update_window(window_id, |cx| { - cx.handle_dispatch_action_from_effect(Some(view_4.id()), &Action("bar".to_string())) + cx.dispatch_action(Some(view_4.id()), &Action("bar".to_string())) }); assert_eq!( @@ -5599,6 +5622,7 @@ mod tests { struct View { id: usize, keymap_context: KeymapContext, + child: Option, } impl Entity for View { @@ -5606,8 +5630,11 @@ mod tests { } impl super::View for View { - fn render(&mut self, _: &mut ViewContext) -> AnyElement { - Empty::new().into_any() + fn render(&mut self, cx: &mut ViewContext) -> AnyElement { + self.child + .as_ref() + .map(|child| ChildView::new(child, cx).into_any()) + .unwrap_or(Empty::new().into_any()) } fn ui_name() -> &'static str { @@ -5624,6 +5651,7 @@ mod tests { View { id, keymap_context: KeymapContext::default(), + child: None, } } } @@ -5638,11 +5666,17 @@ mod tests { view_3.keymap_context.add_identifier("b"); view_3.keymap_context.add_identifier("c"); - let (window_id, view_1) = cx.add_window(Default::default(), |_| view_1); - let view_2 = cx.add_view(&view_1, |_| view_2); - let _view_3 = cx.add_view(&view_2, |cx| { - cx.focus_self(); - view_3 + let (window_id, _view_1) = cx.add_window(Default::default(), |cx| { + let view_2 = cx.add_view(|cx| { + let view_3 = cx.add_view(|cx| { + cx.focus_self(); + view_3 + }); + view_2.child = Some(view_3.into_any()); + view_2 + }); + view_1.child = Some(view_2.into_any()); + view_1 }); // This binding only dispatches an action on view 2 because that view will have @@ -5722,7 +5756,9 @@ mod tests { fn test_keystrokes_for_action(cx: &mut AppContext) { actions!(test, [Action1, Action2, GlobalAction]); - struct View1 {} + struct View1 { + child: ViewHandle, + } struct View2 {} impl Entity for View1 { @@ -5733,8 +5769,8 @@ mod tests { } impl super::View for View1 { - fn render(&mut self, _: &mut ViewContext) -> AnyElement { - Empty::new().into_any() + fn render(&mut self, cx: &mut ViewContext) -> AnyElement { + ChildView::new(&self.child, cx).into_any() } fn ui_name() -> &'static str { "View1" @@ -5749,11 +5785,14 @@ mod tests { } } - let (window_id, view_1) = cx.add_window(Default::default(), |_| View1 {}); - let view_2 = cx.add_view(&view_1, |cx| { - cx.focus_self(); - View2 {} + let (window_id, view_1) = cx.add_window(Default::default(), |cx| { + let view_2 = cx.add_view(|cx| { + cx.focus_self(); + View2 {} + }); + View1 { child: view_2 } }); + let view_2 = view_1.read(cx).child.clone(); cx.add_action(|_: &mut View1, _: &Action1, _cx| {}); cx.add_action(|_: &mut View2, _: &Action2, _cx| {}); @@ -5952,8 +5991,8 @@ mod tests { #[crate::test(self)] #[should_panic(expected = "view dropped with pending condition")] async fn test_view_condition_panic_on_drop(cx: &mut TestAppContext) { - let (_, root_view) = cx.add_window(|_| TestView::default()); - let view = cx.add_view(&root_view, |_| TestView::default()); + let (window_id, _root_view) = cx.add_window(|_| TestView::default()); + let view = cx.add_view(window_id, |_| TestView::default()); let condition = view.condition(cx, |_, _| false); cx.update(|_| drop(view)); @@ -5986,10 +6025,12 @@ mod tests { ); }); - let view = cx.add_view(&root_view, |cx| { - cx.refresh_windows(); - View(0) - }); + let view = cx + .update_window(window_id, |cx| { + cx.refresh_windows(); + cx.add_view(|_| View(0)) + }) + .unwrap(); cx.update_window(window_id, |cx| { assert_eq!( diff --git a/crates/gpui/src/app/menu.rs b/crates/gpui/src/app/menu.rs index ed23296618428465acdc2e5173e91dcc2a353e24..a2ac13984bee5fcc7c248e5750080c84a5353aa0 100644 --- a/crates/gpui/src/app/menu.rs +++ b/crates/gpui/src/app/menu.rs @@ -81,7 +81,7 @@ pub(crate) fn setup_menu_handlers(foreground_platform: &dyn ForegroundPlatform, let dispatched = cx .update_window(main_window_id, |cx| { if let Some(view_id) = cx.focused_view_id() { - cx.handle_dispatch_action_from_effect(Some(view_id), action); + cx.dispatch_action(Some(view_id), action); true } else { false diff --git a/crates/gpui/src/app/test_app_context.rs b/crates/gpui/src/app/test_app_context.rs index 2d079a604287dc63cf0d36832e6b5ad87a3d7afa..af01eec1beb81278f60e6677efdc1f4d8e01ad0d 100644 --- a/crates/gpui/src/app/test_app_context.rs +++ b/crates/gpui/src/app/test_app_context.rs @@ -4,9 +4,9 @@ use crate::{ keymap_matcher::Keystroke, platform, platform::{Event, InputHandler, KeyDownEvent, Platform}, - Action, AnyViewHandle, AppContext, BorrowAppContext, BorrowWindowContext, Entity, FontCache, - Handle, ModelContext, ModelHandle, Subscription, Task, View, ViewContext, ViewHandle, - WeakHandle, WindowContext, + Action, AppContext, BorrowAppContext, BorrowWindowContext, Entity, FontCache, Handle, + ModelContext, ModelHandle, Subscription, Task, View, ViewContext, ViewHandle, WeakHandle, + WindowContext, }; use collections::BTreeMap; use futures::Future; @@ -75,7 +75,7 @@ impl TestAppContext { self.cx .borrow_mut() .update_window(window_id, |window| { - window.handle_dispatch_action_from_effect(window.focused_view_id(), &action); + window.dispatch_action(window.focused_view_id(), &action); }) .expect("window not found"); } @@ -153,12 +153,13 @@ impl TestAppContext { (window_id, view) } - pub fn add_view(&mut self, parent_handle: &AnyViewHandle, build_view: F) -> ViewHandle + pub fn add_view(&mut self, window_id: usize, build_view: F) -> ViewHandle where T: View, F: FnOnce(&mut ViewContext) -> T, { - self.cx.borrow_mut().add_view(parent_handle, build_view) + self.update_window(window_id, |cx| cx.add_view(build_view)) + .expect("window not found") } pub fn observe_global(&mut self, callback: F) -> Subscription diff --git a/crates/gpui/src/app/window.rs b/crates/gpui/src/app/window.rs index f9ccef7ae077268e5f17ae9ae09a8c8b1e53ef6e..3d04b46e06094a55bb66c1feaafda4fd22961736 100644 --- a/crates/gpui/src/app/window.rs +++ b/crates/gpui/src/app/window.rs @@ -14,8 +14,8 @@ use crate::{ text_layout::TextLayoutCache, util::post_inc, Action, AnyView, AnyViewHandle, AppContext, BorrowAppContext, BorrowWindowContext, Effect, - Element, Entity, Handle, LayoutContext, MouseRegion, MouseRegionId, ParentId, SceneBuilder, - Subscription, View, ViewContext, ViewHandle, WindowInvalidation, + Element, Entity, Handle, LayoutContext, MouseRegion, MouseRegionId, SceneBuilder, Subscription, + View, ViewContext, ViewHandle, WindowInvalidation, }; use anyhow::{anyhow, bail, Result}; use collections::{HashMap, HashSet}; @@ -39,6 +39,7 @@ use super::Reference; pub struct Window { pub(crate) root_view: Option, pub(crate) focused_view_id: Option, + pub(crate) parents: HashMap, pub(crate) is_active: bool, pub(crate) is_fullscreen: bool, pub(crate) invalidation: Option, @@ -72,6 +73,7 @@ impl Window { let mut window = Self { root_view: None, focused_view_id: None, + parents: Default::default(), is_active: false, invalidation: None, is_fullscreen: false, @@ -90,9 +92,7 @@ impl Window { }; let mut window_context = WindowContext::mutable(cx, &mut window, window_id); - let root_view = window_context - .build_and_insert_view(ParentId::Root, |cx| Some(build_view(cx))) - .unwrap(); + let root_view = window_context.add_view(|cx| build_view(cx)); if let Some(invalidation) = window_context.window.invalidation.take() { window_context.invalidate(invalidation, appearance); } @@ -471,8 +471,7 @@ impl<'a> WindowContext<'a> { MatchResult::Pending => true, MatchResult::Matches(matches) => { for (view_id, action) in matches { - if self.handle_dispatch_action_from_effect(Some(*view_id), action.as_ref()) - { + if self.dispatch_action(Some(*view_id), action.as_ref()) { self.keystroke_matcher.clear_pending(); handled_by = Some(action.boxed_clone()); break; @@ -943,6 +942,7 @@ impl<'a> WindowContext<'a> { self, )?; + self.window.parents = new_parents; self.window .rendered_views .insert(root_view_id, rendered_root); @@ -1017,14 +1017,11 @@ impl<'a> WindowContext<'a> { self.window.is_fullscreen } - pub(crate) fn handle_dispatch_action_from_effect( - &mut self, - view_id: Option, - action: &dyn Action, - ) -> bool { + pub(crate) fn dispatch_action(&mut self, view_id: Option, action: &dyn Action) -> bool { if let Some(view_id) = view_id { self.halt_action_dispatch = false; self.visit_dispatch_path(view_id, |view_id, capture_phase, cx| { + dbg!(view_id); cx.update_any_view(view_id, |view, cx| { let type_id = view.as_any().type_id(); if let Some((name, mut handlers)) = cx @@ -1067,9 +1064,7 @@ impl<'a> WindowContext<'a> { std::iter::once(view_id) .into_iter() .chain(std::iter::from_fn(move || { - if let Some(ParentId::View(parent_id)) = - self.parents.get(&(self.window_id, view_id)) - { + if let Some(parent_id) = self.window.parents.get(&view_id) { view_id = *parent_id; Some(view_id) } else { @@ -1081,7 +1076,7 @@ impl<'a> WindowContext<'a> { /// Returns the id of the parent of the given view, or none if the given /// view is the root. pub(crate) fn parent(&self, view_id: usize) -> Option { - if let Some(ParentId::View(view_id)) = self.parents.get(&(self.window_id, view_id)) { + if let Some(view_id) = self.window.parents.get(&view_id) { Some(*view_id) } else { None @@ -1170,27 +1165,27 @@ impl<'a> WindowContext<'a> { V: View, F: FnOnce(&mut ViewContext) -> V, { - let root_view = self - .build_and_insert_view(ParentId::Root, |cx| Some(build_root_view(cx))) - .unwrap(); + let root_view = self.add_view(|cx| build_root_view(cx)); self.window.root_view = Some(root_view.clone().into_any()); self.window.focused_view_id = Some(root_view.id()); root_view } - pub(crate) fn build_and_insert_view( - &mut self, - parent_id: ParentId, - build_view: F, - ) -> Option> + pub fn add_view(&mut self, build_view: F) -> ViewHandle + where + T: View, + F: FnOnce(&mut ViewContext) -> T, + { + self.add_option_view(|cx| Some(build_view(cx))).unwrap() + } + + pub fn add_option_view(&mut self, build_view: F) -> Option> where T: View, F: FnOnce(&mut ViewContext) -> Option, { let window_id = self.window_id; let view_id = post_inc(&mut self.next_entity_id); - // Make sure we can tell child views about their parentu - self.parents.insert((window_id, view_id), parent_id); let mut cx = ViewContext::mutable(self, view_id); let handle = if let Some(view) = build_view(&mut cx) { self.views.insert((window_id, view_id), Box::new(view)); @@ -1201,7 +1196,6 @@ impl<'a> WindowContext<'a> { .insert(view_id); Some(ViewHandle::new(window_id, view_id, &self.ref_counts)) } else { - self.parents.remove(&(window_id, view_id)); None }; handle @@ -1385,6 +1379,7 @@ impl Element for ChildView { cx: &mut LayoutContext, ) -> (Vector2F, Self::LayoutState) { if let Some(mut rendered_view) = cx.window.rendered_views.remove(&self.view_id) { + cx.new_parents.insert(self.view_id, cx.view_id()); let size = rendered_view .layout( constraint, diff --git a/crates/project_symbols/src/project_symbols.rs b/crates/project_symbols/src/project_symbols.rs index 6720ef93dec0a2d96701d20c49a75c169189a0f5..25828f17cac959659d89770d5f353c0ba93ab81b 100644 --- a/crates/project_symbols/src/project_symbols.rs +++ b/crates/project_symbols/src/project_symbols.rs @@ -318,10 +318,10 @@ mod tests { }, ); - let (_, workspace) = cx.add_window(|cx| Workspace::test_new(project.clone(), cx)); + let (window_id, workspace) = cx.add_window(|cx| Workspace::test_new(project.clone(), cx)); // Create the project symbols view. - let symbols = cx.add_view(&workspace, |cx| { + let symbols = cx.add_view(window_id, |cx| { ProjectSymbols::new( ProjectSymbolsDelegate::new(workspace.downgrade(), project.clone()), cx, diff --git a/crates/search/src/buffer_search.rs b/crates/search/src/buffer_search.rs index ee5a2e8332e4cc90cab44642fd3b7df246849341..91284a545f3c50b1f19cfb391358ac70b895ae51 100644 --- a/crates/search/src/buffer_search.rs +++ b/crates/search/src/buffer_search.rs @@ -670,13 +670,11 @@ mod tests { cx, ) }); - let (_, root_view) = cx.add_window(|_| EmptyView); + let (window_id, _root_view) = cx.add_window(|_| EmptyView); - let editor = cx.add_view(&root_view, |cx| { - Editor::for_buffer(buffer.clone(), None, cx) - }); + let editor = cx.add_view(window_id, |cx| Editor::for_buffer(buffer.clone(), None, cx)); - let search_bar = cx.add_view(&root_view, |cx| { + let search_bar = cx.add_view(window_id, |cx| { let mut search_bar = BufferSearchBar::new(cx); search_bar.set_active_pane_item(Some(&editor), cx); search_bar.show(false, true, cx); diff --git a/crates/search/src/project_search.rs b/crates/search/src/project_search.rs index ea29f9cfda8f219465103db148b4019f1c0c6203..6266c571bf29ab5363c60780d2c6971de45d2bc5 100644 --- a/crates/search/src/project_search.rs +++ b/crates/search/src/project_search.rs @@ -939,8 +939,6 @@ impl ToolbarItemView for ProjectSearchBar { self.subscription = None; self.active_project_search = None; if let Some(search) = active_pane_item.and_then(|i| i.downcast::()) { - let query_editor = search.read(cx).query_editor.clone(); - cx.reparent(&query_editor); self.subscription = Some(cx.observe(&search, |_, _, cx| cx.notify())); self.active_project_search = Some(search); ToolbarItemLocation::PrimaryLeft { diff --git a/crates/terminal_view/src/terminal_view.rs b/crates/terminal_view/src/terminal_view.rs index 5e04fc982556cc638752cd0a7a4774a27c0d2654..a40cb4508d7218b87c0bbd81c963caf243d48e93 100644 --- a/crates/terminal_view/src/terminal_view.rs +++ b/crates/terminal_view/src/terminal_view.rs @@ -2,7 +2,6 @@ mod persistence; pub mod terminal_button; pub mod terminal_element; -use anyhow::anyhow; use context_menu::{ContextMenu, ContextMenuItem}; use dirs::home_dir; use gpui::{ @@ -628,16 +627,12 @@ impl Item for TerminalView { }) }); - let pane = pane - .upgrade(&cx) - .ok_or_else(|| anyhow!("pane was dropped"))?; - cx.update(|cx| { - let terminal = project.update(cx, |project, cx| { - project.create_terminal(cwd, window_id, cx) - })?; - - Ok(cx.add_view(&pane, |cx| TerminalView::new(terminal, workspace_id, cx))) - }) + let terminal = project.update(&mut cx, |project, cx| { + project.create_terminal(cwd, window_id, cx) + })?; + Ok(pane.update(&mut cx, |_, cx| { + cx.add_view(|cx| TerminalView::new(terminal, workspace_id, cx)) + })?) }) } diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 599f04166a0bc03b6455173d747f83a00797fed0..8b0107d3688a24467ddde6969e0991bf34d0cc9b 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -537,7 +537,6 @@ impl Pane { // If the item already exists, move it to the desired destination and activate it pane.update(cx, |pane, cx| { if existing_item_index != insertion_index { - cx.reparent(item.as_any()); let existing_item_is_active = existing_item_index == pane.active_item_index; // If the caller didn't specify a destination and the added item is already @@ -567,7 +566,6 @@ impl Pane { }); } else { pane.update(cx, |pane, cx| { - cx.reparent(item.as_any()); pane.items.insert(insertion_index, item); if insertion_index <= pane.active_item_index { pane.active_item_index += 1; diff --git a/crates/workspace/src/sidebar.rs b/crates/workspace/src/sidebar.rs index 2b114d83eccf6e7a68ae4d8db0ec96e41330d798..f37bd6da5f248a3a3a942482b2647e3790dcdfe6 100644 --- a/crates/workspace/src/sidebar.rs +++ b/crates/workspace/src/sidebar.rs @@ -146,7 +146,7 @@ impl Sidebar { } }), ]; - cx.reparent(&view); + self.items.push(Item { icon_path, tooltip, diff --git a/crates/workspace/src/status_bar.rs b/crates/workspace/src/status_bar.rs index c31acc2f999333e27ca3d1b2a388df91796edba8..b4de6b3575d392497ccba3c18274820bd51fa371 100644 --- a/crates/workspace/src/status_bar.rs +++ b/crates/workspace/src/status_bar.rs @@ -93,7 +93,6 @@ impl StatusBar { where T: 'static + StatusItemView, { - cx.reparent(item.as_any()); self.left_items.push(Box::new(item)); cx.notify(); } @@ -102,7 +101,6 @@ impl StatusBar { where T: 'static + StatusItemView, { - cx.reparent(item.as_any()); self.right_items.push(Box::new(item)); cx.notify(); } diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 5a008537376d86306d099cf10ebdfe2ee81afa9f..f9a86d3dcc17b38be47022d6d0306305c54f50a4 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -3118,10 +3118,10 @@ mod tests { let fs = FakeFs::new(cx.background()); let project = Project::test(fs, [], cx).await; - let (_, workspace) = cx.add_window(|cx| Workspace::test_new(project.clone(), cx)); + let (window_id, workspace) = cx.add_window(|cx| Workspace::test_new(project.clone(), cx)); // Adding an item with no ambiguity renders the tab without detail. - let item1 = cx.add_view(&workspace, |_| { + let item1 = cx.add_view(window_id, |_| { let mut item = TestItem::new(); item.tab_descriptions = Some(vec!["c", "b1/c", "a/b1/c"]); item @@ -3133,7 +3133,7 @@ mod tests { // Adding an item that creates ambiguity increases the level of detail on // both tabs. - let item2 = cx.add_view(&workspace, |_| { + let item2 = cx.add_view(window_id, |_| { let mut item = TestItem::new(); item.tab_descriptions = Some(vec!["c", "b2/c", "a/b2/c"]); item @@ -3147,7 +3147,7 @@ mod tests { // Adding an item that creates ambiguity increases the level of detail only // on the ambiguous tabs. In this case, the ambiguity can't be resolved so // we stop at the highest detail available. - let item3 = cx.add_view(&workspace, |_| { + let item3 = cx.add_view(window_id, |_| { let mut item = TestItem::new(); item.tab_descriptions = Some(vec!["c", "b2/c", "a/b2/c"]); item @@ -3187,10 +3187,10 @@ mod tests { project.worktrees(cx).next().unwrap().read(cx).id() }); - let item1 = cx.add_view(&workspace, |cx| { + let item1 = cx.add_view(window_id, |cx| { TestItem::new().with_project_items(&[TestProjectItem::new(1, "one.txt", cx)]) }); - let item2 = cx.add_view(&workspace, |cx| { + let item2 = cx.add_view(window_id, |cx| { TestItem::new().with_project_items(&[TestProjectItem::new(2, "two.txt", cx)]) }); @@ -3275,15 +3275,15 @@ mod tests { let (window_id, workspace) = cx.add_window(|cx| Workspace::test_new(project.clone(), cx)); // When there are no dirty items, there's nothing to do. - let item1 = cx.add_view(&workspace, |_| TestItem::new()); + let item1 = cx.add_view(window_id, |_| TestItem::new()); workspace.update(cx, |w, cx| w.add_item(Box::new(item1.clone()), cx)); let task = workspace.update(cx, |w, cx| w.prepare_to_close(false, cx)); assert!(task.await.unwrap()); // When there are dirty untitled items, prompt to save each one. If the user // cancels any prompt, then abort. - let item2 = cx.add_view(&workspace, |_| TestItem::new().with_dirty(true)); - let item3 = cx.add_view(&workspace, |cx| { + let item2 = cx.add_view(window_id, |_| TestItem::new().with_dirty(true)); + let item3 = cx.add_view(window_id, |cx| { TestItem::new() .with_dirty(true) .with_project_items(&[TestProjectItem::new(1, "1.txt", cx)]) @@ -3309,24 +3309,24 @@ mod tests { let project = Project::test(fs, None, cx).await; let (window_id, workspace) = cx.add_window(|cx| Workspace::test_new(project, cx)); - let item1 = cx.add_view(&workspace, |cx| { + let item1 = cx.add_view(window_id, |cx| { TestItem::new() .with_dirty(true) .with_project_items(&[TestProjectItem::new(1, "1.txt", cx)]) }); - let item2 = cx.add_view(&workspace, |cx| { + let item2 = cx.add_view(window_id, |cx| { TestItem::new() .with_dirty(true) .with_conflict(true) .with_project_items(&[TestProjectItem::new(2, "2.txt", cx)]) }); - let item3 = cx.add_view(&workspace, |cx| { + let item3 = cx.add_view(window_id, |cx| { TestItem::new() .with_dirty(true) .with_conflict(true) .with_project_items(&[TestProjectItem::new(3, "3.txt", cx)]) }); - let item4 = cx.add_view(&workspace, |cx| { + let item4 = cx.add_view(window_id, |cx| { TestItem::new() .with_dirty(true) .with_project_items(&[TestProjectItem::new_untitled(cx)]) @@ -3420,7 +3420,7 @@ mod tests { // workspace items with multiple project entries. let single_entry_items = (0..=4) .map(|project_entry_id| { - cx.add_view(&workspace, |cx| { + cx.add_view(window_id, |cx| { TestItem::new() .with_dirty(true) .with_project_items(&[TestProjectItem::new( @@ -3431,7 +3431,7 @@ mod tests { }) }) .collect::>(); - let item_2_3 = cx.add_view(&workspace, |cx| { + let item_2_3 = cx.add_view(window_id, |cx| { TestItem::new() .with_dirty(true) .with_singleton(false) @@ -3440,7 +3440,7 @@ mod tests { single_entry_items[3].read(cx).project_items[0].clone(), ]) }); - let item_3_4 = cx.add_view(&workspace, |cx| { + let item_3_4 = cx.add_view(window_id, |cx| { TestItem::new() .with_dirty(true) .with_singleton(false) @@ -3523,7 +3523,7 @@ mod tests { let project = Project::test(fs, [], cx).await; let (window_id, workspace) = cx.add_window(|cx| Workspace::test_new(project, cx)); - let item = cx.add_view(&workspace, |cx| { + let item = cx.add_view(window_id, |cx| { TestItem::new().with_project_items(&[TestProjectItem::new(1, "1.txt", cx)]) }); let item_id = item.id(); @@ -3638,9 +3638,9 @@ mod tests { let fs = FakeFs::new(cx.background()); let project = Project::test(fs, [], cx).await; - let (_, workspace) = cx.add_window(|cx| Workspace::test_new(project, cx)); + let (window_id, workspace) = cx.add_window(|cx| Workspace::test_new(project, cx)); - let item = cx.add_view(&workspace, |cx| { + let item = cx.add_view(window_id, |cx| { TestItem::new().with_project_items(&[TestProjectItem::new(1, "1.txt", cx)]) }); let pane = workspace.read_with(cx, |workspace, _| workspace.active_pane().clone()); From e9ed40da376b2d8856ea1eba43625f5b29cd8183 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 3 May 2023 16:52:55 +0200 Subject: [PATCH 03/16] Remove the ability to retrieve the view's parent --- crates/collab_ui/src/collab_titlebar_item.rs | 3 ++- crates/context_menu/src/context_menu.rs | 14 ++++++-------- crates/copilot_button/src/copilot_button.rs | 3 ++- crates/editor/src/editor.rs | 4 +++- crates/gpui/src/app.rs | 4 ---- crates/gpui/src/app/window.rs | 10 ---------- crates/project_panel/src/project_panel.rs | 3 ++- crates/terminal_view/src/terminal_button.rs | 3 ++- crates/terminal_view/src/terminal_view.rs | 3 ++- crates/workspace/src/pane.rs | 5 +++-- 10 files changed, 22 insertions(+), 30 deletions(-) diff --git a/crates/collab_ui/src/collab_titlebar_item.rs b/crates/collab_ui/src/collab_titlebar_item.rs index 0942c47910856a8c09102e7bfe7f2cf49b3518c4..69ca64360cbe5363f6ea9811eb8f43f6f1c63a97 100644 --- a/crates/collab_ui/src/collab_titlebar_item.rs +++ b/crates/collab_ui/src/collab_titlebar_item.rs @@ -165,6 +165,7 @@ impl CollabTitlebarItem { }), ); + let view_id = cx.view_id(); Self { workspace: workspace.weak_handle(), project, @@ -172,7 +173,7 @@ impl CollabTitlebarItem { client, contacts_popover: None, user_menu: cx.add_view(|cx| { - let mut menu = ContextMenu::new(cx); + let mut menu = ContextMenu::new(view_id, cx); menu.set_position_mode(OverlayPositionMode::Local); menu }), diff --git a/crates/context_menu/src/context_menu.rs b/crates/context_menu/src/context_menu.rs index e0429bd01b6203ad2a75d7709b2d1f0c0c5e1e2e..1306661e7016d114d91cc8cfe3b715a6168a373e 100644 --- a/crates/context_menu/src/context_menu.rs +++ b/crates/context_menu/src/context_menu.rs @@ -127,7 +127,7 @@ pub struct ContextMenu { visible: bool, previously_focused_view_id: Option, clicked: bool, - parent_view_id: usize, + view_id: usize, _actions_observation: Subscription, } @@ -178,9 +178,7 @@ impl View for ContextMenu { } impl ContextMenu { - pub fn new(cx: &mut ViewContext) -> Self { - let parent_view_id = cx.parent().unwrap(); - + pub fn new(view_id: usize, cx: &mut ViewContext) -> Self { Self { show_count: 0, anchor_position: Default::default(), @@ -191,7 +189,7 @@ impl ContextMenu { visible: Default::default(), previously_focused_view_id: Default::default(), clicked: false, - parent_view_id, + view_id, _actions_observation: cx.observe_actions(Self::action_dispatched), } } @@ -227,7 +225,7 @@ impl ContextMenu { match action { ContextMenuItemAction::Action(action) => { let window_id = cx.window_id(); - let view_id = self.parent_view_id; + let view_id = self.view_id; let action = action.boxed_clone(); cx.app_context() .spawn(|mut cx| async move { @@ -381,7 +379,7 @@ impl ContextMenu { match action { ContextMenuItemAction::Action(action) => KeystrokeLabel::new( - self.parent_view_id, + self.view_id, action.boxed_clone(), style.keystroke.container, style.keystroke.text.clone(), @@ -421,7 +419,7 @@ impl ContextMenu { match item { ContextMenuItem::Item { label, action } => { let action = action.clone(); - let view_id = self.parent_view_id; + let view_id = self.view_id; MouseEventHandler::::new(ix, cx, |state, _| { let style = style.item.style_for(state, Some(ix) == self.selected_index); diff --git a/crates/copilot_button/src/copilot_button.rs b/crates/copilot_button/src/copilot_button.rs index 832bdaf3dae071f03f87721a8f3245982ec47230..dbdf75121cdc92199e9772fcf39191ce2bb4a877 100644 --- a/crates/copilot_button/src/copilot_button.rs +++ b/crates/copilot_button/src/copilot_button.rs @@ -142,8 +142,9 @@ impl View for CopilotButton { impl CopilotButton { pub fn new(cx: &mut ViewContext) -> Self { + let button_view_id = cx.view_id(); let menu = cx.add_view(|cx| { - let mut menu = ContextMenu::new(cx); + let mut menu = ContextMenu::new(button_view_id, cx); menu.set_position_mode(OverlayPositionMode::Local); menu }); diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index df2ca6f43d8b67d9e206cdbfaf8eb817ac89725d..8e3d9dba921acc7a2183387357aa6df94524d301 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -1227,6 +1227,7 @@ impl Editor { get_field_editor_theme: Option>, cx: &mut ViewContext, ) -> Self { + let editor_view_id = cx.view_id(); let display_map = cx.add_model(|cx| { let settings = cx.global::(); let style = build_style(&*settings, get_field_editor_theme.as_deref(), None, cx); @@ -1274,7 +1275,8 @@ impl Editor { background_highlights: Default::default(), nav_history: None, context_menu: None, - mouse_context_menu: cx.add_view(context_menu::ContextMenu::new), + mouse_context_menu: cx + .add_view(|cx| context_menu::ContextMenu::new(editor_view_id, cx)), completion_tasks: Default::default(), next_completion_id: 0, available_code_actions: Default::default(), diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 32ac815232da75181b4787e10d0d7e3155db19dd..5b8b6e5e04fe7291d8cf9a67e471e9a756110fa5 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -2767,10 +2767,6 @@ impl<'a, 'b, V: View> ViewContext<'a, 'b, V> { WeakViewHandle::new(self.window_id, self.view_id) } - pub fn parent(&self) -> Option { - self.window_context.parent(self.view_id) - } - pub fn window_id(&self) -> usize { self.window_id } diff --git a/crates/gpui/src/app/window.rs b/crates/gpui/src/app/window.rs index 3d04b46e06094a55bb66c1feaafda4fd22961736..1b9464ddeb1747d79c25c5d28207ce5a73cfe8c2 100644 --- a/crates/gpui/src/app/window.rs +++ b/crates/gpui/src/app/window.rs @@ -1073,16 +1073,6 @@ impl<'a> WindowContext<'a> { })) } - /// Returns the id of the parent of the given view, or none if the given - /// view is the root. - pub(crate) fn parent(&self, view_id: usize) -> Option { - if let Some(view_id) = self.window.parents.get(&view_id) { - Some(*view_id) - } else { - None - } - } - // Traverses the parent tree. Walks down the tree toward the passed // view calling visit with true. Then walks back up the tree calling visit with false. // If `visit` returns false this function will immediately return. diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 373417b167047ac80cd2d04aa8d7eb22068fa1df..4386626e72718e7715f61ce609d06a779893aede 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -196,6 +196,7 @@ impl ProjectPanel { }) .detach(); + let view_id = cx.view_id(); let mut this = Self { project: project.clone(), list: Default::default(), @@ -206,7 +207,7 @@ impl ProjectPanel { edit_state: None, filename_editor, clipboard_entry: None, - context_menu: cx.add_view(ContextMenu::new), + context_menu: cx.add_view(|cx| ContextMenu::new(view_id, cx)), dragged_entry_destination: None, workspace: workspace.weak_handle(), }; diff --git a/crates/terminal_view/src/terminal_button.rs b/crates/terminal_view/src/terminal_button.rs index 8edf03f527321551d8a1047e38abfe1023e6d9f8..a92f7285b51d4bad489af8ef929cb8a88ef91074 100644 --- a/crates/terminal_view/src/terminal_button.rs +++ b/crates/terminal_view/src/terminal_button.rs @@ -107,11 +107,12 @@ impl View for TerminalButton { impl TerminalButton { pub fn new(workspace: ViewHandle, cx: &mut ViewContext) -> Self { + let button_view_id = cx.view_id(); cx.observe(&workspace, |_, _, cx| cx.notify()).detach(); Self { workspace: workspace.downgrade(), popup_menu: cx.add_view(|cx| { - let mut menu = ContextMenu::new(cx); + let mut menu = ContextMenu::new(button_view_id, cx); menu.set_position_mode(OverlayPositionMode::Local); menu }), diff --git a/crates/terminal_view/src/terminal_view.rs b/crates/terminal_view/src/terminal_view.rs index a40cb4508d7218b87c0bbd81c963caf243d48e93..fd6c7fae1b435dc5d631524cc97832b7d1042534 100644 --- a/crates/terminal_view/src/terminal_view.rs +++ b/crates/terminal_view/src/terminal_view.rs @@ -124,6 +124,7 @@ impl TerminalView { workspace_id: WorkspaceId, cx: &mut ViewContext, ) -> Self { + let view_id = cx.view_id(); cx.observe(&terminal, |_, _, cx| cx.notify()).detach(); cx.subscribe(&terminal, |this, _, event, cx| match event { Event::Wakeup => { @@ -162,7 +163,7 @@ impl TerminalView { terminal, has_new_content: true, has_bell: false, - context_menu: cx.add_view(ContextMenu::new), + context_menu: cx.add_view(|cx| ContextMenu::new(view_id, cx)), blink_state: true, blinking_on: false, blinking_paused: false, diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 8b0107d3688a24467ddde6969e0991bf34d0cc9b..6f7238d4a1950bda715f4eeeefdcfccc23232aae 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -226,8 +226,9 @@ impl Pane { background_actions: BackgroundActions, cx: &mut ViewContext, ) -> Self { + let pane_view_id = cx.view_id(); let handle = cx.weak_handle(); - let context_menu = cx.add_view(ContextMenu::new); + let context_menu = cx.add_view(|cx| ContextMenu::new(pane_view_id, cx)); context_menu.update(cx, |menu, _| { menu.set_position_mode(OverlayPositionMode::Local) }); @@ -252,7 +253,7 @@ impl Pane { kind: TabBarContextMenuKind::New, handle: context_menu, }, - tab_context_menu: cx.add_view(ContextMenu::new), + tab_context_menu: cx.add_view(|cx| ContextMenu::new(pane_view_id, cx)), docked, _background_actions: background_actions, workspace, From c65465b0b5bb0df2695b4744daa15fda99758743 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 3 May 2023 18:31:07 +0200 Subject: [PATCH 04/16] Ensure workspace gets rendered in collab integration tests --- crates/collab/src/tests.rs | 42 +++++++++++++++++++++++++++++++++++--- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/crates/collab/src/tests.rs b/crates/collab/src/tests.rs index 81a505cc4e79b403070873bb9529db8ba42b6ce1..64768d1a4795679cfd54a8f7231d2c79488f01f7 100644 --- a/crates/collab/src/tests.rs +++ b/crates/collab/src/tests.rs @@ -12,7 +12,10 @@ use client::{ use collections::{HashMap, HashSet}; use fs::FakeFs; use futures::{channel::oneshot, StreamExt as _}; -use gpui::{executor::Deterministic, test::EmptyView, ModelHandle, TestAppContext, ViewHandle}; +use gpui::{ + elements::*, executor::Deterministic, AnyElement, Entity, ModelHandle, TestAppContext, View, + ViewContext, ViewHandle, WeakViewHandle, +}; use language::LanguageRegistry; use parking_lot::Mutex; use project::{Project, WorktreeId}; @@ -462,8 +465,41 @@ impl TestClient { project: &ModelHandle, cx: &mut TestAppContext, ) -> ViewHandle { - let (window_id, _) = cx.add_window(|_| EmptyView); - cx.add_view(window_id, |cx| Workspace::test_new(project.clone(), cx)) + struct WorkspaceContainer { + workspace: Option>, + } + + impl Entity for WorkspaceContainer { + type Event = (); + } + + impl View for WorkspaceContainer { + fn ui_name() -> &'static str { + "WorkspaceContainer" + } + + fn render(&mut self, cx: &mut ViewContext) -> AnyElement { + if let Some(workspace) = self + .workspace + .as_ref() + .and_then(|workspace| workspace.upgrade(cx)) + { + ChildView::new(&workspace, cx).into_any() + } else { + Empty::new().into_any() + } + } + } + + // We use a workspace container so that we don't need to remove the window in order to + // drop the workspace and we can use a ViewHandle instead. + let (window_id, container) = cx.add_window(|_| WorkspaceContainer { workspace: None }); + let workspace = cx.add_view(window_id, |cx| Workspace::test_new(project.clone(), cx)); + container.update(cx, |container, cx| { + container.workspace = Some(workspace.downgrade()); + cx.notify(); + }); + workspace } } From 51574427036bf619acd62174ce8c436efd779e04 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 3 May 2023 19:00:32 +0200 Subject: [PATCH 05/16] Fix integration test relying on deferred happening after focus Focus is now one of the last things that happens during `flush_effects`, and we shouldn't have relied on `defer` in the first place to verify focus changes. --- crates/collab/src/tests/integration_tests.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index c6f51a9dc4a8e37ca6ad3d5456d8071e51d00546..36dcff81bef6905aa4c50b62cca0495063dca64b 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -6804,13 +6804,10 @@ async fn test_peers_following_each_other( // Clients A and B follow each other in split panes workspace_a.update(cx_a, |workspace, cx| { workspace.split_pane(workspace.active_pane().clone(), SplitDirection::Right, cx); - let pane_a1 = pane_a1.clone(); - cx.defer(move |workspace, _| { - assert_ne!(*workspace.active_pane(), pane_a1); - }); }); workspace_a .update(cx_a, |workspace, cx| { + assert_ne!(*workspace.active_pane(), pane_a1); let leader_id = *project_a.read(cx).collaborators().keys().next().unwrap(); workspace.toggle_follow(leader_id, cx).unwrap() }) @@ -6818,13 +6815,10 @@ async fn test_peers_following_each_other( .unwrap(); workspace_b.update(cx_b, |workspace, cx| { workspace.split_pane(workspace.active_pane().clone(), SplitDirection::Right, cx); - let pane_b1 = pane_b1.clone(); - cx.defer(move |workspace, _| { - assert_ne!(*workspace.active_pane(), pane_b1); - }); }); workspace_b .update(cx_b, |workspace, cx| { + assert_ne!(*workspace.active_pane(), pane_b1); let leader_id = *project_b.read(cx).collaborators().keys().next().unwrap(); workspace.toggle_follow(leader_id, cx).unwrap() }) From 9e8f852afb481054a2286502ae8d7d30344bc412 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 3 May 2023 19:09:07 +0200 Subject: [PATCH 06/16] Remove `ViewContext::is_child` --- crates/gpui/src/app.rs | 10 ---- crates/workspace/src/dock.rs | 6 +-- crates/workspace/src/pane.rs | 2 + crates/workspace/src/workspace.rs | 89 ++++++++++++------------------- 4 files changed, 37 insertions(+), 70 deletions(-) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 5b8b6e5e04fe7291d8cf9a67e471e9a756110fa5..355e8215e74d5fe0e9dd1953c5d4e9a8aca1bbea 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -2833,16 +2833,6 @@ impl<'a, 'b, V: View> ViewContext<'a, 'b, V> { } } - pub fn is_child(&self, view: impl Into) -> bool { - let view = view.into(); - if self.window_id != view.window_id { - return false; - } - self.ancestors(view.view_id) - .skip(1) // Skip self id - .any(|parent| parent == self.view_id) - } - pub fn blur(&mut self) { let window_id = self.window_id; self.window_context.focus(window_id, None); diff --git a/crates/workspace/src/dock.rs b/crates/workspace/src/dock.rs index 8ac432dc4779a1ccb0132bcd3264fd414356edd7..2ec3dbaea4125454482ea6867e92aeee85e223b8 100644 --- a/crates/workspace/src/dock.rs +++ b/crates/workspace/src/dock.rs @@ -178,11 +178,7 @@ impl Dock { pane.update(cx, |pane, cx| { pane.set_active(false, cx); }); - let pane_id = pane.id(); - cx.subscribe(&pane, move |workspace, _, event, cx| { - workspace.handle_pane_event(pane_id, event, cx); - }) - .detach(); + cx.subscribe(&pane, Workspace::handle_pane_event).detach(); Self { pane, diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 6f7238d4a1950bda715f4eeeefdcfccc23232aae..4024d1fe51ab30d08591446c6a60552eaf9204bc 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -134,6 +134,7 @@ pub enum Event { RemoveItem { item_id: usize }, Split(SplitDirection), ChangeItemTitle, + Focus, } pub struct Pane { @@ -1797,6 +1798,7 @@ impl View for Pane { } fn focus_in(&mut self, focused: AnyViewHandle, cx: &mut ViewContext) { + cx.emit(Event::Focus); self.toolbar.update(cx, |toolbar, cx| { toolbar.pane_focus_update(true, cx); }); diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index f9a86d3dcc17b38be47022d6d0306305c54f50a4..972baae0ca953f248df86092db7af2aa9555e319 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -64,7 +64,7 @@ use crate::{ persistence::model::{SerializedPane, SerializedPaneGroup, SerializedWorkspace}, }; use lazy_static::lazy_static; -use log::{error, warn}; +use log::warn; use notifications::{NotificationHandle, NotifyResultExt}; pub use pane::*; pub use pane_group::*; @@ -524,11 +524,7 @@ impl Workspace { let center_pane = cx .add_view(|cx| Pane::new(weak_handle.clone(), None, app_state.background_actions, cx)); - let pane_id = center_pane.id(); - cx.subscribe(¢er_pane, move |this, _, event, cx| { - this.handle_pane_event(pane_id, event, cx) - }) - .detach(); + cx.subscribe(¢er_pane, Self::handle_pane_event).detach(); cx.focus(¢er_pane); cx.emit(Event::PaneAdded(center_pane.clone())); let dock = Dock::new( @@ -1420,11 +1416,7 @@ impl Workspace { cx, ) }); - let pane_id = pane.id(); - cx.subscribe(&pane, move |this, _, event, cx| { - this.handle_pane_event(pane_id, event, cx) - }) - .detach(); + cx.subscribe(&pane, Self::handle_pane_event).detach(); self.panes.push(pane.clone()); cx.focus(&pane); cx.emit(Event::PaneAdded(pane.clone())); @@ -1621,47 +1613,46 @@ impl Workspace { fn handle_pane_event( &mut self, - pane_id: usize, + pane: ViewHandle, event: &pane::Event, cx: &mut ViewContext, ) { - if let Some(pane) = self.pane(pane_id) { - let is_dock = &pane == self.dock.pane(); - match event { - pane::Event::Split(direction) if !is_dock => { - self.split_pane(pane, *direction, cx); + let is_dock = &pane == self.dock.pane(); + match event { + pane::Event::Split(direction) if !is_dock => { + self.split_pane(pane, *direction, cx); + } + pane::Event::Remove if !is_dock => self.remove_pane(pane, cx), + pane::Event::Remove if is_dock => Dock::hide(self, cx), + pane::Event::ActivateItem { local } => { + if *local { + self.unfollow(&pane, cx); } - pane::Event::Remove if !is_dock => self.remove_pane(pane, cx), - pane::Event::Remove if is_dock => Dock::hide(self, cx), - pane::Event::ActivateItem { local } => { - if *local { - self.unfollow(&pane, cx); - } - if &pane == self.active_pane() { - self.active_item_path_changed(cx); - } + if &pane == self.active_pane() { + self.active_item_path_changed(cx); } - pane::Event::ChangeItemTitle => { - if pane == self.active_pane { - self.active_item_path_changed(cx); - } - self.update_window_edited(cx); + } + pane::Event::ChangeItemTitle => { + if pane == self.active_pane { + self.active_item_path_changed(cx); } - pane::Event::RemoveItem { item_id } => { - self.update_window_edited(cx); - if let hash_map::Entry::Occupied(entry) = self.panes_by_item.entry(*item_id) { - if entry.get().id() == pane.id() { - entry.remove(); - } + self.update_window_edited(cx); + } + pane::Event::RemoveItem { item_id } => { + self.update_window_edited(cx); + if let hash_map::Entry::Occupied(entry) = self.panes_by_item.entry(*item_id) { + if entry.get().id() == pane.id() { + entry.remove(); } } - _ => {} } - - self.serialize_workspace(cx); - } else if self.dock.visible_pane().is_none() { - error!("pane {} not found", pane_id); + pane::Event::Focus => { + self.handle_pane_focused(pane.clone(), cx); + } + _ => {} } + + self.serialize_workspace(cx); } pub fn split_pane( @@ -1760,10 +1751,6 @@ impl Workspace { &self.panes } - fn pane(&self, pane_id: usize) -> Option> { - self.panes.iter().find(|pane| pane.id() == pane_id).cloned() - } - pub fn active_pane(&self) -> &ViewHandle { &self.active_pane } @@ -2783,17 +2770,9 @@ impl View for Workspace { .into_any_named("workspace") } - fn focus_in(&mut self, view: AnyViewHandle, cx: &mut ViewContext) { + fn focus_in(&mut self, _: AnyViewHandle, cx: &mut ViewContext) { if cx.is_self_focused() { cx.focus(&self.active_pane); - } else { - for pane in self.panes() { - let view = view.clone(); - if pane.update(cx, |_, cx| view.id() == cx.view_id() || cx.is_child(view)) { - self.handle_pane_focused(pane.clone(), cx); - break; - } - } } } From 7250754f8e55d012a821c615f11f833b0567cea2 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 3 May 2023 19:13:17 +0200 Subject: [PATCH 07/16] Make dispatch_keystroke public to the crate only --- crates/gpui/src/app/window.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/gpui/src/app/window.rs b/crates/gpui/src/app/window.rs index 1b9464ddeb1747d79c25c5d28207ce5a73cfe8c2..606cfc9a246e39ff20656abbf03ff6dbdbcb42fd 100644 --- a/crates/gpui/src/app/window.rs +++ b/crates/gpui/src/app/window.rs @@ -449,7 +449,7 @@ impl<'a> WindowContext<'a> { }) } - pub fn dispatch_keystroke(&mut self, keystroke: &Keystroke) -> bool { + pub(crate) fn dispatch_keystroke(&mut self, keystroke: &Keystroke) -> bool { let window_id = self.window_id; if let Some(focused_view_id) = self.focused_view_id() { let dispatch_path = self From 040cc4d4c3eb9d968e5accbe4672a1120d67a737 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 3 May 2023 19:25:00 +0200 Subject: [PATCH 08/16] Allow notifying views when the ancestry of another view is outdated --- crates/gpui/src/app.rs | 5 +++-- crates/gpui/src/app/window.rs | 21 ++++++++++++++++++++- crates/gpui/src/elements.rs | 7 ++++--- 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 355e8215e74d5fe0e9dd1953c5d4e9a8aca1bbea..0b618d8e6d3f275fedb37232ee49bf2451b3e129 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -25,6 +25,7 @@ use std::{ use anyhow::{anyhow, Context, Result}; use parking_lot::Mutex; use postage::oneshot; +use smallvec::SmallVec; use smol::prelude::*; use util::ResultExt; use uuid::Uuid; @@ -3198,7 +3199,7 @@ impl BorrowWindowContext for ViewContext<'_, '_, V> { pub struct LayoutContext<'a, 'b, 'c, V: View> { view_context: &'c mut ViewContext<'a, 'b, V>, new_parents: &'c mut HashMap, - views_to_notify_if_ancestors_change: &'c mut HashSet, + views_to_notify_if_ancestors_change: &'c mut HashMap>, pub refreshing: bool, } @@ -3206,7 +3207,7 @@ impl<'a, 'b, 'c, V: View> LayoutContext<'a, 'b, 'c, V> { pub fn new( view_context: &'c mut ViewContext<'a, 'b, V>, new_parents: &'c mut HashMap, - views_to_notify_if_ancestors_change: &'c mut HashSet, + views_to_notify_if_ancestors_change: &'c mut HashMap>, refreshing: bool, ) -> Self { Self { diff --git a/crates/gpui/src/app/window.rs b/crates/gpui/src/app/window.rs index 606cfc9a246e39ff20656abbf03ff6dbdbcb42fd..a96d937b1be07d5224735f4e4ba5bd99dd069ea3 100644 --- a/crates/gpui/src/app/window.rs +++ b/crates/gpui/src/app/window.rs @@ -933,7 +933,7 @@ impl<'a> WindowContext<'a> { let root_view_id = self.window.root_view().id(); let mut rendered_root = self.window.rendered_views.remove(&root_view_id).unwrap(); let mut new_parents = HashMap::default(); - let mut views_to_notify_if_ancestors_change = HashSet::default(); + let mut views_to_notify_if_ancestors_change = HashMap::default(); rendered_root.layout( SizeConstraint::strict(window_size), &mut new_parents, @@ -942,6 +942,25 @@ impl<'a> WindowContext<'a> { self, )?; + for (view_id, view_ids_to_notify) in views_to_notify_if_ancestors_change { + let mut current_view_id = view_id; + loop { + let old_parent_id = self.window.parents.get(¤t_view_id); + let new_parent_id = new_parents.get(¤t_view_id); + if old_parent_id.is_none() && new_parent_id.is_none() { + break; + } else if old_parent_id == new_parent_id { + current_view_id = *old_parent_id.unwrap(); + } else { + let window_id = self.window_id; + for view_id_to_notify in view_ids_to_notify { + self.notify_view(window_id, view_id_to_notify); + } + break; + } + } + } + self.window.parents = new_parents; self.window .rendered_views diff --git a/crates/gpui/src/elements.rs b/crates/gpui/src/elements.rs index 073d12c329255edba4bfc90921359094365f915a..e2c4af143c5fba1a0404389231af5f0c3ec1e171 100644 --- a/crates/gpui/src/elements.rs +++ b/crates/gpui/src/elements.rs @@ -37,9 +37,10 @@ use crate::{ WindowContext, }; use anyhow::{anyhow, Result}; -use collections::{HashMap, HashSet}; +use collections::HashMap; use core::panic; use json::ToJson; +use smallvec::SmallVec; use std::{ any::Any, borrow::Cow, @@ -648,7 +649,7 @@ pub trait AnyRootElement { &mut self, constraint: SizeConstraint, new_parents: &mut HashMap, - views_to_notify_if_ancestors_change: &mut HashSet, + views_to_notify_if_ancestors_change: &mut HashMap>, refreshing: bool, cx: &mut WindowContext, ) -> Result; @@ -673,7 +674,7 @@ impl AnyRootElement for RootElement { &mut self, constraint: SizeConstraint, new_parents: &mut HashMap, - views_to_notify_if_ancestors_change: &mut HashSet, + views_to_notify_if_ancestors_change: &mut HashMap>, refreshing: bool, cx: &mut WindowContext, ) -> Result { From 92183e0d729519431151c04e0fa5deadcfb5a6f1 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 4 May 2023 09:34:42 +0200 Subject: [PATCH 09/16] Ensure querying keystrokes or actions is safe This is achieved by moving `available_actions` into `AsyncAppContext` (where we know no view/window is on the stack) and `keystrokes_for_action` into `LayoutContext` where we'll fetch the previous frame's ancestors and notify the current view if those change after we perform a layout. --- crates/command_palette/src/command_palette.rs | 94 +++++----- crates/gpui/src/app.rs | 160 ++++++++++++++---- crates/gpui/src/app/test_app_context.rs | 28 +-- crates/gpui/src/app/window.rs | 45 +---- crates/gpui/src/elements/keystroke_label.rs | 2 +- crates/gpui/src/keymap_matcher/binding.rs | 10 ++ .../gpui/src/keymap_matcher/keymap_context.rs | 2 +- crates/settings/src/settings_file.rs | 54 +++--- crates/workspace/src/dock.rs | 2 +- 9 files changed, 233 insertions(+), 164 deletions(-) diff --git a/crates/command_palette/src/command_palette.rs b/crates/command_palette/src/command_palette.rs index 123c33cba1e8b81d1485b931c79c8786e454028e..4e0e776000967fd11185e70d3922e49e5c26c6d7 100644 --- a/crates/command_palette/src/command_palette.rs +++ b/crates/command_palette/src/command_palette.rs @@ -2,7 +2,7 @@ use collections::CommandPaletteFilter; use fuzzy::{StringMatch, StringMatchCandidate}; use gpui::{ actions, elements::*, keymap_matcher::Keystroke, Action, AppContext, Element, MouseState, - ViewContext, WindowContext, + ViewContext, }; use picker::{Picker, PickerDelegate, PickerEvent}; use settings::Settings; @@ -41,47 +41,17 @@ struct Command { keystrokes: Vec, } -fn toggle_command_palette(_: &mut Workspace, _: &Toggle, cx: &mut ViewContext) { - let workspace = cx.handle(); - let focused_view_id = cx.focused_view_id().unwrap_or_else(|| workspace.id()); - - cx.window_context().defer(move |cx| { - // Build the delegate before the workspace is put on the stack so we can find it when - // computing the actions. We should really not allow available_actions to be called - // if it's not reliable however. - let delegate = CommandPaletteDelegate::new(focused_view_id, cx); - workspace.update(cx, |workspace, cx| { - workspace.toggle_modal(cx, |_, cx| cx.add_view(|cx| Picker::new(delegate, cx))); - }) +fn toggle_command_palette(workspace: &mut Workspace, _: &Toggle, cx: &mut ViewContext) { + let focused_view_id = cx.focused_view_id().unwrap_or_else(|| cx.view_id()); + workspace.toggle_modal(cx, |_, cx| { + cx.add_view(|cx| Picker::new(CommandPaletteDelegate::new(focused_view_id), cx)) }); } impl CommandPaletteDelegate { - pub fn new(focused_view_id: usize, cx: &mut WindowContext) -> Self { - let actions = cx - .available_actions(focused_view_id) - .filter_map(|(name, action, bindings)| { - if cx.has_global::() { - let filter = cx.global::(); - if filter.filtered_namespaces.contains(action.namespace()) { - return None; - } - } - - Some(Command { - name: humanize_action_name(name), - action, - keystrokes: bindings - .iter() - .map(|binding| binding.keystrokes()) - .last() - .map_or(Vec::new(), |keystrokes| keystrokes.to_vec()), - }) - }) - .collect(); - + pub fn new(focused_view_id: usize) -> Self { Self { - actions, + actions: Default::default(), matches: vec![], selected_ix: 0, focused_view_id, @@ -111,17 +81,46 @@ impl PickerDelegate for CommandPaletteDelegate { query: String, cx: &mut ViewContext>, ) -> gpui::Task<()> { - let candidates = self - .actions - .iter() - .enumerate() - .map(|(ix, command)| StringMatchCandidate { - id: ix, - string: command.name.to_string(), - char_bag: command.name.chars().collect(), - }) - .collect::>(); + let window_id = cx.window_id(); + let view_id = self.focused_view_id; cx.spawn(move |picker, mut cx| async move { + let actions = cx + .available_actions(window_id, view_id) + .into_iter() + .filter_map(|(name, action, bindings)| { + let filtered = cx.read(|cx| { + if cx.has_global::() { + let filter = cx.global::(); + filter.filtered_namespaces.contains(action.namespace()) + } else { + false + } + }); + + if filtered { + None + } else { + Some(Command { + name: humanize_action_name(name), + action, + keystrokes: bindings + .iter() + .map(|binding| binding.keystrokes()) + .last() + .map_or(Vec::new(), |keystrokes| keystrokes.to_vec()), + }) + } + }) + .collect::>(); + let candidates = actions + .iter() + .enumerate() + .map(|(ix, command)| StringMatchCandidate { + id: ix, + string: command.name.to_string(), + char_bag: command.name.chars().collect(), + }) + .collect::>(); let matches = if query.is_empty() { candidates .into_iter() @@ -147,6 +146,7 @@ impl PickerDelegate for CommandPaletteDelegate { picker .update(&mut cx, |picker, _| { let delegate = picker.delegate_mut(); + delegate.actions = actions; delegate.matches = matches; if delegate.matches.is_empty() { delegate.selected_ix = 0; diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 0b618d8e6d3f275fedb37232ee49bf2451b3e129..bf3fc0e5b7aa9a2b061e5adcfb6bb3ffa15ae6b1 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -341,6 +341,15 @@ impl AsyncAppContext { .ok_or_else(|| anyhow!("window not found")) } + pub fn available_actions( + &self, + window_id: usize, + view_id: usize, + ) -> Vec<(&'static str, Box, SmallVec<[Binding; 1]>)> { + self.read_window(window_id, |cx| cx.available_actions(view_id)) + .unwrap_or_default() + } + pub fn has_window(&self, window_id: usize) -> bool { self.read(|cx| cx.windows.contains_key(&window_id)) } @@ -3221,6 +3230,64 @@ impl<'a, 'b, 'c, V: View> LayoutContext<'a, 'b, 'c, V> { pub fn view_context(&mut self) -> &mut ViewContext<'a, 'b, V> { self.view_context } + + /// Return keystrokes that would dispatch the given action on the given view. + pub(crate) fn keystrokes_for_action( + &mut self, + view: &V, + view_id: usize, + action: &dyn Action, + ) -> Option> { + self.notify_if_view_ancestors_change(view_id); + + let window_id = self.window_id; + let mut contexts = Vec::new(); + let mut handler_depth = None; + for (i, view_id) in self.ancestors(view_id).enumerate() { + let view = if view_id == self.view_id { + Some(view as _) + } else { + self.views + .get(&(window_id, view_id)) + .map(|view| view.as_ref()) + }; + + if let Some(view) = view { + if let Some(actions) = self.actions.get(&view.as_any().type_id()) { + if actions.contains_key(&action.as_any().type_id()) { + handler_depth = Some(i); + } + } + contexts.push(view.keymap_context(self)); + } + } + + if self.global_actions.contains_key(&action.as_any().type_id()) { + handler_depth = Some(contexts.len()) + } + + self.keystroke_matcher + .bindings_for_action_type(action.as_any().type_id()) + .find_map(|b| { + handler_depth + .map(|highest_handler| { + if (0..=highest_handler).any(|depth| b.match_context(&contexts[depth..])) { + Some(b.keystrokes().into()) + } else { + None + } + }) + .flatten() + }) + } + + fn notify_if_view_ancestors_change(&mut self, view_id: usize) { + let self_view_id = self.view_id; + self.views_to_notify_if_ancestors_change + .entry(view_id) + .or_default() + .push(self_view_id); + } } impl<'a, 'b, 'c, V: View> Deref for LayoutContext<'a, 'b, 'c, V> { @@ -5740,7 +5807,7 @@ mod tests { } #[crate::test(self)] - fn test_keystrokes_for_action(cx: &mut AppContext) { + fn test_keystrokes_for_action(cx: &mut TestAppContext) { actions!(test, [Action1, Action2, GlobalAction]); struct View1 { @@ -5772,35 +5839,47 @@ mod tests { } } - let (window_id, view_1) = cx.add_window(Default::default(), |cx| { + let (window_id, view_1) = cx.add_window(|cx| { let view_2 = cx.add_view(|cx| { cx.focus_self(); View2 {} }); View1 { child: view_2 } }); - let view_2 = view_1.read(cx).child.clone(); - - cx.add_action(|_: &mut View1, _: &Action1, _cx| {}); - cx.add_action(|_: &mut View2, _: &Action2, _cx| {}); - cx.add_global_action(|_: &GlobalAction, _| {}); + let view_2 = view_1.read_with(cx, |view, _| view.child.clone()); - cx.add_bindings(vec![ - Binding::new("a", Action1, Some("View1")), - Binding::new("b", Action2, Some("View1 > View2")), - Binding::new("c", GlobalAction, Some("View3")), // View 3 does not exist - ]); + cx.update(|cx| { + cx.add_action(|_: &mut View1, _: &Action1, _cx| {}); + cx.add_action(|_: &mut View2, _: &Action2, _cx| {}); + cx.add_global_action(|_: &GlobalAction, _| {}); + cx.add_bindings(vec![ + Binding::new("a", Action1, Some("View1")), + Binding::new("b", Action2, Some("View1 > View2")), + Binding::new("c", GlobalAction, Some("View3")), // View 3 does not exist + ]); + }); - cx.update_window(window_id, |cx| { + let view_1_id = view_1.id(); + view_1.update(cx, |view_1, cx| { // Sanity check + let mut new_parents = Default::default(); + let mut notify_views_if_parents_change = Default::default(); + let mut layout_cx = LayoutContext::new( + cx, + &mut new_parents, + &mut notify_views_if_parents_change, + false, + ); assert_eq!( - cx.keystrokes_for_action(view_1.id(), &Action1) + layout_cx + .keystrokes_for_action(view_1, view_1_id, &Action1) .unwrap() .as_slice(), &[Keystroke::parse("a").unwrap()] ); assert_eq!( - cx.keystrokes_for_action(view_2.id(), &Action2) + layout_cx + .keystrokes_for_action(view_1, view_2.id(), &Action2) .unwrap() .as_slice(), &[Keystroke::parse("b").unwrap()] @@ -5809,44 +5888,53 @@ mod tests { // The 'a' keystroke propagates up the view tree from view_2 // to view_1. The action, Action1, is handled by view_1. assert_eq!( - cx.keystrokes_for_action(view_2.id(), &Action1) + layout_cx + .keystrokes_for_action(view_1, view_2.id(), &Action1) .unwrap() .as_slice(), &[Keystroke::parse("a").unwrap()] ); // Actions that are handled below the current view don't have bindings - assert_eq!(cx.keystrokes_for_action(view_1.id(), &Action2), None); - - // Actions that are handled in other branches of the tree should not have a binding - assert_eq!(cx.keystrokes_for_action(view_2.id(), &GlobalAction), None); - - // Check that global actions do not have a binding, even if a binding does exist in another view assert_eq!( - &available_actions(view_1.id(), cx), - &[ - ("test::Action1", vec![Keystroke::parse("a").unwrap()]), - ("test::GlobalAction", vec![]) - ], + layout_cx.keystrokes_for_action(view_1, view_1_id, &Action2), + None ); - // Check that view 1 actions and bindings are available even when called from view 2 + // Actions that are handled in other branches of the tree should not have a binding assert_eq!( - &available_actions(view_2.id(), cx), - &[ - ("test::Action1", vec![Keystroke::parse("a").unwrap()]), - ("test::Action2", vec![Keystroke::parse("b").unwrap()]), - ("test::GlobalAction", vec![]), - ], + layout_cx.keystrokes_for_action(view_1, view_2.id(), &GlobalAction), + None ); }); + // Check that global actions do not have a binding, even if a binding does exist in another view + assert_eq!( + &available_actions(window_id, view_1.id(), cx), + &[ + ("test::Action1", vec![Keystroke::parse("a").unwrap()]), + ("test::GlobalAction", vec![]) + ], + ); + + // Check that view 1 actions and bindings are available even when called from view 2 + assert_eq!( + &available_actions(window_id, view_2.id(), cx), + &[ + ("test::Action1", vec![Keystroke::parse("a").unwrap()]), + ("test::Action2", vec![Keystroke::parse("b").unwrap()]), + ("test::GlobalAction", vec![]), + ], + ); + // Produces a list of actions and key bindings fn available_actions( + window_id: usize, view_id: usize, - cx: &WindowContext, + cx: &TestAppContext, ) -> Vec<(&'static str, Vec)> { - cx.available_actions(view_id) + cx.available_actions(window_id, view_id) + .into_iter() .map(|(action_name, _, bindings)| { ( action_name, diff --git a/crates/gpui/src/app/test_app_context.rs b/crates/gpui/src/app/test_app_context.rs index af01eec1beb81278f60e6677efdc1f4d8e01ad0d..4af436a7b8d5eaad61d9599c46246b0f27152400 100644 --- a/crates/gpui/src/app/test_app_context.rs +++ b/crates/gpui/src/app/test_app_context.rs @@ -1,7 +1,7 @@ use crate::{ executor, geometry::vector::Vector2F, - keymap_matcher::Keystroke, + keymap_matcher::{Binding, Keystroke}, platform, platform::{Event, InputHandler, KeyDownEvent, Platform}, Action, AppContext, BorrowAppContext, BorrowWindowContext, Entity, FontCache, Handle, @@ -12,6 +12,7 @@ use collections::BTreeMap; use futures::Future; use itertools::Itertools; use parking_lot::{Mutex, RwLock}; +use smallvec::SmallVec; use smol::stream::StreamExt; use std::{ any::Any, @@ -71,17 +72,24 @@ impl TestAppContext { cx } - pub fn dispatch_action(&self, window_id: usize, action: A) { - self.cx - .borrow_mut() - .update_window(window_id, |window| { - window.dispatch_action(window.focused_view_id(), &action); - }) - .expect("window not found"); + pub fn dispatch_action(&mut self, window_id: usize, action: A) { + self.update_window(window_id, |window| { + window.dispatch_action(window.focused_view_id(), &action); + }) + .expect("window not found"); + } + + pub fn available_actions( + &self, + window_id: usize, + view_id: usize, + ) -> Vec<(&'static str, Box, SmallVec<[Binding; 1]>)> { + self.read_window(window_id, |cx| cx.available_actions(view_id)) + .unwrap_or_default() } - pub fn dispatch_global_action(&self, action: A) { - self.cx.borrow_mut().dispatch_global_action_any(&action); + pub fn dispatch_global_action(&mut self, action: A) { + self.update(|cx| cx.dispatch_global_action_any(&action)); } pub fn dispatch_keystroke(&mut self, window_id: usize, keystroke: Keystroke, is_held: bool) { diff --git a/crates/gpui/src/app/window.rs b/crates/gpui/src/app/window.rs index a96d937b1be07d5224735f4e4ba5bd99dd069ea3..4ec9a87bba7010d89beea3ffc4af3859fa5485c1 100644 --- a/crates/gpui/src/app/window.rs +++ b/crates/gpui/src/app/window.rs @@ -356,49 +356,10 @@ impl<'a> WindowContext<'a> { ) } - /// Return keystrokes that would dispatch the given action on the given view. - pub(crate) fn keystrokes_for_action( - &mut self, - view_id: usize, - action: &dyn Action, - ) -> Option> { - let window_id = self.window_id; - let mut contexts = Vec::new(); - let mut handler_depth = None; - for (i, view_id) in self.ancestors(view_id).enumerate() { - if let Some(view) = self.views.get(&(window_id, view_id)) { - if let Some(actions) = self.actions.get(&view.as_any().type_id()) { - if actions.contains_key(&action.as_any().type_id()) { - handler_depth = Some(i); - } - } - contexts.push(view.keymap_context(self)); - } - } - - if self.global_actions.contains_key(&action.as_any().type_id()) { - handler_depth = Some(contexts.len()) - } - - self.keystroke_matcher - .bindings_for_action_type(action.as_any().type_id()) - .find_map(|b| { - handler_depth - .map(|highest_handler| { - if (0..=highest_handler).any(|depth| b.match_context(&contexts[depth..])) { - Some(b.keystrokes().into()) - } else { - None - } - }) - .flatten() - }) - } - - pub fn available_actions( + pub(crate) fn available_actions( &self, view_id: usize, - ) -> impl Iterator, SmallVec<[&Binding; 1]>)> { + ) -> Vec<(&'static str, Box, SmallVec<[Binding; 1]>)> { let window_id = self.window_id; let mut contexts = Vec::new(); let mut handler_depths_by_action_type = HashMap::::default(); @@ -441,12 +402,14 @@ impl<'a> WindowContext<'a> { .filter(|b| { (0..=action_depth).any(|depth| b.match_context(&contexts[depth..])) }) + .cloned() .collect(), )) } else { None } }) + .collect() } pub(crate) fn dispatch_keystroke(&mut self, keystroke: &Keystroke) -> bool { diff --git a/crates/gpui/src/elements/keystroke_label.rs b/crates/gpui/src/elements/keystroke_label.rs index c011649b2e946f980cd5b3eb2e7ab448bde8e2e1..05b6864d568b684eea2ca7d8136720f961c0f054 100644 --- a/crates/gpui/src/elements/keystroke_label.rs +++ b/crates/gpui/src/elements/keystroke_label.rs @@ -42,7 +42,7 @@ impl Element for KeystrokeLabel { cx: &mut LayoutContext, ) -> (Vector2F, AnyElement) { let mut element = if let Some(keystrokes) = - cx.keystrokes_for_action(self.view_id, self.action.as_ref()) + cx.keystrokes_for_action(view, self.view_id, self.action.as_ref()) { Flex::row() .with_children(keystrokes.iter().map(|keystroke| { diff --git a/crates/gpui/src/keymap_matcher/binding.rs b/crates/gpui/src/keymap_matcher/binding.rs index c1cfd14e82d8d3e3235a7a2c3e7d507cd78d9648..aa40e8c6af57f143e7bcaa9cc118dfd40634df19 100644 --- a/crates/gpui/src/keymap_matcher/binding.rs +++ b/crates/gpui/src/keymap_matcher/binding.rs @@ -11,6 +11,16 @@ pub struct Binding { context_predicate: Option, } +impl Clone for Binding { + fn clone(&self) -> Self { + Self { + action: self.action.boxed_clone(), + keystrokes: self.keystrokes.clone(), + context_predicate: self.context_predicate.clone(), + } + } +} + impl Binding { pub fn new(keystrokes: &str, action: A, context: Option<&str>) -> Self { Self::load(keystrokes, Box::new(action), context).unwrap() diff --git a/crates/gpui/src/keymap_matcher/keymap_context.rs b/crates/gpui/src/keymap_matcher/keymap_context.rs index bbf6bfc14bad820cd95c1525c5b673f8b8c04dcc..2eca4fbee981b3752d45ac5879e716be2314acc0 100644 --- a/crates/gpui/src/keymap_matcher/keymap_context.rs +++ b/crates/gpui/src/keymap_matcher/keymap_context.rs @@ -39,7 +39,7 @@ impl KeymapContext { } } -#[derive(Debug, Eq, PartialEq)] +#[derive(Clone, Debug, Eq, PartialEq)] pub enum KeymapContextPredicate { Identifier(String), Equal(String, String), diff --git a/crates/settings/src/settings_file.rs b/crates/settings/src/settings_file.rs index c261879a58750d1247c7c6f9801f8b0d1fcded19..ae0a34e008218d05597f4a41dcf44f946194236c 100644 --- a/crates/settings/src/settings_file.rs +++ b/crates/settings/src/settings_file.rs @@ -80,7 +80,7 @@ mod tests { watch_files, watched_json::watch_settings_file, EditorSettings, Settings, SoftWrap, }; use fs::FakeFs; - use gpui::{actions, elements::*, Action, Entity, View, ViewContext, WindowContext}; + use gpui::{actions, elements::*, Action, Entity, TestAppContext, View, ViewContext}; use theme::ThemeRegistry; struct TestView; @@ -167,13 +167,12 @@ mod tests { let (window_id, _view) = cx.add_window(|_| TestView); // Test loading the keymap base at all - cx.read_window(window_id, |cx| { - assert_key_bindings_for( - cx, - vec![("backspace", &A), ("k", &ActivatePreviousPane)], - line!(), - ); - }); + assert_key_bindings_for( + window_id, + cx, + vec![("backspace", &A), ("k", &ActivatePreviousPane)], + line!(), + ); // Test modifying the users keymap, while retaining the base keymap fs.save( @@ -195,13 +194,12 @@ mod tests { cx.foreground().run_until_parked(); - cx.read_window(window_id, |cx| { - assert_key_bindings_for( - cx, - vec![("backspace", &B), ("k", &ActivatePreviousPane)], - line!(), - ); - }); + assert_key_bindings_for( + window_id, + cx, + vec![("backspace", &B), ("k", &ActivatePreviousPane)], + line!(), + ); // Test modifying the base, while retaining the users keymap fs.save( @@ -219,31 +217,33 @@ mod tests { cx.foreground().run_until_parked(); - cx.read_window(window_id, |cx| { - assert_key_bindings_for( - cx, - vec![("backspace", &B), ("[", &ActivatePrevItem)], - line!(), - ); - }); + assert_key_bindings_for( + window_id, + cx, + vec![("backspace", &B), ("[", &ActivatePrevItem)], + line!(), + ); } fn assert_key_bindings_for<'a>( - cx: &WindowContext, + window_id: usize, + cx: &TestAppContext, actions: Vec<(&'static str, &'a dyn Action)>, line: u32, ) { for (key, action) in actions { // assert that... assert!( - cx.available_actions(0).any(|(_, bound_action, b)| { - // action names match... - bound_action.name() == action.name() + cx.available_actions(window_id, 0) + .into_iter() + .any(|(_, bound_action, b)| { + // action names match... + bound_action.name() == action.name() && bound_action.namespace() == action.namespace() // and key strokes contain the given key && b.iter() .any(|binding| binding.keystrokes().iter().any(|k| k.key == key)) - }), + }), "On {} Failed to find {} with key binding {}", line, action.name(), diff --git a/crates/workspace/src/dock.rs b/crates/workspace/src/dock.rs index 2ec3dbaea4125454482ea6867e92aeee85e223b8..7efcb7f9d31dae6c2dc665bfcd6243843e3f443e 100644 --- a/crates/workspace/src/dock.rs +++ b/crates/workspace/src/dock.rs @@ -726,7 +726,7 @@ mod tests { self.update_workspace(|workspace, cx| Dock::move_dock(workspace, anchor, true, cx)); } - pub fn hide_dock(&self) { + pub fn hide_dock(&mut self) { self.cx.dispatch_action(self.window_id, HideDock); } From 67a3891f15315d3b930b152ca6459ca48084c9df Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 4 May 2023 09:39:44 +0200 Subject: [PATCH 10/16] Make dispatch_event related methods public to the crate only --- crates/gpui/src/app/window.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/gpui/src/app/window.rs b/crates/gpui/src/app/window.rs index 4ec9a87bba7010d89beea3ffc4af3859fa5485c1..1eaa03756afb7b70b080ac6f3e6c6da24edee54b 100644 --- a/crates/gpui/src/app/window.rs +++ b/crates/gpui/src/app/window.rs @@ -457,7 +457,7 @@ impl<'a> WindowContext<'a> { } } - pub fn dispatch_event(&mut self, event: Event, event_reused: bool) -> bool { + pub(crate) fn dispatch_event(&mut self, event: Event, event_reused: bool) -> bool { let mut mouse_events = SmallVec::<[_; 2]>::new(); let mut notified_views: HashSet = Default::default(); let window_id = self.window_id; @@ -793,7 +793,7 @@ impl<'a> WindowContext<'a> { any_event_handled } - pub fn dispatch_key_down(&mut self, event: &KeyDownEvent) -> bool { + pub(crate) fn dispatch_key_down(&mut self, event: &KeyDownEvent) -> bool { let window_id = self.window_id; if let Some(focused_view_id) = self.window.focused_view_id { for view_id in self.ancestors(focused_view_id).collect::>() { @@ -812,7 +812,7 @@ impl<'a> WindowContext<'a> { false } - pub fn dispatch_key_up(&mut self, event: &KeyUpEvent) -> bool { + pub(crate) fn dispatch_key_up(&mut self, event: &KeyUpEvent) -> bool { let window_id = self.window_id; if let Some(focused_view_id) = self.window.focused_view_id { for view_id in self.ancestors(focused_view_id).collect::>() { @@ -831,7 +831,7 @@ impl<'a> WindowContext<'a> { false } - pub fn dispatch_modifiers_changed(&mut self, event: &ModifiersChangedEvent) -> bool { + pub(crate) fn dispatch_modifiers_changed(&mut self, event: &ModifiersChangedEvent) -> bool { let window_id = self.window_id; if let Some(focused_view_id) = self.window.focused_view_id { for view_id in self.ancestors(focused_view_id).collect::>() { From f6f18be9c390e3bc366f993befd3c0cb25f36ac6 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 4 May 2023 09:49:38 +0200 Subject: [PATCH 11/16] Remove `WindowContext::is_child_focused` --- crates/gpui/src/app.rs | 8 -------- crates/gpui/src/app/window.rs | 10 ---------- crates/workspace/src/pane.rs | 11 ++++++++++- crates/workspace/src/workspace.rs | 9 ++------- 4 files changed, 12 insertions(+), 26 deletions(-) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index bf3fc0e5b7aa9a2b061e5adcfb6bb3ffa15ae6b1..862374f3127b89a0ac0a564105fa0ac8f00f2422 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -5360,10 +5360,6 @@ mod tests { cx.focus(&view_2); }); - cx.read_window(window_id, |cx| { - assert!(cx.is_child_focused(&view_1)); - assert!(!cx.is_child_focused(&view_2)); - }); assert_eq!( mem::take(&mut *view_events.lock()), ["view 1 blurred", "view 2 focused"], @@ -5377,10 +5373,6 @@ mod tests { ); view_1.update(cx, |_, cx| cx.focus(&view_1)); - cx.read_window(window_id, |cx| { - assert!(!cx.is_child_focused(&view_1)); - assert!(!cx.is_child_focused(&view_2)); - }); assert_eq!( mem::take(&mut *view_events.lock()), ["view 2 blurred", "view 1 focused"], diff --git a/crates/gpui/src/app/window.rs b/crates/gpui/src/app/window.rs index 1eaa03756afb7b70b080ac6f3e6c6da24edee54b..04c6b423bc30621a7d2340c2fd53ded1c041dc1c 100644 --- a/crates/gpui/src/app/window.rs +++ b/crates/gpui/src/app/window.rs @@ -1085,16 +1085,6 @@ impl<'a> WindowContext<'a> { self.window.focused_view_id } - pub fn is_child_focused(&self, view: &AnyViewHandle) -> bool { - if let Some(focused_view_id) = self.focused_view_id() { - self.ancestors(focused_view_id) - .skip(1) // Skip self id - .any(|parent| parent == view.view_id) - } else { - false - } - } - pub fn window_bounds(&self) -> WindowBounds { self.window.platform_window.bounds() } diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 4024d1fe51ab30d08591446c6a60552eaf9204bc..3215b948aa1eb2cdd5e809d840909fbe697e216f 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -151,6 +151,7 @@ pub struct Pane { docked: Option, _background_actions: BackgroundActions, workspace: WeakViewHandle, + has_focus: bool, } pub struct ItemNavHistory { @@ -258,6 +259,7 @@ impl Pane { docked, _background_actions: background_actions, workspace, + has_focus: false, } } @@ -274,6 +276,10 @@ impl Pane { cx.notify(); } + pub fn has_focus(&self) -> bool { + self.has_focus + } + pub fn set_docked(&mut self, docked: Option, cx: &mut ViewContext) { self.docked = docked; cx.notify(); @@ -1798,7 +1804,7 @@ impl View for Pane { } fn focus_in(&mut self, focused: AnyViewHandle, cx: &mut ViewContext) { - cx.emit(Event::Focus); + self.has_focus = true; self.toolbar.update(cx, |toolbar, cx| { toolbar.pane_focus_update(true, cx); }); @@ -1824,9 +1830,12 @@ impl View for Pane { .insert(active_item.id(), focused.downgrade()); } } + + cx.emit(Event::Focus); } fn focus_out(&mut self, _: AnyViewHandle, cx: &mut ViewContext) { + self.has_focus = false; self.toolbar.update(cx, |toolbar, cx| { toolbar.pane_focus_update(false, cx); }); diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 972baae0ca953f248df86092db7af2aa9555e319..f86d054a5ca13ea2522386adb92da4f544aa2eea 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -2339,19 +2339,14 @@ impl Workspace { } for (pane, item) in items_to_activate { - let active_item_was_focused = pane - .read(cx) - .active_item() - .map(|active_item| cx.is_child_focused(active_item.as_any())) - .unwrap_or_default(); - + let pane_was_focused = pane.read(cx).has_focus(); if let Some(index) = pane.update(cx, |pane, _| pane.index_for_item(item.as_ref())) { pane.update(cx, |pane, cx| pane.activate_item(index, false, false, cx)); } else { Pane::add_item(self, &pane, item.boxed_clone(), false, false, None, cx); } - if active_item_was_focused { + if pane_was_focused { pane.update(cx, |pane, cx| pane.focus_active_item(cx)); } } From 7b7a495be333d7187bb3b3217f6f24d285fa158a Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 4 May 2023 09:56:49 +0200 Subject: [PATCH 12/16] Remove stray `dbg!` statement --- crates/gpui/src/app/window.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/gpui/src/app/window.rs b/crates/gpui/src/app/window.rs index 04c6b423bc30621a7d2340c2fd53ded1c041dc1c..9801119fb46f0baae3b6a9687b60526a6c4fdef0 100644 --- a/crates/gpui/src/app/window.rs +++ b/crates/gpui/src/app/window.rs @@ -1003,7 +1003,6 @@ impl<'a> WindowContext<'a> { if let Some(view_id) = view_id { self.halt_action_dispatch = false; self.visit_dispatch_path(view_id, |view_id, capture_phase, cx| { - dbg!(view_id); cx.update_any_view(view_id, |view, cx| { let type_id = view.as_any().type_id(); if let Some((name, mut handlers)) = cx From 80ad59a620f11d07dddab362191e442988e064c6 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Fri, 5 May 2023 10:04:54 +0200 Subject: [PATCH 13/16] Make focusing the parent an effect to avoid querying ancestors --- crates/gpui/src/app.rs | 78 ++++++++++++-------- crates/search/src/project_search.rs | 2 +- crates/terminal_view/src/terminal_element.rs | 2 +- crates/workspace/src/pane.rs | 2 +- crates/workspace/src/shared_screen.rs | 2 +- 5 files changed, 53 insertions(+), 33 deletions(-) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 23d9662b570d7a6494cdf196e82dba3a190b00fd..fa09f3b859d4e7b80e6eb5bab40d1f058d539a66 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -1466,10 +1466,11 @@ impl AppContext { }); if let Some(view_id) = change_focus_to { - self.pending_effects.push_back(Effect::Focus { - window_id, - view_id: Some(view_id), - }); + self.pending_effects + .push_back(Effect::Focus(FocusEffect::View { + window_id, + view_id: Some(view_id), + })); } self.pending_effects @@ -1491,7 +1492,7 @@ impl AppContext { let mut refreshing = false; let mut updated_windows = HashSet::default(); - let mut focused_views = HashMap::default(); + let mut focus_changes = HashMap::default(); loop { self.remove_dropped_entities(); if let Some(effect) = self.pending_effects.pop_front() { @@ -1566,8 +1567,8 @@ impl AppContext { self.handle_entity_release_effect(view_id, view.as_any()) } - Effect::Focus { window_id, view_id } => { - focused_views.insert(window_id, view_id); + Effect::Focus(effect) => { + focus_changes.insert(effect.window_id(), effect); } Effect::FocusObservation { @@ -1691,8 +1692,8 @@ impl AppContext { }); } - for (window_id, view_id) in focused_views.drain() { - self.handle_focus_effect(window_id, view_id); + for (_, effect) in focus_changes.drain() { + self.handle_focus_effect(effect); } if self.pending_effects.is_empty() { @@ -1869,14 +1870,17 @@ impl AppContext { }); } - fn handle_focus_effect(&mut self, window_id: usize, mut focused_id: Option) { - let mut blurred_id = None; + fn handle_focus_effect(&mut self, effect: FocusEffect) { + let window_id = effect.window_id(); self.update_window(window_id, |cx| { + let focused_id = match effect { + FocusEffect::View { view_id, .. } => view_id, + FocusEffect::ViewParent { view_id, .. } => cx.ancestors(view_id).skip(1).next(), + }; if cx.window.focused_view_id == focused_id { - focused_id = None; return; } - blurred_id = cx.window.focused_view_id; + let blurred_id = cx.window.focused_view_id; cx.window.focused_view_id = focused_id; let blurred_parents = blurred_id @@ -1959,7 +1963,7 @@ impl AppContext { pub fn focus(&mut self, window_id: usize, view_id: Option) { self.pending_effects - .push_back(Effect::Focus { window_id, view_id }); + .push_back(Effect::Focus(FocusEffect::View { window_id, view_id })); } fn spawn_internal(&mut self, task_name: Option<&'static str>, f: F) -> Task @@ -2055,6 +2059,27 @@ pub struct WindowInvalidation { pub removed: Vec, } +#[derive(Debug)] +pub enum FocusEffect { + View { + window_id: usize, + view_id: Option, + }, + ViewParent { + window_id: usize, + view_id: usize, + }, +} + +impl FocusEffect { + fn window_id(&self) -> usize { + match self { + FocusEffect::View { window_id, .. } => *window_id, + FocusEffect::ViewParent { window_id, .. } => *window_id, + } + } +} + pub enum Effect { Subscription { entity_id: usize, @@ -2100,10 +2125,7 @@ pub enum Effect { view_id: usize, view: Box, }, - Focus { - window_id: usize, - view_id: Option, - }, + Focus(FocusEffect), FocusObservation { view_id: usize, subscription_id: usize, @@ -2219,11 +2241,7 @@ impl Debug for Effect { .debug_struct("Effect::ViewRelease") .field("view_id", view_id) .finish(), - Effect::Focus { window_id, view_id } => f - .debug_struct("Effect::Focus") - .field("window_id", window_id) - .field("view_id", view_id) - .finish(), + Effect::Focus(focus) => f.debug_tuple("Effect::Focus").field(focus).finish(), Effect::FocusObservation { view_id, subscription_id, @@ -2849,12 +2867,14 @@ impl<'a, 'b, V: View> ViewContext<'a, 'b, V> { } } - pub fn focus_parent_view(&mut self) { - let next = self.ancestors(self.view_id).next().clone(); - if let Some(parent_view_id) = next { - let window_id = self.window_id; - self.window_context.focus(window_id, Some(parent_view_id)); - } + pub fn focus_parent(&mut self) { + let window_id = self.window_id; + let view_id = self.view_id; + self.pending_effects + .push_back(Effect::Focus(FocusEffect::ViewParent { + window_id, + view_id, + })); } pub fn blur(&mut self) { diff --git a/crates/search/src/project_search.rs b/crates/search/src/project_search.rs index 6266c571bf29ab5363c60780d2c6971de45d2bc5..d68cad71d836a689b42b8a3e4f801107cd20ec29 100644 --- a/crates/search/src/project_search.rs +++ b/crates/search/src/project_search.rs @@ -200,7 +200,7 @@ impl View for ProjectSearchView { .flex(1., true) }) .on_down(MouseButton::Left, |_, _, cx| { - cx.focus_parent_view(); + cx.focus_parent(); }) .into_any_named("project search view") } else { diff --git a/crates/terminal_view/src/terminal_element.rs b/crates/terminal_view/src/terminal_element.rs index 366db99622c7f0efe76169ce0d67d6ff6821ddc8..eb2d68910f2f6e1351b73a51b1173fbe8782d082 100644 --- a/crates/terminal_view/src/terminal_element.rs +++ b/crates/terminal_view/src/terminal_element.rs @@ -370,7 +370,7 @@ impl TerminalElement { f: impl Fn(&mut Terminal, Vector2F, E, &mut ModelContext), ) -> impl Fn(E, &mut TerminalView, &mut EventContext) { move |event, _: &mut TerminalView, cx| { - cx.focus_parent_view(); + cx.focus_parent(); if let Some(conn_handle) = connection.upgrade(cx) { conn_handle.update(cx, |terminal, cx| { f(terminal, origin, event, cx); diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 5bdfe6017674e7df1c5f2055c95acf32208a241e..96320a4baf5c5460030e73efd8276f26c7c6f804 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -1770,7 +1770,7 @@ impl View for Pane { self.render_blank_pane(&theme, cx) }) .on_down(MouseButton::Left, |_, _, cx| { - cx.focus_parent_view(); + cx.focus_parent(); }) .into_any() } diff --git a/crates/workspace/src/shared_screen.rs b/crates/workspace/src/shared_screen.rs index 47c802d938e4a86f562f54bca782309334d7be50..5cc54a6a7f4244f09a1a40fa0cea7c592703331d 100644 --- a/crates/workspace/src/shared_screen.rs +++ b/crates/workspace/src/shared_screen.rs @@ -90,7 +90,7 @@ impl View for SharedScreen { .contained() .with_style(cx.global::().theme.shared_screen) }) - .on_down(MouseButton::Left, |_, _, cx| cx.focus_parent_view()) + .on_down(MouseButton::Left, |_, _, cx| cx.focus_parent()) .into_any() } } From b9ed327b944556cbe33682685ca7b1aa9f66413c Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Fri, 5 May 2023 10:08:22 +0200 Subject: [PATCH 14/16] Replace usages of `is_parent_view_focused` with `is_self_focused` Previously, this was used because we didn't have access to the current view and `EventContext` was an element-only abstraction. Now that the `EventContext` wraps the current view's `ViewContext` we can simply check for the view's focus and avoid querying ancestors. --- crates/gpui/src/app.rs | 8 -------- crates/terminal_view/src/terminal_element.rs | 4 ++-- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index fa09f3b859d4e7b80e6eb5bab40d1f058d539a66..450be671da757f8410d2565906dc4e4bb44928a4 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -2859,14 +2859,6 @@ impl<'a, 'b, V: View> ViewContext<'a, 'b, V> { self.window.focused_view_id == Some(self.view_id) } - pub fn is_parent_view_focused(&self) -> bool { - if let Some(parent_view_id) = self.ancestors(self.view_id).next().clone() { - self.focused_view_id() == Some(parent_view_id) - } else { - false - } - } - pub fn focus_parent(&mut self) { let window_id = self.window_id; let view_id = self.view_id; diff --git a/crates/terminal_view/src/terminal_element.rs b/crates/terminal_view/src/terminal_element.rs index eb2d68910f2f6e1351b73a51b1173fbe8782d082..ae2342cd97359aa267adf0154fa0e0c800858992 100644 --- a/crates/terminal_view/src/terminal_element.rs +++ b/crates/terminal_view/src/terminal_element.rs @@ -408,7 +408,7 @@ impl TerminalElement { ) // Update drag selections .on_drag(MouseButton::Left, move |event, _: &mut TerminalView, cx| { - if cx.is_parent_view_focused() { + if cx.is_self_focused() { if let Some(conn_handle) = connection.upgrade(cx) { conn_handle.update(cx, |terminal, cx| { terminal.mouse_drag(event, origin); @@ -444,7 +444,7 @@ impl TerminalElement { }, ) .on_move(move |event, _: &mut TerminalView, cx| { - if cx.is_parent_view_focused() { + if cx.is_self_focused() { if let Some(conn_handle) = connection.upgrade(cx) { conn_handle.update(cx, |terminal, cx| { terminal.mouse_move(&event, origin); From 080a1f00a3099014c9c124f8a7e4a07b3489313d Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Fri, 5 May 2023 10:47:42 +0200 Subject: [PATCH 15/16] Delay `focus_in` event for window activation till after layout --- crates/gpui/src/app.rs | 124 +++++++++++++++++++++++++---------------- 1 file changed, 77 insertions(+), 47 deletions(-) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 450be671da757f8410d2565906dc4e4bb44928a4..ee12b8bc6ae3b4ad5fdac6e97f636e2a00f6b007 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -1470,6 +1470,7 @@ impl AppContext { .push_back(Effect::Focus(FocusEffect::View { window_id, view_id: Some(view_id), + is_forced: false, })); } @@ -1492,7 +1493,7 @@ impl AppContext { let mut refreshing = false; let mut updated_windows = HashSet::default(); - let mut focus_changes = HashMap::default(); + let mut focus_effects = HashMap::::default(); loop { self.remove_dropped_entities(); if let Some(effect) = self.pending_effects.pop_front() { @@ -1567,8 +1568,15 @@ impl AppContext { self.handle_entity_release_effect(view_id, view.as_any()) } - Effect::Focus(effect) => { - focus_changes.insert(effect.window_id(), effect); + Effect::Focus(mut effect) => { + if focus_effects + .get(&effect.window_id()) + .map_or(false, |prev_effect| prev_effect.is_forced()) + { + effect.force(); + } + + focus_effects.insert(effect.window_id(), effect); } Effect::FocusObservation { @@ -1609,7 +1617,22 @@ impl AppContext { Effect::ActivateWindow { window_id, is_active, - } => self.handle_window_activation_effect(window_id, is_active), + } => { + if self.handle_window_activation_effect(window_id, is_active) + && is_active + { + focus_effects + .entry(window_id) + .or_insert_with(|| FocusEffect::View { + window_id, + view_id: self + .read_window(window_id, |cx| cx.focused_view_id()) + .flatten(), + is_forced: true, + }) + .force(); + } + } Effect::WindowFullscreenObservation { window_id, @@ -1692,7 +1715,7 @@ impl AppContext { }); } - for (_, effect) in focus_changes.drain() { + for (_, effect) in focus_effects.drain() { self.handle_focus_effect(effect); } @@ -1846,28 +1869,18 @@ impl AppContext { }); } - fn handle_window_activation_effect(&mut self, window_id: usize, active: bool) { + fn handle_window_activation_effect(&mut self, window_id: usize, active: bool) -> bool { self.update_window(window_id, |cx| { if cx.window.is_active == active { - return; + return false; } cx.window.is_active = active; - if let Some(focused_id) = cx.window.focused_view_id { - for view_id in cx.ancestors(focused_id).collect::>() { - cx.update_any_view(view_id, |view, cx| { - if active { - view.focus_in(focused_id, cx, view_id); - } else { - view.focus_out(focused_id, cx, view_id); - } - }); - } - } - let mut observations = cx.window_activation_observations.clone(); observations.emit(window_id, |callback| callback(active, cx)); - }); + true + }) + .unwrap_or(false) } fn handle_focus_effect(&mut self, effect: FocusEffect) { @@ -1877,41 +1890,37 @@ impl AppContext { FocusEffect::View { view_id, .. } => view_id, FocusEffect::ViewParent { view_id, .. } => cx.ancestors(view_id).skip(1).next(), }; - if cx.window.focused_view_id == focused_id { - return; - } + + let focus_changed = cx.window.focused_view_id != focused_id; let blurred_id = cx.window.focused_view_id; cx.window.focused_view_id = focused_id; - let blurred_parents = blurred_id - .map(|blurred_id| cx.ancestors(blurred_id).collect::>()) - .unwrap_or_default(); - let focused_parents = focused_id - .map(|focused_id| cx.ancestors(focused_id).collect::>()) - .unwrap_or_default(); - - if let Some(blurred_id) = blurred_id { - for view_id in blurred_parents.iter().copied() { - if let Some(mut view) = cx.views.remove(&(window_id, view_id)) { - view.focus_out(blurred_id, cx, view_id); - cx.views.insert((window_id, view_id), view); + if focus_changed { + if let Some(blurred_id) = blurred_id { + for view_id in cx.ancestors(blurred_id).collect::>() { + if let Some(mut view) = cx.views.remove(&(window_id, view_id)) { + view.focus_out(blurred_id, cx, view_id); + cx.views.insert((window_id, view_id), view); + } } - } - let mut subscriptions = cx.focus_observations.clone(); - subscriptions.emit(blurred_id, |callback| callback(false, cx)); + let mut subscriptions = cx.focus_observations.clone(); + subscriptions.emit(blurred_id, |callback| callback(false, cx)); + } } - if let Some(focused_id) = focused_id { - for view_id in focused_parents { - if let Some(mut view) = cx.views.remove(&(window_id, view_id)) { - view.focus_in(focused_id, cx, view_id); - cx.views.insert((window_id, view_id), view); + if focus_changed || effect.is_forced() { + if let Some(focused_id) = focused_id { + for view_id in cx.ancestors(focused_id).collect::>() { + if let Some(mut view) = cx.views.remove(&(window_id, view_id)) { + view.focus_in(focused_id, cx, view_id); + cx.views.insert((window_id, view_id), view); + } } - } - let mut subscriptions = cx.focus_observations.clone(); - subscriptions.emit(focused_id, |callback| callback(true, cx)); + let mut subscriptions = cx.focus_observations.clone(); + subscriptions.emit(focused_id, |callback| callback(true, cx)); + } } }); } @@ -1963,7 +1972,11 @@ impl AppContext { pub fn focus(&mut self, window_id: usize, view_id: Option) { self.pending_effects - .push_back(Effect::Focus(FocusEffect::View { window_id, view_id })); + .push_back(Effect::Focus(FocusEffect::View { + window_id, + view_id, + is_forced: false, + })); } fn spawn_internal(&mut self, task_name: Option<&'static str>, f: F) -> Task @@ -2064,10 +2077,12 @@ pub enum FocusEffect { View { window_id: usize, view_id: Option, + is_forced: bool, }, ViewParent { window_id: usize, view_id: usize, + is_forced: bool, }, } @@ -2078,6 +2093,20 @@ impl FocusEffect { FocusEffect::ViewParent { window_id, .. } => *window_id, } } + + fn is_forced(&self) -> bool { + match self { + FocusEffect::View { is_forced, .. } => *is_forced, + FocusEffect::ViewParent { is_forced, .. } => *is_forced, + } + } + + fn force(&mut self) { + match self { + FocusEffect::View { is_forced, .. } => *is_forced = true, + FocusEffect::ViewParent { is_forced, .. } => *is_forced = true, + } + } } pub enum Effect { @@ -2866,6 +2895,7 @@ impl<'a, 'b, V: View> ViewContext<'a, 'b, V> { .push_back(Effect::Focus(FocusEffect::ViewParent { window_id, view_id, + is_forced: false, })); } From 5e16f70067ea87390b6e98e6fd2440e28bc7950f Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Fri, 5 May 2023 10:53:15 +0200 Subject: [PATCH 16/16] :lipstick: --- crates/context_menu/src/context_menu.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/context_menu/src/context_menu.rs b/crates/context_menu/src/context_menu.rs index b5c670ccf5d4d20ceea4f047e55f078e5c2f30f6..7f821c06e272a9afd50a055b1552cbbe6f8ca540 100644 --- a/crates/context_menu/src/context_menu.rs +++ b/crates/context_menu/src/context_menu.rs @@ -127,7 +127,7 @@ pub struct ContextMenu { visible: bool, previously_focused_view_id: Option, clicked: bool, - view_id: usize, + parent_view_id: usize, _actions_observation: Subscription, } @@ -177,7 +177,7 @@ impl View for ContextMenu { } impl ContextMenu { - pub fn new(view_id: usize, cx: &mut ViewContext) -> Self { + pub fn new(parent_view_id: usize, cx: &mut ViewContext) -> Self { Self { show_count: 0, anchor_position: Default::default(), @@ -188,7 +188,7 @@ impl ContextMenu { visible: Default::default(), previously_focused_view_id: Default::default(), clicked: false, - view_id, + parent_view_id, _actions_observation: cx.observe_actions(Self::action_dispatched), } } @@ -224,7 +224,7 @@ impl ContextMenu { match action { ContextMenuItemAction::Action(action) => { let window_id = cx.window_id(); - let view_id = self.view_id; + let view_id = self.parent_view_id; let action = action.boxed_clone(); cx.app_context() .spawn(|mut cx| async move { @@ -378,7 +378,7 @@ impl ContextMenu { match action { ContextMenuItemAction::Action(action) => KeystrokeLabel::new( - self.view_id, + self.parent_view_id, action.boxed_clone(), style.keystroke.container, style.keystroke.text.clone(), @@ -418,7 +418,7 @@ impl ContextMenu { match item { ContextMenuItem::Item { label, action } => { let action = action.clone(); - let view_id = self.view_id; + let view_id = self.parent_view_id; MouseEventHandler::::new(ix, cx, |state, _| { let style = style.item.style_for(state, Some(ix) == self.selected_index);