From 5e775d5fa5b44acc218f19adeb0bf437bd745433 Mon Sep 17 00:00:00 2001 From: Qiusheng Wu Date: Sat, 24 Oct 2026 13:55:02 -0400 Subject: [PATCH] fix: address zonal statistics review feedback --- apps/rustgis/examples/navigate_bench.rs | 2 +- apps/rustgis/src/main.rs | 2 +- crates/analysis/src/rasterops.rs | 114 +++++++++--------------- crates/analysis/src/tools.rs | 94 ++++++------------- crates/analysis/src/trend.rs | 4 +- crates/assistant/src/agent.rs | 2 +- crates/data/src/raster/mod.rs | 2 +- crates/engine/src/cmd/imagery.rs | 13 ++- crates/engine/src/cmd/select.rs | 2 +- crates/engine/src/model.rs | 2 +- crates/engine/src/tests.rs | 42 +++++++-- crates/map/src/globe.rs | 2 +- crates/tiles/src/embeddings.rs | 2 +- crates/ui-egui/src/analysispane.rs | 28 +++--- crates/ui-egui/src/lib.rs | 2 +- docs/user-guide/analysis.md | 10 +++ 16 files changed, 146 insertions(+), 177 deletions(-) diff --git a/apps/rustgis/examples/navigate_bench.rs b/apps/rustgis/examples/navigate_bench.rs index 2d519766f..e0c5fc560 100644 --- a/apps/rustgis/examples/navigate_bench.rs +++ b/apps/rustgis/examples/navigate_bench.rs @@ -78,7 +78,7 @@ impl eframe::App for Bench { // From the zoom's start, all of it, however long a frame took. let dt = (t - self.last_wheel.max(LOAD + PAN)).max(0.0); self.last_wheel = t; - let dir = if z < ZOOM / 3.0 || z >= 2.0 * ZOOM / 3.0 { -1.0 } else { 1.0 }; + let dir = if !(ZOOM / 3.0..2.0 * ZOOM / 3.0).contains(&z) { -1.0 } else { 1.0 }; input.events.push(egui::Event::MouseWheel { unit: egui::MouseWheelUnit::Point, delta: egui::vec2(0.0, (dir * 600.0 * dt) as f32), diff --git a/apps/rustgis/src/main.rs b/apps/rustgis/src/main.rs index 2377d413e..d57c2cd65 100644 --- a/apps/rustgis/src/main.rs +++ b/apps/rustgis/src/main.rs @@ -309,7 +309,7 @@ fn main() -> eframe::Result { let files: Vec<&String> = args .iter() .enumerate() - .filter(|(i, a)| !a.starts_with("--") && !(*i > 0 && args.get(i - 1).is_some_and(|p| p == "--backend"))) + .filter(|(i, a)| !(a.starts_with("--") || *i > 0 && args.get(i - 1).is_some_and(|p| p == "--backend"))) .map(|(_, a)| a) .collect(); // Installed plugins, before the first session lists commands and tools. diff --git a/crates/analysis/src/rasterops.rs b/crates/analysis/src/rasterops.rs index 786d4b9cb..7bb573887 100644 --- a/crates/analysis/src/rasterops.rs +++ b/crates/analysis/src/rasterops.rs @@ -523,29 +523,6 @@ 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 or degenerate ring (< 3 points) rejects the entire polygon part. -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 { - return None; - } - 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) -} - /// An error encountered while validating or transforming a polygon geometry. #[derive(Clone, Debug, PartialEq, Eq)] pub enum PolygonTransformError { @@ -643,6 +620,24 @@ pub fn transform_feature_polygons_to_lonlat( Ok(out) } +/// Validates a 1-based band number and returns its 0-based index. +pub(crate) fn band_from_f64(value: f64, total_bands: usize) -> Result { + if !value.is_finite() { + return Err("band number must be a finite number".into()); + } + if value <= 0.0 { + return Err("band numbers must be positive (1-based)".into()); + } + if value.fract() != 0.0 { + return Err("band numbers must be integers, not decimals".into()); + } + // Check before casting, including on wasm32, where huge values would saturate. + if value > total_bands as f64 || value >= usize::MAX as f64 { + return Err(format!("band index {value} is invalid: raster has bands 1 to {total_bands}")); + } + Ok(value as usize - 1) +} + /// Parses a band specification string into 0-based band indices. /// /// Supported inputs: @@ -691,20 +686,7 @@ pub fn parse_bands_str(s: &str, total_bands: usize) -> Result, String 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; + let b_idx = band_from_f64(f, total_bands)?; if !out.contains(&b_idx) { out.push(b_idx); } @@ -716,53 +698,37 @@ pub fn parse_bands_str(s: &str, total_bands: usize) -> Result, String Ok(out) } +/// Projects a complete ring to the raster grid without dropping failed vertices. +fn project_ring(r: &Raster, ring: &[Coord]) -> Option> { + if ring.len() < 3 { + return None; + } + let tr = r.transformer(); + ring.iter() + .map(|&c| { + if !c.is_finite() { + return None; + } + let pt = r.transform.to_pixel(tr.from_lonlat(c)?); + (pt.0.is_finite() && pt.1.is_finite()).then_some(pt) + }) + .collect() +} + /// 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. /// /// The rings are projected onto the raster's own grid once and filled by scanline, so a large /// raster costs a pass over the polygon edges per row rather than a point test per cell. +/// A malformed ring or a failed projection rejects the entire polygon part, including its holes. pub fn polygon_mask(r: &Raster, polygons: &[Vec>]) -> Option<(Range, Range, Vec)> { - let tr = r.transformer(); let projected: Vec>> = polygons .iter() .filter_map(|rings| { - let exterior = rings.first()?; - let mut ext_pts = Vec::with_capacity(exterior.len()); - for c in exterior { - if !c.is_finite() { - return None; - } - let w = tr.from_lonlat(*c)?; - let pt = r.transform.to_pixel(w); - if !pt.0.is_finite() || !pt.1.is_finite() { - return None; - } - ext_pts.push(pt); - } - 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()); - for c in hole { - if !c.is_finite() { - return None; - } - let w = tr.from_lonlat(*c)?; - let pt = r.transform.to_pixel(w); - if !pt.0.is_finite() || !pt.1.is_finite() { - return None; - } - hole_pts.push(pt); - } - if hole_pts.len() < 3 { - return None; - } - valid_rings.push(hole_pts); - } - Some(valid_rings) + // Reject a complete part if any ring fails, so a lost hole cannot enlarge it. + rings.first()?; + rings.iter().map(|ring| project_ring(r, ring)).collect::>>() }) .collect(); let (mut c0, mut r0, mut c1, mut r1) = (f64::INFINITY, f64::INFINITY, f64::NEG_INFINITY, f64::NEG_INFINITY); diff --git a/crates/analysis/src/tools.rs b/crates/analysis/src/tools.rs index feeaf7054..80b64a4ae 100644 --- a/crates/analysis/src/tools.rs +++ b/crates/analysis/src/tools.rs @@ -587,11 +587,11 @@ pub fn find_field_case_insensitive(ds: &Dataset, name: &str) -> Result Err(format!("no field named '{name}'")), - 1 => Ok(matches[0]), + match matches.as_slice() { + [] => Err(format!("no field named '{name}'")), + [only] => Ok(*only), _ => { - let names: Vec = matches.iter().map(|&i| ds.fields[i].name.clone()).collect(); + let names: Vec = matches.iter().filter_map(|&i| ds.fields.get(i).map(|f| f.name.clone())).collect(); Err(format!( "field name '{name}' is ambiguous: case-insensitive collision between {}", names.iter().map(|n| format!("'{n}'")).collect::>().join(" and ") @@ -1546,79 +1546,33 @@ impl StatOp { } /// Parses the `bands` parameter for zonal statistics. +/// Null and an empty string default to band 1, matching the workflow form. pub fn zonal_bands(value: &Json, total_bands: usize) -> Result, String> { - let dummy = serde_json::json!({ "bands": value }); - parse_bands(&dummy, total_bands) -} - -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()); - } - 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}")); - } - Ok(vec![b - 1]) + match value { + Json::Null => Ok(vec![0]), + Json::String(s) => crate::rasterops::parse_bands_str(s, total_bands), + Json::Number(n) => { + Ok(vec![crate::rasterops::band_from_f64(n.as_f64().ok_or_else(|| "invalid band number".to_string())?, total_bands)?]) } - Some(Json::Array(arr)) => { + 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); - } - } - 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) { - out.push(b_idx); - } - } + if !matches!(item, Json::Number(_) | Json::String(_)) { + return Err("band items must be numbers or strings".into()); + } + if item.as_str().is_some_and(|s| s.trim().is_empty()) { + return Err("band items cannot be empty strings".into()); + } + for band in zonal_bands(item, total_bands)? { + if !out.contains(&band) { + out.push(band); } - _ => return Err("band items must be numbers or strings".into()), } } Ok(out) @@ -1627,6 +1581,10 @@ fn parse_bands(p: &Json, total_bands: usize) -> Result, String> { } } +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> { match p.get("stats") { None | Some(Json::Null) => Ok(StatOp::ALL.to_vec()), @@ -3140,9 +3098,9 @@ mod tests { 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 + // 2. An empty band string defaults to band 1; malformed inputs fail. 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": ""})).is_ok()); 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/analysis/src/trend.rs b/crates/analysis/src/trend.rs index 73eb4873d..4db29388c 100644 --- a/crates/analysis/src/trend.rs +++ b/crates/analysis/src/trend.rs @@ -111,7 +111,7 @@ pub fn trend_with(t: &[f64], y: &[f64], test: Test, alpha: f64, slopes: &mut Vec kept.sort_by(|a, b| a.0.total_cmp(&b.0)); let (t, y): (Vec, Vec) = kept.into_iter().unzip(); let n = y.len(); - if !(MIN_VALUES..=MAX_VALUES).contains(&n) || !(alpha > 0.0 && alpha < 1.0) { + if !((MIN_VALUES..=MAX_VALUES).contains(&n) && alpha > 0.0 && alpha < 1.0) { return None; } if let Test::Seasonal(m) = test { @@ -222,7 +222,7 @@ pub fn trend_seasonal_with(t: &[f64], y: &[f64], season: &[usize], alpha: f64, s let mut kept: Vec<(f64, f64, usize)> = t.iter().zip(y).zip(season).filter(|((t, y), _)| t.is_finite() && y.is_finite()).map(|((t, y), s)| (*t, *y, *s)).collect(); kept.sort_by(|a, b| a.0.total_cmp(&b.0)); - if !(MIN_VALUES..=MAX_VALUES).contains(&kept.len()) || !(alpha > 0.0 && alpha < 1.0) { + if !((MIN_VALUES..=MAX_VALUES).contains(&kept.len()) && alpha > 0.0 && alpha < 1.0) { return None; } let t: Vec = kept.iter().map(|k| k.0).collect(); diff --git a/crates/assistant/src/agent.rs b/crates/assistant/src/agent.rs index 91a06c97f..c793244ec 100644 --- a/crates/assistant/src/agent.rs +++ b/crates/assistant/src/agent.rs @@ -209,7 +209,7 @@ impl Agent { fn local_delegation(&self, call: &ToolCall) -> bool { if call.name != "delegate" || self.depth != 0 - || !call.args.get("task").and_then(serde_json::Value::as_str).is_some_and(|s| !s.trim().is_empty()) + || call.args.get("task").and_then(serde_json::Value::as_str).is_none_or(|s| s.trim().is_empty()) { return false; } diff --git a/crates/data/src/raster/mod.rs b/crates/data/src/raster/mod.rs index 24206647f..5335a5158 100644 --- a/crates/data/src/raster/mod.rs +++ b/crates/data/src/raster/mod.rs @@ -400,7 +400,7 @@ impl Raster { if self.georeferenced { return Err(DataError::Format("only a raster with no coordinates can be placed".into())); } - if !bbox.iter().all(|v| v.is_finite()) || !(w < e && s < n) { + if !(bbox.iter().all(|v| v.is_finite()) && w < e && s < n) { return Err(DataError::Format("the box is [west, south, east, north]".into())); } let old = self.transform; diff --git a/crates/engine/src/cmd/imagery.rs b/crates/engine/src/cmd/imagery.rs index 4fb18210b..68f9d1e76 100644 --- a/crates/engine/src/cmd/imagery.rs +++ b/crates/engine/src/cmd/imagery.rs @@ -536,10 +536,10 @@ fn index(s: &mut Session, p: &Value) -> CmdResult { /// - 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 +/// - The raster is cropped to the tight bounding box of valid 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. +/// - Features with invalid geometry or unprojectable coordinates are skipped and reported in the result messages. /// - 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)?; @@ -609,7 +609,14 @@ fn process(s: &mut Session, p: &Value) -> CmdResult { 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)))?; - add(s, out, false) + let mut result = add(s, out, false)?; + if !failed_features.is_empty() { + result["messages"] = json!([format!( + "{pl}: skipped {} feature(s) with invalid polygon geometry or failed coordinate transformation", + failed_features.len() + )]); + } + Ok(result) } "clip" => { // Before the extent is placed on the raster's coordinates. diff --git a/crates/engine/src/cmd/select.rs b/crates/engine/src/cmd/select.rs index 8f5fb2544..a14b49e10 100644 --- a/crates/engine/src/cmd/select.rs +++ b/crates/engine/src/cmd/select.rs @@ -218,7 +218,7 @@ fn identify(s: &mut Session, p: &Value) -> CmdResult { let map = s.map(); let mut results: Vec = hits.iter().filter_map(|(id, row)| feature_json(map, *id, *row)).collect(); // Then the features of vector tile layers, beneath the layers; tiles are never selectable. - if matches!(mode, PopupMode::Topmost | PopupMode::Visible) && !(first_layer && !results.is_empty()) { + if matches!(mode, PopupMode::Topmost | PopupMode::Visible) && (!first_layer || results.is_empty()) { let room = MAX_IDENTIFIED.saturating_sub(results.len()); results.extend(tile_features(s, lon, lat, tol_px, first_layer, room)); } diff --git a/crates/engine/src/model.rs b/crates/engine/src/model.rs index 9593a843d..75004be53 100644 --- a/crates/engine/src/model.rs +++ b/crates/engine/src/model.rs @@ -223,7 +223,7 @@ pub fn validate(s: &Session, g: &ModelGraph) -> Vec { }; for p in t.params.iter().filter(|p| p.required) { let port = port_kind(p.kind).is_some(); - if !(port && wired(p.name)) && !is_set(n.params.get(p.name)) && p.default.is_empty() { + if !(is_set(n.params.get(p.name)) || port && wired(p.name)) && p.default.is_empty() { let msg = if port { "\"{p}\" needs a connection or a layer." } else { "\"{p}\" needs a value." }; out.push(problem("missing-input", Some(&n.id), msg.replace("{p}", p.label))); } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 223bf8319..f2088fbf2 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -15380,11 +15380,25 @@ fn gp_zonal_stats_end_to_end_and_undo() { 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)); + let multiband = Raster::new( + "multi", + w, + h, + vec![vec![2.0; (w * h) as usize], vec![7.0; (w * h) as usize]], + gt, + Crs::LonLat, + None, + ColorHint::Gray, + "Float32", + ) + .unwrap(); + s.map_mut().add_raster("multi", rustgis_map::RasterLayer::from_path(multiband, "multi.tif")); + // "bands": "all" let all_res = s.run( "gp.zonalStats", &json!({ - "raster": "dem", + "raster": "multi", "polygons": "districts", "output": "all_bands_out", "bands": "all", @@ -15394,11 +15408,23 @@ fn gp_zonal_stats_end_to_end_and_undo() { 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()); + let all_layer = s.map().layer_by_name("all_bands_out").unwrap(); + for (field, expected) in [("multi_b1_mean", 2.0), ("multi_b2_mean", 7.0)] { + let idx = all_layer.dataset.field_index(field).expect("all bands must be expanded"); + assert_eq!(all_layer.dataset.value(0, idx), &Value::Float(expected)); + } + let error = s.run("gp.zonalStats", &json!({"raster": "dem", "polygons": "Cities"})).unwrap_err(); + assert!(error.contains("needs polygon geometries"), "{error}"); + for (band, fragment) in [ + (json!(999), "raster has bands"), + (json!(0), "positive"), + (json!(-1), "positive"), + (json!(1.5), "integers"), + (json!(4294967297u64), "raster has bands"), + ] { + let error = s.run("gp.zonalStats", &json!({"raster": "dem", "polygons": "districts", "bands": [band]})).unwrap_err(); + assert!(error.contains(fragment), "{error}"); + } } #[test] @@ -15552,7 +15578,9 @@ fn raster_clip_polygon_selection_semantics_and_partial_failure() { "name": "dem_partial", }), ); - assert!(res_partial.is_ok(), "clip partial should succeed for valid feature: {res_partial:?}"); + let partial_result = res_partial.expect("clip partial should succeed for valid feature"); + let warning = partial_result["messages"][0].as_str().expect("partial failures must be visible in command output"); + assert!(warning.contains("skipped 1 feature(s)"), "{warning}"); 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); diff --git a/crates/map/src/globe.rs b/crates/map/src/globe.rs index c9ae4a565..ed0699e56 100644 --- a/crates/map/src/globe.rs +++ b/crates/map/src/globe.rs @@ -560,7 +560,7 @@ impl Globe { pub fn drape_mesh(&self, merc: &Extent) -> DrapeMesh { use rustgis_geom::merc_to_lonlat; let mut out = DrapeMesh::default(); - if merc.is_empty() || !(merc.width() > 0.0 && merc.height() > 0.0) || !merc.width().is_finite() || !merc.height().is_finite() { + if !(!merc.is_empty() && merc.width() > 0.0 && merc.height() > 0.0 && merc.width().is_finite() && merc.height().is_finite()) { return out; } let ll = diff --git a/crates/tiles/src/embeddings.rs b/crates/tiles/src/embeddings.rs index a83af5400..dda9b6ee9 100644 --- a/crates/tiles/src/embeddings.rs +++ b/crates/tiles/src/embeddings.rs @@ -403,7 +403,7 @@ fn utm_zone(lon: f64, lat: f64) -> u32 { /// The Sentinel-2 (MGRS 100 km) tile holding a place, such as `17SKV`; `None` outside MGRS's /// UTM latitudes (80° S to 84° N). pub fn mgrs_tile(lon: f64, lat: f64) -> Option { - if !(lon.is_finite() && lat.is_finite()) || !(-80.0..84.0).contains(&lat) { + if !(lon.is_finite() && lat.is_finite() && (-80.0..84.0).contains(&lat)) { return None; } let zone = utm_zone(lon, lat); diff --git a/crates/ui-egui/src/analysispane.rs b/crates/ui-egui/src/analysispane.rs index 22d02bcb0..e556324af 100644 --- a/crates/ui-egui/src/analysispane.rs +++ b/crates/ui-egui/src/analysispane.rs @@ -322,6 +322,9 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { 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); + if app.session.map().layer_by_name(&app.workflow.layer2).is_some_and(|l| !l.selection.is_empty()) { + ui.label(egui::RichText::new(tl!("Only selected polygons will be included in the output.")).small().weak()); + } 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)) @@ -340,24 +343,21 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { .labelled_by(output_label.id); }); let band_str = app.workflow.zonal_stats.bands.trim(); - 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 raster_data = + app.session.map().layer_by_name(&app.workflow.layer).and_then(|l| l.raster.as_ref()).and_then(|r| r.data.as_ref()); + let total_bands = raster_data.map_or(0, |d| d.band_count()); + let raster_ready = raster_data.is_some(); + let band_res = rustgis_engine::analysis::zonal_bands(&json!(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; + let can_run = layers_ready && raster_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 let Some(err) = band_err { - ui.label(egui::RichText::new(err).small().color(t.amber)); + } else if !raster_ready { + ui.label(egui::RichText::new(tl!("Raster data is unavailable.")).small().color(t.amber)); + } else if band_err.is_some() { + 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 mut params = json!({ @@ -729,7 +729,7 @@ pub fn workflows(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { "operation": "clip", "polygon": app.workflow.layer2, }); - p["selectedOnly"] = json!(app.workflow.selected_only); + p["selectedOnly"] = json!(effective_selected_only); run(app, "raster.process", p); } } else { diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index ef36a824e..61c72dc1a 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -1577,7 +1577,7 @@ impl RustGisApp { let active = self.session.active_layer.take(); let needs = (spec.enabled)(&self.session).is_some(); self.session.active_layer = active; - if !defaults && !(needs && (spec.enabled)(&self.session).is_none()) { + if !(defaults || needs && (spec.enabled)(&self.session).is_none()) { return None; } let map = self.session.map(); diff --git a/docs/user-guide/analysis.md b/docs/user-guide/analysis.md index 71a87dfe3..210008ee1 100644 --- a/docs/user-guide/analysis.md +++ b/docs/user-guide/analysis.md @@ -550,6 +550,16 @@ For `raster.process` polygon clipping, `selectedOnly: true` requires a selection The Process pane initially checks Selected only when the chosen layer has a selection, and sends the choice explicitly. Clipping and Zonal Statistics share polygon transformation rules so a failed hole cannot silently enlarge the sampled area. +A malformed ring or a failed projection rejects its complete polygon part in all raster masking +operations. Clipping skips invalid features and reports their count in the command result; +its output bounds cover the valid polygons only. + +An empty band field or `bands: ""` selects band 1. The pane disables calculation when raster +data is unavailable and indicates when only selected polygons will appear in the output. + +Field parameters in Dissolve, XY Table to Point, IDW, Kriging and Summary Statistics accept +case-insensitive names. An exact spelling takes precedence; otherwise a name matching several +fields is rejected as ambiguous. This prevents silently using the wrong field. ## Deep learning and hosted imagery