From daabbe1c6cba24694e21ea09f61e6f570ade1b1b Mon Sep 17 00:00:00 2001 From: Dominik Strebel Date: Thu, 5 Nov 2026 15:36:05 +0100 Subject: [PATCH 1/6] feat(ui): Add Data and Load All read their file off the frame (#1291 step 3) On the desktop, Add Data's whole-file reads (a raster, one GDAL converts, a GeoJSON, delimited text or a spreadsheet) and Load All Features read on a thread of their own, with progress and Cancel in the status bar. The command then runs as before and takes what was read instead of reading again, so what it adds, says, logs and puts in Undo is unchanged. Files dropped together read one at a time and land in order; Cancel or another project opened leaves them, and says so. --- apps/rustgis/src/main.rs | 2 + crates/engine/src/cmd/layer.rs | 127 ++++++++-- crates/engine/src/cmd/mod.rs | 1 + crates/engine/src/cmd/raster.rs | 42 +++- crates/engine/src/cmd/reads.rs | 81 +++++++ crates/engine/src/lib.rs | 10 + crates/engine/src/tests.rs | 57 +++++ crates/ui-egui/src/chrome.rs | 6 + crates/ui-egui/src/layerchooser.rs | 15 +- crates/ui-egui/src/lib.rs | 26 +- crates/ui-egui/src/panes.rs | 4 + crates/ui-egui/src/readjob.rs | 373 +++++++++++++++++++++++++++++ docs/user-guide/data.md | 10 + 13 files changed, 729 insertions(+), 25 deletions(-) create mode 100644 crates/engine/src/cmd/reads.rs create mode 100644 crates/ui-egui/src/readjob.rs diff --git a/apps/rustgis/src/main.rs b/apps/rustgis/src/main.rs index 202cd8cf4..48ce13895 100644 --- a/apps/rustgis/src/main.rs +++ b/apps/rustgis/src/main.rs @@ -512,6 +512,8 @@ fn main() -> eframe::Result { let app = { let mut app = app; app.ui.reserved_shortcuts = menu::reserved(); + // Add Data and Load All read on a thread of their own, so a large file doesn't hold the window. + app.reads_off_frame = true; app }; let mut app = app.with_services(services); diff --git a/crates/engine/src/cmd/layer.rs b/crates/engine/src/cmd/layer.rs index 755cadf7b..5f323b387 100644 --- a/crates/engine/src/cmd/layer.rs +++ b/crates/engine/src/cmd/layer.rs @@ -1371,14 +1371,10 @@ fn add_data(s: &mut Session, p: &Value) -> CmdResult { } // A spreadsheet: one sheet ("layer"), read like delimited text. if rustgis_data::sheets::is_sheet_name(path) { - let meta = std::fs::metadata(path).map_err(|e| format!("{path}: {e}"))?; - if meta.len() > super::raster::MAX_UPLOAD_BYTES as u64 { - return Err(format!("{path} is too large to open")); - } - let bytes = std::fs::read(path).map_err(|e| format!("{path}: {e}"))?; - // Read from disk: the table keeps the workbook's path, so an export can't replace it. - let mut read = read_sheet(p, path, &bytes)?; - read.0.source = Some(std::path::absolute(path).map_or_else(|_| path.to_string(), |a| a.display().to_string())); + let read = match super::reads::take(s, &whole_key(path)) { + Some(read) => read, + None => read_sheet_file(p, path), + }?; return add_read(s, p, read, None); } #[cfg(not(target_arch = "wasm32"))] @@ -1394,6 +1390,36 @@ fn add_data(s: &mut Session, p: &Value) -> CmdResult { let full = std::path::absolute(path).map(|p| p.display().to_string()).unwrap_or_else(|_| path.to_string()); return add_layers(s, p, &full, &bytes, None); } + let read = match super::reads::take(s, &whole_key(path)) { + Some(read) => read, + None => read_whole(p, path), + }?; + add_read(s, p, read, None) +} + +/// A file Add Data reads whole: the features, and what reading them said. +type WholeRead = Result<(rustgis_data::Dataset, rustgis_data::ReadReport), String>; + +/// The key Add Data's whole read of `path` is taken by (see [`super::reads`]). +fn whole_key(path: &str) -> String { + format!("whole:{path}") +} + +/// A spreadsheet's sheet ("layer"), read like delimited text. +fn read_sheet_file(p: &Value, path: &str) -> WholeRead { + let meta = std::fs::metadata(path).map_err(|e| format!("{path}: {e}"))?; + if meta.len() > super::raster::MAX_UPLOAD_BYTES as u64 { + return Err(format!("{path} is too large to open")); + } + let bytes = std::fs::read(path).map_err(|e| format!("{path}: {e}"))?; + // Read from disk: the table keeps the workbook's path, so an export can't replace it. + let mut read = read_sheet(p, path, &bytes)?; + read.0.source = Some(std::path::absolute(path).map_or_else(|_| path.to_string(), |a| a.display().to_string())); + Ok(read) +} + +/// Delimited text with the options `p` gives, or another file its format's reader reads whole. +fn read_whole(p: &Value, path: &str) -> WholeRead { let delimited = read_delimited(p, path, || { let meta = std::fs::metadata(path).map_err(|e| format!("{path}: {e}"))?; // The limit the text readers keep without options too. @@ -1402,11 +1428,58 @@ fn add_data(s: &mut Session, p: &Value) -> CmdResult { } std::fs::read(path).map_err(|e| format!("{path}: {e}")) })?; - let read = match delimited { - Some(read) => read, - None => rustgis_data::io::read_path(Path::new(path)).map_err(|e| format!("{path}: {e}"))?, - }; - add_read(s, p, read, None) + match delimited { + Some(read) => Ok(read), + None => rustgis_data::io::read_path(Path::new(path)).map_err(|e| format!("{path}: {e}")), + } +} + +/// Add Data's read of the file `p` names, made before it runs ([`super::reads`]): a raster read +/// whole or converted by GDAL, a GeoJSON read whole, delimited text or a spreadsheet. `None` for +/// any other file, which [`add_data`] reads as it adds it. Each is picked only where `add_data` +/// takes that read, so a read made is never wasted; a few it makes whole are left to it (a GeoJSON +/// with `"stream"` given, a `.json` with a coordinate system or layer given, a large GeoJSON that +/// isn't a FeatureCollection). +#[cfg(not(target_arch = "wasm32"))] +pub(crate) fn add_data_ahead(p: &Value) -> Option { + use super::reads::ahead; + const NAME: &str = "Add Data"; + let path = gdb_path(ps(p, "path")?).into_owned(); + let scan = super::raster::wants_scan(p, &path).ok()?; + if !scan && rustgis_data::photo::is_photo_name(&path) { + return None; + } + if rustgis_data::raster::is_raster_name(&path) { + let key = super::raster::read_key(&path); + return Some(ahead(NAME, key, move || super::raster::read_raster(&path))); + } + if rustgis_data::raster::zarr::is_store(Path::new(&path)) { + return None; + } + if rustgis_data::raster::gdal::is_gdal_raster_name(&path) { + let text = |k: &str| ps(p, k).map(str::trim).filter(|v| !v.is_empty()).map(str::to_string); + let (key, layer, crs) = (super::raster::gdal_key(&path), text("layer"), text("crs")); + return Some(ahead(NAME, key, move || super::raster::read_gdal(&path, layer.as_deref(), crs.as_deref()))); + } + let given = |k: &str| ps(p, k).is_some_and(|v| !v.trim().is_empty()); + let lower = path.to_ascii_lowercase(); + // A GeoJSON read whole: small, with no "stream" given (which add_data decides by), and not a + // CityJSON model (named so, or a .json with a coordinate system or a layer given). + let geojson = is_geojson(&path) + && !rustgis_data::cityjson::is_cityjson_name(&path) + && b(p, "stream").is_none() + && !(lower.ends_with(".json") && (given("crs") || given("layer"))) + && std::fs::metadata(&path).is_ok_and(|m| m.is_file() && m.len() <= rustgis_data::source::geojson::IN_PLACE_ABOVE); + let (key, q) = (whole_key(&path), p.clone()); + if rustgis_data::sheets::is_sheet_name(&path) { + return Some(ahead(NAME, key, move || read_sheet_file(&q, &path))); + } + (geojson || is_delimited_name(&path)).then(|| ahead(NAME, key, move || read_whole(&q, &path))) +} + +#[cfg(target_arch = "wasm32")] +pub(crate) fn add_data_ahead(_: &Value) -> Option { + None } /// Adds every layer of a file that holds several: the Shapefiles (and other files) of a zip, a KML @@ -2622,8 +2695,14 @@ fn define_projection(s: &mut Session, p: &Value) -> CmdResult { /// Loads a streamed layer's features into memory, so it can be edited like any other layer. fn load_all(s: &mut Session, p: &Value) -> CmdResult { let id = s.layer_id(p)?; + let key = load_all_key(s.map().layer(id).ok_or("no such layer")?); + let ahead = super::reads::take::(s, &key); let l = s.map().layer(id).ok_or("no such layer")?; - let ds = l.load_all()?; + let ds = match ahead { + // Read from the source the layer still reads: one opened afresh since is read again. + Some((copied, ds)) if l.source.as_ref().is_some_and(|src| src.same_opening(&copied)) => ds?, + _ => l.load_all()?, + }; let n = ds.features.len(); let l = s.map_mut().layer_mut(id).ok_or("no such layer")?; l.dataset = ds; @@ -2634,6 +2713,26 @@ fn load_all(s: &mut Session, p: &Value) -> CmdResult { Ok(json!({"layer": id, "features": n})) } +/// Load All's read made before it runs: the source it read, and what it read. +type LoadedAll = (rustgis_map::source::SourceLayer, Result); + +/// The key Load All's read of `l` is taken by (see [`super::reads`]). +fn load_all_key(l: &rustgis_map::Layer) -> String { + format!("loadAll:{}", l.instance_id()) +} + +/// Load All's read of the streamed layer `p` names, made before it runs ([`super::reads`]), on a +/// copy of the layer. +pub(crate) fn load_all_ahead(s: &Session, p: &Value) -> Option { + let l = s.map().layer(s.layer_id(p).ok()?)?; + let mut copy = l.clone(); + let source = copy.source.as_mut()?; + // Its own read: the live layer keeps the one under way. + source.detach_read(); + let copied = source.clone(); + Some(super::reads::ahead("Load All Features", load_all_key(l), move || (copied, copy.load_all()))) +} + /// Adds a layer file: a layer with its data, symbology and labels. fn add_layer_file(s: &mut Session, path: &str) -> CmdResult { let bytes = crate::project::read_file(Path::new(path), 512 << 20)?; diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index 30126d162..4849dc4d1 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -38,6 +38,7 @@ pub mod presentation; pub mod project; pub mod qgis; pub mod raster; +pub mod reads; pub mod repair; pub mod report; pub mod s3; diff --git a/crates/engine/src/cmd/raster.rs b/crates/engine/src/cmd/raster.rs index 9223e2fae..57c525583 100644 --- a/crates/engine/src/cmd/raster.rs +++ b/crates/engine/src/cmd/raster.rs @@ -536,7 +536,10 @@ pub(crate) fn say_losses(s: &mut Session, losses: Vec<(String, String)>) { /// Opens a raster file by path. pub fn add_path(s: &mut Session, path: &str, name: Option<&str>, scan: bool) -> CmdResult { - let mut r = raster::read_path(Path::new(path)).map_err(|e| format!("{path}: {e}"))?; + let mut r = match super::reads::take(s, &read_key(path)) { + Some(r) => r, + None => read_raster(path), + }?; if scan { r = r.into_scan().map_err(|e| format!("{path}: {e}"))?; } @@ -547,14 +550,45 @@ pub fn add_path(s: &mut Session, path: &str, name: Option<&str>, scan: bool) -> add_raster(s, &name, RasterLayer::from_path(r, full)) } +/// The raster at `path`, read whole. +pub(crate) fn read_raster(path: &str) -> Result { + raster::read_path(Path::new(path)).map_err(|e| format!("{path}: {e}")) +} + +/// The key [`add_path`]'s read of `path` is taken by (see [`super::reads`]). +pub(crate) fn read_key(path: &str) -> String { + format!("raster:{path}") +} + +/// A raster GDAL converts and the conversion it read. +#[cfg(not(target_arch = "wasm32"))] +type GdalRead = Result<(rustgis_data::raster::gdal::Converted, Raster), String>; + +/// The raster GDAL converts from `path` (`layer` its subdataset, `crs` a coordinate system the +/// file doesn't say), with the conversion. +#[cfg(not(target_arch = "wasm32"))] +pub(crate) fn read_gdal(path: &str, layer: Option<&str>, crs: Option<&str>) -> GdalRead { + use rustgis_data::raster::gdal; + let c = gdal::convert(Path::new(path), layer, crs).map_err(|e| format!("{path}: {e}"))?; + let r = raster::read_converted(Path::new(path), &c).map_err(|e| format!("{path}: {e}"))?; + Ok((c, r)) +} + +/// The key [`add_gdal_path`]'s read of `path` is taken by (see [`super::reads`]). +#[cfg(not(target_arch = "wasm32"))] +pub(crate) fn gdal_key(path: &str) -> String { + format!("gdal:{path}") +} + /// Adds a raster GDAL reads (converted once to a GeoTIFF in the cache): `"layer"` picks a /// subdataset (a NetCDF variable) and `"crs"` gives a coordinate system the file doesn't say. #[cfg(not(target_arch = "wasm32"))] pub(crate) fn add_gdal_path(s: &mut Session, p: &Value, path: &str) -> CmdResult { - use rustgis_data::raster::gdal; let text = |k: &str| ps(p, k).map(str::trim).filter(|v| !v.is_empty()); - let c = gdal::convert(Path::new(path), text("layer"), text("crs")).map_err(|e| format!("{path}: {e}"))?; - let r = raster::read_converted(Path::new(path), &c).map_err(|e| format!("{path}: {e}"))?; + let (c, r) = match super::reads::take::(s, &gdal_key(path)) { + Some(read) => read, + None => read_gdal(path, text("layer"), text("crs")), + }?; if c.assumed_lonlat { s.notify(format!("{path} doesn't say its coordinate system; its coordinates look like longitude/latitude, so it's read as WGS 84 (give \"crs\" if not)"), false); } diff --git a/crates/engine/src/cmd/reads.rs b/crates/engine/src/cmd/reads.rs new file mode 100644 index 000000000..d0523e689 --- /dev/null +++ b/crates/engine/src/cmd/reads.rs @@ -0,0 +1,81 @@ +//! Reads the desktop app makes on a thread of its own before a command adds what they read +//! (#1291 step 3): Add Data's whole-file reads (a raster, a raster GDAL converts, a GeoJSON, +//! delimited text or spreadsheet read whole) and Load All Features. [`read_ahead`] takes what +//! the read needs of the session, [`ReadAhead::run`] reads on the thread, and the app then runs +//! the command as it would have with the read [`Read::placed`] in the session: the command takes +//! it instead of reading again, so what it adds, says, logs and puts in Undo is the same as when +//! it reads here. A script or the command line reads where the command runs. + +use std::any::Any; + +use serde_json::Value; + +use crate::Session; + +/// What a read gave, for the command it was made for: kept in the session ([`Read::placed`]) +/// while that command runs, and taken by the site that would have read it. +pub struct Read { + key: String, + value: Box, +} + +impl Read { + /// Puts the read in the session for the next command to take, until [`Session::drop_read`]. + pub fn placed(self, s: &mut Session) { + s.read_ahead = Some(self); + } +} + +/// A read started for a command, to run on a thread of its own. +pub struct ReadAhead { + /// What the read is called, for its progress and a cancelled run's error. + pub name: &'static str, + key: String, + work: Box Box + Send>, +} + +impl ReadAhead { + /// Reads, blocking until it's done; `control` stops it and counts its progress (see + /// [`rustgis_analysis::control`]). A read that fails gives its failure to the command, which + /// says it as it would have; only a cancelled run (or a bug in a reader) is an error here. + pub fn run(self, control: &std::sync::Arc) -> Result { + let ReadAhead { name, key, work } = self; + let mut value = None; + // A bug in a reader must not take the app down. + let res = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + rustgis_analysis::run_under(name, control, || { + value = Some(work()); + Ok(()) + }) + })) + .unwrap_or_else(|_| Err(format!("{name} failed unexpectedly"))); + res?; + let value = value.ok_or_else(|| format!("{name} read nothing"))?; + Ok(Read { key, value }) + } +} + +/// The read `command` makes with `p`, when the app makes it on a thread of its own before the +/// command runs: `None` for one it runs as it is (a layer read in place, a web service, a file +/// the command reads differently). +pub fn read_ahead(s: &Session, command: &str, p: &Value) -> Option { + match command { + "layer.addData" => super::layer::add_data_ahead(p), + "layer.loadAll" => super::layer::load_all_ahead(s, p), + _ => None, + } +} + +/// `work` as a read for the command to take under `key`. +pub(crate) fn ahead(name: &'static str, key: String, work: impl FnOnce() -> T + Send + 'static) -> ReadAhead { + ReadAhead { name, key, work: Box::new(move || Box::new(work()) as Box) } +} + +/// The read placed for `key`, taken: `None` when none was, so the caller reads here. +pub(crate) fn take(s: &mut Session, key: &str) -> Option { + if s.read_ahead.as_ref().is_none_or(|r| r.key != key) { + return None; + } + let read = s.read_ahead.take()?; + read.value.downcast::().ok().map(|v| *v) +} diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index e6e9ce830..92dcf24dc 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -750,6 +750,9 @@ pub struct Session { pub zarr_off_frame: bool, /// The Zarr steps landed that [`Session::follow_time_landed`] has seen. zarr_seen: u64, + /// A read the app made on a thread of its own for the command it runs next, which takes it + /// instead of reading again ([`cmd::reads`]). + pub(crate) read_ahead: Option, /// Set where a person at a terminal waits on a command (`rustgis-cli run` with stdin and /// stderr a terminal), who can stop it with Ctrl-C: tools and rasters take no work cap /// ([`rustgis_analysis::control::Control::stoppable`]). A program that calls it (an agent, a @@ -1003,6 +1006,7 @@ impl Session { long_runs: None, zarr_off_frame: false, zarr_seen: 0, + read_ahead: None, stoppable: false, }; for text in label_notices { @@ -1143,6 +1147,12 @@ impl Session { self.zarr_off_frame && cmd::zarr::waiting(self) } + /// Drops a read placed for a command ([`cmd::reads::Read::placed`]) that the command didn't + /// take: one refused before it read, or that read another way. + pub fn drop_read(&mut self) { + self.read_ahead = None; + } + /// Shows the Zarr steps read off the frame ([`Session::zarr_off_frame`]) once they land: the /// app asks each frame. True when one landed, so the map draws again. pub fn follow_time_landed(&mut self) -> bool { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 44e8624b3..084e074ea 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -28491,3 +28491,60 @@ fn raster_classes_run_on_another_thread() { assert_eq!((&r["model"], &r["replaced"]), (&json!("scene_model 2"), &json!(false)), "{r}"); assert_eq!(s.project.dl_models.iter().map(|m| m.name.as_str()).collect::>(), ["scene_model", "scene_model 2"]); } + +/// Add Data's and Load All's reads made before the command runs (the desktop app's, on a thread +/// of their own) are the ones the command takes: it adds what they read, without reading again. +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn reads_made_ahead_are_taken_by_the_command() { + use base64::Engine; + use rustgis_analysis::control::Control; + let dir = std::env::temp_dir().join(format!("rustgis-reads-ahead-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let points = r#"{"type":"FeatureCollection","features":[{"type":"Feature","properties":{"n":1},"geometry":{"type":"Point","coordinates":[8,47]}},{"type":"Feature","properties":{"n":2},"geometry":{"type":"Point","coordinates":[9,46]}}]}"#; + let files = [ + ("points.geojson", points.as_bytes().to_vec()), + ("places.csv", b"name,x,y\nA,8,47\nB,9,46\nC,10,45\n".to_vec()), + ("places.xlsx", rustgis_data::sheets::xlsx(&[("Places", &[&["name", "longitude", "latitude"], &["A", "10", "20"]])])), + ("utm.tif", base64::engine::general_purpose::STANDARD.decode(utm_tiff_b64()).unwrap()), + ]; + let mut s = Session::new(Project::default()); + for (name, bytes) in files { + let path = dir.join(name); + std::fs::write(&path, bytes).unwrap(); + let p = json!({"path": path.display().to_string(), "zoom": false}); + let ahead = crate::cmd::reads::read_ahead(&s, "layer.addData", &p).unwrap_or_else(|| panic!("{name} is read ahead")); + let read = ahead.run(&Control::new()).unwrap(); + // The file gone: what the command adds can only come from the read. + std::fs::remove_file(&path).unwrap(); + read.placed(&mut s); + let r = s.run("layer.addData", &p).unwrap_or_else(|e| panic!("{name}: {e}")); + assert!(s.read_ahead.is_none(), "{name}: the read was taken"); + assert!(r.get("layer").is_some() || r.get("layers").is_some(), "{name}: {r}"); + } + assert_eq!(s.map().layers.len(), 4, "{:?}", s.map().layers.iter().map(|l| &l.name).collect::>()); + // A file read in place (and any the command reads differently) reads as it runs. + let gpkg = json!({"path": dir.join("table.gpkg").display().to_string()}); + assert!(crate::cmd::reads::read_ahead(&s, "layer.addData", &gpkg).is_none()); + + // Load All's read, on a copy of the streamed layer. + let path = dir.join("streamed.geojson"); + std::fs::write(&path, points).unwrap(); + let id = s.run("layer.addData", &json!({"path": path.display().to_string(), "stream": true})).unwrap()["layer"].as_u64().unwrap(); + assert!(s.map().layer(id as u32).unwrap().is_streamed()); + let p = json!({"layer": id}); + let read = crate::cmd::reads::read_ahead(&s, "layer.loadAll", &p).unwrap().run(&Control::new()).unwrap(); + read.placed(&mut s); + assert_eq!(s.run("layer.loadAll", &p).unwrap()["features"], 2); + assert!(s.read_ahead.is_none()); + assert!(!s.map().layer(id as u32).unwrap().is_streamed()); + + // A cancelled read gives nothing to add, and says so. + let path = dir.join("again.csv"); + std::fs::write(&path, "name,x,y\nA,8,47\n").unwrap(); + let ahead = crate::cmd::reads::read_ahead(&s, "layer.addData", &json!({"path": path.display().to_string()})).unwrap(); + let control = Control::new(); + control.cancel(); + assert!(ahead.run(&control).err().unwrap().contains("cancelled")); + let _ = std::fs::remove_dir_all(&dir); +} diff --git a/crates/ui-egui/src/chrome.rs b/crates/ui-egui/src/chrome.rs index f5135070b..0ca1b3065 100644 --- a/crates/ui-egui/src/chrome.rs +++ b/crates/ui-egui/src/chrome.rs @@ -345,6 +345,12 @@ pub fn status_bar(app: &mut RustGisApp, ui: &mut Ui) { sep(ui, &t); crate::exportjob::status(app, ui, &t); } + // A file read for Add Data, or a layer's features for Load All. + #[cfg(not(target_arch = "wasm32"))] + if crate::readjob::showing(app) { + sep(ui, &t); + crate::readjob::status(app, ui, &t); + } ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { // The CPU/GPU backend toggle, then the frame rate (live while the map moves, // "— fps" when idle), then the last render's cost. diff --git a/crates/ui-egui/src/layerchooser.rs b/crates/ui-egui/src/layerchooser.rs index d8d1f31c3..a9c2868ff 100644 --- a/crates/ui-egui/src/layerchooser.rs +++ b/crates/ui-egui/src/layerchooser.rs @@ -54,7 +54,7 @@ pub fn add_file(app: &mut RustGisApp, path: &str) -> Result { return app.run_by_user(command, json!({"path": path})); } if !(app.ui.ask_layers && lists_layers(path)) { - return app.run("layer.addData", json!({"path": path})); + return add(app, json!({"path": path})); } // Listed off the frame: a geodatabase's or GDAL's listing can take a while. let slot: crate::Slot> = crate::Slot::default(); @@ -70,11 +70,20 @@ pub fn add_file(app: &mut RustGisApp, path: &str) -> Result { Ok(json!({"choosing": path})) } +/// Adds a file, read on a thread of its own when the desktop app reads it so ([`crate::readjob`]). +fn add(app: &mut RustGisApp, p: Value) -> Result { + #[cfg(not(target_arch = "wasm32"))] + return crate::readjob::run(app, "layer.addData", p); + #[cfg(target_arch = "wasm32")] + app.run("layer.addData", p) +} + /// Adds the layers ticked, each by name (a failure is told, by the command, and the rest still /// added). fn add_chosen(app: &mut RustGisApp, path: &str, layers: &[String]) { + // After any file read before them, in turn. for layer in layers { - let _ = app.run("layer.addData", json!({"path": path, "layer": layer})); + let _ = add(app, json!({"path": path, "layer": layer})); } app.map.invalidate(); } @@ -99,7 +108,7 @@ pub fn show(app: &mut RustGisApp, ctx: &egui::Context) { let path = c.path.clone(); app.ui.choosing.remove(0); // A failure is told by the command. - let _ = app.run("layer.addData", json!({"path": path})); + let _ = add(app, json!({"path": path})); app.map.invalidate(); return; } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 1f35a3fe3..c787797ab 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -66,6 +66,8 @@ mod pluginstart; mod pluginsui; mod portalsui; #[cfg(not(target_arch = "wasm32"))] +mod readjob; +#[cfg(not(target_arch = "wasm32"))] mod relayshot; mod repairsource; mod reportpane; @@ -945,6 +947,12 @@ pub struct RustGisApp { /// An export writing its file on its own thread (see [`exportjob`]). #[cfg(not(target_arch = "wasm32"))] export_job: Option, + /// Add Data's and Load All's reads on their own thread, and those waiting (see [`readjob`]). + #[cfg(not(target_arch = "wasm32"))] + reads: readjob::Reads, + /// Add Data and Load All Features read on a thread of their own (the desktop app; see + /// [`readjob`]). Off, as for tests and the examples, they read as the command runs. + pub reads_off_frame: bool, /// ModelBuilder's view: the model on its canvas. pub mb: modelbuilder::ModelBuilder, pub ws: workspacepanes::WorkspaceState, @@ -1159,6 +1167,9 @@ impl RustGisApp { workflow: analysispane::Workflow::default(), #[cfg(not(target_arch = "wasm32"))] export_job: None, + #[cfg(not(target_arch = "wasm32"))] + reads: readjob::Reads::default(), + reads_off_frame: false, mb: modelbuilder::ModelBuilder::default(), ws: workspacepanes::WorkspaceState::default(), time: timebar::TimeBarState::default(), @@ -2017,6 +2028,7 @@ impl RustGisApp { #[cfg(not(target_arch = "wasm32"))] pub fn end_exports(&mut self) { exportjob::stop(self, std::time::Duration::from_secs(5)); + readjob::stop(self, std::time::Duration::from_secs(5)); } /// Whether this shell opens files by path (the desktop app; the web app picks them in the @@ -4661,11 +4673,17 @@ impl RustGisApp { fieldmenu::play(self, &ctx); chrome::shortcuts(self, &ctx); panes::follow_layer_filter(self); - // A Zarr summary, a trend, a workflow or an export running on its thread lands whatever is - // in view, the File menu's backstage too, so a project saved there holds it (all are looked - // at: `|`, not `||`), and the frames go on while one runs so it's taken when it ends. + // A Zarr summary, a trend, a workflow, an export or a read running on its thread lands + // whatever is in view, the File menu's backstage too, so a project saved there holds it + // (all are looked at: `|`, not `||`), and the frames go on while one runs so it's taken + // when it ends. #[cfg(not(target_arch = "wasm32"))] - if workspacepanes::land_summary(self) | analysispane::land_trend(self) | analysispane::land_job(self) | exportjob::land(self) { + if workspacepanes::land_summary(self) + | analysispane::land_trend(self) + | analysispane::land_job(self) + | exportjob::land(self) + | readjob::land(self) + { ctx.request_repaint_after(std::time::Duration::from_millis(200)); } if self.ui.backstage { diff --git a/crates/ui-egui/src/panes.rs b/crates/ui-egui/src/panes.rs index 23183ce3e..f4cb14b64 100644 --- a/crates/ui-egui/src/panes.rs +++ b/crates/ui-egui/src/panes.rs @@ -2230,6 +2230,10 @@ fn layer_menu(app: &mut RustGisApp, ui: &mut Ui, l: &Layer, reorder: bool) { ui.close(); } if l.is_streamed() && ui.button(tl!("Load All Features")).clicked() { + // Read on a thread of its own in the desktop app (see `readjob`). + #[cfg(not(target_arch = "wasm32"))] + let _ = crate::readjob::run(app, "layer.loadAll", json!({"layer": l.id})); + #[cfg(target_arch = "wasm32")] let _ = app.run("layer.loadAll", json!({"layer": l.id})); ui.close(); } diff --git a/crates/ui-egui/src/readjob.rs b/crates/ui-egui/src/readjob.rs new file mode 100644 index 000000000..986e45fd4 --- /dev/null +++ b/crates/ui-egui/src/readjob.rs @@ -0,0 +1,373 @@ +//! Add Data's whole-file reads and Load All Features on the desktop read on a thread of their own +//! (#1291 step 3), so a large raster, GeoJSON or table, or a streamed layer loaded whole, doesn't +//! hold the window: the status bar shows the read under way, how far it has come and Cancel, and +//! [`land`] then runs the command, which takes what was read ([`reads`]) and adds it as it does +//! when it reads here. Reads go one at a time, in the order asked for: files dropped together +//! wait for the one before, and so does a file added while one is read, so the layers come in +//! that order. + +use std::collections::VecDeque; +use std::sync::Arc; + +use egui::Ui; +use rustgis_engine::analysis::control::Control; +use rustgis_engine::cmd::reads; +use serde_json::{Value, json}; + +use crate::RustGisApp; +use crate::theme::Tokens; + +/// The reads under way and waiting. +#[derive(Default)] +pub struct Reads { + job: Option, + /// The commands waiting for the read before, in order. + waiting: VecDeque, +} + +/// A command asked for, with where it was asked: the project and map then open and, for Load All, +/// the layer. Another project or map open by the time it runs, or the layer gone, leaves it. +struct Asked { + command: &'static str, + params: Value, + /// What it reads, as its notices name it: the file, or the layer. + what: String, + opened: u64, + map: u64, + layer: Option, +} + +/// A read under way: what it is for, its Cancel and progress, and what it read once done. +struct ReadJob { + asked: Asked, + name: &'static str, + control: Arc, + slot: crate::Slot>, +} + +/// Runs `command` (`layer.addData` or `layer.loadAll`) with `p`: its read on a thread of its own +/// when the desktop app makes it so ([`reads::read_ahead`]), after any read under way; here and +/// now otherwise, as the command does. While one is under way or waiting it answers that, not +/// what the command adds, which comes as it lands. +pub fn run(app: &mut RustGisApp, command: &'static str, p: Value) -> Result { + if !app.reads_off_frame { + return app.run(command, p); + } + let layer = (command == "layer.loadAll") + .then(|| app.session.layer_id(&p).ok().and_then(|id| app.session.map().layer(id)).map(|l| (l.instance_id(), l.name.clone()))) + .flatten(); + // Load All asked again for a layer it is reading already. + if let Some((instance, _)) = &layer + && app.reads.job.iter().map(|j| &j.asked).chain(&app.reads.waiting).any(|a| a.layer == Some(*instance)) + { + return Ok(json!({"reading": instance})); + } + let what = match &layer { + Some((_, name)) => name.clone(), + None => p.get("path").and_then(Value::as_str).map(|path| file_name(path).to_string()).unwrap_or_default(), + }; + let asked = Asked { + command, + params: p, + what, + opened: app.session.opened, + map: app.session.map().instance_id(), + layer: layer.map(|(instance, _)| instance), + }; + if busy(app) { + app.reads.waiting.push_back(asked); + return Ok(json!({"waiting": app.reads.waiting.len()})); + } + match placed(app, &asked) { + Ok(p) => start(app, asked, p), + Err(why) => Err(why.to_string()), + } +} + +/// A path's last part, as a notice names the file. +fn file_name(path: &str) -> &str { + std::path::Path::new(path).file_name().and_then(|n| n.to_str()).unwrap_or(path) +} + +/// Whether a read is under way or waiting. +fn busy(app: &RustGisApp) -> bool { + app.reads.job.is_some() || !app.reads.waiting.is_empty() +} + +/// Why a command asked for can't run now: what was open when it was asked isn't any more. +#[derive(Clone, Copy, PartialEq)] +enum Gone { + Project, + Map, + Layer, +} + +impl std::fmt::Display for Gone { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(match self { + Gone::Project => tl!("another project was opened"), + Gone::Map => tl!("the map it was asked for isn't open"), + Gone::Layer => tl!("its layer was removed"), + }) + } +} + +/// `asked`'s parameters for the command now (Load All's layer by its id in the open map), or why +/// it can't run. +fn placed(app: &RustGisApp, asked: &Asked) -> Result { + if app.session.opened != asked.opened { + return Err(Gone::Project); + } + if app.session.map().instance_id() != asked.map { + return Err(Gone::Map); + } + let Some(instance) = asked.layer else { return Ok(asked.params.clone()) }; + let l = app.session.map().layers.iter().find(|l| l.instance_id() == instance).ok_or(Gone::Layer)?; + let mut p = asked.params.clone(); + p["layer"] = json!(l.id); + Ok(p) +} + +/// Starts `asked`'s read with `p`, or runs it here when it reads nothing ahead (or its thread +/// can't start). +fn start(app: &mut RustGisApp, asked: Asked, p: Value) -> Result { + let Some(ahead) = reads::read_ahead(&app.session, asked.command, &p) else { return app.run(asked.command, p) }; + let name = ahead.name; + let control = Control::stoppable(); + let (ctl, slot) = (control.clone(), crate::Slot::default()); + let out = slot.clone(); + // A streamed layer's source read on the thread keeps its requests in the session's diagnostics. + let diagnostics = app.session.diagnostics.capture(); + let spawned = std::thread::Builder::new().name(name.into()).spawn(move || { + let _diagnostics = diagnostics.enter(); + let read = ahead.run(&ctl); + // Filled even when its lock is poisoned, or the read would show as running forever. + *out.lock().unwrap_or_else(std::sync::PoisonError::into_inner) = Some(read); + }); + if let Err(e) = spawned { + log::warn!("{name} reads here: its thread didn't start: {e}"); + return app.run(asked.command, p); + } + app.reads.job = Some(ReadJob { asked, name, control, slot }); + Ok(json!({"started": name})) +} + +/// Runs the command a read was for once it's done (or says why it wasn't), then starts the next +/// waiting: called every frame. `true` while one is under way or waiting. +pub fn land(app: &mut RustGisApp) -> bool { + if !busy(app) { + return false; + } + let mut left: Vec<(String, Gone)> = Vec::new(); + if let Some(job) = &app.reads.job { + match placed(app, &job.asked) { + // What it was for is gone: it stops, and is said with the rest below. + Err(why) => { + job.control.cancel(); + if let Some(job) = app.reads.job.take() { + left.push((job.asked.what, why)); + } + } + Ok(p) => { + let done = job.slot.lock().unwrap_or_else(std::sync::PoisonError::into_inner).take(); + let Some(done) = done else { return true }; + let Some(job) = app.reads.job.take() else { return false }; + if job.control.is_cancelled() { + // Cancelled (once it had read too): it adds nothing, and nor does what waits. + app.session.notify( + crate::i18n::fmt("{tool} of {what} was cancelled", &[("tool", tl_raw!(job.name)), ("what", &job.asked.what)]), + false, + ); + let n = std::mem::take(&mut app.reads.waiting).len(); + if n == 1 { + app.session.notify(tl!("The one waiting after it was left too").to_string(), false); + } else if n > 1 { + app.session.notify(crate::i18n::fmt("The {n} waiting after it were left too", &[("n", &n.to_string())]), false); + } + } else { + match done { + Ok(read) => { + read.placed(&mut app.session); + // A failure is said by the command. + let _ = app.run(job.asked.command, p); + app.session.drop_read(); + app.map.invalidate(); + } + // A bug in a reader: said, and the next goes on. + Err(e) => app.session.notify(format!("{}: {e}", job.asked.what), true), + } + } + } + } + } + // The next waiting, until one reads on its thread. + while app.reads.job.is_none() { + let Some(asked) = app.reads.waiting.pop_front() else { break }; + match placed(app, &asked) { + // A failure is said by the command. + Ok(p) => { + let _ = start(app, asked, p); + app.map.invalidate(); + } + Err(why) => left.push((asked.what, why)), + } + } + // Those left, one notice for each reason, with a count, naming the first. + for why in [Gone::Project, Gone::Map, Gone::Layer] { + let of: Vec<&String> = left.iter().filter(|(_, w)| *w == why).map(|(what, _)| what).collect(); + let reason = why.to_string(); + match of.as_slice() { + [] => {} + [what] => app.session.notify(crate::i18n::fmt("{what} was left: {why}", &[("what", what), ("why", &reason)]), false), + [first, ..] => app.session.notify( + crate::i18n::fmt( + "{n} files or layers were left, the first {what}: {why}", + &[("n", &of.len().to_string()), ("what", first), ("why", &reason)], + ), + false, + ), + } + } + busy(app) +} + +/// Cancels the read under way, and what waits, and waits up to `wait` for its thread to stop. +pub fn stop(app: &mut RustGisApp, wait: std::time::Duration) { + app.reads.waiting.clear(); + let Some(job) = app.reads.job.take() else { return }; + job.control.cancel(); + let end = std::time::Instant::now() + wait; + while job.slot.lock().unwrap_or_else(std::sync::PoisonError::into_inner).is_none() && std::time::Instant::now() < end { + std::thread::sleep(std::time::Duration::from_millis(10)); + } +} + +/// Whether the status bar shows a read. +pub fn showing(app: &RustGisApp) -> bool { + app.reads.job.is_some() +} + +/// The read under way in the status bar: its name, progress, how many wait, and Cancel. +pub fn status(app: &RustGisApp, ui: &mut Ui, t: &Tokens) { + let Some(job) = &app.reads.job else { return }; + ui.spinner(); + // Two labels, not one sentence put together from translated pieces. + ui.label(egui::RichText::new(tl!("Running…")).size(11.5).color(t.text_dim)); + ui.label(egui::RichText::new(tl!(job.name)).size(11.5).strong()); + // How far its current step has come, when its reader counts. + if let Some((done, total)) = job.control.progress() { + ui.add(egui::ProgressBar::new(done as f32 / total.max(1) as f32).show_percentage().desired_width(120.0).desired_height(12.0)); + } + let n = app.reads.waiting.len(); + if n > 0 { + ui.label(egui::RichText::new(crate::i18n::fmt("{n} waiting", &[("n", &n.to_string())])).size(11.5).color(t.text_dim)); + } + if ui.add_enabled(!job.control.is_cancelled(), egui::Button::new(tl!("Cancel")).small()).clicked() { + job.control.cancel(); + } +} + +#[cfg(test)] +#[allow(clippy::unwrap_used)] +mod tests { + use super::*; + + /// Lands every read under way and waiting, failing past `secs`. + fn landed(app: &mut RustGisApp, secs: u64) { + let end = std::time::Instant::now() + std::time::Duration::from_secs(secs); + while land(app) { + assert!(std::time::Instant::now() < end, "the reads never landed"); + std::thread::sleep(std::time::Duration::from_millis(5)); + } + } + + #[test] + fn files_added_read_on_a_thread_and_land_in_order() { + let mut app = RustGisApp::new(rustgis_engine::Session::new(rustgis_engine::Project::default())); + app.reads_off_frame = true; + let dir = std::env::temp_dir().join(format!("rustgis-read-job-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let file = |name: &str| { + let path = dir.join(name); + std::fs::write(&path, "name,x,y\nA,8,47\nB,9,46\n").unwrap(); + path.display().to_string() + }; + let (first, second, third) = (file("first.csv"), file("second.csv"), file("third.csv")); + assert_eq!(crate::layerchooser::add_file(&mut app, &first).unwrap()["started"], "Add Data"); + // One at a time: the next waits for it, and lands after it. + assert_eq!(crate::layerchooser::add_file(&mut app, &second).unwrap()["waiting"], 1); + assert_eq!(crate::layerchooser::add_file(&mut app, &third).unwrap()["waiting"], 2); + assert!(app.session.map().layers.is_empty(), "nothing is added before its read lands"); + landed(&mut app, 60); + let names: Vec<&str> = app.session.map().layers.iter().map(|l| l.name.as_str()).collect(); + assert_eq!(names.len(), 3, "{names:?}"); + assert!(names.iter().all(|n| ["first", "second", "third"].contains(n)), "{names:?}"); + // The layer list's order: the last added on top, as when each reads as it's added. + let mut again = RustGisApp::new(rustgis_engine::Session::new(rustgis_engine::Project::default())); + for path in [&first, &second, &third] { + crate::layerchooser::add_file(&mut again, path).unwrap(); + } + let order: Vec<&str> = again.session.map().layers.iter().map(|l| l.name.as_str()).collect(); + assert_eq!(names, order); + // Undo takes the last back, as for one added here. + app.run("edit.undo", Value::Null).unwrap(); + assert_eq!(app.session.map().layers.len(), 2); + + // Cancel leaves the one under way and what waits, and says so. + let before = app.session.map().layers.len(); + crate::layerchooser::add_file(&mut app, &first).unwrap(); + crate::layerchooser::add_file(&mut app, &second).unwrap(); + app.reads.job.as_ref().unwrap().control.cancel(); + landed(&mut app, 60); + assert_eq!(app.session.map().layers.len(), before); + assert!(app.session.notices.iter().any(|n| n.text == "Add Data of first.csv was cancelled"), "{:?}", app.session.notices.last()); + assert!(app.session.notices.iter().any(|n| n.text == "The one waiting after it was left too"), "{:?}", app.session.notices.last()); + + // Another project opened meanwhile: they aren't added to it. + crate::layerchooser::add_file(&mut app, &first).unwrap(); + crate::layerchooser::add_file(&mut app, &second).unwrap(); + app.run("project.new", Value::Null).unwrap(); + landed(&mut app, 60); + assert!(app.session.map().layers.is_empty()); + assert!( + app.session.notices.iter().any(|n| n.text == "2 files or layers were left, the first first.csv: another project was opened") + ); + + // Closing the app cancels a read under way and waits for it. + crate::layerchooser::add_file(&mut app, &first).unwrap(); + stop(&mut app, std::time::Duration::from_secs(60)); + assert!(!busy(&app)); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn load_all_reads_on_a_thread() { + let mut app = RustGisApp::new(rustgis_engine::Session::new(rustgis_engine::Project::default())); + app.reads_off_frame = true; + let dir = std::env::temp_dir().join(format!("rustgis-read-job-all-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("streamed.geojson"); + std::fs::write(&path, r#"{"type":"FeatureCollection","features":[{"type":"Feature","properties":{},"geometry":{"type":"Point","coordinates":[8,47]}}]}"#) + .unwrap(); + let id = app.run("layer.addData", json!({"path": path.display().to_string(), "stream": true})).unwrap()["layer"].as_u64().unwrap() + as u32; + assert_eq!(run(&mut app, "layer.loadAll", json!({"layer": id})).unwrap()["started"], "Load All Features"); + assert!(app.session.map().layer(id).unwrap().is_streamed()); + landed(&mut app, 60); + let l = app.session.map().layer(id).unwrap(); + assert!(!l.is_streamed() && l.dataset.features.len() == 1); + + // Asked twice, it reads once; another map open when it lands leaves it, and says so. + let id = app.run("layer.addData", json!({"path": path.display().to_string(), "stream": true})).unwrap()["layer"].as_u64().unwrap() + as u32; + run(&mut app, "layer.loadAll", json!({"layer": id})).unwrap(); + assert!(run(&mut app, "layer.loadAll", json!({"layer": id})).unwrap().get("reading").is_some()); + assert_eq!(app.reads.waiting.len(), 0); + app.run("map.new", json!({"name": "Other"})).unwrap(); + landed(&mut app, 60); + let notice = format!("{} was left: the map it was asked for isn't open", app.session.project.maps[0].layer(id).unwrap().name); + assert!(app.session.notices.iter().any(|n| n.text == notice), "{:?}", app.session.notices.last()); + assert!(app.session.project.maps[0].layer(id).unwrap().is_streamed()); + let _ = std::fs::remove_dir_all(&dir); + } +} diff --git a/docs/user-guide/data.md b/docs/user-guide/data.md index 625edfc86..3515ceebd 100644 --- a/docs/user-guide/data.md +++ b/docs/user-guide/data.md @@ -46,6 +46,13 @@ menu to add every layer, as before. Scripts pick one with `layer.addData`'s `"la `info.fileLayers` lists them. On the desktop, **Home › Add › Add Data › Add Folder…** connects a folder to the Browser; the web app does not offer folder connections. +On the desktop, a file Add Data reads whole (a GeoTIFF or another raster, one GDAL converts, a +GeoJSON of up to 32 MB, delimited text or a spreadsheet) is read in the background, so the map +and the rest of the window stay in use: the status bar shows **Add Data**, how many files wait +after it, and **Cancel**. Files dropped or added together are read one at a time and added in +that order. Cancel leaves the file being read and those waiting; opening another project, or +switching to another map, leaves them too, and a message says which. A file read in place, a web +service or a database adds as before, and scripts read the file where the command runs. **Home › Add › Add Data › Delimited Text…** asks for a CSV or a spreadsheet first, then lists its columns to pick the x and y (or WKT) ones from, with the coordinate system (such as `EPSG:2263`) and the separator (changing it reads the columns again); `info.fileColumns` lists them for scripts. Add Data finds them itself @@ -871,6 +878,9 @@ evenly spread sample of them and its tooltip in Layers says so: zoom in to see t [Data formats](data-formats.md)). Smaller files load whole; `layer.addData` with `"stream": true` or `false` overrides the choice. +On the desktop, **Load All Features** from the layer's menu in Layers reads its features in the +background, with **Cancel** in the status bar; the layer stays streamed until they are all read. + On the desktop, opening a large Shapefile for the first time scans its record headers to build a disposable SQLite index outside the dataset directory. Reopening an unchanged file reuses that index; its canonical path, size and modification time identify the cached generation. From 090c6159cfb23d0dfb8966c5b06c52d88d6672ac Mon Sep 17 00:00:00 2001 From: Dominik Strebel Date: Thu, 5 Nov 2026 16:44:08 +0100 Subject: [PATCH 2/6] fix(ui): reads off the frame on every desktop platform; review fixes - reads_off_frame was set inside the macOS-only block: Linux and Windows read on the frame. - Load All says how many rows couldn't be read, and keeps the layer's name if it was renamed while the read ran. - A GeoJSON read whole ahead keeps that route even if the file grew past the in-place size. - A read whose project, map or layer went is cancelled but keeps its place until its thread ends, so reads still go one at a time. - Picking the read runs under a panic guard; a panic there reads where the command runs. - Docs: a file that asks which layers to add joins the order once chosen. --- apps/rustgis/src/main.rs | 4 ++-- crates/engine/src/cmd/layer.rs | 22 +++++++++++++++------- crates/engine/src/cmd/reads.rs | 6 ++++++ crates/ui-egui/src/readjob.rs | 33 +++++++++++++++++++-------------- docs/user-guide/data.md | 2 +- 5 files changed, 43 insertions(+), 24 deletions(-) diff --git a/apps/rustgis/src/main.rs b/apps/rustgis/src/main.rs index 48ce13895..c6d0595f2 100644 --- a/apps/rustgis/src/main.rs +++ b/apps/rustgis/src/main.rs @@ -512,11 +512,11 @@ fn main() -> eframe::Result { let app = { let mut app = app; app.ui.reserved_shortcuts = menu::reserved(); - // Add Data and Load All read on a thread of their own, so a large file doesn't hold the window. - app.reads_off_frame = true; app }; let mut app = app.with_services(services); + // Add Data and Load All read on a thread of their own, so a large file doesn't hold the window. + app.reads_off_frame = true; // Settings › Start-up; `--sample` still opens the sample. let blank = if args.iter().any(|a| a == "--sample") { Some(false) } else { (!files.is_empty()).then_some(true) }; if let Err(e) = app.start(&rustgis_ui_egui::Start { blank, after_crash: unclean, ..Default::default() }) { diff --git a/crates/engine/src/cmd/layer.rs b/crates/engine/src/cmd/layer.rs index 5f323b387..8f46538ed 100644 --- a/crates/engine/src/cmd/layer.rs +++ b/crates/engine/src/cmd/layer.rs @@ -1286,6 +1286,8 @@ fn add_data(s: &mut Session, p: &Value) -> CmdResult { #[cfg(not(target_arch = "wasm32"))] if is_geojson(path) && !rustgis_data::cityjson::is_cityjson_name(path) + // One read whole ahead keeps that route, though the file grew since. + && !super::reads::placed_for(s, &whole_key(path)) && b(p, "stream").unwrap_or_else(|| { std::fs::metadata(path).is_ok_and(|m| m.is_file() && m.len() > rustgis_data::source::geojson::IN_PLACE_ABOVE) }) @@ -2698,12 +2700,17 @@ fn load_all(s: &mut Session, p: &Value) -> CmdResult { let key = load_all_key(s.map().layer(id).ok_or("no such layer")?); let ahead = super::reads::take::(s, &key); let l = s.map().layer(id).ok_or("no such layer")?; - let ds = match ahead { + let (mut ds, unread) = match ahead { // Read from the source the layer still reads: one opened afresh since is read again. - Some((copied, ds)) if l.source.as_ref().is_some_and(|src| src.same_opening(&copied)) => ds?, - _ => l.load_all()?, + Some((copied, read)) if l.source.as_ref().is_some_and(|src| src.same_opening(&copied)) => read?, + _ => l.load_all_counted()?, }; - let n = ds.features.len(); + // Named as the layer is now: it may have been renamed while the read ran. + ds.name = l.dataset.name.clone(); + let (n, name) = (ds.features.len(), l.name.clone()); + if unread > 0 { + s.warn(format!("{name}: {unread} rows couldn't be read and were left out")); + } let l = s.map_mut().layer_mut(id).ok_or("no such layer")?; l.dataset = ds; l.dataset.windowed = false; @@ -2713,8 +2720,9 @@ fn load_all(s: &mut Session, p: &Value) -> CmdResult { Ok(json!({"layer": id, "features": n})) } -/// Load All's read made before it runs: the source it read, and what it read. -type LoadedAll = (rustgis_map::source::SourceLayer, Result); +/// Load All's read made before it runs: the source it read, and what it read with the number of +/// rows that couldn't be read. +type LoadedAll = (rustgis_map::source::SourceLayer, Result<(rustgis_data::Dataset, usize), String>); /// The key Load All's read of `l` is taken by (see [`super::reads`]). fn load_all_key(l: &rustgis_map::Layer) -> String { @@ -2730,7 +2738,7 @@ pub(crate) fn load_all_ahead(s: &Session, p: &Value) -> Option(s: &mut Session, key: &str) -> Option { let read = s.read_ahead.take()?; read.value.downcast::().ok().map(|v| *v) } + +/// Whether a read is placed for `key`, without taking it. +#[cfg(not(target_arch = "wasm32"))] +pub(crate) fn placed_for(s: &Session, key: &str) -> bool { + s.read_ahead.as_ref().is_some_and(|r| r.key == key) +} diff --git a/crates/ui-egui/src/readjob.rs b/crates/ui-egui/src/readjob.rs index 986e45fd4..acaa4e02d 100644 --- a/crates/ui-egui/src/readjob.rs +++ b/crates/ui-egui/src/readjob.rs @@ -40,6 +40,9 @@ struct Asked { /// A read under way: what it is for, its Cancel and progress, and what it read once done. struct ReadJob { asked: Asked, + /// What it was for went while it read: cancelled, it keeps its place until its thread ends + /// (a reader that doesn't stop for Cancel still runs), so reads still go one at a time. + gone: Option, name: &'static str, control: Arc, slot: crate::Slot>, @@ -131,7 +134,9 @@ fn placed(app: &RustGisApp, asked: &Asked) -> Result { /// Starts `asked`'s read with `p`, or runs it here when it reads nothing ahead (or its thread /// can't start). fn start(app: &mut RustGisApp, asked: Asked, p: Value) -> Result { - let Some(ahead) = reads::read_ahead(&app.session, asked.command, &p) else { return app.run(asked.command, p) }; + // Picked on this thread, so a bug in the picking must not take the app down: it reads here. + let picked = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| reads::read_ahead(&app.session, asked.command, &p))); + let Ok(Some(ahead)) = picked else { return app.run(asked.command, p) }; let name = ahead.name; let control = Control::stoppable(); let (ctl, slot) = (control.clone(), crate::Slot::default()); @@ -148,7 +153,7 @@ fn start(app: &mut RustGisApp, asked: Asked, p: Value) -> Result log::warn!("{name} reads here: its thread didn't start: {e}"); return app.run(asked.command, p); } - app.reads.job = Some(ReadJob { asked, name, control, slot }); + app.reads.job = Some(ReadJob { asked, gone: None, name, control, slot }); Ok(json!({"started": name})) } @@ -159,19 +164,19 @@ pub fn land(app: &mut RustGisApp) -> bool { return false; } let mut left: Vec<(String, Gone)> = Vec::new(); + // What it was for is gone: it is cancelled, and said with the rest below once it stops. + let gone = app.reads.job.as_ref().filter(|j| j.gone.is_none()).and_then(|j| placed(app, &j.asked).err()); + if let (Some(why), Some(job)) = (gone, app.reads.job.as_mut()) { + job.control.cancel(); + job.gone = Some(why); + } if let Some(job) = &app.reads.job { - match placed(app, &job.asked) { - // What it was for is gone: it stops, and is said with the rest below. - Err(why) => { - job.control.cancel(); - if let Some(job) = app.reads.job.take() { - left.push((job.asked.what, why)); - } - } - Ok(p) => { - let done = job.slot.lock().unwrap_or_else(std::sync::PoisonError::into_inner).take(); - let Some(done) = done else { return true }; - let Some(job) = app.reads.job.take() else { return false }; + let done = job.slot.lock().unwrap_or_else(std::sync::PoisonError::into_inner).take(); + let Some(done) = done else { return true }; + let Some(job) = app.reads.job.take() else { return false }; + match (job.gone, placed(app, &job.asked)) { + (Some(why), _) | (None, Err(why)) => left.push((job.asked.what, why)), + (None, Ok(p)) => { if job.control.is_cancelled() { // Cancelled (once it had read too): it adds nothing, and nor does what waits. app.session.notify( diff --git a/docs/user-guide/data.md b/docs/user-guide/data.md index 3515ceebd..db728770d 100644 --- a/docs/user-guide/data.md +++ b/docs/user-guide/data.md @@ -50,7 +50,7 @@ On the desktop, a file Add Data reads whole (a GeoTIFF or another raster, one GD GeoJSON of up to 32 MB, delimited text or a spreadsheet) is read in the background, so the map and the rest of the window stay in use: the status bar shows **Add Data**, how many files wait after it, and **Cancel**. Files dropped or added together are read one at a time and added in -that order. Cancel leaves the file being read and those waiting; opening another project, or +that order; one that asks which layers to add joins them once you choose. Cancel leaves the file being read and those waiting; opening another project, or switching to another map, leaves them too, and a message says which. A file read in place, a web service or a database adds as before, and scripts read the file where the command runs. **Home › Add › Add Data › Delimited Text…** asks for a CSV or a spreadsheet first, then lists its columns to pick the x and y (or WKT) From a66869b4b66adbe069bd2f170c1dde770ea63722 Mon Sep 17 00:00:00 2001 From: Dominik Strebel Date: Thu, 5 Nov 2026 17:16:57 +0100 Subject: [PATCH 3/6] fix(engine): Load All reads again when the layer's fields changed during its read A field added or deleted while a Load All read ran would have been lost or brought back by the prepared read; it is now used only while the layer lists the same fields. Tests cover the skipped-rows warning both ways, read ahead and here. --- crates/engine/src/cmd/layer.rs | 10 ++++++++-- crates/engine/src/tests.rs | 31 +++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/layer.rs b/crates/engine/src/cmd/layer.rs index 8f46538ed..ea9484bab 100644 --- a/crates/engine/src/cmd/layer.rs +++ b/crates/engine/src/cmd/layer.rs @@ -2701,8 +2701,14 @@ fn load_all(s: &mut Session, p: &Value) -> CmdResult { let ahead = super::reads::take::(s, &key); let l = s.map().layer(id).ok_or("no such layer")?; let (mut ds, unread) = match ahead { - // Read from the source the layer still reads: one opened afresh since is read again. - Some((copied, read)) if l.source.as_ref().is_some_and(|src| src.same_opening(&copied)) => read?, + // Read from the source the layer still reads, with the fields it lists now: one opened + // afresh since, or a field added or deleted meanwhile, is read again. + Some((copied, read)) + if l.source.as_ref().is_some_and(|src| src.same_opening(&copied)) + && read.as_ref().is_ok_and(|(ds, _)| l.dataset.fields.is_empty() || ds.fields == l.dataset.fields) => + { + read? + } _ => l.load_all_counted()?, }; // Named as the layer is now: it may have been renamed while the read ran. diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 084e074ea..407728434 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -28538,6 +28538,37 @@ fn reads_made_ahead_are_taken_by_the_command() { assert_eq!(s.run("layer.loadAll", &p).unwrap()["features"], 2); assert!(s.read_ahead.is_none()); assert!(!s.map().layer(id as u32).unwrap().is_streamed()); + let unread = |s: &Session| s.notices.iter().filter(|n| n.text.contains("couldn't be read and were left out")).count(); + assert_eq!(unread(&s), 0, "every row read: nothing said"); + + // A row that can't be read is left out and said, once, read ahead or here. + let bad = r#"{"type":"FeatureCollection","features":[{"type":"Feature","properties":{},"geometry":{"type":"Point","coordinates":[8,47]}},{"type":"Feature","properties":{},"geometry":{"type":"Point","coordinates":"x"}}]}"#; + let path = dir.join("bad.geojson"); + std::fs::write(&path, bad).unwrap(); + for ahead in [true, false] { + let mut s = Session::new(Project::default()); + let id = s.run("layer.addData", &json!({"path": path.display().to_string(), "stream": true})).unwrap()["layer"].as_u64().unwrap(); + let p = json!({"layer": id}); + if ahead { + crate::cmd::reads::read_ahead(&s, "layer.loadAll", &p).unwrap().run(&Control::new()).unwrap().placed(&mut s); + } + let before = unread(&s); + assert_eq!(s.run("layer.loadAll", &p).unwrap()["features"], 1); + assert_eq!(unread(&s), before + 1, "{:?}", s.notices.iter().map(|n| &n.text).collect::>()); + } + + // A field added while the read ran: it is read again, with that field kept. + let path = dir.join("fields.geojson"); + std::fs::write(&path, points).unwrap(); + let id = s.run("layer.addData", &json!({"path": path.display().to_string(), "stream": true})).unwrap()["layer"].as_u64().unwrap(); + let p = json!({"layer": id}); + let read = crate::cmd::reads::read_ahead(&s, "layer.loadAll", &p).unwrap().run(&Control::new()).unwrap(); + let mut extra = s.map().layer(id as u32).unwrap().dataset.fields[0].clone(); + extra.name = "added".into(); + s.map_mut().layer_mut(id as u32).unwrap().dataset.fields.push(extra); + read.placed(&mut s); + s.run("layer.loadAll", &p).unwrap(); + assert!(s.map().layer(id as u32).unwrap().dataset.fields.iter().any(|f| f.name == "added")); // A cancelled read gives nothing to add, and says so. let path = dir.join("again.csv"); From fa41e1ba65fb027ec006fc9701c45015116a988c Mon Sep 17 00:00:00 2001 From: Dominik Strebel Date: Thu, 5 Nov 2026 18:13:18 +0100 Subject: [PATCH 4/6] fix(engine): Load All compares the fields listed when its read started The prepared read is used while the layer lists the fields it listed when the read began (an empty list too), and a prepared failure is said instead of reading again on the frame. The reasons a read was left are filled into their notice in logical order (tl_raw). --- crates/engine/src/cmd/layer.rs | 19 ++++++++----------- crates/ui-egui/src/readjob.rs | 7 ++++--- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/crates/engine/src/cmd/layer.rs b/crates/engine/src/cmd/layer.rs index ea9484bab..cd67f052e 100644 --- a/crates/engine/src/cmd/layer.rs +++ b/crates/engine/src/cmd/layer.rs @@ -2701,12 +2701,9 @@ fn load_all(s: &mut Session, p: &Value) -> CmdResult { let ahead = super::reads::take::(s, &key); let l = s.map().layer(id).ok_or("no such layer")?; let (mut ds, unread) = match ahead { - // Read from the source the layer still reads, with the fields it lists now: one opened - // afresh since, or a field added or deleted meanwhile, is read again. - Some((copied, read)) - if l.source.as_ref().is_some_and(|src| src.same_opening(&copied)) - && read.as_ref().is_ok_and(|(ds, _)| l.dataset.fields.is_empty() || ds.fields == l.dataset.fields) => - { + // Read (or failed) from the source the layer still reads, listing the fields it lists + // now: one opened afresh since, or with other fields, is read again. + Some((copied, fields, read)) if l.source.as_ref().is_some_and(|src| src.same_opening(&copied)) && fields == l.dataset.fields => { read? } _ => l.load_all_counted()?, @@ -2726,9 +2723,9 @@ fn load_all(s: &mut Session, p: &Value) -> CmdResult { Ok(json!({"layer": id, "features": n})) } -/// Load All's read made before it runs: the source it read, and what it read with the number of -/// rows that couldn't be read. -type LoadedAll = (rustgis_map::source::SourceLayer, Result<(rustgis_data::Dataset, usize), String>); +/// Load All's read made before it runs: the source it read, the fields the layer listed then, and +/// what it read with the number of rows that couldn't be read. +type LoadedAll = (rustgis_map::source::SourceLayer, Vec, Result<(rustgis_data::Dataset, usize), String>); /// The key Load All's read of `l` is taken by (see [`super::reads`]). fn load_all_key(l: &rustgis_map::Layer) -> String { @@ -2743,8 +2740,8 @@ pub(crate) fn load_all_ahead(s: &Session, p: &Value) -> Option) -> std::fmt::Result { f.write_str(match self { - Gone::Project => tl!("another project was opened"), - Gone::Map => tl!("the map it was asked for isn't open"), - Gone::Layer => tl!("its layer was removed"), + // Filled into a notice's template: in logical order (`tl_raw!`). + Gone::Project => tl_raw!("another project was opened"), + Gone::Map => tl_raw!("the map it was asked for isn't open"), + Gone::Layer => tl_raw!("its layer was removed"), }) } } From 729ad0c60c31ee43bba9ed9e1338e8feede12c6b Mon Sep 17 00:00:00 2001 From: Dominik Strebel Date: Thu, 5 Nov 2026 18:40:42 +0100 Subject: [PATCH 5/6] =?UTF-8?q?docs:=20Delimited=20Text=E2=80=A6=20reads?= =?UTF-8?q?=20as=20the=20command=20runs,=20keeping=20the=20dialog=20for=20?= =?UTF-8?q?another=20try?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/user-guide/data.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/user-guide/data.md b/docs/user-guide/data.md index db728770d..30a4ae9f8 100644 --- a/docs/user-guide/data.md +++ b/docs/user-guide/data.md @@ -52,7 +52,9 @@ and the rest of the window stay in use: the status bar shows **Add Data**, how m after it, and **Cancel**. Files dropped or added together are read one at a time and added in that order; one that asks which layers to add joins them once you choose. Cancel leaves the file being read and those waiting; opening another project, or switching to another map, leaves them too, and a message says which. A file read in place, a web -service or a database adds as before, and scripts read the file where the command runs. +service or a database adds as before, and scripts read the file where the command runs. So does +**Delimited Text…**: it reads the file with the columns picked as you press **Add**, so a column +that doesn't hold coordinates keeps the dialog open for another try. **Home › Add › Add Data › Delimited Text…** asks for a CSV or a spreadsheet first, then lists its columns to pick the x and y (or WKT) ones from, with the coordinate system (such as `EPSG:2263`) and the separator (changing it reads the columns again); `info.fileColumns` lists them for scripts. Add Data finds them itself From 4d48dd3ea04014f2f79bf7878ee487045be1b8a9 Mon Sep 17 00:00:00 2001 From: Dominik Strebel Date: Thu, 5 Nov 2026 18:52:21 +0100 Subject: [PATCH 6/6] fix(ui): a Load All asked again behind a read that was left is queued, not dropped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A read whose map or project went is cancelled but runs on until its thread ends; it no longer counts as reading its layer, so a Load All asked again then waits behind it. A reader's failure notice is put together with i18n::fmt. Docs: Delimited Text… keeps the dialog for a read that fails, not for every column picked wrongly. --- crates/ui-egui/src/readjob.rs | 5 +++-- docs/user-guide/data.md | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/crates/ui-egui/src/readjob.rs b/crates/ui-egui/src/readjob.rs index bd86eaf02..25cd6ecf2 100644 --- a/crates/ui-egui/src/readjob.rs +++ b/crates/ui-egui/src/readjob.rs @@ -61,7 +61,8 @@ pub fn run(app: &mut RustGisApp, command: &'static str, p: Value) -> Result bool { app.map.invalidate(); } // A bug in a reader: said, and the next goes on. - Err(e) => app.session.notify(format!("{}: {e}", job.asked.what), true), + Err(e) => app.session.notify(crate::i18n::fmt("{what}: {e}", &[("what", &job.asked.what), ("e", &e)]), true), } } } diff --git a/docs/user-guide/data.md b/docs/user-guide/data.md index 30a4ae9f8..c120c99ae 100644 --- a/docs/user-guide/data.md +++ b/docs/user-guide/data.md @@ -53,8 +53,9 @@ after it, and **Cancel**. Files dropped or added together are read one at a time that order; one that asks which layers to add joins them once you choose. Cancel leaves the file being read and those waiting; opening another project, or switching to another map, leaves them too, and a message says which. A file read in place, a web service or a database adds as before, and scripts read the file where the command runs. So does -**Delimited Text…**: it reads the file with the columns picked as you press **Add**, so a column -that doesn't hold coordinates keeps the dialog open for another try. +**Delimited Text…**: it reads the file with the columns picked as you press **Add**, so a read +that fails (a column that isn't in the file, a coordinate system it doesn't know) keeps the +dialog open for another try. **Home › Add › Add Data › Delimited Text…** asks for a CSV or a spreadsheet first, then lists its columns to pick the x and y (or WKT) ones from, with the coordinate system (such as `EPSG:2263`) and the separator (changing it reads the columns again); `info.fileColumns` lists them for scripts. Add Data finds them itself