From acf9bfbbde9aa37b91622cedc6ebc1b1435fd8be Mon Sep 17 00:00:00 2001 From: Kayshen-X Date: Mon, 15 Jun 2026 08:08:35 +0800 Subject: [PATCH] perf(canvas): reuse scene walk for hover outline --- .../src/widgets/canvas_viewport.rs | 58 +++++------- .../src/widgets/canvas_viewport_paint.rs | 94 ++++++++++++------- .../widgets/canvas_viewport_paint_tests.rs | 30 +++--- .../src/widgets/canvas_viewport_tests.rs | 46 +++++++++ 4 files changed, 145 insertions(+), 83 deletions(-) diff --git a/crates/op-editor-ui/src/widgets/canvas_viewport.rs b/crates/op-editor-ui/src/widgets/canvas_viewport.rs index 68c31aefd..ba34ba01e 100644 --- a/crates/op-editor-ui/src/widgets/canvas_viewport.rs +++ b/crates/op-editor-ui/src/widgets/canvas_viewport.rs @@ -484,26 +484,20 @@ impl<'a> Widget for CanvasViewport<'a> { let reveal_schedule = indicators .as_ref() .and_then(|indicators| reveal_schedule_for_paint(&indicators.reveals, self.now_ms)); + let mut hover_rect = None; for child in page.children.iter().rev() { - if let Some(reveals) = reveal_schedule { - super::canvas_viewport_paint::paint_node_with_reveals( - cx, - child, - viewport_origin, - viewport.zoom, - edit_caret.clone(), - cull, - reveals, - ); - } else { - super::canvas_viewport_paint::paint_node( - cx, - child, - viewport_origin, - viewport.zoom, - edit_caret.clone(), - cull, - ); + let child_hover = super::canvas_viewport_paint::paint_node_with_options( + cx, + child, + viewport_origin, + viewport.zoom, + edit_caret.clone(), + cull, + reveal_schedule, + self.hovered.as_deref(), + ); + if hover_rect.is_none() { + hover_rect = child_hover; } } if let Some(indicators) = indicators.as_ref() { @@ -516,24 +510,14 @@ impl<'a> Widget for CanvasViewport<'a> { indicators, ); } - if let Some(hovered) = self.hovered.as_ref() { - if let Some(node) = page.find(hovered) { - const HOVER: Color = Color { - r: 0.231, - g: 0.51, - b: 0.965, - a: 1.0, - }; - let b = node.aggregate_bounds(); - let screen = Rect { - origin: Point2D::new( - viewport_origin.x + b.origin.x * viewport.zoom, - viewport_origin.y + b.origin.y * viewport.zoom, - ), - size: Point2D::new(b.size.x * viewport.zoom, b.size.y * viewport.zoom), - }; - paint_dashed_rect(cx, screen, HOVER, 1.5); - } + if let Some(screen) = hover_rect { + const HOVER: Color = Color { + r: 0.231, + g: 0.51, + b: 0.965, + a: 1.0, + }; + paint_dashed_rect(cx, screen, HOVER, 1.5); } super::canvas_frame_labels::paint_frame_labels( cx, diff --git a/crates/op-editor-ui/src/widgets/canvas_viewport_paint.rs b/crates/op-editor-ui/src/widgets/canvas_viewport_paint.rs index 42fdbbe64..26dc3a80a 100644 --- a/crates/op-editor-ui/src/widgets/canvas_viewport_paint.rs +++ b/crates/op-editor-ui/src/widgets/canvas_viewport_paint.rs @@ -310,6 +310,7 @@ struct PaintNodeOptions<'a> { edit_caret: Option, cull: Rect, reveals: Option>, + hovered: Option<&'a str>, } #[derive(Clone, Copy)] @@ -325,30 +326,7 @@ enum RevealPaintState { Active(RevealPhase), } -/// Recursively paint one resolved [`SceneNode`] and its subtree. -/// -/// `viewport_origin` is the canvas-rect origin shifted by the -/// viewport pan; `zoom` is the viewport zoom. The scene already -/// carries layout-resolved absolute doc-space bounds, so paint is a -/// straight `doc → world` transform. -pub fn paint_node( - cx: &mut PaintCx<'_>, - node: &SceneNode, - viewport_origin: Point2D, - zoom: f32, - edit_caret: Option, - cull: Rect, -) { - let options = PaintNodeOptions { - viewport_origin, - zoom, - edit_caret, - cull, - reveals: None, - }; - paint_node_inner(cx, node, &options, false); -} - +#[cfg(test)] pub(crate) fn paint_node_with_reveals( cx: &mut PaintCx<'_>, node: &SceneNode, @@ -357,15 +335,38 @@ pub(crate) fn paint_node_with_reveals( edit_caret: Option, cull: Rect, reveals: RevealSchedule<'_>, -) { +) -> Option { + paint_node_with_options( + cx, + node, + viewport_origin, + zoom, + edit_caret, + cull, + Some(reveals), + None, + ) +} + +pub(crate) fn paint_node_with_options<'a>( + cx: &mut PaintCx<'_>, + node: &SceneNode, + viewport_origin: Point2D, + zoom: f32, + edit_caret: Option, + cull: Rect, + reveals: Option>, + hovered: Option<&'a str>, +) -> Option { let options = PaintNodeOptions { viewport_origin, zoom, edit_caret, cull, - reveals: Some(reveals), + reveals, + hovered, }; - paint_node_inner(cx, node, &options, false); + paint_node_inner(cx, node, &options, false) } fn paint_node_inner( @@ -373,7 +374,7 @@ fn paint_node_inner( node: &SceneNode, options: &PaintNodeOptions<'_>, ancestor_revealing: bool, -) { +) -> Option { let viewport_origin = options.viewport_origin; let zoom = options.zoom; let edit_caret = &options.edit_caret; @@ -385,7 +386,7 @@ fn paint_node_inner( .map(|schedule| reveal_paint_state(schedule, &node.id)) .unwrap_or(RevealPaintState::Idle); if node.hidden || matches!(reveal_state, RevealPaintState::Pending) { - return; + return None; } let own_reveal_phase = match reveal_state { RevealPaintState::Active(phase) if !ancestor_revealing => Some(phase), @@ -413,9 +414,10 @@ fn paint_node_inner( || world_rect.origin.y + world_rect.size.y < cull.origin.y || world_rect.origin.y > cull.origin.y + cull.size.y; if off { - return; + return None; } } + let mut hover_rect = None; let reveal_wrapped = own_reveal_phase .map(|phase| push_reveal_transform(cx, world_rect, phase)) @@ -481,7 +483,11 @@ fn paint_node_inner( paint_widget_visual(cx, node, world_rect, zoom); let clipped = push_clip_content(cx, node, world_rect, zoom); for child in node.children.iter().rev() { - paint_node_inner(cx, child, options, descendant_has_revealing_ancestor); + let child_hover = + paint_node_inner(cx, child, options, descendant_has_revealing_ancestor); + if hover_rect.is_none() { + hover_rect = child_hover; + } } if clipped { cx.backend.restore(); @@ -500,7 +506,11 @@ fn paint_node_inner( // every recursing container branch, not just Frame. let clipped = push_clip_content(cx, node, world_rect, zoom); for child in node.children.iter().rev() { - paint_node_inner(cx, child, options, descendant_has_revealing_ancestor); + let child_hover = + paint_node_inner(cx, child, options, descendant_has_revealing_ancestor); + if hover_rect.is_none() { + hover_rect = child_hover; + } } if clipped { cx.backend.restore(); @@ -581,7 +591,7 @@ fn paint_node_inner( if reveal_wrapped { cx.backend.restore(); } - return; + return hovered_outline_rect(node, options); } // Bezier-aware: when the path carries anchors with control // handles, flatten each cubic segment; otherwise fall back @@ -643,6 +653,24 @@ fn paint_node_inner( if reveal_wrapped { cx.backend.restore(); } + hover_rect.or_else(|| hovered_outline_rect(node, options)) +} + +fn hovered_outline_rect(node: &SceneNode, options: &PaintNodeOptions<'_>) -> Option { + if options.hovered != Some(node.id.as_str()) { + return None; + } + let bounds = node.aggregate_bounds(); + if bounds.size.x <= 0.0 || bounds.size.y <= 0.0 { + return None; + } + Some(Rect { + origin: Point2D::new( + options.viewport_origin.x + bounds.origin.x * options.zoom, + options.viewport_origin.y + bounds.origin.y * options.zoom, + ), + size: Point2D::new(bounds.size.x * options.zoom, bounds.size.y * options.zoom), + }) } fn reveal_paint_state(schedule: RevealSchedule<'_>, node_id: &str) -> RevealPaintState { diff --git a/crates/op-editor-ui/src/widgets/canvas_viewport_paint_tests.rs b/crates/op-editor-ui/src/widgets/canvas_viewport_paint_tests.rs index d879cc376..94efa570b 100644 --- a/crates/op-editor-ui/src/widgets/canvas_viewport_paint_tests.rs +++ b/crates/op-editor-ui/src/widgets/canvas_viewport_paint_tests.rs @@ -35,7 +35,7 @@ mod arc_tests { mod text_tests { use crate::layout_scene::{NodeKind, SceneNode, SceneTextAlign, SceneTextVerticalAlign}; - use crate::widgets::canvas_viewport_paint::{paint_node, paint_svg_path_node}; + use crate::widgets::canvas_viewport_paint::{paint_node_with_options, paint_svg_path_node}; use crate::widgets::canvas_viewport_text::paint_text_node; use crate::widgets::PaintCx; use crate::{Color, ImageDrawMode, Point2D, Rect, RenderBackend, TextLayout}; @@ -97,6 +97,16 @@ mod text_tests { } } + fn paint_node( + cx: &mut PaintCx<'_>, + node: &SceneNode, + viewport_origin: Point2D, + zoom: f32, + cull: Rect, + ) { + let _ = paint_node_with_options(cx, node, viewport_origin, zoom, None, cull, None, None); + } + #[test] fn text_node_paint_honors_horizontal_alignment_and_ts_top_baseline() { let mut node = SceneNode::leaf("t", NodeKind::Text); @@ -143,7 +153,6 @@ mod text_tests { &node, Point2D::ZERO, 1.0, - None, Rect::xywh(0.0, 0.0, 800.0, 600.0), ); @@ -156,7 +165,6 @@ mod text_tests { &node, Point2D::ZERO, 2.0, - None, Rect::xywh(0.0, 0.0, 800.0, 600.0), ); @@ -184,7 +192,6 @@ mod text_tests { &node, viewport_origin, 2.0, - None, Rect::xywh(0.0, 0.0, 800.0, 600.0), ); @@ -572,7 +579,7 @@ mod path_tests { mod clip_tests { use crate::layout_scene::{NodeKind, SceneNode}; - use crate::widgets::canvas_viewport_paint::paint_node; + use crate::widgets::canvas_viewport_paint::paint_node_with_options; use crate::widgets::PaintCx; use crate::{Color, Point2D, Rect, RenderBackend, TextLayout}; @@ -621,6 +628,10 @@ mod clip_tests { } } + fn paint_node(cx: &mut PaintCx<'_>, node: &SceneNode, cull: Rect) { + let _ = paint_node_with_options(cx, node, Point2D::ZERO, 1.0, None, cull, None, None); + } + fn frame_with_child(clip: bool, corner_radius: f32) -> SceneNode { let mut child = SceneNode::leaf("c", NodeKind::Rect); child.bounds = Rect::xywh(10.0, 10.0, 500.0, 20.0); @@ -639,14 +650,7 @@ mod clip_tests { let mut cx = PaintCx { backend: &mut backend, }; - paint_node( - &mut cx, - node, - Point2D::ZERO, - 1.0, - None, - Rect::xywh(0.0, 0.0, 4000.0, 4000.0), - ); + paint_node(&mut cx, node, Rect::xywh(0.0, 0.0, 4000.0, 4000.0)); backend.ops } diff --git a/crates/op-editor-ui/src/widgets/canvas_viewport_tests.rs b/crates/op-editor-ui/src/widgets/canvas_viewport_tests.rs index fec62e88b..4f23d1d6c 100644 --- a/crates/op-editor-ui/src/widgets/canvas_viewport_tests.rs +++ b/crates/op-editor-ui/src/widgets/canvas_viewport_tests.rs @@ -586,6 +586,52 @@ fn multi_selection_overlay_finds_nodes_linearly() { ); } +#[test] +fn hover_outline_does_not_deep_search_after_scene_paint() { + let _guard = crate::agent_indicator_test_support::lock(); + op_editor_core::agent_indicators::clear(); + + let mut node = SceneNode::leaf("hovered-leaf", NodeKind::Rect); + node.bounds = Rect::xywh(24.0, 24.0, 40.0, 40.0); + let depth = 8; + for i in (0..depth).rev() { + let mut frame = SceneNode::leaf(format!("wrap-{i}"), NodeKind::Frame); + frame.bounds = Rect::xywh(0.0, 0.0, 120.0, 120.0); + frame.children = vec![node]; + node = frame; + } + let scene = LayoutScene { + pages: vec![ScenePage { + id: "p".into(), + name: "p".into(), + children: vec![node], + }], + active_page_index: 0, + }; + let state = EditorState::new(); + let mut viewport = CanvasViewport::from_editor(&state, &scene); + viewport.hovered = Some("hovered-leaf".into()); + + crate::layout_scene::reset_find_visit_count(); + let mut backend = RecordingBackend::default(); + { + let mut cx = PaintCx { + backend: &mut backend, + }; + viewport.paint(&mut cx, Rect::xywh(0.0, 0.0, 300.0, 300.0)); + } + + assert_eq!( + crate::layout_scene::find_visit_count(), + 0, + "hover outline should paint during the existing scene walk instead of deep-searching after paint" + ); + assert!( + backend.strokes > 0, + "hover outline should still paint dashed strokes" + ); +} + #[test] fn flipped_node_applies_scale_transform() { let state = sample_state();