From 9b86453b056042f7f8ce589aee75713cdc5e5791 Mon Sep 17 00:00:00 2001 From: Dominik Strebel Date: Mon, 2 Nov 2026 11:14:49 +0100 Subject: [PATCH 1/6] style(analysis): lib.rs with Unix line endings, as every other file It came in with Windows line endings in #1081, the only file in the repository that has them; cargo fmt turns them back to Unix ones on any edit. No change but the line endings. Co-Authored-By: Claude Opus 5.5 --- crates/analysis/src/lib.rs | 834 ++++++++++++++++++------------------- 1 file changed, 417 insertions(+), 417 deletions(-) diff --git a/crates/analysis/src/lib.rs b/crates/analysis/src/lib.rs index e223d8618..1aba9f130 100644 --- a/crates/analysis/src/lib.rs +++ b/crates/analysis/src/lib.rs @@ -1,417 +1,417 @@ -//! L2: geoprocessing tools. -//! -//! Each tool is a [`ToolSpec`]: an id, a toolbox, parameters for the Tools pane, and a -//! pure `run` function over datasets. Inputs refer to layers by name through [`Inputs`]. Work is -//! done in Web Mercator metres, except buffers, which are geodesic: drawn in a projection centred -//! on each feature (see [`geodesic`]) so a 100 km buffer is 100 km on the ground. -//! -//! Tools from external suites (the GeoLibre/Whitebox tools in `rustgis-geolibre`, and plugins -//! in `rustgis-plugins`) join the built-in ones through [`register_external`]: they're listed by [`tools`], found by -//! [`tool`] and run by [`run`] like the rest. -#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] - -pub mod attribute_join; -pub mod convert; -pub mod density; -pub mod dggs; -pub mod geodesic; -pub mod georef; -mod join; -pub mod kriging; -mod merge; -pub mod network; -pub mod neural; -mod parallel; -mod parameters; -pub mod rasterops; -pub mod selection; -mod stats; -pub mod tissot; -mod tools; -pub mod topology; -pub mod trend; -pub mod warp; - -pub use attribute_join::{DuplicateStrategy, JoinFieldOptions, JoinFieldReport, join_field}; -pub use density::{ - KernelType, MAX_DENSITY_CELLS, MAX_DENSITY_WORK, auto_cell_size, kernel_density, points_extent, resolve_grid_extent, - silverman_bandwidth, -}; -pub use parameters::StatisticList; -pub use tools::{TOOLS, find_field_case_insensitive, holds_coordinates, param_applies, simplify_geometry, zonal_bands}; - -use rustgis_data::Dataset; -use serde_json::Value; - -/// Parameter kinds, which the Tools pane turns into widgets. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub enum ParamKind { - /// Any layer. - Layer, - /// A polygon layer. - PolygonLayer, - /// A point layer. - PointLayer, - /// A field of the associated input (the longest `_` name prefix, else the first layer). - Field, - /// Several field names entered as text, with suggestions from the associated input (tools - /// whose runner takes the list as one string). The runner defines the list syntax; - /// suggestions append comma-separated names. - FieldNames, - /// A numeric field of the associated input. - NumericField, - /// A field of the associated input that can hold coordinates: a numeric field, or a text field - /// with numbers written in it (see [`holds_coordinates`]). - CoordinateField, - /// Ordered field names from the named input; accepts the existing string or array values. - Fields { - input: &'static str, - }, - /// Ordered field/statistic objects from the named input, using the existing JSON keys. - FieldStatistics { - input: &'static str, - statistics: StatisticList, - }, - Number, - Text, - /// One of a fixed list of choices. - Choice(&'static [&'static str]), - /// A raster layer. - RasterLayer, - /// Any vector layer (no standalone tables). - VectorLayer, - /// Several raster layers, in order: their names (or file paths) separated by `;`. - RasterLayers, - /// Several vector layers, in order: their names (or file paths) separated by `;`. - VectorLayers, - /// On or off. - Bool, - /// A file path typed by the user (for data RustGIS doesn't open as a layer, such as LiDAR). - Path, - /// A file written by the tool, not an input to read. Native callers supply a destination; - /// browser callers supply a download name (or use the tool's suggested name). - SaveFile { - ext: &'static str, - }, -} - -#[derive(Clone, Copy, Debug)] -pub struct ParamSpec { - pub name: &'static str, - pub label: &'static str, - pub kind: ParamKind, - pub required: bool, - /// Default value as JSON text (`"10"`, `"\"km\""`), or empty. - pub default: &'static str, - /// A longer description, shown on hover (may be empty). - pub help: &'static str, -} - -/// What a tool produced. -#[derive(Clone, Debug)] -pub enum Output { - /// A new layer to add to the map. - Layer(Dataset), - /// Rows to select in an existing layer. - Selection { layer: String, rows: Vec }, - /// An existing layer's dataset replaced (e.g. new fields). - Update { layer: String, dataset: Dataset }, - /// A standalone table. - Table(Dataset), - /// Several new layers and tables (external tools can write more than one output). - Layers(Vec), - /// A downloadable file, never added to the map as a layer. - File { file_name: String, bytes: Vec }, -} - -/// One output of [`Output::Layers`]. -#[derive(Clone, Debug)] -pub enum OutputLayer { - Vector(Dataset), - /// A raster file, kept as its bytes so the project can embed it. - Raster { - name: String, - file_name: String, - bytes: Vec, - }, - Table(Dataset), -} - -/// A raster layer handed to a tool: its file (on disk or embedded), or failing that the pixels. -pub struct RasterInput<'a> { - pub raster: &'a rustgis_data::raster::Raster, - /// The file it was read from, when it is on disk. - pub path: Option<&'a str>, - /// The file's name and bytes, when it is embedded in the project. - pub embedded: Option<(&'a str, &'a [u8])>, -} - -#[derive(Clone, Debug)] -pub struct ToolResult { - pub output: Output, - pub messages: Vec, -} - -/// Access to the map's layers by name. -pub trait Inputs { - fn dataset(&self, layer: &str) -> Option<&Dataset>; - /// A standalone table by its id: an input given as a number, which no name is taken for. - fn table(&self, _id: u32) -> Option<&Dataset> { - None - } - /// Selected rows of a layer; tools honour a selection like desktop GIS does. - fn selection(&self, layer: &str) -> &[usize]; - /// A raster layer by name. - fn raster(&self, _layer: &str) -> Option> { - None - } - /// The active map's flat projection, for tools that show what it does to the Earth (Tissot's - /// indicatrix): its name. `None` when there's no map or it is a globe. - fn projection_name(&self) -> Option { - None - } - /// A longitude/latitude point on the active map's projection plane, in metres. `None` where - /// the projection doesn't reach, or with no flat map ([`Inputs::projection_name`]). - fn project(&self, _c: rustgis_geom::Coord) -> Option { - None - } - /// The longitude/latitude extent the map's view shows, when there is a view. - fn view_extent(&self) -> Option { - None - } - - /// Builds a vector-tile archive without coupling the analysis crate to the tile crate. - fn vector_to_pmtiles( - &self, - _dataset: &Dataset, - _rows: &[usize], - _min_zoom: u8, - _max_zoom: u8, - _source_layer: &str, - ) -> Result<(Vec, Vec), String> { - Err("vector-to-PMTiles export is unavailable in this input provider".into()) - } - - /// Builds a raster tile archive with the layer's existing display style. - fn raster_to_pmtiles(&self, _layer: &str, _min_zoom: u8, _max_zoom: u8) -> Result, String> { - Err("raster-to-PMTiles export is unavailable in this input provider".into()) - } -} - -pub struct ToolSpec { - pub id: &'static str, - pub name: &'static str, - pub toolbox: &'static str, - pub summary: &'static str, - pub params: &'static [ParamSpec], - pub run: fn(&dyn Inputs, &Value) -> Result, -} - -/// Runs an external tool (the spec says which). -pub type ExternalRunner = fn(&ToolSpec, &dyn Inputs, &Value) -> Result; - -/// Where external tools come from: called the first time the tools are listed, and again after -/// [`reload_external`]. -pub type ExternalProvider = fn() -> Vec; - -/// One external tool suite: its tools are loaded lazily and leaked (each load lives as long as -/// the app, and reloads are rare: the user installs a suite or a plugin), so specs can be handed -/// out as `&'static`. -struct Source { - key: &'static str, - provider: ExternalProvider, - runner: ExternalRunner, - tools: Option<&'static [ToolSpec]>, -} - -static SOURCES: std::sync::RwLock> = std::sync::RwLock::new(Vec::new()); - -/// Adds an external tool suite under `key` (the GeoLibre tools, plugins). Only the first -/// registration of a key counts; the provider runs lazily, so registering costs nothing until a -/// tool list is needed. -pub fn register_external(key: &'static str, provider: ExternalProvider, runner: ExternalRunner) { - if let Ok(mut v) = SOURCES.write() - && !v.iter().any(|s| s.key == key) - { - v.push(Source { key, provider, runner, tools: None }); - } -} - -/// The tools of the suite registered as `key` (empty when there is none). -pub fn external_tools_of(key: &str) -> &'static [ToolSpec] { - let provider = match SOURCES.read() { - Ok(v) => match v.iter().find(|s| s.key == key) { - Some(Source { tools: Some(t), .. }) => return t, - Some(s) => s.provider, - None => return &[], - }, - Err(_) => return &[], - }; - // One load at a time: a second caller waits for the first instead of loading again. The - // provider runs without the sources' lock held, so a slow one never blocks the others. - static LOADING: std::sync::Mutex<()> = std::sync::Mutex::new(()); - let _loading = LOADING.lock(); - if let Ok(v) = SOURCES.read() - && let Some(Source { tools: Some(t), .. }) = v.iter().find(|s| s.key == key) - { - return t; - } - let tools: &'static [ToolSpec] = Box::leak( - std::panic::catch_unwind(provider) - .unwrap_or_else(|_| { - log::error!("the external tools '{key}' failed to load"); - Vec::new() - }) - .into_boxed_slice(), - ); - match SOURCES.write() { - Ok(mut v) => match v.iter_mut().find(|s| s.key == key) { - Some(s) => s.tools.get_or_insert(tools), - None => tools, - }, - Err(_) => tools, - } -} - -/// The external tools of every suite, in registration order (empty when none are registered). -pub fn external_tools() -> Vec<&'static ToolSpec> { - let keys: Vec<&'static str> = SOURCES.read().map(|v| v.iter().map(|s| s.key).collect()).unwrap_or_default(); - keys.into_iter().flat_map(external_tools_of).collect() -} - -/// Loads every suite's tools again the next time they're needed. -pub fn reload_external() { - if let Ok(mut v) = SOURCES.write() { - for s in v.iter_mut() { - s.tools = None; - } - } -} - -/// Loads the tools of the suite `key` again the next time they're needed (it was installed, -/// located or removed, or a plugin was). -pub fn reload_external_of(key: &str) { - if let Ok(mut v) = SOURCES.write() - && let Some(s) = v.iter_mut().find(|s| s.key == key) - { - s.tools = None; - } -} - -/// Every tool: the built-in ones, then the external ones. -pub fn tools() -> impl Iterator { - TOOLS.iter().chain(external_tools()) -} - -/// The suite an external tool comes from, with its runner (`None` for a built-in tool). -fn source_of(spec: &ToolSpec) -> Option<(&'static str, ExternalRunner)> { - // By id, not by address: a spec handed out before a reload (an open tool form) is still - // the same tool. - if TOOLS.iter().any(|t| std::ptr::eq(t, spec)) { - return None; - } - let keys: Vec<(&'static str, ExternalRunner)> = - SOURCES.read().map(|v| v.iter().map(|s| (s.key, s.runner)).collect()).unwrap_or_default(); - keys.into_iter().find(|(key, _)| external_tools_of(key).iter().any(|t| t.id == spec.id)) -} - -/// Whether a tool comes from an external suite. -pub fn is_external(spec: &ToolSpec) -> bool { - source_of(spec).is_some() -} - -/// The key of the suite a tool comes from (`None` for a built-in tool). -pub fn external_source(spec: &ToolSpec) -> Option<&'static str> { - source_of(spec).map(|(k, _)| k) -} - -pub fn tool(id: &str) -> Option<&'static ToolSpec> { - let by_id = |t: &&ToolSpec| t.id.eq_ignore_ascii_case(id); - let by_name = |t: &&ToolSpec| t.name.eq_ignore_ascii_case(id); - // A built-in tool wins over an external one with the same name. - tools().find(by_id).or_else(|| TOOLS.iter().find(by_name)).or_else(|| external_tools().into_iter().find(by_name)) -} - -/// The runner for external tools; never called (external specs carry the runner's id). -pub fn external_placeholder(_: &dyn Inputs, _: &Value) -> Result { - Err("this tool runs through the external suite".into()) -} - -/// A tool's parameters with defaults filled in, or an error naming a missing required one. -/// [`run`] does this first; a host that runs an external tool another way (the browser's -/// asynchronous runner) does it too, so both check parameters the same way. -pub fn resolve_params(spec: &ToolSpec, params: &Value) -> Result { - let mut p = params.as_object().cloned().unwrap_or_default(); - // Not an external suite's tool (a copy of a built-in spec counts as built-in). - let built_in = source_of(spec).is_none(); - if built_in { - tools::normalize_params(spec, &mut p); - } - for ps in spec.params { - if !p.contains_key(ps.name) - && !ps.default.is_empty() - && let Ok(v) = serde_json::from_str::(ps.default) - { - p.insert(ps.name.to_string(), v); - } - if ps.required && p.get(ps.name).is_none_or(|v| v.is_null() || v.as_str() == Some("")) { - return Err(format!("{}: '{}' is required", spec.name, ps.label)); - } - // The built-in tools' choices are the whole list. An external tool's (GeoLibre's, a - // plugin's) can be looser, such as GeoLibre's comma-separated lists of options: its own - // runner checks them. - if let ParamKind::Choice(choices) = ps.kind - && built_in - && let Some(v) = p.get_mut(ps.name) - { - *v = choice(spec, ps, choices, v)?; - } - } - Ok(Value::Object(p)) -} - -/// A built-in tool's choice parameter as one of its choices, spelled as declared (`"meters"` is -/// `"Meters"`). Any other value is an error naming the choices: a tool never guesses what an -/// unknown one meant (#406). A blank value is the declared default. -fn choice(spec: &ToolSpec, ps: &ParamSpec, choices: &[&str], v: &Value) -> Result { - let given = match v { - Value::Null => return Ok(Value::Null), - // Blank: the declared default, as when it's left out. - Value::String(s) if s.trim().is_empty() => return Ok(serde_json::from_str(ps.default).unwrap_or_else(|_| v.clone())), - Value::String(s) => s.trim().to_string(), - Value::Number(_) | Value::Bool(_) => v.to_string(), - _ => String::new(), - }; - match choices.iter().find(|c| c.eq_ignore_ascii_case(&given)) { - Some(c) => Ok(Value::String((*c).to_string())), - None => Err(format!("{}: '{}' must be one of {} (not {v})", spec.name, ps.label, choices.join(", "))), - } -} - -/// A multi-layer parameter: semicolon-separated names or an array of names. -pub fn layer_list(value: &Value) -> Result, String> { - let names: Vec<&str> = match value { - Value::String(text) => text.split(';').map(str::trim).filter(|n| !n.is_empty()).collect(), - Value::Array(values) => values - .iter() - .map(|v| v.as_str().map(str::trim).filter(|n| !n.is_empty()).ok_or("input layers must be nonempty names")) - .collect::>()?, - _ => return Err("input layers must be names separated by ; or an array of names".into()), - }; - if names.is_empty() { - return Err("at least one input layer is required".into()); - } - Ok(names) -} - -/// Runs a tool with JSON parameters, filling defaults and checking required ones. -pub fn run(id: &str, inputs: &dyn Inputs, params: &Value) -> Result { - let spec = tool(id).ok_or_else(|| format!("no tool '{id}'"))?; - let p = resolve_params(spec, params)?; - // A bug in a tool must not take the app down. - std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| match source_of(spec) { - Some((_, runner)) => runner(spec, inputs, &p), - None => (spec.run)(inputs, &p), - })) - .unwrap_or_else(|_| Err(format!("{} failed unexpectedly", spec.name))) -} +//! L2: geoprocessing tools. +//! +//! Each tool is a [`ToolSpec`]: an id, a toolbox, parameters for the Tools pane, and a +//! pure `run` function over datasets. Inputs refer to layers by name through [`Inputs`]. Work is +//! done in Web Mercator metres, except buffers, which are geodesic: drawn in a projection centred +//! on each feature (see [`geodesic`]) so a 100 km buffer is 100 km on the ground. +//! +//! Tools from external suites (the GeoLibre/Whitebox tools in `rustgis-geolibre`, and plugins +//! in `rustgis-plugins`) join the built-in ones through [`register_external`]: they're listed by [`tools`], found by +//! [`tool`] and run by [`run`] like the rest. +#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] + +pub mod attribute_join; +pub mod convert; +pub mod density; +pub mod dggs; +pub mod geodesic; +pub mod georef; +mod join; +pub mod kriging; +mod merge; +pub mod network; +pub mod neural; +mod parallel; +mod parameters; +pub mod rasterops; +pub mod selection; +mod stats; +pub mod tissot; +mod tools; +pub mod topology; +pub mod trend; +pub mod warp; + +pub use attribute_join::{DuplicateStrategy, JoinFieldOptions, JoinFieldReport, join_field}; +pub use density::{ + KernelType, MAX_DENSITY_CELLS, MAX_DENSITY_WORK, auto_cell_size, kernel_density, points_extent, resolve_grid_extent, + silverman_bandwidth, +}; +pub use parameters::StatisticList; +pub use tools::{TOOLS, find_field_case_insensitive, holds_coordinates, param_applies, simplify_geometry, zonal_bands}; + +use rustgis_data::Dataset; +use serde_json::Value; + +/// Parameter kinds, which the Tools pane turns into widgets. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum ParamKind { + /// Any layer. + Layer, + /// A polygon layer. + PolygonLayer, + /// A point layer. + PointLayer, + /// A field of the associated input (the longest `_` name prefix, else the first layer). + Field, + /// Several field names entered as text, with suggestions from the associated input (tools + /// whose runner takes the list as one string). The runner defines the list syntax; + /// suggestions append comma-separated names. + FieldNames, + /// A numeric field of the associated input. + NumericField, + /// A field of the associated input that can hold coordinates: a numeric field, or a text field + /// with numbers written in it (see [`holds_coordinates`]). + CoordinateField, + /// Ordered field names from the named input; accepts the existing string or array values. + Fields { + input: &'static str, + }, + /// Ordered field/statistic objects from the named input, using the existing JSON keys. + FieldStatistics { + input: &'static str, + statistics: StatisticList, + }, + Number, + Text, + /// One of a fixed list of choices. + Choice(&'static [&'static str]), + /// A raster layer. + RasterLayer, + /// Any vector layer (no standalone tables). + VectorLayer, + /// Several raster layers, in order: their names (or file paths) separated by `;`. + RasterLayers, + /// Several vector layers, in order: their names (or file paths) separated by `;`. + VectorLayers, + /// On or off. + Bool, + /// A file path typed by the user (for data RustGIS doesn't open as a layer, such as LiDAR). + Path, + /// A file written by the tool, not an input to read. Native callers supply a destination; + /// browser callers supply a download name (or use the tool's suggested name). + SaveFile { + ext: &'static str, + }, +} + +#[derive(Clone, Copy, Debug)] +pub struct ParamSpec { + pub name: &'static str, + pub label: &'static str, + pub kind: ParamKind, + pub required: bool, + /// Default value as JSON text (`"10"`, `"\"km\""`), or empty. + pub default: &'static str, + /// A longer description, shown on hover (may be empty). + pub help: &'static str, +} + +/// What a tool produced. +#[derive(Clone, Debug)] +pub enum Output { + /// A new layer to add to the map. + Layer(Dataset), + /// Rows to select in an existing layer. + Selection { layer: String, rows: Vec }, + /// An existing layer's dataset replaced (e.g. new fields). + Update { layer: String, dataset: Dataset }, + /// A standalone table. + Table(Dataset), + /// Several new layers and tables (external tools can write more than one output). + Layers(Vec), + /// A downloadable file, never added to the map as a layer. + File { file_name: String, bytes: Vec }, +} + +/// One output of [`Output::Layers`]. +#[derive(Clone, Debug)] +pub enum OutputLayer { + Vector(Dataset), + /// A raster file, kept as its bytes so the project can embed it. + Raster { + name: String, + file_name: String, + bytes: Vec, + }, + Table(Dataset), +} + +/// A raster layer handed to a tool: its file (on disk or embedded), or failing that the pixels. +pub struct RasterInput<'a> { + pub raster: &'a rustgis_data::raster::Raster, + /// The file it was read from, when it is on disk. + pub path: Option<&'a str>, + /// The file's name and bytes, when it is embedded in the project. + pub embedded: Option<(&'a str, &'a [u8])>, +} + +#[derive(Clone, Debug)] +pub struct ToolResult { + pub output: Output, + pub messages: Vec, +} + +/// Access to the map's layers by name. +pub trait Inputs { + fn dataset(&self, layer: &str) -> Option<&Dataset>; + /// A standalone table by its id: an input given as a number, which no name is taken for. + fn table(&self, _id: u32) -> Option<&Dataset> { + None + } + /// Selected rows of a layer; tools honour a selection like desktop GIS does. + fn selection(&self, layer: &str) -> &[usize]; + /// A raster layer by name. + fn raster(&self, _layer: &str) -> Option> { + None + } + /// The active map's flat projection, for tools that show what it does to the Earth (Tissot's + /// indicatrix): its name. `None` when there's no map or it is a globe. + fn projection_name(&self) -> Option { + None + } + /// A longitude/latitude point on the active map's projection plane, in metres. `None` where + /// the projection doesn't reach, or with no flat map ([`Inputs::projection_name`]). + fn project(&self, _c: rustgis_geom::Coord) -> Option { + None + } + /// The longitude/latitude extent the map's view shows, when there is a view. + fn view_extent(&self) -> Option { + None + } + + /// Builds a vector-tile archive without coupling the analysis crate to the tile crate. + fn vector_to_pmtiles( + &self, + _dataset: &Dataset, + _rows: &[usize], + _min_zoom: u8, + _max_zoom: u8, + _source_layer: &str, + ) -> Result<(Vec, Vec), String> { + Err("vector-to-PMTiles export is unavailable in this input provider".into()) + } + + /// Builds a raster tile archive with the layer's existing display style. + fn raster_to_pmtiles(&self, _layer: &str, _min_zoom: u8, _max_zoom: u8) -> Result, String> { + Err("raster-to-PMTiles export is unavailable in this input provider".into()) + } +} + +pub struct ToolSpec { + pub id: &'static str, + pub name: &'static str, + pub toolbox: &'static str, + pub summary: &'static str, + pub params: &'static [ParamSpec], + pub run: fn(&dyn Inputs, &Value) -> Result, +} + +/// Runs an external tool (the spec says which). +pub type ExternalRunner = fn(&ToolSpec, &dyn Inputs, &Value) -> Result; + +/// Where external tools come from: called the first time the tools are listed, and again after +/// [`reload_external`]. +pub type ExternalProvider = fn() -> Vec; + +/// One external tool suite: its tools are loaded lazily and leaked (each load lives as long as +/// the app, and reloads are rare: the user installs a suite or a plugin), so specs can be handed +/// out as `&'static`. +struct Source { + key: &'static str, + provider: ExternalProvider, + runner: ExternalRunner, + tools: Option<&'static [ToolSpec]>, +} + +static SOURCES: std::sync::RwLock> = std::sync::RwLock::new(Vec::new()); + +/// Adds an external tool suite under `key` (the GeoLibre tools, plugins). Only the first +/// registration of a key counts; the provider runs lazily, so registering costs nothing until a +/// tool list is needed. +pub fn register_external(key: &'static str, provider: ExternalProvider, runner: ExternalRunner) { + if let Ok(mut v) = SOURCES.write() + && !v.iter().any(|s| s.key == key) + { + v.push(Source { key, provider, runner, tools: None }); + } +} + +/// The tools of the suite registered as `key` (empty when there is none). +pub fn external_tools_of(key: &str) -> &'static [ToolSpec] { + let provider = match SOURCES.read() { + Ok(v) => match v.iter().find(|s| s.key == key) { + Some(Source { tools: Some(t), .. }) => return t, + Some(s) => s.provider, + None => return &[], + }, + Err(_) => return &[], + }; + // One load at a time: a second caller waits for the first instead of loading again. The + // provider runs without the sources' lock held, so a slow one never blocks the others. + static LOADING: std::sync::Mutex<()> = std::sync::Mutex::new(()); + let _loading = LOADING.lock(); + if let Ok(v) = SOURCES.read() + && let Some(Source { tools: Some(t), .. }) = v.iter().find(|s| s.key == key) + { + return t; + } + let tools: &'static [ToolSpec] = Box::leak( + std::panic::catch_unwind(provider) + .unwrap_or_else(|_| { + log::error!("the external tools '{key}' failed to load"); + Vec::new() + }) + .into_boxed_slice(), + ); + match SOURCES.write() { + Ok(mut v) => match v.iter_mut().find(|s| s.key == key) { + Some(s) => s.tools.get_or_insert(tools), + None => tools, + }, + Err(_) => tools, + } +} + +/// The external tools of every suite, in registration order (empty when none are registered). +pub fn external_tools() -> Vec<&'static ToolSpec> { + let keys: Vec<&'static str> = SOURCES.read().map(|v| v.iter().map(|s| s.key).collect()).unwrap_or_default(); + keys.into_iter().flat_map(external_tools_of).collect() +} + +/// Loads every suite's tools again the next time they're needed. +pub fn reload_external() { + if let Ok(mut v) = SOURCES.write() { + for s in v.iter_mut() { + s.tools = None; + } + } +} + +/// Loads the tools of the suite `key` again the next time they're needed (it was installed, +/// located or removed, or a plugin was). +pub fn reload_external_of(key: &str) { + if let Ok(mut v) = SOURCES.write() + && let Some(s) = v.iter_mut().find(|s| s.key == key) + { + s.tools = None; + } +} + +/// Every tool: the built-in ones, then the external ones. +pub fn tools() -> impl Iterator { + TOOLS.iter().chain(external_tools()) +} + +/// The suite an external tool comes from, with its runner (`None` for a built-in tool). +fn source_of(spec: &ToolSpec) -> Option<(&'static str, ExternalRunner)> { + // By id, not by address: a spec handed out before a reload (an open tool form) is still + // the same tool. + if TOOLS.iter().any(|t| std::ptr::eq(t, spec)) { + return None; + } + let keys: Vec<(&'static str, ExternalRunner)> = + SOURCES.read().map(|v| v.iter().map(|s| (s.key, s.runner)).collect()).unwrap_or_default(); + keys.into_iter().find(|(key, _)| external_tools_of(key).iter().any(|t| t.id == spec.id)) +} + +/// Whether a tool comes from an external suite. +pub fn is_external(spec: &ToolSpec) -> bool { + source_of(spec).is_some() +} + +/// The key of the suite a tool comes from (`None` for a built-in tool). +pub fn external_source(spec: &ToolSpec) -> Option<&'static str> { + source_of(spec).map(|(k, _)| k) +} + +pub fn tool(id: &str) -> Option<&'static ToolSpec> { + let by_id = |t: &&ToolSpec| t.id.eq_ignore_ascii_case(id); + let by_name = |t: &&ToolSpec| t.name.eq_ignore_ascii_case(id); + // A built-in tool wins over an external one with the same name. + tools().find(by_id).or_else(|| TOOLS.iter().find(by_name)).or_else(|| external_tools().into_iter().find(by_name)) +} + +/// The runner for external tools; never called (external specs carry the runner's id). +pub fn external_placeholder(_: &dyn Inputs, _: &Value) -> Result { + Err("this tool runs through the external suite".into()) +} + +/// A tool's parameters with defaults filled in, or an error naming a missing required one. +/// [`run`] does this first; a host that runs an external tool another way (the browser's +/// asynchronous runner) does it too, so both check parameters the same way. +pub fn resolve_params(spec: &ToolSpec, params: &Value) -> Result { + let mut p = params.as_object().cloned().unwrap_or_default(); + // Not an external suite's tool (a copy of a built-in spec counts as built-in). + let built_in = source_of(spec).is_none(); + if built_in { + tools::normalize_params(spec, &mut p); + } + for ps in spec.params { + if !p.contains_key(ps.name) + && !ps.default.is_empty() + && let Ok(v) = serde_json::from_str::(ps.default) + { + p.insert(ps.name.to_string(), v); + } + if ps.required && p.get(ps.name).is_none_or(|v| v.is_null() || v.as_str() == Some("")) { + return Err(format!("{}: '{}' is required", spec.name, ps.label)); + } + // The built-in tools' choices are the whole list. An external tool's (GeoLibre's, a + // plugin's) can be looser, such as GeoLibre's comma-separated lists of options: its own + // runner checks them. + if let ParamKind::Choice(choices) = ps.kind + && built_in + && let Some(v) = p.get_mut(ps.name) + { + *v = choice(spec, ps, choices, v)?; + } + } + Ok(Value::Object(p)) +} + +/// A built-in tool's choice parameter as one of its choices, spelled as declared (`"meters"` is +/// `"Meters"`). Any other value is an error naming the choices: a tool never guesses what an +/// unknown one meant (#406). A blank value is the declared default. +fn choice(spec: &ToolSpec, ps: &ParamSpec, choices: &[&str], v: &Value) -> Result { + let given = match v { + Value::Null => return Ok(Value::Null), + // Blank: the declared default, as when it's left out. + Value::String(s) if s.trim().is_empty() => return Ok(serde_json::from_str(ps.default).unwrap_or_else(|_| v.clone())), + Value::String(s) => s.trim().to_string(), + Value::Number(_) | Value::Bool(_) => v.to_string(), + _ => String::new(), + }; + match choices.iter().find(|c| c.eq_ignore_ascii_case(&given)) { + Some(c) => Ok(Value::String((*c).to_string())), + None => Err(format!("{}: '{}' must be one of {} (not {v})", spec.name, ps.label, choices.join(", "))), + } +} + +/// A multi-layer parameter: semicolon-separated names or an array of names. +pub fn layer_list(value: &Value) -> Result, String> { + let names: Vec<&str> = match value { + Value::String(text) => text.split(';').map(str::trim).filter(|n| !n.is_empty()).collect(), + Value::Array(values) => values + .iter() + .map(|v| v.as_str().map(str::trim).filter(|n| !n.is_empty()).ok_or("input layers must be nonempty names")) + .collect::>()?, + _ => return Err("input layers must be names separated by ; or an array of names".into()), + }; + if names.is_empty() { + return Err("at least one input layer is required".into()); + } + Ok(names) +} + +/// Runs a tool with JSON parameters, filling defaults and checking required ones. +pub fn run(id: &str, inputs: &dyn Inputs, params: &Value) -> Result { + let spec = tool(id).ok_or_else(|| format!("no tool '{id}'"))?; + let p = resolve_params(spec, params)?; + // A bug in a tool must not take the app down. + std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| match source_of(spec) { + Some((_, runner)) => runner(spec, inputs, &p), + None => (spec.run)(inputs, &p), + })) + .unwrap_or_else(|_| Err(format!("{} failed unexpectedly", spec.name))) +} From e61dfb1fcef2c8220f5dc0b4bb7a7ae73e0cf4b0 Mon Sep 17 00:00:00 2001 From: Dominik Strebel Date: Mon, 2 Nov 2026 11:17:46 +0100 Subject: [PATCH 2/6] feat(analysis): a running tool can be cancelled and reports its progress (#1291) Step 3a and 3b of #1291: the plumbing for Cancel and a progress bar, used by nothing visible yet. - `analysis::control::Control`: a cancel flag and a count of the items (rows, features) done of the tool's current step. `analysis::run_with` makes it the current one of the thread the tool runs on, so a tool checks it in its long loops without passing it along; `gp::run_tool_with` does the same in the engine. `analysis::run` and `gp.run` are unchanged. - The parallel helpers hand it to their worker threads, stop after the rows or items in hand once it is cancelled, and count what they make: viewshed, IDW, kriging and the buffers of Select by Distance. - The other capped tools check it between features or rows: kernel density, the topology checks, random points, and neural training and classification. - A run cancelled before it returns is an error saying how far its current step came, whatever the tool gave back, so what a tool made by then is never taken for its result; it is kept in the geoprocessing history as a failed run. Co-Authored-By: Claude Opus 5.5 --- crates/analysis/src/control.rs | 161 +++++++++++++++++++++++++++++++ crates/analysis/src/density.rs | 7 +- crates/analysis/src/kriging.rs | 3 +- crates/analysis/src/lib.rs | 20 ++++ crates/analysis/src/network.rs | 7 +- crates/analysis/src/neural.rs | 10 +- crates/analysis/src/parallel.rs | 117 ++++++++++++++++++---- crates/analysis/src/rasterops.rs | 6 +- crates/analysis/src/tools.rs | 43 +++++++++ crates/analysis/src/topology.rs | 29 +++++- crates/engine/src/cmd/gp.rs | 13 ++- crates/engine/src/tests.rs | 17 ++++ 12 files changed, 404 insertions(+), 29 deletions(-) create mode 100644 crates/analysis/src/control.rs diff --git a/crates/analysis/src/control.rs b/crates/analysis/src/control.rs new file mode 100644 index 000000000..0acbdf57a --- /dev/null +++ b/crates/analysis/src/control.rs @@ -0,0 +1,161 @@ +//! Cancelling a running tool and following its progress (#1291). A caller hands +//! [`crate::run_with`] a [`Control`] and keeps a copy: setting its flag from another thread (a +//! Cancel button, a timeout, Ctrl-C) stops the tool at its next check, and its count says how far +//! the tool has come. The run's control is the current one of the thread the tool runs on, so a +//! tool asks [`check`] in its long loops without passing it along; the threads of +//! [`crate::parallel`] take it over from the thread that starts them. + +use std::cell::RefCell; +use std::sync::Arc; +use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; + +/// A running tool's cancel flag and progress, shared between the tool and whoever runs it. +#[derive(Debug, Default)] +pub struct Control { + cancel: AtomicBool, + done: AtomicU64, + total: AtomicU64, +} + +impl Control { + pub fn new() -> Arc { + Arc::new(Control::default()) + } + + /// Asks the tool to stop at its next check. + pub fn cancel(&self) { + self.cancel.store(true, Ordering::Relaxed); + } + + pub fn is_cancelled(&self) -> bool { + self.cancel.load(Ordering::Relaxed) + } + + /// The items (rows, features) done of those the tool's current step has, once it has said + /// how many: `None` before, or for a tool that doesn't count. + pub fn progress(&self) -> Option<(u64, u64)> { + let total = self.total.load(Ordering::Relaxed); + (total > 0).then(|| (self.done.load(Ordering::Relaxed).min(total), total)) + } +} + +/// A tool stopped because it was asked to. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct Cancelled; + +/// What [`crate::run_with`] says of a run that was cancelled (it names the tool and how far it +/// came). +pub(crate) const CANCELLED: &str = "cancelled"; + +impl From for String { + fn from(_: Cancelled) -> String { + CANCELLED.into() + } +} + +impl From for &'static str { + fn from(_: Cancelled) -> &'static str { + CANCELLED + } +} + +thread_local! { + static CURRENT: RefCell>> = const { RefCell::new(None) }; +} + +/// The control of the run on this thread, if any. +pub(crate) fn current() -> Option> { + CURRENT.with(|c| c.borrow().clone()) +} + +/// Makes `control` this thread's current one until the guard is dropped, which puts back the one +/// before (a tool run inside another's keeps the outer run's afterwards). +pub(crate) fn install(control: Option>) -> Installed { + Installed(CURRENT.with(|c| c.replace(control))) +} + +pub(crate) struct Installed(Option>); + +impl Drop for Installed { + fn drop(&mut self) { + let before = self.0.take(); + // Never during the thread's teardown: a guard outlives nothing that late. + let _ = CURRENT.try_with(|c| c.replace(before)); + } +} + +/// Whether the run on this thread was asked to stop. +pub(crate) fn cancelled() -> bool { + CURRENT.with(|c| c.borrow().as_ref().is_some_and(|c| c.is_cancelled())) +} + +/// Stops the tool when its run was asked to: call it between rows or features of a long loop. +pub(crate) fn check() -> Result<(), Cancelled> { + if cancelled() { Err(Cancelled) } else { Ok(()) } +} + +/// Starts counting a step of `total` items (rows, features) for the progress, from none done. +pub(crate) fn begin(total: usize) { + CURRENT.with(|c| { + if let Some(c) = c.borrow().as_ref() { + c.done.store(0, Ordering::Relaxed); + c.total.store(total as u64, Ordering::Relaxed); + } + }); +} + +/// Says that the first `done` items of the step are made: for a loop over them, at the top of each +/// (its index). +pub(crate) fn reached(done: usize) { + CURRENT.with(|c| { + if let Some(c) = c.borrow().as_ref() { + c.done.store(done as u64, Ordering::Relaxed); + } + }); +} + +/// Counts `n` more items of the step done. +pub(crate) fn advance(n: usize) { + CURRENT.with(|c| { + if let Some(c) = c.borrow().as_ref() { + c.done.fetch_add(n as u64, Ordering::Relaxed); + } + }); +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_run_sees_its_own_control_and_the_one_before_comes_back() { + assert!(current().is_none() && check().is_ok()); + let outer = Control::new(); + { + let _g = install(Some(outer.clone())); + begin(10); + advance(4); + assert_eq!(outer.progress(), Some((4, 10))); + let inner = Control::new(); + { + let _g = install(Some(inner.clone())); + inner.cancel(); + assert_eq!(check(), Err(Cancelled)); + } + assert!(check().is_ok(), "the outer run wasn't cancelled"); + outer.cancel(); + assert!(cancelled()); + } + assert!(current().is_none() && check().is_ok()); + } + + #[test] + fn no_progress_before_a_step_is_counted() { + let c = Control::new(); + assert_eq!(c.progress(), None); + let _g = install(Some(c.clone())); + begin(3); + advance(5); + assert_eq!(c.progress(), Some((3, 3)), "never past the total"); + } +} diff --git a/crates/analysis/src/density.rs b/crates/analysis/src/density.rs index fa657e30c..53eccee91 100644 --- a/crates/analysis/src/density.rs +++ b/crates/analysis/src/density.rs @@ -167,7 +167,12 @@ fn accumulate_density( ) where F: Fn(f64, f64) -> f64, { - for &(p, wgt) in points { + crate::control::begin(points.len()); + for (i, &(p, wgt)) in points.iter().enumerate() { + if crate::control::cancelled() { + return; + } + crate::control::reached(i); if wgt <= 0.0 || !wgt.is_finite() || !p.is_finite() { continue; } diff --git a/crates/analysis/src/kriging.rs b/crates/analysis/src/kriging.rs index d2790e899..74ff7b144 100644 --- a/crates/analysis/src/kriging.rs +++ b/crates/analysis/src/kriging.rs @@ -355,6 +355,7 @@ fn kriging_on( let scratch = || (Vec::<(f64, f64, Coord)>::with_capacity(points.len()), KrigingBuffers::new()); let (mut pred_data, mut var_data) = (vec![NODATA; w * h], vec![NODATA; w * h]); let rows = pred_data.chunks_mut(w).zip(var_data.chunks_mut(w)); + crate::control::begin(h); crate::parallel::for_each_row(rows, threads, scratch, |(near, buffers), row, (pred, var): (&mut [f32], &mut [f32])| { let kx = (extent.max_y - (row as f64 + 0.5) * cell_deg).to_radians().cos().abs().max(1e-6); for (col, (pred, var)) in pred.iter_mut().zip(var.iter_mut()).enumerate() { @@ -376,7 +377,7 @@ fn kriging_on( *pred = if val.is_finite() { val as f32 } else { NODATA }; *var = if var_est.is_finite() { var_est as f32 } else { NODATA }; } - }); + })?; let gt = GeoTransform { origin_x: extent.min_x, origin_y: extent.max_y, pixel_width: cell_deg, pixel_height: -cell_deg }; Raster::new(name, w as u32, h as u32, vec![pred_data, var_data], gt, Crs::LonLat, Some(NODATA as f64), ColorHint::Gray, "Float32") diff --git a/crates/analysis/src/lib.rs b/crates/analysis/src/lib.rs index 1aba9f130..cfd2bc1cd 100644 --- a/crates/analysis/src/lib.rs +++ b/crates/analysis/src/lib.rs @@ -11,6 +11,7 @@ #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] pub mod attribute_join; +pub mod control; pub mod convert; pub mod density; pub mod dggs; @@ -415,3 +416,22 @@ pub fn run(id: &str, inputs: &dyn Inputs, params: &Value) -> Result) -> Result { + let _current = control::install(Some(control.clone())); + let res = run(id, inputs, params); + if !control.is_cancelled() { + return res; + } + let name = tool(id).map_or(id, |t| t.name); + Err(match control.progress() { + Some((done, total)) => { + format!("{name} was cancelled {}% of the way through its current step", done.saturating_mul(100) / total.max(1)) + } + None => format!("{name} was cancelled"), + }) +} diff --git a/crates/analysis/src/network.rs b/crates/analysis/src/network.rs index a5c8f51ac..40dec9598 100644 --- a/crates/analysis/src/network.rs +++ b/crates/analysis/src/network.rs @@ -251,7 +251,12 @@ pub fn random_points(ds: &Dataset, rows: &[usize], per: usize, seed: u64) -> Vec (state >> 11) as f64 / (1u64 << 53) as f64 }; let mut out = Vec::new(); - for r in rows { + crate::control::begin(rows.len()); + for (i, r) in rows.iter().enumerate() { + if crate::control::cancelled() { + break; + } + crate::control::reached(i); let Some(Geometry::Polygon(polys)) = ds.features.get(*r).and_then(|f| f.geometry.as_ref()) else { continue }; let e = ds.features.get(*r).and_then(|f| f.geometry.as_ref()).map(|g| g.extent()).unwrap_or_default(); let mut placed = 0; diff --git a/crates/analysis/src/neural.rs b/crates/analysis/src/neural.rs index 50af57f28..612755d04 100644 --- a/crates/analysis/src/neural.rs +++ b/crates/analysis/src/neural.rs @@ -260,7 +260,12 @@ pub fn train( let epochs = epochs.clamp(1, MAX_EPOCHS).min(((MAX_WORK / per_epoch) as usize).max(1)); let (lr, momentum, batch) = (0.05f32, 0.9f32, 32usize); let mut vel: Vec<(Vec, Vec)> = layers.iter().map(|l| (vec![0.0; l.w.len()], vec![0.0; l.b.len()])).collect(); - for _ in 0..epochs { + crate::control::begin(epochs); + for epoch in 0..epochs { + if crate::control::cancelled() { + break; + } + crate::control::reached(epoch); for i in (1..fit.len()).rev() { let j = rng.below(i + 1); fit.swap(i, j); @@ -358,7 +363,10 @@ pub fn classify(r: &Raster, model: &Mlp, nodata: f32) -> Result<(Vec, Vec usize { /// end), so rows that cost more than others are shared out, up to [`BATCH`] when there are many, /// so cheap rows don't wait on the lock. /// Each thread makes its own `scratch()` once; one that panics stops the others after the rows -/// they are making, and its panic is raised again here. +/// they are making, and its panic is raised again here. Each row made counts toward the run's +/// progress; a cancelled run stops after the rows being made, and the rest are left unwritten. pub(crate) fn for_each_row( rows: impl ExactSizeIterator + Send, threads: usize, scratch: impl Fn() -> S + Sync, f: impl Fn(&mut S, usize, T) + Sync, -) { +) -> Result<(), Cancelled> { use std::sync::atomic::{AtomicBool, Ordering}; if threads <= 1 { let mut s = scratch(); - rows.enumerate().for_each(|(i, row)| f(&mut s, i, row)); - return; + for (i, row) in rows.enumerate() { + control::check()?; + f(&mut s, i, row); + control::advance(1); + } + return Ok(()); } + let run = control::current(); let rows = std::sync::Mutex::new(rows.enumerate()); let failed = AtomicBool::new(false); let work = || { let _stop = StopOnPanic(&failed); + let _run = control::install(run.clone()); let mut s = scratch(); let mut taken = Vec::with_capacity(BATCH); - while !failed.load(Ordering::Relaxed) { + while !failed.load(Ordering::Relaxed) && !control::cancelled() { // The lock is held only to take rows; a poisoned one means a thread panicked. if let Ok(mut r) = rows.lock() { let take = (r.len() / threads.saturating_mul(BATCH)).clamp(1, BATCH); @@ -80,9 +89,11 @@ pub(crate) fn for_each_row( if taken.is_empty() { break; } + let n = taken.len(); for (i, row) in taken.drain(..) { f(&mut s, i, row); } + control::advance(n); } }; std::thread::scope(|scope| { @@ -95,6 +106,7 @@ pub(crate) fn for_each_row( } } }); + control::check() } /// `f` on each of `items` with its index, the results in their order: on several threads when @@ -102,8 +114,9 @@ pub(crate) fn for_each_row( /// `scratch()` once and passes it to every `f` it runs, for buffers to reuse; the index lets a /// raster tool map over its rows' numbers. The first error stops the threads after the items /// they are making, and comes back instead of the results: on one thread the first in input -/// order, on several the earliest of those found before they stopped. -pub(crate) fn try_map( +/// order, on several the earliest of those found before they stopped. Each item made counts toward +/// the run's progress; a cancelled run stops likewise, with [`Cancelled`] as its error. +pub(crate) fn try_map>( items: &[T], min_items: usize, scratch: impl Fn() -> S + Sync, @@ -116,7 +129,7 @@ pub(crate) fn try_map( /// until none are left or one has failed, so the work is shared however it is spread, and the /// results go back in their places. A thread that can't start (a limit on threads, say) leaves /// its share to the others; one that panics passes its panic on, as an item made here would. -pub(crate) fn try_map_on( +pub(crate) fn try_map_on>( items: &[T], threads: usize, scratch: impl Fn() -> S + Sync, @@ -125,20 +138,38 @@ pub(crate) fn try_map_on( use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; if threads <= 1 || items.len() <= 1 { let mut s = scratch(); - return items.iter().enumerate().map(|(i, item)| f(&mut s, i, item)).collect(); + return items + .iter() + .enumerate() + .map(|(i, item)| { + control::check()?; + let r = f(&mut s, i, item); + control::advance(1); + r + }) + .collect(); } + let run = control::current(); let next = AtomicUsize::new(0); let failed = AtomicBool::new(false); let work = || { let _stop = StopOnPanic(&failed); + let _run = control::install(run.clone()); let mut s = scratch(); let mut made = Vec::new(); while !failed.load(Ordering::Relaxed) { let from = next.fetch_add(BATCH, Ordering::Relaxed); let Some(batch) = items.get(from..items.len().min(from.saturating_add(BATCH))).filter(|b| !b.is_empty()) else { break }; + if control::cancelled() { + failed.store(true, Ordering::Relaxed); + return Err((from, Cancelled.into())); + } for (k, item) in batch.iter().enumerate() { match f(&mut s, from + k, item) { - Ok(r) => made.push((from + k, r)), + Ok(r) => { + made.push((from + k, r)); + control::advance(1); + } Err(e) => { failed.store(true, Ordering::Relaxed); return Err((from + k, e)); @@ -202,7 +233,7 @@ mod tests { scratch: impl Fn() -> S + Sync, f: impl Fn(&mut S, usize, &T) -> R + Sync, ) -> Vec { - try_map_on(items, threads, scratch, |s, i, item| Ok::(f(s, i, item))).unwrap_or_default() + try_map_on(items, threads, scratch, |s, i, item| Ok::(f(s, i, item))).unwrap_or_default() } fn map( @@ -211,7 +242,7 @@ mod tests { scratch: impl Fn() -> S + Sync, f: impl Fn(&mut S, usize, &T) -> R + Sync, ) -> Vec { - try_map(items, min_items, scratch, |s, i, item| Ok::(f(s, i, item))).unwrap_or_default() + try_map(items, min_items, scratch, |s, i, item| Ok::(f(s, i, item))).unwrap_or_default() } #[test] @@ -285,12 +316,13 @@ mod tests { #[test] fn a_failing_item_stops_the_threads_and_its_error_comes_back() { let items: Vec = (0..1_000).collect(); - let f = |_: &mut (), _: usize, x: &u32| if *x == 500 || *x == 700 { Err(*x) } else { Ok(*x) }; + let f = |_: &mut (), _: usize, x: &u32| if *x == 500 || *x == 700 { Err(x.to_string()) } else { Ok(*x) }; // On one thread, the first in input order. - assert_eq!(try_map_on(&items, 1, || (), f), Err(500)); + assert_eq!(try_map_on(&items, 1, || (), f), Err("500".to_string())); for threads in [2, 3, 16] { - assert!(matches!(try_map_on(&items, threads, || (), f), Err(500 | 700)), "{threads} threads"); - assert_eq!(try_map_on(&items, threads, || (), |_, _, x| Ok::<_, ()>(*x)), Ok(items.clone()), "{threads} threads"); + let e = try_map_on(&items, threads, || (), f).err(); + assert!(matches!(e.as_deref(), Some("500" | "700")), "{threads} threads"); + assert_eq!(try_map_on(&items, threads, || (), |_, _, x| Ok::<_, Cancelled>(*x)), Ok(items.clone()), "{threads} threads"); } // Items still waiting when one fails are left: far fewer than all of them run. let ran = std::sync::atomic::AtomicUsize::new(0); @@ -300,7 +332,7 @@ mod tests { || (), |_, _, x| { ran.fetch_add(1, std::sync::atomic::Ordering::Relaxed); - if *x == 0 { Err(()) } else { Ok(()) } + if *x == 0 { Err(Cancelled) } else { Ok(()) } }, ); assert!(r.is_err()); @@ -312,7 +344,7 @@ mod tests { let alone: Vec = (0..37 * 5).map(|i| i / 5 * 3 + i % 5).collect(); for threads in [1, 2, 5, 16, 64] { let mut out = vec![0usize; 37 * 5]; - for_each_row( + let made = for_each_row( out.chunks_mut(5), threads, || (), @@ -320,17 +352,18 @@ mod tests { row.iter_mut().enumerate().for_each(|(c, o)| *o = r * 3 + c); }, ); + assert_eq!(made, Ok(())); assert_eq!(out, alone, "{threads} threads"); } // Many cheap rows per thread, taken several at a time. let tall: Vec = (0..5_000).collect(); for threads in [2, 16] { let mut out = vec![0usize; 5_000]; - for_each_row(out.chunks_mut(1), threads, || (), |_, r, row: &mut [usize]| row[0] = r); + assert_eq!(for_each_row(out.chunks_mut(1), threads, || (), |_, r, row: &mut [usize]| row[0] = r), Ok(())); assert_eq!(out, tall, "{threads} threads"); } let mut none: Vec = Vec::new(); - for_each_row(none.chunks_mut(5), 4, || (), |_, _, _: &mut [u8]| panic!("no rows")); + assert_eq!(for_each_row(none.chunks_mut(5), 4, || (), |_, _, _: &mut [u8]| panic!("no rows")), Ok(())); // Small work, and the browser: one thread; never more threads than rows. assert_eq!(threads_for_rows(100, MIN_ROW_WORK - 1, 0), 1); assert!(threads_for_rows(3, usize::MAX, 0) <= 3); @@ -365,6 +398,50 @@ mod tests { assert!(ran.load(std::sync::atomic::Ordering::Relaxed) < items.len() / 2, "{}", ran.load(std::sync::atomic::Ordering::Relaxed)); } + #[test] + fn a_cancelled_run_stops_its_threads_and_counts_what_they_made() { + for threads in [1, 4] { + let control = crate::control::Control::new(); + let _run = crate::control::install(Some(control.clone())); + // Rows: the run is cancelled on the 100th; the threads stop after the rows they hold. + let made = std::sync::atomic::AtomicUsize::new(0); + let mut out = vec![0u8; 5_000]; + crate::control::begin(out.len()); + let r = for_each_row( + out.chunks_mut(1), + threads, + || (), + |_, _, row: &mut [u8]| { + if made.fetch_add(1, std::sync::atomic::Ordering::Relaxed) == 99 { + control.cancel(); + } + row[0] = 1; + }, + ); + assert_eq!(r, Err(Cancelled), "{threads} threads"); + let made = made.load(std::sync::atomic::Ordering::Relaxed); + assert!(made < 1_000, "{threads} threads made {made}"); + let (done, total) = control.progress().unwrap_or_default(); + assert!(total == 5_000 && done >= 100 && done <= made as u64, "{threads} threads: {done} of {total}"); + // Items, likewise. + let control2 = crate::control::Control::new(); + let _inner = crate::control::install(Some(control2.clone())); + let items: Vec = (0..5_000).collect(); + let r = try_map_on( + &items, + threads, + || (), + |_, i, x| { + if i == 99 { + control2.cancel(); + } + Ok::<_, String>(*x) + }, + ); + assert_eq!(r.err().as_deref(), Some(crate::control::CANCELLED), "{threads} threads"); + } + } + #[test] fn a_workers_panic_on_a_row_reaches_the_caller_and_stops_the_others() { let caller = std::thread::current().id(); diff --git a/crates/analysis/src/rasterops.rs b/crates/analysis/src/rasterops.rs index 65c7202d4..d83e1bbab 100644 --- a/crates/analysis/src/rasterops.rs +++ b/crates/analysis/src/rasterops.rs @@ -346,6 +346,7 @@ fn viewshed_on(threads: Option, r: &Raster, b: usize, observer: Coord, he let rows = usize::try_from(last - first + 1).unwrap_or(0); let threads = threads.unwrap_or_else(|| crate::parallel::threads_for_rows(rows, usize::try_from(ray_work).unwrap_or(usize::MAX), 0)); let window = out.chunks_mut(w).skip(usize::try_from(first).unwrap_or(0)).take(rows); + crate::control::begin(rows); crate::parallel::for_each_row( window, threads, @@ -384,7 +385,7 @@ fn viewshed_on(threads: Option, r: &Raster, b: usize, observer: Coord, he } } }, - ); + )?; Ok(out) } @@ -431,6 +432,7 @@ fn idw_on( let threads = threads.unwrap_or_else(|| crate::parallel::threads_for_rows(h, w.saturating_mul(h).saturating_mul(points.len()), scratch_bytes)); let mut data = vec![NODATA; w * h]; + crate::control::begin(h); crate::parallel::for_each_row( data.chunks_mut(w), threads, @@ -464,7 +466,7 @@ fn idw_on( *o = if v.is_finite() { v as f32 } else { NODATA }; } }, - ); + )?; let gt = GeoTransform { origin_x: extent.min_x, origin_y: extent.max_y, pixel_width: cell_deg, pixel_height: -cell_deg }; Raster::new(name, w as u32, h as u32, vec![data], gt, Crs::LonLat, Some(NODATA as f64), ColorHint::Gray, "Float32") .map_err(|e| e.to_string()) diff --git a/crates/analysis/src/tools.rs b/crates/analysis/src/tools.rs index 24b8ff04b..31dc4c931 100644 --- a/crates/analysis/src/tools.rs +++ b/crates/analysis/src/tools.rs @@ -6122,6 +6122,49 @@ mod tests { assert_eq!(ends.len(), 2); } + #[test] + fn a_tool_cancelled_while_it_runs_stops_with_how_far_it_came() { + // 2,000 points on 400 × 400 cells: 320 M cell-point pairs, far longer than the watcher takes + // to notice the first rows. + let mut pts = Dataset::new("pts", GeometryType::Point, vec![Field::new("v", FieldType::Double)]); + for i in 0..2_000 { + let (x, y) = (f64::from(i % 50) * 0.2, f64::from(i / 50) * 0.25); + pts.push(Some(Geometry::Point(Coord::new(x, y))), vec![Value::Float(f64::from(i))]); + } + let m = Map(HashMap::from([("pts".to_string(), pts)])); + let params = serde_json::json!({"input": "pts", "field": "v", "extent": "0,0,10,10", "pixelSize": 0.025}); + // Asked to stop before it starts: it never does. + let control = crate::control::Control::new(); + control.cancel(); + assert_eq!( + crate::run_with("idwInterpolation", &m, ¶ms, &control).err().as_deref(), + Some("IDW Interpolation was cancelled 0% of the way through its current step") + ); + // Asked once it has made some rows: it stops well before the end. + let control = crate::control::Control::new(); + let watcher = { + let control = control.clone(); + std::thread::spawn(move || { + let end = std::time::Instant::now() + std::time::Duration::from_secs(60); + while control.progress().is_none_or(|(done, _)| done == 0) && std::time::Instant::now() < end { + std::thread::yield_now(); + } + control.cancel(); + }) + }; + let r = crate::run_with("idwInterpolation", &m, ¶ms, &control); + let _ = watcher.join(); + let e = r.err().unwrap_or_default(); + assert!(e.starts_with("IDW Interpolation was cancelled "), "{e}"); + let (done, total) = control.progress().unwrap_or_default(); + assert!(total == 400 && done < total, "{done} of {total} rows"); + // Without a control nothing changes. + assert!( + crate::run("idwInterpolation", &m, &serde_json::json!({"input": "pts", "field": "v", "extent": "0,0,10,10", "pixelSize": 1.0})) + .is_ok() + ); + } + #[test] fn idw_interpolation_writes_a_raster() { let m = inputs(); diff --git a/crates/analysis/src/topology.rs b/crates/analysis/src/topology.rs index 5f65738c3..028b0c319 100644 --- a/crates/analysis/src/topology.rs +++ b/crates/analysis/src/topology.rs @@ -99,7 +99,12 @@ pub fn overlaps(ds: &Dataset, rows: &[usize]) -> Checked { p.sort_by(|a, b| a.2.min().x.partial_cmp(&b.2.min().x).unwrap_or(std::cmp::Ordering::Equal)); let mut out = Vec::new(); let mut work = 0usize; + crate::control::begin(p.len()); for (i, (ra, a, ba)) in p.iter().enumerate() { + if crate::control::cancelled() { + return Checked::partial(out); + } + crate::control::reached(i); for (rb, b, bb) in p.iter().skip(i + 1) { if bb.min().x > ba.max().x { break; @@ -197,7 +202,12 @@ fn self_crossing(path: &[Coord], closed: bool) -> Option { /// Lines and polygon rings that cross themselves. pub fn self_intersections(ds: &Dataset, rows: &[usize]) -> Checked { let mut out = Vec::new(); - for r in rows { + crate::control::begin(rows.len()); + for (i, r) in rows.iter().enumerate() { + if crate::control::cancelled() { + return Checked::partial(out); + } + crate::control::reached(i); let Some(g) = ds.features.get(*r).and_then(|f| f.geometry.as_ref()) else { continue }; let paths: Vec<(Vec, bool)> = match g { Geometry::Line(parts) => parts.iter().map(|p| (p.clone(), false)).collect(), @@ -240,7 +250,12 @@ pub fn dangles(ds: &Dataset, rows: &[usize], tol_m: f64) -> Checked { // Each part's segments go into the cells their (widened) boxes cover. let mut grid: std::collections::HashMap<(i64, i64), Vec<(usize, usize)>> = std::collections::HashMap::new(); let mut work = 0usize; + crate::control::begin(lines.len()); for (li, (_, parts)) in lines.iter().enumerate() { + if crate::control::cancelled() { + return Checked::partial(Vec::new()); + } + crate::control::reached(li); for (pi, part) in parts.iter().enumerate() { for w in part.windows(2) { let (Some(a), Some(b)) = (w.first(), w.get(1)) else { continue }; @@ -262,7 +277,12 @@ pub fn dangles(ds: &Dataset, rows: &[usize], tol_m: f64) -> Checked { } } let mut out = Vec::new(); + crate::control::begin(lines.len()); for (li, (r, parts)) in lines.iter().enumerate() { + if crate::control::cancelled() { + return Checked::partial(out); + } + crate::control::reached(li); for (pi, part) in parts.iter().enumerate() { // A closed part has no ends. if part.len() < 2 || part.first() == part.last() { @@ -319,7 +339,12 @@ pub fn not_covered(ds: &Dataset, rows: &[usize], by: &Dataset) -> Checked { let all: Vec = (0..by.features.len()).collect(); let polys = polygons(by, &all); let mut out = Vec::new(); - for r in rows { + crate::control::begin(rows.len()); + for (i, r) in rows.iter().enumerate() { + if crate::control::cancelled() { + return Checked::partial(out); + } + crate::control::reached(i); let pts: Vec = match ds.features.get(*r).and_then(|f| f.geometry.as_ref()) { Some(Geometry::Point(c)) => vec![*c], Some(Geometry::MultiPoint(v)) => v.clone(), diff --git a/crates/engine/src/cmd/gp.rs b/crates/engine/src/cmd/gp.rs index dbe1a4937..430ceaee7 100644 --- a/crates/engine/src/cmd/gp.rs +++ b/crates/engine/src/cmd/gp.rs @@ -774,11 +774,22 @@ fn zonal_stats(s: &mut Session, p: &Value) -> CmdResult { /// 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 { + run_tool_with(s, tool, params, &rustgis_analysis::control::Control::new()) +} + +/// [`run_tool`] under `control`: setting its flag from another thread stops the tool at its next +/// check, with an error and nothing applied, and its count follows the tool's progress. +pub fn run_tool_with( + s: &mut Session, + tool: &str, + params: &Value, + control: &std::sync::Arc, +) -> CmdResult { let tool = tool_id(tool).unwrap_or(tool).to_string(); let params = params.clone(); let t0 = now_ms(); let inputs = map_inputs(s, rustgis_analysis::tool(&tool), ¶ms)?; - let mut res = rustgis_analysis::run(&tool, &inputs, ¶ms); + let mut res = rustgis_analysis::run_with(&tool, &inputs, ¶ms, control); if let Ok(result) = &mut res { result.messages.extend(inputs.messages.iter().cloned()); } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 38f58c6a6..bdf6f336e 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -23703,3 +23703,20 @@ fn shapefile_exports_say_which_field_names_they_changed() { assert!(changed(&s, mark).is_empty(), "{:?}", s.notices_since(mark)); std::fs::remove_dir_all(dir).unwrap(); } + +#[test] +fn a_cancelled_tool_leaves_the_map_as_it_was_and_says_so_in_the_history() { + let mut s = world(); + let layers = s.map().layers.len(); + let control = rustgis_analysis::control::Control::new(); + control.cancel(); + let params = json!({"input": "Cities", "field": "pop_max", "pixelSize": 5.0}); + let e = crate::cmd::gp::run_tool_with(&mut s, "idwInterpolation", ¶ms, &control).err().unwrap_or_default(); + assert!(e.starts_with("IDW Interpolation was cancelled"), "{e}"); + assert_eq!(s.map().layers.len(), layers, "nothing was added"); + let last = s.project.gp_history.last().map(|r| (r.ok, r.messages.clone())); + assert_eq!(last, Some((false, vec![e]))); + // The same run, not cancelled, adds its raster. + crate::cmd::gp::run_tool(&mut s, "idwInterpolation", ¶ms).unwrap_or_else(|e| panic!("{e}")); + assert_eq!(s.map().layers.len(), layers + 1); +} From f4e775ae331ff5dd3f53195d1d7de7164de04428 Mon Sep 17 00:00:00 2001 From: Dominik Strebel Date: Mon, 2 Nov 2026 12:10:32 +0100 Subject: [PATCH 3/6] fix(analysis): a run cancelled before it starts doesn't start; buffers count their progress From the review of #1311: - `run_with` returns at once for a control already cancelled, saying the tool was cancelled before it started, instead of entering the tool. - Buffer, Select by Distance and the geodesic buffers set their total before counting, so a cancelled one says how far it came and a finished one shows all its features done, never a total left from an earlier step. Co-Authored-By: Claude Opus 5.5 --- crates/analysis/src/geodesic.rs | 1 + crates/analysis/src/lib.rs | 6 +++++- crates/analysis/src/tools.rs | 16 +++++++++++++++- 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/crates/analysis/src/geodesic.rs b/crates/analysis/src/geodesic.rs index 64ee49b0f..ff03d781c 100644 --- a/crates/analysis/src/geodesic.rs +++ b/crates/analysis/src/geodesic.rs @@ -148,6 +148,7 @@ pub(crate) fn geodesic_buffers_with_limit(geoms: &[&Geometry], metres: f64, limi /// a buffer past the budget stops the others. fn buffers_on(geoms: &[&Geometry], metres: f64, threads: usize, limit: usize) -> Result>, &'static str> { let remaining = std::sync::atomic::AtomicUsize::new(limit); + crate::control::begin(geoms.len()); crate::parallel::try_map_on( geoms, threads, diff --git a/crates/analysis/src/lib.rs b/crates/analysis/src/lib.rs index cfd2bc1cd..69ec709b3 100644 --- a/crates/analysis/src/lib.rs +++ b/crates/analysis/src/lib.rs @@ -420,8 +420,12 @@ pub fn run(id: &str, inputs: &dyn Inputs, params: &Value) -> Result) -> Result { + if control.is_cancelled() { + return Err(format!("{} was cancelled before it started", tool(id).map_or(id, |t| t.name))); + } let _current = control::install(Some(control.clone())); let res = run(id, inputs, params); if !control.is_cancelled() { diff --git a/crates/analysis/src/tools.rs b/crates/analysis/src/tools.rs index 31dc4c931..5d3f52f1c 100644 --- a/crates/analysis/src/tools.rs +++ b/crates/analysis/src/tools.rs @@ -1287,6 +1287,7 @@ fn buffer_rows( ) -> Result, &'static str> { let storage = BufferStorage::new(bytes); let mut output = Vec::new(); + crate::control::begin(geoms.len()); for batch in geoms.chunks(BUFFER_BATCH) { let made = crate::parallel::try_map( batch, @@ -2429,6 +2430,7 @@ fn select_by_location_with_work_limit(inp: &dyn Inputs, p: &Json, limit: usize, // budget; zero-distance selection keeps its existing unbuffered behavior. let storage = BufferStorage::new(if dist > 0.0 { bytes } else { usize::MAX }); let mut selectors = Vec::new(); + crate::control::begin(picked.len()); for batch in picked.chunks(BUFFER_BATCH) { let made = crate::parallel::try_map( batch, @@ -6138,7 +6140,7 @@ mod tests { control.cancel(); assert_eq!( crate::run_with("idwInterpolation", &m, ¶ms, &control).err().as_deref(), - Some("IDW Interpolation was cancelled 0% of the way through its current step") + Some("IDW Interpolation was cancelled before it started") ); // Asked once it has made some rows: it stops well before the end. let control = crate::control::Control::new(); @@ -6165,6 +6167,18 @@ mod tests { ); } + #[test] + fn buffers_count_their_features_toward_the_progress() { + let mut pts = Dataset::new("pts", GeometryType::Point, vec![]); + for i in 0..40 { + pts.push(Some(Geometry::Point(Coord::new(f64::from(i) * 0.1, 0.0))), vec![]); + } + let m = Map(HashMap::from([("pts".to_string(), pts)])); + let control = crate::control::Control::new(); + crate::run_with("buffer", &m, &serde_json::json!({"input": "pts", "distance": 100.0}), &control).unwrap_or_else(|e| panic!("{e}")); + assert_eq!(control.progress(), Some((40, 40))); + } + #[test] fn idw_interpolation_writes_a_raster() { let m = inputs(); From 7bdaeb05a50993c02bc3cb8ccee29603f659a9d1 Mon Sep 17 00:00:00 2001 From: Dominik Strebel Date: Mon, 2 Nov 2026 12:29:27 +0100 Subject: [PATCH 4/6] fix(engine): a tool cancelled before it starts reads none of its inputs From the second review of #1311: - `gp::run_tool_with` checks the control before it reads the tool's inputs (files named among its layers, streamed layers), and keeps the cancelled run in the history as the analysis path does. - `try_map_on` counts its progress a batch at a time on its threads, rather than every thread adding to the shared count for each item. Co-Authored-By: Claude Opus 5.5 --- crates/analysis/src/parallel.rs | 7 +++---- crates/engine/src/cmd/gp.rs | 9 ++++++++- crates/engine/src/tests.rs | 6 ++++++ 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/crates/analysis/src/parallel.rs b/crates/analysis/src/parallel.rs index d85d0fa44..85408220b 100644 --- a/crates/analysis/src/parallel.rs +++ b/crates/analysis/src/parallel.rs @@ -166,16 +166,15 @@ pub(crate) fn try_map_on>( } for (k, item) in batch.iter().enumerate() { match f(&mut s, from + k, item) { - Ok(r) => { - made.push((from + k, r)); - control::advance(1); - } + Ok(r) => made.push((from + k, r)), Err(e) => { failed.store(true, Ordering::Relaxed); return Err((from + k, e)); } } } + // Counted a batch at a time: the threads share one count. + control::advance(batch.len()); } Ok(made) }; diff --git a/crates/engine/src/cmd/gp.rs b/crates/engine/src/cmd/gp.rs index 430ceaee7..9bba22869 100644 --- a/crates/engine/src/cmd/gp.rs +++ b/crates/engine/src/cmd/gp.rs @@ -778,7 +778,8 @@ pub fn run_tool(s: &mut Session, tool: &str, params: &Value) -> CmdResult { } /// [`run_tool`] under `control`: setting its flag from another thread stops the tool at its next -/// check, with an error and nothing applied, and its count follows the tool's progress. +/// check, with an error and nothing applied, and its count follows the tool's progress. One +/// cancelled before it starts reads nothing and runs nothing, and is kept in the history too. pub fn run_tool_with( s: &mut Session, tool: &str, @@ -788,6 +789,12 @@ pub fn run_tool_with( let tool = tool_id(tool).unwrap_or(tool).to_string(); let params = params.clone(); let t0 = now_ms(); + if control.is_cancelled() { + let spec = rustgis_analysis::tool(&tool); + let name = spec.map(|t| t.name).unwrap_or("Tool").to_string(); + let e = format!("{name} was cancelled before it started"); + return apply(s, Ran { name, id: tool, command: None, started: t0 }, recorded(spec, params), Err(e), 0); + } let inputs = map_inputs(s, rustgis_analysis::tool(&tool), ¶ms)?; let mut res = rustgis_analysis::run_with(&tool, &inputs, ¶ms, control); if let Ok(result) = &mut res { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index bdf6f336e..402c5e626 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -23716,6 +23716,12 @@ fn a_cancelled_tool_leaves_the_map_as_it_was_and_says_so_in_the_history() { assert_eq!(s.map().layers.len(), layers, "nothing was added"); let last = s.project.gp_history.last().map(|r| (r.ok, r.messages.clone())); assert_eq!(last, Some((false, vec![e]))); + // Nothing is read for it either: a missing file among its inputs isn't reached. + let missing = std::env::temp_dir().join(format!("rustgis-no-such-{}.geojson", std::process::id())); + let merge = json!({"inputs": [missing.display().to_string(), "Cities"]}); + let e = crate::cmd::gp::run_tool_with(&mut s, "merge", &merge, &control).err().unwrap_or_default(); + assert_eq!(e, "Merge was cancelled before it started"); + assert!(crate::cmd::gp::run_tool(&mut s, "merge", &merge).is_err(), "not cancelled, the missing file is read"); // The same run, not cancelled, adds its raster. crate::cmd::gp::run_tool(&mut s, "idwInterpolation", ¶ms).unwrap_or_else(|e| panic!("{e}")); assert_eq!(s.map().layers.len(), layers + 1); From ae43817f84fbf51dd06d6c2c8358901bbf70ba98 Mon Sep 17 00:00:00 2001 From: Dominik Strebel Date: Mon, 2 Nov 2026 12:46:35 +0100 Subject: [PATCH 5/6] fix(analysis): no stale or finished-looking progress; Select by Location stops between features From CodeRabbit's review of #1311: - `run_with` starts each run with no count, so a control used before doesn't show that run's progress for a tool that doesn't count. - Buffer with dissolve ends the buffers' count before the union, which isn't counted, so a cancel during the union doesn't say "100% of the way". - Select by Location checks the control between the features it selects from, and counts them. - The engine test compares the layers before and after a cancelled run, and checks that the uncancelled merge fails on the missing file itself. Co-Authored-By: Claude Opus 5.5 --- crates/analysis/src/lib.rs | 2 ++ crates/analysis/src/tools.rs | 8 ++++++++ crates/engine/src/tests.rs | 6 +++++- 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/crates/analysis/src/lib.rs b/crates/analysis/src/lib.rs index 69ec709b3..93563a769 100644 --- a/crates/analysis/src/lib.rs +++ b/crates/analysis/src/lib.rs @@ -427,6 +427,8 @@ pub fn run_with(id: &str, inputs: &dyn Inputs, params: &Value, control: &std::sy return Err(format!("{} was cancelled before it started", tool(id).map_or(id, |t| t.name))); } let _current = control::install(Some(control.clone())); + // No count until the tool starts one: a control used before doesn't show that run's. + control::begin(0); let res = run(id, inputs, params); if !control.is_cancelled() { return res; diff --git a/crates/analysis/src/tools.rs b/crates/analysis/src/tools.rs index 5d3f52f1c..a7f2a1b41 100644 --- a/crates/analysis/src/tools.rs +++ b/crates/analysis/src/tools.rs @@ -1357,6 +1357,8 @@ fn buffer_with_limits(inp: &dyn Inputs, p: &Json, limit: usize, bytes: usize) -> } if !all.is_empty() { out.fields.clear(); + // The union isn't counted: the buffers' count ends here rather than stand at its end. + crate::control::begin(0); let u = union_all(all.iter().flat_map(|m| m.0.iter())); if u.coords_count() > limit { return Err(VERTEX_BUDGET_ERROR.into()); @@ -2462,7 +2464,10 @@ fn select_by_location_with_work_limit(inp: &dyn Inputs, p: &Json, limit: usize, let bounded_checks = dist > 0.0 && selectors.len() > ExtentIndex::MAX_LINEAR_FEATURES; let mut remaining_checks = checks; let mut hits = Vec::new(); + crate::control::begin(ds.features.len()); for (r, feature) in ds.features.iter().enumerate() { + crate::control::check()?; + crate::control::reached(r); let Some(g) = feature.geometry.as_ref() else { continue }; let gg = to_geo(g); let ext = merc_extent(&gg); @@ -6177,6 +6182,9 @@ mod tests { let control = crate::control::Control::new(); crate::run_with("buffer", &m, &serde_json::json!({"input": "pts", "distance": 100.0}), &control).unwrap_or_else(|e| panic!("{e}")); assert_eq!(control.progress(), Some((40, 40))); + // The same control for a tool that doesn't count shows no count, not the buffers'. + crate::run_with("convexHull", &m, &serde_json::json!({"input": "pts"}), &control).unwrap_or_else(|e| panic!("{e}")); + assert_eq!(control.progress(), None); } #[test] diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 402c5e626..06dacf0ba 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -23708,12 +23708,14 @@ fn shapefile_exports_say_which_field_names_they_changed() { fn a_cancelled_tool_leaves_the_map_as_it_was_and_says_so_in_the_history() { let mut s = world(); let layers = s.map().layers.len(); + let before = serde_json::to_value(&s.map().layers).unwrap(); let control = rustgis_analysis::control::Control::new(); control.cancel(); let params = json!({"input": "Cities", "field": "pop_max", "pixelSize": 5.0}); let e = crate::cmd::gp::run_tool_with(&mut s, "idwInterpolation", ¶ms, &control).err().unwrap_or_default(); assert!(e.starts_with("IDW Interpolation was cancelled"), "{e}"); assert_eq!(s.map().layers.len(), layers, "nothing was added"); + assert_eq!(serde_json::to_value(&s.map().layers).unwrap(), before, "no layer changed"); let last = s.project.gp_history.last().map(|r| (r.ok, r.messages.clone())); assert_eq!(last, Some((false, vec![e]))); // Nothing is read for it either: a missing file among its inputs isn't reached. @@ -23721,7 +23723,9 @@ fn a_cancelled_tool_leaves_the_map_as_it_was_and_says_so_in_the_history() { let merge = json!({"inputs": [missing.display().to_string(), "Cities"]}); let e = crate::cmd::gp::run_tool_with(&mut s, "merge", &merge, &control).err().unwrap_or_default(); assert_eq!(e, "Merge was cancelled before it started"); - assert!(crate::cmd::gp::run_tool(&mut s, "merge", &merge).is_err(), "not cancelled, the missing file is read"); + // Not cancelled, the missing file is read, and that is what fails. + let e = crate::cmd::gp::run_tool(&mut s, "merge", &merge).err().unwrap_or_default(); + assert!(e.starts_with(&missing.display().to_string()), "{e}"); // The same run, not cancelled, adds its raster. crate::cmd::gp::run_tool(&mut s, "idwInterpolation", ¶ms).unwrap_or_else(|e| panic!("{e}")); assert_eq!(s.map().layers.len(), layers + 1); From 49cd9b4eaab7fbec5c45333ee30e95d207601249 Mon Sep 17 00:00:00 2001 From: Dominik Strebel Date: Mon, 2 Nov 2026 12:59:39 +0100 Subject: [PATCH 6/6] fix(analysis): a finished run's count is complete; no union after a cancel From the fourth review round of #1311: - A run that finishes uncancelled marks its last counted step all done, so a loop that counts each item as it reaches it no longer ends one short (Select by Location, kernel density, the topology checks, random points, neural training and classification). - Buffer with dissolve checks the control before the union, which can't be interrupted once it starts. - The buffer progress test covers dissolve (the union isn't counted) and a Select by Location run (every feature counted at the end). Co-Authored-By: Claude Opus 5.5 --- crates/analysis/src/control.rs | 10 ++++++++++ crates/analysis/src/lib.rs | 5 +++++ crates/analysis/src/tools.rs | 9 +++++++++ 3 files changed, 24 insertions(+) diff --git a/crates/analysis/src/control.rs b/crates/analysis/src/control.rs index 0acbdf57a..35da091ab 100644 --- a/crates/analysis/src/control.rs +++ b/crates/analysis/src/control.rs @@ -114,6 +114,16 @@ pub(crate) fn reached(done: usize) { }); } +/// Counts every item of the step done: the tool has finished it (a loop counting with [`reached`] +/// otherwise ends one short). +pub(crate) fn finish() { + CURRENT.with(|c| { + if let Some(c) = c.borrow().as_ref() { + c.done.store(c.total.load(Ordering::Relaxed), Ordering::Relaxed); + } + }); +} + /// Counts `n` more items of the step done. pub(crate) fn advance(n: usize) { CURRENT.with(|c| { diff --git a/crates/analysis/src/lib.rs b/crates/analysis/src/lib.rs index 93563a769..55133f575 100644 --- a/crates/analysis/src/lib.rs +++ b/crates/analysis/src/lib.rs @@ -431,6 +431,11 @@ pub fn run_with(id: &str, inputs: &dyn Inputs, params: &Value, control: &std::sy control::begin(0); let res = run(id, inputs, params); if !control.is_cancelled() { + // Finished: its last step is all done, however it counted. A failed one stays where it + // stopped. + if res.is_ok() { + control::finish(); + } return res; } let name = tool(id).map_or(id, |t| t.name); diff --git a/crates/analysis/src/tools.rs b/crates/analysis/src/tools.rs index a7f2a1b41..6257b4edb 100644 --- a/crates/analysis/src/tools.rs +++ b/crates/analysis/src/tools.rs @@ -1357,6 +1357,7 @@ fn buffer_with_limits(inp: &dyn Inputs, p: &Json, limit: usize, bytes: usize) -> } if !all.is_empty() { out.fields.clear(); + crate::control::check()?; // The union isn't counted: the buffers' count ends here rather than stand at its end. crate::control::begin(0); let u = union_all(all.iter().flat_map(|m| m.0.iter())); @@ -6185,6 +6186,14 @@ mod tests { // The same control for a tool that doesn't count shows no count, not the buffers'. crate::run_with("convexHull", &m, &serde_json::json!({"input": "pts"}), &control).unwrap_or_else(|e| panic!("{e}")); assert_eq!(control.progress(), None); + // Dissolved, the union after the buffers isn't counted: the buffers' count ends before it. + let dissolve = serde_json::json!({"input": "pts", "distance": 100.0, "dissolve": "All"}); + crate::run_with("buffer", &m, &dissolve, &control).unwrap_or_else(|e| panic!("{e}")); + assert_eq!(control.progress(), None); + // A loop counting each feature as it reaches it ends with all of them counted. + let near = serde_json::json!({"input": "pts", "select": "pts", "distance": 10.0}); + crate::run_with("selectByLocation", &m, &near, &control).unwrap_or_else(|e| panic!("{e}")); + assert_eq!(control.progress(), Some((40, 40))); } #[test]