app: fix chatedit so properties like cursor do eval first before regen. we do this thru lazy eval in draw which makes sense

bin/app/AGENTS.md

@@ -5,4 +5,7 @@
   Imports must always be clustered together so no duplicate `use crate::` lines.
 * `use parking_lot::Mutex as SyncMutex;` is the canonical sync mutex.
   `use async_lock::Mutex as AsyncMutex;` is the canonical async mutex.
+* Internal logic errors should never fail gracefully. Always use `.unwrap()` to
+  enforce explicit failure. Error handling is for the topmost layers coming from
+  the edge of the app such as user input.
 

bin/app/src/ui/edit/action.rs

@@ -33,68 +33,40 @@ struct MenuItem {
     rect: Rectangle,
 }
 
+#[derive(Copy, Clone, PartialEq)]
+pub(super) struct Style {
+    pub font_size: f32,
+    pub fg_color: Color,
+    pub bg_color: Color,
+    pub padding: f32,
+    pub spacing: f32,
+    pub window_scale: f32,
+}
+
 pub struct Menu {
-    font_size: f32,
-    fg_color: Color,
-    bg_color: Color,
-    padding: f32,
-    spacing: f32,
-    window_scale: f32,
-    pub pos: Point,
-    items: Vec<MenuItem>,
+    /// Requested widget-local position, before draw-time clamping.
+    pub anchor: Point,
+    items: Vec<(String, u32)>,
+    style: Option<Style>,
+    /// Position and items used by the last draw, not pending logical changes.
+    pos: Point,
+    rendered_items: Vec<MenuItem>,
 }
 
 impl Menu {
-    pub fn new(
-        font_size: f32,
-        fg_color: Color,
-        bg_color: Color,
-        padding: f32,
-        spacing: f32,
-        window_scale: f32,
-    ) -> Self {
+    pub fn new() -> Self {
         Self {
-            font_size,
-            fg_color,
-            bg_color,
-            padding,
-            spacing,
-            window_scale,
-            pos: Point::zero(),
+            anchor: Point::zero(),
             items: vec![],
+            style: None,
+            pos: Point::zero(),
+            rendered_items: vec![],
         }
     }
 
     pub fn add(&mut self, label: &str, action: u32) {
-        let layout = text::make_layout(
-            label,
-            self.fg_color,
-            self.font_size,
-            0.,
-            self.window_scale,
-            None,
-            &vec![],
-        );
-
-        let item_width = layout.width();
-        let item_height = self.font_size + 2. * self.padding;
-
-        let x_offset = match self.items.last() {
-            Some(item) => item.rect.rhs() + self.spacing,
-            None => 0.,
-        };
-
-        let rect =
-            Rectangle::new(x_offset, -item_height, item_width + 2. * self.padding, item_height);
-
-        self.items.push(MenuItem { layout, action_id: action, rect });
-    }
-
-    pub fn total_width(&self) -> f32 {
-        match self.items.last() {
-            Some(item) => item.rect.rhs(),
-            None => 0.,
-        }
+        self.items.push((label.to_owned(), action));
+        self.style = None;
     }
 }
 
@@ -125,9 +97,8 @@ impl ActionMode {
         let menu = std::mem::take(&mut *self.menu.lock())?;
 
         let local_pos = pos - menu.pos;
-        //d!("interact: pos={:?}, menu.pos={:?}, local_pos={:?}", pos, menu.pos, local_pos);
 
-        for item in &menu.items {
+        for item in &menu.rendered_items {
             if item.rect.contains(local_pos) {
                 d!("Action clicked: {}", item.action_id);
                 return Some(item.action_id);
@@ -146,24 +117,59 @@ impl ActionMode {
         let Some(menu) = &*menu else { return false };
 
         let local_pos = pos - menu.pos;
-        menu.items.iter().any(|item| item.rect.contains(local_pos))
+        menu.rendered_items.iter().any(|item| item.rect.contains(local_pos))
     }
 
     /// Called by the parent layout
-    pub fn get_instrs(&self) -> Vec<DrawInstruction> {
-        let Some(menu) = &*self.menu.lock() else { return vec![] };
+    pub(super) fn get_instrs(&self, style: Style, width: f32) -> Vec<DrawInstruction> {
+        let mut menu = self.menu.lock();
+        let Some(menu) = &mut *menu else { return vec![] };
+
+        if menu.style != Some(style) {
+            let mut x_offset = 0.;
+            menu.rendered_items.clear();
+            for (label, action_id) in &menu.items {
+                let layout = text::make_layout(
+                    label,
+                    style.fg_color,
+                    style.font_size,
+                    0.,
+                    style.window_scale,
+                    None,
+                    &[],
+                );
+                let item_height = style.font_size + 2. * style.padding;
+                let rect = Rectangle::new(
+                    x_offset,
+                    -item_height,
+                    layout.width() + 2. * style.padding,
+                    item_height,
+                );
+                x_offset = rect.rhs() + style.spacing;
+                menu.rendered_items.push(MenuItem { layout, action_id: *action_id, rect });
+            }
+            menu.style = Some(style);
+        }
+
+        let mut total_width = 0.;
+        if let Some(item) = menu.rendered_items.last() {
+            total_width = item.rect.rhs();
+        }
+        menu.pos = menu.anchor;
+        let max_x = width - total_width;
+        menu.pos.x = menu.pos.x.clamp(0., max_x.max(0.));
 
         let mut instrs = vec![DrawInstruction::Move(menu.pos)];
 
-        for item in &menu.items {
+        for item in &menu.rendered_items {
             // Used to reset the pos again
             let mut off_pos = Point::zero();
 
             // Draw background with border
             let mut mesh = MeshBuilder::new(gfxtag!("action_bg"));
             let bg_rect = item.rect.with_zero_pos();
-            mesh.draw_filled_box(&bg_rect, menu.bg_color);
-            mesh.draw_outline(&bg_rect, menu.fg_color, 1.);
+            mesh.draw_filled_box(&bg_rect, style.bg_color);
+            mesh.draw_outline(&bg_rect, style.fg_color, 1.);
 
             off_pos -= item.rect.pos();
             instrs.push(DrawInstruction::Move(item.rect.pos()));
@@ -173,7 +179,7 @@ impl ActionMode {
             let layout_height = item.layout.height();
             // Center text vertically
             let text_y = (item.rect.h - layout_height) / 2.;
-            let text_pos = Point::new(menu.padding, text_y);
+            let text_pos = Point::new(style.padding, text_y);
             let mut txt_instrs =
                 text::render_layout(&item.layout, &self.renderer, gfxtag!("action_txt"));
             off_pos -= text_pos;
@@ -187,3 +193,275 @@ impl ActionMode {
         vec![DrawInstruction::Overlay(instrs)]
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::gfx::GraphicsMethod;
+
+    fn style() -> Style {
+        Style {
+            font_size: 18.,
+            fg_color: [0.9, 0.8, 0.7, 1.],
+            bg_color: [0.1, 0.2, 0.3, 1.],
+            padding: 4.,
+            spacing: 8.,
+            window_scale: 1.,
+        }
+    }
+
+    fn menu() -> Menu {
+        let mut menu = Menu::new();
+        menu.add("Copy", 10);
+        menu.add("Paste", 20);
+        menu.anchor = Point::new(80., 100.);
+        menu
+    }
+
+    #[test]
+    fn first_draw_builds_menu_and_enables_matching_hits() {
+        let (tx, rx) = async_channel::unbounded();
+        let mode = ActionMode::new(Renderer::new(tx));
+        assert!(mode.get_instrs(style(), 1000.).is_empty());
+        mode.set(menu());
+        assert!(rx.is_empty());
+        assert!(!mode.hit(Point::zero()));
+        assert!(!mode.hit(Point::new(81., 99.)));
+        {
+            let menu = mode.menu.lock();
+            let menu = menu.as_ref().unwrap();
+            assert!(menu.style.is_none());
+            assert!(menu.rendered_items.is_empty());
+        }
+
+        let instrs = mode.get_instrs(style(), 1000.);
+        let [DrawInstruction::Overlay(overlay)] = instrs.as_slice() else { panic!() };
+        assert!(matches!(overlay[0], DrawInstruction::Move(pos) if pos == Point::new(80., 100.)));
+        let centers = {
+            let menu = mode.menu.lock();
+            let menu = menu.as_ref().unwrap();
+            assert_eq!(menu.pos, menu.anchor);
+            menu.rendered_items.iter().map(|item| menu.pos + item.rect.center()).collect::<Vec<_>>()
+        };
+        assert!(centers.iter().all(|pos| mode.hit(*pos)));
+        assert_eq!(mode.interact(centers[1]), Some(20));
+        assert!(!mode.hit(centers[0]));
+        assert!(mode.get_instrs(style(), 1000.).is_empty());
+    }
+
+    #[test]
+    fn every_style_field_refreshes_layout_meshes_and_hit_geometry() {
+        let (tx, rx) = async_channel::unbounded();
+        let mode = ActionMode::new(Renderer::new(tx));
+        mode.set(menu());
+        let base = style();
+        for current in [
+            base,
+            Style { font_size: 30., ..base },
+            Style { fg_color: [0.3, 0.7, 0.2, 0.8], ..base },
+            Style { bg_color: [0.8, 0.1, 0.5, 0.6], ..base },
+            Style { padding: 12., ..base },
+            Style { spacing: 24., ..base },
+            Style { window_scale: 2., ..base },
+        ] {
+            // Start each case from the same rendered style to exercise each cache key alone.
+            mode.get_instrs(base, 240.);
+            while rx.try_recv().is_ok() {}
+            let instrs = mode.get_instrs(current, 240.);
+            let [DrawInstruction::Overlay(overlay)] = instrs.as_slice() else { panic!() };
+            let (pos, rects) = {
+                let menu = mode.menu.lock();
+                let menu = menu.as_ref().unwrap();
+                assert!(menu.style == Some(current));
+                assert_eq!(menu.items, vec![("Copy".to_owned(), 10), ("Paste".to_owned(), 20)]);
+                let mut x = 0.;
+                for (item, (label, action_id)) in menu.rendered_items.iter().zip(&menu.items) {
+                    let expected = text::make_layout(
+                        label,
+                        current.fg_color,
+                        current.font_size,
+                        0.,
+                        current.window_scale,
+                        None,
+                        &[],
+                    );
+                    let height = current.font_size + 2. * current.padding;
+                    assert_eq!(
+                        item.rect,
+                        Rectangle::new(x, -height, expected.width() + 2. * current.padding, height)
+                    );
+                    assert_eq!(item.action_id, *action_id);
+                    assert_eq!(item.layout.scale(), current.window_scale);
+                    for line in item.layout.lines() {
+                        for run in line.items() {
+                            if let parley::PositionedLayoutItem::GlyphRun(run) = run {
+                                assert_eq!(run.style().brush, current.fg_color);
+                                assert_eq!(
+                                    run.run().font_size(),
+                                    current.font_size * current.window_scale
+                                );
+                            }
+                        }
+                    }
+                    x = item.rect.rhs() + current.spacing;
+                }
+                let total_width = menu.rendered_items.last().unwrap().rect.rhs();
+                assert_eq!(
+                    menu.pos,
+                    Point::new(80_f32.clamp(0., (240. - total_width).max(0.)), 100.)
+                );
+                (menu.pos, menu.rendered_items.iter().map(|item| item.rect).collect::<Vec<_>>())
+            };
+            let mut cursor = Point::zero();
+            let mut backgrounds = 0;
+            for instr in overlay {
+                match instr {
+                    DrawInstruction::Move(offset) => cursor += *offset,
+                    DrawInstruction::Draw(mesh) if mesh.textures.is_none() => {
+                        assert!(cursor.dist(pos + rects[backgrounds].pos()) < 0.001);
+                        backgrounds += 1;
+                    }
+                    _ => {}
+                }
+            }
+            assert_eq!(backgrounds, 2);
+            assert!(rects.iter().all(|rect| mode.hit(pos + rect.center())));
+            assert!(!mode.hit(pos + Point::new(rects[0].rhs() + current.spacing / 2., -1.)));
+
+            let mut backgrounds = 0;
+            let mut text_meshes = 0;
+            while let Ok((_, method)) = rx.try_recv() {
+                if let GraphicsMethod::NewVertexBuffer((verts, _, tag)) = method {
+                    if tag == gfxtag!("action_bg") {
+                        assert!(verts[..4].iter().all(|v| v.color == current.bg_color));
+                        assert!(verts[4..].iter().all(|v| v.color == current.fg_color));
+                        assert_eq!(verts[3].pos, [rects[backgrounds].w, rects[backgrounds].h]);
+                        backgrounds += 1;
+                    } else if tag == gfxtag!("action_txt") {
+                        assert!(!verts.is_empty());
+                        assert!(verts.iter().all(|v| v.color == current.fg_color));
+                        text_meshes += 1;
+                    }
+                }
+            }
+            assert_eq!(backgrounds, 2);
+            assert!(text_meshes >= 2);
+        }
+    }
+
+    #[test]
+    fn restyling_replaces_stale_hit_rects_and_preserves_actions() {
+        let (tx, _rx) = async_channel::unbounded();
+        let mode = ActionMode::new(Renderer::new(tx));
+        mode.set(menu());
+        mode.get_instrs(Style { font_size: 36., padding: 20., ..style() }, 2000.);
+        let (old_top, old_second) = {
+            let menu = mode.menu.lock();
+            let menu = menu.as_ref().unwrap();
+            (
+                menu.pos + menu.rendered_items[0].rect.pos() + Point::new(1., 1.),
+                menu.pos + Point::new(menu.rendered_items[1].rect.center().x, -1.),
+            )
+        };
+        assert!(mode.hit(old_top));
+        assert!(mode.hit(old_second));
+        let current = Style { spacing: 500., ..style() };
+        mode.get_instrs(current, 2000.);
+        assert!(!mode.hit(old_top));
+        assert!(!mode.hit(old_second));
+        let centers = {
+            let menu = mode.menu.lock();
+            let menu = menu.as_ref().unwrap();
+            menu.rendered_items.iter().map(|item| menu.pos + item.rect.center()).collect::<Vec<_>>()
+        };
+        for (pos, action_id) in centers.into_iter().zip([10, 20]) {
+            assert!(mode.hit(pos));
+            assert_eq!(mode.interact(pos), Some(action_id));
+            mode.set(menu());
+            mode.get_instrs(current, 2000.);
+        }
+        assert_eq!(mode.interact(old_second), None);
+        assert!(mode.get_instrs(current, 2000.).is_empty());
+    }
+
+    #[test]
+    fn width_changes_reclamp_and_restore_anchor_without_rebuilding_layout() {
+        let (tx, _rx) = async_channel::unbounded();
+        let mode = ActionMode::new(Renderer::new(tx));
+        mode.set(menu());
+        mode.get_instrs(style(), 1000.);
+        let (total_width, cached_items) = {
+            let menu = mode.menu.lock();
+            let menu = menu.as_ref().unwrap();
+            (menu.rendered_items.last().unwrap().rect.rhs(), menu.rendered_items.as_ptr())
+        };
+        let narrow_width = total_width + 20.;
+        for (width, x) in [
+            (narrow_width, narrow_width - total_width),
+            (10., 0.),
+            (0., 0.),
+            (1000., 80.),
+            (1000., 80.),
+        ] {
+            let instrs = mode.get_instrs(style(), width);
+            let [DrawInstruction::Overlay(overlay)] = instrs.as_slice() else { panic!() };
+            assert!(matches!(overlay[0], DrawInstruction::Move(pos) if pos == Point::new(x, 100.)));
+            let center = {
+                let menu = mode.menu.lock();
+                let menu = menu.as_ref().unwrap();
+                assert_eq!(menu.anchor, Point::new(80., 100.));
+                assert_eq!(menu.pos, Point::new(x, 100.));
+                assert_eq!(menu.rendered_items.as_ptr(), cached_items);
+                menu.pos + menu.rendered_items[0].rect.center()
+            };
+            assert!(mode.hit(center));
+            assert!(!mode.hit(Point::new(x - 1., 99.)));
+        }
+        mode.menu.lock().as_mut().unwrap().anchor = Point::new(-30., -10.);
+        mode.get_instrs(style(), 1000.);
+        let menu = mode.menu.lock();
+        let menu = menu.as_ref().unwrap();
+        assert_eq!(menu.anchor, Point::new(-30., -10.));
+        assert_eq!(menu.pos, Point::new(0., -10.));
+        assert_eq!(menu.rendered_items.as_ptr(), cached_items);
+    }
+
+    #[test]
+    fn pending_menu_changes_do_not_expose_unrendered_hit_targets() {
+        let (tx, _rx) = async_channel::unbounded();
+        let mode = ActionMode::new(Renderer::new(tx));
+        mode.set(menu());
+        assert_eq!(mode.interact(Point::new(81., 99.)), None);
+        mode.set(menu());
+        mode.get_instrs(style(), 2000.);
+        let old_hit = Point::new(81., 99.);
+        {
+            let mut menu = mode.menu.lock();
+            let menu = menu.as_mut().unwrap();
+            menu.anchor = Point::new(1000., 200.);
+            menu.add("Select All", 30);
+        }
+        assert!(mode.hit(old_hit));
+        assert!(!mode.hit(Point::new(1001., 199.)));
+        mode.get_instrs(style(), 2000.);
+        assert!(!mode.hit(old_hit));
+        let new_hit = {
+            let menu = mode.menu.lock();
+            let menu = menu.as_ref().unwrap();
+            menu.pos + menu.rendered_items[2].rect.center()
+        };
+        assert!(mode.hit(new_hit));
+        assert_eq!(mode.interact(new_hit), Some(30));
+
+        mode.set(menu());
+        mode.get_instrs(style(), 2000.);
+        mode.set(menu());
+        assert!(!mode.hit(old_hit));
+        assert_eq!(mode.interact(old_hit), None);
+        mode.set(menu());
+        mode.get_instrs(style(), 2000.);
+        mode.clear();
+        assert!(!mode.hit(old_hit));
+        assert_eq!(mode.interact(old_hit), None);
+    }
+}

bin/app/src/ui/edit/mod.rs

@@ -38,13 +38,13 @@ use tracing::instrument;
 use crate::android::{is_ime_visible, textinput::AndroidTextInputState};
 use crate::{
     gfx::{
-        anim::Frame as AnimFrame, gfxtag, DrawCall, DrawInstruction, DrawMesh, ManagedSeqAnimPtr,
-        Point, Rectangle, RenderApi, Renderer, Vertex,
+        anim::Frame as AnimFrame, gfxtag, DrawCall, DrawInstruction, ManagedSeqAnimPtr, Point,
+        Rectangle, RenderApi, Renderer, Vertex,
     },
-    mesh::MeshBuilder,
+    mesh::{Color, MeshBuilder},
     prop::{
-        BatchGuardPtr, PropertyAtomicGuard, PropertyBool, PropertyColor, PropertyFloat32,
-        PropertyPtr, PropertyRect, PropertyStr, PropertyUint32, Role,
+        eval_f32_multi, BatchGuardPtr, PropertyAtomicGuard, PropertyBool, PropertyColor,
+        PropertyFloat32, PropertyPtr, PropertyRect, PropertyStr, PropertyUint32, Role,
     },
     scene::{MethodCallSub, Pimpl, SceneNodePtr, SceneNodeWeak},
     text::{self, Editor},
@@ -132,6 +132,13 @@ struct TouchInfo {
 
 pub type BaseEditPtr = Arc<BaseEdit>;
 
+struct CursorRender {
+    anim: ManagedSeqAnimPtr,
+    rect: Rectangle,
+    color: Color,
+    blink_time: u32,
+}
+
 pub struct BaseEdit {
     node: SceneNodeWeak,
     tasks: SyncMutex<Vec<smol::Task<()>>>,
@@ -149,7 +156,7 @@ pub struct BaseEdit {
     select_dc_key: u64,
     text_dc_key: u64,
     cursor_dc_key: u64,
-    cursor_anim: SyncMutex<Option<ManagedSeqAnimPtr>>,
+    cursor_render: SyncMutex<Option<CursorRender>>,
 
     is_active: PropertyBool,
     is_focused: PropertyBool,
@@ -330,7 +337,7 @@ impl BaseEdit {
             select_dc_key: OsRng.gen(),
             text_dc_key: OsRng.gen(),
             cursor_dc_key: OsRng.gen(),
-            cursor_anim: SyncMutex::new(None),
+            cursor_render: SyncMutex::new(None),
 
             is_active,
             is_focused,
@@ -460,25 +467,6 @@ impl BaseEdit {
         cursor_pos + self.behave.inner_pos()
     }
 
-    fn regen_cursor_mesh<R: RenderApi>(&self, renderer: &R) -> DrawMesh {
-        let cursor_width = self.cursor_width.get();
-        let cursor_ascent = self.cursor_ascent.get();
-        let cursor_descent = self.cursor_descent.get();
-        let baseline = self.baseline.get();
-
-        let cursor_rect = Rectangle {
-            x: 0.,
-            y: baseline - cursor_ascent,
-            w: cursor_width,
-            h: cursor_ascent + cursor_descent,
-        };
-        let cursor_color = self.cursor_color.get();
-
-        let mut mesh = MeshBuilder::new(gfxtag!("chatedit_cursor"));
-        mesh.draw_filled_box(&cursor_rect, cursor_color);
-        mesh.alloc(renderer).draw_untextured()
-    }
-
     fn draw_phone_select_handle(&self, mesh: &mut MeshBuilder, mut pos: Point, side: f32) {
         let baseline = self.baseline.get();
         let select_ascent = self.select_ascent.get();
@@ -917,14 +905,7 @@ impl BaseEdit {
     fn gesture_long_press(&self, touch_pos: Point) {
         ed!("gesture_long_press({touch_pos:?}) before=[{}]", self.dbg_state());
 
-        let mut menu = action::Menu::new(
-            self.font_size.get(),
-            self.action_fg_color.get(),
-            self.action_bg_color.get(),
-            self.action_padding.get(),
-            self.action_spacing.get(),
-            self.window_scale.get(),
-        );
+        let mut menu = action::Menu::new();
 
         let atom = &mut self.redraw.make_guard(gfxtag!("BaseEdit::gesture_long_press"));
 
@@ -934,7 +915,7 @@ impl BaseEdit {
         if self.text.get().is_empty() {
             menu.add("Paste", ACTION_PASTE);
             // Set the menu pos to the current cursor pos
-            menu.pos = self.get_cursor_pos();
+            menu.anchor = self.get_cursor_pos();
 
             *self.touch_info.arm.lock() = TouchArm::Inactive;
         } else {
@@ -948,13 +929,7 @@ impl BaseEdit {
                 let editor = self.editor.lock();
                 let (curs_lhs, _) = self.get_select_handles(&editor).unwrap();
                 // Set the menu pos to the LHS of the selection
-                menu.pos = curs_lhs + self.behave.inner_pos();
-            }
-
-            // Adjust menu pos so RHS doesnt leave the RHS of this widget
-            let rect = self.rect.get();
-            if menu.pos.x + menu.total_width() > rect.w {
-                menu.pos.x = rect.w - menu.total_width();
+                menu.anchor = curs_lhs + self.behave.inner_pos();
             }
 
             // Word-selected: release must not set the cursor
@@ -1080,6 +1055,7 @@ impl BaseEdit {
 
         // Move mouse pos within this widget
         self.abs_to_local(&mut clip_mouse_pos);
+        clip_mouse_pos = clip_mouse_pos * self.window_scale.get();
 
         let mut sel_side = 1;
         if let Some(side) = side {
@@ -1167,9 +1143,9 @@ impl BaseEdit {
     /// app-side timed commit and the committed cursor draw call (which
     /// references the anim) never needs to change for pausing.
     fn pause_blinking(&self) {
-        let anim = self.cursor_anim.lock().clone();
-        let Some(anim) = anim else { return };
-        anim.pause(0, self.cursor_idle_time.get() as u64);
+        let cursor = self.cursor_render.lock();
+        let Some(cursor) = &*cursor else { return };
+        cursor.anim.pause(0, self.cursor_idle_time.get() as u64);
     }
 
     #[instrument(target = "ui::edit")]
@@ -1179,10 +1155,46 @@ impl BaseEdit {
         }
 
         let pos = self.get_cursor_pos();
-        let anim = self.cursor_anim.lock().clone().unwrap();
+        let mut cursor = self.cursor_render.lock();
+        self.update_cursor_render(&mut cursor);
+        let anim = cursor.as_ref().unwrap().anim.clone();
         vec![DrawInstruction::Move(pos), DrawInstruction::Animation(anim)]
     }
 
+    fn update_cursor_render(&self, cursor: &mut Option<CursorRender>) {
+        let rect = Rectangle {
+            x: 0.,
+            y: self.baseline.get() - self.cursor_ascent.get(),
+            w: self.cursor_width.get(),
+            h: self.cursor_ascent.get() + self.cursor_descent.get(),
+        };
+        let color = self.cursor_color.get();
+        let blink_time = self.cursor_blink_time.get();
+        let anim = if let Some(cursor) = &*cursor {
+            if cursor.rect == rect && cursor.color == color && cursor.blink_time == blink_time {
+                return
+            }
+            cursor.anim.clone()
+        } else {
+            self.renderer.new_anim(2, false, gfxtag!("baseedit_cursor_blink"))
+        };
+        let mut mesh = MeshBuilder::new(gfxtag!("chatedit_cursor"));
+        mesh.draw_filled_box(&rect, color);
+        let dc_visible = DrawCall::new(
+            vec![DrawInstruction::Draw(mesh.alloc(&self.renderer).draw_untextured())],
+            vec![],
+            2,
+            "cursor_vis",
+        );
+        // Updating frames preserves the animation's timer and interaction pause.
+        anim.update(0, AnimFrame::new(blink_time, dc_visible));
+        anim.update(
+            1,
+            AnimFrame::new(blink_time, DrawCall::new(vec![], vec![], 2, "cursor_invis")),
+        );
+        *cursor = Some(CursorRender { anim, rect, color, blink_time });
+    }
+
     fn regen_bg_mesh<R: RenderApi>(&self, renderer: &R) -> Vec<DrawInstruction> {
         if !self.debug.get() {
             return vec![]
@@ -1311,6 +1323,11 @@ impl BaseEdit {
         self.action_fg_color.eval(atom).expect("action_fg_color");
         self.action_bg_color.eval(atom).expect("action_bg_color");
         self.font_size.eval(atom).expect("font_size");
+        self.lineheight.eval(atom).expect("lineheight");
+        eval_f32_multi(&self.padding, atom, Role::Internal, &[0, 1, 2, 3], vec![])
+            .expect("padding");
+        self.action_padding.eval(atom).expect("action_padding");
+        self.action_spacing.eval(atom).expect("action_spacing");
     }
 
     fn make_draw_calls(&self) -> DrawUpdate {
@@ -1326,7 +1343,17 @@ impl BaseEdit {
         content_instrs.append(&mut bg_instrs);
         content_instrs.push(DrawInstruction::Move(self.behave.scroll()));
 
-        let action_instrs = self.action_mode.get_instrs();
+        let action_instrs = self.action_mode.get_instrs(
+            action::Style {
+                font_size: self.font_size.get(),
+                fg_color: self.action_fg_color.get(),
+                bg_color: self.action_bg_color.get(),
+                padding: self.action_padding.get(),
+                spacing: self.action_spacing.get(),
+                window_scale: self.window_scale.get(),
+            },
+            rect.w,
+        );
 
         // + root (move)
         // -+ content (apply view)
@@ -1667,44 +1694,27 @@ impl UIObject for BaseEdit {
         on_modify.when_change_external(self.window_scale.prop(), scale_changed);
         on_modify.when_change_external(self.text.prop(), reset);
         on_modify.when_change_external(self.text_color.prop(), redraw);
+        on_modify.when_change_external(self.text_hi_color.prop(), redraw);
         on_modify.when_change_external(self.placeholder_text.prop(), redraw);
         on_modify.when_change_external(self.placeholder_color.prop(), redraw);
         on_modify.when_change_external(self.hi_bg_color.prop(), redraw);
+        on_modify.when_change_external(self.action_fg_color.prop(), redraw);
+        on_modify.when_change_external(self.action_bg_color.prop(), redraw);
+        on_modify.when_change_external(self.action_padding.prop(), redraw);
+        on_modify.when_change_external(self.action_spacing.prop(), redraw);
+        // Only multiline edits have a height range.
+        if let Some(height_range) = self.node().get_property("height_range") {
+            on_modify.when_change_external(height_range, redraw);
+        }
         //on_modify.when_change(selected.clone(), redraw);
         on_modify.when_change_external(self.z_index.prop(), redraw);
         on_modify.when_change_external(self.debug.prop(), redraw);
 
-        async fn regen_cursor(self_: Arc<BaseEdit>, _batch: BatchGuardPtr) {
-            // Rebuild the visible frame so cursor style changes apply to
-            // the blink animation.
-            let mesh = self_.regen_cursor_mesh(&self_.renderer);
-            let dc_visible =
-                DrawCall::new(vec![DrawInstruction::Draw(mesh)], vec![], 2, "cursor_vis");
-            if let Some(anim) = self_.cursor_anim.lock().as_ref() {
-                anim.update(0, AnimFrame::new(self_.cursor_blink_time.get(), dc_visible));
-            }
-        }
-        on_modify.when_change_external(self.cursor_color.prop(), regen_cursor);
-        on_modify.when_change_external(self.cursor_ascent.prop(), regen_cursor);
-        on_modify.when_change_external(self.cursor_descent.prop(), regen_cursor);
-        on_modify.when_change_external(self.cursor_width.prop(), regen_cursor);
-
-        // Create the blinking cursor animation using SeqAnim
-        // Frame 0: visible cursor
-        // Frame 1: invisible (empty)
-        let cursor_anim = self.renderer.new_anim(2, false, gfxtag!("baseedit_cursor_blink"));
-        let cursor_mesh = self.regen_cursor_mesh(&self.renderer);
-
-        // Frame 0: visible cursor (just the draw, no position)
-        let dc_visible =
-            DrawCall::new(vec![DrawInstruction::Draw(cursor_mesh)], vec![], 2, "cursor_vis");
-        cursor_anim.update(0, AnimFrame::new(self.cursor_blink_time.get(), dc_visible));
-
-        // Frame 1: invisible (empty draw call)
-        let dc_invisible = DrawCall::new(vec![], vec![], 2, "cursor_invis");
-        cursor_anim.update(1, AnimFrame::new(self.cursor_blink_time.get(), dc_invisible));
-
-        *self.cursor_anim.lock() = Some(cursor_anim);
+        on_modify.when_change_external(self.cursor_color.prop(), redraw);
+        on_modify.when_change_external(self.cursor_ascent.prop(), redraw);
+        on_modify.when_change_external(self.cursor_descent.prop(), redraw);
+        on_modify.when_change_external(self.cursor_width.prop(), redraw);
+        on_modify.when_change_external(self.cursor_blink_time.prop(), redraw);
 
         let (sel_sender, sel_recvr) = async_channel::unbounded();
         *self.sel_sender.lock() = Some(sel_sender);
@@ -1787,8 +1797,8 @@ impl UIObject for BaseEdit {
         atom: &mut PropertyAtomicGuard,
     ) -> Option<DrawUpdate> {
         *self.parent_rect.lock() = Some(parent_rect);
-        self.eval_rect(atom);
         self.eval_style(atom);
+        self.eval_rect(atom);
         // The fresh eval may have changed the content height, so re-clamp
         // the scroll to keep the cursor in view before computing draw instrs.
         self.behave.apply_cursor_scroll();
@@ -1885,14 +1895,7 @@ impl UIObject for BaseEdit {
         }
 
         if btn == MouseButton::Right {
-            let mut menu = action::Menu::new(
-                self.font_size.get(),
-                self.action_fg_color.get(),
-                self.action_bg_color.get(),
-                self.action_padding.get(),
-                self.action_spacing.get(),
-                self.window_scale.get(),
-            );
+            let mut menu = action::Menu::new();
 
             if self.text.get().is_empty() {
                 menu.add("Paste", ACTION_PASTE);
@@ -1903,7 +1906,7 @@ impl UIObject for BaseEdit {
             }
 
             // Mouse pos relative to root layout
-            menu.pos = mouse_pos - rect.pos();
+            menu.anchor = mouse_pos - rect.pos();
 
             self.action_mode.set(menu);
             self.redraw.trigger();
@@ -1935,6 +1938,7 @@ impl UIObject for BaseEdit {
 
         // Move mouse pos within this widget
         self.abs_to_local(&mut mouse_pos);
+        mouse_pos = mouse_pos * self.window_scale.get();
 
         self.editor.lock().driver().move_to_point(mouse_pos.x, mouse_pos.y);
 
@@ -2107,6 +2111,10 @@ impl std::fmt::Debug for BaseEdit {
     }
 }
 
+#[cfg(test)]
+#[path = "tests.rs"]
+mod draw_tests;
+
 #[cfg(test)]
 mod tests {
     use super::*;