From 2931a71f48550a5d78e1071a4833283271fd54fb Mon Sep 17 00:00:00 2001 From: yongyin-leon <269641072+yongyin-leon@users.noreply.github.com> Date: Sat, 24 Oct 2026 22:23:25 +0800 Subject: [PATCH 1/2] fix(edit): preserve same-feature targets when snapping vertices --- crates/engine/src/cmd/design.rs | 35 +++- crates/engine/src/cmd/design/snap_vertex.rs | 115 +++++++++++ crates/engine/tests/vertex_snap_identity.rs | 205 ++++++++++++++++++++ crates/ui-egui/src/vertexedit.rs | 12 +- crates/ui-egui/tests/vertex_snapping.rs | 165 ++++++++++++++++ 5 files changed, 527 insertions(+), 5 deletions(-) create mode 100644 crates/engine/src/cmd/design/snap_vertex.rs create mode 100644 crates/engine/tests/vertex_snap_identity.rs create mode 100644 crates/ui-egui/tests/vertex_snapping.rs diff --git a/crates/engine/src/cmd/design.rs b/crates/engine/src/cmd/design.rs index f5f9c7ef7..b39d8e8e3 100644 --- a/crates/engine/src/cmd/design.rs +++ b/crates/engine/src/cmd/design.rs @@ -12,6 +12,8 @@ use serde_json::{Value as Json, json}; use super::{b, f, need_feature_layer, ps}; use crate::{CmdResult, CommandSpec as C, Session}; +mod snap_vertex; + /// One edited feature: its state before (none for an insert) and its row after (none for a /// delete). pub(crate) struct Change { @@ -1069,7 +1071,31 @@ pub fn snap_at(s: &mut Session, c: Coord) -> Option<(Coord, &'static str)> { /// [`snap_at`], leaving out one feature (layer id, row): a vertex being dragged doesn't snap to /// the shape it is changing. +/// Use [`snap_vertex_except`] when the source node is known and its other parts should remain targets. pub fn snap_except(s: &mut Session, c: Coord, skip: Option<(u32, usize)>) -> Option<(Coord, &'static str)> { + snap_excluding(s, c, skip, None) +} + +/// Snaps while moving one source vertex, excluding that node and its incident edges instead of +/// its whole feature. Other line/polygon parts remain targets. Polygon closing copies alias the +/// first node; a closed line's two endpoints remain distinct, as in `Geometry::move_vertex`. +/// Invalid addresses are ignored. Point/multipoint targets retain the existing snapping policy. +/// Moving a point or multipoint still excludes its whole feature, as with [`snap_except`]. +pub fn snap_vertex_except(s: &mut Session, c: Coord, skip: Option<(u32, usize, rustgis_data::VertexId)>) -> Option<(Coord, &'static str)> { + let skip = skip.filter(|(id, row, v)| { + s.map().layer(*id).and_then(|l| l.dataset.features.get(*row)).and_then(|f| f.geometry.as_ref()).and_then(|g| g.vertex(*v)).is_some() + }); + snap_excluding(s, c, None, skip) +} + +/// Shares the original layer/row ordering, scale rules and distance selection between legacy +/// feature exclusion and source-node exclusion. Only the addressed row needs source identities. +fn snap_excluding( + s: &mut Session, + c: Coord, + skip: Option<(u32, usize)>, + vertex: Option<(u32, usize, rustgis_data::VertexId)>, +) -> Option<(Coord, &'static str)> { let sn = s.map().editing.snapping; if !sn.enabled { return None; @@ -1092,11 +1118,18 @@ pub fn snap_except(s: &mut Session, c: Coord, skip: Option<(u32, usize)>) -> Opt continue; } let id = l.id; - for (row, g) in l.projected_in(&flat, &key).iter().enumerate() { + l.projected_in(&flat, &key); + for (row, g) in l.projected_cache().iter().enumerate() { let Some(g) = g.as_ref().filter(|_| skip != Some((id, row))) else { continue }; if !g.extent().padded(0.0, tol).contains(m) { continue; } + if let Some((_, _, v)) = vertex.filter(|(layer, source_row, _)| *layer == id && *source_row == row) { + if let Some(source) = l.dataset.features.get(row).and_then(|f| f.geometry.as_ref()) { + snap_vertex::consider(source, v, &flat, sn, m, &mut consider); + } + continue; + } let paths: Vec<&Vec> = match g { Geometry::Line(parts) => parts.iter().collect(), Geometry::Polygon(polys) => polys.iter().flatten().collect(), diff --git a/crates/engine/src/cmd/design/snap_vertex.rs b/crates/engine/src/cmd/design/snap_vertex.rs new file mode 100644 index 000000000..8bbb96695 --- /dev/null +++ b/crates/engine/src/cmd/design/snap_vertex.rs @@ -0,0 +1,115 @@ +//! Source-addressed candidates for the one feature whose node is being moved. +//! Rendered parts cannot supply identities: world-projection cuts can add parts and vertices. + +use rustgis_data::{Geometry, VertexId}; +use rustgis_geom::Coord; +use rustgis_map::{design::Snapping, projection::Flat}; + +/// The same projection, switches and map-coordinate pointer apply to every source path. +#[derive(Clone, Copy)] +struct SnapView<'a> { + flat: &'a Flat, + snapping: Snapping, + pointer: Coord, +} + +/// Visits eligible line/polygon paths in source order, without an index or whole-feature clone. +/// Other geometry kinds retain their previous whole-feature exclusion during a node move. +pub(super) fn consider( + geometry: &Geometry, + excluded: VertexId, + flat: &Flat, + snapping: Snapping, + pointer: Coord, + visit: &mut impl FnMut(f64, Coord, &'static str), +) { + let view = SnapView { flat, snapping, pointer }; + let mut path = |points: &[Coord], part, ring, polygon| { + consider_path(points, VertexId::new(part, ring, 0), polygon, excluded, view, visit); + }; + match geometry { + Geometry::Line(parts) => { + for (part, points) in parts.iter().enumerate() { + path(points, part, 0, false); + } + } + Geometry::Polygon(polygons) => { + for (part, rings) in polygons.iter().enumerate() { + for (ring, points) in rings.iter().enumerate() { + path(points, part, ring, true); + } + } + } + _ => {} + } +} + +/// Excludes source endpoints before projection, preserving endpoint-before-vertex-before-edge +/// visitation. Only a polygon's actual repeated closing coordinate aliases its first node. +fn consider_path( + path: &[Coord], + address: VertexId, + polygon: bool, + excluded: VertexId, + view: SnapView<'_>, + visit: &mut impl FnMut(f64, Coord, &'static str), +) { + let SnapView { flat, snapping, pointer } = view; + let closed = polygon && path.len() > 1 && path.first() == path.last(); + let count = path.len().saturating_sub(usize::from(closed)); + let skip = |index| { + address.part == excluded.part && address.ring == excluded.ring && excluded.index == if closed && index == count { 0 } else { index } + }; + let mut point = |index, kind| { + if !skip(index) + && let Some(at) = path.get(index).and_then(|at| flat.forward_checked(*at)) + { + visit((at.x - pointer.x).powi(2) + (at.y - pointer.y).powi(2), at, kind); + } + }; + if snapping.end && !polygon { + for index in [0, path.len().saturating_sub(1)] { + point(index, "end"); + } + } + if snapping.vertex { + for index in 0..count { + point(index, "vertex"); + } + } + if snapping.edge { + for (index, pair) in path.windows(2).enumerate() { + if !skip(index) + && !skip(index + 1) + && let (Some(a), Some(b)) = (pair.first(), pair.get(1)) + { + consider_edge(*a, *b, flat, pointer, visit); + } + } + } +} + +/// Projects a surviving source edge using the same world-edge cuts as ordinary snapping. +/// Each projected piece is considered separately; no segment joins opposite sides of the map. +fn consider_edge(a: Coord, b: Coord, flat: &Flat, pointer: Coord, visit: &mut impl FnMut(f64, Coord, &'static str)) { + let mut segment = |a: Coord, b: Coord| { + let (dx, dy) = (b.x - a.x, b.y - a.y); + let length = dx * dx + dy * dy; + let t = if length > 0.0 { (((pointer.x - a.x) * dx + (pointer.y - a.y) * dy) / length).clamp(0.0, 1.0) } else { 0.0 }; + let at = Coord::new(a.x + t * dx, a.y + t * dy); + visit((at.x - pointer.x).powi(2) + (at.y - pointer.y).powi(2), at, "edge"); + }; + if matches!(flat, Flat::Custom(projector) if projector.edge().is_some()) { + if let Geometry::Line(parts) = flat.project(&Geometry::Line(vec![vec![a, b]])) { + for path in parts { + for pair in path.windows(2) { + if let (Some(a), Some(b)) = (pair.first(), pair.get(1)) { + segment(*a, *b); + } + } + } + } + } else if let (Some(a), Some(b)) = (flat.forward_checked(a), flat.forward_checked(b)) { + segment(a, b); + } +} diff --git a/crates/engine/tests/vertex_snap_identity.rs b/crates/engine/tests/vertex_snap_identity.rs new file mode 100644 index 000000000..25fcbf61b --- /dev/null +++ b/crates/engine/tests/vertex_snap_identity.rs @@ -0,0 +1,205 @@ +//! Source-addressed snapping for ordinary in-memory feature layers. +use rustgis_engine::{ + Project, Session, + cmd::design::{snap_except, snap_vertex_except}, + data::{Dataset, Feature, Geometry, GeometryType, VertexId}, + geom::{Coord, merc_to_lonlat}, + map::{Projection, design::Snapping}, +}; +use serde_json::json; + +/// Makes small fixtures in Mercator metres while storing the original longitude/latitude nodes. +fn c(x: f64, y: f64) -> Coord { + merc_to_lonlat(Coord::new(x, y)) +} + +/// A public upstream session with one source geometry and a two-pixel snapping tolerance. +fn fixture(g: Geometry) -> (Session, u32) { + let mut s = Session::new(Project::new("Source vertex snapping")); + let mut data = Dataset::new("Paths", g.kind(), vec![]); + data.features = vec![Feature { id: 1, geometry: Some(g), attrs: vec![] }].into(); + let id = s.map_mut().add_dataset(data); + s.map_mut().camera.res = 1.0; + s.map_mut().editing.snapping = Snapping { enabled: true, tolerance_px: 2.0, vertex: false, edge: true, end: false }; + (s, id) +} + +/// Requires the expected candidate kind and coordinate, allowing only inverse-projection rounding. +fn hit(actual: Option<(Coord, &'static str)>, expected: Coord, kind: &str) { + let (at, what) = actual.expect("eligible source target"); + assert_eq!(what, kind); + assert!((at.x - expected.x).abs() < 1e-7 && (at.y - expected.y).abs() < 1e-7, "{at:?} != {expected:?}"); +} + +/// The legacy whole-feature exclusion demonstrates the limitation on the very same fixture. +/// Addressed exclusion keeps a different part and a distinct node at the same coordinate. +#[test] +fn another_part_and_a_coincident_node_remain_targets() { + let (mut s, id) = fixture(Geometry::Line(vec![vec![c(0., 0.), c(0., 100.)], vec![c(0., 0.), c(-100., 0.)]])); + let node = Some((id, 0, VertexId::new(0, 0, 0))); + assert!(snap_except(&mut s, c(-50., 0.5), Some((id, 0))).is_none()); + hit(snap_vertex_except(&mut s, c(-50., 0.5), node), c(-50., 0.), "edge"); + assert!(snap_vertex_except(&mut s, c(0.5, 50.), node).is_none(), "the moving node's edge remains excluded"); + s.map_mut().editing.snapping.vertex = true; + s.map_mut().editing.snapping.edge = false; + hit(snap_vertex_except(&mut s, c(0.2, 0.1), node), c(0., 0.), "vertex"); +} + +/// Repeated coordinates within a line are distinct addresses, and a nonincident edge is eligible. +#[test] +fn repeated_coordinates_in_one_part_do_not_exclude_other_nodes() { + let (mut s, id) = fixture(Geometry::Line(vec![vec![c(0., 0.), c(100., 0.), c(0., 0.), c(0., 100.)]])); + let node = Some((id, 0, VertexId::new(0, 0, 0))); + hit(snap_vertex_except(&mut s, c(0.5, 50.), node), c(0., 50.), "edge"); + hit(snap_vertex_except(&mut s, c(50., 0.5), node), c(50., 0.), "edge"); +} + +/// A polygon's first node and its stored closing copy exclude both incident edges, but neither +/// another polygon's coincident node nor a hole's unrelated boundary is removed. +#[test] +fn polygon_closing_aliases_and_hole_addresses_stay_local() { + let outer = vec![c(0., 0.), c(100., 0.), c(100., 100.), c(0., 100.), c(0., 0.)]; + let hole = vec![c(20., 20.), c(40., 20.), c(40., 40.), c(20., 40.), c(20., 20.)]; + let other = vec![c(0., 0.), c(-100., 0.), c(-100., -100.), c(0., -100.), c(0., 0.)]; + let (mut s, id) = fixture(Geometry::Polygon(vec![vec![outer, hole], vec![other]])); + let outer_node = Some((id, 0, VertexId::new(0, 0, 0))); + assert!(snap_vertex_except(&mut s, c(50., 0.5), outer_node).is_none()); + assert!(snap_vertex_except(&mut s, c(0.5, 50.), outer_node).is_none()); + hit(snap_vertex_except(&mut s, c(30., 20.5), outer_node), c(30., 20.), "edge"); + let hole_node = Some((id, 0, VertexId::new(0, 1, 0))); + assert!(snap_vertex_except(&mut s, c(30., 20.5), hole_node).is_none()); + hit(snap_vertex_except(&mut s, c(50., 0.5), hole_node), c(50., 0.), "edge"); + s.map_mut().editing.snapping.vertex = true; + s.map_mut().editing.snapping.edge = false; + hit(snap_vertex_except(&mut s, c(0.2, 0.1), outer_node), c(0., 0.), "vertex"); + let geometry = s.map().layer(id).unwrap().dataset.features[0].geometry.as_ref().unwrap(); + let moved = geometry.move_vertex(VertexId::new(0, 0, 0), c(-5., -5.)).unwrap(); + let Geometry::Polygon(polygons) = moved else { panic!("polygon") }; + assert_eq!(polygons[0][0].first(), Some(&c(-5., -5.))); + assert_eq!(polygons[0][0].first(), polygons[0][0].last()); + assert_eq!(polygons[0][1].first(), Some(&c(20., 20.))); +} + +/// Unlike polygon closure, a closed line has two independent end addresses. The unchanged +/// edit command moves only the requested end, and still makes one undoable edit. +#[test] +fn closed_line_endpoints_keep_the_upstream_edit_contract() { + let (mut s, id) = fixture(Geometry::Line(vec![vec![c(0., 0.), c(100., 0.), c(100., 100.), c(0., 100.), c(0., 0.)]])); + s.map_mut().editing.snapping = Snapping { edge: false, vertex: false, end: true, ..s.map().editing.snapping }; + for index in [0, 4] { + hit(snap_vertex_except(&mut s, c(0.2, 0.1), Some((id, 0, VertexId::new(0, 0, index)))), c(0., 0.), "end"); + } + s.clear_history(); + let at = c(-20., 0.); + s.run("edit.vertex", &json!({"layer":id,"row":0,"action":"move","index":0,"lon":at.x,"lat":at.y})).unwrap(); + let geometry = s.map().layer(id).unwrap().dataset.features[0].geometry.as_ref().unwrap(); + assert_eq!(geometry.vertex(VertexId::new(0, 0, 0)), Some(at)); + assert_eq!(geometry.vertex(VertexId::new(0, 0, 4)), Some(c(0., 0.))); + assert_eq!(s.undo_depth(), 1); + s.run("edit.undo", &json!(null)).unwrap(); + let geometry = s.map().layer(id).unwrap().dataset.features[0].geometry.as_ref().unwrap(); + assert_eq!(geometry.vertex(VertexId::new(0, 0, 0)), Some(c(0., 0.))); +} + +/// The existing point-before-edge choice, type switches, tolerance and disable switch survive. +#[test] +fn switches_tolerance_and_point_priority_remain_unchanged() { + let (mut s, id) = + fixture(Geometry::Line(vec![vec![c(0., 0.), c(0., 100.)], vec![c(100., 0.), c(100., 100.)], vec![c(80., 0.2), c(120., 0.2)]])); + let node = Some((id, 0, VertexId::new(0, 0, 0))); + s.map_mut().editing.snapping.vertex = true; + hit(snap_vertex_except(&mut s, c(99., 0.), node), c(100., 0.), "vertex"); + s.map_mut().editing.snapping.vertex = false; + hit(snap_vertex_except(&mut s, c(99., 0.), node), c(99., 0.2), "edge"); + s.map_mut().editing.snapping.edge = false; + s.map_mut().editing.snapping.end = true; + hit(snap_vertex_except(&mut s, c(98.01, 0.), node), c(100., 0.), "end"); + assert!(snap_vertex_except(&mut s, c(97.99, 0.), node).is_none()); + s.map_mut().editing.snapping.end = false; + assert!(snap_vertex_except(&mut s, c(99., 0.), node).is_none()); + s.map_mut().editing.snapping.enabled = false; + assert!(snap_vertex_except(&mut s, c(100., 0.), node).is_none()); +} + +/// No exclusion, feature exclusion and the existing Point/MultiPoint policy are unchanged. +/// Invalid source addresses are ignored and cannot mutate the project or add undo entries. +#[test] +fn legacy_policy_and_invalid_addresses_are_preserved() { + for geometry in [Geometry::Point(c(0., 0.)), Geometry::MultiPoint(vec![c(0., 0.)]), Geometry::Line(vec![vec![c(0., 0.), c(100., 0.)]])] + { + let (mut s, id) = fixture(geometry); + let expected = snap_except(&mut s, c(0., 0.), None); + let before = serde_json::to_value(&s.project).unwrap(); + let revision = s.revision; + for skip in [ + None, + Some((u32::MAX, 0, VertexId::new(0, 0, 0))), + Some((id, usize::MAX, VertexId::new(0, 0, 0))), + Some((id, 0, VertexId::new(usize::MAX, usize::MAX, usize::MAX))), + ] { + assert_eq!(snap_vertex_except(&mut s, c(0., 0.), skip), expected); + } + assert!(snap_except(&mut s, c(0., 0.), Some((id, 0))).is_none()); + assert_eq!(serde_json::to_value(&s.project).unwrap(), before); + assert_eq!(s.revision, revision); + assert_eq!(s.undo_depth(), 0); + } +} + +/// The addressed row keeps its place in the original layer/row tie order; hidden layers stay out. +#[test] +fn source_row_order_and_visibility_still_choose_the_first_tie() { + let (mut s, id) = fixture(Geometry::Line(vec![vec![c(100., 100.), c(-1., 0.)]])); + let layer = s.map_mut().layer_mut(id).unwrap(); + layer.dataset.features.push(Feature { id: 2, geometry: Some(Geometry::Line(vec![vec![c(200., 100.), c(1., 0.)]])), attrs: vec![] }); + s.map_mut().editing.snapping.vertex = true; + s.map_mut().editing.snapping.edge = false; + hit(snap_vertex_except(&mut s, c(0., 0.), Some((id, 1, VertexId::new(0, 0, 0)))), c(-1., 0.), "vertex"); + hit(snap_vertex_except(&mut s, c(0., 0.), Some((id, 0, VertexId::new(0, 0, 0)))), c(-1., 0.), "vertex"); + s.map_mut().layer_mut(id).unwrap().visible = false; + assert!(snap_vertex_except(&mut s, c(0., 0.), Some((id, 0, VertexId::new(0, 0, 0)))).is_none()); +} + +/// World and regional map projections keep source identities and inverse-projected targets. +#[test] +fn world_and_regional_projections_use_source_nodes() { + for code in ["equirectangular", "EPSG:4549"] { + let at = Coord::new(120.001, 31.); + let (mut s, id) = + fixture(Geometry::Line(vec![vec![Coord::new(120., 31.), Coord::new(120., 31.01)], vec![at, Coord::new(120.001, 31.01)]])); + s.map_mut().projection = Projection::from_code(code).unwrap(); + s.map_mut().camera.center = s.map().to_map(at).unwrap(); + s.map_mut().camera.res = 1.0; + s.map_mut().editing.snapping.vertex = true; + s.map_mut().editing.snapping.edge = false; + let node = Some((id, 0, VertexId::new(0, 0, 0))); + hit(snap_vertex_except(&mut s, at, node), at, "vertex"); + assert!(snap_vertex_except(&mut s, Coord::new(120., 31.), node).is_none()); + } +} + +/// A world-edge cut adds rendered parts but cannot renumber source exclusions, invent node +/// targets at cut intersections, or create an edge across the middle of the world. +#[test] +fn projection_cuts_do_not_supply_source_addresses_or_bridge_the_world() { + let (mut s, id) = fixture(Geometry::Line(vec![ + vec![Coord::new(80., -20.), Coord::new(100., -20.)], + vec![Coord::new(-100., 20.), Coord::new(-110., 20.), Coord::new(-110., 30.)], + ])); + s.map_mut().projection = Projection::from_code("mollweide +lon_0=-90").unwrap(); + s.map_mut().camera.res = 1000.; + s.map_mut().editing.snapping.vertex = true; + s.map_mut().editing.snapping.edge = false; + let node = Some((id, 0, VertexId::new(1, 0, 0))); + hit(snap_vertex_except(&mut s, Coord::new(-110., 20.), node), Coord::new(-110., 20.), "vertex"); + assert!(snap_vertex_except(&mut s, Coord::new(-100., 20.), node).is_none()); + assert!(snap_vertex_except(&mut s, Coord::new(90., -20.), node).is_none()); + let Geometry::Line(parts) = s.map().layer(id).unwrap().projected_cache()[0].as_ref().unwrap() else { panic!("line") }; + assert!(parts.len() > 2, "the fixture must actually split at the world edge"); + s.map_mut().editing.snapping.vertex = false; + s.map_mut().editing.snapping.edge = true; + assert!(snap_vertex_except(&mut s, Coord::new(-90., -20.), node).is_none()); + hit(snap_vertex_except(&mut s, Coord::new(89., -20.), node), Coord::new(89., -20.), "edge"); + // This remains an ordinary in-memory feature layer. + assert_eq!(s.map().layer(id).unwrap().geometry_type(), GeometryType::Polyline); +} diff --git a/crates/ui-egui/src/vertexedit.rs b/crates/ui-egui/src/vertexedit.rs index 737fd6059..1a8ace0f0 100644 --- a/crates/ui-egui/src/vertexedit.rs +++ b/crates/ui-egui/src/vertexedit.rs @@ -136,14 +136,18 @@ pub fn drag_start(app: &mut RustGisApp, rect: Rect, origin: Pos2) -> bool { true } -/// The dragged vertex follows the pointer, snapping when snapping is on. +/// A dragged line/polygon vertex follows the pointer, excluding itself and its incident edges. +/// Point/multipoint moves keep whole-feature exclusion. An insertion also excludes its whole +/// feature, so its source edge cannot become a target. pub fn drag_to(app: &mut RustGisApp, rect: Rect, p: Pos2) { let Some(c) = screen_to_lonlat(app, rect, p) else { return }; let Some(d) = app.map.vertex_drag else { return }; // Asked directly, not as a command: commands mark the map changed, redrawing it every frame. - // The feature being changed isn't a snap target: its old vertex and edges would hold the - // vertex in place. - let snap = rustgis_engine::cmd::design::snap_except(&mut app.session, c, Some((d.layer, d.row))); + let snap = if d.insert { + rustgis_engine::cmd::design::snap_except(&mut app.session, c, Some((d.layer, d.row))) + } else { + rustgis_engine::cmd::design::snap_vertex_except(&mut app.session, c, Some((d.layer, d.row, d.vertex))) + }; if let Some(d) = &mut app.map.vertex_drag { d.at = snap.map_or(c, |(at, _)| at); d.snapped = snap.is_some(); diff --git a/crates/ui-egui/tests/vertex_snapping.rs b/crates/ui-egui/tests/vertex_snapping.rs new file mode 100644 index 000000000..a7736d03c --- /dev/null +++ b/crates/ui-egui/tests/vertex_snapping.rs @@ -0,0 +1,165 @@ +//! Real pointer gestures for source-addressed snapping in the application. +use egui::{Event, Modifiers, PointerButton, Pos2}; +use egui_kittest::Harness; +use rustgis_engine::{ + Project, Session, + data::{Dataset, Feature, Geometry, VertexId}, + geom::{Coord, merc_to_lonlat}, + map::design::Snapping, +}; +use rustgis_ui_egui::RustGisApp; +use serde_json::json; + +/// Stores a small metre-based fixture in the same longitude/latitude format as imported data. +fn c(x: f64, y: f64) -> Coord { + merc_to_lonlat(Coord::new(x, y)) +} + +/// A selected multipart line exposes both an ordinary vertex and an insertion midpoint. +fn harness(bearing: f64) -> Harness<'static, RustGisApp> { + let mut session = Session::new(Project::new("Vertex snapping")); + let geometry = Geometry::Line(vec![vec![c(0., 0.), c(0., 160.)], vec![c(180., 0.), c(180., 160.)]]); + let mut data = Dataset::new("Editable paths", geometry.kind(), vec![]); + data.features = vec![Feature { id: 1, geometry: Some(geometry), attrs: vec![] }].into(); + let id = session.map_mut().add_dataset(data); + session.map_mut().layer_mut(id).unwrap().selection.push(0); + session.map_mut().camera.center = Coord::new(90., 80.); + session.map_mut().camera.res = 1.; + session.map_mut().camera.set_bearing(bearing); + session.map_mut().editing.snapping = Snapping { enabled: true, tolerance_px: 8., vertex: true, edge: false, end: false }; + let mut app = RustGisApp::new(session); + app.run("tool.set", json!({"tool":"editVertices"})).unwrap(); + app.ui.tab = "Edit".into(); + app.session.clear_history(); + let mut h = Harness::builder().with_accessibility_check(false).with_size(egui::vec2(1480., 900.)).with_max_steps(64).build_ui_state( + |ui, app: &mut RustGisApp| { + let ctx = ui.ctx().clone(); + app.logic(&ctx); + app.ui(ui); + }, + app, + ); + for _ in 0..6 { + h.step(); + } + h +} + +/// Converts fixture coordinates using the actual map rectangle and rotated camera. +fn screen(h: &Harness<'_, RustGisApp>, at: Coord) -> Pos2 { + let rect = h.state().map.view_rect.unwrap().0; + let map = h.state().session.map(); + let (x, y) = map.camera.to_screen(map.to_map(at).unwrap(), rect.width() as f64, rect.height() as f64); + rect.min + egui::vec2(x as f32, y as f32) +} + +/// A held drag follows the production threshold and event path instead of calling tool helpers. +fn drag(h: &mut Harness<'_, RustGisApp>, from: Pos2, to: Pos2) { + h.event(Event::PointerMoved(from)); + h.event(Event::PointerButton { pos: from, button: PointerButton::Primary, pressed: true, modifiers: Modifiers::NONE }); + h.step(); + for i in 1..=8 { + h.event(Event::PointerMoved(from + (to - from) * (i as f32 / 8.))); + h.step(); + } +} + +/// Releases the held primary pointer and lets the edit command finish. +fn release(h: &mut Harness<'_, RustGisApp>, at: Pos2) { + h.event(Event::PointerButton { pos: at, button: PointerButton::Primary, pressed: false, modifiers: Modifiers::NONE }); + for _ in 0..3 { + h.step(); + } +} + +/// Reads the first source node; previews must never replace this stored geometry. +fn first(h: &Harness<'_, RustGisApp>) -> Coord { + h.state().session.map().layers[0].dataset.features[0].geometry.as_ref().unwrap().vertex(VertexId::new(0, 0, 0)).unwrap() +} + +/// Rotation does not change snap tolerance; a preview is read-only and release is one undo step. +#[test] +fn rotated_drag_snaps_to_another_part_and_undoes_once() { + let mut h = harness(27.); + let camera = h.state().session.map().camera; + let from = screen(&h, c(0., 0.)); + let to = screen(&h, c(180., 0.)) + egui::vec2(3., 2.); + drag(&mut h, from, to); + let preview = h.state().map.vertex_drag.expect("vertex drag"); + assert!(preview.snapped && !preview.insert); + assert!((preview.at.x - c(180., 0.).x).abs() < 1e-9); + assert_eq!(first(&h), c(0., 0.)); + assert_eq!(h.state().session.undo_depth(), 0); + release(&mut h, to); + assert!((first(&h).x - c(180., 0.).x).abs() < 1e-9); + assert_eq!(h.state().session.undo_depth(), 1); + assert_eq!(h.state().session.map().camera, camera); + h.state_mut().run("edit.undo", json!(null)).unwrap(); + assert_eq!(first(&h), c(0., 0.)); + h.state_mut().run("edit.redo", json!(null)).unwrap(); + assert!((first(&h).x - c(180., 0.).x).abs() < 1e-9); +} + +/// Escape cancels a snapped preview without geometry, camera or undo changes. +#[test] +fn escape_cancels_the_snapped_preview() { + let mut h = harness(0.); + let camera = h.state().session.map().camera; + let from = screen(&h, c(0., 0.)); + let to = screen(&h, c(180., 0.)) + egui::vec2(3., 2.); + drag(&mut h, from, to); + assert!(h.state().map.vertex_drag.unwrap().snapped); + h.key_press(egui::Key::Escape); + h.step(); + release(&mut h, to); + assert!(h.state().map.vertex_drag.is_none()); + assert_eq!(first(&h), c(0., 0.)); + assert_eq!(h.state().session.undo_depth(), 0); + assert_eq!(h.state().session.map().camera, camera); +} + +/// Midpoint insertion retains whole-feature exclusion, so it cannot snap back onto itself. +#[test] +fn midpoint_insertion_keeps_whole_feature_exclusion() { + let mut h = harness(0.); + let from = screen(&h, c(0., 80.)); + let to = screen(&h, c(180., 0.)) + egui::vec2(3., 2.); + drag(&mut h, from, to); + let preview = h.state().map.vertex_drag.unwrap(); + assert!(preview.insert && !preview.snapped); + release(&mut h, to); + let geometry = h.state().session.map().layers[0].dataset.features[0].geometry.as_ref().unwrap(); + assert_eq!(geometry.vertex(VertexId::new(0, 0, 1)), Some(preview.at)); + assert_eq!(geometry.vertex(VertexId::new(0, 0, 2)), Some(c(0., 160.))); + assert_eq!(h.state().session.undo_depth(), 1); +} + +/// Disabled snapping and a pointer beyond tolerance both leave the preview at the pointer. +#[test] +fn disabled_and_outside_tolerance_drags_remain_free() { + for enabled in [false, true] { + let mut h = harness(0.); + h.state_mut().session.map_mut().editing.snapping.enabled = enabled; + let from = screen(&h, c(0., 0.)); + let to = screen(&h, c(180., 0.)) + egui::vec2(if enabled { 12. } else { 3. }, 2.); + drag(&mut h, from, to); + assert!(!h.state().map.vertex_drag.unwrap().snapped); + release(&mut h, to); + assert_eq!(h.state().session.undo_depth(), 1); + assert!((first(&h).x - c(180., 0.).x).abs() > 1e-6); + } +} + +/// Turning editing off prevents this gesture from starting a node edit or creating undo state. +#[test] +fn read_only_map_does_not_start_a_vertex_edit() { + let mut h = harness(0.); + h.state_mut().session.map_mut().editing.settings.enabled = false; + let from = screen(&h, c(0., 0.)); + let to = screen(&h, c(180., 0.)); + drag(&mut h, from, to); + assert!(h.state().map.vertex_drag.is_none()); + release(&mut h, to); + assert_eq!(first(&h), c(0., 0.)); + assert_eq!(h.state().session.undo_depth(), 0); +} From 34f056fb1eb2d372769b74eefbaed92e2edabcc7 Mon Sep 17 00:00:00 2001 From: yongyin-leon <269641072+yongyin-leon@users.noreply.github.com> Date: Sun, 25 Oct 2026 07:06:59 +0800 Subject: [PATCH 2/2] test(engine): skip directories in command parameter audit --- crates/engine/src/tests.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index c7a0fc8cc..ec05d7c2f 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -11050,6 +11050,7 @@ fn coverage_answers_that_are_not_geotiffs_are_errors() { /// The keys a command's source reads from its parameters, found by scanning `src/cmd/*.rs`: the /// command's function (or closure) and the functions it passes its parameters to (in its own /// file, or another's as `module::name(…)`), a few calls deep. +/// Scans top-level Rust modules; helper subdirectories are not source files or command entries. fn keys_read_by_commands() -> Vec<(String, std::collections::BTreeSet)> { use std::collections::{BTreeMap, BTreeSet}; type Fn = (Vec, String); @@ -11058,6 +11059,9 @@ fn keys_read_by_commands() -> Vec<(String, std::collections::BTreeSet)> let mut modules: BTreeMap)> = BTreeMap::new(); for entry in std::fs::read_dir(dir).unwrap() { let path = entry.unwrap().path(); + if !path.is_file() || path.extension().is_none_or(|ext| ext != "rs") { + continue; + } let stem = path.file_stem().unwrap().to_string_lossy().to_string(); let src = std::fs::read_to_string(&path).unwrap(); let mut fns: BTreeMap = BTreeMap::new(); @@ -11217,6 +11221,7 @@ fn literal_keys(body: &str, prefix: &str) -> Vec { .collect() } +/// Checks every discovered command's declared keys, even when helper directories accompany modules. #[test] fn params_name_every_key_a_command_reads() { // Read by a shared helper but of no use to the command.