git.lucas.co / cce-ui
GPU-accelerated UI toolkit (Vulkan)
git clone https://git.lucas.co/cce-ui.git

commitc786b314c9d71c9b96ffe59535c286e3a15ae150
parentb4c3f82968
authorLucas Galante <[email protected]>
date2026-07-12 15:16
refactor(widget)!: popovers + context-menu target keyed by WidgetId (Phase 6bc slice 2)

active_popovers becomes Vec<WidgetId> and ContextMenuState.target
Option<WidgetId>; every occlusion/render walk (is_coordinate_covered,
point_in_active_popover, render_popovers, the dl-text overlay-rect
collection) resolves ids through the generational tree, so a stale
entry is skipped instead of dereferenced. register_popover takes
&mut and refreshes the registry with the pointer it is handed (same
self-registration contract as set_focused); show_context_menu derives
and registers the target id; context_menu::mouse_input takes the
UiContext that resolves the target for action dispatch.
is_coordinate_covered is id-keyed (WidgetId(0) = the no-base sentinel
that matches nothing); EventCtx::widget_addr is gone (use ectx.id).

Verified: 168 tests; workspace builds; A/B vs the slice-1 captures
byte-equivalent incl. the TE File-menu popover; DE leaf context menu
Copy Key -> wl-paste "input.accel_speed"; settings page dropdown
popover renders in-frame and switches pages.

 src/backend/window_runner.rs          |  8 ++---
 src/context.rs                        | 65 +++++++++++++++++++++--------------
 src/layout.rs                         |  8 +++--
 src/main.rs                           |  2 +-
 src/widget/container/parameters_bg.rs |  4 +--
 src/widget/container/scroll_box.rs    |  4 +--
 src/widget/core.rs                    | 32 ++++++++---------
 src/widget/mod.rs                     |  4 +--
 src/widget/model.rs                   | 10 +-----
 9 files changed, 71 insertions(+), 66 deletions(-)

diff --git a/src/backend/window_runner.rs b/src/backend/window_runner.rs
index b22c4cd..536801d 100644
--- a/src/backend/window_runner.rs
+++ b/src/backend/window_runner.rs
@@ -1887,10 +1887,10 @@ impl<A: Application> EngineState<A> {
         let mut areas: Vec<TextArea<'_>> = Vec::new();
         let mut dl_overlay_rects: Vec<(f32, f32, f32, f32)> = Vec::new();
         if let Some(ctx) = self.inner.as_ref().unwrap().ui_context() {
-            for popover_ptr in &ctx.active_popovers {
-                unsafe {
-                    if let Some(popover) = popover_ptr.as_ref() {
-                        if let Some((x, y, w, h)) = popover.popover_rect() {
+            for &pop_id in &ctx.active_popovers {
+                if let Some(ptr) = ctx.tree.get_ptr(pop_id) {
+                    unsafe {
+                        if let Some((x, y, w, h)) = (*ptr).popover_rect() {
                             dl_overlay_rects.push((x, y, w, h));
                         }
                     }
diff --git a/src/context.rs b/src/context.rs
index 415ea84..9346eda 100644
--- a/src/context.rs
+++ b/src/context.rs
@@ -57,7 +57,8 @@ pub struct UiContext {
     /// The focused widget's id (Phase 6bc: stored ids, not pointers — a stale id resolves to
     /// `None` through the generational tree instead of dereferencing freed memory).
     pub focused_widget: Option<WidgetId>,
-    pub active_popovers: Vec<*const (dyn Element + 'static)>,
+    /// Open-popover registrations, id-keyed like focus (Phase 6bc slice 2).
+    pub active_popovers: Vec<WidgetId>,
     pub hover_state: HoverState,
     pub cursor_pos: (f32, f32),
     pub context_menu: ContextMenuState,
@@ -596,29 +597,39 @@ impl UiContext {
         self.active_popovers.clear();
     }
 
-    pub fn register_popover(&mut self, w: &(dyn Element + 'static)) {
-        let ptr = w as *const (dyn Element + 'static);
-        if !self.active_popovers.contains(&ptr) {
-            self.active_popovers.push(ptr);
+    /// Register an open popover. Takes `&mut` so the registry can be refreshed with the
+    /// pointer we are handed (the occlusion walks resolve the stored id through the tree).
+    pub fn register_popover(&mut self, w: &mut (dyn Element + 'static)) {
+        let Some(id) = w.base().map(|b| b.id()) else { return };
+        self.tree.register(id, w as *mut (dyn Element + 'static));
+        if !self.active_popovers.contains(&id) {
+            self.active_popovers.push(id);
         }
     }
 
     pub fn register_popover_ptr(&mut self, ptr: *mut (dyn Element + 'static)) {
-        let const_ptr = ptr as *const (dyn Element + 'static);
-        if !self.active_popovers.contains(&const_ptr) {
-            self.active_popovers.push(const_ptr);
+        if ptr.is_null() {
+            return;
+        }
+        let Some(id) = (unsafe { (*ptr).base().map(|b| b.id()) }) else { return };
+        self.tree.register(id, ptr);
+        if !self.active_popovers.contains(&id) {
+            self.active_popovers.push(id);
         }
     }
 
-    pub fn is_coordinate_covered(&self, query_address: usize, px: f32, py: f32) -> bool {
-        for popover_ptr in self.active_popovers.iter() {
-            let current_data = *popover_ptr as *const () as usize;
-            if query_address == current_data {
+    /// Whether `(px, py)` is covered by an open popover or a popover-carrying widget other
+    /// than `query_id` (the querying widget excludes itself). Pass `WidgetId(0)` for a widget
+    /// with no base — ids start at 1, so it matches nothing, like the legacy address of a
+    /// widget that could never be registered.
+    pub fn is_coordinate_covered(&self, query_id: WidgetId, px: f32, py: f32) -> bool {
+        for &pop_id in self.active_popovers.iter() {
+            if pop_id == query_id {
                 continue;
             }
-            unsafe {
-                if let Some(popover) = popover_ptr.as_ref() {
-                    if let Some((x, y, width, height)) = popover.popover_rect() {
+            if let Some(ptr) = self.tree.get_ptr(pop_id) {
+                unsafe {
+                    if let Some((x, y, width, height)) = (*ptr).popover_rect() {
                         if px >= x && px <= x + width && py >= y && py <= y + height {
                             return true;
                         }
@@ -626,9 +637,8 @@ impl UiContext {
                 }
             }
         }
-        for (_, ptr) in self.tree.iter_registered() {
-            let current_data = ptr as *const () as usize;
-            if query_address == current_data {
+        for (id, ptr) in self.tree.iter_registered() {
+            if id == query_id {
                 continue;
             }
             unsafe {
@@ -695,7 +705,12 @@ impl UiContext {
     }
 
     pub fn show_context_menu(&mut self, x: f32, y: f32, options: Vec<String>, header_count: usize, target: *mut (dyn Element + 'static)) {
-        crate::widget::context_menu::show(x, y, options, header_count, target);
+        if target.is_null() {
+            return;
+        }
+        let Some(id) = (unsafe { (*target).base().map(|b| b.id()) }) else { return };
+        self.tree.register(id, target);
+        crate::widget::context_menu::show(x, y, options, header_count, id);
     }
 
     pub fn handle_right_click(&mut self, target: *mut (dyn Element + 'static), px: f32, py: f32) {
@@ -758,7 +773,7 @@ impl UiContext {
 
         let scroll_y = crate::widget::hover_animation::get_scroll_offset();
         let adjusted_py = py - scroll_y;
-        crate::widget::context_menu::show(px, adjusted_py, options, header_count, target);
+        self.show_context_menu(px, adjusted_py, options, header_count, target);
     }
 
     pub fn hide_context_menu(&mut self) {
@@ -774,7 +789,7 @@ impl UiContext {
     }
 
     pub fn mouse_input_context_menu(&mut self, button: MouseButton, state: ElementState, px: f32, py: f32) -> bool {
-        crate::widget::context_menu::mouse_input(button, state, px, py)
+        crate::widget::context_menu::mouse_input(button, state, px, py, Some(self))
     }
 
     pub fn context_menu_quads(&self) -> Vec<(f32, f32, f32, f32, [f32; 4])> {
@@ -790,10 +805,10 @@ impl UiContext {
     /// there must never start a window move (the widgets beneath may not block dragging,
     /// e.g. Graph's edge-exclusive canvas hit test).
     fn point_in_active_popover(&self, px: f32, py: f32) -> bool {
-        for popover_ptr in &self.active_popovers {
-            unsafe {
-                if let Some(p) = popover_ptr.as_ref() {
-                    if let Some((x, y, w, h)) = p.popover_rect() {
+        for &pop_id in &self.active_popovers {
+            if let Some(ptr) = self.tree.get_ptr(pop_id) {
+                unsafe {
+                    if let Some((x, y, w, h)) = (*ptr).popover_rect() {
                         if px >= x && px <= x + w && py >= y && py <= y + h {
                             return true;
                         }
diff --git a/src/layout.rs b/src/layout.rs
index 6ca3275..9740224 100644
--- a/src/layout.rs
+++ b/src/layout.rs
@@ -3238,9 +3238,11 @@ pub fn render_widget<T: Element + 'static>(pc: &mut dyn RenderTarget, w: &mut T,
 }
 
 pub fn render_popovers(pc: &mut dyn RenderTarget, ctx: &UiContext) {
-    for popover_ptr in &ctx.active_popovers {
-        unsafe {
-            (**popover_ptr).render_popover(pc);
+    for &pop_id in &ctx.active_popovers {
+        if let Some(ptr) = ctx.tree.get_ptr(pop_id) {
+            unsafe {
+                (*ptr).render_popover(pc);
+            }
         }
     }
 }
diff --git a/src/main.rs b/src/main.rs
index 3de4ff7..97487c3 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -253,7 +253,7 @@ impl Application for DemoApp {
         // itself is drawn into this frame below — there is no popup surface.
         self.ui_context.clear_popovers();
         if self.theme_dropdown.popover_rect().is_some() {
-            self.ui_context.register_popover(&self.theme_dropdown);
+            self.ui_context.register_popover(&mut self.theme_dropdown);
         }
 
         let mut pc = PaintCtx::new();
diff --git a/src/widget/container/parameters_bg.rs b/src/widget/container/parameters_bg.rs
index 199d15e..1433fdf 100644
--- a/src/widget/container/parameters_bg.rs
+++ b/src/widget/container/parameters_bg.rs
@@ -910,7 +910,7 @@ impl Input for ParametersBg {
     fn on_event(&mut self, event: &Event, ectx: &mut EventCtx) -> bool {
         // Copied before `ectx.ui` is borrowed: the wheel arm's occlusion check keys on the
         // adapter's address (the pointer hosts register/popover-track).
-        let self_addr = ectx.widget_addr();
+        let self_id = ectx.id;
         match event {
             // Hosts call `unfocus()` directly (the designer's pane switches): commit the
             // focused row and unfocus the children. Needs no ctx, so the direct path's
@@ -1562,7 +1562,7 @@ impl Input for ParametersBg {
 
                 // The legacy tail's `self.hit_test(px, py, ctx)`: occlusion via the adapter's
                 // address, then rect-or-popover containment.
-                if !changed && !ui.is_coordinate_covered(self_addr, px, py) {
+                if !changed && !ui.is_coordinate_covered(self_id, px, py) {
                     let in_rect = px >= self.rect.x
                         && px <= self.rect.x + self.rect.width
                         && py >= self.rect.y
diff --git a/src/widget/container/scroll_box.rs b/src/widget/container/scroll_box.rs
index 6784e3e..25dfd77 100644
--- a/src/widget/container/scroll_box.rs
+++ b/src/widget/container/scroll_box.rs
@@ -87,7 +87,7 @@ impl ScrollBox {
     /// The legacy `Element` default hit test over the base rect (ScrollBox never carried a
     /// label or row expansion, so those branches are folded away).
     fn hit_test(&self, px: f32, py: f32, ctx: &UiContext) -> bool {
-        if ctx.is_coordinate_covered(self as *const Self as *const () as usize, px, py) {
+        if ctx.is_coordinate_covered(self.base.id(), px, py) {
             return false;
         }
         let (x, y, w, h) = (self.base.x, self.base.y, self.base.w, self.base.h);
@@ -199,7 +199,7 @@ impl ScrollBox {
     /// MouseLeave dispatch the default performed was a no-op for ScrollBox.
     pub fn cursor_moved(&mut self, px: f32, py: f32, ctx: &mut UiContext) -> bool {
         ctx.set_cursor_pos(px, py);
-        if ctx.is_coordinate_covered(self as *const Self as *const () as usize, px, py) {
+        if ctx.is_coordinate_covered(self.base.id(), px, py) {
             let was = self.base.hovered;
             if was {
                 self.base.hovered = false;
diff --git a/src/widget/core.rs b/src/widget/core.rs
index fd33eb8..e438b97 100644
--- a/src/widget/core.rs
+++ b/src/widget/core.rs
@@ -416,7 +416,9 @@ pub mod context_menu {
         pub visible: bool,
         pub options: Vec<String>,
         pub hovered_item: Option<usize>,
-        pub target: Option<*mut (dyn Element + 'static)>,
+        /// The action target, id-keyed (Phase 6bc slice 2): dispatch resolves it through the
+        /// caller's generational tree, so a stale target is a no-op, not a UAF.
+        pub target: Option<WidgetId>,
         pub header_count: usize,
     }
 
@@ -435,10 +437,7 @@ pub mod context_menu {
             }
         }
 
-        pub fn show(&mut self, x: f32, y: f32, options: Vec<String>, header_count: usize, target: *mut (dyn Element + 'static)) {
-            if target.is_null() {
-                return;
-            }
+        pub fn show(&mut self, x: f32, y: f32, options: Vec<String>, header_count: usize, target: WidgetId) {
             self.x = x;
             self.y = y;
             self.options = options;
@@ -474,7 +473,7 @@ pub mod context_menu {
             self.hovered_item != was_hovered
         }
 
-        pub fn mouse_input(&mut self, button: MouseButton, state: ElementState, px: f32, py: f32) -> bool {
+        pub fn mouse_input(&mut self, button: MouseButton, state: ElementState, px: f32, py: f32, ctx: Option<&mut crate::context::UiContext>) -> bool {
             if !self.visible { return false; }
             if button != MouseButton::Left || state != ElementState::Pressed {
                 if state == ElementState::Pressed {
@@ -489,8 +488,8 @@ pub mod context_menu {
                 if idx < self.options.len() {
                     if idx >= self.header_count {
                         let opt = self.options[idx].clone();
-                        if let Some(target_ptr) = self.target {
-                            if !target_ptr.is_null() {
+                        if let (Some(target_id), Some(ctx)) = (self.target, ctx) {
+                            if let Some(target_ptr) = ctx.tree.get_ptr(target_id) {
                                 unsafe {
                                     let target = &mut *target_ptr;
                                     match opt.as_str() {
@@ -599,7 +598,7 @@ pub mod context_menu {
         CONTEXT_MENU.with(|m| m.borrow().visible)
     }
 
-    pub fn show(x: f32, y: f32, options: Vec<String>, header_count: usize, target: *mut (dyn Element + 'static)) {
+    pub fn show(x: f32, y: f32, options: Vec<String>, header_count: usize, target: WidgetId) {
         CONTEXT_MENU.with(|m| m.borrow_mut().show(x, y, options, header_count, target));
     }
 
@@ -608,15 +607,12 @@ pub mod context_menu {
     }
 
     pub fn clear_if_matches(w: &dyn Element) {
+        let Some(id) = w.base().map(|b| b.id()) else { return };
         CONTEXT_MENU.with(|m| {
             let mut menu = m.borrow_mut();
-            if let Some(ptr) = menu.target {
-                let current_data = ptr as *const () as usize;
-                let query_data = w as *const dyn Element as *const () as usize;
-                if current_data == query_data {
-                    menu.target = None;
-                    menu.visible = false;
-                }
+            if menu.target == Some(id) {
+                menu.target = None;
+                menu.visible = false;
             }
         });
     }
@@ -636,8 +632,8 @@ pub mod context_menu {
         CONTEXT_MENU.with(|m| m.borrow_mut().cursor_moved(px, py))
     }
 
-    pub fn mouse_input(button: MouseButton, state: ElementState, px: f32, py: f32) -> bool {
-        CONTEXT_MENU.with(|m| m.borrow_mut().mouse_input(button, state, px, py))
+    pub fn mouse_input(button: MouseButton, state: ElementState, px: f32, py: f32, ctx: Option<&mut crate::context::UiContext>) -> bool {
+        CONTEXT_MENU.with(|m| m.borrow_mut().mouse_input(button, state, px, py, ctx))
     }
 
     pub fn extra_quads() -> Vec<(f32, f32, f32, f32, [f32; 4])> {
diff --git a/src/widget/mod.rs b/src/widget/mod.rs
index 8ffde22..c54e63a 100644
--- a/src/widget/mod.rs
+++ b/src/widget/mod.rs
@@ -333,7 +333,7 @@ pub trait Element {
     }
 
     fn hit_test(&self, px: f32, py: f32, ctx: &UiContext) -> bool {
-        if ctx.is_coordinate_covered(self as *const Self as *const () as usize, px, py) {
+        if ctx.is_coordinate_covered(self.base().map(|b| b.id()).unwrap_or(WidgetId(0)), px, py) {
             return false;
         }
         let (x, y, w, h) = self.rect();
@@ -357,7 +357,7 @@ pub trait Element {
 
     fn cursor_moved(&mut self, px: f32, py: f32, ctx: &mut UiContext) -> bool {
         ctx.set_cursor_pos(px, py);
-        if ctx.is_coordinate_covered(self as *const Self as *const () as usize, px, py) {
+        if ctx.is_coordinate_covered(self.base().map(|b| b.id()).unwrap_or(WidgetId(0)), px, py) {
             let was = self.hovered();
             if was {
                 self.set_hovered(false);
diff --git a/src/widget/model.rs b/src/widget/model.rs
index 474fe76..751dc41 100644
--- a/src/widget/model.rs
+++ b/src/widget/model.rs
@@ -406,14 +406,6 @@ impl EventCtx<'_> {
         }
     }
 
-    /// This widget's identity address for the legacy address-keyed walks
-    /// (`UiContext::is_coordinate_covered` excludes the querying widget by pointer — the
-    /// adapter's, which is also what hosts register/popover-track). Zero outside a routed
-    /// path; the coverage walk then simply excludes nothing.
-    pub fn widget_addr(&self) -> usize {
-        self.self_ptr.map(|p| p as *const () as usize).unwrap_or(0)
-    }
-
     /// The adapter's pointer, for legacy sites that must hand it onward — TreeList makes
     /// itself the focus target (`set_focused_ptr`) and the context-menu target
     /// (`show_context_menu`) with the pointer hosts registered. Transitional; dies with
@@ -1571,7 +1563,7 @@ impl<W: Layout + Paint + Input + 'static> Element for Adapted<W> {
         }
         // Preserve the legacy occlusion check (a covering layer swallows the hit), then delegate
         // the geometric test to the narrow trait instead of the row/label-offset machinery.
-        if ctx.is_coordinate_covered(self as *const Self as *const () as usize, px, py) {
+        if ctx.is_coordinate_covered(self.base.id(), px, py) {
             return false;
         }
         let (x, y, w, h) = self.rect();