diff --git a/apps/rustgis/examples/navigate_bench.rs b/apps/rustgis/examples/navigate_bench.rs index 2d519766f..e0c5fc560 100644 --- a/apps/rustgis/examples/navigate_bench.rs +++ b/apps/rustgis/examples/navigate_bench.rs @@ -78,7 +78,7 @@ impl eframe::App for Bench { // From the zoom's start, all of it, however long a frame took. let dt = (t - self.last_wheel.max(LOAD + PAN)).max(0.0); self.last_wheel = t; - let dir = if z < ZOOM / 3.0 || z >= 2.0 * ZOOM / 3.0 { -1.0 } else { 1.0 }; + let dir = if !(ZOOM / 3.0..2.0 * ZOOM / 3.0).contains(&z) { -1.0 } else { 1.0 }; input.events.push(egui::Event::MouseWheel { unit: egui::MouseWheelUnit::Point, delta: egui::vec2(0.0, (dir * 600.0 * dt) as f32), diff --git a/apps/rustgis/src/main.rs b/apps/rustgis/src/main.rs index 2377d413e..d57c2cd65 100644 --- a/apps/rustgis/src/main.rs +++ b/apps/rustgis/src/main.rs @@ -309,7 +309,7 @@ fn main() -> eframe::Result { let files: Vec<&String> = args .iter() .enumerate() - .filter(|(i, a)| !a.starts_with("--") && !(*i > 0 && args.get(i - 1).is_some_and(|p| p == "--backend"))) + .filter(|(i, a)| !(a.starts_with("--") || *i > 0 && args.get(i - 1).is_some_and(|p| p == "--backend"))) .map(|(_, a)| a) .collect(); // Installed plugins, before the first session lists commands and tools. diff --git a/crates/analysis/src/trend.rs b/crates/analysis/src/trend.rs index 73eb4873d..4db29388c 100644 --- a/crates/analysis/src/trend.rs +++ b/crates/analysis/src/trend.rs @@ -111,7 +111,7 @@ pub fn trend_with(t: &[f64], y: &[f64], test: Test, alpha: f64, slopes: &mut Vec kept.sort_by(|a, b| a.0.total_cmp(&b.0)); let (t, y): (Vec, Vec) = kept.into_iter().unzip(); let n = y.len(); - if !(MIN_VALUES..=MAX_VALUES).contains(&n) || !(alpha > 0.0 && alpha < 1.0) { + if !((MIN_VALUES..=MAX_VALUES).contains(&n) && alpha > 0.0 && alpha < 1.0) { return None; } if let Test::Seasonal(m) = test { @@ -222,7 +222,7 @@ pub fn trend_seasonal_with(t: &[f64], y: &[f64], season: &[usize], alpha: f64, s let mut kept: Vec<(f64, f64, usize)> = t.iter().zip(y).zip(season).filter(|((t, y), _)| t.is_finite() && y.is_finite()).map(|((t, y), s)| (*t, *y, *s)).collect(); kept.sort_by(|a, b| a.0.total_cmp(&b.0)); - if !(MIN_VALUES..=MAX_VALUES).contains(&kept.len()) || !(alpha > 0.0 && alpha < 1.0) { + if !((MIN_VALUES..=MAX_VALUES).contains(&kept.len()) && alpha > 0.0 && alpha < 1.0) { return None; } let t: Vec = kept.iter().map(|k| k.0).collect(); diff --git a/crates/assistant/src/agent.rs b/crates/assistant/src/agent.rs index 91a06c97f..c793244ec 100644 --- a/crates/assistant/src/agent.rs +++ b/crates/assistant/src/agent.rs @@ -209,7 +209,7 @@ impl Agent { fn local_delegation(&self, call: &ToolCall) -> bool { if call.name != "delegate" || self.depth != 0 - || !call.args.get("task").and_then(serde_json::Value::as_str).is_some_and(|s| !s.trim().is_empty()) + || call.args.get("task").and_then(serde_json::Value::as_str).is_none_or(|s| s.trim().is_empty()) { return false; } diff --git a/crates/data/src/raster/mod.rs b/crates/data/src/raster/mod.rs index 24206647f..5335a5158 100644 --- a/crates/data/src/raster/mod.rs +++ b/crates/data/src/raster/mod.rs @@ -400,7 +400,7 @@ impl Raster { if self.georeferenced { return Err(DataError::Format("only a raster with no coordinates can be placed".into())); } - if !bbox.iter().all(|v| v.is_finite()) || !(w < e && s < n) { + if !(bbox.iter().all(|v| v.is_finite()) && w < e && s < n) { return Err(DataError::Format("the box is [west, south, east, north]".into())); } let old = self.transform; diff --git a/crates/engine/src/cmd/select.rs b/crates/engine/src/cmd/select.rs index 8f5fb2544..a14b49e10 100644 --- a/crates/engine/src/cmd/select.rs +++ b/crates/engine/src/cmd/select.rs @@ -218,7 +218,7 @@ fn identify(s: &mut Session, p: &Value) -> CmdResult { let map = s.map(); let mut results: Vec = hits.iter().filter_map(|(id, row)| feature_json(map, *id, *row)).collect(); // Then the features of vector tile layers, beneath the layers; tiles are never selectable. - if matches!(mode, PopupMode::Topmost | PopupMode::Visible) && !(first_layer && !results.is_empty()) { + if matches!(mode, PopupMode::Topmost | PopupMode::Visible) && (!first_layer || results.is_empty()) { let room = MAX_IDENTIFIED.saturating_sub(results.len()); results.extend(tile_features(s, lon, lat, tol_px, first_layer, room)); } diff --git a/crates/engine/src/model.rs b/crates/engine/src/model.rs index 9593a843d..977669d17 100644 --- a/crates/engine/src/model.rs +++ b/crates/engine/src/model.rs @@ -223,7 +223,7 @@ pub fn validate(s: &Session, g: &ModelGraph) -> Vec { }; for p in t.params.iter().filter(|p| p.required) { let port = port_kind(p.kind).is_some(); - if !(port && wired(p.name)) && !is_set(n.params.get(p.name)) && p.default.is_empty() { + if !(port && wired(p.name) || is_set(n.params.get(p.name))) && p.default.is_empty() { let msg = if port { "\"{p}\" needs a connection or a layer." } else { "\"{p}\" needs a value." }; out.push(problem("missing-input", Some(&n.id), msg.replace("{p}", p.label))); } diff --git a/crates/map/src/globe.rs b/crates/map/src/globe.rs index c9ae4a565..ed0699e56 100644 --- a/crates/map/src/globe.rs +++ b/crates/map/src/globe.rs @@ -560,7 +560,7 @@ impl Globe { pub fn drape_mesh(&self, merc: &Extent) -> DrapeMesh { use rustgis_geom::merc_to_lonlat; let mut out = DrapeMesh::default(); - if merc.is_empty() || !(merc.width() > 0.0 && merc.height() > 0.0) || !merc.width().is_finite() || !merc.height().is_finite() { + if !(!merc.is_empty() && merc.width() > 0.0 && merc.height() > 0.0 && merc.width().is_finite() && merc.height().is_finite()) { return out; } let ll = diff --git a/crates/tiles/src/embeddings.rs b/crates/tiles/src/embeddings.rs index a83af5400..dda9b6ee9 100644 --- a/crates/tiles/src/embeddings.rs +++ b/crates/tiles/src/embeddings.rs @@ -403,7 +403,7 @@ fn utm_zone(lon: f64, lat: f64) -> u32 { /// The Sentinel-2 (MGRS 100 km) tile holding a place, such as `17SKV`; `None` outside MGRS's /// UTM latitudes (80° S to 84° N). pub fn mgrs_tile(lon: f64, lat: f64) -> Option { - if !(lon.is_finite() && lat.is_finite()) || !(-80.0..84.0).contains(&lat) { + if !(lon.is_finite() && lat.is_finite() && (-80.0..84.0).contains(&lat)) { return None; } let zone = utm_zone(lon, lat); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index bf30b6672..0f30e5b93 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -440,6 +440,35 @@ pub enum RightPane { Plugin, } +/// A Unique Values field choice scoped to the layer and renderer that opened it. +#[derive(Clone, Debug, PartialEq)] +pub struct PendingUniqueSymbology { + project: u64, + map: usize, + layer: u32, + renderer: std::sync::Arc, + pub field: String, +} + +impl PendingUniqueSymbology { + fn new(session: &Session, layer: &rustgis_engine::map::Layer, field: String) -> Self { + Self { + project: session.opened, + map: session.project.active_map, + layer: layer.id, + renderer: std::sync::Arc::new(layer.renderer.clone()), + field, + } + } + + fn is_current(&self, session: &Session) -> bool { + self.project == session.opened + && self.map == session.project.active_map + && Some(self.layer) == session.active_layer + && session.map().layer(self.layer).is_some_and(|layer| layer.renderer == *self.renderer) + } +} + pub struct UiState { pub tab: String, pub backstage: bool, @@ -612,6 +641,8 @@ pub struct UiState { pub copy_image: Option, /// The scale being typed into the status bar, while its box has focus. pub scale_text: Option, + /// A field chosen in the Symbology pane before applying Unique Values. + pub symbology_pending: Option, } impl UiState { @@ -731,6 +762,7 @@ impl Default for UiState { layers_view: LayersView::DrawingOrder, dynamic_text: None, scale_text: None, + symbology_pending: None, } } } @@ -1492,6 +1524,11 @@ impl RustGisApp { if let Err(e) = &r { log::info!("{id}: {e}"); } + // Commands can replace the project, switch maps or change the renderer while the pane + // is closed. A field picked for the old context must not overwrite the new renderer. + if self.ui.symbology_pending.as_ref().is_some_and(|pending| !pending.is_current(&self.session)) { + self.ui.symbology_pending = None; + } // A project replacing this one hasn't been saved in the browser yet (opening a file // names it again). if r.is_ok() && matches!(id, "project.new" | "project.sample" | "project.open" | "project.openText" | "project.fromTemplate") { @@ -1578,7 +1615,7 @@ impl RustGisApp { let active = self.session.active_layer.take(); let needs = (spec.enabled)(&self.session).is_some(); self.session.active_layer = active; - if !defaults && !(needs && (spec.enabled)(&self.session).is_none()) { + if !(defaults || needs && (spec.enabled)(&self.session).is_none()) { return None; } let map = self.session.map(); diff --git a/crates/ui-egui/src/panes.rs b/crates/ui-egui/src/panes.rs index c8d40524c..9897e54ba 100644 --- a/crates/ui-egui/src/panes.rs +++ b/crates/ui-egui/src/panes.rs @@ -3401,6 +3401,9 @@ fn pending_symbology(app: &RustGisApp, ctx: &egui::Context) -> Option = l.dataset.fields.iter().filter(|f| f.ty.is_numeric()).map(|f| f.name.clone()).collect(); + let all: Vec = l.dataset.fields.iter().map(|f| f.name.clone()).collect(); ui.label(egui::RichText::new(&l.name).font(semibold(14.0))); ui.add_space(6.0); ui.label(egui::RichText::new(tl!("Primary symbology")).size(11.5).color(t.text_dim)); @@ -3432,7 +3437,7 @@ fn symbology(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { "graduated" => "graduated", _ => "proportional", }; - let sel = cur_kind == *label; + let sel = if app.ui.symbology_pending.is_some() { *code == "unique" } else { cur_kind == *label }; let (r, resp) = ui.allocate_exact_size(vec2(68.0, 56.0), Sense::click()); resp.widget_info(|| egui::WidgetInfo::selected(egui::Role::Button, true, sel, tl!(*label))); ui.painter().rect_filled( @@ -3462,6 +3467,14 @@ fn symbology(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { let short = tl!(label).split(' ').next().unwrap_or(label); ui.painter().text(pos2(r.center().x, r.max.y - 11.0), Align2::CENTER_CENTER, short, regular(11.0), t.text); if resp.on_hover_text(tl!(*label)).clicked() && !sel { + if *code == "unique" && !all.is_empty() { + let field = l.dataset.fields.iter().find(|f| !f.ty.is_numeric()).or_else(|| l.dataset.fields.first()); + if let Some(field) = field { + app.ui.symbology_pending = Some(crate::PendingUniqueSymbology::new(&app.session, &l, field.name.clone())); + clear_pending_symbology(ui.ctx()); + } + continue; + } match app.run("layer.symbology", json!({"type": code})) { Err(error) => { let pending = PendingSymbology { @@ -3474,20 +3487,53 @@ fn symbology(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { }; ui.data_mut(|data| data.insert_temp(egui::Id::unique(PENDING_SYMBOLOGY), pending)); } - Ok(_) => clear_pending_symbology(ui.ctx()), + Ok(_) => { + clear_pending_symbology(ui.ctx()); + app.ui.symbology_pending = None; + } } } } }); ui.add_space(8.0); + if let Some(mut pending) = app.ui.symbology_pending.clone() { + let pending_field = pending.field.clone(); + let mut selected = if all.contains(&pending_field) { pending_field.clone() } else { all.first().cloned().unwrap_or_default() }; + ui.horizontal(|ui| { + let input_label = ui.label(tl!("Field")); + egui::ComboBox::from_id_salt("sym_pending_field") + .selected_text(&selected) + .width(200.0) + .show_ui(ui, |ui| { + for field in &all { + ui.selectable_value(&mut selected, field.clone(), field); + } + }) + .response + .labelled_by(input_label.id); + let applied = ui.button(tl!("Apply")).clicked() + && match app.run("layer.symbology", json!({"type": "unique", "field": selected.clone()})) { + Ok(_) => true, + Err(error) => { + app.session.notify(error, true); + false + } + }; + if applied { + app.ui.symbology_pending = None; + } else if selected != pending_field { + pending.field = selected; + app.ui.symbology_pending = Some(pending); + } + }); + ui.add_space(6.0); + } if g == GeometryType::Polygon && !l.graphics { if ui.button(tl!("No Fill")).on_hover_text(tl!("Draw only polygon outlines, keeping their colors and widths.")).clicked() { let _ = app.run("layer.noFill", Value::Null); } ui.add_space(6.0); } - let numeric: Vec = l.dataset.fields.iter().filter(|f| f.ty.is_numeric()).map(|f| f.name.clone()).collect(); - let all: Vec = l.dataset.fields.iter().map(|f| f.name.clone()).collect(); if let Some(pending) = pending_symbology(app, ui.ctx()) { ui.label(egui::RichText::new(pending.error.as_ref()).color(ui.visuals().error_fg_color)); let fields = if pending.kind == "unique" { &all } else { &numeric }; diff --git a/crates/ui-egui/tests/smoke.rs b/crates/ui-egui/tests/smoke.rs index f520ea392..61bb5c73a 100644 --- a/crates/ui-egui/tests/smoke.rs +++ b/crates/ui-egui/tests/smoke.rs @@ -3059,6 +3059,86 @@ fn geometry_calculator_switching_back_creates_a_unique_field() { assert_eq!(active.value(0, 0).as_f64(), Some(5.0)); } +#[test] +fn unique_symbology_lets_the_user_choose_a_text_field_before_applying() { + use rustgis_ui_egui::rustgis_engine::data::{Dataset, Field, FieldType, Geometry, GeometryType, Value}; + use rustgis_ui_egui::rustgis_engine::geom::Coord; + + let mut h = harness(); + let mut ds = Dataset::new( + "Text attributes", + GeometryType::Point, + vec![Field::new("id", FieldType::Integer), Field::new("category", FieldType::Text), Field::new("other", FieldType::Text)], + ); + ds.push(Some(Geometry::Point(Coord::new(117.0, 3.0))), vec![Value::Int(1), Value::Text("village".into()), Value::Text("town".into())]); + let id = h.state_mut().session.map_mut().add_dataset(ds); + h.state_mut().session.active_layer = Some(id); + h.state_mut().run("ui.pane.symbology", json!(null)).unwrap(); + h.step(); + h.get_by_role_and_label(egui::accesskit::Role::Button, "Unique Values").click(); + h.step(); + assert_eq!(h.state().ui.symbology_pending.as_ref().map(|pending| pending.field.as_str()), Some("category")); + h.get_by_role_and_label(egui::accesskit::Role::ComboBox, "Field").click(); + h.step(); + h.step(); // Combo popups enter the accessibility tree on the following frame. + h.get_by_label("other").click(); + h.step(); + h.get_by_role_and_label(egui::accesskit::Role::Button, "Apply").click(); + h.step(); + let layer = h.state().session.map().layer(id).unwrap(); + assert!(matches!(layer.renderer, rustgis_ui_egui::rustgis_engine::map::Renderer::Unique { ref field, .. } if field == "other")); + assert_eq!(h.state().ui.symbology_pending, None); +} + +#[test] +fn unique_symbology_pending_choice_follows_its_context() { + for change in ["map", "project", "renderer", "undo", "external renderer", "layer"] { + let mut h = harness(); + h.state_mut().run("layer.select", json!({"layer": "Cities"})).unwrap(); + h.state_mut().run("layer.symbology", json!({"type": "unique", "field": "name"})).unwrap(); + h.state_mut().run("layer.symbology", json!({"type": "single"})).unwrap(); + // A second map deliberately has the same layer IDs and renderer as the first. + let mut other = h.state().session.map().clone(); + other.name = "Other map".into(); + h.state_mut().session.project.maps.push(other); + h.state_mut().run("ui.pane.symbology", json!(null)).unwrap(); + h.step(); + h.get_by_role_and_label(egui::accesskit::Role::Button, "Unique Values").click(); + h.step(); + assert!(h.state().ui.symbology_pending.is_some()); + // Failed commands leave the valid choice available for retry. + assert!(h.state_mut().run("layer.symbology", json!({"type": "unique", "field": "missing field"})).is_err()); + assert!(h.state().ui.symbology_pending.is_some()); + if change != "external renderer" { + h.state_mut().ui.close_right(rustgis_ui_egui::RightPane::Symbology); + } + match change { + "map" => { + h.state_mut().run("map.activate", json!({"name": "Other map"})).unwrap(); + } + "project" => { + h.state_mut().run("project.sample", json!(null)).unwrap(); + } + "renderer" => { + h.state_mut().run("layer.symbology", json!({"type": "unique", "field": "name"})).unwrap(); + } + "undo" => { + h.state_mut().run("edit.undo", json!(null)).unwrap(); + } + "external renderer" => { + // Commands from other clients can bypass RustGisApp::run. + h.state_mut().session.run("layer.symbology", &json!({"type": "unique", "field": "name"})).unwrap(); + h.step(); + } + "layer" => { + h.state_mut().run("layer.select", json!({"layer": "Countries"})).unwrap(); + } + _ => unreachable!(), + } + assert!(h.state().ui.symbology_pending.is_none(), "pending choice survived {change}"); + } +} + #[test] fn map_and_layout_tabs_rename_from_their_menus() { let mut h = harness();