From 803216bf6390148477614c7ecf769ee98da328c5 Mon Sep 17 00:00:00 2001 From: quanguet0409 Date: Fri, 23 Oct 2026 07:47:04 +0700 Subject: [PATCH 01/11] feat(analysis,engine): add zonal statistics geoprocessing tool (gp.zonalStats) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Implement numerically stable statistical accumulator (Welford's algorithm) for mean and population stddev - Implement in-place polygon raster mask sampling with scanline hole subtraction - Add zonalStats geoprocessing tool with multi-band support, stat selection, and field collision resolution - Register gp.zonalStats command in the engine under 'Analyze › Raster' - Add Zonal Statistics workflow section in the Analysis UI pane - Add unit and end-to-end regression tests covering polygon holes, NoData handling, and undo/redo --- crates/analysis/src/rasterops.rs | 277 +++++++++++++++++ crates/analysis/src/tools.rs | 473 +++++++++++++++++++++++++++++ crates/engine/src/cmd/gp.rs | 10 + crates/engine/src/tests.rs | 115 ++++++- crates/ui-egui/src/analysispane.rs | 41 +++ 5 files changed, 915 insertions(+), 1 deletion(-) diff --git a/crates/analysis/src/rasterops.rs b/crates/analysis/src/rasterops.rs index ef44aae51..e8710c22a 100644 --- a/crates/analysis/src/rasterops.rs +++ b/crates/analysis/src/rasterops.rs @@ -601,6 +601,127 @@ pub fn polygon_mask(r: &Raster, polygons: &[Vec>]) -> Option<(Range, + /// Arithmetic mean of valid raster cells inside the zone. + pub mean: Option, + /// Minimum value among valid raster cells inside the zone. + pub min: Option, + /// Maximum value among valid raster cells inside the zone. + pub max: Option, + /// Population standard deviation of valid raster cells inside the zone: sqrt(sum((x - mean)^2) / n). + pub stddev: Option, +} + +/// Numerically stable online accumulator using Welford's algorithm. +#[derive(Clone, Debug, Default)] +pub struct Accumulator { + count: usize, + sum: f64, + min: f64, + max: f64, + mean: f64, + m2: f64, +} + +impl Accumulator { + pub fn new() -> Self { + Self { count: 0, sum: 0.0, min: f64::INFINITY, max: f64::NEG_INFINITY, mean: 0.0, m2: 0.0 } + } + + /// Feeds a valid finite measurement into the accumulator. + pub fn add(&mut self, val: f64) { + if !val.is_finite() { + return; + } + self.count += 1; + self.sum += val; + if val < self.min { + self.min = val; + } + if val > self.max { + self.max = val; + } + let delta = val - self.mean; + self.mean += delta / (self.count as f64); + let delta2 = val - self.mean; + self.m2 += delta * delta2; + } + + pub fn count(&self) -> usize { + self.count + } + + pub fn stats(&self) -> ZoneStats { + if self.count == 0 { + ZoneStats::default() + } else { + let variance = (self.m2 / (self.count as f64)).max(0.0); + ZoneStats { + count: self.count, + sum: Some(self.sum), + mean: Some(self.mean), + min: Some(self.min), + max: Some(self.max), + stddev: Some(variance.sqrt()), + } + } + } +} + +/// Computes summary statistics (count, sum, mean, min, max, stddev) for each requested band +/// over the raster cells whose centres fall inside `polygons` (exterior ring and holes in WGS 84 lon/lat). +/// +/// Returns one `ZoneStats` per requested band. Cells with NoData, NaN, or non-finite values are +/// excluded. If no valid cells fall inside the polygon or if the polygon is outside the raster extent, +/// returns `ZoneStats` with `count = 0` and all numeric statistics set to `None`. +pub fn zonal_statistics(r: &Raster, bands: &[usize], polygons: &[Vec>]) -> Result, String> { + r.require_coordinates()?; + for &b in bands { + if b >= r.band_count() { + return Err(format!("band index {} is out of range: {} has {} bands", b + 1, r.name, r.band_count())); + } + } + if bands.is_empty() { + return Ok(Vec::new()); + } + + let Some((cols, rows, mask)) = polygon_mask(r, polygons) else { + return Ok(vec![ZoneStats::default(); bands.len()]); + }; + + let w = (cols.end - cols.start) as usize; + let mut accs: Vec = (0..bands.len()).map(|_| Accumulator::new()).collect(); + + for (ri, row) in rows.enumerate() { + let line_offset = ri * w; + let line = match mask.get(line_offset..line_offset + w) { + Some(l) => l, + None => continue, + }; + for (ci, &inside) in line.iter().enumerate() { + if !inside { + continue; + } + let col = cols.start + ci as u32; + for (acc_idx, &band) in bands.iter().enumerate() { + if let Some(v) = r.value(band, col as i64, row as i64) + && let Some(acc) = accs.get_mut(acc_idx) + { + acc.add(f64::from(v)); + } + } + } + } + + Ok(accs.into_iter().map(|acc| acc.stats()).collect()) +} + /// A tiny arithmetic expression over band values. enum Expr { Num(f64), @@ -785,4 +906,160 @@ mod tests { assert_eq!(p.len(), 20); assert!(p.last().and_then(|x| x.2).unwrap_or(0.0) > p[0].2.unwrap_or(0.0) + 150.0); } + + #[test] + fn accumulator_exact_math_and_numerical_stability() { + // 1. Exact math on [10.0, 20.0, 30.0, 40.0] + let mut acc = Accumulator::new(); + for &x in &[10.0, 20.0, 30.0, 40.0] { + acc.add(x); + } + let st = acc.stats(); + assert_eq!(st.count, 4); + assert_eq!(st.sum, Some(100.0)); + assert_eq!(st.mean, Some(25.0)); + assert_eq!(st.min, Some(10.0)); + assert_eq!(st.max, Some(40.0)); + let expected_stddev = 125.0f64.sqrt(); // sqrt(500 / 4) + assert!((st.stddev.unwrap() - expected_stddev).abs() < 1e-12); + + // 2. Empty accumulator + let empty_st = Accumulator::new().stats(); + assert_eq!(empty_st.count, 0); + assert_eq!(empty_st.sum, None); + assert_eq!(empty_st.mean, None); + assert_eq!(empty_st.min, None); + assert_eq!(empty_st.max, None); + assert_eq!(empty_st.stddev, None); + + // 3. Single value + let mut single_acc = Accumulator::new(); + single_acc.add(42.5); + let single_st = single_acc.stats(); + assert_eq!(single_st.count, 1); + assert_eq!(single_st.sum, Some(42.5)); + assert_eq!(single_st.mean, Some(42.5)); + assert_eq!(single_st.min, Some(42.5)); + assert_eq!(single_st.max, Some(42.5)); + assert_eq!(single_st.stddev, Some(0.0)); + + // 4. Constant values + let mut const_acc = Accumulator::new(); + for _ in 0..100 { + const_acc.add(7.0); + } + let const_st = const_acc.stats(); + assert_eq!(const_st.count, 100); + assert_eq!(const_st.mean, Some(7.0)); + assert_eq!(const_st.stddev, Some(0.0)); + + // 5. Large magnitude values (Welford's algorithm avoids catastrophic cancellation) + let base = 1e12; + let mut large_acc = Accumulator::new(); + for &x in &[base + 1.0, base + 2.0, base + 3.0, base + 4.0] { + large_acc.add(x); + } + let large_st = large_acc.stats(); + assert_eq!(large_st.count, 4); + assert_eq!(large_st.mean, Some(base + 2.5)); + let std_large = large_st.stddev.unwrap(); + // Variance of [1, 2, 3, 4] is 1.25, stddev is sqrt(1.25) = 1.118033988749895 + assert!((std_large - 1.25f64.sqrt()).abs() < 1e-10, "large magnitude precision: {std_large}"); + + // 6. Non-finite values (NaN and inf) are rejected + let mut defensive_acc = Accumulator::new(); + defensive_acc.add(10.0); + defensive_acc.add(f64::NAN); + defensive_acc.add(f64::INFINITY); + defensive_acc.add(f64::NEG_INFINITY); + defensive_acc.add(20.0); + let def_st = defensive_acc.stats(); + assert_eq!(def_st.count, 2); + assert_eq!(def_st.sum, Some(30.0)); + assert_eq!(def_st.mean, Some(15.0)); + } + + #[test] + fn zonal_statistics_known_cells_nodata_and_holes() { + let r = ramp(); // 20x20 raster, band 0: (i % 20) * 10, band 1: i / 20. + let sq = |x0: f64, y0: f64, x1: f64, y1: f64| { + vec![Coord::new(x0, y0), Coord::new(x1, y0), Coord::new(x1, y1), Coord::new(x0, y1), Coord::new(x0, y0)] + }; + + // 1. Single known cell: col 5, row 5. + // origin_x = 0.0, origin_y = 0.02, pixel_width = 0.001, pixel_height = -0.001 + // col 5 x in [0.005, 0.006], centre at 0.0055. + // row 5 y in [0.014, 0.015], centre at 0.0145. + let cell_poly = vec![vec![sq(0.005, 0.014, 0.006, 0.015)]]; + let stats = zonal_statistics(&r, &[0, 1], &cell_poly).unwrap(); + assert_eq!(stats.len(), 2); + assert_eq!(stats[0].count, 1); + assert_eq!(stats[0].sum, Some(50.0)); // col 5 * 10 = 50 + assert_eq!(stats[0].mean, Some(50.0)); + assert_eq!(stats[0].min, Some(50.0)); + assert_eq!(stats[0].max, Some(50.0)); + assert_eq!(stats[0].stddev, Some(0.0)); + + assert_eq!(stats[1].count, 1); + assert_eq!(stats[1].sum, Some(5.0)); // row 5 = 5 + assert_eq!(stats[1].mean, Some(5.0)); + assert_eq!(stats[1].stddev, Some(0.0)); + + // 2. Polygon outside raster: empty zone result + let outside_poly = vec![vec![sq(1.0, 1.0, 2.0, 2.0)]]; + let outside_stats = zonal_statistics(&r, &[0], &outside_poly).unwrap(); + assert_eq!(outside_stats.len(), 1); + assert_eq!(outside_stats[0], ZoneStats::default()); + + // 3. Polygon with holes + let poly_with_hole = vec![vec![sq(0.005, 0.005, 0.015, 0.015), sq(0.009, 0.009, 0.011, 0.011)]]; + let hole_stats = zonal_statistics(&r, &[0], &poly_with_hole).unwrap(); + // 100 cells - 4 hole cells = 96 cells + assert_eq!(hole_stats[0].count, 96); + assert!(hole_stats[0].sum.is_some()); + assert!(hole_stats[0].mean.is_some()); + + // 4. MultiPolygon + let multipoly = vec![ + vec![sq(0.001, 0.018, 0.003, 0.019)], // 2 cells + vec![sq(0.005, 0.018, 0.007, 0.019)], // 2 cells + ]; + let multi_stats = zonal_statistics(&r, &[0], &multipoly).unwrap(); + assert_eq!(multi_stats[0].count, 4); + + // 5. NoData handling: create a raster with NoData values + let (w, h) = (4u32, 4u32); + // cells: + // [ 10, 20, -9999, 40 ] + // [ 50, NaN, 70, 80 ] + // [ -9999, -9999, -9999, -9999 ] + // [ 130, 140, 150, 160 ] + let data: Vec = + vec![10.0, 20.0, -9999.0, 40.0, 50.0, f32::NAN, 70.0, 80.0, -9999.0, -9999.0, -9999.0, -9999.0, 130.0, 140.0, 150.0, 160.0]; + let gt = GeoTransform { origin_x: 0.0, origin_y: 4.0, pixel_width: 1.0, pixel_height: -1.0 }; + let r_nodata = Raster::new("nd", w, h, vec![data], gt, Crs::LonLat, Some(-9999.0), ColorHint::Gray, "Float32").unwrap(); + + // Zone covering entire row 0: cells are 10, 20, -9999 (NoData), 40 + // Valid values: 10, 20, 40 (count = 3, sum = 70, mean = 70/3, min = 10, max = 40) + let row0_poly = vec![vec![sq(0.0, 3.0, 4.0, 4.0)]]; + let r0_stats = zonal_statistics(&r_nodata, &[0], &row0_poly).unwrap(); + assert_eq!(r0_stats[0].count, 3); + assert_eq!(r0_stats[0].sum, Some(70.0)); + assert_eq!(r0_stats[0].min, Some(10.0)); + assert_eq!(r0_stats[0].max, Some(40.0)); + assert!((r0_stats[0].mean.unwrap() - 70.0 / 3.0).abs() < 1e-10); + + // Zone covering row 2 where ALL cells are NoData (-9999) + let row2_poly = vec![vec![sq(0.0, 1.0, 4.0, 2.0)]]; + let r2_stats = zonal_statistics(&r_nodata, &[0], &row2_poly).unwrap(); + assert_eq!(r2_stats[0].count, 0); + assert_eq!(r2_stats[0].sum, None); + assert_eq!(r2_stats[0].mean, None); + assert_eq!(r2_stats[0].min, None); + assert_eq!(r2_stats[0].max, None); + assert_eq!(r2_stats[0].stddev, None); + + // 6. Invalid band index returns Err + assert!(zonal_statistics(&r, &[99], &cell_poly).is_err()); + } } diff --git a/crates/analysis/src/tools.rs b/crates/analysis/src/tools.rs index 1ffc9ec2f..33ed13de7 100644 --- a/crates/analysis/src/tools.rs +++ b/crates/analysis/src/tools.rs @@ -456,6 +456,56 @@ pub static TOOLS: &[ToolSpec] = &[ ], run: kriging_interpolation, }, + ToolSpec { + id: "zonalStats", + name: "Zonal Statistics", + toolbox: "Analysis › Raster", + summary: "Calculates raster summary statistics for each polygon feature.", + params: &[ + P { + name: "raster", + label: "Input Raster", + kind: K::RasterLayer, + required: true, + default: "", + help: "The raster containing pixel values to summarize.", + }, + P { + name: "polygons", + label: "Zone Polygons", + kind: K::PolygonLayer, + required: true, + default: "", + help: "Polygon layer defining the zones.", + }, + P { + name: "bands", + label: "Bands", + kind: K::Text, + required: false, + default: "\"1\"", + help: "1-based band number(s) (e.g. 1 or [1, 2]); defaults to band 1.", + }, + P { + name: "stats", + label: "Statistics", + kind: K::Text, + required: false, + default: "\"All\"", + help: "Statistical operations: All, Mean, Min, Max, Sum, Count, StdDev (comma-separated or array); defaults to All.", + }, + P { + name: "prefix", + label: "Field Prefix", + kind: K::Text, + required: false, + default: "", + help: "Prefix for generated fields; defaults to raster/band name.", + }, + P { name: "output", label: "Output Name", kind: K::Text, required: false, default: "", help: "Output polygon layer name." }, + ], + run: zonal_stats, + }, ]; // --------------------------------------------------------------------------------------------- @@ -1422,6 +1472,234 @@ fn spatial_join(inp: &dyn Inputs, p: &Json) -> Result { result(out, format!("Spatial Join: {written_features} features joined ({total_matches} matches)")) } +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum StatOp { + Mean, + Min, + Max, + Sum, + Count, + StdDev, +} + +impl StatOp { + fn name(self) -> &'static str { + match self { + StatOp::Mean => "mean", + StatOp::Min => "min", + StatOp::Max => "max", + StatOp::Sum => "sum", + StatOp::Count => "count", + StatOp::StdDev => "stddev", + } + } + + fn field_type(self) -> FieldType { + match self { + StatOp::Count => FieldType::Integer, + _ => FieldType::Double, + } + } + + fn parse(raw: &str) -> Result, String> { + let clean = raw.trim().to_ascii_lowercase(); + match clean.as_str() { + "all" | "" => Ok(vec![StatOp::Mean, StatOp::Min, StatOp::Max, StatOp::Sum, StatOp::Count, StatOp::StdDev]), + "mean" => Ok(vec![StatOp::Mean]), + "min" => Ok(vec![StatOp::Min]), + "max" => Ok(vec![StatOp::Max]), + "sum" => Ok(vec![StatOp::Sum]), + "count" => Ok(vec![StatOp::Count]), + "stddev" | "std" | "std_dev" => Ok(vec![StatOp::StdDev]), + other => Err(format!("unknown statistic '{other}': choose from mean, min, max, sum, count, stddev, or all")), + } + } +} + +fn parse_bands(p: &Json, total_bands: usize) -> Result, String> { + let mut bands = Vec::new(); + if let Some(arr) = p.get("bands").and_then(Json::as_array) { + for item in arr { + let b = match item { + Json::Number(n) => n.as_u64().map(|v| v as usize), + Json::String(str_val) => str_val.trim().parse::().ok(), + _ => None, + }; + let b = b.ok_or_else(|| "band numbers must be positive integers".to_string())?; + bands.push(b); + } + } else if let Some(n) = p.get("bands").and_then(Json::as_u64) { + bands.push(n as usize); + } else { + let txt = s(p, "bands").trim(); + if txt.is_empty() || txt.eq_ignore_ascii_case("all") { + bands.push(1); + } else { + let clean = txt.trim_matches(|c| c == '[' || c == ']'); + for part in clean.split([',', ';', ' ']) { + let part = part.trim(); + if part.is_empty() { + continue; + } + let b = part.parse::().map_err(|_| format!("invalid band number '{part}'"))?; + bands.push(b); + } + } + } + if bands.is_empty() { + bands.push(1); + } + for &b in &bands { + if b < 1 || b > total_bands { + return Err(format!("band index {b} is invalid: raster has bands 1 to {total_bands}")); + } + } + Ok(bands.into_iter().map(|b| b - 1).collect()) +} + +fn parse_stats(p: &Json) -> Result, String> { + let mut out = Vec::new(); + if let Some(arr) = p.get("stats").and_then(Json::as_array) { + if arr.is_empty() { + return Ok(vec![StatOp::Mean, StatOp::Min, StatOp::Max, StatOp::Sum, StatOp::Count, StatOp::StdDev]); + } + for item in arr { + let txt = item.as_str().ok_or_else(|| "statistic names must be strings".to_string())?; + let ops = StatOp::parse(txt)?; + for op in ops { + if !out.contains(&op) { + out.push(op); + } + } + } + } else { + let txt = s(p, "stats").trim(); + if txt.is_empty() { + return Ok(vec![StatOp::Mean, StatOp::Min, StatOp::Max, StatOp::Sum, StatOp::Count, StatOp::StdDev]); + } + let clean = txt.trim_matches(|c| c == '[' || c == ']'); + for part in clean.split([',', ';']) { + let part = part.trim().trim_matches('"'); + if part.is_empty() { + continue; + } + let ops = StatOp::parse(part)?; + for op in ops { + if !out.contains(&op) { + out.push(op); + } + } + } + } + if out.is_empty() { + out = vec![StatOp::Mean, StatOp::Min, StatOp::Max, StatOp::Sum, StatOp::Count, StatOp::StdDev]; + } + Ok(out) +} + +fn zonal_stats(inp: &dyn Inputs, p: &Json) -> Result { + let r_name = { + let n = s(p, "raster"); + if n.is_empty() { s(p, "layer") } else { n } + }; + if r_name.is_empty() { + return Err("parameter 'raster' is required".into()); + } + let r_input = inp.raster(r_name).ok_or_else(|| format!("no raster layer named '{r_name}'"))?; + let r = r_input.raster; + r.require_coordinates()?; + + let poly_param = if !s(p, "polygons").is_empty() { "polygons" } else { "input" }; + let poly_ds = input(inp, p, poly_param)?; + if poly_ds.geometry_type != GeometryType::Polygon { + return Err(format!( + "layer '{}' is {} data; zonal statistics needs polygon geometries", + poly_ds.name, + poly_ds.geometry_type.label() + )); + } + if let Some(poly_crs) = poly_ds.crs { + poly_crs.transformer().map_err(|e| format!("polygon layer coordinate system isn't supported: {e}"))?; + } + + let bands = parse_bands(p, r.band_count())?; + let stats = parse_stats(p)?; + + let mut out = copy_schema(poly_ds, out_name(p, format!("{}_ZonalStats", poly_ds.name)), poly_ds.geometry_type); + out.source = None; + out.crs = poly_ds.crs; + + let user_prefix = s(p, "prefix").trim(); + let multi_bands = bands.len() > 1; + let mut out_fields = Vec::new(); + let mut new_field_names: Vec = Vec::new(); + + for (b_idx, &band) in bands.iter().enumerate() { + let band_num = band + 1; + let band_name = r.bands.get(band).map(|b| b.name.as_str()).unwrap_or(""); + let prefix = if !user_prefix.is_empty() { + if multi_bands { format!("{user_prefix}_b{band_num}") } else { user_prefix.to_string() } + } else if multi_bands { + if !band_name.is_empty() && !band_name.starts_with("Band ") && band_name != "Gray" { + format!("{}_{}", r.name, band_name) + } else { + format!("{}_b{band_num}", r.name) + } + } else { + if !band_name.is_empty() && !band_name.starts_with("Band ") && band_name != "Gray" { + band_name.to_string() + } else { + r.name.to_string() + } + }; + + for &stat in &stats { + let base_name = format!("{prefix}_{}", stat.name()); + let mut final_name = base_name.clone(); + let mut counter = 1usize; + while out.field_index(&final_name).is_some() || new_field_names.iter().any(|n| n.eq_ignore_ascii_case(&final_name)) { + counter += 1; + final_name = format!("{base_name}_{counter}"); + } + new_field_names.push(final_name.clone()); + out_fields.push((b_idx, band, stat, final_name, stat.field_type())); + } + } + + for (_, _, _, name, ty) in &out_fields { + out.fields.push(Field::new(name.clone(), *ty)); + } + + let target_rows = rows(inp, p, poly_param, poly_ds); + for r_idx in target_rows { + let Some(f) = poly_ds.features.get(r_idx) else { continue }; + let stats_by_band = match &f.geometry { + Some(Geometry::Polygon(polys)) if !polys.is_empty() => crate::rasterops::zonal_statistics(r, &bands, polys)?, + _ => vec![crate::rasterops::ZoneStats::default(); bands.len()], + }; + + let mut attrs = f.attrs.clone(); + for (b_idx, _, stat, _, _) in &out_fields { + let b_stats = stats_by_band.get(*b_idx); + let val = match (b_stats, stat) { + (Some(s), StatOp::Count) => Value::Int(s.count as i64), + (Some(s), StatOp::Mean) => s.mean.map(Value::Float).unwrap_or(Value::Null), + (Some(s), StatOp::Min) => s.min.map(Value::Float).unwrap_or(Value::Null), + (Some(s), StatOp::Max) => s.max.map(Value::Float).unwrap_or(Value::Null), + (Some(s), StatOp::Sum) => s.sum.map(Value::Float).unwrap_or(Value::Null), + (Some(s), StatOp::StdDev) => s.stddev.map(Value::Float).unwrap_or(Value::Null), + _ => Value::Null, + }; + attrs.push(val); + } + attrs.resize(out.fields.len(), Value::Null); + out.features.push(rustgis_data::Feature { id: f.id, geometry: f.geometry.clone(), attrs }); + } + + let n_features = out.features.len(); + result(out, format!("Calculated zonal statistics for {n_features} feature(s)")) +} + #[cfg(test)] mod tests { use super::*; @@ -2199,4 +2477,199 @@ mod tests { let err = crate::run("spatialJoin", &m_crs, &serde_json::json!({"target": "pts", "join": "polys"})).unwrap_err(); assert!(err.contains("coordinate reference system")); } + + #[test] + fn zonal_stats_tool_cases() { + struct RasterMap<'a> { + datasets: HashMap, + rasters: HashMap, + } + + impl<'a> Inputs for RasterMap<'a> { + fn dataset(&self, layer: &str) -> Option<&Dataset> { + self.datasets.get(layer) + } + fn selection(&self, _: &str) -> &[usize] { + &[] + } + fn raster(&self, layer: &str) -> Option> { + let r = self.rasters.get(layer)?; + Some(crate::RasterInput { raster: r, path: None, embedded: None }) + } + } + + let (w, h) = (10u32, 10u32); + let mut b1_data: Vec = Vec::with_capacity((w * h) as usize); + let mut b2_data: Vec = Vec::with_capacity((w * h) as usize); + for row in 0..h { + for col in 0..w { + b1_data.push((col * 10 + row) as f32); + let v2 = 100.0 - (col * 10 + row) as f32; + if col == 1 && row == 1 { + b2_data.push(-9999.0); + } else { + b2_data.push(v2); + } + } + } + let gt = rustgis_data::raster::GeoTransform { origin_x: 0.0, origin_y: 10.0, pixel_width: 1.0, pixel_height: -1.0 }; + let raster = rustgis_data::raster::Raster::new( + "elev", + w, + h, + vec![b1_data, b2_data], + gt, + rustgis_data::raster::Crs::LonLat, + Some(-9999.0), + rustgis_data::raster::ColorHint::Gray, + "Float32", + ) + .unwrap(); + + let mut poly_ds = Dataset::new( + "zones", + GeometryType::Polygon, + vec![Field::new("zone_id", FieldType::Integer), Field::new("elev_mean", FieldType::Double)], + ); + + let sq_geom = |x0: f64, y0: f64, x1: f64, y1: f64| { + Geometry::Polygon(vec![vec![vec![ + Coord::new(x0, y0), + Coord::new(x1, y0), + Coord::new(x1, y1), + Coord::new(x0, y1), + Coord::new(x0, y0), + ]]]) + }; + + poly_ds.push(Some(sq_geom(1.0, 7.0, 3.0, 9.0)), vec![Value::Int(101), Value::Float(999.0)]); + poly_ds.push(Some(sq_geom(20.0, 20.0, 25.0, 25.0)), vec![Value::Int(102), Value::Float(888.0)]); + + let hole_geom = Geometry::Polygon(vec![vec![ + vec![Coord::new(0.0, 7.0), Coord::new(3.0, 7.0), Coord::new(3.0, 10.0), Coord::new(0.0, 10.0), Coord::new(0.0, 7.0)], + vec![Coord::new(1.0, 8.0), Coord::new(2.0, 8.0), Coord::new(2.0, 9.0), Coord::new(1.0, 9.0), Coord::new(1.0, 8.0)], + ]]); + poly_ds.push(Some(hole_geom), vec![Value::Int(103), Value::Float(777.0)]); + poly_ds.push(None, vec![Value::Int(104), Value::Float(666.0)]); + + let input_clone = poly_ds.clone(); + let m = RasterMap { + datasets: HashMap::from([("zones".to_string(), poly_ds)]), + rasters: HashMap::from([("elev".to_string(), &raster)]), + }; + + let res = crate::run( + "zonalStats", + &m, + &serde_json::json!({ + "raster": "elev", + "polygons": "zones", + "bands": [1], + }), + ) + .unwrap(); + + let out = layer(res); + assert_eq!(out.features.len(), 4); + + assert_eq!(out.fields[0].name, "zone_id"); + assert_eq!(out.fields[1].name, "elev_mean"); + assert_eq!(out.value(0, 0), &Value::Int(101)); + assert_eq!(out.value(0, 1), &Value::Float(999.0)); + + let mean_idx = out.field_index("elev_mean_2").expect("collision should produce elev_mean_2"); + let min_idx = out.field_index("elev_min").unwrap(); + let max_idx = out.field_index("elev_max").unwrap(); + let sum_idx = out.field_index("elev_sum").unwrap(); + let count_idx = out.field_index("elev_count").unwrap(); + let stddev_idx = out.field_index("elev_stddev").unwrap(); + + assert_eq!(out.value(0, count_idx), &Value::Int(4)); + assert_eq!(out.value(0, sum_idx), &Value::Float(66.0)); + assert_eq!(out.value(0, min_idx), &Value::Float(11.0)); + assert_eq!(out.value(0, max_idx), &Value::Float(22.0)); + assert_eq!(out.value(0, mean_idx), &Value::Float(16.5)); + let std_val = out.value(0, stddev_idx).as_f64().unwrap(); + assert!((std_val - 25.25f64.sqrt()).abs() < 1e-10); + + assert_eq!(out.value(1, count_idx), &Value::Int(0)); + assert_eq!(out.value(1, sum_idx), &Value::Null); + assert_eq!(out.value(1, mean_idx), &Value::Null); + assert_eq!(out.value(1, min_idx), &Value::Null); + assert_eq!(out.value(1, max_idx), &Value::Null); + assert_eq!(out.value(1, stddev_idx), &Value::Null); + + assert_eq!(out.value(2, count_idx), &Value::Int(8)); + + assert_eq!(out.value(3, count_idx), &Value::Int(0)); + assert_eq!(out.value(3, mean_idx), &Value::Null); + + let res_mb = crate::run( + "zonalStats", + &m, + &serde_json::json!({ + "raster": "elev", + "polygons": "zones", + "bands": [1, 2], + "stats": ["mean", "count"], + }), + ) + .unwrap(); + let out_mb = layer(res_mb); + let b1_count = out_mb.field_index("elev_b1_count").unwrap(); + let b2_count = out_mb.field_index("elev_b2_count").unwrap(); + let b2_mean = out_mb.field_index("elev_b2_mean").unwrap(); + + assert_eq!(out_mb.value(0, b1_count), &Value::Int(4)); + assert_eq!(out_mb.value(0, b2_count), &Value::Int(3)); + let b2_m = out_mb.value(0, b2_mean).as_f64().unwrap(); + assert!((b2_m - 245.0 / 3.0).abs() < 1e-10); + + let mut pt_ds = Dataset::new("points", GeometryType::Point, vec![]); + pt_ds.push(Some(Geometry::Point(Coord::new(1.0, 8.0))), vec![]); + let m_pts = + RasterMap { datasets: HashMap::from([("points".to_string(), pt_ds)]), rasters: HashMap::from([("elev".to_string(), &raster)]) }; + let err_pt = crate::run( + "zonalStats", + &m_pts, + &serde_json::json!({ + "raster": "elev", + "polygons": "points", + }), + ) + .unwrap_err(); + assert!(err_pt.contains("Point data")); + + let err_band = crate::run( + "zonalStats", + &m, + &serde_json::json!({ + "raster": "elev", + "polygons": "zones", + "bands": [5], + }), + ) + .unwrap_err(); + assert!(err_band.contains("band index 5 is invalid")); + + let empty_ds = Dataset::new("empty", GeometryType::Polygon, vec![Field::new("id", FieldType::Integer)]); + let m_empty = RasterMap { + datasets: HashMap::from([("empty".to_string(), empty_ds)]), + rasters: HashMap::from([("elev".to_string(), &raster)]), + }; + let res_empty = crate::run( + "zonalStats", + &m_empty, + &serde_json::json!({ + "raster": "elev", + "polygons": "empty", + }), + ) + .unwrap(); + let out_empty = layer(res_empty); + assert_eq!(out_empty.features.len(), 0); + assert!(out_empty.field_index("elev_mean").is_some()); + + assert_eq!(m.datasets["zones"], input_clone); + } } diff --git a/crates/engine/src/cmd/gp.rs b/crates/engine/src/cmd/gp.rs index 9fcaf7075..1682cb644 100644 --- a/crates/engine/src/cmd/gp.rs +++ b/crates/engine/src/cmd/gp.rs @@ -15,6 +15,11 @@ pub fn commands() -> Vec { .params( r#"{"target": layer, "join": layer, "match"?: "Intersects" (default) | "Contains" | "Within", "operation"?: "Join one to one" (default) | "Join one to many", "keepAll"?: "Keep all" (default) | "Keep only matching", "prefix"?: "join_", "output"?: text}"#, ), + C::new("gp.zonalStats", "Zonal Statistics", "Analyze › Raster", zonal_stats) + .when(need_layers) + .params( + r#"{"raster": raster, "polygons": polygon layer, "bands"?: [1-based band numbers] or single band (default 1), "stats"?: ["mean", "min", "max", "sum", "count", "stddev"] or "all" (default "all"), "prefix"?: text, "output"?: text}"#, + ), C::new("gp.bufferPoint", "Buffer A Point", "Analyze › Geoprocessing", buffer_point).params( r#"{"lon": number, "lat": number, "distance": number, "units"?: "Kilometers" (default) | "Meters" | "Miles", "name"?: output}"#, ), @@ -701,6 +706,11 @@ fn spatial_join(s: &mut Session, p: &Value) -> CmdResult { run_tool(s, "spatialJoin", p) } +/// Calculates raster summary statistics for polygon zones. +fn zonal_stats(s: &mut Session, p: &Value) -> CmdResult { + run_tool(s, "zonalStats", p) +} + /// Runs a (built-in) tool and applies its result, as `gp.run` does. pub fn run_tool(s: &mut Session, tool: &str, params: &Value) -> CmdResult { let tool = tool_id(tool).unwrap_or(tool).to_string(); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 9bfe9a1e1..3571999d6 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -9586,7 +9586,14 @@ fn literal_keys(body: &str, prefix: &str) -> Vec { #[test] fn params_name_every_key_a_command_reads() { // Read by a shared helper but of no use to the command. - let unused = [("gp.spatialJoin", "input"), ("profile.alongSelected", "row"), ("sources.remove", "name"), ("sources.remove", "extra")]; + let unused = [ + ("gp.spatialJoin", "input"), + ("gp.zonalStats", "input"), + ("gp.zonalStats", "target"), + ("profile.alongSelected", "row"), + ("sources.remove", "name"), + ("sources.remove", "extra"), + ]; let s = Session::new(Project::default()); let mut missing = Vec::new(); let found = keys_read_by_commands(); @@ -13361,3 +13368,109 @@ fn scans_have_no_coordinates_to_give_away() { assert!(s.run("share.exportRaster", &json!({"path": out.display().to_string()})).is_ok()); let _ = std::fs::remove_dir_all(&dir); } + +#[test] +fn gp_zonal_stats_end_to_end_and_undo() { + use rustgis_data::raster::{ColorHint, Crs, GeoTransform, Raster}; + use rustgis_data::{Dataset, Field, FieldType, Geometry, GeometryType, Value}; + use rustgis_geom::Coord; + + let mut s = world(); + + let (w, h) = (10u32, 10u32); + let mut data = Vec::with_capacity((w * h) as usize); + for row in 0..h { + for col in 0..w { + data.push((col * 10 + row) as f32); + } + } + let gt = GeoTransform { origin_x: 0.0, origin_y: 10.0, pixel_width: 1.0, pixel_height: -1.0 }; + let raster = Raster::new("dem", w, h, vec![data], gt, Crs::LonLat, None, ColorHint::Gray, "Float32").unwrap(); + let raster_layer = rustgis_map::RasterLayer::from_path(raster, "dem.tif"); + s.map_mut().add_raster("dem", raster_layer); + + let mut poly_ds = Dataset::new("districts", GeometryType::Polygon, vec![Field::new("code", FieldType::Text)]); + let sq = |x0: f64, y0: f64, x1: f64, y1: f64| { + Geometry::Polygon(vec![vec![vec![ + Coord::new(x0, y0), + Coord::new(x1, y0), + Coord::new(x1, y1), + Coord::new(x0, y1), + Coord::new(x0, y0), + ]]]) + }; + poly_ds.push(Some(sq(1.0, 7.0, 3.0, 9.0)), vec![Value::Text("D1".into())]); + poly_ds.push(Some(sq(50.0, 50.0, 52.0, 52.0)), vec![Value::Text("D2".into())]); + s.map_mut().add_layer(rustgis_map::Layer::new(0, poly_ds)); + + let layers_before = s.map().layers.len(); + + let res = s.run( + "gp.zonalStats", + &json!({ + "raster": "dem", + "polygons": "districts", + "output": "zonal_out", + }), + ); + assert!(res.is_ok(), "gp.zonalStats failed: {res:?}"); + assert_eq!(s.map().layers.len(), layers_before + 1); + + let out_layer = s.map().layer_by_name("zonal_out").expect("output layer should exist"); + assert_eq!(out_layer.dataset.features.len(), 2); + + let mean_idx = out_layer.dataset.field_index("dem_mean").expect("dem_mean field"); + let count_idx = out_layer.dataset.field_index("dem_count").expect("dem_count field"); + let min_idx = out_layer.dataset.field_index("dem_min").expect("dem_min field"); + let max_idx = out_layer.dataset.field_index("dem_max").expect("dem_max field"); + let sum_idx = out_layer.dataset.field_index("dem_sum").expect("dem_sum field"); + let stddev_idx = out_layer.dataset.field_index("dem_stddev").expect("dem_stddev field"); + + assert_eq!(out_layer.dataset.value(0, 0), &Value::Text("D1".into())); + assert_eq!(out_layer.dataset.value(0, count_idx), &Value::Int(4)); + assert_eq!(out_layer.dataset.value(0, mean_idx), &Value::Float(16.5)); + assert_eq!(out_layer.dataset.value(0, min_idx), &Value::Float(11.0)); + assert_eq!(out_layer.dataset.value(0, max_idx), &Value::Float(22.0)); + assert_eq!(out_layer.dataset.value(0, sum_idx), &Value::Float(66.0)); + let std = out_layer.dataset.value(0, stddev_idx).as_f64().unwrap(); + assert!((std - 25.25f64.sqrt()).abs() < 1e-10); + + assert_eq!(out_layer.dataset.value(1, 0), &Value::Text("D2".into())); + assert_eq!(out_layer.dataset.value(1, count_idx), &Value::Int(0)); + assert_eq!(out_layer.dataset.value(1, mean_idx), &Value::Null); + assert_eq!(out_layer.dataset.value(1, min_idx), &Value::Null); + assert_eq!(out_layer.dataset.value(1, max_idx), &Value::Null); + assert_eq!(out_layer.dataset.value(1, sum_idx), &Value::Null); + assert_eq!(out_layer.dataset.value(1, stddev_idx), &Value::Null); + + assert!(s.undo()); + assert_eq!(s.map().layers.len(), layers_before); + assert!(s.map().layer_by_name("zonal_out").is_none()); + + assert!(s.redo()); + assert_eq!(s.map().layers.len(), layers_before + 1); + assert!(s.map().layer_by_name("zonal_out").is_some()); + + let run_res = s.run( + "gp.run", + &json!({ + "tool": "zonalStats", + "params": { + "raster": "dem", + "polygons": "districts", + "stats": ["mean", "count"], + "output": "via_gp_run", + } + }), + ); + assert!(run_res.is_ok(), "gp.run with zonalStats failed: {run_res:?}"); + let run_layer = s.map().layer_by_name("via_gp_run").expect("via_gp_run layer should exist"); + assert!(run_layer.dataset.field_index("dem_mean").is_some()); + assert!(run_layer.dataset.field_index("dem_count").is_some()); + assert!(run_layer.dataset.field_index("dem_min").is_none()); + + assert!(s.run("gp.zonalStats", &json!({})).is_err()); + assert!(s.run("gp.zonalStats", &json!({"raster": "nonexistent", "polygons": "districts"})).is_err()); + assert!(s.run("gp.zonalStats", &json!({"raster": "dem", "polygons": "Cities"})).is_err()); + assert!(s.run("gp.zonalStats", &json!({"raster": "dem", "polygons": "districts", "bands": [999]})).is_err()); +} diff --git a/crates/ui-egui/src/analysispane.rs b/crates/ui-egui/src/analysispane.rs index 2171e87ad..352454c75 100644 --- a/crates/ui-egui/src/analysispane.rs +++ b/crates/ui-egui/src/analysispane.rs @@ -108,6 +108,7 @@ pub const SECTIONS: &[(&str, &str)] = &[ ("rasterFunctions", "Raster Functions"), ("functionEditor", "Function Editor"), ("suitability", "Suitability Modeler"), + ("zonalStats", "Zonal Statistics"), ("visibility", "Visibility Analysis"), ("lineOfSight", "Exploratory 3D Analysis"), ("interpolate", "Geostatistical Wizard"), @@ -188,6 +189,7 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { let inputs = match w.section { "rasterFunctions" | "functionEditor" | "suitability" | "visibility" | "lineOfSight" | "georeference" | "change" | "spectral" | "classify" | "supervised" | "deepLearning" | "hosted" | "indices" | "process" | "pixels" | "mensuration" | "reality" + | "zonalStats" // The trend's one-layer mode takes a raster (a Zarr variable, or a raster of several bands). | "trend" => &rasters, "interpolate" => &points, @@ -198,6 +200,9 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { if !inputs.contains(&w.layer) { w.layer = inputs.first().cloned().unwrap_or_default(); } + if w.section == "zonalStats" && !polygons.contains(&w.layer2) { + w.layer2 = polygons.first().cloned().unwrap_or_default(); + } ui.add_space(6.0); match app.workflow.section { "rasterFunctions" | "functionEditor" => { @@ -246,6 +251,42 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { } } } + "zonalStats" => { + heading(ui, "Calculate raster statistics per polygon"); + ui.label(egui::RichText::new(tl!("Raster layer")).small().weak()); + combo(ui, "wf_zs_raster", &mut app.workflow.layer, &rasters); + ui.label(egui::RichText::new(tl!("Polygon layer")).small().weak()); + combo(ui, "wf_zs_poly", &mut app.workflow.layer2, &polygons); + ui.horizontal(|ui| { + ui.label(tl!("Band")); + ui.add(egui::TextEdit::singleline(&mut app.workflow.number).hint_text("1").desired_width(40.0)); + ui.label(tl!("Prefix")); + ui.add(egui::TextEdit::singleline(&mut app.workflow.text).hint_text("elev").desired_width(70.0)); + }); + ui.horizontal(|ui| { + ui.label(tl!("Output")); + ui.add(egui::TextEdit::singleline(&mut app.workflow.number2).hint_text(tl!("output layer name")).desired_width(120.0)); + }); + let can_run = !app.workflow.layer.is_empty() && !app.workflow.layer2.is_empty(); + if !can_run { + ui.label(egui::RichText::new(tl!("Add a raster and a polygon layer to calculate zonal statistics.")).small().weak()); + } + if ui.add_enabled(can_run, egui::Button::new(tl!("Calculate Statistics"))).clicked() { + let band = app.workflow.number.trim().parse::().unwrap_or(1); + let mut params = json!({ + "raster": app.workflow.layer, + "polygons": app.workflow.layer2, + "bands": [band], + }); + if !app.workflow.text.trim().is_empty() { + params["prefix"] = json!(app.workflow.text.trim()); + } + if !app.workflow.number2.trim().is_empty() { + params["output"] = json!(app.workflow.number2.trim()); + } + run(app, "gp.zonalStats", params); + } + } "suitability" => { heading(ui, "Score two rasters 1–10 and weight them"); ui.label(egui::RichText::new(tl!("High values of the first raster and low values of the second score best.")).small().weak()); From dc77b58521391d1a77af0a4d5ff024acd0a2be31 Mon Sep 17 00:00:00 2001 From: quanguet0409 Date: Fri, 23 Oct 2026 08:58:54 +0700 Subject: [PATCH 02/11] fix(analysis,ui): address code review for gp.zonalStats - Reproject polygon geometries to WGS 84 before raster sampling when source CRS is projected (e.g. EPSG:3857) - Fix Accumulator::default() to initialize min/max with INFINITY/NEG_INFINITY consistent with new() - Overhaul parse_bands with strict integer/finiteness validation, integral float acceptance, and 'all' expansion - Deduplicate StatOp supported operations with StatOp::ALL constant - Add UI band validation and localized amber warning in Geoprocessing Analysis pane - Add regression tests covering Accumulator default/new extrema, WebMercator reprojection, and band validation --- crates/analysis/src/rasterops.rs | 62 +++++++- crates/analysis/src/tools.rs | 238 +++++++++++++++++++++++------ crates/engine/src/tests.rs | 41 +++++ crates/ui-egui/src/analysispane.rs | 21 ++- 4 files changed, 308 insertions(+), 54 deletions(-) diff --git a/crates/analysis/src/rasterops.rs b/crates/analysis/src/rasterops.rs index e8710c22a..421fcfc1e 100644 --- a/crates/analysis/src/rasterops.rs +++ b/crates/analysis/src/rasterops.rs @@ -619,7 +619,7 @@ pub struct ZoneStats { } /// Numerically stable online accumulator using Welford's algorithm. -#[derive(Clone, Debug, Default)] +#[derive(Clone, Debug)] pub struct Accumulator { count: usize, sum: f64, @@ -629,12 +629,21 @@ pub struct Accumulator { m2: f64, } +impl Default for Accumulator { + /// Constructs an empty accumulator with `min = INFINITY` and `max = NEG_INFINITY`. + fn default() -> Self { + Self::new() + } +} + impl Accumulator { + /// Constructs a new empty accumulator with `min = INFINITY` and `max = NEG_INFINITY`. pub fn new() -> Self { Self { count: 0, sum: 0.0, min: f64::INFINITY, max: f64::NEG_INFINITY, mean: 0.0, m2: 0.0 } } - /// Feeds a valid finite measurement into the accumulator. + /// Feeds a valid finite measurement into the accumulator, updating count, sum, extrema, + /// and running mean / variance using Welford's algorithm. Non-finite values are ignored. pub fn add(&mut self, val: f64) { if !val.is_finite() { return; @@ -653,10 +662,15 @@ impl Accumulator { self.m2 += delta * delta2; } + /// Number of valid finite samples accumulated so far. pub fn count(&self) -> usize { self.count } + /// Computes and returns the accumulated [`ZoneStats`]. + /// + /// If no valid values were added (`count == 0`), returns `ZoneStats::default()` + /// where `count` is 0 and all statistical metrics are `None`. pub fn stats(&self) -> ZoneStats { if self.count == 0 { ZoneStats::default() @@ -979,6 +993,50 @@ mod tests { assert_eq!(def_st.mean, Some(15.0)); } + #[test] + fn accumulator_default_and_new_consistency_and_extrema() { + // default() and new() produce identical initial empty state + let d = Accumulator::default(); + let n = Accumulator::new(); + assert_eq!(d.count(), 0); + assert_eq!(n.count(), 0); + assert_eq!(d.stats(), ZoneStats::default()); + assert_eq!(n.stats(), ZoneStats::default()); + + // Negative-only values: min must be -30, max must be -10 (not 0.0) + let mut neg_acc = Accumulator::default(); + neg_acc.add(-30.0); + neg_acc.add(-20.0); + neg_acc.add(-10.0); + let neg_st = neg_acc.stats(); + assert_eq!(neg_st.count, 3); + assert_eq!(neg_st.min, Some(-30.0)); + assert_eq!(neg_st.max, Some(-10.0)); + assert_eq!(neg_st.sum, Some(-60.0)); + assert_eq!(neg_st.mean, Some(-20.0)); + + // Positive-only values: min must be 10 (not 0.0), max must be 30 + let mut pos_acc = Accumulator::default(); + pos_acc.add(10.0); + pos_acc.add(20.0); + pos_acc.add(30.0); + let pos_st = pos_acc.stats(); + assert_eq!(pos_st.count, 3); + assert_eq!(pos_st.min, Some(10.0)); + assert_eq!(pos_st.max, Some(30.0)); + + // Mixed positive and negative values + let mut mixed_acc = Accumulator::default(); + mixed_acc.add(-5.0); + mixed_acc.add(5.0); + let mixed_st = mixed_acc.stats(); + assert_eq!(mixed_st.count, 2); + assert_eq!(mixed_st.min, Some(-5.0)); + assert_eq!(mixed_st.max, Some(5.0)); + assert_eq!(mixed_st.sum, Some(0.0)); + assert_eq!(mixed_st.mean, Some(0.0)); + } + #[test] fn zonal_statistics_known_cells_nodata_and_holes() { let r = ramp(); // 20x20 raster, band 0: (i % 20) * 10, band 1: i / 20. diff --git a/crates/analysis/src/tools.rs b/crates/analysis/src/tools.rs index 33ed13de7..04a2f87d2 100644 --- a/crates/analysis/src/tools.rs +++ b/crates/analysis/src/tools.rs @@ -1483,6 +1483,8 @@ enum StatOp { } impl StatOp { + pub const ALL: [StatOp; 6] = [StatOp::Mean, StatOp::Min, StatOp::Max, StatOp::Sum, StatOp::Count, StatOp::StdDev]; + fn name(self) -> &'static str { match self { StatOp::Mean => "mean", @@ -1504,7 +1506,7 @@ impl StatOp { fn parse(raw: &str) -> Result, String> { let clean = raw.trim().to_ascii_lowercase(); match clean.as_str() { - "all" | "" => Ok(vec![StatOp::Mean, StatOp::Min, StatOp::Max, StatOp::Sum, StatOp::Count, StatOp::StdDev]), + "all" | "" => Ok(Self::ALL.to_vec()), "mean" => Ok(vec![StatOp::Mean]), "min" => Ok(vec![StatOp::Min]), "max" => Ok(vec![StatOp::Max]), @@ -1516,52 +1518,101 @@ impl StatOp { } } +fn parse_single_band(item: &Json, total_bands: usize, out: &mut Vec) -> Result<(), String> { + match item { + Json::Number(n) => { + let f = n.as_f64().ok_or_else(|| "invalid band number".to_string())?; + if !f.is_finite() { + return Err("band number must be a finite number".into()); + } + if f <= 0.0 { + return Err("band numbers must be positive (1-based)".into()); + } + if f.fract() != 0.0 { + return Err("band numbers must be integers, not decimals".into()); + } + let b = f as usize; + if b > total_bands { + return Err(format!("band index {b} is invalid: raster has bands 1 to {total_bands}")); + } + out.push(b); + Ok(()) + } + Json::String(s) => { + let txt = s.trim(); + if txt.eq_ignore_ascii_case("all") { + out.extend(1..=total_bands); + Ok(()) + } else { + let clean = txt.trim_matches(|c| c == '[' || c == ']'); + for part in clean.split([',', ';', ' ']) { + let part = part.trim().trim_matches('"'); + if part.is_empty() { + continue; + } + if part.eq_ignore_ascii_case("all") { + out.extend(1..=total_bands); + continue; + } + let f: f64 = part.parse().map_err(|_| format!("invalid band number '{part}'"))?; + if !f.is_finite() { + return Err("band number must be a finite number".into()); + } + if f <= 0.0 { + return Err("band numbers must be positive (1-based)".into()); + } + if f.fract() != 0.0 { + return Err("band numbers must be integers, not decimals".into()); + } + let b = f as usize; + if b > total_bands { + return Err(format!("band index {b} is invalid: raster has bands 1 to {total_bands}")); + } + out.push(b); + } + Ok(()) + } + } + _ => Err("band numbers must be positive integers or 'all'".into()), + } +} + fn parse_bands(p: &Json, total_bands: usize) -> Result, String> { - let mut bands = Vec::new(); + let mut raw_bands = Vec::new(); if let Some(arr) = p.get("bands").and_then(Json::as_array) { - for item in arr { - let b = match item { - Json::Number(n) => n.as_u64().map(|v| v as usize), - Json::String(str_val) => str_val.trim().parse::().ok(), - _ => None, - }; - let b = b.ok_or_else(|| "band numbers must be positive integers".to_string())?; - bands.push(b); - } - } else if let Some(n) = p.get("bands").and_then(Json::as_u64) { - bands.push(n as usize); - } else { - let txt = s(p, "bands").trim(); - if txt.is_empty() || txt.eq_ignore_ascii_case("all") { - bands.push(1); + if arr.is_empty() { + raw_bands.push(1); } else { - let clean = txt.trim_matches(|c| c == '[' || c == ']'); - for part in clean.split([',', ';', ' ']) { - let part = part.trim(); - if part.is_empty() { - continue; - } - let b = part.parse::().map_err(|_| format!("invalid band number '{part}'"))?; - bands.push(b); + for item in arr { + parse_single_band(item, total_bands, &mut raw_bands)?; } } + } else if let Some(v) = p.get("bands") { + if v.is_null() { + raw_bands.push(1); + } else { + parse_single_band(v, total_bands, &mut raw_bands)?; + } + } else { + raw_bands.push(1); } - if bands.is_empty() { - bands.push(1); + if raw_bands.is_empty() { + raw_bands.push(1); } - for &b in &bands { - if b < 1 || b > total_bands { - return Err(format!("band index {b} is invalid: raster has bands 1 to {total_bands}")); + let mut unique = Vec::new(); + for b in raw_bands { + if !unique.contains(&b) { + unique.push(b); } } - Ok(bands.into_iter().map(|b| b - 1).collect()) + Ok(unique.into_iter().map(|b| b - 1).collect()) } fn parse_stats(p: &Json) -> Result, String> { let mut out = Vec::new(); if let Some(arr) = p.get("stats").and_then(Json::as_array) { if arr.is_empty() { - return Ok(vec![StatOp::Mean, StatOp::Min, StatOp::Max, StatOp::Sum, StatOp::Count, StatOp::StdDev]); + return Ok(StatOp::ALL.to_vec()); } for item in arr { let txt = item.as_str().ok_or_else(|| "statistic names must be strings".to_string())?; @@ -1575,7 +1626,7 @@ fn parse_stats(p: &Json) -> Result, String> { } else { let txt = s(p, "stats").trim(); if txt.is_empty() { - return Ok(vec![StatOp::Mean, StatOp::Min, StatOp::Max, StatOp::Sum, StatOp::Count, StatOp::StdDev]); + return Ok(StatOp::ALL.to_vec()); } let clean = txt.trim_matches(|c| c == '[' || c == ']'); for part in clean.split([',', ';']) { @@ -1592,7 +1643,7 @@ fn parse_stats(p: &Json) -> Result, String> { } } if out.is_empty() { - out = vec![StatOp::Mean, StatOp::Min, StatOp::Max, StatOp::Sum, StatOp::Count, StatOp::StdDev]; + out = StatOp::ALL.to_vec(); } Ok(out) } @@ -1618,9 +1669,12 @@ fn zonal_stats(inp: &dyn Inputs, p: &Json) -> Result { poly_ds.geometry_type.label() )); } - if let Some(poly_crs) = poly_ds.crs { - poly_crs.transformer().map_err(|e| format!("polygon layer coordinate system isn't supported: {e}"))?; - } + let poly_tr = match poly_ds.crs { + Some(crs) if crs != rustgis_data::raster::Crs::LonLat && crs != rustgis_data::raster::Crs::Epsg(4326) => { + Some(crs.transformer().map_err(|e| format!("polygon layer coordinate system isn't supported: {e}"))?) + } + _ => None, + }; let bands = parse_bands(p, r.band_count())?; let stats = parse_stats(p)?; @@ -1637,20 +1691,15 @@ fn zonal_stats(inp: &dyn Inputs, p: &Json) -> Result { for (b_idx, &band) in bands.iter().enumerate() { let band_num = band + 1; let band_name = r.bands.get(band).map(|b| b.name.as_str()).unwrap_or(""); + let has_named_band = !band_name.is_empty() && !band_name.starts_with("Band ") && band_name != "Gray"; let prefix = if !user_prefix.is_empty() { if multi_bands { format!("{user_prefix}_b{band_num}") } else { user_prefix.to_string() } } else if multi_bands { - if !band_name.is_empty() && !band_name.starts_with("Band ") && band_name != "Gray" { - format!("{}_{}", r.name, band_name) - } else { - format!("{}_b{band_num}", r.name) - } + if has_named_band { format!("{}_{}", r.name, band_name) } else { format!("{}_b{band_num}", r.name) } + } else if has_named_band { + band_name.to_string() } else { - if !band_name.is_empty() && !band_name.starts_with("Band ") && band_name != "Gray" { - band_name.to_string() - } else { - r.name.to_string() - } + r.name.to_string() }; for &stat in &stats { @@ -1674,7 +1723,27 @@ fn zonal_stats(inp: &dyn Inputs, p: &Json) -> Result { for r_idx in target_rows { let Some(f) = poly_ds.features.get(r_idx) else { continue }; let stats_by_band = match &f.geometry { - Some(Geometry::Polygon(polys)) if !polys.is_empty() => crate::rasterops::zonal_statistics(r, &bands, polys)?, + Some(Geometry::Polygon(polys)) if !polys.is_empty() => { + let polys_lonlat: Vec>> = match &poly_tr { + Some(tr) => polys + .iter() + .map(|rings| { + rings + .iter() + .map(|ring| ring.iter().filter_map(|&c| tr.to_lonlat(c)).collect::>()) + .filter(|ring| ring.len() >= 3) + .collect::>() + }) + .filter(|rings| !rings.is_empty()) + .collect(), + None => polys.clone(), + }; + if polys_lonlat.is_empty() { + vec![crate::rasterops::ZoneStats::default(); bands.len()] + } else { + crate::rasterops::zonal_statistics(r, &bands, &polys_lonlat)? + } + } _ => vec![crate::rasterops::ZoneStats::default(); bands.len()], }; @@ -2670,6 +2739,79 @@ mod tests { assert_eq!(out_empty.features.len(), 0); assert!(out_empty.field_index("elev_mean").is_some()); + // 9. Bands validation: "all", integral float, decimal rejection, zero rejection, negative rejection + let res_all_bands = crate::run( + "zonalStats", + &m, + &serde_json::json!({ + "raster": "elev", + "polygons": "zones", + "bands": "all", + }), + ) + .unwrap(); + let out_all = layer(res_all_bands); + assert!(out_all.field_index("elev_b1_mean").is_some()); + assert!(out_all.field_index("elev_b2_mean").is_some()); + + let res_float_band = crate::run( + "zonalStats", + &m, + &serde_json::json!({ + "raster": "elev", + "polygons": "zones", + "bands": [1.0], + }), + ) + .unwrap(); + let out_float = layer(res_float_band); + assert!(out_float.field_index("elev_mean").is_some()); + + assert!(crate::run("zonalStats", &m, &serde_json::json!({"raster": "elev", "polygons": "zones", "bands": [1.5]})).is_err()); + assert!(crate::run("zonalStats", &m, &serde_json::json!({"raster": "elev", "polygons": "zones", "bands": [0]})).is_err()); + assert!(crate::run("zonalStats", &m, &serde_json::json!({"raster": "elev", "polygons": "zones", "bands": [-1]})).is_err()); + + // 10. Projected CRS reprojection: polygon in EPSG:3857 (WebMercator) + let merc_tr = rustgis_data::raster::Crs::WebMercator.transformer().unwrap(); + let p0 = merc_tr.from_lonlat(Coord::new(1.0, 7.0)).unwrap(); + let p1 = merc_tr.from_lonlat(Coord::new(3.0, 9.0)).unwrap(); + let merc_geom = Geometry::Polygon(vec![vec![vec![p0, Coord::new(p1.x, p0.y), p1, Coord::new(p0.x, p1.y), p0]]]); + let mut merc_poly_ds = Dataset::new("merc_zones", GeometryType::Polygon, vec![Field::new("id", FieldType::Integer)]); + merc_poly_ds.crs = Some(rustgis_data::raster::Crs::WebMercator); + merc_poly_ds.push(Some(merc_geom.clone()), vec![Value::Int(1)]); + + let m_merc = RasterMap { + datasets: HashMap::from([("merc_zones".to_string(), merc_poly_ds)]), + rasters: HashMap::from([("elev".to_string(), &raster)]), + }; + let res_merc = crate::run( + "zonalStats", + &m_merc, + &serde_json::json!({ + "raster": "elev", + "polygons": "merc_zones", + "bands": [1], + }), + ) + .unwrap(); + let out_merc = layer(res_merc); + assert_eq!(out_merc.crs, Some(rustgis_data::raster::Crs::WebMercator)); + assert_eq!(out_merc.features[0].geometry, Some(merc_geom)); + let merc_count = out_merc.field_index("elev_count").unwrap(); + let merc_mean = out_merc.field_index("elev_mean").unwrap(); + assert_eq!(out_merc.value(0, merc_count), &Value::Int(4)); + assert_eq!(out_merc.value(0, merc_mean), &Value::Float(16.5)); + + // 11. Unsupported polygon CRS returns error + let mut bad_crs_ds = Dataset::new("bad_crs", GeometryType::Polygon, vec![]); + bad_crs_ds.crs = Some(rustgis_data::raster::Crs::Epsg(60000)); + bad_crs_ds.push(Some(sq_geom(1.0, 7.0, 3.0, 9.0)), vec![]); + let m_bad_crs = RasterMap { + datasets: HashMap::from([("bad_crs".to_string(), bad_crs_ds)]), + rasters: HashMap::from([("elev".to_string(), &raster)]), + }; + assert!(crate::run("zonalStats", &m_bad_crs, &serde_json::json!({"raster": "elev", "polygons": "bad_crs"})).is_err()); + assert_eq!(m.datasets["zones"], input_clone); } } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 3571999d6..5de57bf27 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -13469,8 +13469,49 @@ fn gp_zonal_stats_end_to_end_and_undo() { assert!(run_layer.dataset.field_index("dem_count").is_some()); assert!(run_layer.dataset.field_index("dem_min").is_none()); + // Projected CRS (WebMercator) reprojection end-to-end + let merc_tr = Crs::WebMercator.transformer().unwrap(); + let mp0 = merc_tr.from_lonlat(Coord::new(1.0, 7.0)).unwrap(); + let mp1 = merc_tr.from_lonlat(Coord::new(3.0, 9.0)).unwrap(); + let mut merc_ds = Dataset::new("merc_districts", GeometryType::Polygon, vec![Field::new("code", FieldType::Text)]); + merc_ds.crs = Some(Crs::WebMercator); + merc_ds.push(Some(sq(mp0.x, mp0.y, mp1.x, mp1.y)), vec![Value::Text("M1".into())]); + s.map_mut().add_layer(rustgis_map::Layer::new(0, merc_ds)); + + let merc_res = s.run( + "gp.zonalStats", + &json!({ + "raster": "dem", + "polygons": "merc_districts", + "output": "merc_out", + "bands": [1.0], + }), + ); + assert!(merc_res.is_ok(), "gp.zonalStats with WebMercator failed: {merc_res:?}"); + let merc_layer = s.map().layer_by_name("merc_out").expect("merc_out should exist"); + assert_eq!(merc_layer.dataset.crs, Some(Crs::WebMercator)); + let m_count_idx = merc_layer.dataset.field_index("dem_count").unwrap(); + let m_mean_idx = merc_layer.dataset.field_index("dem_mean").unwrap(); + assert_eq!(merc_layer.dataset.value(0, m_count_idx), &Value::Int(4)); + assert_eq!(merc_layer.dataset.value(0, m_mean_idx), &Value::Float(16.5)); + + // "bands": "all" + let all_res = s.run( + "gp.zonalStats", + &json!({ + "raster": "dem", + "polygons": "districts", + "output": "all_bands_out", + "bands": "all", + }), + ); + assert!(all_res.is_ok(), "gp.zonalStats with bands: all failed: {all_res:?}"); + assert!(s.run("gp.zonalStats", &json!({})).is_err()); assert!(s.run("gp.zonalStats", &json!({"raster": "nonexistent", "polygons": "districts"})).is_err()); assert!(s.run("gp.zonalStats", &json!({"raster": "dem", "polygons": "Cities"})).is_err()); assert!(s.run("gp.zonalStats", &json!({"raster": "dem", "polygons": "districts", "bands": [999]})).is_err()); + assert!(s.run("gp.zonalStats", &json!({"raster": "dem", "polygons": "districts", "bands": [0]})).is_err()); + assert!(s.run("gp.zonalStats", &json!({"raster": "dem", "polygons": "districts", "bands": [-1]})).is_err()); + assert!(s.run("gp.zonalStats", &json!({"raster": "dem", "polygons": "districts", "bands": [1.5]})).is_err()); } diff --git a/crates/ui-egui/src/analysispane.rs b/crates/ui-egui/src/analysispane.rs index 352454c75..45ad50f86 100644 --- a/crates/ui-egui/src/analysispane.rs +++ b/crates/ui-egui/src/analysispane.rs @@ -267,17 +267,30 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { ui.label(tl!("Output")); ui.add(egui::TextEdit::singleline(&mut app.workflow.number2).hint_text(tl!("output layer name")).desired_width(120.0)); }); - let can_run = !app.workflow.layer.is_empty() && !app.workflow.layer2.is_empty(); - if !can_run { + let band_str = app.workflow.number.trim(); + let band_valid = if band_str.is_empty() || band_str.eq_ignore_ascii_case("all") { + true + } else { + band_str.split([',', ';', ' ']).all(|part| { + let p = part.trim().trim_matches(|c| c == '[' || c == ']' || c == '"'); + p.is_empty() || p.parse::().is_ok_and(|b| b >= 1) + }) + }; + let layers_ready = !app.workflow.layer.is_empty() && !app.workflow.layer2.is_empty(); + let can_run = layers_ready && band_valid; + if !layers_ready { ui.label(egui::RichText::new(tl!("Add a raster and a polygon layer to calculate zonal statistics.")).small().weak()); + } else if !band_valid { + ui.label(egui::RichText::new(tl!("Band must be a positive integer, list of bands, or 'all'.")).small().color(t.amber)); } if ui.add_enabled(can_run, egui::Button::new(tl!("Calculate Statistics"))).clicked() { - let band = app.workflow.number.trim().parse::().unwrap_or(1); let mut params = json!({ "raster": app.workflow.layer, "polygons": app.workflow.layer2, - "bands": [band], }); + if !band_str.is_empty() { + params["bands"] = json!(band_str); + } if !app.workflow.text.trim().is_empty() { params["prefix"] = json!(app.workflow.text.trim()); } From 7c02b1b875808bd522782e38241600ad181824fd Mon Sep 17 00:00:00 2001 From: quanguet0409 Date: Sat, 24 Oct 2026 09:50:21 +0700 Subject: [PATCH 03/11] fix(analysis,ui): address review issues on zonal stats reprojection and UI buffers --- crates/analysis/src/rasterops.rs | 52 ++++++++-- crates/analysis/src/tools.rs | 152 +++++++++++++++++++++++++---- crates/engine/src/cmd/imagery.rs | 84 +++++++++++++++- crates/engine/src/tests.rs | 43 ++++++++ crates/ui-egui/src/analysispane.rs | 81 ++++++++++++--- 5 files changed, 373 insertions(+), 39 deletions(-) diff --git a/crates/analysis/src/rasterops.rs b/crates/analysis/src/rasterops.rs index 421fcfc1e..31bc58ec9 100644 --- a/crates/analysis/src/rasterops.rs +++ b/crates/analysis/src/rasterops.rs @@ -533,14 +533,34 @@ pub fn polygon_mask(r: &Raster, polygons: &[Vec>]) -> Option<(Range>> = polygons .iter() - .map(|rings| { - rings - .iter() - .map(|ring| ring.iter().filter_map(|c| tr.from_lonlat(*c)).map(|w| r.transform.to_pixel(w)).collect::>()) - .filter(|ring| ring.len() >= 3) - .collect::>() + .filter_map(|rings| { + let exterior = rings.first()?; + let mut ext_pts = Vec::with_capacity(exterior.len()); + for c in exterior { + let w = tr.from_lonlat(*c)?; + ext_pts.push(r.transform.to_pixel(w)); + } + if ext_pts.len() < 3 { + return None; + } + let mut valid_rings = vec![ext_pts]; + for hole in rings.iter().skip(1) { + let mut hole_pts = Vec::with_capacity(hole.len()); + let mut ok = true; + for c in hole { + if let Some(w) = tr.from_lonlat(*c) { + hole_pts.push(r.transform.to_pixel(w)); + } else { + ok = false; + break; + } + } + if ok && hole_pts.len() >= 3 { + valid_rings.push(hole_pts); + } + } + Some(valid_rings) }) - .filter(|rings| !rings.is_empty()) .collect(); let (mut c0, mut r0, mut c1, mut r1) = (f64::INFINITY, f64::INFINITY, f64::NEG_INFINITY, f64::NEG_INFINITY); for &(x, y) in projected.iter().flatten().flatten() { @@ -1119,5 +1139,23 @@ mod tests { // 6. Invalid band index returns Err assert!(zonal_statistics(&r, &[99], &cell_poly).is_err()); + + // 7. Invalid exterior ring (< 3 points) with surviving hole does NOT promote hole to exterior + let bad_ext_with_hole = vec![vec![ + vec![Coord::new(0.005, 0.005), Coord::new(0.015, 0.015)], // only 2 points! + sq(0.009, 0.009, 0.011, 0.011), // 5 points (hole) + ]]; + let bad_ext_stats = zonal_statistics(&r, &[0], &bad_ext_with_hole).unwrap(); + assert_eq!(bad_ext_stats[0], ZoneStats::default()); + assert!(polygon_mask(&r, &bad_ext_with_hole).is_none()); + + // 8. Exterior ring with non-finite/unprojectable coordinate is rejected + let nan_ext_with_hole = vec![vec![ + vec![Coord::new(f64::NAN, 0.005), Coord::new(0.015, 0.005), Coord::new(0.015, 0.015), Coord::new(f64::NAN, 0.005)], + sq(0.009, 0.009, 0.011, 0.011), + ]]; + let nan_ext_stats = zonal_statistics(&r, &[0], &nan_ext_with_hole).unwrap(); + assert_eq!(nan_ext_stats[0], ZoneStats::default()); + assert!(polygon_mask(&r, &nan_ext_with_hole).is_none()); } } diff --git a/crates/analysis/src/tools.rs b/crates/analysis/src/tools.rs index 04a2f87d2..041e198ac 100644 --- a/crates/analysis/src/tools.rs +++ b/crates/analysis/src/tools.rs @@ -1648,6 +1648,56 @@ fn parse_stats(p: &Json) -> Result, String> { Ok(out) } +/// Projects a polygon (exterior ring + optional holes) to WGS 84 lon/lat coordinates. +/// +/// Returns `None` if the exterior ring fails transformation or has fewer than 3 vertices. +/// A hole is never promoted to become the exterior ring. +/// Holes whose vertices fail transformation or have fewer than 3 vertices are skipped, +/// preserving the valid exterior ring and any surviving valid holes. +/// If any coordinate in a ring cannot be transformed, the entire ring is rejected rather +/// than silently dropping vertices and distorting the zone boundary. +fn reproject_polygon(rings: &[Vec], tr: &rustgis_data::raster::Transformer) -> Option>> { + let exterior = rings.first()?; + let mut ext_lonlat = Vec::with_capacity(exterior.len()); + for &c in exterior { + ext_lonlat.push(tr.to_lonlat(c)?); + } + if ext_lonlat.len() < 3 { + return None; + } + let mut out_rings = vec![ext_lonlat]; + for hole in rings.iter().skip(1) { + let mut hole_lonlat = Vec::with_capacity(hole.len()); + let mut ok = true; + for &c in hole { + if let Some(ll) = tr.to_lonlat(c) { + hole_lonlat.push(ll); + } else { + ok = false; + break; + } + } + if ok && hole_lonlat.len() >= 3 { + out_rings.push(hole_lonlat); + } + } + Some(out_rings) +} + +fn validate_lonlat_polygon(rings: &[Vec]) -> Option>> { + let exterior = rings.first()?; + if exterior.len() < 3 { + return None; + } + let mut out_rings = vec![exterior.clone()]; + for hole in rings.iter().skip(1) { + if hole.len() >= 3 { + out_rings.push(hole.clone()); + } + } + Some(out_rings) +} + fn zonal_stats(inp: &dyn Inputs, p: &Json) -> Result { let r_name = { let n = s(p, "raster"); @@ -1720,33 +1770,29 @@ fn zonal_stats(inp: &dyn Inputs, p: &Json) -> Result { } let target_rows = rows(inp, p, poly_param, poly_ds); + let mut skipped_features = 0usize; for r_idx in target_rows { let Some(f) = poly_ds.features.get(r_idx) else { continue }; - let stats_by_band = match &f.geometry { + let (stats_by_band, feature_valid) = match &f.geometry { Some(Geometry::Polygon(polys)) if !polys.is_empty() => { let polys_lonlat: Vec>> = match &poly_tr { - Some(tr) => polys - .iter() - .map(|rings| { - rings - .iter() - .map(|ring| ring.iter().filter_map(|&c| tr.to_lonlat(c)).collect::>()) - .filter(|ring| ring.len() >= 3) - .collect::>() - }) - .filter(|rings| !rings.is_empty()) - .collect(), - None => polys.clone(), + Some(tr) => polys.iter().filter_map(|rings| reproject_polygon(rings, tr)).collect(), + None => polys.iter().filter_map(|rings| validate_lonlat_polygon(rings)).collect(), }; if polys_lonlat.is_empty() { - vec![crate::rasterops::ZoneStats::default(); bands.len()] + (vec![crate::rasterops::ZoneStats::default(); bands.len()], false) } else { - crate::rasterops::zonal_statistics(r, &bands, &polys_lonlat)? + let st = crate::rasterops::zonal_statistics(r, &bands, &polys_lonlat)?; + (st, true) } } - _ => vec![crate::rasterops::ZoneStats::default(); bands.len()], + _ => (vec![crate::rasterops::ZoneStats::default(); bands.len()], false), }; + if !feature_valid { + skipped_features += 1; + } + let mut attrs = f.attrs.clone(); for (b_idx, _, stat, _, _) in &out_fields { let b_stats = stats_by_band.get(*b_idx); @@ -1766,7 +1812,11 @@ fn zonal_stats(inp: &dyn Inputs, p: &Json) -> Result { } let n_features = out.features.len(); - result(out, format!("Calculated zonal statistics for {n_features} feature(s)")) + let mut messages = vec![format!("Calculated zonal statistics for {n_features} feature(s)")]; + if skipped_features > 0 { + messages.push(format!("{skipped_features} feature(s) had invalid geometry or failed coordinate transformation")); + } + Ok(ToolResult { output: Output::Layer(out), messages }) } #[cfg(test)] @@ -2812,6 +2862,74 @@ mod tests { }; assert!(crate::run("zonalStats", &m_bad_crs, &serde_json::json!({"raster": "elev", "polygons": "bad_crs"})).is_err()); + // 12. Exterior ring dropped during reprojection (does not promote hole) + let hole_p0 = merc_tr.from_lonlat(Coord::new(1.0, 8.0)).unwrap(); + let hole_p1 = merc_tr.from_lonlat(Coord::new(2.0, 9.0)).unwrap(); + let bad_ext_geom = Geometry::Polygon(vec![vec![ + vec![p0, p1], // Only 2 points (< 3 points, invalid exterior ring) + vec![hole_p0, Coord::new(hole_p1.x, hole_p0.y), hole_p1, Coord::new(hole_p0.x, hole_p1.y), hole_p0], // valid hole + ]]); + let mut drop_ext_ds = Dataset::new("drop_ext", GeometryType::Polygon, vec![Field::new("id", FieldType::Integer)]); + drop_ext_ds.crs = Some(rustgis_data::raster::Crs::WebMercator); + drop_ext_ds.push(Some(bad_ext_geom), vec![Value::Int(1)]); + let m_drop_ext = RasterMap { + datasets: HashMap::from([("drop_ext".to_string(), drop_ext_ds)]), + rasters: HashMap::from([("elev".to_string(), &raster)]), + }; + let res_drop_ext = crate::run("zonalStats", &m_drop_ext, &serde_json::json!({"raster": "elev", "polygons": "drop_ext"})).unwrap(); + assert!(res_drop_ext.messages.iter().any(|m| m.contains("failed coordinate transformation") || m.contains("invalid geometry"))); + let out_drop_ext = layer(res_drop_ext); + let drop_count_idx = out_drop_ext.field_index("elev_count").unwrap(); + let drop_mean_idx = out_drop_ext.field_index("elev_mean").unwrap(); + assert_eq!(out_drop_ext.value(0, drop_count_idx), &Value::Int(0)); + assert_eq!(out_drop_ext.value(0, drop_mean_idx), &Value::Null); + + // 13. Silent vertex loss prevention during reprojection: + // A polygon where one exterior ring vertex has non-finite coords (cannot be reprojected to lon/lat). + let nan_ext_geom = Geometry::Polygon(vec![vec![vec![ + Coord::new(f64::NAN, p0.y), + Coord::new(p1.x, p0.y), + p1, + Coord::new(p0.x, p1.y), + Coord::new(f64::NAN, p0.y), + ]]]); + let mut nan_ext_ds = Dataset::new("nan_ext", GeometryType::Polygon, vec![Field::new("id", FieldType::Integer)]); + nan_ext_ds.crs = Some(rustgis_data::raster::Crs::WebMercator); + nan_ext_ds.push(Some(nan_ext_geom), vec![Value::Int(1)]); + let m_nan_ext = RasterMap { + datasets: HashMap::from([("nan_ext".to_string(), nan_ext_ds)]), + rasters: HashMap::from([("elev".to_string(), &raster)]), + }; + let res_nan_ext = crate::run("zonalStats", &m_nan_ext, &serde_json::json!({"raster": "elev", "polygons": "nan_ext"})).unwrap(); + assert!(res_nan_ext.messages.iter().any(|m| m.contains("failed coordinate transformation") || m.contains("invalid geometry"))); + let out_nan_ext = layer(res_nan_ext); + let nan_count_idx = out_nan_ext.field_index("elev_count").unwrap(); + let nan_mean_idx = out_nan_ext.field_index("elev_mean").unwrap(); + assert_eq!(out_nan_ext.value(0, nan_count_idx), &Value::Int(0)); + assert_eq!(out_nan_ext.value(0, nan_mean_idx), &Value::Null); + + // 14. Valid projected polygon with holes reprojects both and subtracts holes correctly + let merc_outer0 = merc_tr.from_lonlat(Coord::new(0.0, 7.0)).unwrap(); + let merc_outer1 = merc_tr.from_lonlat(Coord::new(3.0, 10.0)).unwrap(); + let merc_hole0 = merc_tr.from_lonlat(Coord::new(1.0, 8.0)).unwrap(); + let merc_hole1 = merc_tr.from_lonlat(Coord::new(2.0, 9.0)).unwrap(); + let valid_hole_geom = Geometry::Polygon(vec![vec![ + vec![merc_outer0, Coord::new(merc_outer1.x, merc_outer0.y), merc_outer1, Coord::new(merc_outer0.x, merc_outer1.y), merc_outer0], + vec![merc_hole0, Coord::new(merc_hole1.x, merc_hole0.y), merc_hole1, Coord::new(merc_hole0.x, merc_hole1.y), merc_hole0], + ]]); + let mut hole_ds = Dataset::new("merc_hole", GeometryType::Polygon, vec![Field::new("id", FieldType::Integer)]); + hole_ds.crs = Some(rustgis_data::raster::Crs::WebMercator); + hole_ds.push(Some(valid_hole_geom), vec![Value::Int(1)]); + let m_hole = RasterMap { + datasets: HashMap::from([("merc_hole".to_string(), hole_ds)]), + rasters: HashMap::from([("elev".to_string(), &raster)]), + }; + let res_hole = crate::run("zonalStats", &m_hole, &serde_json::json!({"raster": "elev", "polygons": "merc_hole"})).unwrap(); + let out_hole = layer(res_hole); + let hole_cnt_idx = out_hole.field_index("elev_count").unwrap(); + // 9 outer cells - 1 hole cell = 8 cells + assert_eq!(out_hole.value(0, hole_cnt_idx), &Value::Int(8)); + assert_eq!(m.datasets["zones"], input_clone); } } diff --git a/crates/engine/src/cmd/imagery.rs b/crates/engine/src/cmd/imagery.rs index fecf2d1e2..73de8edc8 100644 --- a/crates/engine/src/cmd/imagery.rs +++ b/crates/engine/src/cmd/imagery.rs @@ -70,7 +70,7 @@ pub fn commands() -> Vec { r#"{"layer": raster, "index": "ndvi"|"ndwi"|"savi"|"nbr"|"ndbi"|"evi", "red"?, "green"?, "blue"?, "nir"?, "swir"?: band numbers, "name"?: output}"#, ), C::new("raster.process", "Process", "Analyze › Imagery Tools", process) - .params(r#"{"layer": raster, "operation": "clip"|"resample", "extent"?: [minLon, minLat, maxLon, maxLat] (clip; the view when omitted), "polygon"?: polygon layer (clip to its selected or all polygons; cells outside become no-data), "factor"?: n (resample: n×n pixels become one), "name"?: output}"#), + .params(r#"{"layer": raster, "operation": "clip"|"resample", "extent"?: [minLon, minLat, maxLon, maxLat] (clip; the view when omitted), "polygon"?: polygon layer (clip to its selected or all polygons; cells outside become no-data), "selectedOnly"?: bool (clip to selected polygons when true, all when false), "factor"?: n (resample: n×n pixels become one), "name"?: output}"#), C::new("raster.mosaic", "Mosaic Rasters", "Analyze › Imagery Tools", raster_mosaic).params( r#"{"inputs": [raster layer or file path, …] (two or more, in one coordinate system and cell size; where they overlap the first wins), "name"?: output, "path"?: GeoTIFF to write the mosaic to}"#, ), @@ -531,16 +531,92 @@ fn process(s: &mut Session, p: &Value) -> CmdResult { .layer_by_name(pl) .filter(|l| l.geometry_type() == GeometryType::Polygon) .ok_or_else(|| format!("no polygon layer '{pl}'"))?; - // The selected polygons, or all of them. - let rows: Vec = if l.selection.is_empty() { (0..l.dataset.features.len()).collect() } else { l.selection.clone() }; + let rows: Vec = match b(p, "selectedOnly") { + Some(true) => { + if l.selection.is_empty() { + return Err(format!("{pl}: no features are selected")); + } + l.selection.clone() + } + Some(false) => (0..l.dataset.features.len()).collect(), + None => { + if l.selection.is_empty() { + (0..l.dataset.features.len()).collect() + } else { + l.selection.clone() + } + } + }; + if rows.is_empty() { + return Err(format!("{pl}: has no features to clip by")); + } + let poly_tr = match l.dataset.crs { + Some(crs) if crs != Crs::LonLat && crs != Crs::Epsg(4326) => { + Some(crs.transformer().map_err(|e| format!("{pl} coordinate system isn't supported: {e}"))?) + } + _ => None, + }; let polygons: Vec>> = rows .iter() .filter_map(|row| match l.dataset.features.get(*row).and_then(|f| f.geometry.as_ref()) { - Some(Geometry::Polygon(polys)) => Some(polys.iter().cloned()), + Some(Geometry::Polygon(polys)) => { + let rings: Vec>> = match &poly_tr { + Some(tr) => polys + .iter() + .filter_map(|rings| { + let exterior = rings.first()?; + let mut ext_pts = Vec::with_capacity(exterior.len()); + for &c in exterior { + ext_pts.push(tr.to_lonlat(c)?); + } + if ext_pts.len() < 3 { + return None; + } + let mut valid_rings = vec![ext_pts]; + for hole in rings.iter().skip(1) { + let mut hole_pts = Vec::with_capacity(hole.len()); + let mut ok = true; + for &c in hole { + if let Some(ll) = tr.to_lonlat(c) { + hole_pts.push(ll); + } else { + ok = false; + break; + } + } + if ok && hole_pts.len() >= 3 { + valid_rings.push(hole_pts); + } + } + Some(valid_rings) + }) + .collect(), + None => polys + .iter() + .filter_map(|rings| { + let exterior = rings.first()?; + if exterior.len() < 3 { + return None; + } + let mut valid_rings = vec![exterior.clone()]; + for hole in rings.iter().skip(1) { + if hole.len() >= 3 { + valid_rings.push(hole.clone()); + } + } + Some(valid_rings) + }) + .collect(), + }; + Some(rings.into_iter()) + } _ => None, }) .flatten() .collect(); + if polygons.is_empty() { + return Err(format!("{pl}: has no valid polygon geometries")); + } let (cols, rows, mask) = rasterops::polygon_mask(&r, &polygons).ok_or_else(|| format!("no cell of {} falls inside {pl}", r.name))?; let out = clip_window(&r, cols, rows, Some(&mask), name(p, format!("{}_clip", r.name)))?; diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 5de57bf27..833da43ce 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -12414,6 +12414,49 @@ fn dem_tiles_mosaic_clip_by_polygon_share_a_stretch_and_report_overviews() { assert!(e.to_string().contains("no cell"), "{e}"); assert!(s.run("raster.process", &json!({"layer": "dem", "operation": "clip", "polygon": "west"})).is_err()); + // selectedOnly: true with no selected features fails with a descriptive error. + s.map_mut().layers.iter_mut().find(|l| l.name == "City").unwrap().selection.clear(); + let e_sel_none = + s.run("raster.process", &json!({"layer": "dem", "operation": "clip", "polygon": "City", "selectedOnly": true})).unwrap_err(); + assert!(e_sel_none.to_string().contains("no features are selected"), "{e_sel_none}"); + + // Add a second feature to City further east in dem's extent. + let ring2 = [ll(361_500.0, 5_700_000.0), ll(362_000.0, 5_700_000.0), ll(362_000.0, 5_699_000.0), ll(361_500.0, 5_700_000.0)]; + s.run("edit.addFeature", &json!({"layer": "City", "geometry": {"type": "Polygon", "coordinates": [ring2]}})).unwrap(); + assert_eq!(s.map().layer_by_name("City").unwrap().dataset.features.len(), 2); + + // With feature 0 selected: selectedOnly: true clips only feature 0. + s.map_mut().layers.iter_mut().find(|l| l.name == "City").unwrap().selection = vec![0]; + let v_sel = s.run("raster.process", &json!({"layer": "dem", "operation": "clip", "polygon": "City", "selectedOnly": true})).unwrap(); + let c_sel = raster(&s, &v_sel); + assert_eq!(c_sel.bands[0].stats.count, 25); + + // selectedOnly: false clips all features even with selection active. + let v_all = s.run("raster.process", &json!({"layer": "dem", "operation": "clip", "polygon": "City", "selectedOnly": false})).unwrap(); + let c_all = raster(&s, &v_all); + assert!(c_all.bands[0].stats.count > 25); + + // An empty polygon layer fails validation. + s.run("layer.template", &json!({"template": "polygons", "name": "EmptyPoly"})).unwrap(); + let e_empty = s.run("raster.process", &json!({"layer": "dem", "operation": "clip", "polygon": "EmptyPoly"})).unwrap_err(); + assert!(e_empty.to_string().contains("has no features"), "{e_empty}"); + + // A polygon layer in a projected CRS (UTM) reprojects and clips accurately. + let mut proj_ds = rustgis_data::Dataset::new("CityProj", rustgis_data::GeometryType::Polygon, vec![]); + proj_ds.crs = Some(utm); + let utm_ring = [ + rustgis_geom::Coord::new(360_500.0, 5_700_000.0), + rustgis_geom::Coord::new(361_500.0, 5_700_000.0), + rustgis_geom::Coord::new(361_500.0, 5_699_500.0), + rustgis_geom::Coord::new(360_500.0, 5_700_000.0), + ]; + proj_ds.push(Some(rustgis_data::Geometry::Polygon(vec![vec![utm_ring.to_vec()]])), vec![]); + s.map_mut().add_layer(rustgis_map::Layer::new(0, proj_ds)); + let v_proj = s.run("raster.process", &json!({"layer": "dem", "operation": "clip", "polygon": "CityProj"})).unwrap(); + let c_proj = raster(&s, &v_proj); + assert_eq!((c_proj.width, c_proj.height, c_proj.crs), (9, 5, utm)); + assert_eq!(c_proj.bands[0].stats.count, 25); + // One stretch shared across tiles, as fixed bounds. s.run("raster.stretch", &json!({"layer": "west", "type": "custom", "min": 100, "max": 300})).unwrap(); s.run("raster.stretch", &json!({"layer": "east", "like": "west"})).unwrap(); diff --git a/crates/ui-egui/src/analysispane.rs b/crates/ui-egui/src/analysispane.rs index 45ad50f86..57ecb11ad 100644 --- a/crates/ui-egui/src/analysispane.rs +++ b/crates/ui-egui/src/analysispane.rs @@ -34,6 +34,16 @@ pub struct Workflow { pub script: String, pub script_out: Vec<(String, bool)>, pub trend: TrendForm, + pub zonal_stats: ZonalStatsForm, + pub selected_only: bool, +} + +/// The Zonal Statistics section's inputs. +#[derive(Default)] +pub struct ZonalStatsForm { + pub bands: String, + pub prefix: String, + pub output: String, } /// The Trend Analysis section's inputs (#579), and a run under way. @@ -200,7 +210,7 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { if !inputs.contains(&w.layer) { w.layer = inputs.first().cloned().unwrap_or_default(); } - if w.section == "zonalStats" && !polygons.contains(&w.layer2) { + if (w.section == "zonalStats" || w.section == "process") && !polygons.contains(&w.layer2) { w.layer2 = polygons.first().cloned().unwrap_or_default(); } ui.add_space(6.0); @@ -259,15 +269,19 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { combo(ui, "wf_zs_poly", &mut app.workflow.layer2, &polygons); ui.horizontal(|ui| { ui.label(tl!("Band")); - ui.add(egui::TextEdit::singleline(&mut app.workflow.number).hint_text("1").desired_width(40.0)); + ui.add(egui::TextEdit::singleline(&mut app.workflow.zonal_stats.bands).hint_text("1").desired_width(40.0)); ui.label(tl!("Prefix")); - ui.add(egui::TextEdit::singleline(&mut app.workflow.text).hint_text("elev").desired_width(70.0)); + ui.add(egui::TextEdit::singleline(&mut app.workflow.zonal_stats.prefix).hint_text("elev").desired_width(70.0)); }); ui.horizontal(|ui| { ui.label(tl!("Output")); - ui.add(egui::TextEdit::singleline(&mut app.workflow.number2).hint_text(tl!("output layer name")).desired_width(120.0)); + ui.add( + egui::TextEdit::singleline(&mut app.workflow.zonal_stats.output) + .hint_text(tl!("output layer name")) + .desired_width(120.0), + ); }); - let band_str = app.workflow.number.trim(); + let band_str = app.workflow.zonal_stats.bands.trim(); let band_valid = if band_str.is_empty() || band_str.eq_ignore_ascii_case("all") { true } else { @@ -291,11 +305,11 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { if !band_str.is_empty() { params["bands"] = json!(band_str); } - if !app.workflow.text.trim().is_empty() { - params["prefix"] = json!(app.workflow.text.trim()); + if !app.workflow.zonal_stats.prefix.trim().is_empty() { + params["prefix"] = json!(app.workflow.zonal_stats.prefix.trim()); } - if !app.workflow.number2.trim().is_empty() { - params["output"] = json!(app.workflow.number2.trim()); + if !app.workflow.zonal_stats.output.trim().is_empty() { + params["output"] = json!(app.workflow.zonal_stats.output.trim()); } run(app, "gp.zonalStats", params); } @@ -645,13 +659,58 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { } "process" => { heading(ui, "Clip or resample"); + ui.label(egui::RichText::new(tl!("Raster layer")).small().weak()); combo(ui, "wf_pr", &mut app.workflow.layer, &rasters); + + ui.add_space(4.0); + ui.label(egui::RichText::new(tl!("Clip to polygon layer")).small().weak()); + let has_polys = !polygons.is_empty(); + if has_polys { + combo(ui, "wf_pr_poly", &mut app.workflow.layer2, &polygons); + let poly_layer = app.session.map().layer_by_name(&app.workflow.layer2); + let sel_count = poly_layer.as_ref().map_or(0, |l| l.selection.len()); + let total_features = poly_layer.as_ref().map_or(0, |l| l.dataset.features.len()); + + ui.horizontal(|ui| { + ui.checkbox(&mut app.workflow.selected_only, tl!("Selected only")); + if sel_count > 0 { + ui.label(egui::RichText::new(format!("({sel_count} of {total_features})")).small().weak()); + } + }); + + let sel_invalid = app.workflow.selected_only && sel_count == 0; + let can_clip_poly = !app.workflow.layer.is_empty() && !app.workflow.layer2.is_empty() && !sel_invalid; + + if sel_invalid { + ui.label(egui::RichText::new(tl!("No features are selected in this layer.")).small().color(t.amber)); + } + + if ui.add_enabled(can_clip_poly, egui::Button::new(tl!("Clip To Polygon"))).clicked() { + let mut p = json!({ + "layer": app.workflow.layer, + "operation": "clip", + "polygon": app.workflow.layer2, + }); + if app.workflow.selected_only { + p["selectedOnly"] = json!(true); + } + run(app, "raster.process", p); + } + } else { + ui.label(egui::RichText::new(tl!("No polygon layer available in the map.")).small().weak()); + } + + ui.add_space(4.0); ui.horizontal(|ui| { - if ui.button(tl!("Clip To Map View")).clicked() { + if ui.add_enabled(!app.workflow.layer.is_empty(), egui::Button::new(tl!("Clip To Map View"))).clicked() { run(app, "raster.process", json!({"layer": app.workflow.layer, "operation": "clip"})); } + }); + + ui.add_space(4.0); + ui.horizontal(|ui| { ui.add(egui::TextEdit::singleline(&mut app.workflow.number).hint_text(tl!("factor")).desired_width(50.0)); - if ui.button(tl!("Resample")).clicked() { + if ui.add_enabled(!app.workflow.layer.is_empty(), egui::Button::new(tl!("Resample"))).clicked() { run( app, "raster.process", From 95134f0ae78e08e164b5b9fed47b862887b9c632 Mon Sep 17 00:00:00 2001 From: giswqs Date: Sat, 24 Oct 2026 08:21:56 -0400 Subject: [PATCH 04/11] Address review feedback - Share polygon transformation across zonal statistics, masking and clipping; reject failed holes and report partial features. - Pad source attributes before statistics and resolve field names case-insensitively, with regressions. - Share band parsing with the UI, validate before conversion and reject invalid statistics types. - Send the clipping selection choice explicitly and document selection and per-zone processing semantics. --- crates/analysis/src/lib.rs | 2 +- crates/analysis/src/rasterops.rs | 66 +++++--- crates/analysis/src/tools.rs | 254 ++++++++++++----------------- crates/engine/src/cmd/imagery.rs | 55 +------ crates/ui-egui/src/analysispane.rs | 20 +-- docs/user-guide/analysis.md | 12 ++ 6 files changed, 174 insertions(+), 235 deletions(-) diff --git a/crates/analysis/src/lib.rs b/crates/analysis/src/lib.rs index 37ef54706..8b915087c 100644 --- a/crates/analysis/src/lib.rs +++ b/crates/analysis/src/lib.rs @@ -24,7 +24,7 @@ pub mod topology; pub mod trend; pub mod warp; -pub use tools::{TOOLS, holds_coordinates, simplify_geometry}; +pub use tools::{TOOLS, holds_coordinates, simplify_geometry, zonal_bands}; use rustgis_data::Dataset; use serde_json::Value; diff --git a/crates/analysis/src/rasterops.rs b/crates/analysis/src/rasterops.rs index 31bc58ec9..4890f6bd7 100644 --- a/crates/analysis/src/rasterops.rs +++ b/crates/analysis/src/rasterops.rs @@ -523,6 +523,30 @@ pub fn zonal_means(r: &Raster, bands: &[usize], rings: &[Vec]) -> Option< (n > 0).then(|| (sums.into_iter().map(|s| s / n as f64).collect(), n)) } +/// Transforms one polygon part, keeping its exterior and holes together. A failed coordinate +/// rejects the whole part, so a missing hole cannot enlarge it. Short holes have no area. +/// `transform` maps input coordinates into the caller's target coordinate system. +pub fn transform_polygon(rings: &[Vec], mut transform: impl FnMut(Coord) -> Option) -> Option>> { + if rings.first()?.len() < 3 { + return None; + } + let mut out = Vec::new(); + for ring in rings { + if ring.len() < 3 { + continue; + } + let transformed: Option> = ring + .iter() + .map(|&c| { + let p = transform(c)?; + (p.x.is_finite() && p.y.is_finite()).then_some(p) + }) + .collect(); + out.push(transformed?); + } + Some(out) +} + /// The cells of `r` whose centres fall inside any of `polygons` (each an exterior ring and its /// holes, in longitude/latitude): the column and row window around the polygons, clamped to the /// raster, and a row-major mask over it. `None` when no cell centre falls inside. @@ -534,32 +558,12 @@ pub fn polygon_mask(r: &Raster, polygons: &[Vec>]) -> Option<(Range>> = polygons .iter() .filter_map(|rings| { - let exterior = rings.first()?; - let mut ext_pts = Vec::with_capacity(exterior.len()); - for c in exterior { - let w = tr.from_lonlat(*c)?; - ext_pts.push(r.transform.to_pixel(w)); - } - if ext_pts.len() < 3 { - return None; - } - let mut valid_rings = vec![ext_pts]; - for hole in rings.iter().skip(1) { - let mut hole_pts = Vec::with_capacity(hole.len()); - let mut ok = true; - for c in hole { - if let Some(w) = tr.from_lonlat(*c) { - hole_pts.push(r.transform.to_pixel(w)); - } else { - ok = false; - break; - } - } - if ok && hole_pts.len() >= 3 { - valid_rings.push(hole_pts); - } - } - Some(valid_rings) + transform_polygon(rings, |c| { + let w = tr.from_lonlat(c)?; + let (x, y) = r.transform.to_pixel(w); + Some(Coord::new(x, y)) + }) + .map(|rings| rings.into_iter().map(|ring| ring.into_iter().map(|c| (c.x, c.y)).collect()).collect()) }) .collect(); let (mut c0, mut r0, mut c1, mut r1) = (f64::INFINITY, f64::INFINITY, f64::NEG_INFINITY, f64::NEG_INFINITY); @@ -867,6 +871,16 @@ mod tests { Raster::new("ramp", w, h, vec![a, b], gt, Crs::LonLat, None, ColorHint::Gray, "Float32").unwrap_or_else(|e| panic!("{e}")) } + #[test] + fn a_failed_hole_transform_rejects_the_polygon_part() { + let square = vec![Coord::new(0.0, 0.0), Coord::new(1.0, 0.0), Coord::new(1.0, 1.0), Coord::new(0.0, 0.0)]; + let mut hole = square.clone(); + hole[0].x = f64::NAN; + assert!(transform_polygon(&[square.clone(), hole], Some).is_none()); + assert!(transform_polygon(std::slice::from_ref(&square), |_| None).is_none()); + assert_eq!(transform_polygon(&[square.clone(), vec![Coord::new(0.1, 0.1)]], Some), Some(vec![square])); + } + #[test] fn polygon_mask_fills_polygons_and_cuts_holes() { let r = ramp(); diff --git a/crates/analysis/src/tools.rs b/crates/analysis/src/tools.rs index bfbbd1b1e..7513c6a0b 100644 --- a/crates/analysis/src/tools.rs +++ b/crates/analysis/src/tools.rs @@ -74,7 +74,7 @@ pub static TOOLS: &[ToolSpec] = &[ summary: "Encloses each feature, or all features, in its convex hull.", params: &[ P { name: "input", label: "Input Features", kind: K::Layer, required: true, default: "", help: "" }, - P { name: "group", label: "Group Option", kind: K::Choice(&["All", "None"]), required: false, default: "\"All\"", help: "" }, + P { name: "group", label: "Group Option", kind: K::Choice(&["All", "None"]), required: false, default: "All", help: "" }, P { name: "output", label: "Output Name", kind: K::Text, required: false, default: "", help: "" }, ], run: convex_hull, @@ -477,14 +477,14 @@ pub static TOOLS: &[ToolSpec] = &[ kind: K::PolygonLayer, required: true, default: "", - help: "Polygon layer defining the zones.", + help: "Polygon layer defining the zones; uses selected features when a selection exists.", }, P { name: "bands", label: "Bands", kind: K::Text, required: false, - default: "\"1\"", + default: "1", help: "1-based band number(s) (e.g. 1 or [1, 2]); defaults to band 1.", }, P { @@ -492,7 +492,7 @@ pub static TOOLS: &[ToolSpec] = &[ label: "Statistics", kind: K::Text, required: false, - default: "\"All\"", + default: "All", help: "Statistical operations: All, Mean, Min, Max, Sum, Count, StdDev (comma-separated or array); defaults to All.", }, P { @@ -1517,86 +1517,60 @@ impl StatOp { } } +fn validate_band(f: f64, total: usize) -> Result { + if !f.is_finite() { + return Err("band number must be a finite number".into()); + } + if f <= 0.0 { + return Err("band numbers must be positive (1-based)".into()); + } + if f.fract() != 0.0 { + return Err("band numbers must be integers, not decimals".into()); + } + if f > total as f64 || f >= usize::MAX as f64 { + return Err(format!("band index {f} is invalid: raster has bands 1 to {total}")); + } + Ok(f as usize) +} + fn parse_single_band(item: &Json, total_bands: usize, out: &mut Vec) -> Result<(), String> { match item { Json::Number(n) => { - let f = n.as_f64().ok_or_else(|| "invalid band number".to_string())?; - if !f.is_finite() { - return Err("band number must be a finite number".into()); - } - if f <= 0.0 { - return Err("band numbers must be positive (1-based)".into()); - } - if f.fract() != 0.0 { - return Err("band numbers must be integers, not decimals".into()); - } - let b = f as usize; - if b > total_bands { - return Err(format!("band index {b} is invalid: raster has bands 1 to {total_bands}")); - } - out.push(b); - Ok(()) + out.push(validate_band(n.as_f64().ok_or("invalid band number")?, total_bands)?); } Json::String(s) => { - let txt = s.trim(); - if txt.eq_ignore_ascii_case("all") { - out.extend(1..=total_bands); - Ok(()) - } else { - let clean = txt.trim_matches(|c| c == '[' || c == ']'); - for part in clean.split([',', ';', ' ']) { - let part = part.trim().trim_matches('"'); - if part.is_empty() { - continue; - } - if part.eq_ignore_ascii_case("all") { - out.extend(1..=total_bands); - continue; - } - let f: f64 = part.parse().map_err(|_| format!("invalid band number '{part}'"))?; - if !f.is_finite() { - return Err("band number must be a finite number".into()); - } - if f <= 0.0 { - return Err("band numbers must be positive (1-based)".into()); - } - if f.fract() != 0.0 { - return Err("band numbers must be integers, not decimals".into()); - } - let b = f as usize; - if b > total_bands { - return Err(format!("band index {b} is invalid: raster has bands 1 to {total_bands}")); - } - out.push(b); + for part in s.trim().trim_matches(|c| c == '[' || c == ']').split([',', ';', ' ']) { + let part = part.trim().trim_matches('"'); + if part.is_empty() { + continue; + } + if part.eq_ignore_ascii_case("all") { + out.extend(1..=total_bands); + } else { + let f = part.parse().map_err(|_| format!("invalid band number '{part}'"))?; + out.push(validate_band(f, total_bands)?); } - Ok(()) } } - _ => Err("band numbers must be positive integers or 'all'".into()), + _ => return Err("band numbers must be positive integers or 'all'".into()), } + Ok(()) } -fn parse_bands(p: &Json, total_bands: usize) -> Result, String> { +/// Parses the Zonal Statistics band's text or JSON list into unique zero-based indices. +pub fn zonal_bands(value: &Json, total_bands: usize) -> Result, String> { let mut raw_bands = Vec::new(); - if let Some(arr) = p.get("bands").and_then(Json::as_array) { - if arr.is_empty() { - raw_bands.push(1); - } else { + match value { + Json::Array(arr) => { for item in arr { parse_single_band(item, total_bands, &mut raw_bands)?; } } - } else if let Some(v) = p.get("bands") { - if v.is_null() { - raw_bands.push(1); - } else { - parse_single_band(v, total_bands, &mut raw_bands)?; - } - } else { - raw_bands.push(1); + v if !v.is_null() => parse_single_band(v, total_bands, &mut raw_bands)?, + _ => {} } if raw_bands.is_empty() { - raw_bands.push(1); + raw_bands.push(validate_band(1.0, total_bands)?); } let mut unique = Vec::new(); for b in raw_bands { @@ -1607,37 +1581,30 @@ fn parse_bands(p: &Json, total_bands: usize) -> Result, String> { Ok(unique.into_iter().map(|b| b - 1).collect()) } +fn parse_bands(p: &Json, total_bands: usize) -> Result, String> { + zonal_bands(p.get("bands").unwrap_or(&Json::Null), total_bands) +} + fn parse_stats(p: &Json) -> Result, String> { - let mut out = Vec::new(); - if let Some(arr) = p.get("stats").and_then(Json::as_array) { - if arr.is_empty() { - return Ok(StatOp::ALL.to_vec()); - } - for item in arr { - let txt = item.as_str().ok_or_else(|| "statistic names must be strings".to_string())?; - let ops = StatOp::parse(txt)?; - for op in ops { - if !out.contains(&op) { - out.push(op); - } - } - } + if p.get("stats").is_some_and(|v| !v.is_null() && !v.is_array() && !v.is_string()) { + return Err("statistic names must be strings or an array of strings".into()); + } + let names: Vec<&str> = if let Some(arr) = p.get("stats").and_then(Json::as_array) { + arr.iter().map(|item| item.as_str().ok_or_else(|| "statistic names must be strings".to_string())).collect::>()? } else { - let txt = s(p, "stats").trim(); - if txt.is_empty() { - return Ok(StatOp::ALL.to_vec()); - } - let clean = txt.trim_matches(|c| c == '[' || c == ']'); - for part in clean.split([',', ';']) { - let part = part.trim().trim_matches('"'); - if part.is_empty() { - continue; - } - let ops = StatOp::parse(part)?; - for op in ops { - if !out.contains(&op) { - out.push(op); - } + s(p, "stats") + .trim() + .trim_matches(|c| c == '[' || c == ']') + .split([',', ';']) + .map(|part| part.trim().trim_matches('"')) + .filter(|part| !part.is_empty()) + .collect() + }; + let mut out = Vec::new(); + for name in names { + for op in StatOp::parse(name)? { + if !out.contains(&op) { + out.push(op); } } } @@ -1647,56 +1614,6 @@ fn parse_stats(p: &Json) -> Result, String> { Ok(out) } -/// Projects a polygon (exterior ring + optional holes) to WGS 84 lon/lat coordinates. -/// -/// Returns `None` if the exterior ring fails transformation or has fewer than 3 vertices. -/// A hole is never promoted to become the exterior ring. -/// Holes whose vertices fail transformation or have fewer than 3 vertices are skipped, -/// preserving the valid exterior ring and any surviving valid holes. -/// If any coordinate in a ring cannot be transformed, the entire ring is rejected rather -/// than silently dropping vertices and distorting the zone boundary. -fn reproject_polygon(rings: &[Vec], tr: &rustgis_data::raster::Transformer) -> Option>> { - let exterior = rings.first()?; - let mut ext_lonlat = Vec::with_capacity(exterior.len()); - for &c in exterior { - ext_lonlat.push(tr.to_lonlat(c)?); - } - if ext_lonlat.len() < 3 { - return None; - } - let mut out_rings = vec![ext_lonlat]; - for hole in rings.iter().skip(1) { - let mut hole_lonlat = Vec::with_capacity(hole.len()); - let mut ok = true; - for &c in hole { - if let Some(ll) = tr.to_lonlat(c) { - hole_lonlat.push(ll); - } else { - ok = false; - break; - } - } - if ok && hole_lonlat.len() >= 3 { - out_rings.push(hole_lonlat); - } - } - Some(out_rings) -} - -fn validate_lonlat_polygon(rings: &[Vec]) -> Option>> { - let exterior = rings.first()?; - if exterior.len() < 3 { - return None; - } - let mut out_rings = vec![exterior.clone()]; - for hole in rings.iter().skip(1) { - if hole.len() >= 3 { - out_rings.push(hole.clone()); - } - } - Some(out_rings) -} - fn zonal_stats(inp: &dyn Inputs, p: &Json) -> Result { let r_name = { let n = s(p, "raster"); @@ -1755,7 +1672,9 @@ fn zonal_stats(inp: &dyn Inputs, p: &Json) -> Result { let base_name = format!("{prefix}_{}", stat.name()); let mut final_name = base_name.clone(); let mut counter = 1usize; - while out.field_index(&final_name).is_some() || new_field_names.iter().any(|n| n.eq_ignore_ascii_case(&final_name)) { + while out.fields.iter().any(|f| f.name.eq_ignore_ascii_case(&final_name)) + || new_field_names.iter().any(|n| n.eq_ignore_ascii_case(&final_name)) + { counter += 1; final_name = format!("{base_name}_{counter}"); } @@ -1775,14 +1694,14 @@ fn zonal_stats(inp: &dyn Inputs, p: &Json) -> Result { let (stats_by_band, feature_valid) = match &f.geometry { Some(Geometry::Polygon(polys)) if !polys.is_empty() => { let polys_lonlat: Vec>> = match &poly_tr { - Some(tr) => polys.iter().filter_map(|rings| reproject_polygon(rings, tr)).collect(), - None => polys.iter().filter_map(|rings| validate_lonlat_polygon(rings)).collect(), + Some(tr) => polys.iter().filter_map(|rings| crate::rasterops::transform_polygon(rings, |c| tr.to_lonlat(c))).collect(), + None => polys.iter().filter_map(|rings| crate::rasterops::transform_polygon(rings, Some)).collect(), }; if polys_lonlat.is_empty() { (vec![crate::rasterops::ZoneStats::default(); bands.len()], false) } else { let st = crate::rasterops::zonal_statistics(r, &bands, &polys_lonlat)?; - (st, true) + (st, polys_lonlat.len() == polys.len()) } } _ => (vec![crate::rasterops::ZoneStats::default(); bands.len()], false), @@ -1793,6 +1712,7 @@ fn zonal_stats(inp: &dyn Inputs, p: &Json) -> Result { } let mut attrs = f.attrs.clone(); + attrs.resize(poly_ds.fields.len(), Value::Null); for (b_idx, _, stat, _, _) in &out_fields { let b_stats = stats_by_band.get(*b_idx); let val = match (b_stats, stat) { @@ -1813,7 +1733,9 @@ fn zonal_stats(inp: &dyn Inputs, p: &Json) -> Result { let n_features = out.features.len(); let mut messages = vec![format!("Calculated zonal statistics for {n_features} feature(s)")]; if skipped_features > 0 { - messages.push(format!("{skipped_features} feature(s) had invalid geometry or failed coordinate transformation")); + messages.push(format!( + "{skipped_features} feature(s) had missing geometry, invalid polygon parts or failed coordinate transformations" + )); } Ok(ToolResult { output: Output::Layer(out), messages }) } @@ -2693,6 +2615,34 @@ mod tests { rasters: HashMap::from([("elev".to_string(), &raster)]), }; + // Missing source attributes stay in their own columns, including case-insensitive collisions. + let mut short = input_clone.clone(); + short.fields[1].name = "ELEV_MEAN".into(); + short.features[0].attrs.truncate(1); + let sparse = + RasterMap { datasets: HashMap::from([("zones".to_string(), short)]), rasters: HashMap::from([("elev".to_string(), &raster)]) }; + let sparse_out = + layer(crate::run("zonalStats", &sparse, &serde_json::json!({"raster":"elev", "polygons":"zones", "stats":"mean"})).unwrap()); + assert_eq!(sparse_out.features[0].attrs[1], Value::Null); + assert_eq!(sparse_out.fields[2].name, "elev_mean_2"); + assert!(matches!(sparse_out.features[0].attrs[2], Value::Float(_))); + + let mut partial = input_clone.clone(); + partial.features.truncate(1); + if let Some(Geometry::Polygon(parts)) = &mut partial.features[0].geometry { + parts.push(vec![vec![Coord::new(1.0, 1.0)]]); + } + let multipart = RasterMap { + datasets: HashMap::from([("zones".to_string(), partial)]), + rasters: HashMap::from([("elev".to_string(), &raster)]), + }; + let partial_result = + crate::run("zonalStats", &multipart, &serde_json::json!({"raster":"elev", "polygons":"zones", "stats":"count"})).unwrap(); + assert!(partial_result.messages.iter().any(|m| m.starts_with("1 feature(s) had missing geometry"))); + let partial_out = layer(partial_result); + let count = partial_out.field_index("elev_count").unwrap(); + assert_eq!(partial_out.value(0, count), &Value::Int(4)); + let res = crate::run( "zonalStats", &m, @@ -2833,6 +2783,10 @@ mod tests { let out_float = layer(res_float_band); assert!(out_float.field_index("elev_mean").is_some()); + assert!( + crate::run("zonalStats", &m, &serde_json::json!({"raster": "elev", "polygons": "zones", "bands": [4294967297u64]})).is_err() + ); + assert!(crate::run("zonalStats", &m, &serde_json::json!({"raster": "elev", "polygons": "zones", "stats": 17})).is_err()); assert!(crate::run("zonalStats", &m, &serde_json::json!({"raster": "elev", "polygons": "zones", "bands": [1.5]})).is_err()); assert!(crate::run("zonalStats", &m, &serde_json::json!({"raster": "elev", "polygons": "zones", "bands": [0]})).is_err()); assert!(crate::run("zonalStats", &m, &serde_json::json!({"raster": "elev", "polygons": "zones", "bands": [-1]})).is_err()); diff --git a/crates/engine/src/cmd/imagery.rs b/crates/engine/src/cmd/imagery.rs index 73de8edc8..d35f127b8 100644 --- a/crates/engine/src/cmd/imagery.rs +++ b/crates/engine/src/cmd/imagery.rs @@ -560,54 +560,15 @@ fn process(s: &mut Session, p: &Value) -> CmdResult { .iter() .filter_map(|row| match l.dataset.features.get(*row).and_then(|f| f.geometry.as_ref()) { Some(Geometry::Polygon(polys)) => { - let rings: Vec>> = match &poly_tr { - Some(tr) => polys - .iter() - .filter_map(|rings| { - let exterior = rings.first()?; - let mut ext_pts = Vec::with_capacity(exterior.len()); - for &c in exterior { - ext_pts.push(tr.to_lonlat(c)?); - } - if ext_pts.len() < 3 { - return None; - } - let mut valid_rings = vec![ext_pts]; - for hole in rings.iter().skip(1) { - let mut hole_pts = Vec::with_capacity(hole.len()); - let mut ok = true; - for &c in hole { - if let Some(ll) = tr.to_lonlat(c) { - hole_pts.push(ll); - } else { - ok = false; - break; - } - } - if ok && hole_pts.len() >= 3 { - valid_rings.push(hole_pts); - } - } - Some(valid_rings) - }) - .collect(), - None => polys - .iter() - .filter_map(|rings| { - let exterior = rings.first()?; - if exterior.len() < 3 { - return None; - } - let mut valid_rings = vec![exterior.clone()]; - for hole in rings.iter().skip(1) { - if hole.len() >= 3 { - valid_rings.push(hole.clone()); - } - } - Some(valid_rings) + let rings: Vec>> = polys + .iter() + .filter_map(|rings| { + rasterops::transform_polygon(rings, |c| match &poly_tr { + Some(tr) => tr.to_lonlat(c), + None => Some(c), }) - .collect(), - }; + }) + .collect(); Some(rings.into_iter()) } _ => None, diff --git a/crates/ui-egui/src/analysispane.rs b/crates/ui-egui/src/analysispane.rs index c7965dd97..e0b32a7e4 100644 --- a/crates/ui-egui/src/analysispane.rs +++ b/crates/ui-egui/src/analysispane.rs @@ -293,14 +293,14 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { ); }); let band_str = app.workflow.zonal_stats.bands.trim(); - let band_valid = if band_str.is_empty() || band_str.eq_ignore_ascii_case("all") { - true - } else { - band_str.split([',', ';', ' ']).all(|part| { - let p = part.trim().trim_matches(|c| c == '[' || c == ']' || c == '"'); - p.is_empty() || p.parse::().is_ok_and(|b| b >= 1) - }) - }; + let total_bands = app + .session + .map() + .layer_by_name(&app.workflow.layer) + .and_then(|l| l.raster.as_ref()) + .and_then(|r| r.data.as_ref()) + .map_or(0, |r| r.band_count()); + let band_valid = rustgis_engine::analysis::zonal_bands(&json!(band_str), total_bands).is_ok(); let layers_ready = !app.workflow.layer.is_empty() && !app.workflow.layer2.is_empty(); let can_run = layers_ready && band_valid; if !layers_ready { @@ -702,9 +702,7 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { "operation": "clip", "polygon": app.workflow.layer2, }); - if app.workflow.selected_only { - p["selectedOnly"] = json!(true); - } + p["selectedOnly"] = json!(app.workflow.selected_only); run(app, "raster.process", p); } } else { diff --git a/docs/user-guide/analysis.md b/docs/user-guide/analysis.md index 052823219..0171ca745 100644 --- a/docs/user-guide/analysis.md +++ b/docs/user-guide/analysis.md @@ -494,6 +494,18 @@ rustgis-cli run --cmd 'cube.reduce={"layers":["ndvi_2015","ndvi_2016","ndvi_2017 rustgis-cli run --cmd 'cube.reduce={"url":"https://data.inpe.br/bdc/stac/v1","collection":"mod13q1-6.1","asset":"NDVI","bbox":[-44.8,-7.5,-44.3,-7.0],"start":"2001","end":"2025","cellSize":1000,"stat":"mean","by":"year","minValid":0.5,"name":"NDVI"}' --save urucui.rgis ``` +## Zonal statistics and polygon clipping + +Zonal Statistics uses the selected polygon features when the layer has a selection. It keeps +original geometries and fields, and reports features with missing or rejected polygon parts. +A failed coordinate transformation rejects the entire polygon part, including its holes. Large +overlapping zones each require a separate pass over their raster window. + +For `raster.process` polygon clipping, `selectedOnly: true` requires a selection, +`selectedOnly: false` uses every polygon, and omitting it uses the selection when present. +The Process pane sends its Selected only choice explicitly. Clipping and Zonal Statistics share +polygon transformation rules so a failed hole cannot silently enlarge the sampled area. + ## Deep learning and hosted imagery **Analyze › Deep Learning Tools** trains a neural network (a multilayer perceptron) on the pixels From 4321b3c60481e3365efe40f6d19ebda76d616450 Mon Sep 17 00:00:00 2001 From: quanguet0409 Date: Sat, 24 Oct 2026 21:15:02 +0700 Subject: [PATCH 05/11] fix(analysis,engine,ui): address review feedback on polygon transforms, attribute padding, band parsing, and clip semantics --- crates/analysis/src/lib.rs | 2 +- crates/analysis/src/rasterops.rs | 307 +++++++++++++++- crates/analysis/src/tools.rs | 549 +++++++++++++++++++---------- crates/engine/src/cmd/imagery.rs | 121 +++---- crates/engine/src/tests.rs | 179 ++++++++++ crates/ui-egui/src/analysispane.rs | 30 +- 6 files changed, 914 insertions(+), 274 deletions(-) diff --git a/crates/analysis/src/lib.rs b/crates/analysis/src/lib.rs index 37ef54706..6c56967c1 100644 --- a/crates/analysis/src/lib.rs +++ b/crates/analysis/src/lib.rs @@ -24,7 +24,7 @@ pub mod topology; pub mod trend; pub mod warp; -pub use tools::{TOOLS, holds_coordinates, simplify_geometry}; +pub use tools::{TOOLS, find_field_case_insensitive, holds_coordinates, simplify_geometry}; use rustgis_data::Dataset; use serde_json::Value; diff --git a/crates/analysis/src/rasterops.rs b/crates/analysis/src/rasterops.rs index 31bc58ec9..44462607b 100644 --- a/crates/analysis/src/rasterops.rs +++ b/crates/analysis/src/rasterops.rs @@ -4,7 +4,7 @@ use std::ops::Range; -use rustgis_data::raster::{ColorHint, Crs, GeoTransform, Raster}; +use rustgis_data::raster::{ColorHint, Crs, GeoTransform, Raster, Transformer}; use rustgis_geom::{Coord, Extent}; /// The no-data value written by these operations. @@ -523,6 +523,176 @@ pub fn zonal_means(r: &Raster, bands: &[usize], rings: &[Vec]) -> Option< (n > 0).then(|| (sums.into_iter().map(|s| s / n as f64).collect(), n)) } +/// An error encountered while validating or transforming a polygon geometry. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum PolygonTransformError { + EmptyPolygon, + InvalidExterior(String), + InvalidHole { index: usize, reason: String }, +} + +impl std::fmt::Display for PolygonTransformError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::EmptyPolygon => write!(f, "polygon has no rings"), + Self::InvalidExterior(r) => write!(f, "exterior ring is invalid: {r}"), + Self::InvalidHole { index, reason } => write!(f, "hole {index} is invalid: {reason}"), + } + } +} + +impl std::error::Error for PolygonTransformError {} + +/// Transforms a single polygon's rings (exterior ring + optional holes) to WGS 84 lon/lat coordinates. +/// +/// - The exterior ring must have at least 3 points and all points must project successfully. +/// - Every interior ring (hole) must have at least 3 points and all points must project successfully. +/// - If any hole fails or is invalid, the entire polygon is rejected with an error rather than +/// silently discarding the hole or altering the polygon geometry. +pub fn transform_polygon_to_lonlat(rings: &[Vec], tr: Option<&Transformer>) -> Result>, PolygonTransformError> { + let exterior = rings.first().ok_or(PolygonTransformError::EmptyPolygon)?; + let mut ext_lonlat = Vec::with_capacity(exterior.len()); + for &c in exterior { + if !c.is_finite() { + return Err(PolygonTransformError::InvalidExterior("non-finite coordinate".into())); + } + let ll = match tr { + Some(t) => t.to_lonlat(c).ok_or_else(|| PolygonTransformError::InvalidExterior("coordinate transformation failed".into()))?, + None => c, + }; + if !ll.is_finite() { + return Err(PolygonTransformError::InvalidExterior("transformed coordinate is non-finite".into())); + } + ext_lonlat.push(ll); + } + if ext_lonlat.len() < 3 { + return Err(PolygonTransformError::InvalidExterior("fewer than 3 vertices".into())); + } + + let mut out_rings = vec![ext_lonlat]; + for (idx, hole) in rings.iter().skip(1).enumerate() { + let hole_num = idx + 1; + let mut hole_lonlat = Vec::with_capacity(hole.len()); + for &c in hole { + if !c.is_finite() { + return Err(PolygonTransformError::InvalidHole { index: hole_num, reason: "non-finite coordinate".into() }); + } + let ll = match tr { + Some(t) => t.to_lonlat(c).ok_or_else(|| PolygonTransformError::InvalidHole { + index: hole_num, + reason: "coordinate transformation failed".into(), + })?, + None => c, + }; + if !ll.is_finite() { + return Err(PolygonTransformError::InvalidHole { index: hole_num, reason: "transformed coordinate is non-finite".into() }); + } + hole_lonlat.push(ll); + } + if hole_lonlat.len() < 3 { + return Err(PolygonTransformError::InvalidHole { index: hole_num, reason: "fewer than 3 vertices".into() }); + } + out_rings.push(hole_lonlat); + } + + Ok(out_rings) +} + +/// Transforms and validates all polygon parts of a (multi-)polygon feature to WGS 84 lon/lat coordinates. +pub fn transform_feature_polygons_to_lonlat( + polys: &[Vec>], + tr: Option<&Transformer>, +) -> Result>>, PolygonTransformError> { + if polys.is_empty() { + return Err(PolygonTransformError::EmptyPolygon); + } + let mut out = Vec::with_capacity(polys.len()); + for (part_idx, rings) in polys.iter().enumerate() { + let valid_poly = transform_polygon_to_lonlat(rings, tr).map_err(|e| match e { + PolygonTransformError::InvalidHole { index, reason } => { + PolygonTransformError::InvalidHole { index, reason: format!("part {part_idx}: {reason}") } + } + PolygonTransformError::InvalidExterior(reason) => PolygonTransformError::InvalidExterior(format!("part {part_idx}: {reason}")), + other => other, + })?; + out.push(valid_poly); + } + Ok(out) +} + +/// Parses a band specification string into 0-based band indices. +/// +/// Supported inputs: +/// - `""` (empty string): defaults to band 1 (`vec![0]`) +/// - `"all"`: all bands `0..total_bands` +/// - comma, semicolon, or space-separated list of 1-based band numbers: `"1, 2, 3"`, `"[1, 2]"`, etc. +/// +/// Validates that: +/// - Every entry is a valid, finite positive integer. +/// - Decimals (e.g. `"1.5"`), negative numbers (e.g. `"-1"`), zero (`"0"`), non-numeric (e.g. `"abc"`), +/// or non-finite values (e.g. `"NaN"`, `"Infinity"`) are strictly rejected. +/// - Every band number is within `1..=total_bands`. +/// - Empty parts (such as `"1,,2"` or `","`) are rejected. +pub fn parse_bands_str(s: &str, total_bands: usize) -> Result, String> { + if total_bands == 0 { + return Err("raster has no bands".into()); + } + let txt = s.trim(); + if txt.is_empty() { + return Ok(vec![0]); + } + if txt.eq_ignore_ascii_case("all") { + return Ok((0..total_bands).collect()); + } + let clean = txt.trim_matches(|c| c == '[' || c == ']'); + if clean.trim().is_empty() { + return Err("band selection cannot be empty brackets".into()); + } + let mut out = Vec::new(); + for part in clean.split([',', ';']) { + let part = part.trim().trim_matches('"').trim(); + if part.is_empty() { + return Err("band selection contains an empty band entry".into()); + } + for subpart in part.split_whitespace() { + let subpart = subpart.trim(); + if subpart.is_empty() { + continue; + } + if subpart.eq_ignore_ascii_case("all") { + for b in 0..total_bands { + if !out.contains(&b) { + out.push(b); + } + } + continue; + } + let f: f64 = subpart.parse().map_err(|_| format!("invalid band number '{subpart}'"))?; + if !f.is_finite() { + return Err("band number must be a finite number".into()); + } + if f <= 0.0 { + return Err("band numbers must be positive (1-based)".into()); + } + if f.fract() != 0.0 { + return Err("band numbers must be integers, not decimals".into()); + } + let b = f as usize; + if b > total_bands { + return Err(format!("band index {b} is invalid: raster has bands 1 to {total_bands}")); + } + let b_idx = b - 1; + if !out.contains(&b_idx) { + out.push(b_idx); + } + } + } + if out.is_empty() { + return Err("no valid band numbers found in input".into()); + } + Ok(out) +} + /// The cells of `r` whose centres fall inside any of `polygons` (each an exterior ring and its /// holes, in longitude/latitude): the column and row window around the polygons, clamped to the /// raster, and a row-major mask over it. `None` when no cell centre falls inside. @@ -537,8 +707,15 @@ pub fn polygon_mask(r: &Raster, polygons: &[Vec>]) -> Option<(Range>]) -> Option<(Range= 3 { - valid_rings.push(hole_pts); + if hole_pts.len() < 3 { + return None; } + valid_rings.push(hole_pts); } Some(valid_rings) }) @@ -1157,5 +1337,112 @@ mod tests { let nan_ext_stats = zonal_statistics(&r, &[0], &nan_ext_with_hole).unwrap(); assert_eq!(nan_ext_stats[0], ZoneStats::default()); assert!(polygon_mask(&r, &nan_ext_with_hole).is_none()); + + // 9. Polygon with defective hole (< 3 vertices or non-finite) is rejected rather than silently discarding the hole + let bad_hole_poly = vec![vec![ + sq(0.005, 0.005, 0.015, 0.015), // valid exterior (5 points) + vec![Coord::new(0.009, 0.009), Coord::new(0.011, 0.011)], // invalid hole: only 2 points! + ]]; + assert!(polygon_mask(&r, &bad_hole_poly).is_none(), "polygon with defective hole must be rejected"); + let bad_hole_stats = zonal_statistics(&r, &[0], &bad_hole_poly).unwrap(); + assert_eq!(bad_hole_stats[0], ZoneStats::default()); + + let nan_hole_poly = vec![vec![ + sq(0.005, 0.005, 0.015, 0.015), + vec![Coord::new(f64::NAN, 0.009), Coord::new(0.011, 0.009), Coord::new(0.011, 0.011), Coord::new(f64::NAN, 0.009)], + ]]; + assert!(polygon_mask(&r, &nan_hole_poly).is_none(), "polygon with non-finite hole must be rejected"); + } + + #[test] + fn polygon_transformation_and_hole_validation_semantics() { + let sq = |x0: f64, y0: f64, x1: f64, y1: f64| { + vec![Coord::new(x0, y0), Coord::new(x1, y0), Coord::new(x1, y1), Coord::new(x0, y1), Coord::new(x0, y0)] + }; + let ext = sq(0.0, 0.0, 10.0, 10.0); + let hole1 = sq(2.0, 2.0, 4.0, 4.0); + let hole2 = sq(6.0, 6.0, 8.0, 8.0); + + // Valid exterior and valid holes + let valid_poly = vec![ext.clone(), hole1.clone(), hole2.clone()]; + let res = transform_polygon_to_lonlat(&valid_poly, None).unwrap(); + assert_eq!(res.len(), 3); + assert_eq!(res[0].len(), ext.len()); + assert_eq!(res[1].len(), hole1.len()); + assert_eq!(res[2].len(), hole2.len()); + + // Empty polygon + let empty_poly: Vec> = Vec::new(); + assert_eq!(transform_polygon_to_lonlat(&empty_poly, None), Err(PolygonTransformError::EmptyPolygon)); + + // Invalid exterior: fewer than 3 vertices + let bad_ext = vec![vec![Coord::new(0.0, 0.0), Coord::new(1.0, 1.0)]]; + assert!(matches!(transform_polygon_to_lonlat(&bad_ext, None), Err(PolygonTransformError::InvalidExterior(_)))); + + // Invalid exterior: non-finite coordinate + let nan_ext = vec![vec![Coord::new(0.0, 0.0), Coord::new(f64::NAN, 1.0), Coord::new(2.0, 2.0), Coord::new(0.0, 0.0)]]; + assert!(matches!(transform_polygon_to_lonlat(&nan_ext, None), Err(PolygonTransformError::InvalidExterior(_)))); + + // Invalid hole 1: fewer than 3 vertices + let bad_hole = vec![ext.clone(), vec![Coord::new(2.0, 2.0), Coord::new(3.0, 3.0)]]; + assert_eq!( + transform_polygon_to_lonlat(&bad_hole, None), + Err(PolygonTransformError::InvalidHole { index: 1, reason: "fewer than 3 vertices".into() }) + ); + + // Invalid hole 2: non-finite coordinate + let nan_hole = vec![ext.clone(), hole1.clone(), vec![Coord::new(6.0, 6.0), Coord::new(f64::INFINITY, 8.0), Coord::new(8.0, 8.0)]]; + assert!(matches!(transform_polygon_to_lonlat(&nan_hole, None), Err(PolygonTransformError::InvalidHole { index: 2, .. }))); + + // Multipart features + let multi_valid = vec![vec![ext.clone(), hole1.clone()], vec![sq(20.0, 20.0, 30.0, 30.0)]]; + let multi_res = transform_feature_polygons_to_lonlat(&multi_valid, None).unwrap(); + assert_eq!(multi_res.len(), 2); + assert_eq!(multi_res[0].len(), 2); + assert_eq!(multi_res[1].len(), 1); + + // Multipart with failing part + let multi_bad = vec![vec![ext.clone()], vec![vec![Coord::new(1.0, 1.0)]]]; + assert!(transform_feature_polygons_to_lonlat(&multi_bad, None).is_err()); + } + + #[test] + fn parse_bands_str_validation_and_boundaries() { + let total = 3usize; + + // Valid formats + assert_eq!(parse_bands_str("", total).unwrap(), vec![0]); + assert_eq!(parse_bands_str(" ", total).unwrap(), vec![0]); + assert_eq!(parse_bands_str("all", total).unwrap(), vec![0, 1, 2]); + assert_eq!(parse_bands_str("ALL", total).unwrap(), vec![0, 1, 2]); + assert_eq!(parse_bands_str("1", total).unwrap(), vec![0]); + assert_eq!(parse_bands_str("3", total).unwrap(), vec![2]); // boundary: total_bands + assert_eq!(parse_bands_str("1, 2", total).unwrap(), vec![0, 1]); + assert_eq!(parse_bands_str("[1, 2, 3]", total).unwrap(), vec![0, 1, 2]); + assert_eq!(parse_bands_str("1; 3", total).unwrap(), vec![0, 2]); + assert_eq!(parse_bands_str(" 1 2 ", total).unwrap(), vec![0, 1]); + + // De-duplication + assert_eq!(parse_bands_str("1, 1, 2, 2, 3", total).unwrap(), vec![0, 1, 2]); + + // Boundary violations and negative + assert!(parse_bands_str("0", total).is_err(), "0 is invalid (1-based)"); + assert!(parse_bands_str("-1", total).is_err(), "negative band is invalid"); + assert!(parse_bands_str("4", total).is_err(), "out of range band is invalid"); + assert!(parse_bands_str("999", total).is_err(), "out of range band is invalid"); + + // Decimals and non-finite + assert!(parse_bands_str("1.5", total).is_err(), "decimal band is invalid"); + assert_eq!(parse_bands_str("1.0", total).unwrap(), vec![0]); // 1.0 has fract == 0.0 + assert!(parse_bands_str("NaN", total).is_err(), "NaN is invalid"); + assert!(parse_bands_str("Infinity", total).is_err(), "Infinity is invalid"); + + // Malformed strings + assert!(parse_bands_str("abc", total).is_err(), "non-numeric string is invalid"); + assert!(parse_bands_str("[]", total).is_err(), "empty brackets are invalid"); + assert!(parse_bands_str("[ ]", total).is_err(), "whitespace brackets are invalid"); + assert!(parse_bands_str("1,,2", total).is_err(), "empty item between commas is invalid"); + assert!(parse_bands_str(",", total).is_err(), "standalone comma is invalid"); + assert!(parse_bands_str(";", total).is_err(), "standalone semicolon is invalid"); } } diff --git a/crates/analysis/src/tools.rs b/crates/analysis/src/tools.rs index bfbbd1b1e..847cb9131 100644 --- a/crates/analysis/src/tools.rs +++ b/crates/analysis/src/tools.rs @@ -573,6 +573,30 @@ fn copy_schema(ds: &Dataset, name: String, g: GeometryType) -> Dataset { Dataset::new(name, g, ds.fields.clone()) } +/// Finds the index of a field by case-insensitive name, explicitly detecting collisions. +/// +/// If multiple fields in `ds.fields` match `name` case-insensitively, returns an error +/// detailing the collision rather than silently selecting an arbitrary field. +pub fn find_field_case_insensitive(ds: &Dataset, name: &str) -> Result { + let mut matches = Vec::new(); + for (idx, f) in ds.fields.iter().enumerate() { + if f.name.eq_ignore_ascii_case(name) { + matches.push(idx); + } + } + match matches.len() { + 0 => Err(format!("no field named '{name}'")), + 1 => Ok(matches[0]), + _ => { + let names: Vec = matches.iter().map(|&i| ds.fields[i].name.clone()).collect(); + Err(format!( + "field name '{name}' is ambiguous: case-insensitive collision between {}", + names.iter().map(|n| format!("'{n}'")).collect::>().join(" and ") + )) + } + } +} + /// Union of polygons; rings are oriented first because the overlay infers its fill rule from /// the first ring's winding, and input files mix orientations. fn union_all<'a>(polys: impl IntoIterator>) -> MultiPolygon { @@ -682,7 +706,7 @@ fn dissolve(inp: &dyn Inputs, p: &Json) -> Result { return Err("Dissolve needs a polygon layer".into()); } let field = s(p, "field").trim(); - let fi = if field.is_empty() { None } else { Some(ds.field_index(field).ok_or_else(|| format!("no field named '{field}'"))?) }; + let fi = if field.is_empty() { None } else { Some(find_field_case_insensitive(ds, field)?) }; let mut groups: Vec<(Value, Vec>)> = Vec::new(); for r in rows(inp, p, "input", ds) { let Some(mp) = ds.features.get(r).and_then(|f| f.geometry.as_ref()).and_then(to_multipolygon) else { continue }; @@ -795,7 +819,7 @@ fn xy_table_to_point(inp: &dyn Inputs, p: &Json) -> Result { let ds = input(inp, p, "input")?; let field = |k: &str| { let name = s(p, k).trim(); - ds.field_index(name).ok_or_else(|| format!("no field named '{name}'")) + find_field_case_insensitive(ds, name) }; let (xi, yi) = (field("x")?, field("y")?); let crs = match s(p, "crs").trim() { @@ -913,7 +937,7 @@ fn select_by_attributes(inp: &dyn Inputs, p: &Json) -> Result Result { let ds = input(inp, p, "input")?; let field = s(p, "field"); - let fi = ds.field_index(field).ok_or_else(|| format!("no field named '{field}'"))?; + let fi = find_field_case_insensitive(ds, field)?; let mut pts: Vec<(Coord, f64)> = Vec::new(); for r in rows(inp, p, "input", ds) { let Some(g) = ds.features.get(r).and_then(|f| f.geometry.as_ref()) else { continue }; @@ -985,7 +1009,7 @@ fn kriging_interpolation(inp: &dyn Inputs, p: &Json) -> Result = Vec::new(); for r in rows(inp, p, "input", ds) { let Some(g) = ds.features.get(r).and_then(|f| f.geometry.as_ref()) else { continue }; @@ -1205,9 +1229,9 @@ fn calculate_geometry(inp: &dyn Inputs, p: &Json) -> Result fn summary_statistics(inp: &dyn Inputs, p: &Json) -> Result { let ds = input(inp, p, "input")?; let field = s(p, "field"); - let fi = ds.field_index(field).ok_or_else(|| format!("no field named '{field}'"))?; + let fi = find_field_case_insensitive(ds, field)?; let case = s(p, "case").trim(); - let ci = if case.is_empty() { None } else { Some(ds.field_index(case).ok_or_else(|| format!("no field named '{case}'"))?) }; + let ci = if case.is_empty() { None } else { Some(find_field_case_insensitive(ds, case)?) }; let mut groups: Vec<(Value, Vec)> = Vec::new(); for r in rows(inp, p, "input", ds) { let key = ci.map(|c| ds.value(r, c).clone()).unwrap_or(Value::Null); @@ -1505,21 +1529,32 @@ impl StatOp { fn parse(raw: &str) -> Result, String> { let clean = raw.trim().to_ascii_lowercase(); match clean.as_str() { - "all" | "" => Ok(Self::ALL.to_vec()), + "all" => Ok(Self::ALL.to_vec()), "mean" => Ok(vec![StatOp::Mean]), "min" => Ok(vec![StatOp::Min]), "max" => Ok(vec![StatOp::Max]), "sum" => Ok(vec![StatOp::Sum]), "count" => Ok(vec![StatOp::Count]), "stddev" | "std" | "std_dev" => Ok(vec![StatOp::StdDev]), + "" => Err("empty statistic name: choose from mean, min, max, sum, count, stddev, or all".into()), other => Err(format!("unknown statistic '{other}': choose from mean, min, max, sum, count, stddev, or all")), } } } -fn parse_single_band(item: &Json, total_bands: usize, out: &mut Vec) -> Result<(), String> { - match item { - Json::Number(n) => { +fn parse_bands(p: &Json, total_bands: usize) -> Result, String> { + if total_bands == 0 { + return Err("raster has no bands".into()); + } + match p.get("bands") { + None | Some(Json::Null) => Ok(vec![0]), + Some(Json::String(s)) => { + if s.trim().is_empty() { + return Err("bands parameter cannot be an empty string".into()); + } + crate::rasterops::parse_bands_str(s, total_bands) + } + Some(Json::Number(n)) => { let f = n.as_f64().ok_or_else(|| "invalid band number".to_string())?; if !f.is_finite() { return Err("band number must be a finite number".into()); @@ -1534,167 +1569,100 @@ fn parse_single_band(item: &Json, total_bands: usize, out: &mut Vec) -> R if b > total_bands { return Err(format!("band index {b} is invalid: raster has bands 1 to {total_bands}")); } - out.push(b); - Ok(()) - } - Json::String(s) => { - let txt = s.trim(); - if txt.eq_ignore_ascii_case("all") { - out.extend(1..=total_bands); - Ok(()) - } else { - let clean = txt.trim_matches(|c| c == '[' || c == ']'); - for part in clean.split([',', ';', ' ']) { - let part = part.trim().trim_matches('"'); - if part.is_empty() { - continue; - } - if part.eq_ignore_ascii_case("all") { - out.extend(1..=total_bands); - continue; - } - let f: f64 = part.parse().map_err(|_| format!("invalid band number '{part}'"))?; - if !f.is_finite() { - return Err("band number must be a finite number".into()); - } - if f <= 0.0 { - return Err("band numbers must be positive (1-based)".into()); - } - if f.fract() != 0.0 { - return Err("band numbers must be integers, not decimals".into()); + Ok(vec![b - 1]) + } + Some(Json::Array(arr)) => { + if arr.is_empty() { + return Err("bands array cannot be empty".into()); + } + let mut out = Vec::new(); + for item in arr { + match item { + Json::Number(n) => { + let f = n.as_f64().ok_or_else(|| "invalid band number".to_string())?; + if !f.is_finite() { + return Err("band number must be a finite number".into()); + } + if f <= 0.0 { + return Err("band numbers must be positive (1-based)".into()); + } + if f.fract() != 0.0 { + return Err("band numbers must be integers, not decimals".into()); + } + let b = f as usize; + if b > total_bands { + return Err(format!("band index {b} is invalid: raster has bands 1 to {total_bands}")); + } + let b_idx = b - 1; + if !out.contains(&b_idx) { + out.push(b_idx); + } } - let b = f as usize; - if b > total_bands { - return Err(format!("band index {b} is invalid: raster has bands 1 to {total_bands}")); + Json::String(s) => { + let sub = crate::rasterops::parse_bands_str(s, total_bands)?; + for b_idx in sub { + if !out.contains(&b_idx) { + out.push(b_idx); + } + } } - out.push(b); + _ => return Err("band items must be numbers or strings".into()), } - Ok(()) } + Ok(out) } - _ => Err("band numbers must be positive integers or 'all'".into()), + _ => Err("bands must be a number, string, or array of numbers/strings".into()), } } -fn parse_bands(p: &Json, total_bands: usize) -> Result, String> { - let mut raw_bands = Vec::new(); - if let Some(arr) = p.get("bands").and_then(Json::as_array) { - if arr.is_empty() { - raw_bands.push(1); - } else { - for item in arr { - parse_single_band(item, total_bands, &mut raw_bands)?; - } - } - } else if let Some(v) = p.get("bands") { - if v.is_null() { - raw_bands.push(1); - } else { - parse_single_band(v, total_bands, &mut raw_bands)?; - } - } else { - raw_bands.push(1); - } - if raw_bands.is_empty() { - raw_bands.push(1); - } - let mut unique = Vec::new(); - for b in raw_bands { - if !unique.contains(&b) { - unique.push(b); - } - } - Ok(unique.into_iter().map(|b| b - 1).collect()) -} - fn parse_stats(p: &Json) -> Result, String> { - let mut out = Vec::new(); - if let Some(arr) = p.get("stats").and_then(Json::as_array) { - if arr.is_empty() { - return Ok(StatOp::ALL.to_vec()); - } - for item in arr { - let txt = item.as_str().ok_or_else(|| "statistic names must be strings".to_string())?; - let ops = StatOp::parse(txt)?; - for op in ops { - if !out.contains(&op) { - out.push(op); + match p.get("stats") { + None | Some(Json::Null) => Ok(StatOp::ALL.to_vec()), + Some(Json::Array(arr)) => { + if arr.is_empty() { + return Err("stats array cannot be empty".into()); + } + let mut out = Vec::new(); + for item in arr { + let txt = item.as_str().ok_or_else(|| "statistic names must be strings".to_string())?; + if txt.trim().is_empty() { + return Err("statistic name cannot be empty".into()); + } + let ops = StatOp::parse(txt)?; + for op in ops { + if !out.contains(&op) { + out.push(op); + } } } + Ok(out) } - } else { - let txt = s(p, "stats").trim(); - if txt.is_empty() { - return Ok(StatOp::ALL.to_vec()); - } - let clean = txt.trim_matches(|c| c == '[' || c == ']'); - for part in clean.split([',', ';']) { - let part = part.trim().trim_matches('"'); - if part.is_empty() { - continue; + Some(Json::String(txt)) => { + let trimmed = txt.trim(); + if trimmed.is_empty() { + return Err("stats parameter cannot be empty string".into()); } - let ops = StatOp::parse(part)?; - for op in ops { - if !out.contains(&op) { - out.push(op); - } + let clean = trimmed.trim_matches(|c| c == '[' || c == ']'); + if clean.trim().is_empty() { + return Err("stats parameter cannot be empty brackets".into()); } - } - } - if out.is_empty() { - out = StatOp::ALL.to_vec(); - } - Ok(out) -} - -/// Projects a polygon (exterior ring + optional holes) to WGS 84 lon/lat coordinates. -/// -/// Returns `None` if the exterior ring fails transformation or has fewer than 3 vertices. -/// A hole is never promoted to become the exterior ring. -/// Holes whose vertices fail transformation or have fewer than 3 vertices are skipped, -/// preserving the valid exterior ring and any surviving valid holes. -/// If any coordinate in a ring cannot be transformed, the entire ring is rejected rather -/// than silently dropping vertices and distorting the zone boundary. -fn reproject_polygon(rings: &[Vec], tr: &rustgis_data::raster::Transformer) -> Option>> { - let exterior = rings.first()?; - let mut ext_lonlat = Vec::with_capacity(exterior.len()); - for &c in exterior { - ext_lonlat.push(tr.to_lonlat(c)?); - } - if ext_lonlat.len() < 3 { - return None; - } - let mut out_rings = vec![ext_lonlat]; - for hole in rings.iter().skip(1) { - let mut hole_lonlat = Vec::with_capacity(hole.len()); - let mut ok = true; - for &c in hole { - if let Some(ll) = tr.to_lonlat(c) { - hole_lonlat.push(ll); - } else { - ok = false; - break; + let mut out = Vec::new(); + for part in clean.split([',', ';']) { + let part = part.trim().trim_matches('"').trim(); + if part.is_empty() { + return Err("stats selection contains an empty statistic entry".into()); + } + let ops = StatOp::parse(part)?; + for op in ops { + if !out.contains(&op) { + out.push(op); + } + } } + Ok(out) } - if ok && hole_lonlat.len() >= 3 { - out_rings.push(hole_lonlat); - } - } - Some(out_rings) -} - -fn validate_lonlat_polygon(rings: &[Vec]) -> Option>> { - let exterior = rings.first()?; - if exterior.len() < 3 { - return None; - } - let mut out_rings = vec![exterior.clone()]; - for hole in rings.iter().skip(1) { - if hole.len() >= 3 { - out_rings.push(hole.clone()); - } + _ => Err("stats parameter must be a string or array of strings".into()), } - Some(out_rings) } fn zonal_stats(inp: &dyn Inputs, p: &Json) -> Result { @@ -1770,22 +1738,26 @@ fn zonal_stats(inp: &dyn Inputs, p: &Json) -> Result { let target_rows = rows(inp, p, poly_param, poly_ds); let mut skipped_features = 0usize; + let mut failure_details = Vec::new(); for r_idx in target_rows { let Some(f) = poly_ds.features.get(r_idx) else { continue }; let (stats_by_band, feature_valid) = match &f.geometry { Some(Geometry::Polygon(polys)) if !polys.is_empty() => { - let polys_lonlat: Vec>> = match &poly_tr { - Some(tr) => polys.iter().filter_map(|rings| reproject_polygon(rings, tr)).collect(), - None => polys.iter().filter_map(|rings| validate_lonlat_polygon(rings)).collect(), - }; - if polys_lonlat.is_empty() { - (vec![crate::rasterops::ZoneStats::default(); bands.len()], false) - } else { - let st = crate::rasterops::zonal_statistics(r, &bands, &polys_lonlat)?; - (st, true) + match crate::rasterops::transform_feature_polygons_to_lonlat(polys, poly_tr.as_ref()) { + Ok(polys_lonlat) => { + let st = crate::rasterops::zonal_statistics(r, &bands, &polys_lonlat)?; + (st, true) + } + Err(e) => { + failure_details.push(format!("feature {}: {e}", f.id)); + (vec![crate::rasterops::ZoneStats::default(); bands.len()], false) + } } } - _ => (vec![crate::rasterops::ZoneStats::default(); bands.len()], false), + _ => { + failure_details.push(format!("feature {}: missing or empty geometry", f.id)); + (vec![crate::rasterops::ZoneStats::default(); bands.len()], false) + } }; if !feature_valid { @@ -1793,6 +1765,7 @@ fn zonal_stats(inp: &dyn Inputs, p: &Json) -> Result { } let mut attrs = f.attrs.clone(); + attrs.resize(poly_ds.fields.len(), Value::Null); for (b_idx, _, stat, _, _) in &out_fields { let b_stats = stats_by_band.get(*b_idx); let val = match (b_stats, stat) { @@ -1806,7 +1779,6 @@ fn zonal_stats(inp: &dyn Inputs, p: &Json) -> Result { }; attrs.push(val); } - attrs.resize(out.fields.len(), Value::Null); out.features.push(rustgis_data::Feature { id: f.id, geometry: f.geometry.clone(), attrs }); } @@ -1814,6 +1786,12 @@ fn zonal_stats(inp: &dyn Inputs, p: &Json) -> Result { let mut messages = vec![format!("Calculated zonal statistics for {n_features} feature(s)")]; if skipped_features > 0 { messages.push(format!("{skipped_features} feature(s) had invalid geometry or failed coordinate transformation")); + for detail in failure_details.iter().take(5) { + messages.push(detail.clone()); + } + if failure_details.len() > 5 { + messages.push(format!("... and {} more feature failure(s)", failure_details.len() - 5)); + } } Ok(ToolResult { output: Output::Layer(out), messages }) } @@ -1821,6 +1799,7 @@ fn zonal_stats(inp: &dyn Inputs, p: &Json) -> Result { #[cfg(test)] mod tests { use super::*; + use rustgis_data::Feature; use std::collections::HashMap; struct Map(HashMap); @@ -1833,6 +1812,24 @@ mod tests { } } + struct RasterMap<'a> { + datasets: HashMap, + rasters: HashMap, + } + + impl<'a> Inputs for RasterMap<'a> { + fn dataset(&self, layer: &str) -> Option<&Dataset> { + self.datasets.get(layer) + } + fn selection(&self, _: &str) -> &[usize] { + &[] + } + fn raster(&self, layer: &str) -> Option> { + let r = self.rasters.get(layer)?; + Some(crate::RasterInput { raster: r, path: None, embedded: None }) + } + } + fn sq(x: f64, y: f64, s: f64) -> Geometry { Geometry::Polygon(vec![vec![vec![ Coord::new(x, y), @@ -2615,24 +2612,6 @@ mod tests { #[test] fn zonal_stats_tool_cases() { - struct RasterMap<'a> { - datasets: HashMap, - rasters: HashMap, - } - - impl<'a> Inputs for RasterMap<'a> { - fn dataset(&self, layer: &str) -> Option<&Dataset> { - self.datasets.get(layer) - } - fn selection(&self, _: &str) -> &[usize] { - &[] - } - fn raster(&self, layer: &str) -> Option> { - let r = self.rasters.get(layer)?; - Some(crate::RasterInput { raster: r, path: None, embedded: None }) - } - } - let (w, h) = (10u32, 10u32); let mut b1_data: Vec = Vec::with_capacity((w * h) as usize); let mut b2_data: Vec = Vec::with_capacity((w * h) as usize); @@ -2948,4 +2927,208 @@ mod tests { assert_eq!(m.datasets["zones"], input_clone); } + + #[test] + fn zonal_stats_attribute_padding_and_alignment_regression() { + use rustgis_data::raster::{ColorHint, Crs, GeoTransform, Raster}; + + let (w, h) = (4u32, 4u32); + let data: Vec = (1..=16).map(|v| v as f32).collect(); + let gt = GeoTransform { origin_x: 0.0, origin_y: 4.0, pixel_width: 1.0, pixel_height: -1.0 }; + let raster = Raster::new("elev", w, h, vec![data], gt, Crs::LonLat, None, ColorHint::Gray, "Float32").unwrap(); + + let fields = + vec![Field::new("name", FieldType::Text), Field::new("category", FieldType::Text), Field::new("score", FieldType::Double)]; + let mut ds = Dataset::new("zones_pad", GeometryType::Polygon, fields); + let sq_geom = |x0: f64, y0: f64, x1: f64, y1: f64| { + Geometry::Polygon(vec![vec![vec![ + Coord::new(x0, y0), + Coord::new(x1, y0), + Coord::new(x1, y1), + Coord::new(x0, y1), + Coord::new(x0, y0), + ]]]) + }; + let geom = Some(sq_geom(0.0, 3.0, 2.0, 4.0)); // 2 cells: 1.0, 2.0 + + // Feature 1: shorter attribute array (only 1 attribute instead of 3) + ds.features.push(Feature { id: 1, geometry: geom.clone(), attrs: vec![Value::Text("FeatureA".into())] }); + // Feature 2: full attribute array (3 attributes) + ds.features.push(Feature { + id: 2, + geometry: geom.clone(), + attrs: vec![Value::Text("FeatureB".into()), Value::Text("Cat1".into()), Value::Float(88.5)], + }); + // Feature 3: empty attribute array (0 attributes) + ds.features.push(Feature { id: 3, geometry: geom, attrs: vec![] }); + + let m = + RasterMap { datasets: HashMap::from([("zones_pad".to_string(), ds)]), rasters: HashMap::from([("elev".to_string(), &raster)]) }; + + let res = crate::run("zonalStats", &m, &serde_json::json!({"raster": "elev", "polygons": "zones_pad"})).unwrap(); + let out = layer(res); + + // Schema must contain 3 original fields + 6 stat fields + assert_eq!(out.fields.len(), 9); + assert_eq!(out.fields[0].name, "name"); + assert_eq!(out.fields[1].name, "category"); + assert_eq!(out.fields[2].name, "score"); + assert_eq!(out.fields[3].name, "elev_mean"); + assert_eq!(out.fields[4].name, "elev_min"); + assert_eq!(out.fields[5].name, "elev_max"); + assert_eq!(out.fields[6].name, "elev_sum"); + assert_eq!(out.fields[7].name, "elev_count"); + assert_eq!(out.fields[8].name, "elev_stddev"); + + // Feature 1: attrs must be padded to 3 before stats fields! + let f1 = &out.features[0]; + assert_eq!(f1.attrs.len(), 9); + assert_eq!(f1.attrs[0], Value::Text("FeatureA".into())); + assert_eq!(f1.attrs[1], Value::Null, "missing category attribute must be padded with Null"); + assert_eq!(f1.attrs[2], Value::Null, "missing score attribute must be padded with Null"); + assert_eq!(f1.attrs[3], Value::Float(1.5), "elev_mean must be at index 3, not in column 1!"); + assert_eq!(f1.attrs[7], Value::Int(2), "elev_count must be at index 7"); + + // Feature 2: complete attributes + let f2 = &out.features[1]; + assert_eq!(f2.attrs.len(), 9); + assert_eq!(f2.attrs[0], Value::Text("FeatureB".into())); + assert_eq!(f2.attrs[1], Value::Text("Cat1".into())); + assert_eq!(f2.attrs[2], Value::Float(88.5)); + assert_eq!(f2.attrs[3], Value::Float(1.5)); + + // Feature 3: empty attributes padded completely + let f3 = &out.features[2]; + assert_eq!(f3.attrs.len(), 9); + assert_eq!(f3.attrs[0], Value::Null); + assert_eq!(f3.attrs[1], Value::Null); + assert_eq!(f3.attrs[2], Value::Null); + assert_eq!(f3.attrs[3], Value::Float(1.5)); + } + + #[test] + fn field_name_case_insensitive_and_collision_regression() { + let fields = vec![Field::new("Population", FieldType::Double), Field::new("Area_KM2", FieldType::Double)]; + let mut ds = Dataset::new("cities", GeometryType::None, fields); + ds.push(None, vec![Value::Float(100.0), Value::Float(10.0)]); + ds.push(None, vec![Value::Float(200.0), Value::Float(20.0)]); + + // 1. Case-insensitive lookup succeeds when casing differs + assert_eq!(find_field_case_insensitive(&ds, "population").unwrap(), 0); + assert_eq!(find_field_case_insensitive(&ds, "POPULATION").unwrap(), 0); + assert_eq!(find_field_case_insensitive(&ds, "area_km2").unwrap(), 1); + assert_eq!(find_field_case_insensitive(&ds, "AREA_km2").unwrap(), 1); + + // 2. Missing field + assert!(find_field_case_insensitive(&ds, "unknown").is_err()); + + // 3. Summary statistics with lowercase lookup preserves original casing in output fields + let m = Map(HashMap::from([("cities".to_string(), ds.clone())])); + let res = crate::run("summaryStatistics", &m, &serde_json::json!({"input": "cities", "field": "population"})).unwrap(); + let out = layer(res); + assert!(out.field_index("COUNT_Population").is_some(), "output field must preserve original casing"); + assert!(out.field_index("SUM_Population").is_some()); + assert!(out.field_index("MEAN_Population").is_some()); + + // 4. Case-insensitive collision detection + let colliding_fields = vec![Field::new("Population", FieldType::Double), Field::new("population", FieldType::Double)]; + let ds_collision = Dataset::new("colliding", GeometryType::None, colliding_fields); + let err = find_field_case_insensitive(&ds_collision, "population").unwrap_err(); + assert!(err.contains("ambiguous") && err.contains("collision"), "collision must be explicitly reported: {err}"); + let err2 = find_field_case_insensitive(&ds_collision, "POPULATION").unwrap_err(); + assert!(err2.contains("collision")); + } + + #[test] + fn zonal_stats_partial_feature_failures_regression() { + use rustgis_data::raster::{ColorHint, Crs, GeoTransform, Raster}; + + let (w, h) = (4u32, 4u32); + let data: Vec = (1..=16).map(|v| v as f32).collect(); + let gt = GeoTransform { origin_x: 0.0, origin_y: 4.0, pixel_width: 1.0, pixel_height: -1.0 }; + let raster = Raster::new("elev", w, h, vec![data], gt, Crs::LonLat, None, ColorHint::Gray, "Float32").unwrap(); + + let fields = vec![Field::new("zone_id", FieldType::Integer)]; + let mut ds = Dataset::new("mixed_zones", GeometryType::Polygon, fields); + + let sq_ring = |x0: f64, y0: f64, x1: f64, y1: f64| { + vec![Coord::new(x0, y0), Coord::new(x1, y0), Coord::new(x1, y1), Coord::new(x0, y1), Coord::new(x0, y0)] + }; + + // Feature 1: Valid polygon with hole (outer square 0..3, inner hole 1..2) + let f1_geom = Geometry::Polygon(vec![vec![ + sq_ring(0.0, 1.0, 3.0, 4.0), // outer 3x3 = 9 cells + sq_ring(1.0, 2.0, 2.0, 3.0), // hole 1x1 = 1 cell + ]]); + ds.features.push(Feature { id: 101, geometry: Some(f1_geom), attrs: vec![Value::Int(1)] }); + + // Feature 2: Defective polygon: hole has fewer than 3 vertices! + let f2_geom = Geometry::Polygon(vec![vec![ + sq_ring(0.0, 1.0, 3.0, 4.0), + vec![Coord::new(1.0, 2.0), Coord::new(2.0, 3.0)], // only 2 points! + ]]); + ds.features.push(Feature { id: 102, geometry: Some(f2_geom), attrs: vec![Value::Int(2)] }); + + // Feature 3: Valid simple polygon + let f3_geom = Geometry::Polygon(vec![vec![sq_ring(0.0, 3.0, 1.0, 4.0)]]); // 1 cell + ds.features.push(Feature { id: 103, geometry: Some(f3_geom), attrs: vec![Value::Int(3)] }); + + let m = RasterMap { + datasets: HashMap::from([("mixed_zones".to_string(), ds)]), + rasters: HashMap::from([("elev".to_string(), &raster)]), + }; + + let res = crate::run("zonalStats", &m, &serde_json::json!({"raster": "elev", "polygons": "mixed_zones"})).unwrap(); + let messages = res.messages.clone(); + let out = layer(res); + + assert_eq!(out.features.len(), 3); + let count_idx = out.field_index("elev_count").unwrap(); + let mean_idx = out.field_index("elev_mean").unwrap(); + + // Feature 1 succeeded (9 - 1 = 8 cells) + assert_eq!(out.value(0, count_idx), &Value::Int(8)); + assert!(out.value(0, mean_idx).as_f64().is_some()); + + // Feature 2 failed (hole was defective -> polygon rejected -> default stats) + assert_eq!(out.value(1, count_idx), &Value::Int(0)); + assert_eq!(out.value(1, mean_idx), &Value::Null); + + // Feature 3 succeeded (1 cell) + assert_eq!(out.value(2, count_idx), &Value::Int(1)); + assert!(out.value(2, mean_idx).as_f64().is_some()); + + // Messages must explicitly report feature 102 failure + assert!(messages.iter().any(|m| m.contains("1 feature(s) had invalid geometry"))); + assert!(messages.iter().any(|m| m.contains("feature 102") && m.contains("fewer than 3 vertices"))); + } + + #[test] + fn parse_bands_and_stats_validation_regression() { + use rustgis_data::raster::{ColorHint, Crs, GeoTransform, Raster}; + + let (w, h) = (2u32, 2u32); + let data: Vec = vec![1.0, 2.0, 3.0, 4.0]; + let gt = GeoTransform { origin_x: 0.0, origin_y: 2.0, pixel_width: 1.0, pixel_height: -1.0 }; + let raster = Raster::new("elev", w, h, vec![data], gt, Crs::LonLat, None, ColorHint::Gray, "Float32").unwrap(); + + let mut ds = Dataset::new("zones", GeometryType::Polygon, vec![Field::new("id", FieldType::Integer)]); + let sq_ring = vec![Coord::new(0.0, 0.0), Coord::new(2.0, 0.0), Coord::new(2.0, 2.0), Coord::new(0.0, 2.0), Coord::new(0.0, 0.0)]; + ds.push(Some(Geometry::Polygon(vec![vec![sq_ring]])), vec![Value::Int(1)]); + + let m = RasterMap { datasets: HashMap::from([("zones".to_string(), ds)]), rasters: HashMap::from([("elev".to_string(), &raster)]) }; + + // 1. Invalid stats inputs + assert!(crate::run("zonalStats", &m, &serde_json::json!({"raster": "elev", "polygons": "zones", "stats": []})).is_err()); + assert!(crate::run("zonalStats", &m, &serde_json::json!({"raster": "elev", "polygons": "zones", "stats": ""})).is_err()); + assert!(crate::run("zonalStats", &m, &serde_json::json!({"raster": "elev", "polygons": "zones", "stats": ["median"]})).is_err()); + assert!(crate::run("zonalStats", &m, &serde_json::json!({"raster": "elev", "polygons": "zones", "stats": [123]})).is_err()); + + // 2. Invalid bands inputs + assert!(crate::run("zonalStats", &m, &serde_json::json!({"raster": "elev", "polygons": "zones", "bands": []})).is_err()); + assert!(crate::run("zonalStats", &m, &serde_json::json!({"raster": "elev", "polygons": "zones", "bands": ""})).is_err()); + assert!(crate::run("zonalStats", &m, &serde_json::json!({"raster": "elev", "polygons": "zones", "bands": 1.5})).is_err()); + assert!(crate::run("zonalStats", &m, &serde_json::json!({"raster": "elev", "polygons": "zones", "bands": 99})).is_err()); + assert!(crate::run("zonalStats", &m, &serde_json::json!({"raster": "elev", "polygons": "zones", "bands": -1})).is_err()); + } } diff --git a/crates/engine/src/cmd/imagery.rs b/crates/engine/src/cmd/imagery.rs index 73de8edc8..4fb18210b 100644 --- a/crates/engine/src/cmd/imagery.rs +++ b/crates/engine/src/cmd/imagery.rs @@ -519,6 +519,28 @@ fn index(s: &mut Session, p: &Value) -> CmdResult { add(s, rasterops::derive(&r, &name(p, format!("{}_{idx}", r.name)), vec![out])?, false) } +/// Processes a raster via clipping or resampling. +/// +/// ### Polygon Clipping Semantics (`operation: "clip"`, `"polygon": ""`) +/// - **Selection Propagation (`selectedOnly`)**: +/// - When `Some(true)`: Clips strictly using features currently selected in the polygon layer. +/// If the selection is empty, returns an actionable error (`": no features are selected"`). +/// - When `Some(false)`: Clips using all features in the polygon layer, ignoring any selection. +/// - When omitted (`None` or `null`): For backward compatibility, clips to the selection if non-empty, +/// otherwise clips using all features. +/// - Non-boolean values are rejected with an explicit validation error. +/// - **Per-Zone and Overlap Semantics**: +/// - Every polygon geometry (including multipart geometries) is transformed to WGS 84 lon/lat. +/// - Polygon exterior rings and interior holes are strictly validated; invalid or failed holes reject +/// the containing polygon rather than distorting the boundary. +/// - Polygons are combined into a composite scanline mask over the raster. Holes cut out cells using the +/// even-odd rule within each polygon. Where multiple polygons overlap, their coverage is combined +/// (union: any cell falling inside at least one polygon is retained). +/// - The raster is cropped to the tight bounding box of all selected polygons, and cells outside the +/// polygons receive NoData. +/// - **Partial Failure Reporting**: +/// - Features with invalid geometry or unprojectable coordinates are skipped and logged with a warning. +/// - If all features fail, the operation fails and reports the specific failure cause of the first feature. fn process(s: &mut Session, p: &Value) -> CmdResult { let r = raster(s, p)?; match ps(p, "operation").ok_or("operation is required")? { @@ -531,20 +553,20 @@ fn process(s: &mut Session, p: &Value) -> CmdResult { .layer_by_name(pl) .filter(|l| l.geometry_type() == GeometryType::Polygon) .ok_or_else(|| format!("no polygon layer '{pl}'"))?; - let rows: Vec = match b(p, "selectedOnly") { - Some(true) => { + let rows: Vec = match p.get("selectedOnly") { + Some(Value::Bool(true)) => { if l.selection.is_empty() { return Err(format!("{pl}: no features are selected")); } l.selection.clone() } - Some(false) => (0..l.dataset.features.len()).collect(), - None => { - if l.selection.is_empty() { - (0..l.dataset.features.len()).collect() - } else { - l.selection.clone() - } + Some(Value::Bool(false)) => (0..l.dataset.features.len()).collect(), + None | Some(Value::Null) => { + // Backward-compatible default: if features are selected, clip to selection; otherwise clip all. + if l.selection.is_empty() { (0..l.dataset.features.len()).collect() } else { l.selection.clone() } + } + Some(_) => { + return Err("parameter 'selectedOnly' must be a boolean (true or false)".into()); } }; if rows.is_empty() { @@ -556,67 +578,34 @@ fn process(s: &mut Session, p: &Value) -> CmdResult { } _ => None, }; - let polygons: Vec>> = rows - .iter() - .filter_map(|row| match l.dataset.features.get(*row).and_then(|f| f.geometry.as_ref()) { - Some(Geometry::Polygon(polys)) => { - let rings: Vec>> = match &poly_tr { - Some(tr) => polys - .iter() - .filter_map(|rings| { - let exterior = rings.first()?; - let mut ext_pts = Vec::with_capacity(exterior.len()); - for &c in exterior { - ext_pts.push(tr.to_lonlat(c)?); - } - if ext_pts.len() < 3 { - return None; - } - let mut valid_rings = vec![ext_pts]; - for hole in rings.iter().skip(1) { - let mut hole_pts = Vec::with_capacity(hole.len()); - let mut ok = true; - for &c in hole { - if let Some(ll) = tr.to_lonlat(c) { - hole_pts.push(ll); - } else { - ok = false; - break; - } - } - if ok && hole_pts.len() >= 3 { - valid_rings.push(hole_pts); - } - } - Some(valid_rings) - }) - .collect(), - None => polys - .iter() - .filter_map(|rings| { - let exterior = rings.first()?; - if exterior.len() < 3 { - return None; - } - let mut valid_rings = vec![exterior.clone()]; - for hole in rings.iter().skip(1) { - if hole.len() >= 3 { - valid_rings.push(hole.clone()); - } - } - Some(valid_rings) - }) - .collect(), - }; - Some(rings.into_iter()) + let mut polygons: Vec>> = Vec::new(); + let mut failed_features = Vec::new(); + for &row in &rows { + let Some(f) = l.dataset.features.get(row) else { continue }; + match &f.geometry { + Some(Geometry::Polygon(polys)) => match rasterops::transform_feature_polygons_to_lonlat(polys, poly_tr.as_ref()) { + Ok(transformed) => { + polygons.extend(transformed); + } + Err(e) => { + failed_features.push((f.id, e)); + } + }, + _ => { + failed_features + .push((f.id, rasterops::PolygonTransformError::InvalidExterior("missing or non-polygon geometry".into()))); } - _ => None, - }) - .flatten() - .collect(); + } + } if polygons.is_empty() { + if let Some((first_id, first_err)) = failed_features.first() { + return Err(format!("{pl}: has no valid polygon geometries (feature {first_id}: {first_err})")); + } return Err(format!("{pl}: has no valid polygon geometries")); } + if !failed_features.is_empty() { + log::warn!("{pl}: {} feature(s) failed polygon transformation during clipping", failed_features.len()); + } let (cols, rows, mask) = rasterops::polygon_mask(&r, &polygons).ok_or_else(|| format!("no cell of {} falls inside {pl}", r.name))?; let out = clip_window(&r, cols, rows, Some(&mask), name(p, format!("{}_clip", r.name)))?; diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 1a67925bf..dc4f6b5da 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -14988,6 +14988,185 @@ fn gp_zonal_stats_end_to_end_and_undo() { assert!(s.run("gp.zonalStats", &json!({"raster": "dem", "polygons": "districts", "bands": [1.5]})).is_err()); } +#[test] +fn raster_clip_polygon_selection_semantics_and_partial_failure() { + use rustgis_data::raster::{ColorHint, Crs, GeoTransform, Raster}; + use rustgis_data::{Dataset, Field, FieldType, Geometry, GeometryType, Value}; + use rustgis_geom::Coord; + + let mut s = world(); + + // 10x10 raster: origin (0.0, 10.0), cell size (1.0, -1.0) + let (w, h) = (10u32, 10u32); + let mut data = Vec::with_capacity((w * h) as usize); + for row in 0..h { + for col in 0..w { + data.push((col * 10 + row) as f32); + } + } + let gt = GeoTransform { origin_x: 0.0, origin_y: 10.0, pixel_width: 1.0, pixel_height: -1.0 }; + let raster = Raster::new("dem", w, h, vec![data], gt, Crs::LonLat, None, ColorHint::Gray, "Float32").unwrap(); + let raster_layer = rustgis_map::RasterLayer::from_path(raster, "dem.tif"); + s.map_mut().add_raster("dem", raster_layer); + + let sq = |x0: f64, y0: f64, x1: f64, y1: f64| { + Geometry::Polygon(vec![vec![vec![ + Coord::new(x0, y0), + Coord::new(x1, y0), + Coord::new(x1, y1), + Coord::new(x0, y1), + Coord::new(x0, y0), + ]]]) + }; + + let mut poly_ds = Dataset::new("clip_zones", GeometryType::Polygon, vec![Field::new("name", FieldType::Text)]); + // Feature 0: [1.0, 7.0] to [3.0, 9.0] + poly_ds.push(Some(sq(1.0, 7.0, 3.0, 9.0)), vec![Value::Text("Z1".into())]); + // Feature 1: [2.0, 6.0] to [4.0, 8.0] (overlaps with Z1) + poly_ds.push(Some(sq(2.0, 6.0, 4.0, 8.0)), vec![Value::Text("Z2".into())]); + // Feature 2: [6.0, 1.0] to [8.0, 3.0] (separate area) + poly_ds.push(Some(sq(6.0, 1.0, 8.0, 3.0)), vec![Value::Text("Z3".into())]); + // Feature 3: Degenerate hole (< 3 coords) -> should trigger rejection of this feature + let bad_hole_poly = Geometry::Polygon(vec![vec![ + vec![Coord::new(4.0, 4.0), Coord::new(6.0, 4.0), Coord::new(6.0, 6.0), Coord::new(4.0, 6.0), Coord::new(4.0, 4.0)], + vec![Coord::new(4.5, 4.5), Coord::new(5.0, 5.0)], // invalid hole with only 2 points + ]]); + poly_ds.push(Some(bad_hole_poly), vec![Value::Text("Z4_bad".into())]); + + let poly_layer_id = s.map_mut().add_layer(rustgis_map::Layer::new(0, poly_ds)); + + // Test 1: selectedOnly: "invalid" or non-boolean returns validation error + let err_type = s.run( + "raster.process", + &json!({ + "layer": "dem", + "operation": "clip", + "polygon": "clip_zones", + "selectedOnly": "yes", + }), + ); + assert!(err_type.is_err()); + assert!(err_type.unwrap_err().contains("must be a boolean")); + + // Test 2: selectedOnly: true with empty selection returns error + let err_empty_sel = s.run( + "raster.process", + &json!({ + "layer": "dem", + "operation": "clip", + "polygon": "clip_zones", + "selectedOnly": true, + }), + ); + assert!(err_empty_sel.is_err()); + assert!(err_empty_sel.unwrap_err().contains("no features are selected")); + + // Test 3: selectedOnly: true with Feature 0 selected + { + let l = s.map_mut().layer_mut(poly_layer_id).unwrap(); + l.selection = vec![0]; + } + let res_sel = s.run( + "raster.process", + &json!({ + "layer": "dem", + "operation": "clip", + "polygon": "clip_zones", + "selectedOnly": true, + "name": "dem_sel_0", + }), + ); + assert!(res_sel.is_ok(), "clip selected failed: {res_sel:?}"); + let sel_layer = s.map().layer_by_name("dem_sel_0").expect("dem_sel_0 exists"); + let sel_raster = sel_layer.raster.as_ref().unwrap().data.as_ref().unwrap(); + // Feature 0 covers x: [1.0, 3.0], y: [7.0, 9.0] -> 2 cols x 2 rows + assert_eq!(sel_raster.width, 2); + assert_eq!(sel_raster.height, 2); + + // Test 4: Overlapping polygons combined coverage (Features 0 and 1) + { + let l = s.map_mut().layer_mut(poly_layer_id).unwrap(); + l.selection = vec![0, 1]; + } + let res_overlap = s.run( + "raster.process", + &json!({ + "layer": "dem", + "operation": "clip", + "polygon": "clip_zones", + "selectedOnly": true, + "name": "dem_overlap", + }), + ); + assert!(res_overlap.is_ok(), "clip overlap failed: {res_overlap:?}"); + let ov_layer = s.map().layer_by_name("dem_overlap").expect("dem_overlap exists"); + let ov_raster = ov_layer.raster.as_ref().unwrap().data.as_ref().unwrap(); + // Features 0 & 1 union bounding box: x: [1.0, 4.0], y: [6.0, 9.0] -> 3 cols x 3 rows + assert_eq!(ov_raster.width, 3); + assert_eq!(ov_raster.height, 3); + + // Test 5: selectedOnly: false ignores active selection and clips using all valid features + let res_all = s.run( + "raster.process", + &json!({ + "layer": "dem", + "operation": "clip", + "polygon": "clip_zones", + "selectedOnly": false, + "name": "dem_all", + }), + ); + assert!(res_all.is_ok(), "clip all failed: {res_all:?}"); + let all_layer = s.map().layer_by_name("dem_all").expect("dem_all exists"); + let all_raster = all_layer.raster.as_ref().unwrap().data.as_ref().unwrap(); + // Features 0, 1, 2 bounding box spans x: [1.0, 8.0], y: [1.0, 9.0] -> 7 cols x 8 rows + assert_eq!(all_raster.width, 7); + assert_eq!(all_raster.height, 8); + + // Test 6: Partial failure reporting: Feature 0 (valid) + Feature 3 (invalid hole) + // Feature 3 should be skipped with a warning, Feature 0 should succeed + { + let l = s.map_mut().layer_mut(poly_layer_id).unwrap(); + l.selection = vec![0, 3]; + } + let res_partial = s.run( + "raster.process", + &json!({ + "layer": "dem", + "operation": "clip", + "polygon": "clip_zones", + "selectedOnly": true, + "name": "dem_partial", + }), + ); + assert!(res_partial.is_ok(), "clip partial should succeed for valid feature: {res_partial:?}"); + let partial_layer = s.map().layer_by_name("dem_partial").expect("dem_partial exists"); + let partial_raster = partial_layer.raster.as_ref().unwrap().data.as_ref().unwrap(); + assert_eq!(partial_raster.width, 2); + assert_eq!(partial_raster.height, 2); + + // Test 7: Total failure when all selected features fail validation (only Feature 3 selected) + { + let l = s.map_mut().layer_mut(poly_layer_id).unwrap(); + l.selection = vec![3]; + } + let res_bad = s.run( + "raster.process", + &json!({ + "layer": "dem", + "operation": "clip", + "polygon": "clip_zones", + "selectedOnly": true, + "name": "dem_bad", + }), + ); + assert!(res_bad.is_err()); + let err_msg = res_bad.unwrap_err(); + assert!(err_msg.contains("has no valid polygon geometries")); + assert!(err_msg.contains("feature 4")); + assert!(err_msg.contains("hole 1 is invalid")); +} + #[test] fn layer_selection_replaces_toggles_and_extends() { let mut s = world(); diff --git a/crates/ui-egui/src/analysispane.rs b/crates/ui-egui/src/analysispane.rs index 770116aa8..40f50dbde 100644 --- a/crates/ui-egui/src/analysispane.rs +++ b/crates/ui-egui/src/analysispane.rs @@ -282,20 +282,24 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { ); }); let band_str = app.workflow.zonal_stats.bands.trim(); - let band_valid = if band_str.is_empty() || band_str.eq_ignore_ascii_case("all") { - true - } else { - band_str.split([',', ';', ' ']).all(|part| { - let p = part.trim().trim_matches(|c| c == '[' || c == ']' || c == '"'); - p.is_empty() || p.parse::().is_ok_and(|b| b >= 1) - }) - }; + let total_bands = app + .session + .map() + .layer_by_name(&app.workflow.layer) + .and_then(|l| l.raster.as_ref()) + .and_then(|r| r.data.as_ref()) + .map(|d| d.bands.len()) + .unwrap_or(1); + let band_res = + if band_str.is_empty() { Ok(vec![0]) } else { rustgis_engine::analysis::rasterops::parse_bands_str(band_str, total_bands) }; + let band_valid = band_res.is_ok(); + let band_err = band_res.err(); let layers_ready = !app.workflow.layer.is_empty() && !app.workflow.layer2.is_empty(); let can_run = layers_ready && band_valid; if !layers_ready { ui.label(egui::RichText::new(tl!("Add a raster and a polygon layer to calculate zonal statistics.")).small().weak()); - } else if !band_valid { - ui.label(egui::RichText::new(tl!("Band must be a positive integer, list of bands, or 'all'.")).small().color(t.amber)); + } else if let Some(err) = band_err { + ui.label(egui::RichText::new(err).small().color(t.amber)); } if ui.add_enabled(can_run, egui::Button::new(tl!("Calculate Statistics"))).clicked() { let mut params = json!({ @@ -686,14 +690,12 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { } if ui.add_enabled(can_clip_poly, egui::Button::new(tl!("Clip To Polygon"))).clicked() { - let mut p = json!({ + let p = json!({ "layer": app.workflow.layer, "operation": "clip", "polygon": app.workflow.layer2, + "selectedOnly": app.workflow.selected_only, }); - if app.workflow.selected_only { - p["selectedOnly"] = json!(true); - } run(app, "raster.process", p); } } else { From 6e531cc2ebe77d3a2e67d4d7551ad64713cd8574 Mon Sep 17 00:00:00 2001 From: quanguet0409 Date: Sat, 24 Oct 2026 21:48:38 +0700 Subject: [PATCH 06/11] fix(ui): address egui 0.37 clippy warnings on deprecated id and unused close --- crates/ui-egui/src/analysispane.rs | 1 + crates/ui-egui/src/scriptcomplete.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/crates/ui-egui/src/analysispane.rs b/crates/ui-egui/src/analysispane.rs index 40f50dbde..7102d2476 100644 --- a/crates/ui-egui/src/analysispane.rs +++ b/crates/ui-egui/src/analysispane.rs @@ -1207,6 +1207,7 @@ fn script(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens, fit: bool) { let out_h = script_out_height(ui); // One id whether the pane scrolls as a whole or not, so resizing it across that keeps the // box's focus, cursor and selection. + #[allow(deprecated)] let box_id = egui::Id::new("script_text"); // The suggestion list's keys (#662), taken before the box sees them. if let Some((offer, item)) = crate::scriptcomplete::keys(ui, box_id, &app.workflow.script) { diff --git a/crates/ui-egui/src/scriptcomplete.rs b/crates/ui-egui/src/scriptcomplete.rs index 84f485c8b..7d551da27 100644 --- a/crates/ui-egui/src/scriptcomplete.rs +++ b/crates/ui-egui/src/scriptcomplete.rs @@ -889,6 +889,7 @@ pub fn filter(ui: &Ui, box_id: Id) -> egui::EventFilter { } /// Closes the list as Escape does: not shown again until what's typed changes. +#[allow(dead_code)] pub fn close(ui: &Ui, box_id: Id) { let mut list = load(ui, box_id); if let Some((offer, _)) = list.open.take() { From 70450a4690cac7db97ae312c60e3e267edafc181 Mon Sep 17 00:00:00 2001 From: quanguet0409 Date: Sat, 24 Oct 2026 21:58:15 +0700 Subject: [PATCH 07/11] fix(ui): add accessible labels for zonal stats controls and restore Ctrl+Enter script shortcut --- crates/ui-egui/src/analysispane.rs | 63 ++++++++++++++++++++-------- crates/ui-egui/src/scriptcomplete.rs | 1 - 2 files changed, 46 insertions(+), 18 deletions(-) diff --git a/crates/ui-egui/src/analysispane.rs b/crates/ui-egui/src/analysispane.rs index 7102d2476..1dabae58e 100644 --- a/crates/ui-egui/src/analysispane.rs +++ b/crates/ui-egui/src/analysispane.rs @@ -87,12 +87,16 @@ fn heading(ui: &mut Ui, text: &str) { ui.add_space(2.0); } -fn combo(ui: &mut Ui, id: &str, value: &mut String, options: &[String]) { - egui::ComboBox::from_id_salt(id).selected_text(value.as_str()).width(180.0).show_ui(ui, |ui| { - for o in options { - ui.selectable_value(value, o.clone(), o.as_str()); - } - }); +fn combo(ui: &mut Ui, id: &str, value: &mut String, options: &[String]) -> egui::Response { + egui::ComboBox::from_id_salt(id) + .selected_text(value.as_str()) + .width(180.0) + .show_ui(ui, |ui| { + for o in options { + ui.selectable_value(value, o.clone(), o.as_str()); + } + }) + .response } fn run(app: &mut RustGisApp, id: &str, p: Value) { @@ -263,23 +267,26 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { } "zonalStats" => { heading(ui, "Calculate raster statistics per polygon"); - ui.label(egui::RichText::new(tl!("Raster layer")).small().weak()); - combo(ui, "wf_zs_raster", &mut app.workflow.layer, &rasters); - ui.label(egui::RichText::new(tl!("Polygon layer")).small().weak()); - combo(ui, "wf_zs_poly", &mut app.workflow.layer2, &polygons); + let raster_label = ui.label(egui::RichText::new(tl!("Raster layer")).small().weak()); + combo(ui, "wf_zs_raster", &mut app.workflow.layer, &rasters).labelled_by(raster_label.id); + let polygon_label = ui.label(egui::RichText::new(tl!("Polygon layer")).small().weak()); + combo(ui, "wf_zs_poly", &mut app.workflow.layer2, &polygons).labelled_by(polygon_label.id); ui.horizontal(|ui| { - ui.label(tl!("Band")); - ui.add(egui::TextEdit::singleline(&mut app.workflow.zonal_stats.bands).hint_text("1").desired_width(40.0)); - ui.label(tl!("Prefix")); - ui.add(egui::TextEdit::singleline(&mut app.workflow.zonal_stats.prefix).hint_text("elev").desired_width(70.0)); + let band_label = ui.label(tl!("Band")); + ui.add(egui::TextEdit::singleline(&mut app.workflow.zonal_stats.bands).hint_text("1").desired_width(40.0)) + .labelled_by(band_label.id); + let prefix_label = ui.label(tl!("Prefix")); + ui.add(egui::TextEdit::singleline(&mut app.workflow.zonal_stats.prefix).hint_text("elev").desired_width(70.0)) + .labelled_by(prefix_label.id); }); ui.horizontal(|ui| { - ui.label(tl!("Output")); + let output_label = ui.label(tl!("Output")); ui.add( egui::TextEdit::singleline(&mut app.workflow.zonal_stats.output) .hint_text(tl!("output layer name")) .desired_width(120.0), - ); + ) + .labelled_by(output_label.id); }); let band_str = app.workflow.zonal_stats.bands.trim(); let total_bands = app @@ -1209,6 +1216,26 @@ fn script(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens, fit: bool) { // box's focus, cursor and selection. #[allow(deprecated)] let box_id = egui::Id::new("script_text"); + // Ctrl+Enter (⌘+Enter) runs the script (#773), taken before the box sees it, so it runs rather + // than breaking the line; once a press, not again for each repeat of a key held down. The box + // keeps the focus, so the list closes, as Escape closes it. + let ctrl_enter = ui.memory(|m| m.has_focus(box_id)) + && ui.input_mut(|i| { + let mut run = false; + i.events.retain(|e| match e { + egui::Event::Key { key: egui::Key::Enter, pressed: true, repeat, modifiers, .. } + if modifiers.matches_logically(egui::Modifiers::COMMAND) => + { + run |= !repeat; + false + } + _ => true, + }); + run + }); + if ctrl_enter { + crate::scriptcomplete::close(ui, box_id); + } // The suggestion list's keys (#662), taken before the box sees them. if let Some((offer, item)) = crate::scriptcomplete::keys(ui, box_id, &app.workflow.script) { crate::scriptcomplete::accept(ui, box_id, &mut app.workflow.script, &offer, &item, &app.session); @@ -1235,7 +1262,9 @@ fn script(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens, fit: bool) { crate::scriptcomplete::braces(ui, before, &out.inner, &mut app.workflow.script); } ui.horizontal(|ui| { - if ui.button(tl!("Run")).clicked() { + let shortcut = if cfg!(target_os = "macos") { "⌘+Enter" } else { "Ctrl+Enter" }; + let run = ui.button(tl!("Run")).on_hover_text(crate::i18n::fmt("Run the script ({keys})", &[("keys", shortcut)])).clicked(); + if run || ctrl_enter { let script = app.workflow.script.clone(); let out = app.run("script.run", json!({"script": script})); app.workflow.script_out = match out { diff --git a/crates/ui-egui/src/scriptcomplete.rs b/crates/ui-egui/src/scriptcomplete.rs index 7d551da27..84f485c8b 100644 --- a/crates/ui-egui/src/scriptcomplete.rs +++ b/crates/ui-egui/src/scriptcomplete.rs @@ -889,7 +889,6 @@ pub fn filter(ui: &Ui, box_id: Id) -> egui::EventFilter { } /// Closes the list as Escape does: not shown again until what's typed changes. -#[allow(dead_code)] pub fn close(ui: &Ui, box_id: Id) { let mut list = load(ui, box_id); if let Some((offer, _)) = list.open.take() { From 09d2fd7ab6b6b5ba8f4d8301cff2cecdd6f0ddda Mon Sep 17 00:00:00 2001 From: quanguet0409 Date: Sat, 24 Oct 2026 22:10:06 +0700 Subject: [PATCH 08/11] fix(ui): preserve selection-aware clipping default and associate zonal stats labels --- crates/ui-egui/src/analysispane.rs | 303 +++++++++++++++++++---------- 1 file changed, 196 insertions(+), 107 deletions(-) diff --git a/crates/ui-egui/src/analysispane.rs b/crates/ui-egui/src/analysispane.rs index 1dabae58e..c1e9fda5f 100644 --- a/crates/ui-egui/src/analysispane.rs +++ b/crates/ui-egui/src/analysispane.rs @@ -33,9 +33,11 @@ pub struct Workflow { pub error: Option, pub script: String, pub script_out: Vec<(String, bool)>, + /// The Script pane's last run, shown above its results. + pub script_last: Option, pub trend: TrendForm, + pub selected_only: Option, pub zonal_stats: ZonalStatsForm, - pub selected_only: bool, } /// The Zonal Statistics section's inputs. @@ -46,6 +48,15 @@ pub struct ZonalStatsForm { pub output: String, } +/// A Script pane run, as the line above its results tells it: its number this session (so a run +/// made again by mistake shows), how long it took and when it started (seconds since 1970; 0 for +/// a clock that can't tell). +pub struct ScriptRun { + pub n: u32, + pub secs: f64, + pub at: u64, +} + /// The Trend Analysis section's inputs (#579), and a run under way. #[derive(Default)] pub struct TrendForm { @@ -87,7 +98,7 @@ fn heading(ui: &mut Ui, text: &str) { ui.add_space(2.0); } -fn combo(ui: &mut Ui, id: &str, value: &mut String, options: &[String]) -> egui::Response { +fn combo(ui: &mut Ui, id: &str, name: &str, value: &mut String, options: &[String]) { egui::ComboBox::from_id_salt(id) .selected_text(value.as_str()) .width(180.0) @@ -97,6 +108,7 @@ fn combo(ui: &mut Ui, id: &str, value: &mut String, options: &[String]) -> egui: } }) .response + .on_hover_text(tl!(name)); } fn run(app: &mut RustGisApp, id: &str, p: Value) { @@ -181,7 +193,9 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { app.workflow.pick = None; } } - }); + }) + .response + .on_hover_text(tl!("Workflow")); let map = app.session.map(); let rasters: Vec = map.layers.iter().filter(|l| l.is_raster()).map(|l| l.name.clone()).collect(); let features: Vec = map.layers.iter().filter(|l| !l.is_raster() && !l.graphics).map(|l| l.name.clone()).collect(); @@ -221,14 +235,14 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { match app.workflow.section { "rasterFunctions" | "functionEditor" => { heading(ui, if app.workflow.section == "rasterFunctions" { "Apply a function to a raster" } else { "Chain functions" }); - combo(ui, "wf_r", &mut app.workflow.layer, &rasters); + combo(ui, "wf_r", "Layer", &mut app.workflow.layer, &rasters); let funcs: Vec = ["slope", "aspect", "hillshade", "ndvi", "bandMath", "threshold"].iter().map(|s| s.to_string()).collect(); if app.workflow.function.is_empty() { app.workflow.function = "hillshade".into(); } ui.horizontal(|ui| { - combo(ui, "wf_f", &mut app.workflow.function, &funcs); + combo(ui, "wf_f", "Function", &mut app.workflow.function, &funcs); match app.workflow.function.as_str() { "bandMath" => { ui.add(egui::TextEdit::singleline(&mut app.workflow.text).hint_text("(b4-b3)/(b4+b3)").desired_width(120.0)); @@ -265,12 +279,49 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { } } } + "suitability" => { + heading(ui, "Score two rasters 1–10 and weight them"); + ui.label(egui::RichText::new(tl!("High values of the first raster and low values of the second score best.")).small().weak()); + combo(ui, "wf_s1", "First raster", &mut app.workflow.layer, &rasters); + combo(ui, "wf_s2", "Second raster", &mut app.workflow.layer2, &rasters); + ui.horizontal(|ui| { + let input_label = ui.label(tl!("Weights")); + ui.add(egui::TextEdit::singleline(&mut app.workflow.number).hint_text("1").desired_width(40.0)).labelled_by(input_label.id); + ui.add(egui::TextEdit::singleline(&mut app.workflow.number2).hint_text("1").desired_width(40.0)) + .labelled_by(input_label.id); + }); + if ui.button(tl!("Run")).clicked() { + let crit = |app: &RustGisApp, n: &str, high: bool, w: f64| -> Value { + let st = app + .session + .map() + .layer_by_name(n) + .and_then(|l| l.raster.as_ref()) + .and_then(|r| r.data.as_ref()) + .and_then(|d| d.bands.first()) + .map(|b| (b.stats.min, b.stats.max)) + .unwrap_or((0.0, 1.0)); + let step = (st.1 - st.0) / 10.0; + let ranges: Vec = (0..10) + .map(|i| { + let lo = st.0 + step * i as f64; + let hi = if i == 9 { f64::MAX } else { lo + step }; + json!([if i == 0 { f64::MIN } else { lo }, hi, if high { i + 1 } else { 10 - i }]) + }) + .collect(); + json!({"layer": n, "ranges": ranges, "weight": w}) + }; + let c1 = crit(app, &app.workflow.layer, true, num(&app.workflow.number, 1.0)); + let c2 = crit(app, &app.workflow.layer2, false, num(&app.workflow.number2, 1.0)); + run(app, "analysis.suitability", json!({"criteria": [c1, c2]})); + } + } "zonalStats" => { heading(ui, "Calculate raster statistics per polygon"); - let raster_label = ui.label(egui::RichText::new(tl!("Raster layer")).small().weak()); - combo(ui, "wf_zs_raster", &mut app.workflow.layer, &rasters).labelled_by(raster_label.id); - let polygon_label = ui.label(egui::RichText::new(tl!("Polygon layer")).small().weak()); - combo(ui, "wf_zs_poly", &mut app.workflow.layer2, &polygons).labelled_by(polygon_label.id); + ui.label(egui::RichText::new(tl!("Raster layer")).small().weak()); + combo(ui, "wf_zs_raster", "Raster layer", &mut app.workflow.layer, &rasters); + ui.label(egui::RichText::new(tl!("Polygon layer")).small().weak()); + combo(ui, "wf_zs_poly", "Polygon layer", &mut app.workflow.layer2, &polygons); ui.horizontal(|ui| { let band_label = ui.label(tl!("Band")); ui.add(egui::TextEdit::singleline(&mut app.workflow.zonal_stats.bands).hint_text("1").desired_width(40.0)) @@ -325,45 +376,9 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { run(app, "gp.zonalStats", params); } } - "suitability" => { - heading(ui, "Score two rasters 1–10 and weight them"); - ui.label(egui::RichText::new(tl!("High values of the first raster and low values of the second score best.")).small().weak()); - combo(ui, "wf_s1", &mut app.workflow.layer, &rasters); - combo(ui, "wf_s2", &mut app.workflow.layer2, &rasters); - ui.horizontal(|ui| { - ui.label(tl!("Weights")); - ui.add(egui::TextEdit::singleline(&mut app.workflow.number).hint_text("1").desired_width(40.0)); - ui.add(egui::TextEdit::singleline(&mut app.workflow.number2).hint_text("1").desired_width(40.0)); - }); - if ui.button(tl!("Run")).clicked() { - let crit = |app: &RustGisApp, n: &str, high: bool, w: f64| -> Value { - let st = app - .session - .map() - .layer_by_name(n) - .and_then(|l| l.raster.as_ref()) - .and_then(|r| r.data.as_ref()) - .and_then(|d| d.bands.first()) - .map(|b| (b.stats.min, b.stats.max)) - .unwrap_or((0.0, 1.0)); - let step = (st.1 - st.0) / 10.0; - let ranges: Vec = (0..10) - .map(|i| { - let lo = st.0 + step * i as f64; - let hi = if i == 9 { f64::MAX } else { lo + step }; - json!([if i == 0 { f64::MIN } else { lo }, hi, if high { i + 1 } else { 10 - i }]) - }) - .collect(); - json!({"layer": n, "ranges": ranges, "weight": w}) - }; - let c1 = crit(app, &app.workflow.layer, true, num(&app.workflow.number, 1.0)); - let c2 = crit(app, &app.workflow.layer2, false, num(&app.workflow.number2, 1.0)); - run(app, "analysis.suitability", json!({"criteria": [c1, c2]})); - } - } "visibility" => { heading(ui, "Viewshed from a point"); - combo(ui, "wf_v", &mut app.workflow.layer, &rasters); + combo(ui, "wf_v", "Layer", &mut app.workflow.layer, &rasters); ui.horizontal(|ui| { ui.add(egui::TextEdit::singleline(&mut app.workflow.number).hint_text(tl!("observer height m")).desired_width(110.0)); ui.add(egui::TextEdit::singleline(&mut app.workflow.number2).hint_text(tl!("radius m")).desired_width(80.0)); @@ -379,7 +394,7 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { } "lineOfSight" => { heading(ui, "Line of sight between two points"); - combo(ui, "wf_l", &mut app.workflow.layer, &rasters); + combo(ui, "wf_l", "Layer", &mut app.workflow.layer, &rasters); pick_button(app, ui, "los", 2, "Click the observer, then the target"); if let Some(p) = picked(app, "los") && let (Some(a), Some(b)) = (p.first(), p.get(1)) @@ -389,8 +404,8 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { } "interpolate" => { heading(ui, "Interpolate a surface from points (IDW)"); - combo(ui, "wf_i", &mut app.workflow.layer, &points); - combo(ui, "wf_if", &mut app.workflow.field, &numeric); + combo(ui, "wf_i", "Layer", &mut app.workflow.layer, &points); + combo(ui, "wf_if", "Field", &mut app.workflow.field, &numeric); ui.horizontal(|ui| { ui.add(egui::TextEdit::singleline(&mut app.workflow.number).hint_text(tl!("cell size °")).desired_width(80.0)); ui.add(egui::TextEdit::singleline(&mut app.workflow.number2).hint_text(tl!("power 2")).desired_width(60.0)); @@ -405,13 +420,13 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { } "network" => { heading(ui, "Shortest route over a line network"); - combo(ui, "wf_n", &mut app.workflow.layer, &lines); + combo(ui, "wf_n", "Layer", &mut app.workflow.layer, &lines); let cost_fields = fields_of(&app.workflow.layer, true); ui.horizontal(|ui| { ui.label(tl!("Cost")); let mut options = vec![String::new()]; options.extend(cost_fields); - combo(ui, "wf_nc", &mut app.workflow.field, &options); + combo(ui, "wf_nc", "Cost", &mut app.workflow.field, &options); }); pick_button(app, ui, "route", 2, "Click the start, then the end"); if let Some(p) = picked(app, "route") @@ -438,7 +453,7 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { } "simulation" => { heading(ui, "Random points in polygons"); - combo(ui, "wf_sim", &mut app.workflow.layer, &polygons); + combo(ui, "wf_sim", "Layer", &mut app.workflow.layer, &polygons); ui.horizontal(|ui| { ui.add(egui::TextEdit::singleline(&mut app.workflow.number).hint_text(tl!("points each")).desired_width(80.0)); ui.add(egui::TextEdit::singleline(&mut app.workflow.number2).hint_text(tl!("seed")).desired_width(60.0)); @@ -453,7 +468,7 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { } "profile" | "explore" => { heading(ui, if app.workflow.section == "profile" { "Profile every field" } else { "Histograms and correlations" }); - combo(ui, "wf_p", &mut app.workflow.layer, &features); + combo(ui, "wf_p", "Layer", &mut app.workflow.layer, &features); let cmd = if app.workflow.section == "profile" { "data.profile" } else { "data.explore" }; if ui.button(tl!("Run")).clicked() { run(app, cmd, json!({"layer": app.workflow.layer})); @@ -461,19 +476,19 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { } "similar" => { heading(ui, "Features most like the selected one"); - combo(ui, "wf_e", &mut app.workflow.layer, &features); + combo(ui, "wf_e", "Layer", &mut app.workflow.layer, &features); if ui.button(tl!("Find Similar")).clicked() { run(app, "data.similar", json!({"layer": app.workflow.layer, "count": 10})); } } "interop" => { heading(ui, "Convert a layer to another format"); - combo(ui, "wf_x", &mut app.workflow.layer, &features); + combo(ui, "wf_x", "Layer", &mut app.workflow.layer, &features); let formats: Vec = ["kml", "gpx", "wkt", "geojson", "csv"].iter().map(|s| s.to_string()).collect(); if app.workflow.format.is_empty() { app.workflow.format = "kml".into(); } - combo(ui, "wf_xf", &mut app.workflow.format, &formats); + combo(ui, "wf_xf", "Format", &mut app.workflow.format, &formats); if ui.button(tl!("Convert…")).clicked() { match app.session.run("data.convert", &json!({"layer": app.workflow.layer, "format": app.workflow.format})) { Ok(v) => { @@ -516,7 +531,7 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { } "georeference" => { heading(ui, "Place a raster on the map"); - combo(ui, "wf_g", &mut app.workflow.layer, &rasters); + combo(ui, "wf_g", "Layer", &mut app.workflow.layer, &rasters); ui.label( egui::RichText::new(tl!("Stretch it over the map view, or give control points (one per line: column row lon lat).")) .small() @@ -564,13 +579,13 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { } "change" => { heading(ui, "Compare two rasters"); - combo(ui, "wf_c1", &mut app.workflow.layer, &rasters); - combo(ui, "wf_c2", &mut app.workflow.layer2, &rasters); + combo(ui, "wf_c1", "Before", &mut app.workflow.layer, &rasters); + combo(ui, "wf_c2", "After", &mut app.workflow.layer2, &rasters); let methods: Vec = ["difference", "ratio", "percent"].iter().map(|s| s.to_string()).collect(); if app.workflow.format.is_empty() || !methods.contains(&app.workflow.format) { app.workflow.format = "difference".into(); } - combo(ui, "wf_cm", &mut app.workflow.format, &methods); + combo(ui, "wf_cm", "Method", &mut app.workflow.format, &methods); if ui.button(tl!("Run")).clicked() { run( app, @@ -582,7 +597,7 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { "trend" => trend(app, ui, &rasters), "spectral" => { heading(ui, "Band values at a point"); - combo(ui, "wf_sp", &mut app.workflow.layer, &rasters); + combo(ui, "wf_sp", "Layer", &mut app.workflow.layer, &rasters); pick_button(app, ui, "spectral", 1, "Click a pixel on the map"); if let Some(p) = picked(app, "spectral").and_then(|v| v.first().copied()) { run(app, "raster.spectral", json!({"layer": app.workflow.layer, "lon": p.x, "lat": p.y})); @@ -590,7 +605,7 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { } "classify" => { heading(ui, "Unsupervised classes (k-means)"); - combo(ui, "wf_k", &mut app.workflow.layer, &rasters); + combo(ui, "wf_k", "Layer", &mut app.workflow.layer, &rasters); ui.add(egui::TextEdit::singleline(&mut app.workflow.number).hint_text(tl!("classes")).desired_width(60.0)); if ui.button(tl!("Classify")).clicked() { run(app, "raster.classify", json!({"layer": app.workflow.layer, "classes": num(&app.workflow.number, 5.0).round() as u64})); @@ -598,10 +613,10 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { } "supervised" => { heading(ui, "Classify from training polygons"); - combo(ui, "wf_su", &mut app.workflow.layer, &rasters); - combo(ui, "wf_st", &mut app.workflow.layer2, &polygons); + combo(ui, "wf_su", "Raster", &mut app.workflow.layer, &rasters); + combo(ui, "wf_st", "Training polygons", &mut app.workflow.layer2, &polygons); let fields = fields_of(&app.workflow.layer2, false); - combo(ui, "wf_sf", &mut app.workflow.field, &fields); + combo(ui, "wf_sf", "Field", &mut app.workflow.field, &fields); if ui.button(tl!("Classify")).clicked() { run( app, @@ -612,10 +627,10 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { } "deepLearning" => { heading(ui, "Train a neural network on training polygons"); - combo(ui, "wf_dl", &mut app.workflow.layer, &rasters); - combo(ui, "wf_dt", &mut app.workflow.layer2, &polygons); + combo(ui, "wf_dl", "Raster", &mut app.workflow.layer, &rasters); + combo(ui, "wf_dt", "Training polygons", &mut app.workflow.layer2, &polygons); let fields = fields_of(&app.workflow.layer2, false); - combo(ui, "wf_df", &mut app.workflow.field, &fields); + combo(ui, "wf_df", "Field", &mut app.workflow.field, &fields); ui.horizontal(|ui| { ui.add(egui::TextEdit::singleline(&mut app.workflow.number).hint_text(tl!("epochs")).desired_width(60.0)); if ui.button(tl!("Train And Classify")).clicked() { @@ -648,16 +663,17 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { } "hosted" => { heading(ui, "Cut a raster into web map tiles"); - combo(ui, "wf_ho", &mut app.workflow.layer, &rasters); + combo(ui, "wf_ho", "Layer", &mut app.workflow.layer, &rasters); if ui.button(tl!("Create Hosted Imagery…")).clicked() { let _ = app.run("ui.saveItem", json!({"kind": "hostedImagery", "layer": app.workflow.layer})); } } "indices" => { heading(ui, "Spectral indices"); - combo(ui, "wf_ix", &mut app.workflow.layer, &rasters); - ui.label(egui::RichText::new(tl!("Bands: blue green red NIR SWIR")).small().weak()); - ui.add(egui::TextEdit::singleline(&mut app.workflow.text).hint_text("1 2 3 4 5").desired_width(120.0)); + combo(ui, "wf_ix", "Layer", &mut app.workflow.layer, &rasters); + let input_label = ui.label(egui::RichText::new(tl!("Bands: blue green red NIR SWIR")).small().weak()); + ui.add(egui::TextEdit::singleline(&mut app.workflow.text).hint_text("1 2 3 4 5").desired_width(120.0)) + .labelled_by(input_label.id); ui.horizontal_wrapped(|ui| { for idx in ["ndvi", "ndwi", "savi", "nbr", "ndbi", "evi"] { if ui.button(idx.to_uppercase()).clicked() { @@ -670,26 +686,30 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { } "process" => { heading(ui, "Clip or resample"); - ui.label(egui::RichText::new(tl!("Raster layer")).small().weak()); - combo(ui, "wf_pr", &mut app.workflow.layer, &rasters); + combo(ui, "wf_pr", "Raster layer", &mut app.workflow.layer, &rasters); ui.add_space(4.0); ui.label(egui::RichText::new(tl!("Clip to polygon layer")).small().weak()); let has_polys = !polygons.is_empty(); if has_polys { - combo(ui, "wf_pr_poly", &mut app.workflow.layer2, &polygons); + let layer2_before = app.workflow.layer2.clone(); + combo(ui, "wf_pr_poly", "Polygon layer", &mut app.workflow.layer2, &polygons); + if app.workflow.layer2 != layer2_before { + app.workflow.selected_only = None; + } let poly_layer = app.session.map().layer_by_name(&app.workflow.layer2); let sel_count = poly_layer.as_ref().map_or(0, |l| l.selection.len()); let total_features = poly_layer.as_ref().map_or(0, |l| l.dataset.features.len()); + let selected_only = app.workflow.selected_only.get_or_insert(sel_count > 0); ui.horizontal(|ui| { - ui.checkbox(&mut app.workflow.selected_only, tl!("Selected only")); + ui.checkbox(selected_only, tl!("Selected only")); if sel_count > 0 { ui.label(egui::RichText::new(format!("({sel_count} of {total_features})")).small().weak()); } }); - let sel_invalid = app.workflow.selected_only && sel_count == 0; + let sel_invalid = *selected_only && sel_count == 0; let can_clip_poly = !app.workflow.layer.is_empty() && !app.workflow.layer2.is_empty() && !sel_invalid; if sel_invalid { @@ -701,7 +721,7 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { "layer": app.workflow.layer, "operation": "clip", "polygon": app.workflow.layer2, - "selectedOnly": app.workflow.selected_only, + "selectedOnly": *selected_only, }); run(app, "raster.process", p); } @@ -718,7 +738,9 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { ui.add_space(4.0); ui.horizontal(|ui| { - ui.add(egui::TextEdit::singleline(&mut app.workflow.number).hint_text(tl!("factor")).desired_width(50.0)); + let factor_label = ui.label(tl!("factor")); + ui.add(egui::TextEdit::singleline(&mut app.workflow.number).hint_text(tl!("factor")).desired_width(50.0)) + .labelled_by(factor_label.id); if ui.add_enabled(!app.workflow.layer.is_empty(), egui::Button::new(tl!("Resample"))).clicked() { run( app, @@ -730,7 +752,7 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { } "pixels" => { heading(ui, "Set the pixels inside a polygon"); - combo(ui, "wf_px", &mut app.workflow.layer, &rasters); + combo(ui, "wf_px", "Layer", &mut app.workflow.layer, &rasters); ui.add(egui::TextEdit::singleline(&mut app.workflow.number).hint_text(tl!("value (empty: no-data)")).desired_width(140.0)); pick_button(app, ui, "pixels", 100, "Click the polygon's corners"); let n = app.workflow.pick.as_ref().filter(|p| p.purpose == "pixels").map_or(0, |p| p.points.len()); @@ -746,7 +768,7 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { } "mensuration" => { heading(ui, "Measure with an elevation raster"); - combo(ui, "wf_me", &mut app.workflow.layer, &rasters); + combo(ui, "wf_me", "Layer", &mut app.workflow.layer, &rasters); ui.horizontal_wrapped(|ui| { pick_button(app, ui, "m_distance", 2, "Distance"); pick_button(app, ui, "m_height", 2, "Height"); @@ -807,7 +829,7 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { } }); if kind == "reality" { - combo(ui, "wf_mesh", &mut app.workflow.layer, &rasters); + combo(ui, "wf_mesh", "Layer", &mut app.workflow.layer, &rasters); if ui.button(tl!("Export Mesh (OBJ)…")).clicked() { match app.session.run("imagery.mesh", &json!({"layer": app.workflow.layer})) { Ok(v) => { @@ -924,11 +946,11 @@ fn trend(app: &mut RustGisApp, ui: &mut Ui, rasters: &[String]) { } }); } else { - combo(ui, "wf_tr_layer", &mut app.workflow.layer, rasters); + combo(ui, "wf_tr_layer", "Layer", &mut app.workflow.layer, rasters); } let f = &mut app.workflow.trend; egui::Grid::new("trend_grid").num_columns(2).spacing(vec2(8.0, 6.0)).show(ui, |ui| { - ui.label(tl!("Test")); + let input_label = ui.label(tl!("Test")); let tests = [ ("hamedRao", "Modified Mann–Kendall (Hamed & Rao)"), ("yueWang", "Modified Mann–Kendall (Yue & Wang)"), @@ -936,19 +958,24 @@ fn trend(app: &mut RustGisApp, ui: &mut Ui, rasters: &[String]) { ("original", "Mann–Kendall"), ]; let shown = tests.iter().find(|t| t.0 == f.test).map_or("", |t| t.1); - egui::ComboBox::from_id_salt("trend_test").selected_text(tl!(shown)).width(240.0).show_ui(ui, |ui| { - for (code, label) in tests { - ui.selectable_value(&mut f.test, code.to_string(), tl!(label)); - } - }); + egui::ComboBox::from_id_salt("trend_test") + .selected_text(tl!(shown)) + .width(240.0) + .show_ui(ui, |ui| { + for (code, label) in tests { + ui.selectable_value(&mut f.test, code.to_string(), tl!(label)); + } + }) + .response + .labelled_by(input_label.id); ui.end_row(); if f.test == "seasonal" { - ui.label(tl!("Seasons a year")); - ui.add(egui::DragValue::new(&mut f.seasons).range(2..=366)); + let input_label = ui.label(tl!("Seasons a year")); + ui.add(egui::DragValue::new(&mut f.seasons).range(2..=366)).labelled_by(input_label.id); ui.end_row(); } - ui.label(tl!("Significance level")); - ui.add(egui::TextEdit::singleline(&mut f.alpha).desired_width(60.0)); + let input_label = ui.label(tl!("Significance level")); + ui.add(egui::TextEdit::singleline(&mut f.alpha).desired_width(60.0)).labelled_by(input_label.id); ui.end_row(); ui.label(tl!("Slope per")); ui.horizontal(|ui| { @@ -956,17 +983,17 @@ fn trend(app: &mut RustGisApp, ui: &mut Ui, rasters: &[String]) { ui.radio_value(&mut f.decade, true, tl!("decade")); }); ui.end_row(); - ui.label(tl!("From")); - ui.add(egui::TextEdit::singleline(&mut f.start).hint_text("2000-01-01").desired_width(110.0)); + let input_label = ui.label(tl!("From")); + ui.add(egui::TextEdit::singleline(&mut f.start).hint_text("2000-01-01").desired_width(110.0)).labelled_by(input_label.id); ui.end_row(); - ui.label(tl!("To")); - ui.add(egui::TextEdit::singleline(&mut f.end).hint_text("2024-12-31").desired_width(110.0)); + let input_label = ui.label(tl!("To")); + ui.add(egui::TextEdit::singleline(&mut f.end).hint_text("2024-12-31").desired_width(110.0)).labelled_by(input_label.id); ui.end_row(); - ui.label(tl!("Months")); - ui.add(egui::TextEdit::singleline(&mut f.months).hint_text("12, 1, 2").desired_width(110.0)); + let input_label = ui.label(tl!("Months")); + ui.add(egui::TextEdit::singleline(&mut f.months).hint_text("12, 1, 2").desired_width(110.0)).labelled_by(input_label.id); ui.end_row(); - ui.label(tl!("Dates a cell needs")); - ui.add(egui::TextEdit::singleline(&mut f.min_valid).hint_text(tl!("half")).desired_width(60.0)); + let input_label = ui.label(tl!("Dates a cell needs")); + ui.add(egui::TextEdit::singleline(&mut f.min_valid).hint_text(tl!("half")).desired_width(60.0)).labelled_by(input_label.id); ui.end_row(); }); ui.label( @@ -1185,12 +1212,14 @@ fn script_rows(ui: &Ui) -> f32 { ui.text_style_height(&egui::TextStyle::Monospace) * SCRIPT_MIN_ROWS as f32 } -/// The height the Script pane needs below its box: the buttons, and a run's results (their own -/// scroll, at most SCRIPT_OUT_ROWS lines) when there are any. +/// The height the Script pane needs below its box: the buttons, the last run's line, and its +/// results (their own scroll, at most SCRIPT_OUT_ROWS lines) when there are any. fn script_below(app: &RustGisApp, ui: &Ui) -> f32 { let gap = ui.spacing().item_spacing.y; + // A row: as tall as a button, whatever its text. + let last = if app.workflow.script_last.is_none() { 0.0 } else { ui.spacing().interact_size.y + gap }; let results = if app.workflow.script_out.is_empty() { 0.0 } else { script_out_height(ui) + gap }; - ui.spacing().interact_size.y + gap * 3.0 + results + ui.spacing().interact_size.y + gap * 3.0 + last + results } /// The most height a run's results take. @@ -1214,8 +1243,7 @@ fn script(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens, fit: bool) { let out_h = script_out_height(ui); // One id whether the pane scrolls as a whole or not, so resizing it across that keeps the // box's focus, cursor and selection. - #[allow(deprecated)] - let box_id = egui::Id::new("script_text"); + let box_id = egui::Id::unique("script_text"); // Ctrl+Enter (⌘+Enter) runs the script (#773), taken before the box sees it, so it runs rather // than breaking the line; once a press, not again for each repeat of a key held down. The box // keeps the focus, so the list closes, as Escape closes it. @@ -1262,11 +1290,18 @@ fn script(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens, fit: bool) { crate::scriptcomplete::braces(ui, before, &out.inner, &mut app.workflow.script); } ui.horizontal(|ui| { - let shortcut = if cfg!(target_os = "macos") { "⌘+Enter" } else { "Ctrl+Enter" }; + // As egui reads the keys: ⌘ on a Mac, the browser's too (from its user agent). + let shortcut = if ui.ctx().os().is_mac() { "⌘+Enter" } else { "Ctrl+Enter" }; let run = ui.button(tl!("Run")).on_hover_text(crate::i18n::fmt("Run the script ({keys})", &[("keys", shortcut)])).clicked(); if run || ctrl_enter { let script = app.workflow.script.clone(); + // When it started and how long it took: a tool run outside the app in the browser + // (`gp.finish`) ends later. + let at = web_time::SystemTime::now().duration_since(web_time::UNIX_EPOCH).map_or(0, |d| d.as_secs()); + let started = web_time::Instant::now(); let out = app.run("script.run", json!({"script": script})); + let n = app.workflow.script_last.as_ref().map_or(1, |r| r.n.saturating_add(1)); + app.workflow.script_last = Some(ScriptRun { n, secs: started.elapsed().as_secs_f64(), at }); app.workflow.script_out = match out { Ok(v) => v["ran"] .as_array() @@ -1280,6 +1315,21 @@ fn script(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens, fit: bool) { let _ = app.run("ui.commandSearch", Value::Null); } }); + // Above the results' scroll, so it stays in view: its number and how long it took, and on + // the right when it started, as the Geoprocessing History tells it (UTC: the app has no time + // zone). + if let Some(r) = &app.workflow.script_last { + ui.horizontal(|ui| { + let text = crate::i18n::fmt("Run #{n} ({secs} s)", &[("n", &r.n.to_string()), ("secs", &format!("{:.2}", r.secs))]); + ui.label(egui::RichText::new(text).small().strong()); + if r.at > 0 { + let (_, time) = rustgis_engine::map::graphics::civil(r.at); + ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { + ui.label(egui::RichText::new(crate::i18n::fmt("{time} UTC", &[("time", &time)])).small().weak()) + }); + } + }); + } // In a scroll of their own, so a long error (it wraps) can't push them out of the pane. egui::ScrollArea::vertical().id_salt("script_out").max_height(out_h).auto_shrink([false, true]).show(ui, |ui| { for (line, err) in &app.workflow.script_out { @@ -1288,3 +1338,42 @@ fn script(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens, fit: bool) { } }); } + +#[cfg(test)] +mod tests { + use super::*; + use egui_kittest::kittest::Queryable; + + /// Every workflow has named inputs, including both weights sharing a row (#790). + #[test] + fn every_workflow_names_its_inputs() { + for &(section, _) in SECTIONS { + let mut app = RustGisApp::with_sample(); + app.workflow.section = section; + let mut fonts = false; + let mut h = egui_kittest::Harness::builder().with_size(egui::vec2(500.0, 1200.0)).build_ui_state( + move |ui, app: &mut RustGisApp| { + if !fonts { + crate::theme::install_fonts(ui.ctx(), &[], None); + fonts = true; + return; + } + workflows(app, ui, &Tokens::light()); + }, + app, + ); + h.run(); + if section == "suitability" { + assert_eq!(h.query_all_by_role_and_label(egui::accesskit::Role::TextInput, "Weights").count(), 2); + for label in ["First raster", "Second raster"] { + assert!(h.query_by_role_and_label(egui::accesskit::Role::ComboBox, label).is_some()); + } + } + if section == "zonalStats" { + for label in ["Band", "Prefix", "Output"] { + assert!(h.query_by_role_and_label(egui::accesskit::Role::TextInput, label).is_some()); + } + } + } + } +} From d921c7d7001e323c521447d68c4fff5fe5d44108 Mon Sep 17 00:00:00 2001 From: quanguet0409 Date: Sat, 24 Oct 2026 22:16:35 +0700 Subject: [PATCH 09/11] fix(analysis): prefer exact match in find_field_case_insensitive before ambiguous collision scan --- crates/analysis/src/tools.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/crates/analysis/src/tools.rs b/crates/analysis/src/tools.rs index 37a3445f5..cd686ef9e 100644 --- a/crates/analysis/src/tools.rs +++ b/crates/analysis/src/tools.rs @@ -578,6 +578,9 @@ fn copy_schema(ds: &Dataset, name: String, g: GeometryType) -> Dataset { /// If multiple fields in `ds.fields` match `name` case-insensitively, returns an error /// detailing the collision rather than silently selecting an arbitrary field. pub fn find_field_case_insensitive(ds: &Dataset, name: &str) -> Result { + if let Some(idx) = ds.fields.iter().position(|f| f.name == name) { + return Ok(idx); + } let mut matches = Vec::new(); for (idx, f) in ds.fields.iter().enumerate() { if f.name.eq_ignore_ascii_case(name) { @@ -3036,12 +3039,16 @@ mod tests { assert!(out.field_index("SUM_Population").is_some()); assert!(out.field_index("MEAN_Population").is_some()); - // 4. Case-insensitive collision detection + // 4. Exact match precedence and case-insensitive collision detection let colliding_fields = vec![Field::new("Population", FieldType::Double), Field::new("population", FieldType::Double)]; let ds_collision = Dataset::new("colliding", GeometryType::None, colliding_fields); - let err = find_field_case_insensitive(&ds_collision, "population").unwrap_err(); + // Exact matches must resolve directly: + assert_eq!(find_field_case_insensitive(&ds_collision, "population").unwrap(), 1); + assert_eq!(find_field_case_insensitive(&ds_collision, "Population").unwrap(), 0); + // Inexact matches with multiple candidates are ambiguous: + let err = find_field_case_insensitive(&ds_collision, "POPULATION").unwrap_err(); assert!(err.contains("ambiguous") && err.contains("collision"), "collision must be explicitly reported: {err}"); - let err2 = find_field_case_insensitive(&ds_collision, "POPULATION").unwrap_err(); + let err2 = find_field_case_insensitive(&ds_collision, "pOpUlAtIoN").unwrap_err(); assert!(err2.contains("collision")); } From 20fd0bf6d10ba088a01aad57135e1e96be515703 Mon Sep 17 00:00:00 2001 From: quanguet0409 Date: Sat, 24 Oct 2026 22:22:19 +0700 Subject: [PATCH 10/11] fix(analysis): reject empty string items in bands array --- crates/analysis/src/tools.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/analysis/src/tools.rs b/crates/analysis/src/tools.rs index cd686ef9e..feeaf7054 100644 --- a/crates/analysis/src/tools.rs +++ b/crates/analysis/src/tools.rs @@ -1608,6 +1608,9 @@ fn parse_bands(p: &Json, total_bands: usize) -> Result, String> { } } Json::String(s) => { + if s.trim().is_empty() { + return Err("band items cannot be empty strings".into()); + } let sub = crate::rasterops::parse_bands_str(s, total_bands)?; for b_idx in sub { if !out.contains(&b_idx) { @@ -3143,5 +3146,7 @@ mod tests { assert!(crate::run("zonalStats", &m, &serde_json::json!({"raster": "elev", "polygons": "zones", "bands": 1.5})).is_err()); assert!(crate::run("zonalStats", &m, &serde_json::json!({"raster": "elev", "polygons": "zones", "bands": 99})).is_err()); assert!(crate::run("zonalStats", &m, &serde_json::json!({"raster": "elev", "polygons": "zones", "bands": -1})).is_err()); + assert!(crate::run("zonalStats", &m, &serde_json::json!({"raster": "elev", "polygons": "zones", "bands": [""]})).is_err()); + assert!(crate::run("zonalStats", &m, &serde_json::json!({"raster": "elev", "polygons": "zones", "bands": [" "]})).is_err()); } } From e0f4da72c78be02d2c66da9b51c938e9f9670349 Mon Sep 17 00:00:00 2001 From: quanguet0409 Date: Sat, 24 Oct 2026 23:02:52 +0700 Subject: [PATCH 11/11] fix(ui): dynamic selected_only default and localized selection count label --- crates/ui-egui/src/analysispane.rs | 19 +++++++++++++------ crates/ui-egui/tests/smoke.rs | 3 +++ 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/crates/ui-egui/src/analysispane.rs b/crates/ui-egui/src/analysispane.rs index c1e9fda5f..22d02bcb0 100644 --- a/crates/ui-egui/src/analysispane.rs +++ b/crates/ui-egui/src/analysispane.rs @@ -701,15 +701,22 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { let sel_count = poly_layer.as_ref().map_or(0, |l| l.selection.len()); let total_features = poly_layer.as_ref().map_or(0, |l| l.dataset.features.len()); - let selected_only = app.workflow.selected_only.get_or_insert(sel_count > 0); + let effective_selected_only = app.workflow.selected_only.unwrap_or(sel_count > 0); + let mut checked = effective_selected_only; ui.horizontal(|ui| { - ui.checkbox(selected_only, tl!("Selected only")); + if ui.checkbox(&mut checked, tl!("Selected only")).changed() { + app.workflow.selected_only = Some(checked); + } if sel_count > 0 { - ui.label(egui::RichText::new(format!("({sel_count} of {total_features})")).small().weak()); + let text = crate::i18n::fmt( + "({selected} of {total})", + &[("selected", &sel_count.to_string()), ("total", &total_features.to_string())], + ); + ui.label(egui::RichText::new(text).small().weak()); } }); - let sel_invalid = *selected_only && sel_count == 0; + let sel_invalid = effective_selected_only && sel_count == 0; let can_clip_poly = !app.workflow.layer.is_empty() && !app.workflow.layer2.is_empty() && !sel_invalid; if sel_invalid { @@ -717,12 +724,12 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { } if ui.add_enabled(can_clip_poly, egui::Button::new(tl!("Clip To Polygon"))).clicked() { - let p = json!({ + let mut p = json!({ "layer": app.workflow.layer, "operation": "clip", "polygon": app.workflow.layer2, - "selectedOnly": *selected_only, }); + p["selectedOnly"] = json!(app.workflow.selected_only); run(app, "raster.process", p); } } else { diff --git a/crates/ui-egui/tests/smoke.rs b/crates/ui-egui/tests/smoke.rs index 79386ccd9..f4d8cb007 100644 --- a/crates/ui-egui/tests/smoke.rs +++ b/crates/ui-egui/tests/smoke.rs @@ -245,12 +245,15 @@ fn polygon_clip_defaults_to_selection_and_honors_opt_out() { h.state_mut().session.map_mut().layers.iter_mut().find(|l| l.name == "Countries").unwrap().selection.push(0); h.run_steps(4); assert_eq!(h.state().workflow.selected_only, None); + let total = h.state().session.map().layer_by_name("Countries").unwrap().dataset.features.len(); + assert!(h.query_by_label(&format!("(1 of {total})")).is_some()); h.get_by_label("Selected only").click(); h.run_steps(3); assert_eq!(h.state().workflow.selected_only, Some(false)); h.state_mut().session.map_mut().layers.iter_mut().find(|l| l.name == "Countries").unwrap().selection.push(1); h.run_steps(3); assert_eq!(h.state().workflow.selected_only, Some(false)); + assert!(h.query_by_label(&format!("(2 of {total})")).is_some()); h.get_by_label("Selected only").click(); h.run_steps(3); assert_eq!(h.state().workflow.selected_only, Some(true));