From db2e79e2a4157d8f37d06eeee76d7bc49b4a0097 Mon Sep 17 00:00:00 2001 From: yongyin-leon <269641072+yongyin-leon@users.noreply.github.com> Date: Mon, 26 Oct 2026 15:46:15 +0800 Subject: [PATCH 1/3] feat(edit): move coincident polygon vertices together --- ROADMAP.md | 4 +- crates/engine/src/cmd/design.rs | 6 + crates/engine/src/cmd/design/shared_vertex.rs | 73 ++++++ crates/engine/src/tests.rs | 3 + crates/engine/tests/move_shared_vertex.rs | 241 ++++++++++++++++++ crates/ui-egui/src/chrome.rs | 1 + crates/ui-egui/src/lib.rs | 13 + crates/ui-egui/src/mapview.rs | 3 + crates/ui-egui/src/ribbon.rs | 13 + crates/ui-egui/src/vertexedit.rs | 223 +++++++++++++++- crates/ui-egui/tests/move_shared_vertex.rs | 230 +++++++++++++++++ docs/user-guide/editing.md | 30 +++ 12 files changed, 832 insertions(+), 8 deletions(-) create mode 100644 crates/engine/src/cmd/design/shared_vertex.rs create mode 100644 crates/engine/tests/move_shared_vertex.rs create mode 100644 crates/ui-egui/tests/move_shared_vertex.rs diff --git a/ROADMAP.md b/ROADMAP.md index 429fb9cdd..9d0005f0d 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -102,7 +102,9 @@ And alongside it: - Export into an existing GeoPackage as an added or replaced table, leaving its other tables ([#852](https://github.com/opengeos/RustGIS/issues/852); export refuses an existing file today). - Polygon editing: cut a polygon, reshape and trace _(open)_; then shared edges, where moving one - boundary moves its neighbour's too. + boundary moves its neighbour's too. Coincident polygon vertices can already move together + with Shared Vertices (`edit.moveSharedVertex`); different vertex densities and linked + insertion/deletion remain open. - Editor tracking fields (created/edited by and at). - Attribute table columns: rename, hide and reorder _(open)_. - Persistent attribute joins and virtual fields. _Coordinate first_ (they change `Feature`). diff --git a/crates/engine/src/cmd/design.rs b/crates/engine/src/cmd/design.rs index 4caf16dfd..0f26bfb9e 100644 --- a/crates/engine/src/cmd/design.rs +++ b/crates/engine/src/cmd/design.rs @@ -12,6 +12,7 @@ use serde_json::{Value as Json, json}; use super::{b, f, need_feature_layer, ps}; use crate::{CmdResult, CommandSpec as C, Session}; +mod shared_vertex; mod snap_vertex; /// One edited feature: its state before (none for an insert) and its row after (none for a @@ -208,6 +209,8 @@ pub(crate) fn apply(s: &mut Session, id: u32, changes: Vec) -> Result<() Ok(()) } +/// Registers data-design and editing commands, including opt-in exact-coordinate polygon moves. +/// Feature edits share `apply` for rules, archives and counts inside the session transaction. pub fn commands() -> Vec { vec![ // Data design. @@ -361,6 +364,9 @@ pub fn commands() -> Vec { r#"{"layer"?, "row": number, "action": "move"|"insert"|"delete", "part"?: number, "ring"?: number, "index": number (insert: the vertex the new one follows), "lon"?, "lat"?}"#, ), C::new("edit.merge", "Merge", "Edit › Features › Modify", merge).when(need_editable_selection), + C::new("edit.moveSharedVertex", "Move Shared Vertex", "Edit › Features › Modify", shared_vertex::run).when(need_editable).params( + r#"{"layer"?, "row": number, "part"?: number, "ring"?: number, "index": number, "lon": degrees, "lat": degrees}"#, + ), C::new("edit.explode", "Explode", "Edit › Features › Modify", explode).when(need_editable_selection), C::new("edit.split", "Split", "Edit › Features › Modify", split).when(need_editable_selection).params(r#"{"lon", "lat"}"#), C::new("edit.traverse", "Traverse", "Edit › Features › Modify", traverse).when(need_editable).params( diff --git a/crates/engine/src/cmd/design/shared_vertex.rs b/crates/engine/src/cmd/design/shared_vertex.rs new file mode 100644 index 000000000..cc7ef6d56 --- /dev/null +++ b/crates/engine/src/cmd/design/shared_vertex.rs @@ -0,0 +1,73 @@ +//! Moves every polygon vertex stored at the source coordinate, within one layer. +//! Exact coordinate equality includes point-only contacts; no edge remapping or tolerance. + +use rustgis_data::{Geometry, VertexId}; +use rustgis_geom::Coord; +use serde_json::{Value, json}; + +use super::{Change, apply}; +use crate::{CmdResult, Session}; + +/// Moves coincident polygon nodes through one design-rule batch and the caller's transaction. +/// Source addresses match `edit.vertex`; closed rings follow `Geometry::move_vertex`. +/// Same-position calls still run design rules and retain ordinary command history behaviour. +pub(super) fn run(s: &mut Session, p: &Value) -> CmdResult { + let id = s.layer_id(p)?; + let row = index(p, "row", None)?; + let vertex = VertexId::new(index(p, "part", Some(0))?, index(p, "ring", Some(0))?, index(p, "index", None)?); + let lon = super::f(p, "lon").ok_or("lon is required")?; + let lat = super::f(p, "lat").ok_or("lat is required")?; + if !(lon.is_finite() && lat.is_finite()) || lon.abs() > 360.0 || lat.abs() > 90.0 { + return Err("lon and lat are degrees".into()); + } + let at = Coord::new(lon, lat); + let layer = s.map().layer(id).ok_or("no such layer")?; + let source = layer.dataset.features.get(row).ok_or("no such row")?.geometry.as_ref().ok_or("the feature has no shape")?; + if !matches!(source, Geometry::Polygon(_)) { + return Err("shared vertices can only be moved in polygons".into()); + } + let original = source.vertex(vertex).ok_or("no such vertex")?; + // Retaining shared rows adds no table copy and lets the result include calculation changes. + let before = layer.dataset.features.clone(); + let mut replacements = Vec::new(); + for (row, feature) in before.iter().enumerate() { + let Some(geometry @ Geometry::Polygon(_)) = &feature.geometry else { continue }; + let mut nodes = Vec::new(); + geometry.for_each_vertex(|v, c| { + if c == original { + nodes.push(v); + } + }); + if nodes.is_empty() { + continue; + } + let mut moved = geometry.clone(); + for node in nodes { + moved = moved.move_vertex(node, at).ok_or("no such vertex")?; + } + super::super::edit::validate(&moved)?; + replacements.push((row, moved)); + } + let layer = s.map_mut().layer_mut(id).ok_or("no such layer")?; + let mut changes = Vec::new(); + for (row, geometry) in replacements { + let feature = layer.dataset.features.get_mut(row).ok_or("no such row")?; + changes.push(Change { before: Some(feature.clone()), row: Some(row) }); + feature.geometry = Some(geometry); + } + layer.touch_geometry(); + let rows: Vec = changes.iter().filter_map(|c| c.row).collect(); + apply(s, id, changes)?; + let layer = s.map().layer(id).ok_or("no such layer")?; + let modified = rows.iter().filter(|&&row| before.get(row) != layer.dataset.features.get(row)).count(); + Ok(json!({"part": vertex.part, "ring": vertex.ring, "index": vertex.index, "modified": modified})) +} + +/// Reads a platform-sized address; optional null parts/rings use the same defaults as `edit.vertex`. +fn index(p: &Value, key: &str, default: Option) -> Result { + match p.get(key) { + None | Some(Value::Null) if default.is_some() => default.ok_or_else(|| format!("{key} is required")), + Some(v) => v.as_u64().and_then(|n| usize::try_from(n).ok()).ok_or_else(|| format!("{key} must be a whole number")), + None => Err(format!("{key} is required")), + } +} diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index def24f2b7..2ccd63e30 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1635,6 +1635,9 @@ fn hostile_params_never_panic() { let junk = [ Value::Null, json!({}), + // Shared vertex moves: valid row/destination, hostile node addresses and coordinate types. + json!({"layer":"Countries","row":0,"part":u64::MAX,"ring":-1,"index":[],"lon":12,"lat":45}), + json!({"layer":"Countries","row":0,"part":0,"ring":0,"index":0,"lon":{},"lat":1e300}), json!({"layer": 99999, "path": "/nonexistent/x.geojson", "name": "", "where": "((", "tool": "buffer", "params": 7}), json!({"layer": "Countries", "row": 1e30, "field": "nope", "value": [1], "scale": -1, "lon": 1e300, "lat": "x", "classes": 1e9, "type": "graduated"}), json!({"layer": "Cities", "rows": [1, 99999999], "geometry": {"type": "Polygon", "coordinates": [[]]}, "width": 1e9, "height": -3, "expression": "'"}), diff --git a/crates/engine/tests/move_shared_vertex.rs b/crates/engine/tests/move_shared_vertex.rs new file mode 100644 index 000000000..6b421d3bb --- /dev/null +++ b/crates/engine/tests/move_shared_vertex.rs @@ -0,0 +1,241 @@ +//! Exact-coordinate polygon moves use the ordinary design-rule and undo transaction. +use rustgis_engine::{ + Project, Session, + data::{Dataset, Feature, Field, FieldType, Geometry, GeometryType, Value as Cell, VertexId}, + geom::Coord, + map::design::{AttributeRule, RuleKind}, +}; +use serde_json::{Value, json}; + +/// Builds an explicitly closed rectangular polygon without import normalization. +fn rectangle(left: f64, bottom: f64, right: f64, top: f64) -> Geometry { + Geometry::Polygon(vec![vec![vec![ + Coord::new(left, bottom), + Coord::new(right, bottom), + Coord::new(right, top), + Coord::new(left, top), + Coord::new(left, bottom), + ]]]) +} + +/// Four parcels meet at zero, including two pairs with only a point in common. +fn fixture() -> (Session, u32) { + let mut s = Session::new(Project::new("Shared vertex moves")); + let id = add_layer( + &mut s, + "Parcels", + vec![rectangle(0., 0., 1., 1.), rectangle(-1., 0., 0., 1.), rectangle(-1., -1., 0., 0.), rectangle(0., -1., 1., 0.)], + ); + s.active_layer = Some(id); + s.clear_history(); + (s, id) +} + +/// Adds source geometries with non-row feature IDs and a field usable by calculation/constraint rules. +fn add_layer(s: &mut Session, name: &str, geometries: Vec) -> u32 { + let mut data = Dataset::new(name, GeometryType::Polygon, vec![Field::new("n", FieldType::Integer)]); + data.features = geometries + .into_iter() + .enumerate() + .map(|(i, geometry)| Feature { id: 40 + i as u64, geometry: Some(geometry), attrs: vec![Cell::Int(i as i64)] }) + .collect::>() + .into(); + s.map_mut().add_dataset(data) +} + +/// Reads full feature values so tests catch accidental attribute, ID or non-target changes. +fn features(s: &Session, id: u32) -> Vec { + s.map().layer(id).unwrap().dataset.features.iter().cloned().collect() +} + +/// Runs the shared command at the fixture's first node; row is a position, not feature ID 40. +fn move_to(s: &mut Session, id: u32, at: Coord) -> Value { + s.run("edit.moveSharedVertex", &json!({"layer":id,"row":0,"index":0,"lon":at.x,"lat":at.y})).unwrap() +} + +/// Point-only neighbours and filtered, unselected rows move in one batch, with complete undo/redo. +#[test] +fn all_four_corners_move_without_selection_or_visibility_limits() { + let (mut s, id) = fixture(); + let layer = s.map_mut().layer_mut(id).unwrap(); + layer.visible = false; + layer.definition_query = Some("n = 0".into()); + layer.selection = vec![0]; + layer.design.archive = Some(Vec::new()); + let before = features(&s, id); + let result = move_to(&mut s, id, Coord::new(0.2, 0.3)); + assert_eq!(result, json!({"part":0,"ring":0,"index":0,"modified":4})); + let after = features(&s, id); + for (old, new) in before.iter().zip(&after) { + let mut expected = old.geometry.clone().unwrap(); + for (v, c) in old.geometry.as_ref().unwrap().vertices() { + if c == Coord::new(0., 0.) { + expected = expected.move_vertex(v, Coord::new(0.2, 0.3)).unwrap(); + } + } + assert_eq!(new.geometry.as_ref(), Some(&expected)); + assert_eq!((new.id, &new.attrs), (old.id, &old.attrs)); + } + assert_eq!(s.map().layer(id).unwrap().design.archive.as_ref().unwrap().len(), 4); + assert_eq!(s.edit_counts[&id].modified, 4); + assert_eq!(s.undo_depth(), 1); + s.run("edit.undo", &Value::Null).unwrap(); + assert_eq!(features(&s, id), before); + assert!(s.map().layer(id).unwrap().design.archive.as_ref().unwrap().is_empty()); + assert_eq!(s.edit_counts[&id].modified, 4, "undo retains cumulative edit counts"); + s.run("edit.redo", &Value::Null).unwrap(); + assert_eq!(features(&s, id), after); + assert_eq!(s.map().layer(id).unwrap().design.archive.as_ref().unwrap().len(), 4); +} + +/// All original addresses in a feature move, including holes, other parts and repeated coordinates. +#[test] +fn multipart_holes_and_repeated_nodes_follow_the_original_coordinate() { + let mut s = Session::new(Project::new("Multipart nodes")); + let c = Coord::new(1., 1.); + let mut geometry = rectangle(0., 0., 4., 4.); + if let Geometry::Polygon(parts) = &mut geometry { + parts[0].push(vec![c, Coord::new(2., 1.), Coord::new(2., 2.), c]); + parts.push(vec![vec![c, Coord::new(3., 1.), c, Coord::new(3., 3.), c]]); + } + let id = add_layer(&mut s, "Parts", vec![geometry.clone()]); + let at = Coord::new(1.1, 1.2); + let result = s.run("edit.moveSharedVertex", &json!({"layer":id,"row":0,"part":0,"ring":1,"index":0,"lon":at.x,"lat":at.y})).unwrap(); + assert_eq!(result, json!({"part":0,"ring":1,"index":0,"modified":1})); + let moved = features(&s, id)[0].geometry.clone().unwrap(); + for (v, old) in geometry.vertices() { + assert_eq!(moved.vertex(v), Some(if old == c { at } else { old })); + } + if let Geometry::Polygon(parts) = &moved { + for rings in parts { + for ring in rings { + assert_eq!(ring.first(), ring.last()); + } + } + } +} + +/// Exact stored equality never captures nearby coordinates, non-polygons, or another layer. +#[test] +fn near_vertices_and_other_layers_remain_unchanged() { + let (mut s, id) = fixture(); + let near = rectangle(f64::EPSILON, 0., 2., 2.); + s.map_mut().layer_mut(id).unwrap().dataset.features.push(Feature { id: 99, geometry: Some(near.clone()), attrs: vec![Cell::Int(9)] }); + let other = add_layer(&mut s, "Other", vec![rectangle(0., 0., 1., 1.)]); + let untouched = features(&s, other); + assert_eq!(move_to(&mut s, id, Coord::new(0.2, 0.3))["modified"], 4); + assert_eq!(features(&s, id)[4].geometry.as_ref(), Some(&near)); + assert_eq!(features(&s, other), untouched); +} + +/// Different segmentation is accepted: intermediate nodes are neither inserted nor remapped. +#[test] +fn differing_density_moves_existing_nodes_and_keeps_the_documented_limitation() { + let mut s = Session::new(Project::new("Unequal density")); + let a = rectangle(0., 0., 2., 2.); + let b = Geometry::Polygon(vec![vec![vec![ + Coord::new(2., 0.), + Coord::new(1., 0.), + Coord::new(0., 0.), + Coord::new(0., -2.), + Coord::new(2., -2.), + Coord::new(2., 0.), + ]]]); + let id = add_layer(&mut s, "Parcels", vec![a, b]); + let result = s.run("edit.moveSharedVertex", &json!({"layer":id,"row":0,"index":1,"lon":2,"lat":0.5})).unwrap(); + assert_eq!(result["modified"], 2); + let after = features(&s, id); + assert_eq!(after[0].geometry.as_ref().unwrap().vertex(VertexId::new(0, 0, 1)), Some(Coord::new(2., 0.5))); + assert_eq!(after[1].geometry.as_ref().unwrap().vertex(VertexId::new(0, 0, 0)), Some(Coord::new(2., 0.5))); + assert_eq!(after[1].geometry.as_ref().unwrap().vertex(VertexId::new(0, 0, 1)), Some(Coord::new(1., 0.))); + assert_eq!(after[1].geometry.as_ref().unwrap().vertices().len(), 5); +} + +/// A later row's constraint rejection restores every member, archives, counts, dirty and redo. +#[test] +fn later_rule_rejection_rolls_back_the_whole_batch() { + let (mut s, id) = fixture(); + let layer = s.map_mut().layer_mut(id).unwrap(); + layer.design.archive = Some(Vec::new()); + layer.design.rules.push(AttributeRule { + name: "reject later parcel".into(), + kind: RuleKind::Constraint, + field: String::new(), + expression: "n < 2".into(), + }); + s.run("layer.rename", &json!({"layer":id,"name":"Renamed"})).unwrap(); + s.run("edit.undo", &Value::Null).unwrap(); + let before = serde_json::to_value(&s.project).unwrap(); + let counts = s.edit_counts.clone(); + let dirty = s.dirty; + assert!(s.can_redo()); + let error = s.run("edit.moveSharedVertex", &json!({"layer":id,"row":0,"index":0,"lon":0.2,"lat":0.3})).unwrap_err(); + assert!(error.contains("reject later parcel")); + assert_eq!(serde_json::to_value(&s.project).unwrap(), before); + assert_eq!(s.edit_counts, counts); + assert_eq!(s.dirty, dirty); + assert_eq!(s.undo_depth(), 0); + assert!(s.can_redo()); +} + +/// Same-position moves retain ordinary rule/count/history semantics while reporting actual changes. +#[test] +fn same_position_runs_calculations_and_keeps_existing_undo_behaviour() { + let (mut s, id) = fixture(); + assert_eq!(move_to(&mut s, id, Coord::new(0., 0.))["modified"], 0); + assert_eq!(s.edit_counts[&id].modified, 4); + assert_eq!(s.undo_depth(), 1); + assert!(s.dirty); + s.map_mut().layer_mut(id).unwrap().design.rules.push(AttributeRule { + name: "increment".into(), + kind: RuleKind::Calculation, + field: "n".into(), + expression: "n + 1".into(), + }); + assert_eq!(move_to(&mut s, id, Coord::new(0., 0.))["modified"], 4); + assert_eq!(s.edit_counts[&id].modified, 8); + assert_eq!(s.undo_depth(), 2); + assert_eq!(features(&s, id)[3].attrs[0], Cell::Int(4)); +} + +/// Hostile addresses and destinations fail without changing data or history, including closing aliases. +#[test] +fn malformed_inputs_and_wrong_geometry_are_rejected_without_mutation() { + let (mut s, id) = fixture(); + for key in ["row", "part", "ring", "index", "lon", "lat"] { + for bad in [Value::Null, json!(-1), json!(1.5), json!("junk"), json!([]), json!(u64::MAX)] { + let mut p = json!({"layer":id,"row":0,"part":0,"ring":0,"index":0,"lon":0.2,"lat":0.3}); + p[key] = bad; + if (key == "part" || key == "ring") && p[key].is_null() { + continue; + } + if (key == "lon" || key == "lat") && p[key].as_f64().is_some_and(|v| v.abs() <= 90.) { + continue; + } + let before = features(&s, id); + assert!(s.run("edit.moveSharedVertex", &p).is_err(), "{p}"); + assert_eq!(features(&s, id), before); + assert_eq!(s.undo_depth(), 0); + } + } + for index in [4, 999] { + assert!(s.run("edit.moveSharedVertex", &json!({"layer":id,"row":0,"index":index,"lon":0.2,"lat":0.3})).is_err()); + } + s.map_mut().layer_mut(id).unwrap().dataset.features[0].geometry = Some(Geometry::Point(Coord::new(0., 0.))); + assert!(s.run("edit.moveSharedVertex", &json!({"layer":id,"row":0,"index":0,"lon":0.2,"lat":0.3})).is_err()); +} + +/// Existing streamed-edit and editing-disabled guards reject the command before any row changes. +#[test] +fn streamed_and_disabled_editing_use_existing_guards() { + let (mut s, id) = fixture(); + s.map_mut().editing.settings.enabled = false; + let before = features(&s, id); + assert!(s.run("edit.moveSharedVertex", &json!({"layer":id,"row":0,"index":0,"lon":1,"lat":1})).is_err()); + assert_eq!(features(&s, id), before); + s.map_mut().editing.settings.enabled = true; + let stream = s.map_mut().add_streamed("Stream", Default::default()); + let error = s.run("edit.moveSharedVertex", &json!({"layer":stream,"row":0,"index":0,"lon":1,"lat":1})).unwrap_err(); + assert!(error.contains("Load All Features"), "{error}"); + assert_eq!(s.undo_depth(), 0); +} diff --git a/crates/ui-egui/src/chrome.rs b/crates/ui-egui/src/chrome.rs index be870a3bf..9afb34c54 100644 --- a/crates/ui-egui/src/chrome.rs +++ b/crates/ui-egui/src/chrome.rs @@ -645,6 +645,7 @@ pub(crate) const UI_COMMANDS: &[(&str, &str, &str)] = &[ ("ui.map.gpu", "Toggle GPU Rendering", "View"), ("ui.importShapefile", "Import Shapefile with Projection…", "Home › Add"), ("ui.measurementUnits", "Toggle Imperial Measurements", "Home › Inspect › Measure"), + ("ui.moveSharedVertices", "Shared Vertices", "Edit › Features"), ("ui.exportMap", "Export Map…", "Publish › Output"), ("ui.checkUpdate", "Check for Updates…", "Help › Help"), ]; diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 16c664263..aa0b66c4b 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -1781,6 +1781,7 @@ impl RustGisApp { } } + /// Dispatches UI-only commands; transient tool switches do not dirty or persist the project. fn run_ui(&mut self, id: &str, p: &Value) -> Result { // A pane's command toggles it; `{"close": true}` closes it, in view or not (a tab's ✕), // whatever else the command is given (SQL to run, a question to ask). @@ -1984,6 +1985,18 @@ impl RustGisApp { save("measurement_units", if imperial { "imperial" } else { "metric" }); } } + "ui.moveSharedVertices" => { + let on = match p.get("on") { + None => !self.map.move_shared_vertices, + Some(Value::Bool(on)) => *on, + _ => return Err(tl!("Shared vertices must be on or off").into()), + }; + if on != self.map.move_shared_vertices { + self.map.cancel_vertex_drag(); + self.map.move_shared_vertices = on; + } + return Ok(serde_json::json!({"on": on})); + } "ui.map.gpu" => { self.map.gpu_enabled = p.get("on").and_then(Value::as_bool).unwrap_or(!self.map.gpu_enabled); // `"remember": false` turns it on or off for this visit only (a `?gpu` link). diff --git a/crates/ui-egui/src/mapview.rs b/crates/ui-egui/src/mapview.rs index 63c19f28c..5d5b4615a 100644 --- a/crates/ui-egui/src/mapview.rs +++ b/crates/ui-egui/src/mapview.rs @@ -147,6 +147,8 @@ pub struct MapView { pub vertex_drag: Option, /// Non-owning source identity for the held vertex gesture; never persisted with the project. pub(crate) vertex_drag_owner: Option, + /// Session-only opt-in for coincident polygon moves; never saved with a project or preference. + pub move_shared_vertices: bool, /// The magnifying lens follows the pointer. pub magnifier: bool, pub popup: Option, @@ -243,6 +245,7 @@ impl Default for MapView { vertex: None, vertex_drag: None, vertex_drag_owner: None, + move_shared_vertices: false, magnifier: false, popup: None, popup_rev: 0, diff --git a/crates/ui-egui/src/ribbon.rs b/crates/ui-egui/src/ribbon.rs index ba299d008..328c1d7fa 100644 --- a/crates/ui-egui/src/ribbon.rs +++ b/crates/ui-egui/src/ribbon.rs @@ -2334,6 +2334,7 @@ fn active_layer(app: &RustGisApp) -> Option<&rustgis_engine::map::Layer> { app.session.active_layer.and_then(|id| app.session.map().layer(id)) } +/// Keeps edit commands and transient vertex modes together, with deeper settings in Editing. fn edit_tab(app: &mut RustGisApp, ui: &mut Ui) { let editing = app.session.map().editing.clone(); let pane = app.ui.right == Some(RightPane::Editing); @@ -2428,6 +2429,18 @@ fn edit_tab(app: &mut RustGisApp, ui: &mut Ui) { pane, false, ); + col(ui, |ui| { + small( + ui, + app, + "vertices", + "Shared Vertices", + "Move polygon vertices stored at the same coordinate in this layer, including point-only contacts.", + "ui.moveSharedVertices", + Value::Null, + app.map.move_shared_vertices, + ); + }); }); group(ui, "Tools", |ui| { big_menu(ui, app, "tools", "Editor\nTools", "Every editing tool.", |ui, app| { diff --git a/crates/ui-egui/src/vertexedit.rs b/crates/ui-egui/src/vertexedit.rs index 90b53e8dd..f0f5c0777 100644 --- a/crates/ui-egui/src/vertexedit.rs +++ b/crates/ui-egui/src/vertexedit.rs @@ -1,7 +1,7 @@ //! The Edit Vertices map tool: the selected features' vertices drawn as handles, with a midpoint //! handle on each edge. Drag a vertex to move it, drag a midpoint (or click an edge) to insert -//! one, click a vertex and press Delete to remove it. Every change is one `edit.vertex` command, -//! so undo, attribute rules and the edit counts work as for any other edit. +//! one, click a vertex and press Delete to remove it. Shared Vertices opts polygon moves into +//! `edit.moveSharedVertex`; other edits use `edit.vertex`. Each is one command for undo and rules. use egui::{Color32, Painter, Pos2, Rect, Stroke, vec2}; use rustgis_engine::data::{Feature, Geometry, VertexId}; @@ -43,6 +43,9 @@ pub(crate) struct DragOwner { map: usize, map_name: String, rows: std::sync::Weak>, + shared: bool, + /// Bounded geographic edges captured once, discarded with this gesture's row identity. + preview: Option, } impl DragOwner { @@ -56,16 +59,20 @@ impl DragOwner { map: app.session.project.active_map, map_name: map.name.clone(), rows: layer.dataset.features.allocation_key(), + shared: app.map.move_shared_vertices, + preview: shared_origin(app, d) + .map(|original| SharedPreview { original, edges: shared_edges(layer.dataset.features.iter(), original) }), }) } /// A gesture is usable only in its original context while its source is still an editable - /// target. Any row edit in that layer cancels conservatively; styles and camera changes do not. + /// target. Row edits or a changed shared mode cancel; styles and camera changes do not. fn matches(&self, app: &RustGisApp, d: VertexDrag) -> bool { let map = app.session.map(); self.project == app.session.opened && self.map == app.session.project.active_map && self.map_name == map.name + && self.shared == app.map.move_shared_vertices && app.session.tool == rustgis_engine::Tool::EditVertices && map.layer(d.layer).is_some_and(|l| self.rows.ptr_eq(&l.dataset.features.allocation_key())) && targets(app).iter().any(|(l, r, g)| *l == d.layer && *r == d.row && g.vertex(d.vertex).is_some()) @@ -225,15 +232,32 @@ pub fn drag_stop(app: &mut RustGisApp) { } } -/// Sends one move or insertion through the existing command contract, then chooses the result. -/// Held drags check ownership before reaching here; edge clicks commit immediately. +/// Sends a polygon move through the shared command only when opted in; insertion and other +/// geometry kinds retain `edit.vertex`. Held drags check ownership; edge clicks commit directly. fn commit(app: &mut RustGisApp, d: VertexDrag) { let action = if d.insert { "insert" } else { "move" }; - let p = json!({ + let mut p = json!({ "layer": d.layer, "row": d.row, "action": action, "part": d.vertex.part, "ring": d.vertex.ring, "index": d.vertex.index, "lon": d.at.x, "lat": d.at.y, }); - if let Ok(v) = app.run("edit.vertex", p) { + let shared = app.map.move_shared_vertices + && !d.insert + && app + .session + .map() + .layer(d.layer) + .and_then(|l| l.dataset.features.get(d.row)) + .and_then(|f| f.geometry.as_ref()) + .is_some_and(|g| matches!(g, Geometry::Polygon(_))); + let command = if shared { + if let Some(p) = p.as_object_mut() { + p.remove("action"); + } + "edit.moveSharedVertex" + } else { + "edit.vertex" + }; + if let Ok(v) = app.run(command, p) { let num = |k: &str| v.get(k).and_then(serde_json::Value::as_u64).map(|n| n as usize); if let (Some(part), Some(ring), Some(index)) = (num("part"), num("ring"), num("index")) { app.map.vertex = Some((d.layer, d.row, VertexId::new(part, ring, index))); @@ -286,6 +310,60 @@ fn neighbours(g: &Geometry, v: VertexId) -> Vec { prev.into_iter().chain(next).collect() } +/// The original coordinate shared by a polygon move, before any projected preview is drawn. +fn shared_origin(app: &RustGisApp, d: VertexDrag) -> Option { + if !app.map.move_shared_vertices || d.insert { + return None; + } + let g = app.session.map().layer(d.layer)?.dataset.features.get(d.row)?.geometry.as_ref()?; + matches!(g, Geometry::Polygon(_)).then(|| g.vertex(d.vertex)).flatten() +} + +/// Gesture-local original edges, never screen points or retained source rows. Camera changes +/// reproject this bounded overlay; row/context changes discard its owning `DragOwner`. +struct SharedPreview { + original: Coord, + edges: Vec<(Coord, Coord)>, +} + +/// Collects incident polygon edges once at press, stopping the lazy traversal at the overlay +/// cap even within a large ring. Ring closure and short paths follow `Geometry::for_each_edge`. +/// This cap never supplies or restricts the command's independently collected propagation set. +fn shared_edges<'a>(features: impl Iterator, original: Coord) -> Vec<(Coord, Coord)> { + features + .filter_map(|f| match &f.geometry { + Some(Geometry::Polygon(polys)) => Some(polys), + _ => None, + }) + .flatten() + .flatten() + .flat_map(|ring| { + let repeated = ring.len() > 1 && ring.first() == ring.last(); + let n = ring.len().saturating_sub(usize::from(repeated)); + let count = if n > 2 { n } else { n.saturating_sub(1) }; + ring.iter().take(n).copied().zip(ring.iter().take(n).copied().cycle().skip(1)).take(count) + }) + .inspect(|_| { + #[cfg(test)] + tests::SCANNED_EDGES.set(tests::SCANNED_EDGES.get() + 1); + }) + .filter(|(a, b)| *a == original || *b == original) + .take(MAX_HANDLES) + .collect() +} + +/// Reprojects only the cached incident edges, including unselected neighbours. Each frame's +/// shared overlay work is bounded independently of layer size and never mutates source rows. +fn draw_shared(app: &RustGisApp, p: &Painter, rect: Rect, d: VertexDrag, preview: &SharedPreview, accent: Color32) { + for &(a, b) in &preview.edges { + let a = if a == preview.original { d.at } else { a }; + let b = if b == preview.original { d.at } else { b }; + if let Some((a, b)) = edge_on_screen(app, rect, a, b) { + p.line_segment([a, b], Stroke::new(2.0, accent)); + } + } +} + /// The pointer's cursor over a handle. pub fn cursor(app: &RustGisApp, rect: Rect, p: Pos2) -> Option { if app.map.vertex_drag.is_some() { @@ -301,6 +379,10 @@ pub fn cursor(app: &RustGisApp, rect: Rect, p: Pos2) -> Option pub fn draw(app: &RustGisApp, p: &Painter, rect: Rect, accent: Color32) { let green = Color32::from_rgb(0x16, 0x9A, 0x5A); let drag = app.map.vertex_drag; + let shared = drag.and_then(|d| app.map.vertex_drag_owner.as_ref()?.preview.as_ref().map(|preview| (d, preview))); + if let Some((d, preview)) = shared { + draw_shared(app, p, rect, d, preview, accent); + } // Handles off the map aren't drawn and don't count towards the cap. let shown = rect.expand(8.0); let mut handles = 0; @@ -343,6 +425,9 @@ pub fn draw(app: &RustGisApp, p: &Painter, rect: Rect, accent: Color32) { if drag.is_some_and(|d| !d.insert && d.layer == layer && d.row == row && d.vertex == v) { return; } + if shared.is_some_and(|(d, preview)| d.layer == layer && preview.original == c) { + return; + } let Some(s) = lonlat_to_screen(app, rect, c).filter(|s| shown.contains(*s)) else { return }; let chosen = app.map.vertex == Some((layer, row, v)); let size = if chosen { 9.0 } else { 7.0 }; @@ -353,3 +438,127 @@ pub fn draw(app: &RustGisApp, p: &Painter, rect: Rect, accent: Color32) { }); } } + +#[cfg(test)] +#[allow(clippy::unwrap_used)] +mod tests { + use super::*; + use rustgis_engine::{Project, Session, data::Dataset}; + + std::thread_local! { + /// Deterministic traversal evidence, confined to the current test thread. + pub(super) static SCANNED_EDGES: std::cell::Cell = const { std::cell::Cell::new(0) }; + } + + /// A closed square with two edges incident to its first vertex. + fn square(x: f64) -> Geometry { + Geometry::Polygon(vec![vec![vec![ + Coord::new(x, 0.), + Coord::new(x + 1., 0.), + Coord::new(x + 1., 1.), + Coord::new(x, 1.), + Coord::new(x, 0.), + ]]]) + } + + /// Only the first row is selected; remaining rows must still participate in shared moves. + fn app(geometries: impl Iterator) -> RustGisApp { + let mut session = Session::new(Project::new("Shared preview cache")); + let mut data = Dataset::new("Parcels", rustgis_engine::data::GeometryType::Polygon, vec![]); + data.features = + geometries.enumerate().map(|(id, g)| Feature { id: id as u64, geometry: Some(g), attrs: vec![] }).collect::>().into(); + let id = session.map_mut().add_dataset(data); + session.active_layer = Some(id); + session.map_mut().layer_mut(id).unwrap().selection = vec![0]; + session.map_mut().camera.res = 1000.; + session.map_mut().editing.snapping.enabled = false; + let mut app = RustGisApp::new(session); + app.run("tool.set", json!({"tool":"editVertices"})).unwrap(); + app.run("ui.moveSharedVertices", json!({"on":true})).unwrap(); + app.session.clear_history(); + app + } + + /// A held gesture scans once, retains no rows, reprojects after camera changes, and drops + /// its cache when any row changes. Restarting captures fresh edges under the new identity. + #[test] + fn shared_preview_reuses_edges_and_invalidates_with_owner() { + let mut app = app(std::iter::repeat_with(|| square(0.)).take(2).chain(std::iter::repeat_with(|| square(20.)).take(500))); + let rect = Rect::from_min_size(Pos2::ZERO, egui::vec2(1000., 800.)); + SCANNED_EDGES.set(0); + let from = lonlat_to_screen(&app, rect, Coord::new(0., 0.)).unwrap(); + assert!(drag_start(&mut app, rect, from)); + assert_eq!(SCANNED_EDGES.get(), 2008); + let edges = &app.map.vertex_drag_owner.as_ref().unwrap().preview.as_ref().unwrap().edges; + assert_eq!(edges.len(), 4); + let allocation = edges.as_ptr(); + let ctx = egui::Context::default(); + for _ in 0..8 { + app.session.map_mut().camera.center.x += 10.; + cancel_stale_drag(&mut app); + let to = lonlat_to_screen(&app, rect, Coord::new(0.2, 0.3)).unwrap(); + drag_to(&mut app, rect, to); + let mut output = ctx.run_ui(Default::default(), |ui| { + draw(&app, &ui.ctx().layer_painter(egui::LayerId::background()), rect, Color32::RED); + }); + output.textures_delta.clear(); + assert_eq!(SCANNED_EDGES.get(), 2008, "no layer scan on movement or redraw"); + assert_eq!(app.map.vertex_drag_owner.as_ref().unwrap().preview.as_ref().unwrap().edges.as_ptr(), allocation); + } + app.session.map_mut().layers[0].dataset.features[1].id = 999; + cancel_stale_drag(&mut app); + assert!(app.map.vertex_drag_owner.is_none()); + assert!(app.map.vertex_drag.is_none()); + let from = lonlat_to_screen(&app, rect, Coord::new(0., 0.)).unwrap(); + assert!(drag_start(&mut app, rect, from)); + assert_eq!(SCANNED_EDGES.get(), 4016, "new gesture scans under the new row identity"); + assert_eq!(app.session.undo_depth(), 0); + } + + /// The overlay stops at its cap without visiting the remaining layer, whereas release + /// still moves every coincident row, including rows omitted from that overlay, in one undo. + #[test] + fn shared_preview_cap_does_not_limit_command_propagation() { + let mut app = app(std::iter::repeat_with(|| square(0.)).take(2101)); + let rect = Rect::from_min_size(Pos2::ZERO, egui::vec2(1000., 800.)); + SCANNED_EDGES.set(0); + let from = lonlat_to_screen(&app, rect, Coord::new(0., 0.)).unwrap(); + assert!(drag_start(&mut app, rect, from)); + assert_eq!(app.map.vertex_drag_owner.as_ref().unwrap().preview.as_ref().unwrap().edges.len(), MAX_HANDLES); + assert_eq!(SCANNED_EDGES.get(), 8000, "stop before the remaining 101 rows"); + let to = lonlat_to_screen(&app, rect, Coord::new(0.2, 0.3)).unwrap(); + drag_to(&mut app, rect, to); + let at = app.map.vertex_drag.unwrap().at; + drag_stop(&mut app); + assert!(app.map.vertex_drag_owner.is_none()); + assert_eq!(app.session.undo_depth(), 1); + for f in app.session.map().layers[0].dataset.features.iter() { + assert_eq!(f.geometry.as_ref().unwrap(), &square(0.).move_vertex(VertexId::new(0, 0, 0), at).unwrap()); + } + app.run("edit.undo", json!(null)).unwrap(); + assert!(app.session.map().layers[0].dataset.features.iter().all(|f| f.geometry.as_ref() == Some(&square(0.)))); + } + + /// Lazy polygon traversal matches the engine's closure convention, including short and + /// unclosed rings, holes, multiple parts, repeated nodes and exact rather than near matches. + #[test] + fn shared_preview_edges_match_geometry_contract() { + let o = Coord::new(0., 0.); + let a = Coord::new(1., 0.); + let b = Coord::new(0., 1.); + let g = Geometry::Polygon(vec![ + vec![vec![], vec![o], vec![o, a], vec![o, a, o], vec![o, a, b], vec![o, a, b, o]], + vec![vec![o, a, o, b, o], vec![Coord::new(f64::EPSILON, 0.), a, b]], + ]); + let expected: Vec<_> = g.edges().into_iter().filter(|(_, a, b)| *a == o || *b == o).map(|(_, a, b)| (a, b)).collect(); + let features = [ + Feature { id: 0, geometry: Some(g), attrs: vec![] }, + Feature { id: 1, geometry: Some(Geometry::Line(vec![vec![o, a]])), attrs: vec![] }, + ]; + assert_eq!(shared_edges(features.iter(), o), expected); + let dense = Feature { id: 2, geometry: Some(Geometry::Polygon(vec![vec![vec![o; MAX_HANDLES * 2]]])), attrs: vec![] }; + SCANNED_EDGES.set(0); + assert_eq!(shared_edges(std::iter::once(&dense), o).len(), MAX_HANDLES); + assert_eq!(SCANNED_EDGES.get(), MAX_HANDLES, "also stop inside one large ring"); + } +} diff --git a/crates/ui-egui/tests/move_shared_vertex.rs b/crates/ui-egui/tests/move_shared_vertex.rs new file mode 100644 index 000000000..1d19e14da --- /dev/null +++ b/crates/ui-egui/tests/move_shared_vertex.rs @@ -0,0 +1,230 @@ +//! Real Ribbon and pointer events for the opt-in shared polygon move command. +use egui::{Event, Modifiers, PointerButton, Pos2}; +use egui_kittest::{Harness, kittest::Queryable}; +use rustgis_engine::{ + Project, Session, + data::{Dataset, Feature, Geometry, GeometryType, VertexId}, + geom::{Coord, merc_to_lonlat}, +}; +use rustgis_ui_egui::RustGisApp; +use serde_json::json; + +/// Stores metre-based test geometry in longitude/latitude without import rounding. +fn c(x: f64, y: f64) -> Coord { + merc_to_lonlat(Coord::new(x, y)) +} + +/// Four parcels meet at zero, but only the north-east parcel is selected. +fn harness() -> Harness<'static, RustGisApp> { + let mut session = Session::new(Project::new("Shared polygon vertices")); + let mut data = Dataset::new("Parcels", GeometryType::Polygon, vec![]); + data.features = [(0., 0., 160., 160.), (-160., 0., 0., 160.), (-160., -160., 0., 0.), (0., -160., 160., 0.)] + .into_iter() + .enumerate() + .map(|(i, (l, b, r, t))| Feature { + id: i as u64 + 40, + geometry: Some(Geometry::Polygon(vec![vec![vec![c(l, b), c(r, b), c(r, t), c(l, t), c(l, b)]]])), + attrs: vec![], + }) + .collect::>() + .into(); + let id = session.map_mut().add_dataset(data); + session.active_layer = Some(id); + session.map_mut().layer_mut(id).unwrap().selection = vec![0]; + session.map_mut().camera.center = Coord::new(0., 0.); + session.map_mut().camera.res = 0.8; + session.map_mut().camera.set_bearing(17.); + session.map_mut().editing.snapping.enabled = 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(1366., 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 +} + +/// Returns each parcel's complete geometry, independent of selected vertex handles. +fn geometries(h: &Harness<'_, RustGisApp>) -> Vec { + h.state().session.map().layers[0].dataset.features.iter().map(|f| f.geometry.clone().unwrap()).collect() +} + +/// Projects a test point with 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) +} + +/// Starts and holds a real drag; the caller can inspect preview purity or change ownership. +fn drag(h: &mut Harness<'_, RustGisApp>, from: Coord, to: Coord) -> Pos2 { + let from = screen(h, from); + let to = screen(h, to); + 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(); + } + assert!(h.state().map.vertex_drag.is_some()); + to +} + +/// Releases the primary pointer through the same production path as a desktop or browser gesture. +fn release(h: &mut Harness<'_, RustGisApp>, to: Pos2) { + h.event(Event::PointerButton { pos: to, button: PointerButton::Primary, pressed: false, modifiers: Modifiers::NONE }); + for _ in 0..3 { + h.step(); + } +} + +/// The actual Ribbon toggle is transient; preview is pure and one release moves all four corners. +#[test] +fn ribbon_toggle_routes_polygon_drag_and_undoes_all_neighbours() { + let mut h = harness(); + let before = geometries(&h); + assert!(!h.state().map.move_shared_vertices); + h.get_by_label("Shared Vertices").click(); + h.run(); + assert!(h.state().map.move_shared_vertices); + assert_eq!(h.state().session.undo_depth(), 0); + assert!(!h.state().session.dirty); + let to = drag(&mut h, c(0., 0.), c(45., 30.)); + let at = h.state().map.vertex_drag.unwrap().at; + assert_eq!(geometries(&h), before); + release(&mut h, to); + let after = geometries(&h); + for (old, new) in before.iter().zip(&after) { + for (v, p) in old.vertices() { + assert_eq!(new.vertex(v), Some(if p == c(0., 0.) { at } else { p })); + } + } + assert_eq!(h.state().session.undo_depth(), 1); + h.state_mut().run("edit.undo", json!(null)).unwrap(); + assert_eq!(geometries(&h), before); + h.state_mut().run("edit.redo", json!(null)).unwrap(); + assert_eq!(geometries(&h), after); + let project = serde_json::to_string(&h.state().session.project).unwrap(); + let fresh = RustGisApp::new(Session::new(Project::from_json(&project).unwrap())); + assert!(!fresh.map.move_shared_vertices); +} + +/// The default/off path retains a single-feature move even when other corners coincide. +#[test] +fn disabled_toggle_keeps_the_existing_vertex_command() { + let mut h = harness(); + let before = geometries(&h); + let to = drag(&mut h, c(0., 0.), c(45., 30.)); + release(&mut h, to); + let after = geometries(&h); + assert_ne!(after[0], before[0]); + assert_eq!(after[1..], before[1..]); + assert_eq!(h.state().session.undo_depth(), 1); +} + +/// Neither enabling the switch nor an owned drag changes the existing insertion/deletion commands. +#[test] +fn insert_and_delete_remain_single_feature_edits() { + let mut h = harness(); + h.state_mut().run("ui.moveSharedVertices", json!({"on":true})).unwrap(); + let before = geometries(&h); + let to = drag(&mut h, c(80., 0.), c(80., -30.)); + assert!(h.state().map.vertex_drag.unwrap().insert); + release(&mut h, to); + assert_eq!(geometries(&h)[1..], before[1..]); + assert_eq!(geometries(&h)[0].vertices().len(), 5); + h.key_press(egui::Key::Delete); + h.step(); + assert_eq!(geometries(&h), before); + assert_eq!(h.state().session.undo_depth(), 2); +} + +/// A changed neighbour or switch invalidates the held shared move and swallows its release. +#[test] +fn changed_layer_or_toggle_cancels_a_shared_gesture() { + for toggle in [false, true] { + let mut h = harness(); + h.state_mut().run("ui.moveSharedVertices", json!({"on":true})).unwrap(); + let to = drag(&mut h, c(0., 0.), c(45., 30.)); + if toggle { + h.state_mut().run("ui.moveSharedVertices", json!({"on":false})).unwrap(); + } else { + h.state_mut().session.map_mut().layers[0].dataset.features[1].id = 999; + } + let before = geometries(&h); + release(&mut h, to); + assert_eq!(geometries(&h), before); + assert_eq!(h.state().session.undo_depth(), 0); + assert!(h.state().map.vertex_drag.is_none()); + } +} + +/// Escape preserves data/history, and a held click at its starting point remains a UI no-op. +#[test] +fn escape_and_same_position_release_do_not_commit() { + let mut h = harness(); + h.state_mut().run("ui.moveSharedVertices", json!({"on":true})).unwrap(); + let before = geometries(&h); + let to = drag(&mut h, c(0., 0.), c(45., 30.)); + h.key_press(egui::Key::Escape); + h.step(); + release(&mut h, to); + assert_eq!(geometries(&h), before); + assert_eq!(h.state().session.undo_depth(), 0); + // Escape may end the active tool; test return-to-origin in an independent owned gesture. + let mut h = harness(); + h.state_mut().run("ui.moveSharedVertices", json!({"on":true})).unwrap(); + let before = geometries(&h); + drag(&mut h, c(0., 0.), c(45., 30.)); + let from = screen(&h, c(0., 0.)); + h.event(Event::PointerMoved(from)); + h.step(); + release(&mut h, from); + assert_eq!(geometries(&h), before); + assert_eq!(h.state().session.undo_depth(), 0); +} + +/// Non-polygons keep single-feature moves even while the transient polygon mode is enabled. +#[test] +fn lines_and_points_keep_the_existing_move_path() { + for geometry in [Geometry::Line(vec![vec![c(0., 0.), c(0., 160.)]]), Geometry::Point(c(0., 0.))] { + let mut h = harness(); + let data = &mut h.state_mut().session.map_mut().layers[0].dataset; + data.geometry_type = geometry.kind(); + data.features = vec![ + Feature { id: 1, geometry: Some(geometry.clone()), attrs: vec![] }, + Feature { id: 2, geometry: Some(geometry.clone()), attrs: vec![] }, + ] + .into(); + h.state_mut().run("ui.moveSharedVertices", json!({"on":true})).unwrap(); + h.step(); + let to = drag(&mut h, c(0., 0.), c(45., 30.)); + release(&mut h, to); + assert_ne!(geometries(&h)[0], geometry); + assert_eq!(geometries(&h)[1], geometry); + assert_eq!(h.state().session.undo_depth(), 1); + } +} + +/// A malformed UI switch cannot change its state, source ownership, or project/history. +#[test] +fn malformed_toggle_is_rejected() { + let mut h = harness(); + for on in [json!("yes"), json!(1), json!([]), json!(null)] { + assert!(h.state_mut().run("ui.moveSharedVertices", json!({"on":on})).is_err()); + assert!(!h.state().map.move_shared_vertices); + } + assert_eq!(h.state().session.undo_depth(), 0); + assert!(geometries(&h)[0].vertex(VertexId::new(0, 0, 0)).is_some()); +} diff --git a/docs/user-guide/editing.md b/docs/user-guide/editing.md index 06947c036..31edcad73 100644 --- a/docs/user-guide/editing.md +++ b/docs/user-guide/editing.md @@ -67,6 +67,36 @@ you edit the value or choose units; click **Apply elevation** to apply the new i have dangles, must be covered by); Validate lists the errors, and clicking one zooms to it. - **Status**: features added, changed and deleted per layer since the last save. +### Moving shared polygon vertices + +**Edit › Features › Shared Vertices** opts polygon moves into linked editing. It starts off, +lasts only for the current app session, and is not saved in projects or preferences. With it on, +dragging a polygon vertex moves every polygon vertex in that layer stored at exactly the same +original coordinate, including unselected and filtered-out rows, point-only contacts, other +polygon parts and holes. Unrelated polygons touching by chance at that point move together too; +turn the switch off to move just the source feature. Inserting and deleting vertices, and moving +line or point vertices, keep their existing behaviour. Changing the switch cancels a held drag. + +The command uses row positions and the same vertex addresses as `edit.vertex`: + +```json +{"layer":"Parcels","row":0,"part":0,"ring":0,"index":3,"lon":12.5,"lat":41.9} +``` + +Run these parameters with `edit.moveSharedVertex`. It returns the source `part`, `ring` and +`index`, plus `modified`: the number of features whose geometry or attributes actually changed. +Every matched feature runs through attribute rules and archiving in one transaction; a rejection +rolls back the whole edit. A successful command is one undo step. As with `edit.vertex`, a direct +same-position command still runs the rules and records an edit, even if `modified` is zero. +Undo retains cumulative edit counts. Streamed layers require **Load All Features** first. + +This moves coincident **vertices**, not every shared boundary. For a boundary stored as P–Q +on one polygon and P–R–Q on its neighbour, moving Q moves the two stored Q vertices but leaves +R in place. It does not remap intermediate vertices, insert missing ones or refuse the edit +because vertex densities differ; the two boundaries can separate. There is no tolerance or +nearby-coordinate matching. Use the `mustNotOverlap` and `mustNotHaveGaps` topology rules, then +**Edit › Topology › Error Inspector (validate)** (`topology.validate`) to inspect a coverage. + ### Distance units in commands `edit.move`, `edit.traverse` and constant `edit.elevation` accept `"units": "Feet"`, From 4d2da00859d61e1207d8044255ad6bec20152fb1 Mon Sep 17 00:00:00 2001 From: yongyin-leon <269641072+yongyin-leon@users.noreply.github.com> Date: Mon, 26 Oct 2026 16:53:35 +0800 Subject: [PATCH 2/3] docs(edit): correct Error Inspector menu path --- docs/user-guide/editing.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user-guide/editing.md b/docs/user-guide/editing.md index 31edcad73..14541f87e 100644 --- a/docs/user-guide/editing.md +++ b/docs/user-guide/editing.md @@ -95,7 +95,7 @@ on one polygon and P–R–Q on its neighbour, moving Q moves the two stored Q v R in place. It does not remap intermediate vertices, insert missing ones or refuse the edit because vertex densities differ; the two boundaries can separate. There is no tolerance or nearby-coordinate matching. Use the `mustNotOverlap` and `mustNotHaveGaps` topology rules, then -**Edit › Topology › Error Inspector (validate)** (`topology.validate`) to inspect a coverage. +**Edit › Manage Edits › Topology › Error Inspector (validate)** (`topology.validate`) to inspect a coverage. ### Distance units in commands From 977e982c51f774aaaabd4c4a7a89a0bb11529173 Mon Sep 17 00:00:00 2001 From: yongyin-leon <269641072+yongyin-leon@users.noreply.github.com> Date: Mon, 26 Oct 2026 20:16:56 +0800 Subject: [PATCH 3/3] perf(edit): clone shared vertex geometries once --- crates/engine/src/cmd/design/shared_vertex.rs | 36 +++++----- crates/engine/tests/move_shared_vertex.rs | 65 +++++++++++++++++++ 2 files changed, 86 insertions(+), 15 deletions(-) diff --git a/crates/engine/src/cmd/design/shared_vertex.rs b/crates/engine/src/cmd/design/shared_vertex.rs index cc7ef6d56..c7489d102 100644 --- a/crates/engine/src/cmd/design/shared_vertex.rs +++ b/crates/engine/src/cmd/design/shared_vertex.rs @@ -27,24 +27,13 @@ pub(super) fn run(s: &mut Session, p: &Value) -> CmdResult { return Err("shared vertices can only be moved in polygons".into()); } let original = source.vertex(vertex).ok_or("no such vertex")?; - // Retaining shared rows adds no table copy and lets the result include calculation changes. + // This clone retains the Arc, not a second row table. The first write can still copy the + // full table, which the undo snapshot also shares; keep these rows for post-rule counts. let before = layer.dataset.features.clone(); let mut replacements = Vec::new(); for (row, feature) in before.iter().enumerate() { - let Some(geometry @ Geometry::Polygon(_)) = &feature.geometry else { continue }; - let mut nodes = Vec::new(); - geometry.for_each_vertex(|v, c| { - if c == original { - nodes.push(v); - } - }); - if nodes.is_empty() { - continue; - } - let mut moved = geometry.clone(); - for node in nodes { - moved = moved.move_vertex(node, at).ok_or("no such vertex")?; - } + let Some(geometry) = &feature.geometry else { continue }; + let Some(moved) = move_matching(geometry, original, at) else { continue }; super::super::edit::validate(&moved)?; replacements.push((row, moved)); } @@ -63,6 +52,23 @@ pub(super) fn run(s: &mut Session, p: &Value) -> CmdResult { Ok(json!({"part": vertex.part, "ring": vertex.ring, "index": vertex.index, "modified": modified})) } +/// Copies a matching polygon once and rewrites its original-coordinate matches in linear time. +/// Stored closing duplicates match their first node and move with it, as with `move_vertex`; +/// open rings stay unnormalized. Non-polygons and polygons without a match need no clone. +fn move_matching(geometry: &Geometry, original: Coord, at: Coord) -> Option { + let Geometry::Polygon(parts) = geometry else { return None }; + if !parts.iter().flatten().flatten().any(|c| *c == original) { + return None; + } + let mut moved = parts.clone(); + for c in moved.iter_mut().flatten().flatten() { + if *c == original { + *c = at; + } + } + Some(Geometry::Polygon(moved)) +} + /// Reads a platform-sized address; optional null parts/rings use the same defaults as `edit.vertex`. fn index(p: &Value, key: &str, default: Option) -> Result { match p.get(key) { diff --git a/crates/engine/tests/move_shared_vertex.rs b/crates/engine/tests/move_shared_vertex.rs index 6b421d3bb..2e1805c4c 100644 --- a/crates/engine/tests/move_shared_vertex.rs +++ b/crates/engine/tests/move_shared_vertex.rs @@ -121,13 +121,78 @@ fn near_vertices_and_other_layers_remain_unchanged() { let (mut s, id) = fixture(); let near = rectangle(f64::EPSILON, 0., 2., 2.); s.map_mut().layer_mut(id).unwrap().dataset.features.push(Feature { id: 99, geometry: Some(near.clone()), attrs: vec![Cell::Int(9)] }); + s.map_mut().layer_mut(id).unwrap().dataset.features.push(Feature { + id: 100, + geometry: Some(Geometry::Point(Coord::new(0., 0.))), + attrs: vec![Cell::Int(10)], + }); + s.map_mut().layer_mut(id).unwrap().dataset.features.push(Feature { id: 101, geometry: None, attrs: vec![Cell::Int(11)] }); + let unchanged = features(&s, id).into_iter().skip(4).collect::>(); let other = add_layer(&mut s, "Other", vec![rectangle(0., 0., 1., 1.)]); let untouched = features(&s, other); assert_eq!(move_to(&mut s, id, Coord::new(0.2, 0.3))["modified"], 4); assert_eq!(features(&s, id)[4].geometry.as_ref(), Some(&near)); + assert_eq!(&features(&s, id)[4..], unchanged.as_slice()); assert_eq!(features(&s, other), untouched); } +/// Frozen pre-optimization address loop, used only as a small-input closing-alias oracle. +fn addressed_move(geometry: &Geometry, original: Coord, at: Coord) -> Geometry { + let mut moved = geometry.clone(); + geometry.for_each_vertex(|vertex, c| { + if c == original { + moved = moved.move_vertex(vertex, at).unwrap(); + } + }); + moved +} + +/// Single-copy rewriting agrees with vertex-address moves when destinations create closure too. +#[test] +fn coincident_rewrite_preserves_open_and_closed_ring_address_semantics() { + let c = Coord::new(0., 0.); + let a = Coord::new(1., 0.); + let b = Coord::new(1., 1.); + let d = Coord::new(0., 1.); + let rings = [vec![c, a, b, c, d, c], vec![c, a, b, c, d], vec![a, c, b, d, c], vec![c, a, b, d, a], vec![a, b, c, d, a]]; + for ring in rings { + let index = ring.iter().position(|point| *point == c).unwrap(); + for at in [c, a, Coord::new(-0.1, 0.2)] { + let geometry = Geometry::Polygon(vec![vec![ring.clone()]]); + let expected = addressed_move(&geometry, c, at); + let mut s = Session::new(Project::new("Closing aliases")); + let id = add_layer(&mut s, "Parcels", vec![geometry.clone()]); + let result = s.run("edit.moveSharedVertex", &json!({"layer":id,"row":0,"index":index,"lon":at.x,"lat":at.y})).unwrap(); + assert_eq!(features(&s, id)[0].geometry.as_ref(), Some(&expected), "{ring:?} → {at:?}"); + assert_eq!(result["modified"], usize::from(expected != geometry)); + } + } +} + +/// Many stored matches rewrite together without changing row identity, attributes or one-step undo. +#[test] +fn densely_repeated_nodes_move_and_undo_as_one_feature() { + let c = Coord::new(0., 0.); + let at = Coord::new(-0.1, 0.2); + let mut ring = vec![c; 32_768]; + ring.extend([Coord::new(1., 0.), Coord::new(1., 1.), Coord::new(0., 1.), c]); + let geometry = Geometry::Polygon(vec![vec![ring.clone()]]); + let expected = Geometry::Polygon(vec![vec![ring.into_iter().map(|point| if point == c { at } else { point }).collect()]]); + let mut s = Session::new(Project::new("Repeated nodes")); + let id = add_layer(&mut s, "Parcels", vec![geometry]); + s.clear_history(); + let before = features(&s, id); + assert_eq!(move_to(&mut s, id, at)["modified"], 1); + let after = features(&s, id); + assert_eq!(after[0].geometry.as_ref(), Some(&expected)); + assert_eq!((after[0].id, &after[0].attrs), (before[0].id, &before[0].attrs)); + assert_eq!(s.undo_depth(), 1); + s.run("edit.undo", &Value::Null).unwrap(); + assert_eq!(features(&s, id), before); + s.run("edit.redo", &Value::Null).unwrap(); + assert_eq!(features(&s, id), after); +} + /// Different segmentation is accepted: intermediate nodes are neither inserted nor remapped. #[test] fn differing_density_moves_existing_nodes_and_keeps_the_documented_limitation() {