From b6da5be9d0e5598cb7611f1b321a9c001c2df55a Mon Sep 17 00:00:00 2001 From: Cory Douglas Martin <4108748+flight1973@users.noreply.github.com> Date: Mon, 9 Nov 2026 16:14:43 -0600 Subject: [PATCH 01/11] fix(analysis): Clip and the overlay tools say which mask features cover nothing Clip, Erase, Intersect, Identity, Union, Symmetrical Difference, Update and Split skipped a feature of the mask or overlay layer that has no polygon (no geometry, a point or line in the polygon layer, an empty polygon) without saying so. Minimum Bounding Geometry now writes flat hulls as points and lines in a polygon layer, so these are easier to hit. Each tool now counts them and gives one notice; Clip also counts a polygon with no area, and the overlay tools a feature whose parts union to nothing. Empty update and overlay polygons are no longer written out by Update, Union and Symmetrical Difference. Minimum Bounding Geometry with Group: All looks at Cancel per feature, and both group modes join the long tools in the cancel test. --- AGENTS.md | 2 +- crates/analysis/src/tools.rs | 202 ++++++++++++++++++++++++++++++----- crates/engine/src/tests.rs | 6 +- docs/user-guide/analysis.md | 11 +- 4 files changed, 191 insertions(+), 30 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 35a227851..a6d1b7a65 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -47,7 +47,7 @@ is out of scope. background command as its work starts and fails one that never looks; its lists must match the background registries, so a new background command is added there. It also runs the long tools (Buffer, Dissolve, Spatial Join, Clip, Erase, Intersect, Identity, Union, Symmetrical - Difference, Update, Split, Kernel Density, Kriging); a new tool + Difference, Update, Split, Minimum Bounding Geometry, Kernel Density, Kriging); a new tool whose work grows with its input is added to that list. Passing shows the work looks at Cancel, not that it looks often: a long step without a check (one union, one read) is said in its doc comment. diff --git a/crates/analysis/src/tools.rs b/crates/analysis/src/tools.rs index f27de2453..a2aaec472 100644 --- a/crates/analysis/src/tools.rs +++ b/crates/analysis/src/tools.rs @@ -1281,6 +1281,23 @@ fn result(ds: Dataset, msg: String) -> Result { Ok(ToolResult { output: Output::Layer(ds), messages: vec![msg] }) } +/// A feature's polygons, or none when it has no polygon to give: no geometry, an empty polygon, +/// or a point or line, which a polygon layer can hold (Minimum Bounding Geometry writes its flat +/// hulls so). +fn polygon_of(f: Option<&Feature>) -> Option> { + f.and_then(|f| f.geometry.as_ref()).and_then(to_multipolygon).filter(|mp| mp.0.iter().any(|p| !p.exterior().0.is_empty())) +} + +/// The notice for the `n` features of `layer` that `tool` left out of its masks or overlay +/// polygons as they cover nothing; `what` lists what they hold. None when there are none. +fn no_polygon_note(tool: &str, layer: &str, n: usize, what: &str) -> Option { + (n > 0).then(|| format!("{tool}: {n} feature(s) of '{layer}' left out, covering nothing: {what}")) +} + +/// What a feature [`overlay_zones`] leaves out holds: [`polygon_of`] gives nothing for it, or +/// its parts, unioned, have no area. +const NO_POLYGON: &str = "no geometry, a point or line, an empty polygon, or several parts with no area"; + /// A copy of a feature's attributes for an output with the same schema. fn copy_schema(ds: &Dataset, name: String, g: GeometryType) -> Dataset { Dataset::new(name, g, ds.fields.clone()) @@ -1585,19 +1602,26 @@ fn clip(inp: &dyn Inputs, p: &Json) -> Result { return Err(format!("'{}' is not a polygon layer", clip_ds.name)); } let mut masks = Vec::new(); + // Mask features that clip nothing, left out and counted: no polygon, or one with no area. + let mut no_mask = 0usize; let mask_rows = rows(inp, p, "clip", clip_ds); crate::control::begin(mask_rows.len()); for (done, r) in mask_rows.into_iter().enumerate() { // Each mask is a union: Cancel is looked at, and progress counted, before each. crate::control::check()?; crate::control::reached(done); - let Some(m) = clip_ds.features.get(r).and_then(|f| f.geometry.as_ref()).and_then(to_multipolygon) else { continue }; + let Some(m) = polygon_of(clip_ds.features.get(r)) else { + no_mask += 1; + continue; + }; let ext = merc_extent(&m); // Union even one polygon: collapsed rings have no filled area. Keep the original // bounds too: rounding an edge must not hide a line the boolean overlay clips to it. let mask = union_all(m.0.iter()); let ext = ext.union(&merc_extent(&mask)); - if !mask.0.is_empty() { + if mask.0.is_empty() { + no_mask += 1; + } else { masks.push((mask, ext)); } } @@ -1654,7 +1678,9 @@ fn clip(inp: &dyn Inputs, p: &Json) -> Result { } } let n = out.features.len(); - result(out, format!("Clip: {n} features kept")) + let mut r = result(out, format!("Clip: {n} features kept"))?; + r.messages.extend(no_polygon_note("Clip", &clip_ds.name, no_mask, "no geometry, a point or line, or a polygon with no area")); + Ok(r) } /// Erase: each input feature's part outside every erase polygon, as [`outside`] gives it (a point @@ -1666,7 +1692,9 @@ fn erase(inp: &dyn Inputs, p: &Json) -> Result { if ds.geometry_type == GeometryType::None { return Err(format!("'{}' has no geometry", ds.name)); } - let (er, zones) = overlay_zones(inp, p, "erase")?; + let zs = overlay_zones(inp, p, "erase")?; + let left_out = zs.note("Erase"); + let OverlayZones { layer: er, zones, .. } = zs; let index = ExtentIndex::new(zones.iter().map(|z| z.ext)); let at_poles = AtPoles::new(zones.iter().map(|z| poles(&z.ext))); let mut out = copy_schema(ds, out_name(p, format!("{}_Erase", ds.name)), ds.geometry_type); @@ -1682,7 +1710,9 @@ fn erase(inp: &dyn Inputs, p: &Json) -> Result { } } let n = out.features.len(); - result(out, format!("Erase: {n} features remain")) + let mut r = result(out, format!("Erase: {n} features remain"))?; + r.messages.extend(left_out); + Ok(r) } /// An overlay polygon: its row in the overlay layer, its extent and its polygons in the analysis @@ -1701,8 +1731,9 @@ fn mask_of(poly: MultiPolygon) -> MultiPolygon { } /// The polygon layer of an overlay tool's parameter `k` (`overlay`, `erase`, `update`, `split`), -/// which must be polygons, and its (selected) features with geometry as [`Zone`]s, in order. -fn overlay_zones<'a>(inp: &'a dyn Inputs, p: &Json, k: &str) -> Result<(&'a Dataset, Vec), String> { +/// which must be polygons, and its (selected) features with a polygon as [`Zone`]s, in order; the +/// others ([`NO_POLYGON`]) are counted for the tool's notice. +fn overlay_zones<'a>(inp: &'a dyn Inputs, p: &Json, k: &str) -> Result, String> { let ov = input(inp, p, k)?; if ov.geometry_type != GeometryType::Polygon { return Err(format!("'{}' is not a polygon layer", ov.name)); @@ -1711,15 +1742,33 @@ fn overlay_zones<'a>(inp: &'a dyn Inputs, p: &Json, k: &str) -> Result<(&'a Data let zone_rows = rows(inp, p, k, ov); crate::control::begin(zone_rows.len()); let mut zones = Vec::with_capacity(zone_rows.len()); + let mut no_polygon = 0usize; for (done, row) in zone_rows.into_iter().enumerate() { crate::control::check()?; crate::control::reached(done); - let Some(g) = ov.features.get(row).and_then(|f| f.geometry.as_ref()) else { continue }; - if let Some(poly) = to_multipolygon(g).map(mask_of) { - zones.push(Zone { row, ext: g.extent(), poly }); + let f = ov.features.get(row); + // Parts are unioned (see [`mask_of`]): several with no area between them come out empty. + match (polygon_of(f).map(mask_of), f.and_then(|f| f.geometry.as_ref())) { + (Some(poly), Some(g)) if !poly.0.is_empty() => zones.push(Zone { row, ext: g.extent(), poly }), + _ => no_polygon += 1, } } - Ok((ov, zones)) + Ok(OverlayZones { layer: ov, zones, no_polygon }) +} + +/// The overlay polygons [`overlay_zones`] gives, from `layer`, and how many of its features it +/// left out for having no polygon ([`polygon_of`]). +struct OverlayZones<'a> { + layer: &'a Dataset, + zones: Vec, + no_polygon: usize, +} + +impl OverlayZones<'_> { + /// The notice for the features left out, for `tool`'s result. + fn note(&self, tool: &str) -> Option { + no_polygon_note(tool, &self.layer.name, self.no_polygon, NO_POLYGON) + } } /// An overlay tool's output layer: the input's fields, then the overlay's, an overlay field whose @@ -1888,7 +1937,9 @@ fn overlay(inp: &dyn Inputs, p: &Json, op: Overlay) -> Result Result Value { /// update polygon as it is, its values copied into the input's fields of the same name (matched /// exactly, else ignoring case; converted by [`converted`]), the input's other fields empty. The /// output has the input's fields only. Input features without geometry, and those no update -/// polygon overlaps, are kept as they are; update features without geometry are left out, and -/// update polygons that overlap each other each come out whole. An input field that no update +/// polygon overlaps, are kept as they are; update features without a polygon ([`NO_POLYGON`]) are +/// left out, saying how many, and update polygons that overlap each other each come out whole. An input field that no update /// field matches exactly, but two match ignoring case, is an error rather than a guess. fn update(inp: &dyn Inputs, p: &Json) -> Result { let ds = input(inp, p, "input")?; if ds.geometry_type != GeometryType::Polygon { return Err(format!("'{}' is not a polygon layer", ds.name)); } - let (up, zones) = overlay_zones(inp, p, "update")?; + let zs = overlay_zones(inp, p, "update")?; + let left_out = zs.note("Update"); + let OverlayZones { layer: up, zones, .. } = zs; // Each input field's field in the update layer, if it has one: an ambiguous one fails here, // before any geometry work. let from = ds @@ -2056,7 +2111,9 @@ fn update(inp: &dyn Inputs, p: &Json) -> Result { out.push_next(u.geometry.clone(), attrs); } let added = out.features.len() - kept; - result(out, format!("Update: {kept} input features kept, {added} update features added")) + let mut r = result(out, format!("Update: {kept} input features kept, {added} update features added"))?; + r.messages.extend(left_out); + Ok(r) } /// The most split areas (split polygons, or values of the split field) Split takes: more is an @@ -2094,7 +2151,9 @@ fn split(inp: &dyn Inputs, p: &Json) -> Result { if ds.geometry_type == GeometryType::None { return Err(format!("'{}' has no geometry", ds.name)); } - let (sp, zones) = overlay_zones(inp, p, "split")?; + let zs = overlay_zones(inp, p, "split")?; + let left_out = zs.note("Split"); + let OverlayZones { layer: sp, zones, .. } = zs; // A field name, or none; anything else is an error, not one layer per polygon. let field = match p.get("field") { None | Some(Json::Null) => None, @@ -2179,7 +2238,7 @@ fn split(inp: &dyn Inputs, p: &Json) -> Result { let areas = if empty == 1 { "split area" } else { "split areas" }; msg = format!("{msg} ({empty} {areas} held no features)"); } - Ok(ToolResult { output: Output::Layers(layers), messages: vec![msg] }) + Ok(ToolResult { output: Output::Layers(layers), messages: std::iter::once(msg).chain(left_out).collect() }) } /// Dissolve: one feature for each value of the dissolve field (one for all without it), the union @@ -2209,8 +2268,7 @@ fn dissolve(inp: &dyn Inputs, p: &Json) -> Result { for (done, r) in input_rows.into_iter().enumerate() { crate::control::check()?; crate::control::reached(done); - let polygon = ds.features.get(r).and_then(|f| f.geometry.as_ref()).and_then(to_multipolygon); - let Some(mp) = polygon.filter(|mp| mp.0.iter().any(|p| !p.exterior().0.is_empty())) else { + let Some(mp) = polygon_of(ds.features.get(r)) else { not_polygons += 1; continue; }; @@ -2278,6 +2336,9 @@ fn dissolve(inp: &dyn Inputs, p: &Json) -> Result { /// latitude past ±90° is taken as the pole.) const HULL_LIMIT_M: f64 = 1e12; +/// Minimum Bounding Geometry. Cancel is looked at per feature, as each one's points are gathered +/// (and its hull made, one hull each); the one hull of all features' points, with `group` All, is +/// a single step without a look. fn convex_hull(inp: &dyn Inputs, p: &Json) -> Result { let ds = input(inp, p, "input")?; let each = s(p, "group") == "None"; @@ -2364,8 +2425,13 @@ fn convex_hull(inp: &dyn Inputs, p: &Json) -> Result { } else { // All together; `count` is the features hulled, those with a point to give. With none, // no hull is written, as one feature each. + // Cancel is looked at, and progress counted, per feature; the one hull of all their + // points is a single step without a look. let (mut pts, mut hulled) = (Vec::new(), 0usize); - for (_, g) in &features { + crate::control::begin(features.len()); + for (done, (_, g)) in features.iter().enumerate() { + crate::control::check()?; + crate::control::reached(done); let (these, passed, past) = points(g); gaps += passed; poles += past; @@ -2416,9 +2482,9 @@ fn convex_hull(inp: &dyn Inputs, p: &Json) -> Result { GeometryType::Point => "the output is a point layer".to_string(), GeometryType::Polyline => "the output is a line layer".to_string(), // Points and lines, every hull flat: a polygon layer, there being no one kind. - _ if flat >= n => "they are kept in the output's polygon layer, points and lines together, where Dissolve leaves them out and as Clip masks they cover nothing".to_string(), + _ if flat >= n => "they are kept in the output's polygon layer, points and lines together, where Dissolve, Clip and the overlay tools leave them out, saying how many".to_string(), _ => format!( - "they are kept in the output's polygon layer, among its {} polygon(s), where Dissolve leaves them out and as Clip masks they cover nothing", + "they are kept in the output's polygon layer, among its {} polygon(s), where Dissolve, Clip and the overlay tools leave them out, saying how many", n.saturating_sub(flat) ), }; @@ -6870,6 +6936,92 @@ mod tests { assert!(e.contains("none of the 2 feature(s) has a polygon"), "{e}"); } + /// Clip and the overlay tools say how many mask or overlay features they left out as covering + /// nothing: a point and a line in a polygon layer (as Minimum Bounding Geometry's flat hulls + /// are), empty polygons, a feature without geometry and one of two parts with no area, and for + /// Clip a single polygon with no area too. Those change nothing in the output; a layer of + /// polygons says nothing of the kind, and only the selected features are counted. + #[test] + fn overlay_tools_count_mask_features_that_cover_nothing() { + struct Sel(Map, Option>); + impl Inputs for Sel { + fn dataset(&self, layer: &str) -> Option<&Dataset> { + self.0.dataset(layer) + } + fn selection(&self, layer: &str) -> &[usize] { + match &self.1 { + Some(rows) if layer == "masks" => rows, + _ => &[], + } + } + } + // On one parallel: a straight line on the analysis plane too. + let flat = |y: f64| vec![Coord::new(5.0, y), Coord::new(6.0, y), Coord::new(7.0, y), Coord::new(5.0, y)]; + let run = |tool: &str, key: &str, extra: bool, selected: Option>| { + let mut polys = Dataset::new("polys", GeometryType::Polygon, vec![Field::new("a", FieldType::Integer)]); + polys.push(Some(sq(0.0, 0.0, 2.0)), vec![Value::Int(1)]); + polys.push(Some(sq(5.0, 0.0, 2.0)), vec![Value::Int(2)]); + let mut masks = Dataset::new("masks", GeometryType::Polygon, vec![Field::new("b", FieldType::Integer)]); + masks.push(Some(sq(1.0, 1.0, 2.0)), vec![Value::Int(10)]); + if extra { + // Over the second input polygon, where a mask would change the output. + masks.push(Some(Geometry::Point(Coord::new(6.0, 1.0))), vec![Value::Int(11)]); + masks.push(Some(Geometry::Line(vec![vec![Coord::new(5.0, 1.0), Coord::new(7.0, 1.0)]])), vec![Value::Int(12)]); + masks.push(Some(Geometry::Polygon(vec![])), vec![Value::Int(13)]); + masks.push(Some(Geometry::Polygon(vec![vec![]])), vec![Value::Int(14)]); + masks.push(None, vec![Value::Int(15)]); + masks.push(Some(Geometry::Polygon(vec![vec![flat(1.0)], vec![flat(1.5)]])), vec![Value::Int(16)]); + if tool == "clip" { + masks.push(Some(Geometry::Polygon(vec![vec![flat(1.0)]])), vec![Value::Int(17)]); + } + } + let m = Sel(Map(HashMap::from([("polys".to_string(), polys), ("masks".to_string(), masks)])), selected); + let res = crate::run(tool, &m, &serde_json::json!({"input": "polys", key: "masks"})).unwrap_or_else(|e| panic!("{tool}: {e}")); + let notes: Vec = res.messages.iter().filter(|m| m.contains("covering nothing")).cloned().collect(); + let shapes: Vec> = match res.output { + Output::Layer(ds) => ds.features.into_iter().map(|f| f.geometry).collect(), + Output::Layers(layers) => layers + .into_iter() + .flat_map(|l| match l { + OutputLayer::Vector(ds) => ds.features.into_iter().map(|f| f.geometry).collect(), + _ => Vec::new(), + }) + .collect(), + _ => panic!("{tool}: a layer"), + }; + (shapes, notes) + }; + let tools = [ + ("clip", "clip", "Clip"), + ("erase", "erase", "Erase"), + ("intersect", "overlay", "Intersect"), + ("identity", "overlay", "Identity"), + ("union", "overlay", "Union"), + ("symmetricalDifference", "overlay", "Symmetrical Difference"), + ("update", "update", "Update"), + ("split", "split", "Split"), + ]; + let note = |name: &str, n: usize| { + let what = if name == "Clip" { + "no geometry, a point or line, or a polygon with no area" + } else { + "no geometry, a point or line, an empty polygon, or several parts with no area" + }; + format!("{name}: {n} feature(s) of 'masks' left out, covering nothing: {what}") + }; + for (tool, key, name) in tools { + let (clean, none) = run(tool, key, false, None); + assert!(none.is_empty(), "{tool}: {none:?}"); + let (shapes, notes) = run(tool, key, true, None); + assert_eq!(shapes, clean, "{tool}: the features covering nothing change nothing"); + assert_eq!(notes, [note(name, if tool == "clip" { 7 } else { 6 })], "{tool}"); + // The square and the point and line selected: only those two are counted. + let (shapes, notes) = run(tool, key, true, Some(vec![0, 1, 2])); + assert_eq!(shapes, clean, "{tool}: selected"); + assert_eq!(notes, [note(name, 2)], "{tool}: selected"); + } + } + /// Dissolve's groups, found by their keys' hash: in the order first met, −0 with 0, each NaN /// on its own (it equals no value), and a number apart from the same text and a whole number /// apart from the same fraction. @@ -8982,7 +9134,7 @@ mod tests { crate::run_with("buffer", &m, &serde_json::json!({"input": "pts", "distance": 100.0}), &control).unwrap_or_else(|e| panic!("{e}")); assert_eq!(control.progress(), Some((40, 40))); // The same control for a tool that doesn't count shows no count, not the buffers'. - crate::run_with("convexHull", &m, &serde_json::json!({"input": "pts"}), &control).unwrap_or_else(|e| panic!("{e}")); + crate::run_with("featureVerticesToPoints", &m, &serde_json::json!({"input": "pts"}), &control).unwrap_or_else(|e| panic!("{e}")); assert_eq!(control.progress(), None); // Dissolved, the union after the buffers isn't counted: the buffers' count ends before it. let dissolve = serde_json::json!({"input": "pts", "distance": 100.0, "dissolve": "All"}); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index c1d9ec20e..e51f0157f 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -37084,8 +37084,8 @@ fn long_commands_stop_when_cancelled_while_they_run() { } // The tools whose work grows past a second or two on real data: buffers, overlays, joins, - // densities and interpolation. Run in the background as the Analysis pane runs them; a new - // long tool is added here (AGENTS.md › Never hold the window). + // hulls, densities and interpolation. Run in the background as the Analysis pane runs them; a + // new long tool is added here (AGENTS.md › Never hold the window). let tools = [ ("buffer", json!({"input": "Cities", "distance": 100})), ("dissolve", json!({"input": "Countries"})), @@ -37098,6 +37098,8 @@ fn long_commands_stop_when_cancelled_while_they_run() { ("symmetricalDifference", json!({"input": "Lakes", "overlay": "Countries"})), ("update", json!({"input": "Countries", "update": "Lakes"})), ("split", json!({"input": "Rivers", "split": "Countries"})), + ("convexHull", json!({"input": "Countries", "group": "None"})), + ("convexHull", json!({"input": "Countries", "group": "All"})), ("kernelDensity", json!({"input": "Cities", "crs": "EPSG:3857"})), ("kriging", json!({"input": "Cities", "field": "pop_max"})), ]; diff --git a/docs/user-guide/analysis.md b/docs/user-guide/analysis.md index 02428abc6..b5c944af2 100644 --- a/docs/user-guide/analysis.md +++ b/docs/user-guide/analysis.md @@ -611,6 +611,12 @@ field), empty ones included. Selections in both layers are honoured. gp.run={"tool":"split","params":{"input":"roads","split":"districts","field":"name"}} ``` +Clip, Erase, Intersect, Identity, Union, Symmetrical Difference, Update and Split leave out a +feature of the clip, erase, overlay, update or split layer that covers nothing, and say how many: +one without geometry, a point or line in the polygon layer (as Minimum Bounding Geometry's flat +hulls are), an empty polygon, or a polygon with no area. Clip leaves out any polygon with no area; +the others leave out one whose parts, two or more, all have no area, and keep a single such part. + **Near** (`near`) copies `input` and adds, for each feature, the nearest feature of `near`: its OID as the attribute table shows it (`near_fid`), the distance to it (`near_dist`, in `units`, Kilometers by default) and the point on it nearest the input feature (`near_x` and `near_y`, in @@ -782,8 +788,9 @@ writing no hull for it; if every feature is skipped, it is an error. A latitude taken as that pole. A hull with no area is written as what it is: a point for points in one place, or the line between the two farthest apart for points on one straight line on the map. The layer is a point or line layer when every hull is one, and the tool says so; otherwise it's a polygon layer -holding any points and lines among its polygons. Dissolve leaves those out, saying how many; as -Clip masks they cover nothing; a Shapefile export leaves them out, saying how many; and a +holding any points and lines among its polygons. Dissolve leaves those out, saying how many; so +do Clip and the overlay tools when the layer is their mask or overlay; a Shapefile export leaves +them out, saying how many; and a GeoPackage export keeps them in a column that holds any kind of geometry. It says how many of each. From e4685a1dd2cb0e387cef02c937486a264939b0f2 Mon Sep 17 00:00:00 2001 From: Cory Douglas Martin <4108748+flight1973@users.noreply.github.com> Date: Mon, 9 Nov 2026 16:47:50 -0600 Subject: [PATCH 02/11] fix(analysis): Update keeps its update polygons; leaving out every mask is an error Review follow-up. overlay_zones keeps every feature with a polygon as a zone again, so Update still writes an update polygon whose parts have no area, as it is. Union and Symmetrical Difference, where such a feature's row would vanish, count it in a notice of its own. Clip and the overlay tools stop with an error when every feature of the mask or overlay layer covers nothing, as Dissolve does, rather than give an empty layer or the input unchanged. A row selected twice is counted once, and Minimum Bounding Geometry's notice says the overlay tools leave its points and lines out of a mask or overlay layer only. --- crates/analysis/src/tools.rs | 172 ++++++++++++++++++++--------------- docs/user-guide/analysis.md | 6 +- 2 files changed, 104 insertions(+), 74 deletions(-) diff --git a/crates/analysis/src/tools.rs b/crates/analysis/src/tools.rs index a2aaec472..58435c72e 100644 --- a/crates/analysis/src/tools.rs +++ b/crates/analysis/src/tools.rs @@ -1281,11 +1281,10 @@ fn result(ds: Dataset, msg: String) -> Result { Ok(ToolResult { output: Output::Layer(ds), messages: vec![msg] }) } -/// A feature's polygons, or none when it has no polygon to give: no geometry, an empty polygon, -/// or a point or line, which a polygon layer can hold (Minimum Bounding Geometry writes its flat -/// hulls so). -fn polygon_of(f: Option<&Feature>) -> Option> { - f.and_then(|f| f.geometry.as_ref()).and_then(to_multipolygon).filter(|mp| mp.0.iter().any(|p| !p.exterior().0.is_empty())) +/// A geometry's polygons, or none when it has no polygon to give: an empty polygon, or a point +/// or line, which a polygon layer can hold (Minimum Bounding Geometry writes its flat hulls so). +fn polygon_of(g: &Geometry) -> Option> { + to_multipolygon(g).filter(|mp| mp.0.iter().any(|p| !p.exterior().0.is_empty())) } /// The notice for the `n` features of `layer` that `tool` left out of its masks or overlay @@ -1294,9 +1293,22 @@ fn no_polygon_note(tool: &str, layer: &str, n: usize, what: &str) -> Option 0).then(|| format!("{tool}: {n} feature(s) of '{layer}' left out, covering nothing: {what}")) } -/// What a feature [`overlay_zones`] leaves out holds: [`polygon_of`] gives nothing for it, or -/// its parts, unioned, have no area. -const NO_POLYGON: &str = "no geometry, a point or line, an empty polygon, or several parts with no area"; +/// The error when `tool` would leave out every one of the `n` features of `layer` it was given +/// (`what` lists what they hold): its output would be empty, or its input unchanged. +fn nothing_covered(tool: &str, layer: &str, n: usize, what: &str) -> String { + format!("{tool}: none of the {n} feature(s) of '{layer}' covers anything: {what}") +} + +/// What a feature [`overlay_zones`] leaves out holds: no geometry, or a geometry [`polygon_of`] +/// gives nothing for. +const NO_POLYGON: &str = "no geometry, a point or line, or an empty polygon"; + +/// What a mask Clip leaves out holds: [`NO_POLYGON`], or polygons whose union has no area. +const NO_MASK: &str = "no geometry, a point or line, or a polygon with no area"; + +/// What an overlay feature Union and Symmetrical Difference leave out of their output holds, +/// though it is a zone: parts whose union (see [`mask_of`]) has no area. +const NO_AREA: &str = "two or more polygon parts, all with no area"; /// A copy of a feature's attributes for an output with the same schema. fn copy_schema(ds: &Dataset, name: String, g: GeometryType) -> Dataset { @@ -1604,13 +1616,13 @@ fn clip(inp: &dyn Inputs, p: &Json) -> Result { let mut masks = Vec::new(); // Mask features that clip nothing, left out and counted: no polygon, or one with no area. let mut no_mask = 0usize; - let mask_rows = rows(inp, p, "clip", clip_ds); + let mask_rows = each_once(rows(inp, p, "clip", clip_ds)); crate::control::begin(mask_rows.len()); for (done, r) in mask_rows.into_iter().enumerate() { // Each mask is a union: Cancel is looked at, and progress counted, before each. crate::control::check()?; crate::control::reached(done); - let Some(m) = polygon_of(clip_ds.features.get(r)) else { + let Some(m) = clip_ds.features.get(r).and_then(|f| f.geometry.as_ref()).and_then(polygon_of) else { no_mask += 1; continue; }; @@ -1625,6 +1637,9 @@ fn clip(inp: &dyn Inputs, p: &Json) -> Result { masks.push((mask, ext)); } } + if masks.is_empty() && no_mask > 0 { + return Err(nothing_covered("Clip", &clip_ds.name, no_mask, NO_MASK)); + } let index = ExtentIndex::new(masks.iter().map(|(_, ext)| *ext)); // The masks reaching each pole: a point at one is in them at any longitude (#1033). let at_poles = AtPoles::new(masks.iter().map(|(m, _)| plane_poles(m))); @@ -1679,7 +1694,7 @@ fn clip(inp: &dyn Inputs, p: &Json) -> Result { } let n = out.features.len(); let mut r = result(out, format!("Clip: {n} features kept"))?; - r.messages.extend(no_polygon_note("Clip", &clip_ds.name, no_mask, "no geometry, a point or line, or a polygon with no area")); + r.messages.extend(no_polygon_note("Clip", &clip_ds.name, no_mask, NO_MASK)); Ok(r) } @@ -1692,9 +1707,7 @@ fn erase(inp: &dyn Inputs, p: &Json) -> Result { if ds.geometry_type == GeometryType::None { return Err(format!("'{}' has no geometry", ds.name)); } - let zs = overlay_zones(inp, p, "erase")?; - let left_out = zs.note("Erase"); - let OverlayZones { layer: er, zones, .. } = zs; + let (er, zones, left_out) = overlay_zones(inp, p, "erase", "Erase")?; let index = ExtentIndex::new(zones.iter().map(|z| z.ext)); let at_poles = AtPoles::new(zones.iter().map(|z| poles(&z.ext))); let mut out = copy_schema(ds, out_name(p, format!("{}_Erase", ds.name)), ds.geometry_type); @@ -1711,7 +1724,7 @@ fn erase(inp: &dyn Inputs, p: &Json) -> Result { } let n = out.features.len(); let mut r = result(out, format!("Erase: {n} features remain"))?; - r.messages.extend(left_out); + r.messages.extend(no_polygon_note("Erase", &er.name, left_out, NO_POLYGON)); Ok(r) } @@ -1730,45 +1743,36 @@ fn mask_of(poly: MultiPolygon) -> MultiPolygon { if poly.0.len() > 1 { union_all(poly.0.iter()) } else { poly.orient(Direction::Default) } } -/// The polygon layer of an overlay tool's parameter `k` (`overlay`, `erase`, `update`, `split`), -/// which must be polygons, and its (selected) features with a polygon as [`Zone`]s, in order; the -/// others ([`NO_POLYGON`]) are counted for the tool's notice. -fn overlay_zones<'a>(inp: &'a dyn Inputs, p: &Json, k: &str) -> Result, String> { +/// The polygon layer of `tool`'s parameter `k` (`overlay`, `erase`, `update`, `split`), which +/// must be polygons, its (selected) features with a polygon as [`Zone`]s, in order, each once, and +/// how many it left out ([`NO_POLYGON`]), for the tool's notice. Leaving out every one is an error. +fn overlay_zones<'a>(inp: &'a dyn Inputs, p: &Json, k: &str, tool: &str) -> Result<(&'a Dataset, Vec, usize), String> { let ov = input(inp, p, k)?; if ov.geometry_type != GeometryType::Polygon { return Err(format!("'{}' is not a polygon layer", ov.name)); } // A zone of several parts is a union: Cancel is looked at, and progress counted, before each. - let zone_rows = rows(inp, p, k, ov); + let zone_rows = each_once(rows(inp, p, k, ov)); crate::control::begin(zone_rows.len()); let mut zones = Vec::with_capacity(zone_rows.len()); let mut no_polygon = 0usize; for (done, row) in zone_rows.into_iter().enumerate() { crate::control::check()?; crate::control::reached(done); - let f = ov.features.get(row); - // Parts are unioned (see [`mask_of`]): several with no area between them come out empty. - match (polygon_of(f).map(mask_of), f.and_then(|f| f.geometry.as_ref())) { - (Some(poly), Some(g)) if !poly.0.is_empty() => zones.push(Zone { row, ext: g.extent(), poly }), - _ => no_polygon += 1, - } + let Some(g) = ov.features.get(row).and_then(|f| f.geometry.as_ref()) else { + no_polygon += 1; + continue; + }; + let Some(poly) = polygon_of(g) else { + no_polygon += 1; + continue; + }; + zones.push(Zone { row, ext: g.extent(), poly: mask_of(poly) }); } - Ok(OverlayZones { layer: ov, zones, no_polygon }) -} - -/// The overlay polygons [`overlay_zones`] gives, from `layer`, and how many of its features it -/// left out for having no polygon ([`polygon_of`]). -struct OverlayZones<'a> { - layer: &'a Dataset, - zones: Vec, - no_polygon: usize, -} - -impl OverlayZones<'_> { - /// The notice for the features left out, for `tool`'s result. - fn note(&self, tool: &str) -> Option { - no_polygon_note(tool, &self.layer.name, self.no_polygon, NO_POLYGON) + if zones.is_empty() && no_polygon > 0 { + return Err(nothing_covered(tool, &ov.name, no_polygon, NO_POLYGON)); } + Ok((ov, zones, no_polygon)) } /// An overlay tool's output layer: the input's fields, then the overlay's, an overlay field whose @@ -1937,9 +1941,7 @@ fn overlay(inp: &dyn Inputs, p: &Json, op: Overlay) -> Result Result Result Result { if ds.geometry_type != GeometryType::Polygon { return Err(format!("'{}' is not a polygon layer", ds.name)); } - let zs = overlay_zones(inp, p, "update")?; - let left_out = zs.note("Update"); - let OverlayZones { layer: up, zones, .. } = zs; + let (up, zones, left_out) = overlay_zones(inp, p, "update", "Update")?; // Each input field's field in the update layer, if it has one: an ambiguous one fails here, // before any geometry work. let from = ds @@ -2112,7 +2119,7 @@ fn update(inp: &dyn Inputs, p: &Json) -> Result { } let added = out.features.len() - kept; let mut r = result(out, format!("Update: {kept} input features kept, {added} update features added"))?; - r.messages.extend(left_out); + r.messages.extend(no_polygon_note("Update", &up.name, left_out, NO_POLYGON)); Ok(r) } @@ -2151,9 +2158,7 @@ fn split(inp: &dyn Inputs, p: &Json) -> Result { if ds.geometry_type == GeometryType::None { return Err(format!("'{}' has no geometry", ds.name)); } - let zs = overlay_zones(inp, p, "split")?; - let left_out = zs.note("Split"); - let OverlayZones { layer: sp, zones, .. } = zs; + let (sp, zones, left_out) = overlay_zones(inp, p, "split", "Split")?; // A field name, or none; anything else is an error, not one layer per polygon. let field = match p.get("field") { None | Some(Json::Null) => None, @@ -2238,7 +2243,10 @@ fn split(inp: &dyn Inputs, p: &Json) -> Result { let areas = if empty == 1 { "split area" } else { "split areas" }; msg = format!("{msg} ({empty} {areas} held no features)"); } - Ok(ToolResult { output: Output::Layers(layers), messages: std::iter::once(msg).chain(left_out).collect() }) + Ok(ToolResult { + output: Output::Layers(layers), + messages: std::iter::once(msg).chain(no_polygon_note("Split", &sp.name, left_out, NO_POLYGON)).collect(), + }) } /// Dissolve: one feature for each value of the dissolve field (one for all without it), the union @@ -2268,7 +2276,7 @@ fn dissolve(inp: &dyn Inputs, p: &Json) -> Result { for (done, r) in input_rows.into_iter().enumerate() { crate::control::check()?; crate::control::reached(done); - let Some(mp) = polygon_of(ds.features.get(r)) else { + let Some(mp) = ds.features.get(r).and_then(|f| f.geometry.as_ref()).and_then(polygon_of) else { not_polygons += 1; continue; }; @@ -2482,9 +2490,9 @@ fn convex_hull(inp: &dyn Inputs, p: &Json) -> Result { GeometryType::Point => "the output is a point layer".to_string(), GeometryType::Polyline => "the output is a line layer".to_string(), // Points and lines, every hull flat: a polygon layer, there being no one kind. - _ if flat >= n => "they are kept in the output's polygon layer, points and lines together, where Dissolve, Clip and the overlay tools leave them out, saying how many".to_string(), + _ if flat >= n => "they are kept in the output's polygon layer, points and lines together, where Dissolve leaves them out, as do Clip and the overlay tools from a mask or overlay layer, saying how many".to_string(), _ => format!( - "they are kept in the output's polygon layer, among its {} polygon(s), where Dissolve, Clip and the overlay tools leave them out, saying how many", + "they are kept in the output's polygon layer, among its {} polygon(s), where Dissolve leaves them out, as do Clip and the overlay tools from a mask or overlay layer, saying how many", n.saturating_sub(flat) ), }; @@ -6938,9 +6946,10 @@ mod tests { /// Clip and the overlay tools say how many mask or overlay features they left out as covering /// nothing: a point and a line in a polygon layer (as Minimum Bounding Geometry's flat hulls - /// are), empty polygons, a feature without geometry and one of two parts with no area, and for - /// Clip a single polygon with no area too. Those change nothing in the output; a layer of - /// polygons says nothing of the kind, and only the selected features are counted. + /// are), empty polygons and a feature without geometry; Clip also a polygon with no area, and + /// Union and Symmetrical Difference one of two parts with no area. Those change nothing in the + /// output; a layer of polygons says nothing of the kind; only the selected features are + /// counted, each once; and leaving out every one is an error. #[test] fn overlay_tools_count_mask_features_that_cover_nothing() { struct Sel(Map, Option>); @@ -6970,7 +6979,10 @@ mod tests { masks.push(Some(Geometry::Polygon(vec![])), vec![Value::Int(13)]); masks.push(Some(Geometry::Polygon(vec![vec![]])), vec![Value::Int(14)]); masks.push(None, vec![Value::Int(15)]); - masks.push(Some(Geometry::Polygon(vec![vec![flat(1.0)], vec![flat(1.5)]])), vec![Value::Int(16)]); + // Update writes an update polygon as it is, so one with no area changes its output. + if tool != "update" { + masks.push(Some(Geometry::Polygon(vec![vec![flat(1.0)], vec![flat(1.5)]])), vec![Value::Int(16)]); + } if tool == "clip" { masks.push(Some(Geometry::Polygon(vec![vec![flat(1.0)]])), vec![Value::Int(17)]); } @@ -7001,24 +7013,32 @@ mod tests { ("update", "update", "Update"), ("split", "split", "Split"), ]; - let note = |name: &str, n: usize| { - let what = if name == "Clip" { - "no geometry, a point or line, or a polygon with no area" - } else { - "no geometry, a point or line, an empty polygon, or several parts with no area" - }; - format!("{name}: {n} feature(s) of 'masks' left out, covering nothing: {what}") - }; + let note = |name: &str, n: usize, what: &str| format!("{name}: {n} feature(s) of 'masks' left out, covering nothing: {what}"); for (tool, key, name) in tools { let (clean, none) = run(tool, key, false, None); assert!(none.is_empty(), "{tool}: {none:?}"); let (shapes, notes) = run(tool, key, true, None); assert_eq!(shapes, clean, "{tool}: the features covering nothing change nothing"); - assert_eq!(notes, [note(name, if tool == "clip" { 7 } else { 6 })], "{tool}"); - // The square and the point and line selected: only those two are counted. - let (shapes, notes) = run(tool, key, true, Some(vec![0, 1, 2])); + let expected = match tool { + "clip" => vec![note(name, 7, NO_MASK)], + "union" | "symmetricalDifference" => vec![note(name, 5, NO_POLYGON), note(name, 1, NO_AREA)], + _ => vec![note(name, 5, NO_POLYGON)], + }; + assert_eq!(notes, expected, "{tool}"); + // The square, and the point and line twice over, selected: those two are counted once. + let (shapes, notes) = run(tool, key, true, Some(vec![0, 1, 2, 2, 1])); assert_eq!(shapes, clean, "{tool}: selected"); - assert_eq!(notes, [note(name, 2)], "{tool}: selected"); + let what = if tool == "clip" { NO_MASK } else { NO_POLYGON }; + assert_eq!(notes, [note(name, 2, what)], "{tool}: selected"); + // The point and line alone: nothing would be clipped or cut by, an error saying why. + let mut points = Dataset::new("masks", GeometryType::Polygon, vec![]); + points.push(Some(Geometry::Point(Coord::new(6.0, 1.0))), vec![]); + points.push(None, vec![]); + let mut polys = Dataset::new("polys", GeometryType::Polygon, vec![]); + polys.push(Some(sq(0.0, 0.0, 2.0)), vec![]); + let m = Map(HashMap::from([("polys".to_string(), polys), ("masks".to_string(), points)])); + let e = crate::run(tool, &m, &serde_json::json!({"input": "polys", key: "masks"})).err().unwrap_or_default(); + assert_eq!(e, format!("{name}: none of the 2 feature(s) of 'masks' covers anything: {what}"), "{tool}"); } } @@ -10333,6 +10353,8 @@ mod tests { .is_some() ); for selection in [None, Some((0..80).filter(|i| i % 3 != 0).collect()), Some(vec![82]), Some(vec![83])] { + // Mask 83 has no geometry. + let without_geometry = selection == Some(vec![83]); let mut selected = HashMap::new(); if let Some(s) = selection { selected.insert("masks".into(), s); @@ -10340,6 +10362,12 @@ mod tests { } let inp = Selected(indexed_overlay_inputs(), selected); let p = serde_json::json!({"input":"features", "clip":"masks"}); + if without_geometry { + // Nothing to clip by: an error saying why, not an empty layer. + let e = crate::run("clip", &inp, &p).err().unwrap_or_default(); + assert!(e.contains("none of the 1 feature(s) of 'masks' covers anything"), "{e}"); + continue; + } let masks = &inp.0.0["masks"]; let polys: Vec<_> = rows(&inp, &p, "clip", masks) .iter() diff --git a/docs/user-guide/analysis.md b/docs/user-guide/analysis.md index b5c944af2..3c9296e50 100644 --- a/docs/user-guide/analysis.md +++ b/docs/user-guide/analysis.md @@ -614,8 +614,10 @@ gp.run={"tool":"split","params":{"input":"roads","split":"districts","field":"na Clip, Erase, Intersect, Identity, Union, Symmetrical Difference, Update and Split leave out a feature of the clip, erase, overlay, update or split layer that covers nothing, and say how many: one without geometry, a point or line in the polygon layer (as Minimum Bounding Geometry's flat -hulls are), an empty polygon, or a polygon with no area. Clip leaves out any polygon with no area; -the others leave out one whose parts, two or more, all have no area, and keep a single such part. +hulls are), or an empty polygon. Clip also leaves out a polygon with no area, and Union and +Symmetrical Difference an overlay feature whose parts, two or more, all have no area. When every +feature of that layer is left out, the tool stops with an error rather than give an empty layer or +the input unchanged. **Near** (`near`) copies `input` and adds, for each feature, the nearest feature of `near`: its OID as the attribute table shows it (`near_fid`), the distance to it (`near_dist`, in `units`, From a297c57097181019191077713ceb40e4c04bab99 Mon Sep 17 00:00:00 2001 From: Cory Douglas Martin <4108748+flight1973@users.noreply.github.com> Date: Mon, 9 Nov 2026 17:13:13 -0600 Subject: [PATCH 03/11] fix(analysis): the overlay tools leave out polygons with no area, as Clip does Review follow-up. An erase, overlay or split polygon with no area, one part or several, covers nothing, yet Erase, Intersect, Identity and Split kept it as a zone without a word, and Union and Symmetrical Difference wrote a single one out. overlay_zones now leaves these out and counts them, with a notice of their own, and counts them toward the error when every feature is left out. Update keeps them, writing its update polygons as they are. Minimum Bounding Geometry uses each_once for its rows, and the cancel test names each tool with its parameters, so the two convexHull runs are told apart. --- crates/analysis/src/tools.rs | 113 +++++++++++++++++++++++------------ crates/engine/src/tests.rs | 3 +- docs/user-guide/analysis.md | 7 +-- 3 files changed, 79 insertions(+), 44 deletions(-) diff --git a/crates/analysis/src/tools.rs b/crates/analysis/src/tools.rs index 58435c72e..268adfef3 100644 --- a/crates/analysis/src/tools.rs +++ b/crates/analysis/src/tools.rs @@ -1306,9 +1306,26 @@ const NO_POLYGON: &str = "no geometry, a point or line, or an empty polygon"; /// What a mask Clip leaves out holds: [`NO_POLYGON`], or polygons whose union has no area. const NO_MASK: &str = "no geometry, a point or line, or a polygon with no area"; -/// What an overlay feature Union and Symmetrical Difference leave out of their output holds, -/// though it is a zone: parts whose union (see [`mask_of`]) has no area. -const NO_AREA: &str = "two or more polygon parts, all with no area"; +/// What an overlay feature [`overlay_zones`] leaves out for having no area holds. +const NO_AREA: &str = "a polygon with no area"; + +/// How many features of an overlay tool's mask or overlay layer [`overlay_zones`] left out: those +/// with no polygon ([`NO_POLYGON`]), and those whose polygons have no area ([`NO_AREA`]). +#[derive(Clone, Copy)] +struct LeftOut { + no_polygon: usize, + no_area: usize, +} + +impl LeftOut { + /// `tool`'s notices for them, one for each kind there are any of, `layer` being their layer. + fn notes(self, tool: &str, layer: &str) -> impl Iterator { + let (tool, layer) = (tool.to_string(), layer.to_string()); + [(self.no_polygon, NO_POLYGON), (self.no_area, NO_AREA)] + .into_iter() + .filter_map(move |(n, what)| no_polygon_note(&tool, &layer, n, what)) + } +} /// A copy of a feature's attributes for an output with the same schema. fn copy_schema(ds: &Dataset, name: String, g: GeometryType) -> Dataset { @@ -1707,7 +1724,7 @@ fn erase(inp: &dyn Inputs, p: &Json) -> Result { if ds.geometry_type == GeometryType::None { return Err(format!("'{}' has no geometry", ds.name)); } - let (er, zones, left_out) = overlay_zones(inp, p, "erase", "Erase")?; + let (er, zones, left_out) = overlay_zones(inp, p, "erase", "Erase", false)?; let index = ExtentIndex::new(zones.iter().map(|z| z.ext)); let at_poles = AtPoles::new(zones.iter().map(|z| poles(&z.ext))); let mut out = copy_schema(ds, out_name(p, format!("{}_Erase", ds.name)), ds.geometry_type); @@ -1724,7 +1741,7 @@ fn erase(inp: &dyn Inputs, p: &Json) -> Result { } let n = out.features.len(); let mut r = result(out, format!("Erase: {n} features remain"))?; - r.messages.extend(no_polygon_note("Erase", &er.name, left_out, NO_POLYGON)); + r.messages.extend(left_out.notes("Erase", &er.name)); Ok(r) } @@ -1745,8 +1762,16 @@ fn mask_of(poly: MultiPolygon) -> MultiPolygon { /// The polygon layer of `tool`'s parameter `k` (`overlay`, `erase`, `update`, `split`), which /// must be polygons, its (selected) features with a polygon as [`Zone`]s, in order, each once, and -/// how many it left out ([`NO_POLYGON`]), for the tool's notice. Leaving out every one is an error. -fn overlay_zones<'a>(inp: &'a dyn Inputs, p: &Json, k: &str, tool: &str) -> Result<(&'a Dataset, Vec, usize), String> { +/// how many it left out, for the tool's notices: those with no polygon, and those whose polygons +/// have no area, as they cover nothing, unless `keep_no_area` (Update writes its update polygons +/// as they are). Leaving out every one is an error. +fn overlay_zones<'a>( + inp: &'a dyn Inputs, + p: &Json, + k: &str, + tool: &str, + keep_no_area: bool, +) -> Result<(&'a Dataset, Vec, LeftOut), String> { let ov = input(inp, p, k)?; if ov.geometry_type != GeometryType::Polygon { return Err(format!("'{}' is not a polygon layer", ov.name)); @@ -1755,24 +1780,32 @@ fn overlay_zones<'a>(inp: &'a dyn Inputs, p: &Json, k: &str, tool: &str) -> Resu let zone_rows = each_once(rows(inp, p, k, ov)); crate::control::begin(zone_rows.len()); let mut zones = Vec::with_capacity(zone_rows.len()); - let mut no_polygon = 0usize; + let mut left = LeftOut { no_polygon: 0, no_area: 0 }; for (done, row) in zone_rows.into_iter().enumerate() { crate::control::check()?; crate::control::reached(done); let Some(g) = ov.features.get(row).and_then(|f| f.geometry.as_ref()) else { - no_polygon += 1; + left.no_polygon += 1; continue; }; let Some(poly) = polygon_of(g) else { - no_polygon += 1; + left.no_polygon += 1; continue; }; - zones.push(Zone { row, ext: g.extent(), poly: mask_of(poly) }); + // No area: rings on a line, or parts whose union (several are unioned) is empty. + let flat = geo::Area::unsigned_area(&poly) == 0.0; + let poly = mask_of(poly); + if !keep_no_area && (flat || poly.0.is_empty()) { + left.no_area += 1; + continue; + } + zones.push(Zone { row, ext: g.extent(), poly }); } - if zones.is_empty() && no_polygon > 0 { - return Err(nothing_covered(tool, &ov.name, no_polygon, NO_POLYGON)); + let n = left.no_polygon + left.no_area; + if zones.is_empty() && n > 0 { + return Err(nothing_covered(tool, &ov.name, n, if left.no_area > 0 { NO_MASK } else { NO_POLYGON })); } - Ok((ov, zones, no_polygon)) + Ok((ov, zones, left)) } /// An overlay tool's output layer: the input's fields, then the overlay's, an overlay field whose @@ -1941,7 +1974,7 @@ fn overlay(inp: &dyn Inputs, p: &Json, op: Overlay) -> Result Result Result Result { if ds.geometry_type != GeometryType::Polygon { return Err(format!("'{}' is not a polygon layer", ds.name)); } - let (up, zones, left_out) = overlay_zones(inp, p, "update", "Update")?; + let (up, zones, left_out) = overlay_zones(inp, p, "update", "Update", true)?; // Each input field's field in the update layer, if it has one: an ambiguous one fails here, // before any geometry work. let from = ds @@ -2119,7 +2145,7 @@ fn update(inp: &dyn Inputs, p: &Json) -> Result { } let added = out.features.len() - kept; let mut r = result(out, format!("Update: {kept} input features kept, {added} update features added"))?; - r.messages.extend(no_polygon_note("Update", &up.name, left_out, NO_POLYGON)); + r.messages.extend(left_out.notes("Update", &up.name)); Ok(r) } @@ -2158,7 +2184,7 @@ fn split(inp: &dyn Inputs, p: &Json) -> Result { if ds.geometry_type == GeometryType::None { return Err(format!("'{}' has no geometry", ds.name)); } - let (sp, zones, left_out) = overlay_zones(inp, p, "split", "Split")?; + let (sp, zones, left_out) = overlay_zones(inp, p, "split", "Split", false)?; // A field name, or none; anything else is an error, not one layer per polygon. let field = match p.get("field") { None | Some(Json::Null) => None, @@ -2243,10 +2269,7 @@ fn split(inp: &dyn Inputs, p: &Json) -> Result { let areas = if empty == 1 { "split area" } else { "split areas" }; msg = format!("{msg} ({empty} {areas} held no features)"); } - Ok(ToolResult { - output: Output::Layers(layers), - messages: std::iter::once(msg).chain(no_polygon_note("Split", &sp.name, left_out, NO_POLYGON)).collect(), - }) + Ok(ToolResult { output: Output::Layers(layers), messages: std::iter::once(msg).chain(left_out.notes("Split", &sp.name)).collect() }) } /// Dissolve: one feature for each value of the dissolve field (one for all without it), the union @@ -2375,8 +2398,7 @@ fn convex_hull(inp: &dyn Inputs, p: &Json) -> Result { (these, gaps, poles) }; // Each row once, however often a selection lists it, in the selection's order. - let mut seen = std::collections::HashSet::new(); - let picked: Vec = rows(inp, p, "input", ds).into_iter().filter(|r| seen.insert(*r)).collect(); + let picked = each_once(rows(inp, p, "input", ds)); // The features with a geometry, each with it; the rest are left out, and counted. let features: Vec<(&Feature, &Geometry)> = picked.iter().filter_map(|&r| ds.features.get(r).and_then(|f| Some((f, f.geometry.as_ref()?)))).collect(); @@ -6946,9 +6968,9 @@ mod tests { /// Clip and the overlay tools say how many mask or overlay features they left out as covering /// nothing: a point and a line in a polygon layer (as Minimum Bounding Geometry's flat hulls - /// are), empty polygons and a feature without geometry; Clip also a polygon with no area, and - /// Union and Symmetrical Difference one of two parts with no area. Those change nothing in the - /// output; a layer of polygons says nothing of the kind; only the selected features are + /// are), empty polygons and a feature without geometry, and polygons with no area (one part or + /// two), all but Update, which writes its update polygons as they are. Those change nothing in + /// the output; a layer of polygons says nothing of the kind; only the selected features are /// counted, each once; and leaving out every one is an error. #[test] fn overlay_tools_count_mask_features_that_cover_nothing() { @@ -6982,8 +7004,6 @@ mod tests { // Update writes an update polygon as it is, so one with no area changes its output. if tool != "update" { masks.push(Some(Geometry::Polygon(vec![vec![flat(1.0)], vec![flat(1.5)]])), vec![Value::Int(16)]); - } - if tool == "clip" { masks.push(Some(Geometry::Polygon(vec![vec![flat(1.0)]])), vec![Value::Int(17)]); } } @@ -7021,8 +7041,8 @@ mod tests { assert_eq!(shapes, clean, "{tool}: the features covering nothing change nothing"); let expected = match tool { "clip" => vec![note(name, 7, NO_MASK)], - "union" | "symmetricalDifference" => vec![note(name, 5, NO_POLYGON), note(name, 1, NO_AREA)], - _ => vec![note(name, 5, NO_POLYGON)], + "update" => vec![note(name, 5, NO_POLYGON)], + _ => vec![note(name, 5, NO_POLYGON), note(name, 2, NO_AREA)], }; assert_eq!(notes, expected, "{tool}"); // The square, and the point and line twice over, selected: those two are counted once. @@ -7039,6 +7059,21 @@ mod tests { let m = Map(HashMap::from([("polys".to_string(), polys), ("masks".to_string(), points)])); let e = crate::run(tool, &m, &serde_json::json!({"input": "polys", key: "masks"})).err().unwrap_or_default(); assert_eq!(e, format!("{name}: none of the 2 feature(s) of 'masks' covers anything: {what}"), "{tool}"); + // A polygon with no area alone: the same, but Update writes it, as it is, saying nothing. + let mut flats = Dataset::new("masks", GeometryType::Polygon, vec![]); + flats.push(Some(Geometry::Polygon(vec![vec![flat(1.0)]])), vec![]); + let mut polys = Dataset::new("polys", GeometryType::Polygon, vec![]); + polys.push(Some(sq(0.0, 0.0, 2.0)), vec![]); + let m = Map(HashMap::from([("polys".to_string(), polys), ("masks".to_string(), flats)])); + let res = crate::run(tool, &m, &serde_json::json!({"input": "polys", key: "masks"})); + if tool == "update" { + let res = res.unwrap_or_else(|e| panic!("{e}")); + assert!(res.messages.iter().all(|m| !m.contains("covering nothing")), "{:?}", res.messages); + assert_eq!(layer(res).features.len(), 2); + } else { + let e = res.err().unwrap_or_default(); + assert_eq!(e, format!("{name}: none of the 1 feature(s) of 'masks' covers anything: {NO_MASK}"), "{tool}"); + } } } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index e51f0157f..dd4e64b80 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -37112,7 +37112,8 @@ fn long_commands_stop_when_cancelled_while_they_run() { let done = run.run(&control); // The result is kept, then `gp.finish` applies it (or says why it can't), as in the app. let finished = crate::cmd::gp::tool_complete(&mut s, id, done).and_then(|()| s.run("gp.finish", &json!({"id": id})).map(|_| ())); - note(tool, finished, &control); + // With its parameters: a tool listed twice (convexHull) is told apart. + note(&format!("{tool} {params}"), finished, &control); } assert!(kept_going.is_empty(), "these ran on after Cancel:\n{}", kept_going.join("\n")); } diff --git a/docs/user-guide/analysis.md b/docs/user-guide/analysis.md index 3c9296e50..a0c04122c 100644 --- a/docs/user-guide/analysis.md +++ b/docs/user-guide/analysis.md @@ -614,10 +614,9 @@ gp.run={"tool":"split","params":{"input":"roads","split":"districts","field":"na Clip, Erase, Intersect, Identity, Union, Symmetrical Difference, Update and Split leave out a feature of the clip, erase, overlay, update or split layer that covers nothing, and say how many: one without geometry, a point or line in the polygon layer (as Minimum Bounding Geometry's flat -hulls are), or an empty polygon. Clip also leaves out a polygon with no area, and Union and -Symmetrical Difference an overlay feature whose parts, two or more, all have no area. When every -feature of that layer is left out, the tool stops with an error rather than give an empty layer or -the input unchanged. +hulls are), an empty polygon, or a polygon with no area (Update keeps one with no area, writing its +update polygons as they are). When every feature of that layer is left out, the tool stops with an +error rather than give an empty layer or the input unchanged. **Near** (`near`) copies `input` and adds, for each feature, the nearest feature of `near`: its OID as the attribute table shows it (`near_fid`), the distance to it (`near_dist`, in `units`, From 6e6929f60ad25134bf8debc74adf6230e808d98b Mon Sep 17 00:00:00 2001 From: Cory Douglas Martin <4108748+flight1973@users.noreply.github.com> Date: Mon, 9 Nov 2026 17:40:22 -0600 Subject: [PATCH 04/11] fix(analysis): no area means an empty union; the error only where nothing would come out Review follow-up. The overlay tools judged a polygon to have no area by its shoelace area, which is zero for a bowtie that covers ground, so they left such a polygon out where Clip keeps it. They now keep every polygon as a zone, as before; Union and Symmetrical Difference count an overlay feature whose parts union to nothing, as nothing of it would be written, and Clip counts a mask whose union is empty, each in a notice of its own, apart from the features with no polygon. Leaving out every feature is an error only for Clip, Intersect and Split, whose result would be empty; Erase, Identity, Union, Symmetrical Difference and Update give the input, with the notice, as before. The error says how many of each kind. --- crates/analysis/src/tools.rs | 199 +++++++++++++++++++---------------- docs/user-guide/analysis.md | 10 +- 2 files changed, 113 insertions(+), 96 deletions(-) diff --git a/crates/analysis/src/tools.rs b/crates/analysis/src/tools.rs index 268adfef3..07fb82de8 100644 --- a/crates/analysis/src/tools.rs +++ b/crates/analysis/src/tools.rs @@ -1293,24 +1293,27 @@ fn no_polygon_note(tool: &str, layer: &str, n: usize, what: &str) -> Option 0).then(|| format!("{tool}: {n} feature(s) of '{layer}' left out, covering nothing: {what}")) } -/// The error when `tool` would leave out every one of the `n` features of `layer` it was given -/// (`what` lists what they hold): its output would be empty, or its input unchanged. -fn nothing_covered(tool: &str, layer: &str, n: usize, what: &str) -> String { - format!("{tool}: none of the {n} feature(s) of '{layer}' covers anything: {what}") +/// The error when `tool`, whose output would be empty, left out every feature of `layer` it was +/// given, `left` saying how many of each kind. +fn nothing_covered(tool: &str, layer: &str, left: LeftOut) -> String { + let what = match (left.no_polygon, left.no_area) { + (_, 0) => NO_POLYGON.to_string(), + (0, _) => NO_AREA.to_string(), + (a, b) => format!("{a} with no polygon ({NO_POLYGON}), {b} {NO_AREA}"), + }; + format!("{tool}: none of the {} feature(s) of '{layer}' covers anything: {what}", left.no_polygon + left.no_area) } /// What a feature [`overlay_zones`] leaves out holds: no geometry, or a geometry [`polygon_of`] /// gives nothing for. const NO_POLYGON: &str = "no geometry, a point or line, or an empty polygon"; -/// What a mask Clip leaves out holds: [`NO_POLYGON`], or polygons whose union has no area. -const NO_MASK: &str = "no geometry, a point or line, or a polygon with no area"; - -/// What an overlay feature [`overlay_zones`] leaves out for having no area holds. +/// What a feature whose polygons, unioned, have no area holds: Clip leaves out such a mask, and +/// Union and Symmetrical Difference such an overlay feature, as nothing of it would be written. const NO_AREA: &str = "a polygon with no area"; -/// How many features of an overlay tool's mask or overlay layer [`overlay_zones`] left out: those -/// with no polygon ([`NO_POLYGON`]), and those whose polygons have no area ([`NO_AREA`]). +/// How many features of a mask or overlay layer Clip or an overlay tool left out: those with no +/// polygon ([`NO_POLYGON`]), and those whose polygons have no area ([`NO_AREA`]). #[derive(Clone, Copy)] struct LeftOut { no_polygon: usize, @@ -1319,11 +1322,11 @@ struct LeftOut { impl LeftOut { /// `tool`'s notices for them, one for each kind there are any of, `layer` being their layer. - fn notes(self, tool: &str, layer: &str) -> impl Iterator { - let (tool, layer) = (tool.to_string(), layer.to_string()); + fn notes(self, tool: &str, layer: &str) -> Vec { [(self.no_polygon, NO_POLYGON), (self.no_area, NO_AREA)] .into_iter() - .filter_map(move |(n, what)| no_polygon_note(&tool, &layer, n, what)) + .filter_map(|(n, what)| no_polygon_note(tool, layer, n, what)) + .collect() } } @@ -1632,7 +1635,7 @@ fn clip(inp: &dyn Inputs, p: &Json) -> Result { } let mut masks = Vec::new(); // Mask features that clip nothing, left out and counted: no polygon, or one with no area. - let mut no_mask = 0usize; + let mut left = LeftOut { no_polygon: 0, no_area: 0 }; let mask_rows = each_once(rows(inp, p, "clip", clip_ds)); crate::control::begin(mask_rows.len()); for (done, r) in mask_rows.into_iter().enumerate() { @@ -1640,7 +1643,7 @@ fn clip(inp: &dyn Inputs, p: &Json) -> Result { crate::control::check()?; crate::control::reached(done); let Some(m) = clip_ds.features.get(r).and_then(|f| f.geometry.as_ref()).and_then(polygon_of) else { - no_mask += 1; + left.no_polygon += 1; continue; }; let ext = merc_extent(&m); @@ -1649,13 +1652,13 @@ fn clip(inp: &dyn Inputs, p: &Json) -> Result { let mask = union_all(m.0.iter()); let ext = ext.union(&merc_extent(&mask)); if mask.0.is_empty() { - no_mask += 1; + left.no_area += 1; } else { masks.push((mask, ext)); } } - if masks.is_empty() && no_mask > 0 { - return Err(nothing_covered("Clip", &clip_ds.name, no_mask, NO_MASK)); + if masks.is_empty() && left.no_polygon + left.no_area > 0 { + return Err(nothing_covered("Clip", &clip_ds.name, left)); } let index = ExtentIndex::new(masks.iter().map(|(_, ext)| *ext)); // The masks reaching each pole: a point at one is in them at any longitude (#1033). @@ -1711,7 +1714,7 @@ fn clip(inp: &dyn Inputs, p: &Json) -> Result { } let n = out.features.len(); let mut r = result(out, format!("Clip: {n} features kept"))?; - r.messages.extend(no_polygon_note("Clip", &clip_ds.name, no_mask, NO_MASK)); + r.messages.extend(left.notes("Clip", &clip_ds.name)); Ok(r) } @@ -1762,15 +1765,14 @@ fn mask_of(poly: MultiPolygon) -> MultiPolygon { /// The polygon layer of `tool`'s parameter `k` (`overlay`, `erase`, `update`, `split`), which /// must be polygons, its (selected) features with a polygon as [`Zone`]s, in order, each once, and -/// how many it left out, for the tool's notices: those with no polygon, and those whose polygons -/// have no area, as they cover nothing, unless `keep_no_area` (Update writes its update polygons -/// as they are). Leaving out every one is an error. +/// how many it left out with no polygon ([`NO_POLYGON`]), for the tool's notices. Leaving out +/// every one is an error when `needs_one`: the tool's output would be empty (Intersect, Split). fn overlay_zones<'a>( inp: &'a dyn Inputs, p: &Json, k: &str, tool: &str, - keep_no_area: bool, + needs_one: bool, ) -> Result<(&'a Dataset, Vec, LeftOut), String> { let ov = input(inp, p, k)?; if ov.geometry_type != GeometryType::Polygon { @@ -1792,18 +1794,10 @@ fn overlay_zones<'a>( left.no_polygon += 1; continue; }; - // No area: rings on a line, or parts whose union (several are unioned) is empty. - let flat = geo::Area::unsigned_area(&poly) == 0.0; - let poly = mask_of(poly); - if !keep_no_area && (flat || poly.0.is_empty()) { - left.no_area += 1; - continue; - } - zones.push(Zone { row, ext: g.extent(), poly }); + zones.push(Zone { row, ext: g.extent(), poly: mask_of(poly) }); } - let n = left.no_polygon + left.no_area; - if zones.is_empty() && n > 0 { - return Err(nothing_covered(tool, &ov.name, n, if left.no_area > 0 { NO_MASK } else { NO_POLYGON })); + if needs_one && zones.is_empty() && left.no_polygon > 0 { + return Err(nothing_covered(tool, &ov.name, left)); } Ok((ov, zones, left)) } @@ -1974,7 +1968,7 @@ fn overlay(inp: &dyn Inputs, p: &Json, op: Overlay) -> Result Result Value { /// exactly, else ignoring case; converted by [`converted`]), the input's other fields empty. The /// output has the input's fields only. Input features without geometry, and those no update /// polygon overlaps, are kept as they are; update features without a polygon ([`NO_POLYGON`]) are -/// left out, saying how many, and update polygons that overlap each other each come out whole. An input field that no update -/// field matches exactly, but two match ignoring case, is an error rather than a guess. +/// left out, saying how many, and update polygons that overlap each other each come out whole. An +/// input field that no update field matches exactly, but two match ignoring case, is an error +/// rather than a guess. fn update(inp: &dyn Inputs, p: &Json) -> Result { let ds = input(inp, p, "input")?; if ds.geometry_type != GeometryType::Polygon { return Err(format!("'{}' is not a polygon layer", ds.name)); } - let (up, zones, left_out) = overlay_zones(inp, p, "update", "Update", true)?; + let (up, zones, left_out) = overlay_zones(inp, p, "update", "Update", false)?; // Each input field's field in the update layer, if it has one: an ambiguous one fails here, // before any geometry work. let from = ds @@ -2184,7 +2184,7 @@ fn split(inp: &dyn Inputs, p: &Json) -> Result { if ds.geometry_type == GeometryType::None { return Err(format!("'{}' has no geometry", ds.name)); } - let (sp, zones, left_out) = overlay_zones(inp, p, "split", "Split", false)?; + let (sp, zones, left_out) = overlay_zones(inp, p, "split", "Split", true)?; // A field name, or none; anything else is an error, not one layer per polygon. let field = match p.get("field") { None | Some(Json::Null) => None, @@ -6968,10 +6968,11 @@ mod tests { /// Clip and the overlay tools say how many mask or overlay features they left out as covering /// nothing: a point and a line in a polygon layer (as Minimum Bounding Geometry's flat hulls - /// are), empty polygons and a feature without geometry, and polygons with no area (one part or - /// two), all but Update, which writes its update polygons as they are. Those change nothing in - /// the output; a layer of polygons says nothing of the kind; only the selected features are - /// counted, each once; and leaving out every one is an error. + /// are), empty polygons and a feature without geometry. Clip also leaves out a polygon with no + /// area, and Union and Symmetrical Difference a feature of two such parts, which nothing of + /// would be written. Those change nothing in the output; a layer of polygons, a bowtie among + /// them, says nothing of the kind; only the selected features are counted, each once; and + /// leaving out every one is an error where the output would be empty. #[test] fn overlay_tools_count_mask_features_that_cover_nothing() { struct Sel(Map, Option>); @@ -6988,10 +6989,35 @@ mod tests { } // On one parallel: a straight line on the analysis plane too. let flat = |y: f64| vec![Coord::new(5.0, y), Coord::new(6.0, y), Coord::new(7.0, y), Coord::new(5.0, y)]; - let run = |tool: &str, key: &str, extra: bool, selected: Option>| { + // Two triangles meeting at a point, wound opposite ways: its signed area sums to zero. + let bowtie = || { + let c = Coord::new; + Geometry::Polygon(vec![vec![vec![c(5.0, 0.0), c(7.0, 2.0), c(7.0, 0.0), c(5.0, 2.0), c(5.0, 0.0)]]]) + }; + let polys = || { let mut polys = Dataset::new("polys", GeometryType::Polygon, vec![Field::new("a", FieldType::Integer)]); polys.push(Some(sq(0.0, 0.0, 2.0)), vec![Value::Int(1)]); polys.push(Some(sq(5.0, 0.0, 2.0)), vec![Value::Int(2)]); + polys + }; + let run = |tool: &str, key: &str, masks: Dataset, selected: Option>| { + let m = Sel(Map(HashMap::from([("polys".to_string(), polys()), ("masks".to_string(), masks)])), selected); + let res = crate::run(tool, &m, &serde_json::json!({"input": "polys", key: "masks"}))?; + let notes: Vec = res.messages.iter().filter(|m| m.contains("covering nothing")).cloned().collect(); + let shapes: Vec> = match res.output { + Output::Layer(ds) => ds.features.into_iter().map(|f| f.geometry).collect(), + Output::Layers(layers) => layers + .into_iter() + .flat_map(|l| match l { + OutputLayer::Vector(ds) => ds.features.into_iter().map(|f| f.geometry).collect(), + _ => Vec::new(), + }) + .collect(), + _ => panic!("{tool}: a layer"), + }; + Ok::<_, String>((shapes, notes)) + }; + let masks = |extra: bool, tool: &str| { let mut masks = Dataset::new("masks", GeometryType::Polygon, vec![Field::new("b", FieldType::Integer)]); masks.push(Some(sq(1.0, 1.0, 2.0)), vec![Value::Int(10)]); if extra { @@ -7001,27 +7027,17 @@ mod tests { masks.push(Some(Geometry::Polygon(vec![])), vec![Value::Int(13)]); masks.push(Some(Geometry::Polygon(vec![vec![]])), vec![Value::Int(14)]); masks.push(None, vec![Value::Int(15)]); - // Update writes an update polygon as it is, so one with no area changes its output. + // Update writes an update polygon as it is; Union and Symmetrical Difference + // write one part with no area as it is too. if tool != "update" { + // Two polygons, each a ring on a line. masks.push(Some(Geometry::Polygon(vec![vec![flat(1.0)], vec![flat(1.5)]])), vec![Value::Int(16)]); + } + if tool == "clip" { masks.push(Some(Geometry::Polygon(vec![vec![flat(1.0)]])), vec![Value::Int(17)]); } } - let m = Sel(Map(HashMap::from([("polys".to_string(), polys), ("masks".to_string(), masks)])), selected); - let res = crate::run(tool, &m, &serde_json::json!({"input": "polys", key: "masks"})).unwrap_or_else(|e| panic!("{tool}: {e}")); - let notes: Vec = res.messages.iter().filter(|m| m.contains("covering nothing")).cloned().collect(); - let shapes: Vec> = match res.output { - Output::Layer(ds) => ds.features.into_iter().map(|f| f.geometry).collect(), - Output::Layers(layers) => layers - .into_iter() - .flat_map(|l| match l { - OutputLayer::Vector(ds) => ds.features.into_iter().map(|f| f.geometry).collect(), - _ => Vec::new(), - }) - .collect(), - _ => panic!("{tool}: a layer"), - }; - (shapes, notes) + masks }; let tools = [ ("clip", "clip", "Clip"), @@ -7035,46 +7051,47 @@ mod tests { ]; let note = |name: &str, n: usize, what: &str| format!("{name}: {n} feature(s) of 'masks' left out, covering nothing: {what}"); for (tool, key, name) in tools { - let (clean, none) = run(tool, key, false, None); + let (clean, none) = run(tool, key, masks(false, tool), None).unwrap_or_else(|e| panic!("{tool}: {e}")); assert!(none.is_empty(), "{tool}: {none:?}"); - let (shapes, notes) = run(tool, key, true, None); + let (shapes, notes) = run(tool, key, masks(true, tool), None).unwrap_or_else(|e| panic!("{tool}: {e}")); assert_eq!(shapes, clean, "{tool}: the features covering nothing change nothing"); let expected = match tool { - "clip" => vec![note(name, 7, NO_MASK)], - "update" => vec![note(name, 5, NO_POLYGON)], - _ => vec![note(name, 5, NO_POLYGON), note(name, 2, NO_AREA)], + "clip" => vec![note(name, 5, NO_POLYGON), note(name, 2, NO_AREA)], + "union" | "symmetricalDifference" => vec![note(name, 5, NO_POLYGON), note(name, 1, NO_AREA)], + _ => vec![note(name, 5, NO_POLYGON)], }; assert_eq!(notes, expected, "{tool}"); // The square, and the point and line twice over, selected: those two are counted once. - let (shapes, notes) = run(tool, key, true, Some(vec![0, 1, 2, 2, 1])); + let (shapes, notes) = run(tool, key, masks(true, tool), Some(vec![0, 1, 2, 2, 1])).unwrap_or_else(|e| panic!("{tool}: {e}")); assert_eq!(shapes, clean, "{tool}: selected"); - let what = if tool == "clip" { NO_MASK } else { NO_POLYGON }; - assert_eq!(notes, [note(name, 2, what)], "{tool}: selected"); - // The point and line alone: nothing would be clipped or cut by, an error saying why. - let mut points = Dataset::new("masks", GeometryType::Polygon, vec![]); - points.push(Some(Geometry::Point(Coord::new(6.0, 1.0))), vec![]); - points.push(None, vec![]); - let mut polys = Dataset::new("polys", GeometryType::Polygon, vec![]); - polys.push(Some(sq(0.0, 0.0, 2.0)), vec![]); - let m = Map(HashMap::from([("polys".to_string(), polys), ("masks".to_string(), points)])); - let e = crate::run(tool, &m, &serde_json::json!({"input": "polys", key: "masks"})).err().unwrap_or_default(); - assert_eq!(e, format!("{name}: none of the 2 feature(s) of 'masks' covers anything: {what}"), "{tool}"); - // A polygon with no area alone: the same, but Update writes it, as it is, saying nothing. - let mut flats = Dataset::new("masks", GeometryType::Polygon, vec![]); - flats.push(Some(Geometry::Polygon(vec![vec![flat(1.0)]])), vec![]); - let mut polys = Dataset::new("polys", GeometryType::Polygon, vec![]); - polys.push(Some(sq(0.0, 0.0, 2.0)), vec![]); - let m = Map(HashMap::from([("polys".to_string(), polys), ("masks".to_string(), flats)])); - let res = crate::run(tool, &m, &serde_json::json!({"input": "polys", key: "masks"})); - if tool == "update" { - let res = res.unwrap_or_else(|e| panic!("{e}")); - assert!(res.messages.iter().all(|m| !m.contains("covering nothing")), "{:?}", res.messages); - assert_eq!(layer(res).features.len(), 2); - } else { - let e = res.err().unwrap_or_default(); - assert_eq!(e, format!("{name}: none of the 1 feature(s) of 'masks' covers anything: {NO_MASK}"), "{tool}"); + assert_eq!(notes, [note(name, 2, NO_POLYGON)], "{tool}: selected"); + // A bowtie covers ground: kept, without a word. + let mut bowties = Dataset::new("masks", GeometryType::Polygon, vec![Field::new("b", FieldType::Integer)]); + bowties.push(Some(bowtie()), vec![Value::Int(20)]); + let (_, notes) = run(tool, key, bowties, None).unwrap_or_else(|e| panic!("{tool}: {e}")); + assert!(notes.is_empty(), "{tool}: {notes:?}"); + // The point alone, and a feature without geometry: an error where nothing would come + // out (Clip, Intersect, Split); the input, as it comes out with no mask, elsewhere. + let mut points = Dataset::new("masks", GeometryType::Polygon, vec![Field::new("b", FieldType::Integer)]); + points.push(Some(Geometry::Point(Coord::new(6.0, 1.0))), vec![Value::Int(11)]); + points.push(None, vec![Value::Int(15)]); + match (tool, run(tool, key, points, None)) { + ("clip" | "intersect" | "split", res) => assert_eq!( + res.err().unwrap_or_default(), + format!("{name}: none of the 2 feature(s) of 'masks' covers anything: {NO_POLYGON}"), + "{tool}" + ), + (_, res) => assert_eq!(res.map(|(_, notes)| notes), Ok(vec![note(name, 2, NO_POLYGON)]), "{tool}"), } } + // Clip's error says how many of each kind. + let mut both = Dataset::new("masks", GeometryType::Polygon, vec![]); + both.push(None, vec![]); + both.push(Some(Geometry::Polygon(vec![vec![flat(1.0)]])), vec![]); + assert_eq!( + run("clip", "clip", both, None).err().unwrap_or_default(), + format!("Clip: none of the 2 feature(s) of 'masks' covers anything: 1 with no polygon ({NO_POLYGON}), 1 {NO_AREA}") + ); } /// Dissolve's groups, found by their keys' hash: in the order first met, −0 with 0, each NaN diff --git a/docs/user-guide/analysis.md b/docs/user-guide/analysis.md index a0c04122c..81f66dd6c 100644 --- a/docs/user-guide/analysis.md +++ b/docs/user-guide/analysis.md @@ -612,11 +612,11 @@ gp.run={"tool":"split","params":{"input":"roads","split":"districts","field":"na ``` Clip, Erase, Intersect, Identity, Union, Symmetrical Difference, Update and Split leave out a -feature of the clip, erase, overlay, update or split layer that covers nothing, and say how many: -one without geometry, a point or line in the polygon layer (as Minimum Bounding Geometry's flat -hulls are), an empty polygon, or a polygon with no area (Update keeps one with no area, writing its -update polygons as they are). When every feature of that layer is left out, the tool stops with an -error rather than give an empty layer or the input unchanged. +feature of the clip, erase, overlay, update or split layer with no polygon, and say how many: one +without geometry, a point or line in the polygon layer (as Minimum Bounding Geometry's flat hulls +are), or an empty polygon. Clip also leaves out a polygon with no area, and Union and Symmetrical +Difference an overlay feature whose parts, together, have none, saying how many. When Clip, +Intersect or Split has none left, so that its result would be empty, it stops with an error. **Near** (`near`) copies `input` and adds, for each feature, the nearest feature of `near`: its OID as the attribute table shows it (`near_fid`), the distance to it (`near_dist`, in `units`, From 939beea7a74ca6c21829af2a7049d5ee3361b484 Mon Sep 17 00:00:00 2001 From: Cory Douglas Martin <4108748+flight1973@users.noreply.github.com> Date: Mon, 9 Nov 2026 18:05:33 -0600 Subject: [PATCH 05/11] fix(analysis): every overlay tool judges no area as Clip does Review follow-up. A polygon has no area when its union is empty, as Clip judges its masks: rings on a line, one part or several, but not a bowtie, which covers its two triangles. overlay_zones now leaves such features out of Erase, Intersect, Identity, Union, Symmetrical Difference and Split, counted in a notice of their own and toward the error for Intersect and Split, instead of Union and Symmetrical Difference alone catching some of them later. Update keeps them, as it writes its update polygons as they are. A one-part polygon is unioned once more to judge it. --- crates/analysis/src/tools.rs | 98 ++++++++++++++++++++---------------- docs/user-guide/analysis.md | 11 ++-- 2 files changed, 61 insertions(+), 48 deletions(-) diff --git a/crates/analysis/src/tools.rs b/crates/analysis/src/tools.rs index 07fb82de8..8dbf4486b 100644 --- a/crates/analysis/src/tools.rs +++ b/crates/analysis/src/tools.rs @@ -1287,12 +1287,6 @@ fn polygon_of(g: &Geometry) -> Option> { to_multipolygon(g).filter(|mp| mp.0.iter().any(|p| !p.exterior().0.is_empty())) } -/// The notice for the `n` features of `layer` that `tool` left out of its masks or overlay -/// polygons as they cover nothing; `what` lists what they hold. None when there are none. -fn no_polygon_note(tool: &str, layer: &str, n: usize, what: &str) -> Option { - (n > 0).then(|| format!("{tool}: {n} feature(s) of '{layer}' left out, covering nothing: {what}")) -} - /// The error when `tool`, whose output would be empty, left out every feature of `layer` it was /// given, `left` saying how many of each kind. fn nothing_covered(tool: &str, layer: &str, left: LeftOut) -> String { @@ -1308,13 +1302,18 @@ fn nothing_covered(tool: &str, layer: &str, left: LeftOut) -> String { /// gives nothing for. const NO_POLYGON: &str = "no geometry, a point or line, or an empty polygon"; -/// What a feature whose polygons, unioned, have no area holds: Clip leaves out such a mask, and -/// Union and Symmetrical Difference such an overlay feature, as nothing of it would be written. +/// What a feature whose polygons, unioned, have no area holds ([`has_no_area`]). const NO_AREA: &str = "a polygon with no area"; +/// Whether polygons cover no area, as Clip's masks are judged: their union is empty (rings on a +/// line, parts that come to nothing). A bowtie covers its two triangles. +fn has_no_area(poly: &MultiPolygon) -> bool { + union_all(poly.0.iter()).0.is_empty() +} + /// How many features of a mask or overlay layer Clip or an overlay tool left out: those with no /// polygon ([`NO_POLYGON`]), and those whose polygons have no area ([`NO_AREA`]). -#[derive(Clone, Copy)] +#[derive(Clone, Copy, Default)] struct LeftOut { no_polygon: usize, no_area: usize, @@ -1325,7 +1324,8 @@ impl LeftOut { fn notes(self, tool: &str, layer: &str) -> Vec { [(self.no_polygon, NO_POLYGON), (self.no_area, NO_AREA)] .into_iter() - .filter_map(|(n, what)| no_polygon_note(tool, layer, n, what)) + .filter(|&(n, _)| n > 0) + .map(|(n, what)| format!("{tool}: {n} feature(s) of '{layer}' left out, covering nothing: {what}")) .collect() } } @@ -1635,7 +1635,7 @@ fn clip(inp: &dyn Inputs, p: &Json) -> Result { } let mut masks = Vec::new(); // Mask features that clip nothing, left out and counted: no polygon, or one with no area. - let mut left = LeftOut { no_polygon: 0, no_area: 0 }; + let mut left = LeftOut::default(); let mask_rows = each_once(rows(inp, p, "clip", clip_ds)); crate::control::begin(mask_rows.len()); for (done, r) in mask_rows.into_iter().enumerate() { @@ -1727,7 +1727,7 @@ fn erase(inp: &dyn Inputs, p: &Json) -> Result { if ds.geometry_type == GeometryType::None { return Err(format!("'{}' has no geometry", ds.name)); } - let (er, zones, left_out) = overlay_zones(inp, p, "erase", "Erase", false)?; + let (er, zones, left_out) = overlay_zones(inp, p, "erase", "Erase", (false, false))?; let index = ExtentIndex::new(zones.iter().map(|z| z.ext)); let at_poles = AtPoles::new(zones.iter().map(|z| poles(&z.ext))); let mut out = copy_schema(ds, out_name(p, format!("{}_Erase", ds.name)), ds.geometry_type); @@ -1765,14 +1765,16 @@ fn mask_of(poly: MultiPolygon) -> MultiPolygon { /// The polygon layer of `tool`'s parameter `k` (`overlay`, `erase`, `update`, `split`), which /// must be polygons, its (selected) features with a polygon as [`Zone`]s, in order, each once, and -/// how many it left out with no polygon ([`NO_POLYGON`]), for the tool's notices. Leaving out -/// every one is an error when `needs_one`: the tool's output would be empty (Intersect, Split). +/// how many it left out, for the tool's notices: those with no polygon, and those with no area +/// ([`has_no_area`]), as they cover nothing, unless `keeps_no_area` (Update writes its update +/// polygons as they are). Leaving out every one is an error when `needs_one`: the tool's output +/// would be empty (Intersect, Split). A polygon of one part is unioned once more, to judge it. fn overlay_zones<'a>( inp: &'a dyn Inputs, p: &Json, k: &str, tool: &str, - needs_one: bool, + (needs_one, keeps_no_area): (bool, bool), ) -> Result<(&'a Dataset, Vec, LeftOut), String> { let ov = input(inp, p, k)?; if ov.geometry_type != GeometryType::Polygon { @@ -1782,21 +1784,24 @@ fn overlay_zones<'a>( let zone_rows = each_once(rows(inp, p, k, ov)); crate::control::begin(zone_rows.len()); let mut zones = Vec::with_capacity(zone_rows.len()); - let mut left = LeftOut { no_polygon: 0, no_area: 0 }; + let mut left = LeftOut::default(); for (done, row) in zone_rows.into_iter().enumerate() { crate::control::check()?; crate::control::reached(done); - let Some(g) = ov.features.get(row).and_then(|f| f.geometry.as_ref()) else { + let Some((g, poly)) = ov.features.get(row).and_then(|f| f.geometry.as_ref()).and_then(|g| Some((g, polygon_of(g)?))) else { left.no_polygon += 1; continue; }; - let Some(poly) = polygon_of(g) else { - left.no_polygon += 1; + // Several parts are unioned, so their mask says; one part is only oriented. + let several = poly.0.len() > 1; + let poly = mask_of(poly); + if !keeps_no_area && (if several { poly.0.is_empty() } else { has_no_area(&poly) }) { + left.no_area += 1; continue; - }; - zones.push(Zone { row, ext: g.extent(), poly: mask_of(poly) }); + } + zones.push(Zone { row, ext: g.extent(), poly }); } - if needs_one && zones.is_empty() && left.no_polygon > 0 { + if needs_one && zones.is_empty() && left.no_polygon + left.no_area > 0 { return Err(nothing_covered(tool, &ov.name, left)); } Ok((ov, zones, left)) @@ -1968,7 +1973,7 @@ fn overlay(inp: &dyn Inputs, p: &Json, op: Overlay) -> Result Result Result { if ds.geometry_type != GeometryType::Polygon { return Err(format!("'{}' is not a polygon layer", ds.name)); } - let (up, zones, left_out) = overlay_zones(inp, p, "update", "Update", false)?; + let (up, zones, left_out) = overlay_zones(inp, p, "update", "Update", (false, true))?; // Each input field's field in the update layer, if it has one: an ambiguous one fails here, // before any geometry work. let from = ds @@ -2184,7 +2184,7 @@ fn split(inp: &dyn Inputs, p: &Json) -> Result { if ds.geometry_type == GeometryType::None { return Err(format!("'{}' has no geometry", ds.name)); } - let (sp, zones, left_out) = overlay_zones(inp, p, "split", "Split", true)?; + let (sp, zones, left_out) = overlay_zones(inp, p, "split", "Split", (true, false))?; // A field name, or none; anything else is an error, not one layer per polygon. let field = match p.get("field") { None | Some(Json::Null) => None, @@ -6968,11 +6968,11 @@ mod tests { /// Clip and the overlay tools say how many mask or overlay features they left out as covering /// nothing: a point and a line in a polygon layer (as Minimum Bounding Geometry's flat hulls - /// are), empty polygons and a feature without geometry. Clip also leaves out a polygon with no - /// area, and Union and Symmetrical Difference a feature of two such parts, which nothing of - /// would be written. Those change nothing in the output; a layer of polygons, a bowtie among - /// them, says nothing of the kind; only the selected features are counted, each once; and - /// leaving out every one is an error where the output would be empty. + /// are), empty polygons, a feature without geometry, and polygons with no area, of one part or + /// two, which Update alone keeps, writing its update polygons as they are. Those change nothing + /// in the output; a layer of polygons, a bowtie among them, says nothing of the kind; only the + /// selected features are counted, each once; and leaving out every one is an error where the + /// output would be empty. #[test] fn overlay_tools_count_mask_features_that_cover_nothing() { struct Sel(Map, Option>); @@ -7027,13 +7027,10 @@ mod tests { masks.push(Some(Geometry::Polygon(vec![])), vec![Value::Int(13)]); masks.push(Some(Geometry::Polygon(vec![vec![]])), vec![Value::Int(14)]); masks.push(None, vec![Value::Int(15)]); - // Update writes an update polygon as it is; Union and Symmetrical Difference - // write one part with no area as it is too. + // Update writes an update polygon as it is, so one with no area changes its output. if tool != "update" { - // Two polygons, each a ring on a line. + // Two polygons, each a ring on a line; then one. masks.push(Some(Geometry::Polygon(vec![vec![flat(1.0)], vec![flat(1.5)]])), vec![Value::Int(16)]); - } - if tool == "clip" { masks.push(Some(Geometry::Polygon(vec![vec![flat(1.0)]])), vec![Value::Int(17)]); } } @@ -7056,9 +7053,8 @@ mod tests { let (shapes, notes) = run(tool, key, masks(true, tool), None).unwrap_or_else(|e| panic!("{tool}: {e}")); assert_eq!(shapes, clean, "{tool}: the features covering nothing change nothing"); let expected = match tool { - "clip" => vec![note(name, 5, NO_POLYGON), note(name, 2, NO_AREA)], - "union" | "symmetricalDifference" => vec![note(name, 5, NO_POLYGON), note(name, 1, NO_AREA)], - _ => vec![note(name, 5, NO_POLYGON)], + "update" => vec![note(name, 5, NO_POLYGON)], + _ => vec![note(name, 5, NO_POLYGON), note(name, 2, NO_AREA)], }; assert_eq!(notes, expected, "{tool}"); // The square, and the point and line twice over, selected: those two are counted once. @@ -7083,6 +7079,22 @@ mod tests { ), (_, res) => assert_eq!(res.map(|(_, notes)| notes), Ok(vec![note(name, 2, NO_POLYGON)]), "{tool}"), } + // A polygon with no area alone: the same, but Update writes it, as it is, saying nothing. + let mut flats = Dataset::new("masks", GeometryType::Polygon, vec![Field::new("b", FieldType::Integer)]); + flats.push(Some(Geometry::Polygon(vec![vec![flat(1.0)]])), vec![Value::Int(17)]); + match (tool, run(tool, key, flats, None)) { + ("clip" | "intersect" | "split", res) => assert_eq!( + res.err().unwrap_or_default(), + format!("{name}: none of the 1 feature(s) of 'masks' covers anything: {NO_AREA}"), + "{tool}" + ), + ("update", res) => { + let (shapes, notes) = res.unwrap_or_else(|e| panic!("{e}")); + assert!(notes.is_empty(), "{notes:?}"); + assert_eq!(shapes.len(), 3, "the two input polygons and the update polygon"); + } + (_, res) => assert_eq!(res.map(|(_, notes)| notes), Ok(vec![note(name, 1, NO_AREA)]), "{tool}"), + } } // Clip's error says how many of each kind. let mut both = Dataset::new("masks", GeometryType::Polygon, vec![]); diff --git a/docs/user-guide/analysis.md b/docs/user-guide/analysis.md index 81f66dd6c..3cfbd082b 100644 --- a/docs/user-guide/analysis.md +++ b/docs/user-guide/analysis.md @@ -612,11 +612,12 @@ gp.run={"tool":"split","params":{"input":"roads","split":"districts","field":"na ``` Clip, Erase, Intersect, Identity, Union, Symmetrical Difference, Update and Split leave out a -feature of the clip, erase, overlay, update or split layer with no polygon, and say how many: one -without geometry, a point or line in the polygon layer (as Minimum Bounding Geometry's flat hulls -are), or an empty polygon. Clip also leaves out a polygon with no area, and Union and Symmetrical -Difference an overlay feature whose parts, together, have none, saying how many. When Clip, -Intersect or Split has none left, so that its result would be empty, it stops with an error. +feature of the clip, erase, overlay, update or split layer that covers nothing, and say how many: +one without geometry, a point or line in the polygon layer (as Minimum Bounding Geometry's flat +hulls are), an empty polygon, or a polygon with no area, such as rings on a line (one that crosses +itself covers its loops). Update keeps a polygon with no area, as it writes its update polygons as +they are. When Clip, Intersect or Split has none left, so that its result would be empty, it stops +with an error. **Near** (`near`) copies `input` and adds, for each feature, the nearest feature of `near`: its OID as the attribute table shows it (`near_fid`), the distance to it (`near_dist`, in `units`, From 875c083cf044f3af2ce24634edc5560cb4588c73 Mon Sep 17 00:00:00 2001 From: Cory Douglas Martin <4108748+flight1973@users.noreply.github.com> Date: Mon, 9 Nov 2026 18:42:41 -0600 Subject: [PATCH 06/11] fix(analysis): the overlay tools keep their polygons as they were; Cancel for hulls moves out Narrow the change to what was silent. Judging which overlay polygons have no area changed results for points and lines on flat rings, could drop very thin polygons the union rounds away, and doubled the overlay tools' setup work. Erase, Intersect, Identity, Union, Symmetrical Difference, Update and Split keep every polygon as a zone, as on main, and add only the notices: features with no polygon, and in Union and Symmetrical Difference a feature whose parts union to nothing, which would otherwise vanish. Clip keeps counting masks whose union is empty. Leaving out every feature is an error only for Intersect and Split, and for Clip. Minimum Bounding Geometry's Cancel check for Group: All goes to a pull request of its own. --- AGENTS.md | 2 +- crates/analysis/src/tools.rs | 94 +++++++++++++++--------------------- crates/engine/src/tests.rs | 9 ++-- docs/user-guide/analysis.md | 10 ++-- 4 files changed, 48 insertions(+), 67 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index a6d1b7a65..35a227851 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -47,7 +47,7 @@ is out of scope. background command as its work starts and fails one that never looks; its lists must match the background registries, so a new background command is added there. It also runs the long tools (Buffer, Dissolve, Spatial Join, Clip, Erase, Intersect, Identity, Union, Symmetrical - Difference, Update, Split, Minimum Bounding Geometry, Kernel Density, Kriging); a new tool + Difference, Update, Split, Kernel Density, Kriging); a new tool whose work grows with its input is added to that list. Passing shows the work looks at Cancel, not that it looks often: a long step without a check (one union, one read) is said in its doc comment. diff --git a/crates/analysis/src/tools.rs b/crates/analysis/src/tools.rs index 8dbf4486b..2f35fc1a5 100644 --- a/crates/analysis/src/tools.rs +++ b/crates/analysis/src/tools.rs @@ -1302,15 +1302,11 @@ fn nothing_covered(tool: &str, layer: &str, left: LeftOut) -> String { /// gives nothing for. const NO_POLYGON: &str = "no geometry, a point or line, or an empty polygon"; -/// What a feature whose polygons, unioned, have no area holds ([`has_no_area`]). +/// What a feature whose polygons, unioned, come to nothing holds: Clip leaves out such a mask, and +/// Union and Symmetrical Difference such an overlay feature of several parts, as nothing of it +/// would be written. const NO_AREA: &str = "a polygon with no area"; -/// Whether polygons cover no area, as Clip's masks are judged: their union is empty (rings on a -/// line, parts that come to nothing). A bowtie covers its two triangles. -fn has_no_area(poly: &MultiPolygon) -> bool { - union_all(poly.0.iter()).0.is_empty() -} - /// How many features of a mask or overlay layer Clip or an overlay tool left out: those with no /// polygon ([`NO_POLYGON`]), and those whose polygons have no area ([`NO_AREA`]). #[derive(Clone, Copy, Default)] @@ -1727,7 +1723,7 @@ fn erase(inp: &dyn Inputs, p: &Json) -> Result { if ds.geometry_type == GeometryType::None { return Err(format!("'{}' has no geometry", ds.name)); } - let (er, zones, left_out) = overlay_zones(inp, p, "erase", "Erase", (false, false))?; + let (er, zones, left_out) = overlay_zones(inp, p, "erase", "Erase", false)?; let index = ExtentIndex::new(zones.iter().map(|z| z.ext)); let at_poles = AtPoles::new(zones.iter().map(|z| poles(&z.ext))); let mut out = copy_schema(ds, out_name(p, format!("{}_Erase", ds.name)), ds.geometry_type); @@ -1765,16 +1761,14 @@ fn mask_of(poly: MultiPolygon) -> MultiPolygon { /// The polygon layer of `tool`'s parameter `k` (`overlay`, `erase`, `update`, `split`), which /// must be polygons, its (selected) features with a polygon as [`Zone`]s, in order, each once, and -/// how many it left out, for the tool's notices: those with no polygon, and those with no area -/// ([`has_no_area`]), as they cover nothing, unless `keeps_no_area` (Update writes its update -/// polygons as they are). Leaving out every one is an error when `needs_one`: the tool's output -/// would be empty (Intersect, Split). A polygon of one part is unioned once more, to judge it. +/// how many it left out with no polygon ([`NO_POLYGON`]), for the tool's notices. Leaving out +/// every one is an error when `needs_one`: the tool's output would be empty (Intersect, Split). fn overlay_zones<'a>( inp: &'a dyn Inputs, p: &Json, k: &str, tool: &str, - (needs_one, keeps_no_area): (bool, bool), + needs_one: bool, ) -> Result<(&'a Dataset, Vec, LeftOut), String> { let ov = input(inp, p, k)?; if ov.geometry_type != GeometryType::Polygon { @@ -1792,16 +1786,9 @@ fn overlay_zones<'a>( left.no_polygon += 1; continue; }; - // Several parts are unioned, so their mask says; one part is only oriented. - let several = poly.0.len() > 1; - let poly = mask_of(poly); - if !keeps_no_area && (if several { poly.0.is_empty() } else { has_no_area(&poly) }) { - left.no_area += 1; - continue; - } - zones.push(Zone { row, ext: g.extent(), poly }); + zones.push(Zone { row, ext: g.extent(), poly: mask_of(poly) }); } - if needs_one && zones.is_empty() && left.no_polygon + left.no_area > 0 { + if needs_one && zones.is_empty() && left.no_polygon > 0 { return Err(nothing_covered(tool, &ov.name, left)); } Ok((ov, zones, left)) @@ -1973,7 +1960,7 @@ fn overlay(inp: &dyn Inputs, p: &Json, op: Overlay) -> Result Result Result { if ds.geometry_type != GeometryType::Polygon { return Err(format!("'{}' is not a polygon layer", ds.name)); } - let (up, zones, left_out) = overlay_zones(inp, p, "update", "Update", (false, true))?; + let (up, zones, left_out) = overlay_zones(inp, p, "update", "Update", false)?; // Each input field's field in the update layer, if it has one: an ambiguous one fails here, // before any geometry work. let from = ds @@ -2184,7 +2176,7 @@ fn split(inp: &dyn Inputs, p: &Json) -> Result { if ds.geometry_type == GeometryType::None { return Err(format!("'{}' has no geometry", ds.name)); } - let (sp, zones, left_out) = overlay_zones(inp, p, "split", "Split", (true, false))?; + let (sp, zones, left_out) = overlay_zones(inp, p, "split", "Split", true)?; // A field name, or none; anything else is an error, not one layer per polygon. let field = match p.get("field") { None | Some(Json::Null) => None, @@ -2367,9 +2359,6 @@ fn dissolve(inp: &dyn Inputs, p: &Json) -> Result { /// latitude past ±90° is taken as the pole.) const HULL_LIMIT_M: f64 = 1e12; -/// Minimum Bounding Geometry. Cancel is looked at per feature, as each one's points are gathered -/// (and its hull made, one hull each); the one hull of all features' points, with `group` All, is -/// a single step without a look. fn convex_hull(inp: &dyn Inputs, p: &Json) -> Result { let ds = input(inp, p, "input")?; let each = s(p, "group") == "None"; @@ -2398,7 +2387,8 @@ fn convex_hull(inp: &dyn Inputs, p: &Json) -> Result { (these, gaps, poles) }; // Each row once, however often a selection lists it, in the selection's order. - let picked = each_once(rows(inp, p, "input", ds)); + let mut seen = std::collections::HashSet::new(); + let picked: Vec = rows(inp, p, "input", ds).into_iter().filter(|r| seen.insert(*r)).collect(); // The features with a geometry, each with it; the rest are left out, and counted. let features: Vec<(&Feature, &Geometry)> = picked.iter().filter_map(|&r| ds.features.get(r).and_then(|f| Some((f, f.geometry.as_ref()?)))).collect(); @@ -2455,13 +2445,8 @@ fn convex_hull(inp: &dyn Inputs, p: &Json) -> Result { } else { // All together; `count` is the features hulled, those with a point to give. With none, // no hull is written, as one feature each. - // Cancel is looked at, and progress counted, per feature; the one hull of all their - // points is a single step without a look. let (mut pts, mut hulled) = (Vec::new(), 0usize); - crate::control::begin(features.len()); - for (done, (_, g)) in features.iter().enumerate() { - crate::control::check()?; - crate::control::reached(done); + for (_, g) in &features { let (these, passed, past) = points(g); gaps += passed; poles += past; @@ -6968,11 +6953,11 @@ mod tests { /// Clip and the overlay tools say how many mask or overlay features they left out as covering /// nothing: a point and a line in a polygon layer (as Minimum Bounding Geometry's flat hulls - /// are), empty polygons, a feature without geometry, and polygons with no area, of one part or - /// two, which Update alone keeps, writing its update polygons as they are. Those change nothing - /// in the output; a layer of polygons, a bowtie among them, says nothing of the kind; only the - /// selected features are counted, each once; and leaving out every one is an error where the - /// output would be empty. + /// are), empty polygons and a feature without geometry. Clip also leaves out a polygon with no + /// area, and Union and Symmetrical Difference a feature of two such parts, which nothing of + /// would be written. Those change nothing in the output; a layer of polygons, a bowtie among + /// them, says nothing of the kind; only the selected features are counted, each once; and + /// leaving out every one is an error where the output would be empty. #[test] fn overlay_tools_count_mask_features_that_cover_nothing() { struct Sel(Map, Option>); @@ -7027,10 +7012,13 @@ mod tests { masks.push(Some(Geometry::Polygon(vec![])), vec![Value::Int(13)]); masks.push(Some(Geometry::Polygon(vec![vec![]])), vec![Value::Int(14)]); masks.push(None, vec![Value::Int(15)]); - // Update writes an update polygon as it is, so one with no area changes its output. + // Update writes an update polygon as it is, so one with no area changes its output; + // the other overlay tools keep one part with no area as a polygon, as it is. if tool != "update" { - // Two polygons, each a ring on a line; then one. + // Two polygons, each a ring on a line. masks.push(Some(Geometry::Polygon(vec![vec![flat(1.0)], vec![flat(1.5)]])), vec![Value::Int(16)]); + } + if tool == "clip" { masks.push(Some(Geometry::Polygon(vec![vec![flat(1.0)]])), vec![Value::Int(17)]); } } @@ -7053,8 +7041,9 @@ mod tests { let (shapes, notes) = run(tool, key, masks(true, tool), None).unwrap_or_else(|e| panic!("{tool}: {e}")); assert_eq!(shapes, clean, "{tool}: the features covering nothing change nothing"); let expected = match tool { - "update" => vec![note(name, 5, NO_POLYGON)], - _ => vec![note(name, 5, NO_POLYGON), note(name, 2, NO_AREA)], + "clip" => vec![note(name, 5, NO_POLYGON), note(name, 2, NO_AREA)], + "union" | "symmetricalDifference" => vec![note(name, 5, NO_POLYGON), note(name, 1, NO_AREA)], + _ => vec![note(name, 5, NO_POLYGON)], }; assert_eq!(notes, expected, "{tool}"); // The square, and the point and line twice over, selected: those two are counted once. @@ -7079,21 +7068,16 @@ mod tests { ), (_, res) => assert_eq!(res.map(|(_, notes)| notes), Ok(vec![note(name, 2, NO_POLYGON)]), "{tool}"), } - // A polygon with no area alone: the same, but Update writes it, as it is, saying nothing. + // One ring on a line alone: Clip's mask, unioned, is nothing; the overlay tools keep + // it as the polygon it is, as before. let mut flats = Dataset::new("masks", GeometryType::Polygon, vec![Field::new("b", FieldType::Integer)]); flats.push(Some(Geometry::Polygon(vec![vec![flat(1.0)]])), vec![Value::Int(17)]); match (tool, run(tool, key, flats, None)) { - ("clip" | "intersect" | "split", res) => assert_eq!( + ("clip", res) => assert_eq!( res.err().unwrap_or_default(), - format!("{name}: none of the 1 feature(s) of 'masks' covers anything: {NO_AREA}"), - "{tool}" + format!("Clip: none of the 1 feature(s) of 'masks' covers anything: {NO_AREA}") ), - ("update", res) => { - let (shapes, notes) = res.unwrap_or_else(|e| panic!("{e}")); - assert!(notes.is_empty(), "{notes:?}"); - assert_eq!(shapes.len(), 3, "the two input polygons and the update polygon"); - } - (_, res) => assert_eq!(res.map(|(_, notes)| notes), Ok(vec![note(name, 1, NO_AREA)]), "{tool}"), + (_, res) => assert_eq!(res.map(|(_, notes)| notes), Ok(vec![]), "{tool}"), } } // Clip's error says how many of each kind. @@ -9218,7 +9202,7 @@ mod tests { crate::run_with("buffer", &m, &serde_json::json!({"input": "pts", "distance": 100.0}), &control).unwrap_or_else(|e| panic!("{e}")); assert_eq!(control.progress(), Some((40, 40))); // The same control for a tool that doesn't count shows no count, not the buffers'. - crate::run_with("featureVerticesToPoints", &m, &serde_json::json!({"input": "pts"}), &control).unwrap_or_else(|e| panic!("{e}")); + crate::run_with("convexHull", &m, &serde_json::json!({"input": "pts"}), &control).unwrap_or_else(|e| panic!("{e}")); assert_eq!(control.progress(), None); // Dissolved, the union after the buffers isn't counted: the buffers' count ends before it. let dissolve = serde_json::json!({"input": "pts", "distance": 100.0, "dissolve": "All"}); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index dd4e64b80..c1d9ec20e 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -37084,8 +37084,8 @@ fn long_commands_stop_when_cancelled_while_they_run() { } // The tools whose work grows past a second or two on real data: buffers, overlays, joins, - // hulls, densities and interpolation. Run in the background as the Analysis pane runs them; a - // new long tool is added here (AGENTS.md › Never hold the window). + // densities and interpolation. Run in the background as the Analysis pane runs them; a new + // long tool is added here (AGENTS.md › Never hold the window). let tools = [ ("buffer", json!({"input": "Cities", "distance": 100})), ("dissolve", json!({"input": "Countries"})), @@ -37098,8 +37098,6 @@ fn long_commands_stop_when_cancelled_while_they_run() { ("symmetricalDifference", json!({"input": "Lakes", "overlay": "Countries"})), ("update", json!({"input": "Countries", "update": "Lakes"})), ("split", json!({"input": "Rivers", "split": "Countries"})), - ("convexHull", json!({"input": "Countries", "group": "None"})), - ("convexHull", json!({"input": "Countries", "group": "All"})), ("kernelDensity", json!({"input": "Cities", "crs": "EPSG:3857"})), ("kriging", json!({"input": "Cities", "field": "pop_max"})), ]; @@ -37112,8 +37110,7 @@ fn long_commands_stop_when_cancelled_while_they_run() { let done = run.run(&control); // The result is kept, then `gp.finish` applies it (or says why it can't), as in the app. let finished = crate::cmd::gp::tool_complete(&mut s, id, done).and_then(|()| s.run("gp.finish", &json!({"id": id})).map(|_| ())); - // With its parameters: a tool listed twice (convexHull) is told apart. - note(&format!("{tool} {params}"), finished, &control); + note(tool, finished, &control); } assert!(kept_going.is_empty(), "these ran on after Cancel:\n{}", kept_going.join("\n")); } diff --git a/docs/user-guide/analysis.md b/docs/user-guide/analysis.md index 3cfbd082b..efb9a8402 100644 --- a/docs/user-guide/analysis.md +++ b/docs/user-guide/analysis.md @@ -612,11 +612,11 @@ gp.run={"tool":"split","params":{"input":"roads","split":"districts","field":"na ``` Clip, Erase, Intersect, Identity, Union, Symmetrical Difference, Update and Split leave out a -feature of the clip, erase, overlay, update or split layer that covers nothing, and say how many: -one without geometry, a point or line in the polygon layer (as Minimum Bounding Geometry's flat -hulls are), an empty polygon, or a polygon with no area, such as rings on a line (one that crosses -itself covers its loops). Update keeps a polygon with no area, as it writes its update polygons as -they are. When Clip, Intersect or Split has none left, so that its result would be empty, it stops +feature of the clip, erase, overlay, update or split layer with no polygon, and say how many: one +without geometry, a point or line in the polygon layer (as Minimum Bounding Geometry's flat hulls +are), or an empty polygon. Clip also leaves out a mask whose polygons, unioned, have no area, and +Union and Symmetrical Difference an overlay feature whose parts, unioned, come to nothing, saying +how many. When Clip, Intersect or Split has none left, so that its result would be empty, it stops with an error. **Near** (`near`) copies `input` and adds, for each feature, the nearest feature of `near`: its OID From 11ece6da1b5a6974ba629770e14353d042512b61 Mon Sep 17 00:00:00 2001 From: Cory Douglas Martin <4108748+flight1973@users.noreply.github.com> Date: Mon, 9 Nov 2026 19:05:28 -0600 Subject: [PATCH 07/11] fix(analysis): Split checks its field before the split polygons Review follow-up. Split read its split polygons, and could stop with "none covers anything", before it checked the field parameter, so a wrong field was hidden behind that error. The user guide now says the error comes when no feature of the layer has a polygon (for Clip, when none covers any area), which is when it does. --- crates/analysis/src/tools.rs | 13 ++++++++++--- docs/user-guide/analysis.md | 4 ++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/crates/analysis/src/tools.rs b/crates/analysis/src/tools.rs index 2f35fc1a5..88364b2d8 100644 --- a/crates/analysis/src/tools.rs +++ b/crates/analysis/src/tools.rs @@ -2176,14 +2176,15 @@ fn split(inp: &dyn Inputs, p: &Json) -> Result { if ds.geometry_type == GeometryType::None { return Err(format!("'{}' has no geometry", ds.name)); } - let (sp, zones, left_out) = overlay_zones(inp, p, "split", "Split", true)?; - // A field name, or none; anything else is an error, not one layer per polygon. + // A field name, or none; anything else is an error, not one layer per polygon. Checked before + // the split polygons are read, so a wrong field isn't hidden behind them. let field = match p.get("field") { None | Some(Json::Null) => None, Some(Json::String(name)) if name.trim().is_empty() => None, - Some(Json::String(name)) => Some(find_field_case_insensitive(sp, name.trim())?), + Some(Json::String(name)) => Some(find_field_case_insensitive(input(inp, p, "split")?, name.trim())?), Some(other) => return Err(format!("Split Field must be a field name, not {other}")), }; + let (sp, zones, left_out) = overlay_zones(inp, p, "split", "Split", true)?; // The split polygons of each layer, in the order their first polygon comes, keyed by the exact // value, so values that would be named alike stay apart. A layer named by a value and one named // by an id are kept apart even when the two read the same. @@ -7080,6 +7081,12 @@ mod tests { (_, res) => assert_eq!(res.map(|(_, notes)| notes), Ok(vec![]), "{tool}"), } } + // A wrong Split field is said before the split polygons are looked at. + let mut points = Dataset::new("masks", GeometryType::Polygon, vec![]); + points.push(Some(Geometry::Point(Coord::new(6.0, 1.0))), vec![]); + let m = Map(HashMap::from([("polys".to_string(), polys()), ("masks".to_string(), points)])); + let e = crate::run("split", &m, &serde_json::json!({"input": "polys", "split": "masks", "field": 42})).err().unwrap_or_default(); + assert_eq!(e, "Split Field must be a field name, not 42"); // Clip's error says how many of each kind. let mut both = Dataset::new("masks", GeometryType::Polygon, vec![]); both.push(None, vec![]); diff --git a/docs/user-guide/analysis.md b/docs/user-guide/analysis.md index efb9a8402..97a5d5ef9 100644 --- a/docs/user-guide/analysis.md +++ b/docs/user-guide/analysis.md @@ -616,8 +616,8 @@ feature of the clip, erase, overlay, update or split layer with no polygon, and without geometry, a point or line in the polygon layer (as Minimum Bounding Geometry's flat hulls are), or an empty polygon. Clip also leaves out a mask whose polygons, unioned, have no area, and Union and Symmetrical Difference an overlay feature whose parts, unioned, come to nothing, saying -how many. When Clip, Intersect or Split has none left, so that its result would be empty, it stops -with an error. +how many. When none of the features of that layer has a polygon (for Clip, none covers any area), +Clip, Intersect and Split, whose result would be empty, stop with an error. **Near** (`near`) copies `input` and adds, for each feature, the nearest feature of `near`: its OID as the attribute table shows it (`near_fid`), the distance to it (`near_dist`, in `units`, From 92659ef302d1730169be99c454d7c84e4b090365 Mon Sep 17 00:00:00 2001 From: Cory Douglas Martin <4108748+flight1973@users.noreply.github.com> Date: Tue, 10 Nov 2026 14:29:51 -0600 Subject: [PATCH 08/11] fix(analysis): every overlay tool counts a feature whose parts union to nothing Review follow-up. An overlay feature of several parts whose union is empty was counted only by Union and Symmetrical Difference; Erase, Intersect, Identity and Split kept it as a zone that covers nothing, without a word. overlay_zones now leaves it out and counts it for all of them, and Intersect and Split count it toward their error. Update keeps it, as it writes its update polygons as they are. The union was made anyway, so this costs nothing, and a one-part polygon is still only oriented, as before. A ZoneUse value says which of these a tool is, in place of a bare flag. --- crates/analysis/src/tools.rs | 74 +++++++++++++++++++++++++----------- docs/user-guide/analysis.md | 6 +-- 2 files changed, 54 insertions(+), 26 deletions(-) diff --git a/crates/analysis/src/tools.rs b/crates/analysis/src/tools.rs index 88364b2d8..37d74f88b 100644 --- a/crates/analysis/src/tools.rs +++ b/crates/analysis/src/tools.rs @@ -1303,8 +1303,7 @@ fn nothing_covered(tool: &str, layer: &str, left: LeftOut) -> String { const NO_POLYGON: &str = "no geometry, a point or line, or an empty polygon"; /// What a feature whose polygons, unioned, come to nothing holds: Clip leaves out such a mask, and -/// Union and Symmetrical Difference such an overlay feature of several parts, as nothing of it -/// would be written. +/// the overlay tools such a feature of several parts (see [`ZoneUse`]). const NO_AREA: &str = "a polygon with no area"; /// How many features of a mask or overlay layer Clip or an overlay tool left out: those with no @@ -1723,7 +1722,7 @@ fn erase(inp: &dyn Inputs, p: &Json) -> Result { if ds.geometry_type == GeometryType::None { return Err(format!("'{}' has no geometry", ds.name)); } - let (er, zones, left_out) = overlay_zones(inp, p, "erase", "Erase", false)?; + let (er, zones, left_out) = overlay_zones(inp, p, "erase", "Erase", ZoneUse::Cuts)?; let index = ExtentIndex::new(zones.iter().map(|z| z.ext)); let at_poles = AtPoles::new(zones.iter().map(|z| poles(&z.ext))); let mut out = copy_schema(ds, out_name(p, format!("{}_Erase", ds.name)), ds.geometry_type); @@ -1759,16 +1758,31 @@ fn mask_of(poly: MultiPolygon) -> MultiPolygon { if poly.0.len() > 1 { union_all(poly.0.iter()) } else { poly.orient(Direction::Default) } } +/// How an overlay tool uses its overlay polygons, for [`overlay_zones`]. +#[derive(Clone, Copy, PartialEq, Eq)] +enum ZoneUse { + /// Its output comes only from inside them (Intersect, Split): with none left, it would be + /// empty, an error. + Inside, + /// It cuts by them (Erase, Identity, Union, Symmetrical Difference); with none left, the input + /// comes out as it is. + Cuts, + /// It writes each as it is (Update): one whose parts union to nothing is kept, as it is. + AsIs, +} + /// The polygon layer of `tool`'s parameter `k` (`overlay`, `erase`, `update`, `split`), which /// must be polygons, its (selected) features with a polygon as [`Zone`]s, in order, each once, and -/// how many it left out with no polygon ([`NO_POLYGON`]), for the tool's notices. Leaving out -/// every one is an error when `needs_one`: the tool's output would be empty (Intersect, Split). +/// how many it left out, for the tool's notices: those with no polygon ([`NO_POLYGON`]), and, +/// unless `use_` is [`ZoneUse::AsIs`], those of several parts whose union is empty ([`NO_AREA`]), +/// as they cover nothing. A polygon of one part is only oriented, its edges kept exactly, so one +/// with no area stays a zone, as it always has. fn overlay_zones<'a>( inp: &'a dyn Inputs, p: &Json, k: &str, tool: &str, - needs_one: bool, + use_: ZoneUse, ) -> Result<(&'a Dataset, Vec, LeftOut), String> { let ov = input(inp, p, k)?; if ov.geometry_type != GeometryType::Polygon { @@ -1786,9 +1800,14 @@ fn overlay_zones<'a>( left.no_polygon += 1; continue; }; - zones.push(Zone { row, ext: g.extent(), poly: mask_of(poly) }); + let poly = mask_of(poly); + if poly.0.is_empty() && use_ != ZoneUse::AsIs { + left.no_area += 1; + continue; + } + zones.push(Zone { row, ext: g.extent(), poly }); } - if needs_one && zones.is_empty() && left.no_polygon > 0 { + if use_ == ZoneUse::Inside && zones.is_empty() && left.no_polygon + left.no_area > 0 { return Err(nothing_covered(tool, &ov.name, left)); } Ok((ov, zones, left)) @@ -1960,7 +1979,8 @@ fn overlay(inp: &dyn Inputs, p: &Json, op: Overlay) -> Result Result Result { if ds.geometry_type != GeometryType::Polygon { return Err(format!("'{}' is not a polygon layer", ds.name)); } - let (up, zones, left_out) = overlay_zones(inp, p, "update", "Update", false)?; + let (up, zones, left_out) = overlay_zones(inp, p, "update", "Update", ZoneUse::AsIs)?; // Each input field's field in the update layer, if it has one: an ambiguous one fails here, // before any geometry work. let from = ds @@ -2184,7 +2199,7 @@ fn split(inp: &dyn Inputs, p: &Json) -> Result { Some(Json::String(name)) => Some(find_field_case_insensitive(input(inp, p, "split")?, name.trim())?), Some(other) => return Err(format!("Split Field must be a field name, not {other}")), }; - let (sp, zones, left_out) = overlay_zones(inp, p, "split", "Split", true)?; + let (sp, zones, left_out) = overlay_zones(inp, p, "split", "Split", ZoneUse::Inside)?; // The split polygons of each layer, in the order their first polygon comes, keyed by the exact // value, so values that would be named alike stay apart. A layer named by a value and one named // by an id are kept apart even when the two read the same. @@ -6954,11 +6969,11 @@ mod tests { /// Clip and the overlay tools say how many mask or overlay features they left out as covering /// nothing: a point and a line in a polygon layer (as Minimum Bounding Geometry's flat hulls - /// are), empty polygons and a feature without geometry. Clip also leaves out a polygon with no - /// area, and Union and Symmetrical Difference a feature of two such parts, which nothing of - /// would be written. Those change nothing in the output; a layer of polygons, a bowtie among - /// them, says nothing of the kind; only the selected features are counted, each once; and - /// leaving out every one is an error where the output would be empty. + /// are), empty polygons and a feature without geometry, and a feature of two parts with no + /// area (all but Update, which writes it as it is); Clip also a single polygon with no area. + /// Those change nothing in the output; a layer of polygons, a bowtie among them, says nothing + /// of the kind; only the selected features are counted, each once; and leaving out every one + /// is an error where the output would be empty. #[test] fn overlay_tools_count_mask_features_that_cover_nothing() { struct Sel(Map, Option>); @@ -7043,8 +7058,8 @@ mod tests { assert_eq!(shapes, clean, "{tool}: the features covering nothing change nothing"); let expected = match tool { "clip" => vec![note(name, 5, NO_POLYGON), note(name, 2, NO_AREA)], - "union" | "symmetricalDifference" => vec![note(name, 5, NO_POLYGON), note(name, 1, NO_AREA)], - _ => vec![note(name, 5, NO_POLYGON)], + "update" => vec![note(name, 5, NO_POLYGON)], + _ => vec![note(name, 5, NO_POLYGON), note(name, 1, NO_AREA)], }; assert_eq!(notes, expected, "{tool}"); // The square, and the point and line twice over, selected: those two are counted once. @@ -7080,6 +7095,19 @@ mod tests { ), (_, res) => assert_eq!(res.map(|(_, notes)| notes), Ok(vec![]), "{tool}"), } + // A feature of two rings on a line alone: left out, an error where nothing would come + // out; Update writes it as it is. + let mut twos = Dataset::new("masks", GeometryType::Polygon, vec![Field::new("b", FieldType::Integer)]); + twos.push(Some(Geometry::Polygon(vec![vec![flat(1.0)], vec![flat(1.5)]])), vec![Value::Int(16)]); + match (tool, run(tool, key, twos, None)) { + ("clip" | "intersect" | "split", res) => assert_eq!( + res.err().unwrap_or_default(), + format!("{name}: none of the 1 feature(s) of 'masks' covers anything: {NO_AREA}"), + "{tool}" + ), + ("update", res) => assert_eq!(res.map(|(_, notes)| notes), Ok(vec![]), "{tool}"), + (_, res) => assert_eq!(res.map(|(_, notes)| notes), Ok(vec![note(name, 1, NO_AREA)]), "{tool}"), + } } // A wrong Split field is said before the split polygons are looked at. let mut points = Dataset::new("masks", GeometryType::Polygon, vec![]); diff --git a/docs/user-guide/analysis.md b/docs/user-guide/analysis.md index 97a5d5ef9..5f1cb9b01 100644 --- a/docs/user-guide/analysis.md +++ b/docs/user-guide/analysis.md @@ -615,9 +615,9 @@ Clip, Erase, Intersect, Identity, Union, Symmetrical Difference, Update and Spli feature of the clip, erase, overlay, update or split layer with no polygon, and say how many: one without geometry, a point or line in the polygon layer (as Minimum Bounding Geometry's flat hulls are), or an empty polygon. Clip also leaves out a mask whose polygons, unioned, have no area, and -Union and Symmetrical Difference an overlay feature whose parts, unioned, come to nothing, saying -how many. When none of the features of that layer has a polygon (for Clip, none covers any area), -Clip, Intersect and Split, whose result would be empty, stop with an error. +the others a feature of several parts that, unioned, come to nothing (Update writes it as it is), +saying how many. When none of the features of that layer is left, Clip, Intersect and Split, whose +result would be empty, stop with an error. **Near** (`near`) copies `input` and adds, for each feature, the nearest feature of `near`: its OID as the attribute table shows it (`near_fid`), the distance to it (`near_dist`, in `units`, From f1037e8bac772b2cb0712cac6fb006f51aaeef32 Mon Sep 17 00:00:00 2001 From: giswqs <5016453+giswqs@users.noreply.github.com> Date: Tue, 10 Nov 2026 20:38:03 +0000 Subject: [PATCH 09/11] docs(analysis): say a row selected twice is used once Overlay tools (Intersect, Identity, Union, Symmetrical Difference, Erase, Update, Split) dedupe their overlay selection, so a row selected twice makes one zone, not two. Intersect's section in the analysis guide now says so, since its rules are the overlay tools' rules. No code changed in this commit; the dedupe itself came with the overlay notices change. Co-Authored-By: Claude Haiku 5.5 Applied by the claude-autofix workflow. --- docs/user-guide/analysis.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/user-guide/analysis.md b/docs/user-guide/analysis.md index b660749ce..06b783193 100644 --- a/docs/user-guide/analysis.md +++ b/docs/user-guide/analysis.md @@ -560,8 +560,8 @@ overlay must be polygons. An overlay field whose name the input already has gets Overlapping features within one layer are not split against each other. Like Clip and Erase, the overlay works on the map's Web Mercator coordinates, so the parts' edges are straight on the map within ±85.05° (beyond it, see **Near the poles** above). -Selections in both layers are honoured. To measure each part, run Calculate Geometry Attributes on -the result: +Selections in both layers are honoured, and a row selected twice is used once, so its parts come out +once. To measure each part, run Calculate Geometry Attributes on the result: ```text gp.run={"tool":"intersect","params":{"input":"parcels","overlay":"floodplain","output":"parcels_in_floodplain"}} From 6a44cca41e5c31e92ded9c6fcbc0ea31ca3600c5 Mon Sep 17 00:00:00 2001 From: Cory Douglas Martin <4108748+flight1973@users.noreply.github.com> Date: Tue, 10 Nov 2026 15:06:24 -0600 Subject: [PATCH 10/11] docs(analysis): Clip and Split doc comments name the notices; rustfmt after the merge Review follow-up. Clip's and Split's doc comments say what they leave out and count, and when it's an error. A line of the Minimum Bounding Geometry paragraph is reflowed, and overlay_zones' signature is formatted as rustfmt wants after the merge from main. --- crates/analysis/src/tools.rs | 15 ++++++++++++--- docs/user-guide/analysis.md | 5 ++--- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/crates/analysis/src/tools.rs b/crates/analysis/src/tools.rs index 16c33386d..5cf0deff1 100644 --- a/crates/analysis/src/tools.rs +++ b/crates/analysis/src/tools.rs @@ -1621,7 +1621,9 @@ fn part(g: &Geometry, mask: &MultiPolygon, inside: bool) -> Option Result { let ds = input(inp, p, "input")?; let clip_ds = input(inp, p, "clip")?; @@ -1810,7 +1812,13 @@ enum ZoneMade { /// unless `use_` is [`ZoneUse::AsIs`], those of several parts whose union is empty ([`NO_AREA`]), /// as they cover nothing. A polygon of one part is only oriented, its edges kept exactly, so one /// with no area stays a zone, as it always has. -fn overlay_zones<'a>(inp: &'a dyn Inputs, p: &Json, k: &str, tool: &str, use_: ZoneUse) -> Result<(&'a Dataset, Vec, LeftOut), String> { +fn overlay_zones<'a>( + inp: &'a dyn Inputs, + p: &Json, + k: &str, + tool: &str, + use_: ZoneUse, +) -> Result<(&'a Dataset, Vec, LeftOut), String> { overlay_zones_on(inp, p, k, tool, use_, None) } @@ -2263,7 +2271,8 @@ fn split_key(v: &Value) -> String { /// [`MAX_SPLIT_NAME`] characters, control characters dropped); one with no value is named by its /// id, as is one whose value is blank once its control characters are dropped. A name the map /// already has gets a number when the layer is added. Layers left with no features are left out; -/// selections in both layers count. +/// selections in both layers count. Split features left out by [`overlay_zones`] are counted; with +/// none left, it's an error. fn split(inp: &dyn Inputs, p: &Json) -> Result { let ds = input(inp, p, "input")?; if ds.geometry_type == GeometryType::None { diff --git a/docs/user-guide/analysis.md b/docs/user-guide/analysis.md index 06b783193..b7c67778d 100644 --- a/docs/user-guide/analysis.md +++ b/docs/user-guide/analysis.md @@ -792,9 +792,8 @@ or the line between the two farthest apart for points on one straight line on th a point or line layer when every hull is one, and the tool says so; otherwise it's a polygon layer holding any points and lines among its polygons. Dissolve leaves those out, saying how many; so do Clip and the overlay tools when the layer is their mask or overlay; a Shapefile export leaves -them out, saying how many; and a -GeoPackage export keeps them in a column that holds any kind of geometry. It says how many of -each. +them out, saying how many; and a GeoPackage export keeps them in a column that holds any kind of +geometry. It says how many of each. **Dissolve** (`dissolve`) merges the polygons sharing a value of `field` into one feature for each value, or all of them into one without a field. Each feature has the value and `count`, the From ba84dd109725889b3c0e77ca6ce3cbe6ee3402e5 Mon Sep 17 00:00:00 2001 From: Cory Douglas Martin <4108748+flight1973@users.noreply.github.com> Date: Tue, 10 Nov 2026 16:59:01 -0600 Subject: [PATCH 11/11] docs(analysis): wording of the mixed error, the used-once rule for every overlay tool Review follow-up. The error naming both kinds read "2 a polygon with no area"; it says "2 with no area" now. The user guide says a feature selected twice is used once in every tool of the shared paragraph, not under Intersect alone, and the Minimum Bounding Geometry paragraph says the tool counts the hulls with no area, rather than seeming to say the GeoPackage export counts each kind. A test comment is corrected. --- crates/analysis/src/tools.rs | 8 ++++---- docs/user-guide/analysis.md | 5 +++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/crates/analysis/src/tools.rs b/crates/analysis/src/tools.rs index 5cf0deff1..0e32bb9a6 100644 --- a/crates/analysis/src/tools.rs +++ b/crates/analysis/src/tools.rs @@ -1293,7 +1293,7 @@ fn nothing_covered(tool: &str, layer: &str, left: LeftOut) -> String { let what = match (left.no_polygon, left.no_area) { (_, 0) => NO_POLYGON.to_string(), (0, _) => NO_AREA.to_string(), - (a, b) => format!("{a} with no polygon ({NO_POLYGON}), {b} {NO_AREA}"), + (a, b) => format!("{a} with no polygon ({NO_POLYGON}), {b} with no area"), }; format!("{tool}: none of the {} feature(s) of '{layer}' covers anything: {what}", left.no_polygon + left.no_area) } @@ -7115,8 +7115,8 @@ mod tests { masks.push(Some(Geometry::Polygon(vec![])), vec![Value::Int(13)]); masks.push(Some(Geometry::Polygon(vec![vec![]])), vec![Value::Int(14)]); masks.push(None, vec![Value::Int(15)]); - // Update writes an update polygon as it is, so one with no area changes its output; - // the other overlay tools keep one part with no area as a polygon, as it is. + // Update writes an update polygon as it is, so this one would change its output; + // the others leave it out, counted. if tool != "update" { // Two polygons, each a ring on a line. masks.push(Some(Geometry::Polygon(vec![vec![flat(1.0)], vec![flat(1.5)]])), vec![Value::Int(16)]); @@ -7208,7 +7208,7 @@ mod tests { both.push(Some(Geometry::Polygon(vec![vec![flat(1.0)]])), vec![]); assert_eq!( run("clip", "clip", both, None).err().unwrap_or_default(), - format!("Clip: none of the 2 feature(s) of 'masks' covers anything: 1 with no polygon ({NO_POLYGON}), 1 {NO_AREA}") + format!("Clip: none of the 2 feature(s) of 'masks' covers anything: 1 with no polygon ({NO_POLYGON}), 1 with no area") ); } diff --git a/docs/user-guide/analysis.md b/docs/user-guide/analysis.md index b7c67778d..3c1288fab 100644 --- a/docs/user-guide/analysis.md +++ b/docs/user-guide/analysis.md @@ -617,7 +617,8 @@ without geometry, a point or line in the polygon layer (as Minimum Bounding Geom are), or an empty polygon. Clip also leaves out a mask whose polygons, unioned, have no area, and the others a feature of several parts that, unioned, come to nothing (Update writes it as it is), saying how many. When none of the features of that layer is left, Clip, Intersect and Split, whose -result would be empty, stop with an error. +result would be empty, stop with an error. In all of them, a feature selected twice in that layer is +used once. **Near** (`near`) copies `input` and adds, for each feature, the nearest feature of `near`: its OID as the attribute table shows it (`near_fid`), the distance to it (`near_dist`, in `units`, @@ -793,7 +794,7 @@ a point or line layer when every hull is one, and the tool says so; otherwise it holding any points and lines among its polygons. Dissolve leaves those out, saying how many; so do Clip and the overlay tools when the layer is their mask or overlay; a Shapefile export leaves them out, saying how many; and a GeoPackage export keeps them in a column that holds any kind of -geometry. It says how many of each. +geometry. The tool says how many hulls have no area. **Dissolve** (`dissolve`) merges the polygons sharing a value of `field` into one feature for each value, or all of them into one without a field. Each feature has the value and `count`, the