From 25e9739e57e3acb20efbebc255efbfbda7d7f509 Mon Sep 17 00:00:00 2001 From: Kayshen-X Date: Sun, 17 May 2026 23:00:56 +0800 Subject: [PATCH] =?UTF-8?q?fix(canvas):=20codex=20review=20round=204=20?= =?UTF-8?q?=E2=80=94=20arc=20clamp,=20path=20hit-test,=20drag=20cleanup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the remaining round-4 codex findings (the `short_src` byte-slice panic + the arc-handle reverse-iteration fixes already landed via an earlier sweep). - `cmd_set_ellipse_arc` clamps `sweep_angle` to ±360° — an API / MCP sweep beyond a full turn just over-draws; it now persists a sane single-revolution value. - Path hit-test (`point_in_node`) follows the flattened, bezier- aware outline instead of the bounding box: a curved or thin path no longer selects empty bbox space, a zero-height stroked path stays clickable, and a filled closed path is hittable across its interior (new `point_in_polygon` even-odd test). - The viewport-less `apply_release` now commits / clears `path_anchor_drag` + `arc_handle_drag` (parity with `apply_release_with_viewport`) so a drag can't leak across that release path. op-editor-core 242 / op-editor-ui 157 / op-host-native 21 tests green. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- .../op-editor-core/src/command_node_attrs.rs | 4 ++- crates/op-editor-ui/src/layout_scene_hit.rs | 36 +++++++++++++++++++ .../op-host-native/src/widget_host/input.rs | 15 ++++++++ 3 files changed, 54 insertions(+), 1 deletion(-) diff --git a/crates/op-editor-core/src/command_node_attrs.rs b/crates/op-editor-core/src/command_node_attrs.rs index d7c81d2ce..593401d0a 100644 --- a/crates/op-editor-core/src/command_node_attrs.rs +++ b/crates/op-editor-core/src/command_node_attrs.rs @@ -340,7 +340,9 @@ impl EditorState { e.start_angle = Some(a); } if let Some(a) = sweep_angle { - e.sweep_angle = Some(a); + // A sweep beyond a full turn just over-draws — + // clamp to a single revolution either direction. + e.sweep_angle = Some(a.clamp(-360.0, 360.0)); } if let Some(r) = inner_radius { e.inner_radius = Some(r); diff --git a/crates/op-editor-ui/src/layout_scene_hit.rs b/crates/op-editor-ui/src/layout_scene_hit.rs index ccb4e0bc1..4708265f6 100644 --- a/crates/op-editor-ui/src/layout_scene_hit.rs +++ b/crates/op-editor-ui/src/layout_scene_hit.rs @@ -157,6 +157,25 @@ fn point_in_node(node: &SceneNode, local: Point2D, bounds: Rect, zoom: f32) -> b let screen_slack = 4.0 / zoom.max(0.0001); return distance_point_to_segment(local, from, to) <= stroke_half + screen_slack; } + // Paths hit-test against their flattened (bezier-aware) outline, + // not the bounding box — a curved / thin path otherwise selects + // empty bbox space, and a zero-height stroked path (which fails + // the positive-area gate below) stays clickable. + if matches!(node.kind, NodeKind::Path) { + let poly = crate::widgets::canvas_viewport_paint::flatten_path(node); + if poly.len() < 2 { + return false; + } + let stroke_half = node.stroke.map(|s| s.width / 2.0).unwrap_or(1.0); + let slack = 4.0 / zoom.max(0.0001); + for seg in poly.windows(2) { + if distance_point_to_segment(local, seg[0], seg[1]) <= stroke_half + slack { + return true; + } + } + // A filled closed path is also hittable across its interior. + return node.path_closed && node.fill.is_some() && point_in_polygon(local, &poly); + } // Non-line kinds need real positive area on both axes. if bounds.size.x <= 0.0 || bounds.size.y <= 0.0 { return false; @@ -232,6 +251,23 @@ fn point_in_triangle(p: Point2D, a: Point2D, b: Point2D, c: Point2D) -> bool { !(has_neg && has_pos) } +/// Even-odd ray-cast point-in-polygon test over a closed vertex ring. +fn point_in_polygon(p: Point2D, poly: &[Point2D]) -> bool { + if poly.len() < 3 { + return false; + } + let mut inside = false; + let mut j = poly.len() - 1; + for i in 0..poly.len() { + let (a, b) = (poly[i], poly[j]); + if (a.y > p.y) != (b.y > p.y) && p.x < (b.x - a.x) * (p.y - a.y) / (b.y - a.y) + a.x { + inside = !inside; + } + j = i; + } + inside +} + /// Shortest distance from `p` to the segment `a`–`b`. fn distance_point_to_segment(p: Point2D, a: Point2D, b: Point2D) -> f32 { let dx = b.x - a.x; diff --git a/crates/op-host-native/src/widget_host/input.rs b/crates/op-host-native/src/widget_host/input.rs index d0d0bb225..e1a0d0dc7 100644 --- a/crates/op-host-native/src/widget_host/input.rs +++ b/crates/op-host-native/src/widget_host/input.rs @@ -607,6 +607,21 @@ impl WidgetHostNative { // Same story as marquee — no viewport, drop the candidate. return true; } + // Path-anchor / arc-handle drags — commit the history snapshot + // when they actually moved (parity with the with-viewport + // release; without this a stale drag would leak). + if let Some(drag) = self.path_anchor_drag.take() { + if drag.moved { + self.editor_state.history_push_past(drag.pre_drag_snapshot); + } + return true; + } + if let Some(drag) = self.arc_handle_drag.take() { + if drag.moved { + self.editor_state.history_push_past(drag.pre_drag_snapshot); + } + return true; + } // Chat drag without viewport — drop it (best effort). if self.chat_drag.take().is_some() { return true;