From 89fca08b8ecb587a42834ece4febf4dd91cc4ac8 Mon Sep 17 00:00:00 2001 From: Kayshen-X Date: Thu, 9 Jul 2026 21:12:18 +0800 Subject: [PATCH] fix(canvas): align overlays with ancestor transforms --- .../src/widgets/canvas_path_overlay.rs | 39 +++- .../src/widgets/canvas_selection_overlay.rs | 73 ++++-- .../src/widgets/canvas_viewport.rs | 33 ++- .../src/widgets/canvas_viewport_paint.rs | 10 +- .../src/widgets/canvas_viewport_tests.rs | 207 +++++++++++++++++- 5 files changed, 324 insertions(+), 38 deletions(-) diff --git a/crates/op-editor-ui/src/widgets/canvas_path_overlay.rs b/crates/op-editor-ui/src/widgets/canvas_path_overlay.rs index 36c4962d9..20def0a22 100644 --- a/crates/op-editor-ui/src/widgets/canvas_path_overlay.rs +++ b/crates/op-editor-ui/src/widgets/canvas_path_overlay.rs @@ -16,6 +16,7 @@ use crate::layout_scene::{SceneAnchor, SceneNode}; use crate::theme::Theme; +use crate::widgets::canvas_overlay_transform::OverlayTransform; use crate::widgets::canvas_viewport::path_handle_positions; use crate::widgets::PaintCx; use crate::{Color, Point2D, Rect}; @@ -70,6 +71,7 @@ pub(super) fn paint_path_overlays( pen_dragging_handle: bool, selected_count: usize, selected_node: Option<&SceneNode>, + selected_transforms: &[OverlayTransform], canvas_rect: Rect, viewport: &Viewport, ) { @@ -95,7 +97,7 @@ pub(super) fn paint_path_overlays( if !matches!(node.kind, crate::layout_scene::NodeKind::Path) || node.hidden { return; } - with_node_rotation(cx, node, canvas_rect, viewport, |cx| { + with_node_overlay_transform(cx, node, canvas_rect, viewport, selected_transforms, |cx| { if matches!(tool, op_editor_core::Tool::Pen) { paint_ghost_edit_handles(cx, node, theme, canvas_rect, viewport); } else { @@ -104,18 +106,36 @@ pub(super) fn paint_path_overlays( }); } -/// Wrap `f` in a save/rotate/restore matching the node's rotation so -/// the overlay sits on the rotated path (handle coords are stored in -/// the node's unrotated local frame). -fn with_node_rotation( +/// Wrap `f` in the same root→node transform chain the path painted +/// under. Fallback to the legacy own-node rotation when called without +/// traversal-captured transforms. +fn with_node_overlay_transform( cx: &mut PaintCx<'_>, node: &SceneNode, canvas_rect: Rect, viewport: &Viewport, + transforms: &[OverlayTransform], f: impl FnOnce(&mut PaintCx<'_>), ) { - let rotated = node.rotation.abs() > f32::EPSILON; - if rotated { + let transformed = super::canvas_overlay_transform::replay_on_backend(cx, transforms) + || replay_legacy_node_rotation(cx, node, canvas_rect, viewport, transforms); + f(cx); + if transformed { + cx.backend.restore(); + } +} + +fn replay_legacy_node_rotation( + cx: &mut PaintCx<'_>, + node: &SceneNode, + canvas_rect: Rect, + viewport: &Viewport, + transforms: &[OverlayTransform], +) -> bool { + if !transforms.is_empty() || node.rotation.abs() <= f32::EPSILON { + return false; + } + { let b = node.aggregate_bounds(); let pivot = to_screen_point( Point2D::new(b.origin.x + b.size.x / 2.0, b.origin.y + b.size.y / 2.0), @@ -125,10 +145,7 @@ fn with_node_rotation( cx.backend.save(); cx.backend.rotate(node.rotation, pivot); } - f(cx); - if rotated { - cx.backend.restore(); - } + true } fn to_screen_point(p: Point2D, canvas_rect: Rect, viewport: &Viewport) -> Point2D { diff --git a/crates/op-editor-ui/src/widgets/canvas_selection_overlay.rs b/crates/op-editor-ui/src/widgets/canvas_selection_overlay.rs index 52120ac16..d3cd2aa5e 100644 --- a/crates/op-editor-ui/src/widgets/canvas_selection_overlay.rs +++ b/crates/op-editor-ui/src/widgets/canvas_selection_overlay.rs @@ -1,5 +1,6 @@ use crate::layout_scene::{NodeKind, SceneNode}; use crate::theme::Theme; +use crate::widgets::canvas_overlay_transform::OverlayTransform; use crate::widgets::PaintCx; use crate::{Color, Point2D, Rect, TextLayout}; use op_editor_core::agent_indicators::AgentIndicators; @@ -26,6 +27,7 @@ pub(super) fn paint_selected_node( node: &SceneNode, input: &SelectionPaintInput<'_>, show_handles: bool, + transforms: &[OverlayTransform], ) { let Some(world_rect) = selection_world_rect(node, input) else { return; @@ -34,15 +36,8 @@ pub(super) fn paint_selected_node( node.kind, NodeKind::Frame | NodeKind::Group | NodeKind::Other(_) ); - let rotated = node.rotation.abs() > f32::EPSILON; - if rotated { - let pivot = Point2D::new( - world_rect.origin.x + world_rect.size.x / 2.0, - world_rect.origin.y + world_rect.size.y / 2.0, - ); - cx.backend.save(); - cx.backend.rotate(node.rotation, pivot); - } + let transformed = super::canvas_overlay_transform::replay_on_backend(cx, transforms) + || replay_legacy_node_rotation(cx, node, world_rect, transforms); super::canvas_viewport_overlay::paint_selection_overlay( cx, world_rect, @@ -50,7 +45,7 @@ pub(super) fn paint_selected_node( is_container, show_handles, ); - if rotated { + if transformed { cx.backend.restore(); } if let (true, Some(label)) = (show_handles, input.selection_label) { @@ -58,6 +53,26 @@ pub(super) fn paint_selected_node( } } +fn replay_legacy_node_rotation( + cx: &mut PaintCx<'_>, + node: &SceneNode, + world_rect: Rect, + transforms: &[OverlayTransform], +) -> bool { + if !transforms.is_empty() || node.rotation.abs() <= f32::EPSILON { + return false; + } + { + let pivot = Point2D::new( + world_rect.origin.x + world_rect.size.x / 2.0, + world_rect.origin.y + world_rect.size.y / 2.0, + ); + cx.backend.save(); + cx.backend.rotate(node.rotation, pivot); + } + true +} + pub(super) fn paint_multi_selection_overlays( cx: &mut PaintCx<'_>, roots: &[SceneNode], @@ -70,9 +85,10 @@ pub(super) fn paint_multi_selection_overlays( let selected: HashSet<&str> = selected_ids.iter().map(String::as_str).collect(); let mut union_rect = None; for root in roots { + let mut transforms = Vec::new(); union_rect = union_optional_rects( union_rect, - paint_selected_subtree(cx, root, &selected, input), + paint_selected_subtree(cx, root, &selected, input, &mut transforms), ); } if let (Some(world_rect), Some(label)) = (union_rect, input.selection_label) { @@ -85,21 +101,52 @@ fn paint_selected_subtree( node: &SceneNode, selected: &HashSet<&str>, input: &SelectionPaintInput<'_>, + transforms: &mut Vec, ) -> Option { + let pushed = push_node_transform(node, input, transforms); let mut union_rect = None; if selected.contains(node.id.as_str()) { union_rect = selection_world_rect(node, input); - paint_selected_node(cx, node, input, false); + paint_selected_node(cx, node, input, false, transforms); } for child in &node.children { union_rect = union_optional_rects( union_rect, - paint_selected_subtree(cx, child, selected, input), + paint_selected_subtree(cx, child, selected, input, transforms), ); } + if pushed { + transforms.pop(); + } union_rect } +fn push_node_transform( + node: &SceneNode, + input: &SelectionPaintInput<'_>, + transforms: &mut Vec, +) -> bool { + if !node.flip_x && !node.flip_y && node.rotation.abs() <= f32::EPSILON { + return false; + } + let bounds = node.aggregate_bounds(); + let pivot = Point2D::new( + input.canvas_rect.origin.x + + input.viewport.pan_x + + (bounds.origin.x + bounds.size.x / 2.0) * input.viewport.zoom, + input.canvas_rect.origin.y + + input.viewport.pan_y + + (bounds.origin.y + bounds.size.y / 2.0) * input.viewport.zoom, + ); + transforms.push(OverlayTransform { + rotation: node.rotation, + flip_x: node.flip_x, + flip_y: node.flip_y, + pivot, + }); + true +} + fn selection_world_rect(node: &SceneNode, input: &SelectionPaintInput<'_>) -> Option { if node.hidden || input.indicators.is_some_and(|indicators| { diff --git a/crates/op-editor-ui/src/widgets/canvas_viewport.rs b/crates/op-editor-ui/src/widgets/canvas_viewport.rs index db5137852..4e6838c9d 100644 --- a/crates/op-editor-ui/src/widgets/canvas_viewport.rs +++ b/crates/op-editor-ui/src/widgets/canvas_viewport.rs @@ -785,6 +785,7 @@ impl<'a> Widget for CanvasViewport<'a> { node, &selection_input, show_handles, + &paint_hits.selected_transforms, ); } else if !self.selected_set.is_empty() { super::canvas_selection_overlay::paint_multi_selection_overlays( @@ -814,6 +815,7 @@ impl<'a> Widget for CanvasViewport<'a> { self.pen_dragging_handle, self.selected_set.len(), anchor_selected_node, + &paint_hits.selected_transforms, rect, viewport, ); @@ -830,17 +832,24 @@ impl<'a> Widget for CanvasViewport<'a> { rect.origin.y + viewport.pan_y + p.y * zoom, ) }; - // Rotate the overlay to match a rotated ellipse. - let rotated = node.rotation.abs() > f32::EPSILON; - if rotated { - let b = node.bounds; - let pivot = to_screen(Point2D::new( - b.origin.x + b.size.x / 2.0, - b.origin.y + b.size.y / 2.0, - )); - cx.backend.save(); - cx.backend.rotate(node.rotation, pivot); - } + // Replay the selected ellipse's transform chain so + // arc handles follow rotated/flipped ancestors too. + let transformed = super::canvas_overlay_transform::replay_on_backend( + cx, + &paint_hits.selected_transforms, + ) || { + let rotated = node.rotation.abs() > f32::EPSILON; + if rotated { + let b = node.bounds; + let pivot = to_screen(Point2D::new( + b.origin.x + b.size.x / 2.0, + b.origin.y + b.size.y / 2.0, + )); + cx.backend.save(); + cx.backend.rotate(node.rotation, pivot); + } + rotated + }; let r = 4.5; // screen-px radius for (_, p) in handles { let center = to_screen(p); @@ -853,7 +862,7 @@ impl<'a> Widget for CanvasViewport<'a> { cx.backend.fill_oval(bounds, self.theme.primary); cx.backend.stroke_oval(bounds, self.theme.background, 1.5); } - if rotated { + if transformed { cx.backend.restore(); } } 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 7f3059972..0ac25e707 100644 --- a/crates/op-editor-ui/src/widgets/canvas_viewport_paint.rs +++ b/crates/op-editor-ui/src/widgets/canvas_viewport_paint.rs @@ -278,6 +278,7 @@ pub struct PaintNodeHits<'a> { /// empty when `hover_rect` is `None` or the chain is identity. pub(crate) hover_transforms: Vec, pub(crate) selected_node: Option<&'a SceneNode>, + pub(crate) selected_transforms: Vec, pub(crate) pen_node: Option<&'a SceneNode>, } @@ -288,6 +289,7 @@ impl<'a> PaintNodeHits<'a> { transforms: &[OverlayTransform], ) -> Self { let hover_rect = hovered_outline_rect(node, options); + let selected_node = (options.selected == Some(node.id.as_str())).then_some(node); Self { hover_transforms: if hover_rect.is_some() { transforms.to_vec() @@ -295,7 +297,12 @@ impl<'a> PaintNodeHits<'a> { Vec::new() }, hover_rect, - selected_node: (options.selected == Some(node.id.as_str())).then_some(node), + selected_transforms: if selected_node.is_some() { + transforms.to_vec() + } else { + Vec::new() + }, + selected_node, pen_node: (options.pen == Some(node.id.as_str())).then_some(node), } } @@ -307,6 +314,7 @@ impl<'a> PaintNodeHits<'a> { } if self.selected_node.is_none() { self.selected_node = child.selected_node; + self.selected_transforms = child.selected_transforms; } if self.pen_node.is_none() { self.pen_node = child.pen_node; 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 100dc5c6f..f0c19ec17 100644 --- a/crates/op-editor-ui/src/widgets/canvas_viewport_tests.rs +++ b/crates/op-editor-ui/src/widgets/canvas_viewport_tests.rs @@ -18,6 +18,7 @@ enum Op { Rotate, Fill, Stroke, + StrokeOval, Text, } @@ -99,6 +100,14 @@ impl crate::RenderBackend for RecordingBackend { self.stroke_colors.push(color); self.ops.push(Op::Stroke); } + fn fill_oval(&mut self, _: Rect, _: Color) { + self.ops.push(Op::Fill); + } + fn stroke_oval(&mut self, _: Rect, color: Color, _: f32) { + self.strokes += 1; + self.stroke_colors.push(color); + self.ops.push(Op::StrokeOval); + } fn stroke_svg_path(&mut self, _: &str, _: Point2D, _: f32, color: Color, _: f32) { self.strokes += 1; self.stroke_colors.push(color); @@ -1075,6 +1084,12 @@ const HOVER_OUTLINE_COLOR: Color = Color { b: 0.965, a: 1.0, }; +const SELECTION_BLUE: Color = Color { + r: 13.0 / 255.0, + g: 153.0 / 255.0, + b: 1.0, + a: 1.0, +}; /// Replay the recorded op stream up to the first stroke painted in /// `color`, tracking the save/restore transform stack, and return the @@ -1103,7 +1118,7 @@ fn active_rotations_at_first_stroke( .push(backend.rotations[rot_i]); rot_i += 1; } - Op::Stroke => { + Op::Stroke | Op::StrokeOval => { if backend.stroke_colors[stroke_i] == color { return Some(stack.last().cloned().unwrap_or_default()); } @@ -1115,6 +1130,43 @@ fn active_rotations_at_first_stroke( None } +fn active_rotations_at_first_oval_stroke( + backend: &RecordingBackend, + color: Color, +) -> Option> { + let mut stroke_i = 0usize; + let mut rot_i = 0usize; + let mut stack: Vec> = vec![Vec::new()]; + for op in &backend.ops { + match op { + Op::Save => { + let top = stack.last().cloned().unwrap_or_default(); + stack.push(top); + } + Op::Restore => { + stack.pop(); + } + Op::Rotate => { + stack + .last_mut() + .expect("unbalanced save/restore") + .push(backend.rotations[rot_i]); + rot_i += 1; + } + Op::Stroke | Op::StrokeOval => { + let is_match = + matches!(op, Op::StrokeOval) && backend.stroke_colors[stroke_i] == color; + if is_match { + return Some(stack.last().cloned().unwrap_or_default()); + } + stroke_i += 1; + } + _ => {} + } + } + None +} + #[test] fn hover_outline_rotates_with_rotated_frame() { let _guard = crate::agent_indicator_test_support::lock(); @@ -1198,6 +1250,159 @@ fn hover_outline_on_child_applies_ancestor_rotation() { ); } +#[test] +fn selection_overlay_on_child_applies_ancestor_rotation() { + let _guard = crate::agent_indicator_test_support::lock(); + op_editor_core::agent_indicators::clear(); + let mut scene = sample_scene(); + let rotation = 0.5_f32; + scene.pages[0].children[0].rotation = rotation; + let mut state = EditorState::new(); + state.set_single_selection(op_editor_core::NodeId::new("n2")); + let viewport = CanvasViewport::from_editor(&state, &scene); + let mut backend = RecordingBackend::default(); + let rect = Rect::xywh(0.0, 0.0, 800.0, 600.0); + { + let mut cx = PaintCx { + backend: &mut backend, + }; + viewport.paint(&mut cx, rect); + } + + let rotations = active_rotations_at_first_stroke(&backend, viewport.theme.primary) + .expect("selected child overlay should paint primary strokes"); + assert!( + rotations.iter().any(|(r, _)| (*r - rotation).abs() < 1e-4), + "selection overlay should inherit the rotated ancestor transform; got {rotations:?}" + ); +} + +#[test] +fn multi_selection_overlay_on_children_applies_ancestor_rotation() { + let _guard = crate::agent_indicator_test_support::lock(); + op_editor_core::agent_indicators::clear(); + let mut scene = sample_scene(); + let rotation = 0.5_f32; + scene.pages[0].children[0].rotation = rotation; + let mut state = EditorState::new(); + state.selection.set = vec![ + op_editor_core::NodeId::new("n2"), + op_editor_core::NodeId::new("n3"), + ]; + state.selection.anchor = op_editor_core::NodeId::new("n2"); + let viewport = CanvasViewport::from_editor(&state, &scene); + let mut backend = RecordingBackend::default(); + { + let mut cx = PaintCx { + backend: &mut backend, + }; + viewport.paint(&mut cx, Rect::xywh(0.0, 0.0, 800.0, 600.0)); + } + + let rotations = active_rotations_at_first_stroke(&backend, viewport.theme.primary) + .expect("multi-selection overlay should paint primary strokes"); + assert!( + rotations.iter().any(|(r, _)| (*r - rotation).abs() < 1e-4), + "multi-selection overlay should inherit the rotated ancestor transform; got {rotations:?}" + ); +} + +#[test] +fn path_editor_overlay_on_child_applies_ancestor_rotation() { + let _guard = crate::agent_indicator_test_support::lock(); + op_editor_core::agent_indicators::clear(); + let mut path = SceneNode::leaf("editing-path", NodeKind::Path); + path.bounds = Rect::xywh(60.0, 80.0, 120.0, 40.0); + path.points = vec![Point2D::new(60.0, 80.0), Point2D::new(180.0, 120.0)]; + path.path_anchors = vec![ + SceneAnchor { + pos: Point2D::new(60.0, 80.0), + handle_in: None, + handle_out: None, + point_type: ScenePointType::Corner, + }, + SceneAnchor { + pos: Point2D::new(180.0, 120.0), + handle_in: None, + handle_out: None, + point_type: ScenePointType::Corner, + }, + ]; + let rotation = 0.5_f32; + let mut frame = SceneNode::leaf("rotated-frame", NodeKind::Frame); + frame.bounds = Rect::xywh(40.0, 40.0, 320.0, 200.0); + frame.rotation = rotation; + frame.children = vec![path]; + let scene = LayoutScene { + pages: vec![ScenePage { + id: "p".into(), + name: "p".into(), + children: vec![frame], + }], + active_page_index: 0, + }; + let mut state = EditorState::new(); + state.set_single_selection(op_editor_core::NodeId::new("editing-path")); + let mut viewport = CanvasViewport::from_editor(&state, &scene); + viewport.tool = op_editor_core::Tool::Select; + let mut backend = RecordingBackend::default(); + { + let mut cx = PaintCx { + backend: &mut backend, + }; + viewport.paint(&mut cx, Rect::xywh(0.0, 0.0, 800.0, 600.0)); + } + + let rotations = active_rotations_at_first_oval_stroke(&backend, SELECTION_BLUE) + .expect("path editor should paint selection-blue anchor strokes"); + assert!( + rotations.iter().any(|(r, _)| (*r - rotation).abs() < 1e-4), + "path editor overlay should inherit the rotated ancestor transform; got {rotations:?}" + ); +} + +#[test] +fn arc_handles_on_child_apply_ancestor_rotation() { + let _guard = crate::agent_indicator_test_support::lock(); + op_editor_core::agent_indicators::clear(); + let mut ellipse = SceneNode::leaf("arc-ellipse", NodeKind::Ellipse); + ellipse.bounds = Rect::xywh(60.0, 80.0, 120.0, 80.0); + ellipse.arc_start_angle = Some(0.0); + ellipse.arc_sweep_angle = Some(90.0); + ellipse.arc_inner_radius = Some(0.5); + let rotation = 0.5_f32; + let mut frame = SceneNode::leaf("rotated-frame", NodeKind::Frame); + frame.bounds = Rect::xywh(40.0, 40.0, 320.0, 200.0); + frame.rotation = rotation; + frame.children = vec![ellipse]; + let scene = LayoutScene { + pages: vec![ScenePage { + id: "p".into(), + name: "p".into(), + children: vec![frame], + }], + active_page_index: 0, + }; + let mut state = EditorState::new(); + state.set_single_selection(op_editor_core::NodeId::new("arc-ellipse")); + let mut viewport = CanvasViewport::from_editor(&state, &scene); + viewport.tool = op_editor_core::Tool::Select; + let mut backend = RecordingBackend::default(); + { + let mut cx = PaintCx { + backend: &mut backend, + }; + viewport.paint(&mut cx, Rect::xywh(0.0, 0.0, 800.0, 600.0)); + } + + let rotations = active_rotations_at_first_oval_stroke(&backend, viewport.theme.background) + .expect("arc handles should paint oval strokes"); + assert!( + rotations.iter().any(|(r, _)| (*r - rotation).abs() < 1e-4), + "arc handles should inherit the rotated ancestor transform; got {rotations:?}" + ); +} + #[test] fn flipped_node_applies_scale_transform() { let state = sample_state();