Fix a few identity mixups in GPUI

Mikayla and nathan created

co-authored-by: nathan <nathan@zed.dev>

Change summary

crates/diagnostics2/src/items.rs    |  2 
crates/gpui2/src/element.rs         |  6 --
crates/gpui2/src/view.rs            | 22 ++++-----
crates/gpui2/src/window.rs          | 68 +++++++++++++++++++++++++-----
crates/workspace2/src/dock.rs       |  2 
crates/workspace2/src/pane.rs       |  4 
crates/workspace2/src/pane_group.rs | 24 +++++-----
7 files changed, 84 insertions(+), 44 deletions(-)

Detailed changes

crates/diagnostics2/src/items.rs 🔗

@@ -44,7 +44,7 @@ impl Render for DiagnosticIndicator {
         };
 
         h_stack()
-            .id(cx.entity_id())
+            .id("diagnostic-indicator")
             .on_action(cx.listener(Self::go_to_next_diagnostic))
             .rounded_md()
             .flex_none()

crates/gpui2/src/element.rs 🔗

@@ -432,10 +432,6 @@ impl AnyElement {
         AnyElement(Box::new(Some(DrawableElement::new(element))) as Box<dyn ElementObject>)
     }
 
-    pub fn element_id(&self) -> Option<ElementId> {
-        self.0.element_id()
-    }
-
     pub fn layout(&mut self, cx: &mut WindowContext) -> LayoutId {
         self.0.layout(cx)
     }
@@ -490,7 +486,7 @@ impl RenderOnce for AnyElement {
     type Element = Self;
 
     fn element_id(&self) -> Option<ElementId> {
-        AnyElement::element_id(self)
+        None
     }
 
     fn render_once(self) -> Self::Element {

crates/gpui2/src/view.rs 🔗

@@ -248,7 +248,7 @@ impl<V: 'static + Render> RenderOnce for View<V> {
     type Element = View<V>;
 
     fn element_id(&self) -> Option<ElementId> {
-        Some(self.model.entity_id.into())
+        Some(ElementId::from_entity_id(self.model.entity_id))
     }
 
     fn render_once(self) -> Self::Element {
@@ -260,7 +260,7 @@ impl RenderOnce for AnyView {
     type Element = Self;
 
     fn element_id(&self) -> Option<ElementId> {
-        Some(self.model.entity_id.into())
+        Some(ElementId::from_entity_id(self.model.entity_id))
     }
 
     fn render_once(self) -> Self::Element {
@@ -308,27 +308,23 @@ where
 }
 
 mod any_view {
-    use crate::{AnyElement, AnyView, BorrowWindow, Element, LayoutId, Render, WindowContext};
+    use crate::{AnyElement, AnyView, Element, LayoutId, Render, WindowContext};
 
     pub(crate) fn layout<V: 'static + Render>(
         view: &AnyView,
         cx: &mut WindowContext,
     ) -> (LayoutId, AnyElement) {
-        cx.with_element_id(Some(view.model.entity_id), |cx| {
-            let view = view.clone().downcast::<V>().unwrap();
-            let mut element = view.update(cx, |view, cx| view.render(cx).into_any());
-            let layout_id = element.layout(cx);
-            (layout_id, element)
-        })
+        let view = view.clone().downcast::<V>().unwrap();
+        let mut element = view.update(cx, |view, cx| view.render(cx).into_any());
+        let layout_id = element.layout(cx);
+        (layout_id, element)
     }
 
     pub(crate) fn paint<V: 'static + Render>(
-        view: &AnyView,
+        _view: &AnyView,
         element: AnyElement,
         cx: &mut WindowContext,
     ) {
-        cx.with_element_id(Some(view.model.entity_id), |cx| {
-            element.paint(cx);
-        })
+        element.paint(cx);
     }
 }

crates/gpui2/src/window.rs 🔗

@@ -230,9 +230,15 @@ pub struct Window {
     pub(crate) focus: Option<FocusId>,
 }
 
+pub(crate) struct ElementStateBox {
+    inner: Box<dyn Any>,
+    #[cfg(debug_assertions)]
+    type_name: &'static str,
+}
+
 // #[derive(Default)]
 pub(crate) struct Frame {
-    pub(crate) element_states: HashMap<GlobalElementId, Box<dyn Any>>,
+    pub(crate) element_states: HashMap<GlobalElementId, ElementStateBox>,
     mouse_listeners: HashMap<TypeId, Vec<(StackingOrder, AnyMouseListener)>>,
     pub(crate) dispatch_tree: DispatchTree,
     pub(crate) focus_listeners: Vec<AnyFocusListener>,
@@ -1815,10 +1821,37 @@ pub trait BorrowWindow: BorrowMut<Window> + BorrowMut<AppContext> {
                         .remove(&global_id)
                 })
             {
+                let ElementStateBox {
+                    inner,
+
+                    #[cfg(debug_assertions)]
+                    type_name
+                } = any;
                 // Using the extra inner option to avoid needing to reallocate a new box.
-                let mut state_box = any
+                let mut state_box = inner
                     .downcast::<Option<S>>()
-                    .expect("invalid element state type for id");
+                    .map_err(|_| {
+                        #[cfg(debug_assertions)]
+                        {
+                            anyhow!(
+                                "invalid element state type for id, requested_type {:?}, actual type: {:?}",
+                                std::any::type_name::<S>(),
+                                type_name
+                            )
+                        }
+
+                        #[cfg(not(debug_assertions))]
+                        {
+                            anyhow!(
+                                "invalid element state type for id, requested_type {:?}",
+                                std::any::type_name::<S>(),
+                            )
+                        }
+                    })
+                    .unwrap();
+
+                // Actual: Option<AnyElement> <- View
+                // Requested: () <- AnyElemet
                 let state = state_box
                     .take()
                     .expect("element state is already on the stack");
@@ -1827,14 +1860,27 @@ pub trait BorrowWindow: BorrowMut<Window> + BorrowMut<AppContext> {
                 cx.window_mut()
                     .current_frame
                     .element_states
-                    .insert(global_id, state_box);
+                    .insert(global_id, ElementStateBox {
+                        inner: state_box,
+
+                        #[cfg(debug_assertions)]
+                        type_name
+                    });
                 result
             } else {
                 let (result, state) = f(None, cx);
                 cx.window_mut()
                     .current_frame
                     .element_states
-                    .insert(global_id, Box::new(Some(state)));
+                    .insert(global_id,
+                        ElementStateBox {
+                            inner: Box::new(Some(state)),
+
+                            #[cfg(debug_assertions)]
+                            type_name: std::any::type_name::<S>()
+                        }
+
+                    );
                 result
             }
         })
@@ -2599,6 +2645,12 @@ pub enum ElementId {
     FocusHandle(FocusId),
 }
 
+impl ElementId {
+    pub(crate) fn from_entity_id(entity_id: EntityId) -> Self {
+        ElementId::View(entity_id)
+    }
+}
+
 impl TryInto<SharedString> for ElementId {
     type Error = anyhow::Error;
 
@@ -2611,12 +2663,6 @@ impl TryInto<SharedString> for ElementId {
     }
 }
 
-impl From<EntityId> for ElementId {
-    fn from(id: EntityId) -> Self {
-        ElementId::View(id)
-    }
-}
-
 impl From<usize> for ElementId {
     fn from(id: usize) -> Self {
         ElementId::Integer(id)

crates/workspace2/src/dock.rs 🔗

@@ -721,7 +721,7 @@ impl Render for PanelButtons {
                                         let panel = panel.clone();
                                         menu = menu.entry(
                                             ListItem::new(
-                                                panel.entity_id(),
+                                                position.to_label(),
                                                 Label::new(format!("Dock {}", position.to_label())),
                                             ),
                                             move |_, cx| {

crates/workspace2/src/pane.rs 🔗

@@ -1350,7 +1350,7 @@ impl Pane {
             let id = item.item_id();
 
             div()
-                .id(item.item_id())
+                .id(ix)
                 .invisible()
                 .group_hover("", |style| style.visible())
                 .child(
@@ -1382,7 +1382,7 @@ impl Pane {
 
         div()
             .group("")
-            .id(item.item_id())
+            .id(ix)
             .cursor_pointer()
             .when_some(item.tab_tooltip_text(cx), |div, text| {
                 div.tooltip(move |cx| cx.build_view(|cx| Tooltip::new(text.clone())).into())

crates/workspace2/src/pane_group.rs 🔗

@@ -214,7 +214,7 @@ impl Member {
                 //     Some(pane)
                 // };
 
-                div().size_full().child(pane.clone())
+                div().size_full().child(pane.clone()).into_any()
 
                 //         Stack::new()
                 //             .with_child(pane_element.contained().with_border(leader_border))
@@ -230,16 +230,18 @@ impl Member {
                 //     .bg(cx.theme().colors().editor)
                 //     .children();
             }
-            Member::Axis(axis) => axis.render(
-                project,
-                basis + 1,
-                follower_states,
-                active_call,
-                active_pane,
-                zoomed,
-                app_state,
-                cx,
-            ),
+            Member::Axis(axis) => axis
+                .render(
+                    project,
+                    basis + 1,
+                    follower_states,
+                    active_call,
+                    active_pane,
+                    zoomed,
+                    app_state,
+                    cx,
+                )
+                .into_any(),
         }
 
         // enum FollowIntoExternalProject {}