Replace `Window::parent_view_id()` with `Window::current_view()` (#24212)

Mikayla Maki created

Chatted with @as-cii about limitations in the `Window::parent_view_id()`
API (see:
https://github.com/zed-industries/zed/pull/24182/commits/662153dcfdd80804f4041761c09c4a309d79f6d4)
and realized that I shouldn't be using the dispatch tree's data
structures as they are layout dependent. I've introduced a new stack to
`Window`, `rendered_entity_stack`, that tracks exactly which view's
elements are being drawn. As such, I've also been able to remove the
`Option<>` around the previous API.

Release Notes:

- N/A

Change summary

crates/gpui/src/elements/img.rs | 10 +----
crates/gpui/src/key_dispatch.rs |  4 --
crates/gpui/src/view.rs         | 33 ++++++++++++++-------
crates/gpui/src/window.rs       | 55 +++++++++++++++-------------------
4 files changed, 50 insertions(+), 52 deletions(-)

Detailed changes

crates/gpui/src/elements/img.rs 🔗

@@ -353,15 +353,11 @@ impl Element for Img {
                                         }
                                     }
                                 } else {
-                                    let parent_view_id = window.parent_view_id();
+                                    let current_view = window.current_view();
                                     let task = window.spawn(cx, |mut cx| async move {
                                         cx.background_executor().timer(LOADING_DELAY).await;
-                                        cx.update(move |window, cx| {
-                                            if let Some(parent_view_id) = parent_view_id {
-                                                cx.notify(parent_view_id);
-                                            } else {
-                                                window.refresh();
-                                            }
+                                        cx.update(move |_, cx| {
+                                            cx.notify(current_view);
                                         })
                                         .ok();
                                     });

crates/gpui/src/key_dispatch.rs 🔗

@@ -219,10 +219,6 @@ impl DispatchTree {
         self.focusable_node_ids.insert(focus_id, node_id);
     }
 
-    pub fn parent_view_id(&self) -> Option<EntityId> {
-        self.view_stack.last().copied()
-    }
-
     pub fn set_view_id(&mut self, view_id: EntityId) {
         if self.view_stack.last().copied() != Some(view_id) {
             let node_id = *self.node_stack.last().unwrap();

crates/gpui/src/view.rs 🔗

@@ -155,9 +155,11 @@ impl Element for AnyView {
             let layout_id = window.request_layout(root_style, None, cx);
             (layout_id, None)
         } else {
-            let mut element = (self.render)(self, window, cx);
-            let layout_id = element.request_layout(window, cx);
-            (layout_id, Some(element))
+            window.with_rendered_view(self.entity_id(), |window| {
+                let mut element = (self.render)(self, window, cx);
+                let layout_id = element.request_layout(window, cx);
+                (layout_id, Some(element))
+            })
         }
     }
 
@@ -197,12 +199,16 @@ impl Element for AnyView {
 
                     let refreshing = mem::replace(&mut window.refreshing, true);
                     let prepaint_start = window.prepaint_index();
-                    let (mut element, accessed_entities) = cx.detect_accessed_entities(|cx| {
-                        let mut element = (self.render)(self, window, cx);
-                        element.layout_as_root(bounds.size.into(), window, cx);
-                        element.prepaint_at(bounds.origin, window, cx);
-                        element
-                    });
+                    let (mut element, accessed_entities) =
+                        window.with_rendered_view(self.entity_id(), |window| {
+                            cx.detect_accessed_entities(|cx| {
+                                let mut element = (self.render)(self, window, cx);
+                                element.layout_as_root(bounds.size.into(), window, cx);
+                                element.prepaint_at(bounds.origin, window, cx);
+                                element
+                            })
+                        });
+
                     let prepaint_end = window.prepaint_index();
                     window.refreshing = refreshing;
 
@@ -223,7 +229,10 @@ impl Element for AnyView {
             )
         } else {
             let mut element = element.take().unwrap();
-            element.prepaint(window, cx);
+            window.with_rendered_view(self.entity_id(), |window| {
+                element.prepaint(window, cx);
+            });
+
             Some(element)
         }
     }
@@ -247,7 +256,9 @@ impl Element for AnyView {
 
                     if let Some(element) = element {
                         let refreshing = mem::replace(&mut window.refreshing, true);
-                        element.paint(window, cx);
+                        window.with_rendered_view(self.entity_id(), |window| {
+                            element.paint(window, cx);
+                        });
                         window.refreshing = refreshing;
                     } else {
                         window.reuse_paint(element_state.paint_range.clone());

crates/gpui/src/window.rs 🔗

@@ -612,6 +612,7 @@ pub struct Window {
     pub(crate) root: Option<AnyView>,
     pub(crate) element_id_stack: SmallVec<[ElementId; 32]>,
     pub(crate) text_style_stack: Vec<TextStyleRefinement>,
+    pub(crate) rendered_entity_stack: Vec<EntityId>,
     pub(crate) element_offset_stack: Vec<Point<Pixels>>,
     pub(crate) element_opacity: Option<f32>,
     pub(crate) content_mask_stack: Vec<ContentMask<Pixels>>,
@@ -895,6 +896,7 @@ impl Window {
             root: None,
             element_id_stack: SmallVec::default(),
             text_style_stack: Vec::new(),
+            rendered_entity_stack: Vec::new(),
             element_offset_stack: Vec::new(),
             content_mask_stack: Vec::new(),
             element_opacity: None,
@@ -971,27 +973,6 @@ impl ContentMask<Pixels> {
 }
 
 impl Window {
-    /// Indicate that a view has changed, which will invoke any observers and also mark the window as dirty.
-    /// If this view or any of its ancestors are *cached*, notifying it will cause it or its ancestors to be redrawn.
-    /// Note that this method will always cause a redraw, the entire window is refreshed if view_id is None.
-    pub(crate) fn notify(
-        &mut self,
-        notify_effect: bool,
-        entity_id: Option<EntityId>,
-        cx: &mut App,
-    ) {
-        let Some(view_id) = entity_id else {
-            self.refresh();
-            return;
-        };
-
-        self.mark_view_dirty(view_id);
-
-        if notify_effect {
-            self.invalidator.invalidate_view(view_id, cx);
-        }
-    }
-
     fn mark_view_dirty(&mut self, view_id: EntityId) {
         // Mark ancestor views as dirty. If already in the `dirty_views` set, then all its ancestors
         // should already be dirty.
@@ -1300,8 +1281,8 @@ impl Window {
     ///
     /// If called from within a view, it will notify that view on the next frame. Otherwise, it will refresh the entire window.
     pub fn request_animation_frame(&self) {
-        let parent_id = self.parent_view_id();
-        self.on_next_frame(move |window, cx| window.notify(true, parent_id, cx));
+        let entity = self.current_view();
+        self.on_next_frame(move |_, cx| cx.notify(entity));
     }
 
     /// Spawn the future returned by the given closure on the application thread pool.
@@ -1534,6 +1515,7 @@ impl Window {
     pub fn draw(&mut self, cx: &mut App) {
         self.invalidate_entities();
         cx.entities.clear_accessed();
+        debug_assert!(self.rendered_entity_stack.is_empty());
         self.invalidator.set_dirty(false);
         self.requested_autoscroll = None;
 
@@ -1596,6 +1578,7 @@ impl Window {
                 .retain(&(), |listener| listener(&event, self, cx));
         }
 
+        debug_assert!(self.rendered_entity_stack.is_empty());
         self.record_entities_accessed(cx);
         self.reset_cursor_style(cx);
         self.refreshing = false;
@@ -2074,14 +2057,14 @@ impl Window {
         let (task, is_first) = cx.fetch_asset::<A>(source);
         task.clone().now_or_never().or_else(|| {
             if is_first {
-                let parent_id = self.parent_view_id();
+                let entity = self.current_view();
                 self.spawn(cx, {
                     let task = task.clone();
                     |mut cx| async move {
                         task.await;
 
-                        cx.on_next_frame(move |window, cx| {
-                            window.notify(true, parent_id, cx);
+                        cx.on_next_frame(move |_, cx| {
+                            cx.notify(entity);
                         });
                     }
                 })
@@ -2690,12 +2673,12 @@ impl Window {
         Ok(())
     }
 
-    #[must_use]
     /// Add a node to the layout tree for the current frame. Takes the `Style` of the element for which
     /// layout is being requested, along with the layout ids of any children. This method is called during
     /// calls to the [`Element::request_layout`] trait method and enables any element to participate in layout.
     ///
     /// This method should only be called as part of the request_layout or prepaint phase of element drawing.
+    #[must_use]
     pub fn request_layout(
         &mut self,
         style: Style,
@@ -2826,9 +2809,21 @@ impl Window {
         self.next_frame.dispatch_tree.set_view_id(view_id);
     }
 
-    /// Get the last view id for the current element
-    pub fn parent_view_id(&self) -> Option<EntityId> {
-        self.next_frame.dispatch_tree.parent_view_id()
+    /// Get the entity ID for the currently rendering view
+    pub fn current_view(&self) -> EntityId {
+        self.invalidator.debug_assert_paint_or_prepaint();
+        self.rendered_entity_stack.last().copied().unwrap()
+    }
+
+    pub(crate) fn with_rendered_view<R>(
+        &mut self,
+        id: EntityId,
+        f: impl FnOnce(&mut Self) -> R,
+    ) -> R {
+        self.rendered_entity_stack.push(id);
+        let result = f(self);
+        self.rendered_entity_stack.pop();
+        result
     }
 
     /// Sets an input handler, such as [`ElementInputHandler`][element_input_handler], which interfaces with the