From 38fde7a66ef871653106ba9ba7b38bccecc46710 Mon Sep 17 00:00:00 2001 From: alteralto <182909819+alteralto@users.noreply.github.com> Date: Sat, 24 Oct 2026 17:54:15 +0300 Subject: [PATCH 1/4] test(symbology): manual class breaks, as given --- crates/engine/src/tests.rs | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 033cefa8c..d34c7d104 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -15344,3 +15344,39 @@ fn toggling_out_the_only_picked_layer_leaves_none_active() { assert!(s.selected_layers().is_empty()); assert!(s.run("layer.remove", &Value::Null).is_err(), "nothing picked, nothing removed"); } + +#[test] +fn manual_breaks_are_the_ones_given() { + use rustgis_map::{ClassMethod, Renderer}; + let mut s = world(); + let breaks = |s: &Session| match &s.map().layer_by_name("Countries").unwrap().renderer { + Renderer::Graduated { breaks, method, .. } => (breaks.clone(), *method), + r => panic!("{r:?}"), + }; + let l = s.map().layer_by_name("Countries").unwrap(); + let fi = l.dataset.field_index("pop_est").unwrap(); + let max = (0..l.dataset.features.len()).filter_map(|r| l.dataset.value(r, fi).as_f64()).fold(f64::MIN, f64::max); + let manual = |b: Value| json!({"layer": "Countries", "type": "graduated", "field": "pop_est", "method": "manual", "breaks": b}); + // The bounds as typed; the field's maximum closes the last class, so every country has one. + s.run("layer.symbology", &manual(json!([1e6, 1e7, 1e8]))).unwrap(); + assert_eq!(breaks(&s), (vec![1e6, 1e7, 1e8, max], ClassMethod::Manual)); + // Out of order or repeated, they come back sorted, once. + s.run("layer.symbology", &manual(json!([5e7, 1e6, 5e7]))).unwrap(); + assert_eq!(breaks(&s).0, vec![1e6, 5e7, max]); + // "breaks" alone means manual. + let mut p = manual(json!([2e7])); + p.as_object_mut().unwrap().remove("method"); + s.run("layer.symbology", &p).unwrap(); + assert_eq!(breaks(&s), (vec![2e7, max], ClassMethod::Manual)); + // Manual with no bounds keeps the layer's current ones, so switching to it changes nothing. + s.run("layer.symbology", &json!({"layer": "Countries", "type": "graduated", "field": "pop_est", "method": "quantile", "classes": 4})).unwrap(); + let q = breaks(&s).0; + s.run("layer.symbology", &json!({"layer": "Countries", "type": "graduated", "field": "pop_est", "method": "manual"})).unwrap(); + assert_eq!(breaks(&s), (q, ClassMethod::Manual)); + // Not numbers, none, or more than twelve classes are refused, and the layer keeps its style. + for bad in [json!([]), json!(["a lot"]), json!((1..=13).map(|i| i as f64 * 1e6).collect::>())] { + let before = breaks(&s); + assert!(s.run("layer.symbology", &manual(bad.clone())).is_err(), "{bad}"); + assert_eq!(breaks(&s), before); + } +} From ba8eb0ebbbf44938f4981512b400557c2790e1fc Mon Sep 17 00:00:00 2001 From: alteralto <182909819+alteralto@users.noreply.github.com> Date: Sat, 24 Oct 2026 18:05:33 +0300 Subject: [PATCH 2/4] feat(symbology): manual class breaks, typed in the Symbology pane --- ROADMAP.md | 3 +- .../rustgis-python/python/rustgis/__init__.py | 3 +- crates/assistant/skills/thematic-map/SKILL.md | 2 +- crates/engine/src/cmd/layer.rs | 26 +++++++- crates/engine/src/tests.rs | 3 +- crates/map/src/symbology.rs | 58 +++++++++++++++-- crates/ui-egui/src/panes.rs | 62 +++++++++++++++---- docs/user-guide/cartography.md | 4 +- 8 files changed, 136 insertions(+), 25 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 483933bd6..a9a18b442 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -57,7 +57,6 @@ The bugs users hit, the docs and a few quick wins: good first pull requests. - A pass over the user guide, the comparison pages and the README against what the app does today (vertex editing, spatial join and kriging are listed as missing or undocumented; some pages still say 16 languages). -- Styling: manual breaks _(open)_. - Selection by polygon and lasso, and a radius tool on the map (Neighborhood Explorer already selects by radius) _(open)_. @@ -188,7 +187,7 @@ Open questions rather than plans; discuss before building. layers (XYZ/TMS/WMS, Mapbox Vector Tiles, MapLibre styles, PMTiles and MBTiles archives), a basemap gallery (OpenFreeMap, USGS Imagery), worldviews. - Cartography: single, unique values, graduated colours (natural breaks, equal interval, quantile, - geometric interval, standard deviation) and proportional symbols; layer groups; + geometric interval, standard deviation, manual) and proportional symbols; layer groups; label classes with SQL queries, placement and annotation; custom colour ramps (and QGIS, GDAL and GMT palette files); masking, display filters, blend modes, aggregation, extrusion; graphics. - Editing: create, move, rotate, scale, merge, split, explode, vertex editing; snapping, templates, elevation, diff --git a/apps/rustgis-python/python/rustgis/__init__.py b/apps/rustgis-python/python/rustgis/__init__.py index 4e68bfe08..18b8185c2 100644 --- a/apps/rustgis-python/python/rustgis/__init__.py +++ b/apps/rustgis-python/python/rustgis/__init__.py @@ -158,7 +158,8 @@ def style(self, type: str = "single", field: Optional[str] = None, **params: Any field: The field the colours or sizes come from (not for ``"single"``). **params: More ``layer.symbology`` parameters: ``classes`` (1 to 12), ``method`` (``"naturalBreaks"``, ``"equalInterval"``, ``"quantile"``, - ``"geometricInterval"``, ``"standardDeviation"``), ``ramp``. + ``"geometricInterval"``, ``"standardDeviation"``, ``"manual"``), + ``breaks`` (manual: the classes' upper bounds), ``ramp``. Returns: The layer, so calls can be chained. diff --git a/crates/assistant/skills/thematic-map/SKILL.md b/crates/assistant/skills/thematic-map/SKILL.md index 8ffd91f3c..b52dbf9ba 100644 --- a/crates/assistant/skills/thematic-map/SKILL.md +++ b/crates/assistant/skills/thematic-map/SKILL.md @@ -9,7 +9,7 @@ description: Style a layer by its data (graduated colours, categories, proportio field suits graduated or proportional symbols; text or a few distinct values suit unique values. 2. Style it with `run_command` `layer.symbology`: - Numbers: `{"layer": L, "type": "graduated", "field": F, "classes": 5, "method": "naturalBreaks"}` (or - `quantile`, `equalInterval`, `geometricInterval`, `standardDeviation`), with an optional `"ramp"`. + `quantile`, `equalInterval`, `geometricInterval`, `standardDeviation`, or `"breaks": [upper bounds]` for classes set by hand), with an optional `"ramp"`. For counts or sizes, `"type": "proportional"` usually reads better than colour. - Categories: `{"layer": L, "type": "unique", "field": F}`. More than about 12 categories is hard to read, so suggest grouping first. diff --git a/crates/engine/src/cmd/layer.rs b/crates/engine/src/cmd/layer.rs index 034e0f2a5..32abe367a 100644 --- a/crates/engine/src/cmd/layer.rs +++ b/crates/engine/src/cmd/layer.rs @@ -109,7 +109,7 @@ pub fn commands() -> Vec { .when(need_layer), C::new("layer.symbology", "Symbology", "Style › Drawing", symbology) .when(need_feature_layer) - .params(r#"{"layer"?, "type": "single"|"unique"|"graduated"|"proportional", "field"?, "classes"?: 1..12, "method"?: "naturalBreaks"|"equalInterval"|"quantile"|"geometricInterval"|"standardDeviation", "ramp"?: string}"#), + .params(r#"{"layer"?, "type": "single"|"unique"|"graduated"|"proportional", "field"?, "classes"?: 1..12, "method"?: "naturalBreaks"|"equalInterval"|"quantile"|"geometricInterval"|"standardDeviation"|"manual", "breaks"?: [numbers] (manual: the classes' upper bounds, closed by the field's largest value), "ramp"?: string}"#), C::new("layer.ramp", "Color Scheme", "Style › Drawing", |s, p| { let r = s.ramp(s_req(p, "ramp")?)?; with(s, p, |l| match &mut l.raster { @@ -2672,7 +2672,17 @@ fn symbology(s: &mut Session, p: &Value) -> CmdResult { Some(n) => Some(s.ramp(n).or_else(|e| current.filter(|r| r.name().eq_ignore_ascii_case(n.trim())).ok_or(e))?), None => current, }; - let method = ps(p, "method").and_then(ClassMethod::from_name).unwrap_or_default(); + // Manual bounds, as typed; "breaks" alone means manual. + let typed = match p.get("breaks") { + None | Some(Value::Null) => None, + Some(Value::Array(a)) => Some(a.iter().map(|v| v.as_f64().ok_or("breaks are numbers")).collect::, _>>()?), + Some(_) => return Err("breaks are a list of numbers".into()), + }; + let method = match ps(p, "method").and_then(ClassMethod::from_name) { + Some(m) => m, + None if typed.is_some() => ClassMethod::Manual, + None => ClassMethod::default(), + }; let classes = f(p, "classes").map(|c| c.clamp(1.0, 12.0) as usize).unwrap_or(5); let l = s.map().layer(id).ok_or("no such layer")?; let g = l.geometry_type(); @@ -2714,7 +2724,17 @@ fn symbology(s: &mut Session, p: &Value) -> CmdResult { "single" | "singlesymbol" => Renderer::Single(base), "unique" | "uniquevalues" => Renderer::unique(&l.dataset, &rows()?, &field(false)?, ramp.unwrap_or(Ramp::Harbour), base)?, "graduated" | "graduatedcolors" => { - Renderer::graduated(&l.dataset, &rows()?, &field(true)?, classes, method, ramp.unwrap_or(Ramp::Ember), base)? + let (field, ramp) = (field(true)?, ramp.unwrap_or(Ramp::Ember)); + // Manual with no bounds keeps the layer's current ones on the same field, so switching + // to it changes nothing. + let kept = match &l.renderer { + Renderer::Graduated { field: f, breaks, .. } if *f == field => Some(breaks.clone()), + _ => None, + }; + match (method, typed.or(kept)) { + (ClassMethod::Manual, Some(b)) => Renderer::graduated_manual(&l.dataset, &rows()?, &field, &b, ramp, base)?, + _ => Renderer::graduated(&l.dataset, &rows()?, &field, classes, method, ramp, base)?, + } } "proportional" | "proportionalsymbols" => { if g != GeometryType::Point { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index d34c7d104..fab968e51 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -15369,7 +15369,8 @@ fn manual_breaks_are_the_ones_given() { s.run("layer.symbology", &p).unwrap(); assert_eq!(breaks(&s), (vec![2e7, max], ClassMethod::Manual)); // Manual with no bounds keeps the layer's current ones, so switching to it changes nothing. - s.run("layer.symbology", &json!({"layer": "Countries", "type": "graduated", "field": "pop_est", "method": "quantile", "classes": 4})).unwrap(); + s.run("layer.symbology", &json!({"layer": "Countries", "type": "graduated", "field": "pop_est", "method": "quantile", "classes": 4})) + .unwrap(); let q = breaks(&s).0; s.run("layer.symbology", &json!({"layer": "Countries", "type": "graduated", "field": "pop_est", "method": "manual"})).unwrap(); assert_eq!(breaks(&s), (q, ClassMethod::Manual)); diff --git a/crates/map/src/symbology.rs b/crates/map/src/symbology.rs index 7868ba668..795a73db4 100644 --- a/crates/map/src/symbology.rs +++ b/crates/map/src/symbology.rs @@ -315,15 +315,18 @@ pub enum ClassMethod { GeometricInterval, /// Classes one standard deviation wide about the mean. StandardDeviation, + /// Bounds typed by hand, kept as they are (see [`Renderer::graduated_manual`]). + Manual, } impl ClassMethod { - pub const ALL: [ClassMethod; 5] = [ + pub const ALL: [ClassMethod; 6] = [ ClassMethod::NaturalBreaks, ClassMethod::EqualInterval, ClassMethod::Quantile, ClassMethod::GeometricInterval, ClassMethod::StandardDeviation, + ClassMethod::Manual, ]; pub fn label(self) -> &'static str { match self { @@ -332,6 +335,7 @@ impl ClassMethod { ClassMethod::Quantile => "Quantile", ClassMethod::GeometricInterval => "Geometric Interval", ClassMethod::StandardDeviation => "Standard Deviation", + ClassMethod::Manual => "Manual", } } pub fn from_name(s: &str) -> Option { @@ -342,6 +346,7 @@ impl ClassMethod { "quantile" => ClassMethod::Quantile, "geometricinterval" | "geometric_interval" | "geometric interval" | "geometric" => ClassMethod::GeometricInterval, "standarddeviation" | "standard_deviation" | "standard deviation" | "stddev" => ClassMethod::StandardDeviation, + "manual" => ClassMethod::Manual, _ => return None, }) } @@ -520,6 +525,51 @@ impl Renderer { method: ClassMethod, ramp: Ramp, base: Symbol, + ) -> Result { + let k = classes.clamp(1, 12); + Self::graduated_by(ds, rows, field, method, ramp, base, |vals| Ok(class_breaks(vals, k, method))) + } + + /// Builds a graduated-colours renderer on the class bounds typed by hand: the classes' upper + /// bounds, sorted and each once, closed by the field's largest value when they stop short of + /// it, so every feature has a class. At most 12 classes. + pub fn graduated_manual( + ds: &Dataset, + rows: &[bool], + field: &str, + breaks: &[f64], + ramp: Ramp, + base: Symbol, + ) -> Result { + Self::graduated_by(ds, rows, field, ClassMethod::Manual, ramp, base, |vals| { + let mut b: Vec = breaks.iter().copied().filter(|v| v.is_finite()).collect(); + if b.is_empty() { + return Err("manual breaks need at least one number".into()); + } + b.sort_by(|a, b| a.total_cmp(b)); + b.dedup(); + if let Some(&hi) = vals.last() + && b.last().is_some_and(|&l| l < hi) + { + b.push(hi); + } + if b.len() > 12 { + return Err(format!("{} classes; Graduated Colors takes at most 12", b.len())); + } + Ok(b) + }) + } + + /// A graduated renderer on `field`, with the bounds `breaks` works out from the field's sorted + /// values. + fn graduated_by( + ds: &Dataset, + rows: &[bool], + field: &str, + method: ClassMethod, + ramp: Ramp, + base: Symbol, + breaks: impl FnOnce(&[f64]) -> Result, String>, ) -> Result { let fi = ds.field_index(field).ok_or_else(|| format!("no field named '{field}'"))?; if !ds.fields.get(fi).is_some_and(|f| f.ty.is_numeric()) { @@ -530,8 +580,7 @@ impl Renderer { return Err(format!("'{field}' has no numeric values")); } vals.sort_by(|a, b| a.partial_cmp(b).unwrap_or(std::cmp::Ordering::Equal)); - let k = classes.clamp(1, 12); - let breaks = class_breaks(&vals, k, method); + let breaks = breaks(&vals)?; let n = breaks.len(); let symbols = (0..n).map(|i| base.with_ramp_fill(ramp.pick(i, n), ds.geometry_type)).collect(); let min = vals.first().copied().unwrap_or(0.0); @@ -1059,7 +1108,8 @@ pub fn class_breaks(sorted: &[f64], k: usize, method: ClassMethod) -> Vec { return vec![hi]; } let mut b = match method { - ClassMethod::EqualInterval => (1..=k).map(|i| lo + (hi - lo) * i as f64 / k as f64).collect::>(), + // Manual bounds are typed, not worked out: asked to work them out, it starts from equal intervals. + ClassMethod::EqualInterval | ClassMethod::Manual => (1..=k).map(|i| lo + (hi - lo) * i as f64 / k as f64).collect::>(), ClassMethod::Quantile => (1..=k).map(|i| sorted.get(((n * i) / k).saturating_sub(1).min(n - 1)).copied().unwrap_or(hi)).collect(), ClassMethod::NaturalBreaks => jenks(sorted, k), ClassMethod::GeometricInterval => { diff --git a/crates/ui-egui/src/panes.rs b/crates/ui-egui/src/panes.rs index f9d925844..10da1cc63 100644 --- a/crates/ui-egui/src/panes.rs +++ b/crates/ui-egui/src/panes.rs @@ -3498,7 +3498,7 @@ fn symbology(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { ); legend_list(app, ui, &l, t); } - Renderer::Graduated { field, breaks, ramp, method, .. } => { + Renderer::Graduated { field, breaks, ramp, method, min, .. } => { field_combo(app, ui, "Field", field, &numeric, "graduated"); ui.horizontal(|ui| { let input_label = ui.label(tl!("Method")); @@ -3510,16 +3510,20 @@ fn symbology(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { } }).response.labelled_by(input_label.id); }); - ui.horizontal(|ui| { - let input_label = ui.label(tl!("Classes")); - let mut k = breaks.len(); - if ui.add(egui::DragValue::new(&mut k).range(1..=12)).labelled_by(input_label.id).changed() { - let _ = app.run( - "layer.symbology", - json!({"type": "graduated", "field": field, "method": format!("{method:?}"), "classes": k, "ramp": ramp.name()}), - ); - } - }); + if *method == ClassMethod::Manual { + manual_breaks(app, ui, field, breaks, *min, ramp); + } else { + ui.horizontal(|ui| { + let input_label = ui.label(tl!("Classes")); + let mut k = breaks.len(); + if ui.add(egui::DragValue::new(&mut k).range(1..=12)).labelled_by(input_label.id).changed() { + let _ = app.run( + "layer.symbology", + json!({"type": "graduated", "field": field, "method": format!("{method:?}"), "classes": k, "ramp": ramp.name()}), + ); + } + }); + } ramp_picker(app, ui, ramp, t); ui.add_space(6.0); legend_list(app, ui, &l, t); @@ -4767,7 +4771,8 @@ fn raster_symbology(app: &mut RustGisApp, ui: &mut Ui, l: &Layer, t: &Tokens) { .selected_text(tl!(method.label())) .width(180.0) .show_ui(ui, |ui| { - for mm in ClassMethod::ALL { + // Manual bounds are for Graduated Colors; a raster's are worked out. + for mm in ClassMethod::ALL.into_iter().filter(|m| *m != ClassMethod::Manual) { if ui.selectable_label(mm == m, tl!(mm.label())).clicked() { let _ = app.run( "raster.symbology", @@ -5655,3 +5660,36 @@ mod tests { assert_eq!(h.state().session.revision, rev, "the menu ran a command on every frame"); } } + +/// Graduated Colors' bounds typed by hand: each class's upper bound as a number, a button to take +/// a bound out, and one to split the widest class in two. +fn manual_breaks(app: &mut RustGisApp, ui: &mut Ui, field: &str, breaks: &[f64], min: f64, ramp: &Ramp) { + let mut b = breaks.to_vec(); + let speed = ((b.last().copied().unwrap_or(min) - min).abs() / 200.0).max(1e-9); + let mut changed = false; + let mut remove = None; + for (i, v) in b.iter_mut().enumerate() { + ui.horizontal(|ui| { + let input_label = ui.label(crate::i18n::fmt("Class {n} up to", &[("n", &(i + 1).to_string())])); + changed |= ui.add(egui::DragValue::new(v).speed(speed)).labelled_by(input_label.id).changed(); + if breaks.len() > 1 && ui.small_button("×").on_hover_text(tl!("Take this bound out")).clicked() { + remove = Some(i); + } + }); + } + if let Some(i) = remove { + b.remove(i); + changed = true; + } + if b.len() < 12 && ui.button(tl!("Split the widest class")).clicked() { + let lower = |i: usize| if i == 0 { min } else { b[i - 1] }; + if let Some(i) = (0..b.len()).max_by(|&x, &y| (b[x] - lower(x)).total_cmp(&(b[y] - lower(y)))) { + b.insert(i, (lower(i) + b[i]) / 2.0); + changed = true; + } + } + if changed { + let _ = + app.run("layer.symbology", json!({"type": "graduated", "field": field, "method": "manual", "breaks": b, "ramp": ramp.name()})); + } +} diff --git a/docs/user-guide/cartography.md b/docs/user-guide/cartography.md index 6a45dbc56..4244d4082 100644 --- a/docs/user-guide/cartography.md +++ b/docs/user-guide/cartography.md @@ -22,7 +22,9 @@ Select a layer in Layers and open **Style › Symbology** (or the Symbology pane widths grow by one ratio, for values piled up at the low end; for a field that reaches zero or below, the minimum is taken as if it sat at a hundredth of the range, so the widths grow by 101^(1/classes). Standard deviation makes classes one deviation wide about the mean, and leaves - out a class no value falls in, so a skewed field can get fewer classes than asked. If no field is + out a class no value falls in, so a skewed field can get fewer classes than asked. Manual keeps the + bounds you type: each class's upper bound is a number to edit or take out, **Split the widest + class** adds one, and the field's largest value closes the last class. If no field is chosen, the current renderer's field is kept when it is numeric; otherwise the first numeric field is used. If the layer has no numeric field, or the switch fails for another reason, the pane shows why and keeps the current renderer. When numeric From fb84a9fa496b64183547511c5576fb148bc512d3 Mon Sep 17 00:00:00 2001 From: alteralto <182909819+alteralto@users.noreply.github.com> Date: Sat, 24 Oct 2026 18:26:04 +0300 Subject: [PATCH 3/4] fix(symbology): a dragged bound applies once; manual is refused for rasters --- crates/engine/src/cmd/raster.rs | 3 ++ crates/engine/src/tests.rs | 2 + crates/ui-egui/src/panes.rs | 78 +++++++++++++++++++-------------- 3 files changed, 50 insertions(+), 33 deletions(-) diff --git a/crates/engine/src/cmd/raster.rs b/crates/engine/src/cmd/raster.rs index ca37047eb..996f35003 100644 --- a/crates/engine/src/cmd/raster.rs +++ b/crates/engine/src/cmd/raster.rs @@ -572,6 +572,9 @@ fn symbology(s: &mut Session, p: &Value) -> CmdResult { "classify" => { let classes = f(p, "classes").map(|c| c.clamp(1.0, 32.0) as usize).unwrap_or(5); let method = ps(p, "method").and_then(ClassMethod::from_name).unwrap_or_default(); + if method == ClassMethod::Manual { + return Err("manual breaks are for Graduated Colors; a raster's classes are worked out".into()); + } let ramp = ramp.or(cur_ramp.filter(|r| !r.is_qualitative())).unwrap_or(Ramp::Ember); classify_masked(r, bi, classes, method, ramp, &rl.style.mask) } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index fab968e51..f91813e9b 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1206,6 +1206,8 @@ fn raster_layers_open_style_identify_and_round_trip() { ] { s.run("raster.symbology", &p).unwrap_or_else(|e| panic!("{p}: {e}")); } + // Bounds typed by hand are for Graduated Colors: a raster's classes are worked out. + assert!(s.run("raster.symbology", &json!({"type": "classify", "method": "manual"})).is_err()); assert!(s.run("raster.symbology", &json!({"type": "colormap"})).is_err()); assert!(s.run("raster.symbology", &json!({"band": 9})).is_err()); s.run("raster.bands", &json!({"red": 3, "green": 2, "blue": 1})).unwrap(); diff --git a/crates/ui-egui/src/panes.rs b/crates/ui-egui/src/panes.rs index 10da1cc63..5a81a1419 100644 --- a/crates/ui-egui/src/panes.rs +++ b/crates/ui-egui/src/panes.rs @@ -3897,6 +3897,51 @@ fn legend_list(app: &mut RustGisApp, ui: &mut Ui, l: &Layer, t: &Tokens) { ); } +/// Graduated Colors' bounds typed by hand: each class's upper bound as a number, a button to take +/// a bound out, and one to split the widest class in two. A bound being dragged or typed is held +/// here and applied once, when the drag stops or the field is left, so a drag is one undo step. +fn manual_breaks(app: &mut RustGisApp, ui: &mut Ui, field: &str, breaks: &[f64], min: f64, ramp: &Ramp) { + let mut b = breaks.to_vec(); + let speed = ((b.last().copied().unwrap_or(min) - min).abs() / 200.0).max(1e-9); + let mut apply = false; + let mut remove = None; + for (i, v) in b.iter_mut().enumerate() { + ui.horizontal(|ui| { + let id = egui::Id::unique(("manual-break", field, i)); + if let Some(held) = ui.data(|d| d.get_temp::(id)) { + *v = held; + } + let input_label = ui.label(crate::i18n::fmt("Class {n} up to", &[("n", &(i + 1).to_string())])); + let r = ui.add(egui::DragValue::new(v).speed(speed)).labelled_by(input_label.id); + if r.changed() { + ui.data_mut(|d| d.insert_temp(id, *v)); + } + if (r.drag_stopped() || r.lost_focus()) && ui.data(|d| d.get_temp::(id)).is_some() { + ui.data_mut(|d| d.remove_temp::(id)); + apply = true; + } + if breaks.len() > 1 && ui.small_button("×").on_hover_text(tl!("Take this bound out")).clicked() { + remove = Some(i); + } + }); + } + if let Some(i) = remove { + b.remove(i); + apply = true; + } + if b.len() < 12 && ui.button(tl!("Split the widest class")).clicked() { + let lower = |i: usize| if i == 0 { min } else { b[i - 1] }; + if let Some(i) = (0..b.len()).max_by(|&x, &y| (b[x] - lower(x)).total_cmp(&(b[y] - lower(y)))) { + b.insert(i, (lower(i) + b[i]) / 2.0); + apply = true; + } + } + if apply { + let _ = + app.run("layer.symbology", json!({"type": "graduated", "field": field, "method": "manual", "breaks": b, "ramp": ramp.name()})); + } +} + fn symbol_editor(app: &mut RustGisApp, ui: &mut Ui, s: Symbol, g: GeometryType, t: &Tokens) { egui::Frame::NONE.fill(t.pane_header).corner_radius(CornerRadius::same(8)).inner_margin(egui::Margin::same(10)).show(ui, |ui| { ui.set_width(ui.available_width()); @@ -5660,36 +5705,3 @@ mod tests { assert_eq!(h.state().session.revision, rev, "the menu ran a command on every frame"); } } - -/// Graduated Colors' bounds typed by hand: each class's upper bound as a number, a button to take -/// a bound out, and one to split the widest class in two. -fn manual_breaks(app: &mut RustGisApp, ui: &mut Ui, field: &str, breaks: &[f64], min: f64, ramp: &Ramp) { - let mut b = breaks.to_vec(); - let speed = ((b.last().copied().unwrap_or(min) - min).abs() / 200.0).max(1e-9); - let mut changed = false; - let mut remove = None; - for (i, v) in b.iter_mut().enumerate() { - ui.horizontal(|ui| { - let input_label = ui.label(crate::i18n::fmt("Class {n} up to", &[("n", &(i + 1).to_string())])); - changed |= ui.add(egui::DragValue::new(v).speed(speed)).labelled_by(input_label.id).changed(); - if breaks.len() > 1 && ui.small_button("×").on_hover_text(tl!("Take this bound out")).clicked() { - remove = Some(i); - } - }); - } - if let Some(i) = remove { - b.remove(i); - changed = true; - } - if b.len() < 12 && ui.button(tl!("Split the widest class")).clicked() { - let lower = |i: usize| if i == 0 { min } else { b[i - 1] }; - if let Some(i) = (0..b.len()).max_by(|&x, &y| (b[x] - lower(x)).total_cmp(&(b[y] - lower(y)))) { - b.insert(i, (lower(i) + b[i]) / 2.0); - changed = true; - } - } - if changed { - let _ = - app.run("layer.symbology", json!({"type": "graduated", "field": field, "method": "manual", "breaks": b, "ramp": ramp.name()})); - } -} From b71355731f39cf5dfdfe81a6ed6de6f2e18c6fa2 Mon Sep 17 00:00:00 2001 From: alteralto <182909819+alteralto@users.noreply.github.com> Date: Sat, 24 Oct 2026 19:13:19 +0300 Subject: [PATCH 4/4] fix(symbology): manual breaks show a refused edit, keep the closing bound, key held values by layer; breaks need the manual method --- crates/engine/src/cmd/layer.rs | 3 +++ crates/engine/src/tests.rs | 4 ++++ crates/ui-egui/src/panes.rs | 15 ++++++++++----- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/crates/engine/src/cmd/layer.rs b/crates/engine/src/cmd/layer.rs index 32abe367a..6b93c44b2 100644 --- a/crates/engine/src/cmd/layer.rs +++ b/crates/engine/src/cmd/layer.rs @@ -2679,6 +2679,9 @@ fn symbology(s: &mut Session, p: &Value) -> CmdResult { Some(_) => return Err("breaks are a list of numbers".into()), }; let method = match ps(p, "method").and_then(ClassMethod::from_name) { + Some(m) if m != ClassMethod::Manual && typed.is_some() => { + return Err(format!("breaks go with the manual method, not {}", m.label())); + } Some(m) => m, None if typed.is_some() => ClassMethod::Manual, None => ClassMethod::default(), diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index f91813e9b..656b94123 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -15376,6 +15376,10 @@ fn manual_breaks_are_the_ones_given() { let q = breaks(&s).0; s.run("layer.symbology", &json!({"layer": "Countries", "type": "graduated", "field": "pop_est", "method": "manual"})).unwrap(); assert_eq!(breaks(&s), (q, ClassMethod::Manual)); + // Bounds with a method that works them out are refused rather than ignored. + let mut p = manual(json!([2e7])); + p["method"] = json!("quantile"); + assert!(s.run("layer.symbology", &p).is_err()); // Not numbers, none, or more than twelve classes are refused, and the layer keeps its style. for bad in [json!([]), json!(["a lot"]), json!((1..=13).map(|i| i as f64 * 1e6).collect::>())] { let before = breaks(&s); diff --git a/crates/ui-egui/src/panes.rs b/crates/ui-egui/src/panes.rs index 5a81a1419..94295e664 100644 --- a/crates/ui-egui/src/panes.rs +++ b/crates/ui-egui/src/panes.rs @@ -3905,9 +3905,10 @@ fn manual_breaks(app: &mut RustGisApp, ui: &mut Ui, field: &str, breaks: &[f64], let speed = ((b.last().copied().unwrap_or(min) - min).abs() / 200.0).max(1e-9); let mut apply = false; let mut remove = None; + let (layer, last) = (app.session.active_layer, b.len().saturating_sub(1)); for (i, v) in b.iter_mut().enumerate() { ui.horizontal(|ui| { - let id = egui::Id::unique(("manual-break", field, i)); + let id = egui::Id::unique(("manual-break", layer, field, i)); if let Some(held) = ui.data(|d| d.get_temp::(id)) { *v = held; } @@ -3920,7 +3921,9 @@ fn manual_breaks(app: &mut RustGisApp, ui: &mut Ui, field: &str, breaks: &[f64], ui.data_mut(|d| d.remove_temp::(id)); apply = true; } - if breaks.len() > 1 && ui.small_button("×").on_hover_text(tl!("Take this bound out")).clicked() { + // The last bound closes the classes at the field's largest value: taking it out would + // only bring it back. + if i < last && ui.small_button("×").on_hover_text(tl!("Take this bound out")).clicked() { remove = Some(i); } }); @@ -3936,9 +3939,11 @@ fn manual_breaks(app: &mut RustGisApp, ui: &mut Ui, field: &str, breaks: &[f64], apply = true; } } - if apply { - let _ = - app.run("layer.symbology", json!({"type": "graduated", "field": field, "method": "manual", "breaks": b, "ramp": ramp.name()})); + if apply + && let Err(e) = + app.run("layer.symbology", json!({"type": "graduated", "field": field, "method": "manual", "breaks": b, "ramp": ramp.name()})) + { + app.session.notify(e, true); } }