From 4914600b27977396b70f02972185fef27ece1706 Mon Sep 17 00:00:00 2001 From: ninhhaidang Date: Tue, 3 Nov 2026 22:05:15 +0700 Subject: [PATCH 01/13] feat(symbology): Invert Color Ramp reverses the colors of a layer's classes (#1355) `layer.invertRamp` reverses the order of the colors of a Graduated Colors or Unique Values layer's classes: the first class takes the last one's color, and so on. Colors edited by hand travel with the reversal, a polygon fill keeps its own opacity, and applying it twice gives the original back. On a Stretch raster it switches the existing invert option. Single and Proportional layers have no color scheme to invert and are refused. The Symbology pane shows an Invert Color Ramp button on the Color scheme row of Graduated and Unique Values layers. No existing command, parameter or project format changes. --- crates/engine/src/cmd/layer.rs | 16 +++++++++++++++ crates/engine/src/tests.rs | 37 ++++++++++++++++++++++++++++++++++ crates/map/src/raster.rs | 11 ++++++++++ crates/map/src/symbology.rs | 36 +++++++++++++++++++++++++++++++++ crates/ui-egui/src/panes.rs | 21 ++++++++++++++++--- docs/user-guide/cartography.md | 7 +++++++ 6 files changed, 125 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/cmd/layer.rs b/crates/engine/src/cmd/layer.rs index 9158c57b8..fdd6c746a 100644 --- a/crates/engine/src/cmd/layer.rs +++ b/crates/engine/src/cmd/layer.rs @@ -154,6 +154,22 @@ pub fn commands() -> Vec { }) }).params(r#"{"layer"?, "ramp": colour ramp name}"#) .when(need_layer), + C::new("layer.invertRamp", "Invert Color Ramp", "Style › Drawing", |s, p| { + let id = s.layer_id(p)?; + let l = s.map_mut().layer_mut(id).ok_or("no such layer")?; + let done = match &mut l.raster { + Some(rl) => rl.style.invert_ramp(), + None => { + let g = l.geometry_type(); + l.renderer.reverse_ramp(g) + } + }; + if !done { + return Err("the layer has no color scheme to invert: use Graduated or Unique Values (or a Stretch raster)".into()); + } + Ok(Value::Null) + }).params(r#"{"layer"?}"#) + .when(need_layer), C::new("layer.classStyle", "Class Style", "Style › Drawing", class_style) .when(need_feature_layer) .params( diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index f7a342f91..eb715a78d 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -25481,3 +25481,40 @@ fn a_pdf_says_which_frames_and_pages_it_draws_as_pictures() { s.notices_since(mark)[0].text.ends_with("(page 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20 and 5 more)") ); } + +#[test] +fn layer_invert_ramp_reverses_the_class_colors() { + // #1355: Invert Color Ramp reverses the order of a layer's class colours. + let mut s = world(); + let colors = |s: &Session| -> Vec { + match &s.map().layer_by_name("Countries").unwrap().renderer { + rustgis_map::Renderer::Graduated { symbols, .. } => symbols.iter().map(|y| y.fill).collect(), + rustgis_map::Renderer::Unique { classes, .. } => classes.iter().map(|c| c.1.fill).collect(), + _ => vec![], + } + }; + // A single symbol has no color scheme to invert. + s.run("layer.symbology", &json!({"layer": "Countries", "type": "single"})).unwrap(); + assert!(s.run("layer.invertRamp", &json!({"layer": "Countries"})).is_err()); + s.run("layer.symbology", &json!({"layer": "Countries", "type": "graduated", "field": "pop_est", "classes": 5})).unwrap(); + let before = colors(&s); + assert_eq!(before.len(), 5); + s.run("layer.invertRamp", &json!({"layer": "Countries"})).unwrap(); + let mut reversed = before.clone(); + reversed.reverse(); + assert_eq!(colors(&s), reversed); + // Twice is the original; one undo step undoes one inversion. + s.run("layer.invertRamp", &json!({"layer": "Countries"})).unwrap(); + assert_eq!(colors(&s), before); + s.run("layer.invertRamp", &json!({"layer": "Countries"})).unwrap(); + s.run("edit.undo", &Value::Null).unwrap(); + assert_eq!(colors(&s), before); + // Unique Values too. + s.run("layer.symbology", &json!({"layer": "Countries", "type": "unique", "field": "income_grp"})).unwrap(); + let before = colors(&s); + s.run("layer.invertRamp", &json!({"layer": "Countries"})).unwrap(); + let mut reversed = before.clone(); + reversed.reverse(); + assert_eq!(colors(&s), reversed); + assert!(s.run("layer.invertRamp", &json!({"layer": "Nope"})).is_err()); +} diff --git a/crates/map/src/raster.rs b/crates/map/src/raster.rs index ae357e7b1..6d03f2944 100644 --- a/crates/map/src/raster.rs +++ b/crates/map/src/raster.rs @@ -735,6 +735,17 @@ impl RasterStyle { } } + /// Flips a Stretch's colour scheme end for end (#1355). Returns false for other renderers. + pub fn invert_ramp(&mut self) -> bool { + match &mut self.renderer { + RasterRenderer::Stretch { invert, .. } => { + *invert = !*invert; + true + } + _ => false, + } + } + /// Recolours with a colour scheme. pub fn set_ramp(&mut self, ramp: Ramp) { match &mut self.renderer { diff --git a/crates/map/src/symbology.rs b/crates/map/src/symbology.rs index 212b2059f..50c94df15 100644 --- a/crates/map/src/symbology.rs +++ b/crates/map/src/symbology.rs @@ -163,6 +163,23 @@ impl Symbol { self } + /// The colours a ramp gives a class, taken from `src`: the fill, and a line's or polygon's + /// outline. A polygon keeps its own fill opacity. + fn with_colors_of(mut self, src: &Symbol, g: GeometryType) -> Symbol { + match g { + GeometryType::Polyline => { + self.fill = src.fill; + self.stroke = src.stroke; + } + GeometryType::Polygon => { + self.fill = src.fill.with_alpha(self.fill.a()); + self.stroke = src.stroke; + } + _ => self.fill = src.fill, + } + self + } + /// Ramp colours change hue, not a polygon's existing fill opacity (even below 150). fn with_ramp_fill(self, c: Rgba, g: GeometryType) -> Symbol { let mut out = self.with_fill(c, g); @@ -1541,6 +1558,25 @@ impl Renderer { } } + /// Reverses the colours of the classes (#1355): the first class takes the last one's colour, and + /// so on. Colours edited by hand travel with their class's old position, and a polygon fill + /// keeps its own opacity. Applied twice it gives the original back. Returns false for a + /// renderer without classes coloured from a ramp. + pub fn reverse_ramp(&mut self, g: GeometryType) -> bool { + fn reversed(symbols: &mut [&mut Symbol], g: GeometryType) { + let old: Vec = symbols.iter().map(|s| **s).collect(); + for (s, from) in symbols.iter_mut().zip(old.iter().rev()) { + **s = s.with_colors_of(from, g); + } + } + match self { + Renderer::Unique { classes, .. } => reversed(&mut classes.iter_mut().map(|c| &mut c.1).collect::>(), g), + Renderer::Graduated { symbols, .. } => reversed(&mut symbols.iter_mut().collect::>(), g), + _ => return false, + } + true + } + /// Recolours every class from a ramp, preserving each polygon fill's opacity. pub fn apply_ramp(&mut self, ramp: Ramp, g: GeometryType) { match self { diff --git a/crates/ui-egui/src/panes.rs b/crates/ui-egui/src/panes.rs index f1493272c..8ede656e6 100644 --- a/crates/ui-egui/src/panes.rs +++ b/crates/ui-egui/src/panes.rs @@ -4214,7 +4214,7 @@ fn symbology(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { let e = if text.is_empty() { Value::Null } else { json!(text) }; let _ = app.run("layer.symbology", json!({"type": "unique", "expression": e, "ramp": ramp.name()})); } - ramp_picker(app, ui, ramp, t); + ramp_picker_with(app, ui, ramp, t, true); ui.add_space(6.0); ui.label( egui::RichText::new(crate::i18n::fmt("{n} classes", &[("n", &classes.len().to_string())])).size(11.5).color(t.text_dim), @@ -4251,7 +4251,7 @@ fn symbology(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { } }); } - ramp_picker(app, ui, ramp, t); + ramp_picker_with(app, ui, ramp, t, true); ui.add_space(6.0); legend_list(app, ui, &l, t); } @@ -4376,8 +4376,23 @@ fn field_combo(app: &mut RustGisApp, ui: &mut Ui, label: &str, cur: &str, fields } fn ramp_picker(app: &mut RustGisApp, ui: &mut Ui, cur: &Ramp, t: &Tokens) { + ramp_picker_with(app, ui, cur, t, false); +} + +/// The color scheme picker; `invert` adds an Invert Color Ramp button to its heading row, for a +/// layer whose classes take their colors from the scheme (#1355). +fn ramp_picker_with(app: &mut RustGisApp, ui: &mut Ui, cur: &Ramp, t: &Tokens, invert: bool) { ui.add_space(4.0); - ui.label(egui::RichText::new(tl!("Color scheme")).size(11.5).color(t.text_dim)); + ui.horizontal(|ui| { + ui.label(egui::RichText::new(tl!("Color scheme")).size(11.5).color(t.text_dim)); + if invert { + ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { + if ui.small_button(tl!("Invert Color Ramp")).on_hover_text(tl!("Reverse the order of the classes' colors.")).clicked() { + let _ = app.run("layer.invertRamp", json!({})); + } + }); + } + }); let grid = |app: &mut RustGisApp, ui: &mut Ui, id: &str, ramps: Vec| { // Two to a row, in the width left inside the group's indent. let w = (ui.available_width() - 8.0) / 2.0; diff --git a/docs/user-guide/cartography.md b/docs/user-guide/cartography.md index 900211869..1173fec34 100644 --- a/docs/user-guide/cartography.md +++ b/docs/user-guide/cartography.md @@ -182,6 +182,13 @@ As with other class colour edits, changing the field or graduated ranges starts a different scheme through `layer.symbology` resets polygon opacity to its default. Layer/group transparency still applies on top of the symbol's opacity in maps and exports. +**Invert Color Ramp** (Symbology pane, or `layer.invertRamp`) reverses the order of the colours of +a Graduated Colors or Unique Values layer's classes, so the highest class takes the lowest +class's colour. Colours edited by hand travel with the reversal, a polygon fill keeps its opacity, +and applying it twice gives the original back. Choosing a colour scheme again, or classing the +layer again, starts from the scheme's own order. On a Stretch raster it switches the existing +"Invert color scheme" option. + ### Line strokes A line layer draws one stroke by default. The **Strokes** section of the Symbology pane stacks From 7606e5271d866ecd568f05cf221134c73f926411 Mon Sep 17 00:00:00 2001 From: ninhhaidang Date: Tue, 3 Nov 2026 22:21:56 +0700 Subject: [PATCH 02/13] fix(symbology): Invert Color Ramp says which classes it leaves as they are (#1355) A class drawn with a stack of fills or strokes of its own (`class_fills`, `class_strokes`, and ``'s) is drawn instead of its symbol, so reversing the symbols' colors can't reach it, and the command answered Ok without saying so. Those stacks stay where they are, as moving them would carry a class's widths and patterns to another class, and a notice gives how many classes were left as they are. Tests: a hand-edited class color travels to the other end with each fill keeping its own opacity, and a class with fills of its own keeps them and is reported. --- crates/engine/src/cmd/layer.rs | 9 +++++++++ crates/engine/src/tests.rs | 28 ++++++++++++++++++++++++++++ crates/map/src/symbology.rs | 23 +++++++++++++++++++++-- docs/user-guide/cartography.md | 4 +++- 4 files changed, 61 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/cmd/layer.rs b/crates/engine/src/cmd/layer.rs index fdd6c746a..0a63aba29 100644 --- a/crates/engine/src/cmd/layer.rs +++ b/crates/engine/src/cmd/layer.rs @@ -157,16 +157,25 @@ pub fn commands() -> Vec { C::new("layer.invertRamp", "Invert Color Ramp", "Style › Drawing", |s, p| { let id = s.layer_id(p)?; let l = s.map_mut().layer_mut(id).ok_or("no such layer")?; + let mut kept = 0; let done = match &mut l.raster { Some(rl) => rl.style.invert_ramp(), None => { let g = l.geometry_type(); + kept = l.renderer.classes_with_own_stacks(); l.renderer.reverse_ramp(g) } }; if !done { return Err("the layer has no color scheme to invert: use Graduated or Unique Values (or a Stretch raster)".into()); } + if kept > 0 { + // Their own stacks are drawn instead of the symbol, so the reversal can't reach them. + s.notify( + format!("{kept} class(es) drawn with fills or strokes of their own kept their colors: edit those with Class Style or the stroke and fill editors"), + false, + ); + } Ok(Value::Null) }).params(r#"{"layer"?}"#) .when(need_layer), diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index eb715a78d..9bac912d6 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -25517,4 +25517,32 @@ fn layer_invert_ramp_reverses_the_class_colors() { reversed.reverse(); assert_eq!(colors(&s), reversed); assert!(s.run("layer.invertRamp", &json!({"layer": "Nope"})).is_err()); + + // A colour set by hand travels with the reversal, and each polygon fill keeps its own opacity. + s.run("layer.symbology", &json!({"layer": "Countries", "type": "graduated", "field": "pop_est", "classes": 5})).unwrap(); + s.run("layer.classStyle", &json!({"layer": "Countries", "index": 0, "color": "#11223344"})).unwrap(); + let before = colors(&s); + s.run("layer.invertRamp", &json!({"layer": "Countries"})).unwrap(); + let after = colors(&s); + assert_eq!(after.len(), 5); + assert_eq!(after[4].with_alpha(0), before[0].with_alpha(0), "the edited colour moved to the other end"); + assert_eq!(after[4].a(), before[4].a(), "a fill keeps its own opacity"); + assert_eq!(after[0].a(), before[0].a()); + assert_eq!(before[0].a(), 0x44); + + // A class drawn with a stack of fills of its own is drawn instead of its symbol, so the + // reversal can't reach it: it keeps its fills, and the command says so. + s.run("layer.symbology", &json!({"layer": "Countries", "type": "graduated", "field": "pop_est", "classes": 5})).unwrap(); + s.run("layer.fillLayers", &json!({"layer": "Countries", "index": 0, "fills": [{"color": "#FF0000", "pattern": "solid"}]})).unwrap(); + let own = |s: &Session| match &s.map().layer_by_name("Countries").unwrap().renderer { + rustgis_map::Renderer::Graduated { class_fills, .. } => class_fills.keys().copied().collect::>(), + _ => vec![], + }; + let mark = s.notice_mark(); + let before = colors(&s); + s.run("layer.invertRamp", &json!({"layer": "Countries"})).unwrap(); + assert_eq!(own(&s), vec![0], "the class keeps its own fills"); + assert!(colors(&s) != before); + let notices = s.notices_since(mark); + assert!(notices.iter().any(|n| n.text.starts_with("1 class(es) drawn with fills or strokes of their own")), "{notices:?}"); } diff --git a/crates/map/src/symbology.rs b/crates/map/src/symbology.rs index 50c94df15..b16af9e77 100644 --- a/crates/map/src/symbology.rs +++ b/crates/map/src/symbology.rs @@ -1558,10 +1558,29 @@ impl Renderer { } } + /// How many classes are drawn with a stack of fills or strokes of their own (`class_fills`, + /// `class_strokes`, and ``'s), which [`Renderer::reverse_ramp`] leaves as + /// they are: the stack is drawn instead of the class's symbol. + pub fn classes_with_own_stacks(&self) -> usize { + match self { + Renderer::Unique { class_fills, class_strokes, other_fills, other_strokes, .. } => { + let keys: std::collections::BTreeSet<&String> = class_fills.keys().chain(class_strokes.keys()).collect(); + keys.len() + usize::from(!other_fills.is_empty() || !other_strokes.is_empty()) + } + Renderer::Graduated { class_fills, class_strokes, .. } => { + let keys: std::collections::BTreeSet<&usize> = class_fills.keys().chain(class_strokes.keys()).collect(); + keys.len() + } + _ => 0, + } + } + /// Reverses the colours of the classes (#1355): the first class takes the last one's colour, and /// so on. Colours edited by hand travel with their class's old position, and a polygon fill - /// keeps its own opacity. Applied twice it gives the original back. Returns false for a - /// renderer without classes coloured from a ramp. + /// keeps its own opacity. Applied twice it gives the original back. A class with a stack of + /// fills or strokes of its own keeps it, as the stack is drawn instead of the symbol + /// ([`Renderer::classes_with_own_stacks`]). Returns false for a renderer without classes + /// coloured from a ramp. pub fn reverse_ramp(&mut self, g: GeometryType) -> bool { fn reversed(symbols: &mut [&mut Symbol], g: GeometryType) { let old: Vec = symbols.iter().map(|s| **s).collect(); diff --git a/docs/user-guide/cartography.md b/docs/user-guide/cartography.md index 1173fec34..8654e299c 100644 --- a/docs/user-guide/cartography.md +++ b/docs/user-guide/cartography.md @@ -185,7 +185,9 @@ Layer/group transparency still applies on top of the symbol's opacity in maps an **Invert Color Ramp** (Symbology pane, or `layer.invertRamp`) reverses the order of the colours of a Graduated Colors or Unique Values layer's classes, so the highest class takes the lowest class's colour. Colours edited by hand travel with the reversal, a polygon fill keeps its opacity, -and applying it twice gives the original back. Choosing a colour scheme again, or classing the +and applying it twice gives the original back. A class drawn with fills or strokes of its own +(stacked fills, line strokes) keeps them, since they are drawn instead of the class's symbol; a +notice says how many classes were left as they are. Choosing a colour scheme again, or classing the layer again, starts from the scheme's own order. On a Stretch raster it switches the existing "Invert color scheme" option. From 75b5ab73f5db419ab5bf2a7acd84eb2fc44d4974 Mon Sep 17 00:00:00 2001 From: ninhhaidang Date: Tue, 3 Nov 2026 22:35:22 +0700 Subject: [PATCH 03/13] fix(symbology): Invert Color Ramp reverses along the legend's order and skips what isn't drawn (#1355) `reverse_ramp` reversed the classes in the order they are stored. Unique Values lists them in `class_order` (numbers before text, 9 before 10), so a layer stored as 10, 100, 9 kept its displayed sequence. It now reverses the colors along the legend. The reversal takes part only the classes that show their symbol's colors: a repeat of a Unique Values value (only the first entry draws) and a class drawn from a stack of fills or strokes of its own stay as they are, so they no longer shift the colors of the classes around them or leak a swapped outline. Docs: choosing a color scheme again starts from the scheme's order, but classing again on the same field, ranges and scheme keeps the reversed colors, as it keeps colors edited by hand. Tests: the order, a repeated value and a stacked class (map), a class with fills of its own keeping its colors while the others reverse among themselves, a Stretch raster switching its invert option and a classified raster being refused (engine). --- crates/engine/src/cmd/layer.rs | 12 ++- crates/engine/src/tests.rs | 28 ++++++- crates/map/src/symbology.rs | 130 ++++++++++++++++++++++++++------- docs/user-guide/cartography.md | 17 +++-- 4 files changed, 142 insertions(+), 45 deletions(-) diff --git a/crates/engine/src/cmd/layer.rs b/crates/engine/src/cmd/layer.rs index 0a63aba29..ae97b775f 100644 --- a/crates/engine/src/cmd/layer.rs +++ b/crates/engine/src/cmd/layer.rs @@ -157,22 +157,20 @@ pub fn commands() -> Vec { C::new("layer.invertRamp", "Invert Color Ramp", "Style › Drawing", |s, p| { let id = s.layer_id(p)?; let l = s.map_mut().layer_mut(id).ok_or("no such layer")?; - let mut kept = 0; - let done = match &mut l.raster { - Some(rl) => rl.style.invert_ramp(), + let kept = match &mut l.raster { + Some(rl) => rl.style.invert_ramp().then_some(0), None => { let g = l.geometry_type(); - kept = l.renderer.classes_with_own_stacks(); l.renderer.reverse_ramp(g) } }; - if !done { + let Some(kept) = kept else { return Err("the layer has no color scheme to invert: use Graduated or Unique Values (or a Stretch raster)".into()); - } + }; if kept > 0 { // Their own stacks are drawn instead of the symbol, so the reversal can't reach them. s.notify( - format!("{kept} class(es) drawn with fills or strokes of their own kept their colors: edit those with Class Style or the stroke and fill editors"), + format!("{kept} class(es) drawn with fills or strokes of their own were left as they are: edit those with their fill and stroke editors"), false, ); } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 9bac912d6..2777eb0e9 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -25542,7 +25542,31 @@ fn layer_invert_ramp_reverses_the_class_colors() { let before = colors(&s); s.run("layer.invertRamp", &json!({"layer": "Countries"})).unwrap(); assert_eq!(own(&s), vec![0], "the class keeps its own fills"); - assert!(colors(&s) != before); + // Class 0 keeps its colour, and the others reverse among themselves. + let after = colors(&s); + assert_eq!(after[0], before[0]); + assert_eq!(after[1..], [before[4], before[3], before[2], before[1]]); let notices = s.notices_since(mark); - assert!(notices.iter().any(|n| n.text.starts_with("1 class(es) drawn with fills or strokes of their own")), "{notices:?}"); + assert!( + notices.iter().any(|n| n.text.starts_with("1 class(es) drawn with fills or strokes of their own were left as they are")), + "{notices:?}" + ); + + // A Stretch raster switches its invert option, and other rasters are refused. + use rustgis_data::raster::{ColorHint, Crs, GeoTransform, Raster}; + let gt = GeoTransform { origin_x: 0.0, origin_y: 2.0, pixel_width: 1.0, pixel_height: -1.0 }; + let r = Raster::new("dem", 2, 2, vec![vec![1.0, 2.0, 3.0, 4.0]], gt, Crs::LonLat, None, ColorHint::Gray, "Float32").unwrap(); + s.map_mut().add_raster("dem", rustgis_map::RasterLayer::from_path(r, "dem.tif")); + s.run("raster.symbology", &json!({"layer": "dem", "type": "stretch", "ramp": "Terrain"})).unwrap(); + let inverted = |s: &Session| match s.map().layer_by_name("dem").and_then(|l| l.raster.as_ref()).map(|r| r.style.renderer.clone()) { + Some(rustgis_map::RasterRenderer::Stretch { invert, .. }) => Some(invert), + _ => None, + }; + assert_eq!(inverted(&s), Some(false)); + s.run("layer.invertRamp", &json!({"layer": "dem"})).unwrap(); + assert_eq!(inverted(&s), Some(true)); + s.run("layer.invertRamp", &json!({"layer": "dem"})).unwrap(); + assert_eq!(inverted(&s), Some(false)); + s.run("raster.symbology", &json!({"layer": "dem", "type": "classify", "classes": 2, "method": "equalInterval"})).unwrap(); + assert!(s.run("layer.invertRamp", &json!({"layer": "dem"})).is_err(), "a classified raster has no invert option"); } diff --git a/crates/map/src/symbology.rs b/crates/map/src/symbology.rs index b16af9e77..6957fb161 100644 --- a/crates/map/src/symbology.rs +++ b/crates/map/src/symbology.rs @@ -1558,42 +1558,68 @@ impl Renderer { } } - /// How many classes are drawn with a stack of fills or strokes of their own (`class_fills`, - /// `class_strokes`, and ``'s), which [`Renderer::reverse_ramp`] leaves as - /// they are: the stack is drawn instead of the class's symbol. - pub fn classes_with_own_stacks(&self) -> usize { + /// The classes whose colors a ramp gives, in the order the legend lists them, and how many + /// more the ramp can't recolor: a class with a stack of fills or strokes of its own + /// (`class_fills`, `class_strokes`) is drawn from the stack instead of its symbol. Only the + /// first entry of a Unique Values value is drawn, so a repeated value counts once. They are + /// indices into `classes` (Unique Values) or `symbols` (Graduated Colors). `None` for the + /// other renderers. + fn ramp_slots(&self) -> Option<(Vec, usize)> { + let (mut slots, mut kept) = (Vec::new(), 0); match self { - Renderer::Unique { class_fills, class_strokes, other_fills, other_strokes, .. } => { - let keys: std::collections::BTreeSet<&String> = class_fills.keys().chain(class_strokes.keys()).collect(); - keys.len() + usize::from(!other_fills.is_empty() || !other_strokes.is_empty()) + Renderer::Unique { classes, class_fills, class_strokes, .. } => { + let mut seen = std::collections::HashSet::new(); + for i in self.class_order() { + let Some((value, _)) = classes.get(i) else { continue }; + if !seen.insert(value.as_str()) { + continue; + } + if class_fills.contains_key(value) || class_strokes.contains_key(value) { + kept += 1; + } else { + slots.push(i); + } + } } - Renderer::Graduated { class_fills, class_strokes, .. } => { - let keys: std::collections::BTreeSet<&usize> = class_fills.keys().chain(class_strokes.keys()).collect(); - keys.len() + Renderer::Graduated { symbols, class_fills, class_strokes, .. } => { + for i in 0..symbols.len() { + if class_fills.contains_key(&i) || class_strokes.contains_key(&i) { + kept += 1; + } else { + slots.push(i); + } + } } - _ => 0, + _ => return None, } + Some((slots, kept)) } - /// Reverses the colours of the classes (#1355): the first class takes the last one's colour, and - /// so on. Colours edited by hand travel with their class's old position, and a polygon fill - /// keeps its own opacity. Applied twice it gives the original back. A class with a stack of - /// fills or strokes of its own keeps it, as the stack is drawn instead of the symbol - /// ([`Renderer::classes_with_own_stacks`]). Returns false for a renderer without classes - /// coloured from a ramp. - pub fn reverse_ramp(&mut self, g: GeometryType) -> bool { - fn reversed(symbols: &mut [&mut Symbol], g: GeometryType) { - let old: Vec = symbols.iter().map(|s| **s).collect(); - for (s, from) in symbols.iter_mut().zip(old.iter().rev()) { - **s = s.with_colors_of(from, g); - } - } + fn class_symbol_mut(&mut self, i: usize) -> Option<&mut Symbol> { match self { - Renderer::Unique { classes, .. } => reversed(&mut classes.iter_mut().map(|c| &mut c.1).collect::>(), g), - Renderer::Graduated { symbols, .. } => reversed(&mut symbols.iter_mut().collect::>(), g), - _ => return false, + Renderer::Unique { classes, .. } => classes.get_mut(i).map(|c| &mut c.1), + Renderer::Graduated { symbols, .. } => symbols.get_mut(i), + _ => None, } - true + } + + /// Reverses the colours of the classes (#1355), in the order the legend lists them: the first + /// class takes the last one's colour, and so on. Colours edited by hand travel with their + /// class's old position, and a polygon fill keeps its own opacity. Applied twice it gives the + /// original back. A class drawn from a stack of fills or strokes of its own, and a repeat of a + /// Unique Values value (which no feature draws), keep theirs and take no part in the reversal. + /// Returns how many classes were kept for their own stacks, or `None` for a renderer without + /// classes coloured from a ramp. + pub fn reverse_ramp(&mut self, g: GeometryType) -> Option { + let (slots, kept) = self.ramp_slots()?; + let old: Vec = slots.iter().filter_map(|&i| self.class_symbol_mut(i).map(|s| *s)).collect(); + for (pos, &i) in slots.iter().enumerate() { + let from = old.len().checked_sub(1 + pos).and_then(|k| old.get(k)); + if let (Some(sym), Some(from)) = (self.class_symbol_mut(i), from) { + *sym = sym.with_colors_of(from, g); + } + } + Some(kept) } /// Recolours every class from a ramp, preserving each polygon fill's opacity. @@ -4014,4 +4040,52 @@ mod tests { more.carry_edits_from(&gold, g); assert_eq!(more, fresh, "another number of classes starts fresh"); } + + fn unique_with(values: &[(&str, u8)]) -> Renderer { + let sym = |v: u8| Symbol { fill: Rgba::rgb(v, 0, 0), ..Symbol::default_for(GeometryType::Point, 0) }; + Renderer::Unique { + field: "f".into(), + classes: values.iter().map(|(k, v)| (k.to_string(), sym(*v))).collect(), + other: sym(0), + ramp: Ramp::Greys, + class_labels: BTreeMap::new(), + other_label: None, + class_strokes: BTreeMap::new(), + other_strokes: Vec::new(), + class_fills: Default::default(), + other_fills: Vec::new(), + expression: None, + } + } + + /// #1355: Invert Color Ramp reverses the colours along the legend's order, which is not the + /// order the classes are stored in; a value repeated in the classes isn't drawn, so it takes + /// no part; and a class with a stack of its own keeps its colours without shifting the rest. + #[test] + fn reverse_ramp_follows_the_legend_and_skips_what_is_not_drawn() { + let reds = |r: &Renderer| match r { + Renderer::Unique { classes, .. } => classes.iter().map(|c| (c.0.clone(), c.1.fill.r())).collect::>(), + _ => vec![], + }; + let g = GeometryType::Point; + // Stored 10, 100, 9: the legend lists 9, 10, 100 (numbers), so 9 and 100 swap colours. + let mut r = unique_with(&[("10", 1), ("100", 2), ("9", 3)]); + assert_eq!(r.reverse_ramp(g), Some(0)); + assert_eq!(reds(&r), vec![("10".into(), 1), ("100".into(), 3), ("9".into(), 2)]); + r.reverse_ramp(g); + assert_eq!(reds(&r), vec![("10".into(), 1), ("100".into(), 2), ("9".into(), 3)], "twice is the original"); + // A repeat of a value is not drawn: it stays, and the drawn entries reverse among themselves. + let mut r = unique_with(&[("a", 1), ("b", 2), ("a", 9), ("c", 3)]); + r.reverse_ramp(g); + assert_eq!(reds(&r), vec![("a".into(), 3), ("b".into(), 2), ("a".into(), 9), ("c".into(), 1)]); + // A class with a stack of its own is kept, counted, and the others reverse among themselves. + let mut r = unique_with(&[("a", 1), ("b", 2), ("c", 3), ("d", 4)]); + if let Renderer::Unique { class_strokes, .. } = &mut r { + class_strokes.insert("a".into(), Vec::new()); + } + assert_eq!(r.reverse_ramp(g), Some(1)); + assert_eq!(reds(&r), vec![("a".into(), 1), ("b".into(), 4), ("c".into(), 3), ("d".into(), 2)]); + // Nothing to reverse for a single symbol. + assert_eq!(Renderer::Single(Symbol::default_for(g, 0)).reverse_ramp(g), None); + } } diff --git a/docs/user-guide/cartography.md b/docs/user-guide/cartography.md index 8654e299c..78dab5a30 100644 --- a/docs/user-guide/cartography.md +++ b/docs/user-guide/cartography.md @@ -182,14 +182,15 @@ As with other class colour edits, changing the field or graduated ranges starts a different scheme through `layer.symbology` resets polygon opacity to its default. Layer/group transparency still applies on top of the symbol's opacity in maps and exports. -**Invert Color Ramp** (Symbology pane, or `layer.invertRamp`) reverses the order of the colours of -a Graduated Colors or Unique Values layer's classes, so the highest class takes the lowest -class's colour. Colours edited by hand travel with the reversal, a polygon fill keeps its opacity, -and applying it twice gives the original back. A class drawn with fills or strokes of its own -(stacked fills, line strokes) keeps them, since they are drawn instead of the class's symbol; a -notice says how many classes were left as they are. Choosing a colour scheme again, or classing the -layer again, starts from the scheme's own order. On a Stretch raster it switches the existing -"Invert color scheme" option. +**Invert Color Ramp** (Symbology pane, or `layer.invertRamp`) reverses the colours of a Graduated +Colors or Unique Values layer's classes along the legend's order, so the highest class takes the +lowest class's colour. Colours edited by hand travel with the reversal, a polygon fill keeps its +opacity, and applying it twice gives the original back. A class drawn with fills or strokes of its +own (stacked fills, line strokes) keeps them, since they are drawn instead of the class's symbol, +and the others reverse among themselves; a notice says how many classes were left as they are. +Choosing a colour scheme again starts from the scheme's own order, but classing the layer again +on the same field, ranges and scheme keeps the reversed colours, as it keeps other colours edited +by hand. On a Stretch raster the command switches the existing "Invert color scheme" option. ### Line strokes From 71680537402feccfa9abaffaeb65cd55f66194a1 Mon Sep 17 00:00:00 2001 From: ninhhaidang Date: Tue, 3 Nov 2026 22:54:57 +0700 Subject: [PATCH 04/13] docs(symbology): Invert Color Ramp leaves a polygon fill's opacity at its position (#1355) --- docs/user-guide/cartography.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/user-guide/cartography.md b/docs/user-guide/cartography.md index 78dab5a30..7ed808b27 100644 --- a/docs/user-guide/cartography.md +++ b/docs/user-guide/cartography.md @@ -184,8 +184,10 @@ Layer/group transparency still applies on top of the symbol's opacity in maps an **Invert Color Ramp** (Symbology pane, or `layer.invertRamp`) reverses the colours of a Graduated Colors or Unique Values layer's classes along the legend's order, so the highest class takes the -lowest class's colour. Colours edited by hand travel with the reversal, a polygon fill keeps its -opacity, and applying it twice gives the original back. A class drawn with fills or strokes of its +lowest class's colour. Colours edited by hand travel with the reversal, but a polygon fill's +opacity stays with its position in the legend, as it does when a colour scheme is applied: a +semi-transparent fill picked for the first class is still the first class's opacity afterwards. +Applying it twice gives the original back. A class drawn with fills or strokes of its own (stacked fills, line strokes) keeps them, since they are drawn instead of the class's symbol, and the others reverse among themselves; a notice says how many classes were left as they are. Choosing a colour scheme again starts from the scheme's own order, but classing the layer again From e2963a93f80f86119d5d370302bbc5ec2f9924fa Mon Sep 17 00:00:00 2001 From: ninhhaidang Date: Tue, 3 Nov 2026 23:24:34 +0700 Subject: [PATCH 05/13] fix(symbology): Invert Color Ramp keeps only the classes whose own stack is drawn (#1355) A class's own stack replaces its symbol only for the geometry that draws it: a polygon's stacked fills, a line's or a point's stacked strokes. A polygon's stacked strokes and a line's stacked fills are not drawn, and an empty stack is no stack (the renderer ignores it), so those classes now take part in the reversal instead of keeping a color nothing shows. The Color scheme heading is wrapped in a row only where the Invert Color Ramp button is, so the raster panes' layout is as it was. Tests: every combination of stack kind, geometry and empty stack (map); an inversion that leaves nothing as it was says nothing, and a class's own fills are the same after it (engine). --- crates/engine/src/tests.rs | 10 +++-- crates/map/src/symbology.rs | 76 ++++++++++++++++++++++++++++------ crates/ui-egui/src/panes.rs | 13 +++--- docs/user-guide/cartography.md | 8 ++-- 4 files changed, 84 insertions(+), 23 deletions(-) diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 2777eb0e9..b3bab82c6 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -25499,7 +25499,9 @@ fn layer_invert_ramp_reverses_the_class_colors() { s.run("layer.symbology", &json!({"layer": "Countries", "type": "graduated", "field": "pop_est", "classes": 5})).unwrap(); let before = colors(&s); assert_eq!(before.len(), 5); + let mark = s.notice_mark(); s.run("layer.invertRamp", &json!({"layer": "Countries"})).unwrap(); + assert!(s.notices_since(mark).is_empty(), "nothing was left as it was, so nothing is said: {:?}", s.notices_since(mark)); let mut reversed = before.clone(); reversed.reverse(); assert_eq!(colors(&s), reversed); @@ -25535,13 +25537,15 @@ fn layer_invert_ramp_reverses_the_class_colors() { s.run("layer.symbology", &json!({"layer": "Countries", "type": "graduated", "field": "pop_est", "classes": 5})).unwrap(); s.run("layer.fillLayers", &json!({"layer": "Countries", "index": 0, "fills": [{"color": "#FF0000", "pattern": "solid"}]})).unwrap(); let own = |s: &Session| match &s.map().layer_by_name("Countries").unwrap().renderer { - rustgis_map::Renderer::Graduated { class_fills, .. } => class_fills.keys().copied().collect::>(), - _ => vec![], + rustgis_map::Renderer::Graduated { class_fills, .. } => class_fills.clone(), + _ => Default::default(), }; let mark = s.notice_mark(); let before = colors(&s); + let fills_before = own(&s); + assert_eq!(fills_before.keys().copied().collect::>(), vec![0]); s.run("layer.invertRamp", &json!({"layer": "Countries"})).unwrap(); - assert_eq!(own(&s), vec![0], "the class keeps its own fills"); + assert_eq!(own(&s), fills_before, "the class keeps its own fills, as they were"); // Class 0 keeps its colour, and the others reverse among themselves. let after = colors(&s); assert_eq!(after[0], before[0]); diff --git a/crates/map/src/symbology.rs b/crates/map/src/symbology.rs index 6957fb161..232a24736 100644 --- a/crates/map/src/symbology.rs +++ b/crates/map/src/symbology.rs @@ -1559,12 +1559,18 @@ impl Renderer { } /// The classes whose colors a ramp gives, in the order the legend lists them, and how many - /// more the ramp can't recolor: a class with a stack of fills or strokes of its own - /// (`class_fills`, `class_strokes`) is drawn from the stack instead of its symbol. Only the - /// first entry of a Unique Values value is drawn, so a repeated value counts once. They are - /// indices into `classes` (Unique Values) or `symbols` (Graduated Colors). `None` for the - /// other renderers. - fn ramp_slots(&self) -> Option<(Vec, usize)> { + /// more the ramp can't recolor: a class whose own stack of fills (a polygon) or strokes (a line + /// or a point's marks) is drawn instead of its symbol (see the drawing in `rustgis-render`: + /// a polygon's stacked strokes and a line's stacked fills are not drawn at all). An empty stack + /// is no stack. Only the first entry of a Unique Values value is drawn, so a repeated value + /// counts once. They are indices into `classes` (Unique Values) or `symbols` (Graduated + /// Colors). `None` for the other renderers. + fn ramp_slots(&self, g: GeometryType) -> Option<(Vec, usize)> { + // Whether a class's own stacks replace its symbol, for the geometry the layer draws. + let replaced = |fills: Option<&Vec>, strokes: Option<&Vec>| match g { + GeometryType::Polygon => fills.is_some_and(|f| !f.is_empty()), + _ => strokes.is_some_and(|s| !s.is_empty()), + }; let (mut slots, mut kept) = (Vec::new(), 0); match self { Renderer::Unique { classes, class_fills, class_strokes, .. } => { @@ -1574,7 +1580,7 @@ impl Renderer { if !seen.insert(value.as_str()) { continue; } - if class_fills.contains_key(value) || class_strokes.contains_key(value) { + if replaced(class_fills.get(value), class_strokes.get(value)) { kept += 1; } else { slots.push(i); @@ -1583,7 +1589,7 @@ impl Renderer { } Renderer::Graduated { symbols, class_fills, class_strokes, .. } => { for i in 0..symbols.len() { - if class_fills.contains_key(&i) || class_strokes.contains_key(&i) { + if replaced(class_fills.get(&i), class_strokes.get(&i)) { kept += 1; } else { slots.push(i); @@ -1606,12 +1612,12 @@ impl Renderer { /// Reverses the colours of the classes (#1355), in the order the legend lists them: the first /// class takes the last one's colour, and so on. Colours edited by hand travel with their /// class's old position, and a polygon fill keeps its own opacity. Applied twice it gives the - /// original back. A class drawn from a stack of fills or strokes of its own, and a repeat of a - /// Unique Values value (which no feature draws), keep theirs and take no part in the reversal. + /// original back. A class drawn from a stack of its own (fills for a polygon, strokes for a line or + /// a point), and a repeat of a Unique Values value (which no feature draws), keep theirs and take no part in the reversal. /// Returns how many classes were kept for their own stacks, or `None` for a renderer without /// classes coloured from a ramp. pub fn reverse_ramp(&mut self, g: GeometryType) -> Option { - let (slots, kept) = self.ramp_slots()?; + let (slots, kept) = self.ramp_slots(g)?; let old: Vec = slots.iter().filter_map(|&i| self.class_symbol_mut(i).map(|s| *s)).collect(); for (pos, &i) in slots.iter().enumerate() { let from = old.len().checked_sub(1 + pos).and_then(|k| old.get(k)); @@ -4081,11 +4087,57 @@ mod tests { // A class with a stack of its own is kept, counted, and the others reverse among themselves. let mut r = unique_with(&[("a", 1), ("b", 2), ("c", 3), ("d", 4)]); if let Renderer::Unique { class_strokes, .. } = &mut r { - class_strokes.insert("a".into(), Vec::new()); + class_strokes.insert("a".into(), vec![LineStroke::default()]); } assert_eq!(r.reverse_ramp(g), Some(1)); assert_eq!(reds(&r), vec![("a".into(), 1), ("b".into(), 4), ("c".into(), 3), ("d".into(), 2)]); // Nothing to reverse for a single symbol. assert_eq!(Renderer::Single(Symbol::default_for(g, 0)).reverse_ramp(g), None); } + + /// #1355: which stack replaces a class's symbol depends on what the layer draws: a polygon's + /// own fills, a line's or a point's own strokes. The other kind is not drawn, and an empty + /// stack is no stack, so those classes take part in the reversal. + #[test] + fn reverse_ramp_keeps_only_the_classes_whose_stack_is_drawn() { + let reds = |r: &Renderer| match r { + Renderer::Unique { classes, .. } => classes.iter().map(|c| c.1.fill.r()).collect::>(), + _ => vec![], + }; + let with = |fills: bool, strokes: bool, empty: bool| { + let mut r = unique_with(&[("a", 1), ("b", 2), ("c", 3)]); + if let Renderer::Unique { class_fills, class_strokes, .. } = &mut r { + if fills { + class_fills.insert("a".into(), if empty { vec![] } else { vec![FillLayer::default()] }); + } + if strokes { + class_strokes.insert("a".into(), if empty { vec![] } else { vec![LineStroke::default()] }); + } + } + r + }; + let (kept, reversed) = (vec![1, 3, 2], vec![3, 2, 1]); + // (stacks of class a, geometry) -> the classes that reverse, and how many are kept. + for (fills, strokes, empty, g, take_part) in [ + (true, false, false, GeometryType::Polygon, false), + (false, true, false, GeometryType::Polygon, true), + (false, true, false, GeometryType::Polyline, false), + (true, false, false, GeometryType::Polyline, true), + (false, true, false, GeometryType::Point, false), + (true, false, false, GeometryType::Point, true), + (true, true, true, GeometryType::Polygon, true), + (true, true, true, GeometryType::Polyline, true), + ] { + let mut r = with(fills, strokes, empty); + let n = r.reverse_ramp(g); + let case = format!("fills {fills}, strokes {strokes}, empty {empty}, {g:?}"); + if take_part { + assert_eq!(n, Some(0), "{case}"); + assert_eq!(reds(&r), reversed, "{case}"); + } else { + assert_eq!(n, Some(1), "{case}"); + assert_eq!(reds(&r), kept, "{case}"); + } + } + } } diff --git a/crates/ui-egui/src/panes.rs b/crates/ui-egui/src/panes.rs index 8ede656e6..43ae50dc6 100644 --- a/crates/ui-egui/src/panes.rs +++ b/crates/ui-egui/src/panes.rs @@ -4383,16 +4383,19 @@ fn ramp_picker(app: &mut RustGisApp, ui: &mut Ui, cur: &Ramp, t: &Tokens) { /// layer whose classes take their colors from the scheme (#1355). fn ramp_picker_with(app: &mut RustGisApp, ui: &mut Ui, cur: &Ramp, t: &Tokens, invert: bool) { ui.add_space(4.0); - ui.horizontal(|ui| { - ui.label(egui::RichText::new(tl!("Color scheme")).size(11.5).color(t.text_dim)); - if invert { + let heading = egui::RichText::new(tl!("Color scheme")).size(11.5).color(t.text_dim); + if invert { + ui.horizontal(|ui| { + ui.label(heading); ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { if ui.small_button(tl!("Invert Color Ramp")).on_hover_text(tl!("Reverse the order of the classes' colors.")).clicked() { let _ = app.run("layer.invertRamp", json!({})); } }); - } - }); + }); + } else { + ui.label(heading); + } let grid = |app: &mut RustGisApp, ui: &mut Ui, id: &str, ramps: Vec| { // Two to a row, in the width left inside the group's indent. let w = (ui.available_width() - 8.0) / 2.0; diff --git a/docs/user-guide/cartography.md b/docs/user-guide/cartography.md index 7ed808b27..f81c9f19e 100644 --- a/docs/user-guide/cartography.md +++ b/docs/user-guide/cartography.md @@ -187,9 +187,11 @@ Colors or Unique Values layer's classes along the legend's order, so the highest lowest class's colour. Colours edited by hand travel with the reversal, but a polygon fill's opacity stays with its position in the legend, as it does when a colour scheme is applied: a semi-transparent fill picked for the first class is still the first class's opacity afterwards. -Applying it twice gives the original back. A class drawn with fills or strokes of its -own (stacked fills, line strokes) keeps them, since they are drawn instead of the class's symbol, -and the others reverse among themselves; a notice says how many classes were left as they are. +Applying it twice gives the original back. A class drawn from a stack of its own (stacked +fills on a polygon layer, stacked strokes on a line or point layer) keeps it, since the stack is +drawn instead of the class's symbol, and the others reverse among themselves; a notice says how +many classes were left as they are. A stack the layer doesn't draw (a polygon's stacked strokes) +doesn't count. Choosing a colour scheme again starts from the scheme's own order, but classing the layer again on the same field, ranges and scheme keeps the reversed colours, as it keeps other colours edited by hand. On a Stretch raster the command switches the existing "Invert color scheme" option. From 90deb10a6eebc920371fea2cd4df5007d478173f Mon Sep 17 00:00:00 2001 From: ninhhaidang Date: Tue, 3 Nov 2026 23:54:02 +0700 Subject: [PATCH 06/13] fix(symbology): Invert Color Ramp reverses the classes a stack takes its color from (#1355) A layer of a class's own stack with no color of its own takes the symbol's (`FillLayer::color`, `LineStroke::color`, a point's marks), so the class still shows its ramp color through it, as a road's black casing sits over a core that follows the class. Only a stack whose every layer sets its own color hides the symbol and keeps the class out of the reversal. Before, any drawn stack did. Docs: say which stack hides the symbol, and that a layer holding several kinds of geometry is reversed as its main kind, as choosing a color scheme does. Tests: a stack of strokes or fills with its own colors (kept), with some (reversed) and with none (reversed), for a polygon, a line and a point. --- crates/map/src/symbology.rs | 72 +++++++++++++++++++++++++++------- docs/user-guide/cartography.md | 13 +++--- 2 files changed, 66 insertions(+), 19 deletions(-) diff --git a/crates/map/src/symbology.rs b/crates/map/src/symbology.rs index 232a24736..45781e6fb 100644 --- a/crates/map/src/symbology.rs +++ b/crates/map/src/symbology.rs @@ -1559,17 +1559,21 @@ impl Renderer { } /// The classes whose colors a ramp gives, in the order the legend lists them, and how many - /// more the ramp can't recolor: a class whose own stack of fills (a polygon) or strokes (a line - /// or a point's marks) is drawn instead of its symbol (see the drawing in `rustgis-render`: - /// a polygon's stacked strokes and a line's stacked fills are not drawn at all). An empty stack - /// is no stack. Only the first entry of a Unique Values value is drawn, so a repeated value - /// counts once. They are indices into `classes` (Unique Values) or `symbols` (Graduated - /// Colors). `None` for the other renderers. + /// more the ramp can't recolor: a class whose own stack hides its symbol's colors. The stack + /// that is drawn depends on the geometry (a polygon's fills; a line's strokes or a point's + /// marks; see the drawing in `rustgis-render`, which doesn't draw a polygon's stacked strokes + /// or a line's stacked fills), an empty stack is no stack, and a layer of a stack without a + /// color of its own takes the symbol's (`FillLayer::color`, `LineStroke::color`), so the + /// class still shows its ramp color through it: a road's black casing over a core that follows + /// the class. Only a stack whose every layer sets its own color hides the symbol. Only the + /// first entry of a Unique Values value is drawn, so a repeated value counts once. They are + /// indices into `classes` (Unique Values) or `symbols` (Graduated Colors). `None` for the + /// other renderers. fn ramp_slots(&self, g: GeometryType) -> Option<(Vec, usize)> { - // Whether a class's own stacks replace its symbol, for the geometry the layer draws. + // Whether a class's own stacks hide its symbol's colors, for the geometry the layer draws. let replaced = |fills: Option<&Vec>, strokes: Option<&Vec>| match g { - GeometryType::Polygon => fills.is_some_and(|f| !f.is_empty()), - _ => strokes.is_some_and(|s| !s.is_empty()), + GeometryType::Polygon => fills.is_some_and(|f| !f.is_empty() && f.iter().all(|l| l.color.is_some())), + _ => strokes.is_some_and(|s| !s.is_empty() && s.iter().all(|l| l.color.is_some())), }; let (mut slots, mut kept) = (Vec::new(), 0); match self { @@ -1612,8 +1616,8 @@ impl Renderer { /// Reverses the colours of the classes (#1355), in the order the legend lists them: the first /// class takes the last one's colour, and so on. Colours edited by hand travel with their /// class's old position, and a polygon fill keeps its own opacity. Applied twice it gives the - /// original back. A class drawn from a stack of its own (fills for a polygon, strokes for a line or - /// a point), and a repeat of a Unique Values value (which no feature draws), keep theirs and take no part in the reversal. + /// original back. A class whose own stack hides its symbol (every layer of its fills, for a polygon, or + /// strokes, for a line or a point, sets a color of its own), and a repeat of a Unique Values value (which no feature draws), keep theirs and take no part in the reversal. /// Returns how many classes were kept for their own stacks, or `None` for a renderer without /// classes coloured from a ramp. pub fn reverse_ramp(&mut self, g: GeometryType) -> Option { @@ -4087,7 +4091,7 @@ mod tests { // A class with a stack of its own is kept, counted, and the others reverse among themselves. let mut r = unique_with(&[("a", 1), ("b", 2), ("c", 3), ("d", 4)]); if let Renderer::Unique { class_strokes, .. } = &mut r { - class_strokes.insert("a".into(), vec![LineStroke::default()]); + class_strokes.insert("a".into(), vec![LineStroke { color: Some(Rgba::rgb(9, 9, 9)), ..LineStroke::default() }]); } assert_eq!(r.reverse_ramp(g), Some(1)); assert_eq!(reds(&r), vec![("a".into(), 1), ("b".into(), 4), ("c".into(), 3), ("d".into(), 2)]); @@ -4107,11 +4111,12 @@ mod tests { let with = |fills: bool, strokes: bool, empty: bool| { let mut r = unique_with(&[("a", 1), ("b", 2), ("c", 3)]); if let Renderer::Unique { class_fills, class_strokes, .. } = &mut r { + let own = Some(Rgba::rgb(9, 9, 9)); if fills { - class_fills.insert("a".into(), if empty { vec![] } else { vec![FillLayer::default()] }); + class_fills.insert("a".into(), if empty { vec![] } else { vec![FillLayer { color: own, ..FillLayer::default() }] }); } if strokes { - class_strokes.insert("a".into(), if empty { vec![] } else { vec![LineStroke::default()] }); + class_strokes.insert("a".into(), if empty { vec![] } else { vec![LineStroke { color: own, ..LineStroke::default() }] }); } } r @@ -4140,4 +4145,43 @@ mod tests { } } } + + /// #1355: a stack layer without a color of its own takes the symbol's, so the class still shows + /// its ramp color through it and takes part in the reversal: a road's black casing over a core + /// that follows the class. Only a stack whose every layer has its own color hides the symbol. + #[test] + fn reverse_ramp_reverses_the_classes_a_stack_inherits_from() { + let reds = |r: &Renderer| match r { + Renderer::Unique { classes, .. } => classes.iter().map(|c| c.1.fill.r()).collect::>(), + _ => vec![], + }; + let own = Some(Rgba::rgb(9, 9, 9)); + let casing = LineStroke { color: own, width: Some(6.0), ..LineStroke::default() }; + let core = LineStroke::default(); + let (solid_own, solid_inherit) = (FillLayer { color: own, ..FillLayer::default() }, FillLayer::default()); + for (stack_of_strokes, g, hides) in [ + (vec![casing], GeometryType::Polyline, true), + (vec![casing, core], GeometryType::Polyline, false), + (vec![core], GeometryType::Polyline, false), + (vec![casing], GeometryType::Point, true), + (vec![casing, core], GeometryType::Point, false), + ] { + let mut r = unique_with(&[("a", 1), ("b", 2), ("c", 3)]); + if let Renderer::Unique { class_strokes, .. } = &mut r { + class_strokes.insert("a".into(), stack_of_strokes.clone()); + } + let n = r.reverse_ramp(g); + let case = format!("{} strokes, {g:?}", stack_of_strokes.len()); + assert_eq!((n, reds(&r)), if hides { (Some(1), vec![1, 3, 2]) } else { (Some(0), vec![3, 2, 1]) }, "{case}"); + } + for (fills, hides) in [(vec![solid_own], true), (vec![solid_own, solid_inherit], false), (vec![solid_inherit], false)] { + let mut r = unique_with(&[("a", 1), ("b", 2), ("c", 3)]); + if let Renderer::Unique { class_fills, .. } = &mut r { + class_fills.insert("a".into(), fills.clone()); + } + let n = r.reverse_ramp(GeometryType::Polygon); + let case = format!("{} fills", fills.len()); + assert_eq!((n, reds(&r)), if hides { (Some(1), vec![1, 3, 2]) } else { (Some(0), vec![3, 2, 1]) }, "{case}"); + } + } } diff --git a/docs/user-guide/cartography.md b/docs/user-guide/cartography.md index f81c9f19e..dbc8ddb4c 100644 --- a/docs/user-guide/cartography.md +++ b/docs/user-guide/cartography.md @@ -187,11 +187,14 @@ Colors or Unique Values layer's classes along the legend's order, so the highest lowest class's colour. Colours edited by hand travel with the reversal, but a polygon fill's opacity stays with its position in the legend, as it does when a colour scheme is applied: a semi-transparent fill picked for the first class is still the first class's opacity afterwards. -Applying it twice gives the original back. A class drawn from a stack of its own (stacked -fills on a polygon layer, stacked strokes on a line or point layer) keeps it, since the stack is -drawn instead of the class's symbol, and the others reverse among themselves; a notice says how -many classes were left as they are. A stack the layer doesn't draw (a polygon's stacked strokes) -doesn't count. +Applying it twice gives the original back. A class whose own stack hides its symbol keeps it, and the others reverse +among themselves; a notice says how many classes were left as they are. A stack hides the symbol when it +is drawn (stacked fills on a polygon layer, stacked strokes on a line or point layer; a polygon's +stacked strokes and a line's stacked fills are not drawn) and every layer of it sets a color of its +own. A layer of a stack left on "follow the class" shows the class's color, so that class reverses +with the rest: a road's black casing over a core that follows the class. On a layer that holds +several kinds of geometry, the reversal treats every class as the layer's main kind, as choosing a +color scheme does. Choosing a colour scheme again starts from the scheme's own order, but classing the layer again on the same field, ranges and scheme keeps the reversed colours, as it keeps other colours edited by hand. On a Stretch raster the command switches the existing "Invert color scheme" option. From 75c7da417d5fcf762060981cee2bb591adc70130 Mon Sep 17 00:00:00 2001 From: ninhhaidang Date: Wed, 4 Nov 2026 00:14:35 +0700 Subject: [PATCH 07/13] fix(symbology): Invert Color Ramp says when the layer's own stack hides the colors (#1355) Classes without a stack of their own are drawn from the layer's `line_strokes` or `fill_layers`. When every layer of that stack sets its own color, the symbol's colors, which a color scheme sets, show in none of them: on a line or point layer the reversal changed nothing that is drawn, and on a polygon layer only the outlines, without a word. The colors are still reversed (so undo and a later removal of the stack agree), and a notice now says that the classes show the stack's colors. A stack with a layer that follows the class (no color of its own) shows the scheme and says nothing. Docs: the `` swatch is no color of the scheme and stays; the notice counts the classes with a stack of their own; the layer's own stack. Two lines over 140 columns wrapped. Tests: a line layer and a polygon layer, with a layer-wide stack of colors of its own (notice), with a core that follows the class (none) and without a stack (none); `` stays. --- crates/engine/src/cmd/layer.rs | 37 ++++++++++++++++++++++++++++++---- crates/engine/src/tests.rs | 34 +++++++++++++++++++++++++++++++ crates/map/src/symbology.rs | 10 +++++---- docs/user-guide/cartography.md | 22 ++++++++++++-------- 4 files changed, 87 insertions(+), 16 deletions(-) diff --git a/crates/engine/src/cmd/layer.rs b/crates/engine/src/cmd/layer.rs index ae97b775f..8993afd15 100644 --- a/crates/engine/src/cmd/layer.rs +++ b/crates/engine/src/cmd/layer.rs @@ -157,11 +157,12 @@ pub fn commands() -> Vec { C::new("layer.invertRamp", "Invert Color Ramp", "Style › Drawing", |s, p| { let id = s.layer_id(p)?; let l = s.map_mut().layer_mut(id).ok_or("no such layer")?; - let kept = match &mut l.raster { - Some(rl) => rl.style.invert_ramp().then_some(0), + let (kept, layer_stack) = match &mut l.raster { + Some(rl) => (rl.style.invert_ramp().then_some(0), None), None => { let g = l.geometry_type(); - l.renderer.reverse_ramp(g) + let stack = layer_stack_hiding_symbol(l, g); + (l.renderer.reverse_ramp(g), stack) } }; let Some(kept) = kept else { @@ -170,7 +171,20 @@ pub fn commands() -> Vec { if kept > 0 { // Their own stacks are drawn instead of the symbol, so the reversal can't reach them. s.notify( - format!("{kept} class(es) drawn with fills or strokes of their own were left as they are: edit those with their fill and stroke editors"), + format!( + "{kept} class(es) drawn with fills or strokes of their own were left as they are: \ + edit those with their fill and stroke editors" + ), + false, + ); + } + if let Some(what) = layer_stack { + // The classes without a stack of their own are drawn from the layer's, in colors of its own. + s.notify( + format!( + "The layer's stacked {what} set their own colors, so the classes show those and little of the color \ + scheme: the colors were reversed, but change the {what} to see them" + ), false, ); } @@ -774,6 +788,21 @@ fn show_fill(sym: &mut rustgis_map::Symbol) { } } +/// "fills" or "strokes" when the layer's own stack (`fill_layers` of a polygon layer, `line_strokes` of a +/// line or point layer) is drawn for the classes that have none of their own and every layer of it sets its +/// own color, so the symbol's colors, which a color scheme sets, show in none of them. +fn layer_stack_hiding_symbol(l: &rustgis_map::Layer, g: GeometryType) -> Option<&'static str> { + if l.graphics { + return None; + } + match g { + GeometryType::Polygon if !l.fill_layers.is_empty() && l.fill_layers.iter().all(|f| f.color.is_some()) => Some("fills"), + GeometryType::Polygon => None, + _ if !l.line_strokes.is_empty() && l.line_strokes.iter().all(|s| s.color.is_some()) => Some("strokes"), + _ => None, + } +} + /// `layer.fillLayers`: a polygon layer's stacked fills (or a class's own, with `value`, `index` /// or `other`), bottom first; `[]` goes back to the symbol's single fill. fn fill_layers(s: &mut Session, p: &Value) -> CmdResult { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index b3bab82c6..ca5c32b85 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -25574,3 +25574,37 @@ fn layer_invert_ramp_reverses_the_class_colors() { s.run("raster.symbology", &json!({"layer": "dem", "type": "classify", "classes": 2, "method": "equalInterval"})).unwrap(); assert!(s.run("layer.invertRamp", &json!({"layer": "dem"})).is_err(), "a classified raster has no invert option"); } + +#[test] +fn layer_invert_ramp_says_when_the_layers_own_stack_hides_the_colors() { + // #1355: classes without a stack of their own are drawn from the layer's `line_strokes` or + // `fill_layers`; when every layer of it sets its own color, the color scheme shows in none of + // them, and the command says so (the colors are reversed all the same). + let notices = |s: &mut Session, layer: &str| -> Vec { + let mark = s.notice_mark(); + s.run("layer.invertRamp", &json!({"layer": layer})).unwrap(); + s.notices_since(mark).iter().map(|n| n.text.clone()).collect() + }; + let mut s = world(); + // A line layer: strokes that all set a color hide the symbol; a core that follows the class doesn't. + s.run("layer.symbology", &json!({"layer": "Rivers", "type": "graduated", "field": "scalerank", "classes": 4})).unwrap(); + assert!(notices(&mut s, "Rivers").is_empty(), "no stack, nothing to say"); + let own = json!([{"color": "#000000", "width": 5}, {"color": "#FFFFFF", "width": 2}]); + s.run("layer.lineStrokes", &json!({"layer": "Rivers", "strokes": own})).unwrap(); + let said = notices(&mut s, "Rivers"); + assert_eq!(said.len(), 1, "{said:?}"); + assert!(said[0].starts_with("The layer's stacked strokes set their own colors"), "{said:?}"); + let casing_and_core = json!([{"color": "#000000", "width": 5}, {"width": 2}]); + s.run("layer.lineStrokes", &json!({"layer": "Rivers", "strokes": casing_and_core})).unwrap(); + assert!(notices(&mut s, "Rivers").is_empty(), "the core follows the class, so the scheme shows"); + s.run("layer.lineStrokes", &json!({"layer": "Rivers", "strokes": []})).unwrap(); + assert!(notices(&mut s, "Rivers").is_empty()); + // A polygon layer: its stacked fills. + s.run("layer.symbology", &json!({"layer": "Countries", "type": "graduated", "field": "pop_est", "classes": 4})).unwrap(); + s.run("layer.fillLayers", &json!({"layer": "Countries", "fills": [{"color": "#00AA00", "pattern": "solid"}]})).unwrap(); + let said = notices(&mut s, "Countries"); + assert_eq!(said.len(), 1, "{said:?}"); + assert!(said[0].starts_with("The layer's stacked fills set their own colors"), "{said:?}"); + s.run("layer.fillLayers", &json!({"layer": "Countries", "fills": [{"pattern": "solid"}]})).unwrap(); + assert!(notices(&mut s, "Countries").is_empty(), "a fill that follows the class shows the scheme"); +} diff --git a/crates/map/src/symbology.rs b/crates/map/src/symbology.rs index 45781e6fb..10ccdc3fe 100644 --- a/crates/map/src/symbology.rs +++ b/crates/map/src/symbology.rs @@ -1616,10 +1616,11 @@ impl Renderer { /// Reverses the colours of the classes (#1355), in the order the legend lists them: the first /// class takes the last one's colour, and so on. Colours edited by hand travel with their /// class's old position, and a polygon fill keeps its own opacity. Applied twice it gives the - /// original back. A class whose own stack hides its symbol (every layer of its fills, for a polygon, or - /// strokes, for a line or a point, sets a color of its own), and a repeat of a Unique Values value (which no feature draws), keep theirs and take no part in the reversal. - /// Returns how many classes were kept for their own stacks, or `None` for a renderer without - /// classes coloured from a ramp. + /// original back. A class whose own stack hides its symbol (every layer of its fills, for a + /// polygon, or strokes, for a line or a point, sets a color of its own), and a repeat of a Unique + /// Values value (which no feature draws), keep theirs and take no part in the reversal. The + /// `` symbol is no ramp color and stays. Returns how many classes were kept for + /// their own stacks, or `None` for a renderer without classes coloured from a ramp. pub fn reverse_ramp(&mut self, g: GeometryType) -> Option { let (slots, kept) = self.ramp_slots(g)?; let old: Vec = slots.iter().filter_map(|&i| self.class_symbol_mut(i).map(|s| *s)).collect(); @@ -4082,6 +4083,7 @@ mod tests { let mut r = unique_with(&[("10", 1), ("100", 2), ("9", 3)]); assert_eq!(r.reverse_ramp(g), Some(0)); assert_eq!(reds(&r), vec![("10".into(), 1), ("100".into(), 3), ("9".into(), 2)]); + assert!(matches!(&r, Renderer::Unique { other, .. } if other.fill.r() == 0), " is no ramp color"); r.reverse_ramp(g); assert_eq!(reds(&r), vec![("10".into(), 1), ("100".into(), 2), ("9".into(), 3)], "twice is the original"); // A repeat of a value is not drawn: it stays, and the drawn entries reverse among themselves. diff --git a/docs/user-guide/cartography.md b/docs/user-guide/cartography.md index dbc8ddb4c..78f0d6098 100644 --- a/docs/user-guide/cartography.md +++ b/docs/user-guide/cartography.md @@ -187,14 +187,20 @@ Colors or Unique Values layer's classes along the legend's order, so the highest lowest class's colour. Colours edited by hand travel with the reversal, but a polygon fill's opacity stays with its position in the legend, as it does when a colour scheme is applied: a semi-transparent fill picked for the first class is still the first class's opacity afterwards. -Applying it twice gives the original back. A class whose own stack hides its symbol keeps it, and the others reverse -among themselves; a notice says how many classes were left as they are. A stack hides the symbol when it -is drawn (stacked fills on a polygon layer, stacked strokes on a line or point layer; a polygon's -stacked strokes and a line's stacked fills are not drawn) and every layer of it sets a color of its -own. A layer of a stack left on "follow the class" shows the class's color, so that class reverses -with the rest: a road's black casing over a core that follows the class. On a layer that holds -several kinds of geometry, the reversal treats every class as the layer's main kind, as choosing a -color scheme does. +Applying it twice gives the original back. The `` swatch is no colour of the +scheme and stays as it is. + +A class whose own stack hides its symbol keeps it, and the others reverse among themselves; a +notice says how many classes with a stack of their own were left as they are. A stack hides the +symbol when it is drawn (stacked fills on a polygon layer, stacked strokes on a line or point +layer; a polygon's stacked strokes and a line's stacked fills are not drawn) and every layer of it +sets a colour of its own. A layer of a stack left on "follow the class" shows the class's colour, +so that class reverses with the rest: a road's black casing over a core that follows the class. +When the layer's own stack (the one classes without a stack of their own are drawn from) sets a +colour in every layer, the classes show those colours and little of the colour scheme: the colours +are reversed all the same, and a notice says so. On a layer that holds several kinds of geometry, +the reversal treats every class as the layer's main kind, as choosing a colour scheme does. + Choosing a colour scheme again starts from the scheme's own order, but classing the layer again on the same field, ranges and scheme keeps the reversed colours, as it keeps other colours edited by hand. On a Stretch raster the command switches the existing "Invert color scheme" option. From 2d689d7c8139c04bfa687fadca4a4cc140925a02 Mon Sep 17 00:00:00 2001 From: ninhhaidang Date: Wed, 4 Nov 2026 00:43:47 +0700 Subject: [PATCH 08/13] fix(symbology): Invert Color Ramp refuses a graphics layer and only says what it did (#1355) A graphics layer draws each feature in its own style and ignores its renderer, so reversing the renderer's colors changed nothing and said nothing: the command now refuses it. `reverse_ramp` says how many classes changed places and how many were kept (`RampReversal`), and the notice about the layer's own stack, which says the colors were reversed, comes only when some were: with every class drawn from a stack of its own, or a lone class, none were. The rule for a stack that hides the symbol's colors (drawn for the geometry, and every layer of it sets a color of its own) is one function, `stack_hides_symbol`, for a class's own stack and the layer's. Tests: a graphics layer is refused; no layer-stack notice when no class changed places; the reversed and kept counts. Docs: a graphics layer has no color scheme to invert. --- crates/engine/src/cmd/layer.rs | 33 ++++++++++++----------- crates/engine/src/tests.rs | 23 ++++++++++++++++ crates/map/src/symbology.rs | 49 +++++++++++++++++++++++++--------- docs/user-guide/cartography.md | 4 ++- 4 files changed, 80 insertions(+), 29 deletions(-) diff --git a/crates/engine/src/cmd/layer.rs b/crates/engine/src/cmd/layer.rs index 8993afd15..79e112649 100644 --- a/crates/engine/src/cmd/layer.rs +++ b/crates/engine/src/cmd/layer.rs @@ -155,21 +155,26 @@ pub fn commands() -> Vec { }).params(r#"{"layer"?, "ramp": colour ramp name}"#) .when(need_layer), C::new("layer.invertRamp", "Invert Color Ramp", "Style › Drawing", |s, p| { + use rustgis_map::symbology::RampReversal; let id = s.layer_id(p)?; let l = s.map_mut().layer_mut(id).ok_or("no such layer")?; - let (kept, layer_stack) = match &mut l.raster { - Some(rl) => (rl.style.invert_ramp().then_some(0), None), + if l.graphics && l.raster.is_none() { + return Err("a graphics layer draws each feature in its own style, so it has no color scheme to invert".into()); + } + let (done, layer_stack) = match &mut l.raster { + Some(rl) => (rl.style.invert_ramp().then_some(RampReversal { reversed: 0, kept: 0 }), None), None => { let g = l.geometry_type(); let stack = layer_stack_hiding_symbol(l, g); (l.renderer.reverse_ramp(g), stack) } }; - let Some(kept) = kept else { + let Some(done) = done else { return Err("the layer has no color scheme to invert: use Graduated or Unique Values (or a Stretch raster)".into()); }; - if kept > 0 { + if done.kept > 0 { // Their own stacks are drawn instead of the symbol, so the reversal can't reach them. + let kept = done.kept; s.notify( format!( "{kept} class(es) drawn with fills or strokes of their own were left as they are: \ @@ -178,7 +183,7 @@ pub fn commands() -> Vec { false, ); } - if let Some(what) = layer_stack { + if let Some(what) = layer_stack.filter(|_| done.reversed > 0) { // The classes without a stack of their own are drawn from the layer's, in colors of its own. s.notify( format!( @@ -789,18 +794,14 @@ fn show_fill(sym: &mut rustgis_map::Symbol) { } /// "fills" or "strokes" when the layer's own stack (`fill_layers` of a polygon layer, `line_strokes` of a -/// line or point layer) is drawn for the classes that have none of their own and every layer of it sets its -/// own color, so the symbol's colors, which a color scheme sets, show in none of them. +/// line or point layer), which the classes without a stack of their own are drawn from, hides the +/// symbol's colors, which a color scheme sets ([`rustgis_map::symbology::stack_hides_symbol`]). fn layer_stack_hiding_symbol(l: &rustgis_map::Layer, g: GeometryType) -> Option<&'static str> { - if l.graphics { - return None; - } - match g { - GeometryType::Polygon if !l.fill_layers.is_empty() && l.fill_layers.iter().all(|f| f.color.is_some()) => Some("fills"), - GeometryType::Polygon => None, - _ if !l.line_strokes.is_empty() && l.line_strokes.iter().all(|s| s.color.is_some()) => Some("strokes"), - _ => None, - } + rustgis_map::symbology::stack_hides_symbol(g, &l.fill_layers, &l.line_strokes).then_some(if g == GeometryType::Polygon { + "fills" + } else { + "strokes" + }) } /// `layer.fillLayers`: a polygon layer's stacked fills (or a class's own, with `value`, `index` diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index ca5c32b85..2373efcb8 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -25607,4 +25607,27 @@ fn layer_invert_ramp_says_when_the_layers_own_stack_hides_the_colors() { assert!(said[0].starts_with("The layer's stacked fills set their own colors"), "{said:?}"); s.run("layer.fillLayers", &json!({"layer": "Countries", "fills": [{"pattern": "solid"}]})).unwrap(); assert!(notices(&mut s, "Countries").is_empty(), "a fill that follows the class shows the scheme"); + + // The notice about the layer's stack comes only when some classes changed places: with every + // class drawn from a stack of its own, none did, and the layer's stack is not what shows. + s.run("layer.symbology", &json!({"layer": "Countries", "type": "graduated", "field": "pop_est", "classes": 2})).unwrap(); + s.run("layer.fillLayers", &json!({"layer": "Countries", "fills": [{"color": "#00AA00", "pattern": "solid"}]})).unwrap(); + for index in 0..2 { + s.run("layer.fillLayers", &json!({"layer": "Countries", "index": index, "fills": [{"color": "#AA0000", "pattern": "solid"}]})) + .unwrap(); + } + let said = notices(&mut s, "Countries"); + assert_eq!(said.len(), 1, "{said:?}"); + assert!(said[0].starts_with("2 class(es) drawn with fills or strokes of their own"), "{said:?}"); +} + +#[test] +fn layer_invert_ramp_refuses_a_graphics_layer() { + // #1355: a graphics layer draws each feature in its own style, whatever its renderer holds, so + // reversing the renderer's colors would change nothing and say nothing. + let mut s = world(); + s.run("graphics.add", &json!({"shape": "rectangle", "points": [[0, 45], [10, 55]], "fill": "#FF000040"})).unwrap(); + let id = s.map().layers.iter().find(|l| l.graphics).map(|l| l.id).unwrap(); + let err = s.run("layer.invertRamp", &json!({"layer": id})).unwrap_err(); + assert!(err.contains("graphics layer"), "{err}"); } diff --git a/crates/map/src/symbology.rs b/crates/map/src/symbology.rs index 10ccdc3fe..a77620839 100644 --- a/crates/map/src/symbology.rs +++ b/crates/map/src/symbology.rs @@ -356,6 +356,28 @@ pub struct LineStroke { pub markers: Option, } +/// Whether a stack of fills (a polygon's) or strokes (a line's or a point's) hides the colors a +/// color scheme sets on a symbol (#1355): the stack is drawn for the geometry (a polygon's stacked +/// strokes and a line's stacked fills are not) and every layer of it sets a color of its own. A layer +/// without one takes the symbol's (`FillLayer::color`, `LineStroke::color`), so the class still shows +/// its color through it: a road's black casing over a core that follows the class. An empty stack is +/// no stack. Used for a class's own stack and for the layer's. +pub fn stack_hides_symbol(g: GeometryType, fills: &[FillLayer], strokes: &[LineStroke]) -> bool { + match g { + GeometryType::Polygon => !fills.is_empty() && fills.iter().all(|f| f.color.is_some()), + _ => !strokes.is_empty() && strokes.iter().all(|s| s.color.is_some()), + } +} + +/// What [`Renderer::reverse_ramp`] did. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct RampReversal { + /// How many classes changed colors with another: 0 when fewer than two classes show the scheme. + pub reversed: usize, + /// How many classes were left as they were, their own stack hiding their symbol. + pub kept: usize, +} + impl Default for LineStroke { fn default() -> Self { LineStroke { @@ -1571,9 +1593,8 @@ impl Renderer { /// other renderers. fn ramp_slots(&self, g: GeometryType) -> Option<(Vec, usize)> { // Whether a class's own stacks hide its symbol's colors, for the geometry the layer draws. - let replaced = |fills: Option<&Vec>, strokes: Option<&Vec>| match g { - GeometryType::Polygon => fills.is_some_and(|f| !f.is_empty() && f.iter().all(|l| l.color.is_some())), - _ => strokes.is_some_and(|s| !s.is_empty() && s.iter().all(|l| l.color.is_some())), + let replaced = |fills: Option<&Vec>, strokes: Option<&Vec>| { + stack_hides_symbol(g, fills.map_or(&[][..], Vec::as_slice), strokes.map_or(&[][..], Vec::as_slice)) }; let (mut slots, mut kept) = (Vec::new(), 0); match self { @@ -1619,9 +1640,10 @@ impl Renderer { /// original back. A class whose own stack hides its symbol (every layer of its fills, for a /// polygon, or strokes, for a line or a point, sets a color of its own), and a repeat of a Unique /// Values value (which no feature draws), keep theirs and take no part in the reversal. The - /// `` symbol is no ramp color and stays. Returns how many classes were kept for - /// their own stacks, or `None` for a renderer without classes coloured from a ramp. - pub fn reverse_ramp(&mut self, g: GeometryType) -> Option { + /// `` symbol is no ramp color and stays. Says how many classes changed places + /// and how many were kept for their own stacks, or `None` for a renderer without classes + /// coloured from a ramp. + pub fn reverse_ramp(&mut self, g: GeometryType) -> Option { let (slots, kept) = self.ramp_slots(g)?; let old: Vec = slots.iter().filter_map(|&i| self.class_symbol_mut(i).map(|s| *s)).collect(); for (pos, &i) in slots.iter().enumerate() { @@ -1630,7 +1652,8 @@ impl Renderer { *sym = sym.with_colors_of(from, g); } } - Some(kept) + // One class, or none, has no other to change places with. + Some(RampReversal { reversed: if slots.len() > 1 { slots.len() } else { 0 }, kept }) } /// Recolours every class from a ramp, preserving each polygon fill's opacity. @@ -4081,7 +4104,7 @@ mod tests { let g = GeometryType::Point; // Stored 10, 100, 9: the legend lists 9, 10, 100 (numbers), so 9 and 100 swap colours. let mut r = unique_with(&[("10", 1), ("100", 2), ("9", 3)]); - assert_eq!(r.reverse_ramp(g), Some(0)); + assert_eq!(r.reverse_ramp(g), Some(RampReversal { reversed: 3, kept: 0 })); assert_eq!(reds(&r), vec![("10".into(), 1), ("100".into(), 3), ("9".into(), 2)]); assert!(matches!(&r, Renderer::Unique { other, .. } if other.fill.r() == 0), " is no ramp color"); r.reverse_ramp(g); @@ -4095,8 +4118,10 @@ mod tests { if let Renderer::Unique { class_strokes, .. } = &mut r { class_strokes.insert("a".into(), vec![LineStroke { color: Some(Rgba::rgb(9, 9, 9)), ..LineStroke::default() }]); } - assert_eq!(r.reverse_ramp(g), Some(1)); + assert_eq!(r.reverse_ramp(g), Some(RampReversal { reversed: 3, kept: 1 })); assert_eq!(reds(&r), vec![("a".into(), 1), ("b".into(), 4), ("c".into(), 3), ("d".into(), 2)]); + // A lone class has no other to change places with. + assert_eq!(unique_with(&[("a", 1)]).reverse_ramp(g), Some(RampReversal { reversed: 0, kept: 0 })); // Nothing to reverse for a single symbol. assert_eq!(Renderer::Single(Symbol::default_for(g, 0)).reverse_ramp(g), None); } @@ -4136,7 +4161,7 @@ mod tests { (true, true, true, GeometryType::Polyline, true), ] { let mut r = with(fills, strokes, empty); - let n = r.reverse_ramp(g); + let n = r.reverse_ramp(g).map(|x| x.kept); let case = format!("fills {fills}, strokes {strokes}, empty {empty}, {g:?}"); if take_part { assert_eq!(n, Some(0), "{case}"); @@ -4172,7 +4197,7 @@ mod tests { if let Renderer::Unique { class_strokes, .. } = &mut r { class_strokes.insert("a".into(), stack_of_strokes.clone()); } - let n = r.reverse_ramp(g); + let n = r.reverse_ramp(g).map(|x| x.kept); let case = format!("{} strokes, {g:?}", stack_of_strokes.len()); assert_eq!((n, reds(&r)), if hides { (Some(1), vec![1, 3, 2]) } else { (Some(0), vec![3, 2, 1]) }, "{case}"); } @@ -4181,7 +4206,7 @@ mod tests { if let Renderer::Unique { class_fills, .. } = &mut r { class_fills.insert("a".into(), fills.clone()); } - let n = r.reverse_ramp(GeometryType::Polygon); + let n = r.reverse_ramp(GeometryType::Polygon).map(|x| x.kept); let case = format!("{} fills", fills.len()); assert_eq!((n, reds(&r)), if hides { (Some(1), vec![1, 3, 2]) } else { (Some(0), vec![3, 2, 1]) }, "{case}"); } diff --git a/docs/user-guide/cartography.md b/docs/user-guide/cartography.md index 78f0d6098..c86990b2c 100644 --- a/docs/user-guide/cartography.md +++ b/docs/user-guide/cartography.md @@ -203,7 +203,9 @@ the reversal treats every class as the layer's main kind, as choosing a colour s Choosing a colour scheme again starts from the scheme's own order, but classing the layer again on the same field, ranges and scheme keeps the reversed colours, as it keeps other colours edited -by hand. On a Stretch raster the command switches the existing "Invert color scheme" option. +by hand. On a Stretch raster the command switches the existing "Invert color scheme" option. A +graphics layer draws each feature in its own style, so it has no colour scheme to invert and the +command says so. ### Line strokes From 540d9c0eaa36a5bd028fbcee24ffb1427fb3d9fe Mon Sep 17 00:00:00 2001 From: ninhhaidang Date: Wed, 4 Nov 2026 14:31:12 +0700 Subject: [PATCH 09/13] fix(symbology): hide Invert Color Ramp on a graphics layer, where the command refuses (#1355) --- crates/ui-egui/src/panes.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/panes.rs b/crates/ui-egui/src/panes.rs index 43ae50dc6..6d99d48d7 100644 --- a/crates/ui-egui/src/panes.rs +++ b/crates/ui-egui/src/panes.rs @@ -4214,7 +4214,7 @@ fn symbology(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { let e = if text.is_empty() { Value::Null } else { json!(text) }; let _ = app.run("layer.symbology", json!({"type": "unique", "expression": e, "ramp": ramp.name()})); } - ramp_picker_with(app, ui, ramp, t, true); + ramp_picker_with(app, ui, ramp, t, !l.graphics); ui.add_space(6.0); ui.label( egui::RichText::new(crate::i18n::fmt("{n} classes", &[("n", &classes.len().to_string())])).size(11.5).color(t.text_dim), @@ -4251,7 +4251,7 @@ fn symbology(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { } }); } - ramp_picker_with(app, ui, ramp, t, true); + ramp_picker_with(app, ui, ramp, t, !l.graphics); ui.add_space(6.0); legend_list(app, ui, &l, t); } From faaa4132038210048a26fbcbffc237a05b0fa14b Mon Sep 17 00:00:00 2001 From: ninhhaidang Date: Wed, 4 Nov 2026 15:24:03 +0700 Subject: [PATCH 10/13] fix(symbology): Invert Color Ramp says when fewer than two classes leave nothing to invert (#1355) --- crates/engine/src/cmd/layer.rs | 6 ++++++ crates/engine/src/tests.rs | 21 +++++++++++++++++++++ docs/user-guide/cartography.md | 3 ++- 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/layer.rs b/crates/engine/src/cmd/layer.rs index e653316ad..14253ae9a 100644 --- a/crates/engine/src/cmd/layer.rs +++ b/crates/engine/src/cmd/layer.rs @@ -161,6 +161,7 @@ pub fn commands() -> Vec { if l.graphics && l.raster.is_none() { return Err("a graphics layer draws each feature in its own style, so it has no color scheme to invert".into()); } + let raster = l.raster.is_some(); let (done, layer_stack) = match &mut l.raster { Some(rl) => (rl.style.invert_ramp().then_some(RampReversal { reversed: 0, kept: 0 }), None), None => { @@ -172,6 +173,11 @@ pub fn commands() -> Vec { let Some(done) = done else { return Err("the layer has no color scheme to invert: use Graduated or Unique Values (or a Stretch raster)".into()); }; + // A raster's Invert is a switch, so it has no classes to count. Classes with fewer than two + // to change places, and none left for their own stacks (which say so below), change nothing. + if !raster && done.reversed == 0 && done.kept == 0 { + return Err("the layer has fewer than two classes showing the color scheme, so there is nothing to invert".into()); + } if done.kept > 0 { // Their own stacks are drawn instead of the symbol, so the reversal can't reach them. let kept = done.kept; diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 2aaeaf589..4ca692bd8 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -26105,6 +26105,27 @@ fn layer_invert_ramp_says_when_the_layers_own_stack_hides_the_colors() { assert!(said[0].starts_with("2 class(es) drawn with fills or strokes of their own"), "{said:?}"); } +#[test] +fn layer_invert_ramp_says_when_there_is_nothing_to_invert() { + // #1355: with fewer than two classes showing the scheme, an inversion would change nothing, so + // the command refuses rather than succeed silently. A raster's Invert is a switch and is no + // class count, and a class kept for its own stack has its own notice. + let mut s = world(); + s.run("layer.symbology", &json!({"layer": "Countries", "type": "graduated", "field": "pop_est", "classes": 1})).unwrap(); + let classes = |s: &Session| match &s.map().layer_by_name("Countries").unwrap().renderer { + rustgis_map::Renderer::Graduated { symbols, .. } => symbols.len(), + _ => 0, + }; + assert_eq!(classes(&s), 1, "one class to invert"); + let err = s.run("layer.invertRamp", &json!({"layer": "Countries"})).unwrap_err(); + assert!(err.contains("nothing to invert"), "{err}"); + // Two classes reverse, with no notice. + s.run("layer.symbology", &json!({"layer": "Countries", "type": "graduated", "field": "pop_est", "classes": 2})).unwrap(); + let mark = s.notice_mark(); + s.run("layer.invertRamp", &json!({"layer": "Countries"})).unwrap(); + assert!(s.notices_since(mark).is_empty()); +} + #[test] fn layer_invert_ramp_refuses_a_graphics_layer() { // #1355: a graphics layer draws each feature in its own style, whatever its renderer holds, so diff --git a/docs/user-guide/cartography.md b/docs/user-guide/cartography.md index f7c71965c..5ddcfc562 100644 --- a/docs/user-guide/cartography.md +++ b/docs/user-guide/cartography.md @@ -188,7 +188,8 @@ lowest class's colour. Colours edited by hand travel with the reversal, but a po opacity stays with its position in the legend, as it does when a colour scheme is applied: a semi-transparent fill picked for the first class is still the first class's opacity afterwards. Applying it twice gives the original back. The `` swatch is no colour of the -scheme and stays as it is. +scheme and stays as it is. With fewer than two classes showing the scheme there is nothing to +reverse, and the command says so instead of succeeding. A class whose own stack hides its symbol keeps it, and the others reverse among themselves; a notice says how many classes with a stack of their own were left as they are. A stack hides the From 16f602a0f9a98f42da448ab32ff04800dd71d70f Mon Sep 17 00:00:00 2001 From: ninhhaidang Date: Fri, 6 Nov 2026 20:22:31 +0700 Subject: [PATCH 11/13] fix(symbology): Invert Color Ramp names the layer's stack only when a reversed class is drawn from it (#1355) --- crates/engine/src/cmd/layer.rs | 4 ++-- crates/engine/src/tests.rs | 14 ++++++++++++++ crates/map/src/symbology.rs | 25 +++++++++++++++++-------- 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/crates/engine/src/cmd/layer.rs b/crates/engine/src/cmd/layer.rs index 14253ae9a..ffea71a30 100644 --- a/crates/engine/src/cmd/layer.rs +++ b/crates/engine/src/cmd/layer.rs @@ -163,7 +163,7 @@ pub fn commands() -> Vec { } let raster = l.raster.is_some(); let (done, layer_stack) = match &mut l.raster { - Some(rl) => (rl.style.invert_ramp().then_some(RampReversal { reversed: 0, kept: 0 }), None), + Some(rl) => (rl.style.invert_ramp().then_some(RampReversal { reversed: 0, kept: 0, on_layer_stack: 0 }), None), None => { let g = l.geometry_type(); let stack = layer_stack_hiding_symbol(l, g); @@ -189,7 +189,7 @@ pub fn commands() -> Vec { false, ); } - if let Some(what) = layer_stack.filter(|_| done.reversed > 0) { + if let Some(what) = layer_stack.filter(|_| done.on_layer_stack > 0) { // The classes without a stack of their own are drawn from the layer's, in colors of its own. s.notify( format!( diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 4ca692bd8..495ef15a1 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -26103,6 +26103,20 @@ fn layer_invert_ramp_says_when_the_layers_own_stack_hides_the_colors() { let said = notices(&mut s, "Countries"); assert_eq!(said.len(), 1, "{said:?}"); assert!(said[0].starts_with("2 class(es) drawn with fills or strokes of their own"), "{said:?}"); + + // Their own stacks replace the layer's even where they follow the class, so the layer's stack + // is not what shows and the command says nothing of it. + for index in 0..2 { + s.run("layer.fillLayers", &json!({"layer": "Countries", "index": index, "fills": [{"pattern": "solid"}]})).unwrap(); + } + assert!(notices(&mut s, "Countries").is_empty(), "every class has a stack of its own"); + // One class left on the layer's stack is enough to say it. + s.run("layer.symbology", &json!({"layer": "Countries", "type": "graduated", "field": "pop_est", "classes": 3})).unwrap(); + s.run("layer.fillLayers", &json!({"layer": "Countries", "fills": [{"color": "#00AA00", "pattern": "solid"}]})).unwrap(); + s.run("layer.fillLayers", &json!({"layer": "Countries", "index": 0, "fills": [{"pattern": "solid"}]})).unwrap(); + let said = notices(&mut s, "Countries"); + assert_eq!(said.len(), 1, "{said:?}"); + assert!(said[0].starts_with("The layer's stacked fills set their own colors"), "{said:?}"); } #[test] diff --git a/crates/map/src/symbology.rs b/crates/map/src/symbology.rs index a77620839..7f8d1bdfc 100644 --- a/crates/map/src/symbology.rs +++ b/crates/map/src/symbology.rs @@ -376,6 +376,8 @@ pub struct RampReversal { pub reversed: usize, /// How many classes were left as they were, their own stack hiding their symbol. pub kept: usize, + /// How many of the `reversed` classes have no stack of their own, so they are drawn from the layer's. + pub on_layer_stack: usize, } impl Default for LineStroke { @@ -1591,12 +1593,16 @@ impl Renderer { /// first entry of a Unique Values value is drawn, so a repeated value counts once. They are /// indices into `classes` (Unique Values) or `symbols` (Graduated Colors). `None` for the /// other renderers. - fn ramp_slots(&self, g: GeometryType) -> Option<(Vec, usize)> { + fn ramp_slots(&self, g: GeometryType) -> Option<(Vec, usize, usize)> { // Whether a class's own stacks hide its symbol's colors, for the geometry the layer draws. let replaced = |fills: Option<&Vec>, strokes: Option<&Vec>| { stack_hides_symbol(g, fills.map_or(&[][..], Vec::as_slice), strokes.map_or(&[][..], Vec::as_slice)) }; - let (mut slots, mut kept) = (Vec::new(), 0); + // A class's own stack, of the kind the layer draws, replaces the layer's whenever it isn't empty. + let bare = |fills: Option<&Vec>, strokes: Option<&Vec>| { + if g == GeometryType::Polygon { fills.is_none_or(|f| f.is_empty()) } else { strokes.is_none_or(|s| s.is_empty()) } + }; + let (mut slots, mut kept, mut on_layer) = (Vec::new(), 0, 0); match self { Renderer::Unique { classes, class_fills, class_strokes, .. } => { let mut seen = std::collections::HashSet::new(); @@ -1609,6 +1615,7 @@ impl Renderer { kept += 1; } else { slots.push(i); + on_layer += usize::from(bare(class_fills.get(value), class_strokes.get(value))); } } } @@ -1618,12 +1625,13 @@ impl Renderer { kept += 1; } else { slots.push(i); + on_layer += usize::from(bare(class_fills.get(&i), class_strokes.get(&i))); } } } _ => return None, } - Some((slots, kept)) + Some((slots, kept, on_layer)) } fn class_symbol_mut(&mut self, i: usize) -> Option<&mut Symbol> { @@ -1644,7 +1652,7 @@ impl Renderer { /// and how many were kept for their own stacks, or `None` for a renderer without classes /// coloured from a ramp. pub fn reverse_ramp(&mut self, g: GeometryType) -> Option { - let (slots, kept) = self.ramp_slots(g)?; + let (slots, kept, on_layer_stack) = self.ramp_slots(g)?; let old: Vec = slots.iter().filter_map(|&i| self.class_symbol_mut(i).map(|s| *s)).collect(); for (pos, &i) in slots.iter().enumerate() { let from = old.len().checked_sub(1 + pos).and_then(|k| old.get(k)); @@ -1653,7 +1661,8 @@ impl Renderer { } } // One class, or none, has no other to change places with. - Some(RampReversal { reversed: if slots.len() > 1 { slots.len() } else { 0 }, kept }) + let moved = slots.len() > 1; + Some(RampReversal { reversed: if moved { slots.len() } else { 0 }, kept, on_layer_stack: if moved { on_layer_stack } else { 0 } }) } /// Recolours every class from a ramp, preserving each polygon fill's opacity. @@ -4104,7 +4113,7 @@ mod tests { let g = GeometryType::Point; // Stored 10, 100, 9: the legend lists 9, 10, 100 (numbers), so 9 and 100 swap colours. let mut r = unique_with(&[("10", 1), ("100", 2), ("9", 3)]); - assert_eq!(r.reverse_ramp(g), Some(RampReversal { reversed: 3, kept: 0 })); + assert_eq!(r.reverse_ramp(g), Some(RampReversal { reversed: 3, kept: 0, on_layer_stack: 3 })); assert_eq!(reds(&r), vec![("10".into(), 1), ("100".into(), 3), ("9".into(), 2)]); assert!(matches!(&r, Renderer::Unique { other, .. } if other.fill.r() == 0), " is no ramp color"); r.reverse_ramp(g); @@ -4118,10 +4127,10 @@ mod tests { if let Renderer::Unique { class_strokes, .. } = &mut r { class_strokes.insert("a".into(), vec![LineStroke { color: Some(Rgba::rgb(9, 9, 9)), ..LineStroke::default() }]); } - assert_eq!(r.reverse_ramp(g), Some(RampReversal { reversed: 3, kept: 1 })); + assert_eq!(r.reverse_ramp(g), Some(RampReversal { reversed: 3, kept: 1, on_layer_stack: 3 })); assert_eq!(reds(&r), vec![("a".into(), 1), ("b".into(), 4), ("c".into(), 3), ("d".into(), 2)]); // A lone class has no other to change places with. - assert_eq!(unique_with(&[("a", 1)]).reverse_ramp(g), Some(RampReversal { reversed: 0, kept: 0 })); + assert_eq!(unique_with(&[("a", 1)]).reverse_ramp(g), Some(RampReversal { reversed: 0, kept: 0, on_layer_stack: 0 })); // Nothing to reverse for a single symbol. assert_eq!(Renderer::Single(Symbol::default_for(g, 0)).reverse_ramp(g), None); } From 80053d2535ba059e79f9f48a71593ec062841f7d Mon Sep 17 00:00:00 2001 From: ninhhaidang Date: Fri, 6 Nov 2026 21:35:45 +0700 Subject: [PATCH 12/13] fix(symbology): a point's hand-picked outline travels with its class's fill in Invert Color Ramp (#1355) --- crates/map/src/symbology.rs | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/crates/map/src/symbology.rs b/crates/map/src/symbology.rs index 4814d2f58..ced48715e 100644 --- a/crates/map/src/symbology.rs +++ b/crates/map/src/symbology.rs @@ -163,8 +163,8 @@ impl Symbol { self } - /// The colours a ramp gives a class, taken from `src`: the fill, and a line's or polygon's - /// outline. A polygon keeps its own fill opacity. + /// The colours a ramp gives a class, taken from `src`: the fill and the outline (a point's too, + /// which a ramp never sets, so it is one picked by hand). A polygon keeps its own fill opacity. fn with_colors_of(mut self, src: &Symbol, g: GeometryType) -> Symbol { match g { GeometryType::Polyline => { @@ -175,7 +175,10 @@ impl Symbol { self.fill = src.fill.with_alpha(self.fill.a()); self.stroke = src.stroke; } - _ => self.fill = src.fill, + _ => { + self.fill = src.fill; + self.stroke = src.stroke; + } } self } @@ -4389,6 +4392,21 @@ mod tests { } assert_eq!(r.reverse_ramp(g), Some(RampReversal { reversed: 3, kept: 1, on_layer_stack: 3 })); assert_eq!(reds(&r), vec![("a".into(), 1), ("b".into(), 4), ("c".into(), 3), ("d".into(), 2)]); + // A point's hand-picked outline travels with its fill. + let mut r = unique_with(&[("a", 1), ("b", 2)]); + if let Renderer::Unique { classes, .. } = &mut r + && let Some(c) = classes.first_mut() + { + c.1.stroke = Rgba::rgb(200, 0, 0); + } + r.reverse_ramp(g); + let strokes = |r: &Renderer| match r { + Renderer::Unique { classes, .. } => classes.iter().map(|c| (c.1.fill.r(), c.1.stroke.r())).collect::>(), + _ => vec![], + }; + assert_eq!(strokes(&r), vec![(2, 255), (1, 200)]); + r.reverse_ramp(g); + assert_eq!(strokes(&r), vec![(1, 200), (2, 255)], "twice is the original"); // A lone class has no other to change places with. assert_eq!(unique_with(&[("a", 1)]).reverse_ramp(g), Some(RampReversal { reversed: 0, kept: 0, on_layer_stack: 0 })); // Nothing to reverse for a single symbol. From 70cf58ff2ee908cfcad7f273b8938f719dbdce33 Mon Sep 17 00:00:00 2001 From: ninhhaidang Date: Fri, 6 Nov 2026 21:48:01 +0700 Subject: [PATCH 13/13] fix(symbology): Invert Color Ramp says when Symbol by Expression colors are drawn over the classes (#1355) --- crates/engine/src/cmd/layer.rs | 8 ++++++++ crates/engine/src/tests.rs | 22 ++++++++++++++++++++++ docs/user-guide/cartography.md | 2 ++ 3 files changed, 32 insertions(+) diff --git a/crates/engine/src/cmd/layer.rs b/crates/engine/src/cmd/layer.rs index 648dfecc2..b4a33a69f 100644 --- a/crates/engine/src/cmd/layer.rs +++ b/crates/engine/src/cmd/layer.rs @@ -162,6 +162,7 @@ pub fn commands() -> Vec { return Err("a graphics layer draws each feature in its own style, so it has no color scheme to invert".into()); } let raster = l.raster.is_some(); + let by_expression = !raster && (l.symbol_by.fill.is_some() || l.symbol_by.stroke.is_some()); let (done, layer_stack) = match &mut l.raster { Some(rl) => (rl.style.invert_ramp().then_some(RampReversal { reversed: 0, kept: 0, on_layer_stack: 0 }), None), None => { @@ -199,6 +200,13 @@ pub fn commands() -> Vec { false, ); } + if by_expression && done.reversed > 0 { + // Drawn over the class colors, wherever the expression gives a color. + s.notify( + "The layer's Symbol by Expression sets some features' colors, so the reversed colors show only where it gives none", + false, + ); + } Ok(Value::Null) }).params(r#"{"layer"?}"#) .when(need_layer), diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 4656c68bb..9767b6df8 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -28129,6 +28129,28 @@ fn layer_invert_ramp_says_when_the_layers_own_stack_hides_the_colors() { assert!(said[0].starts_with("The layer's stacked fills set their own colors"), "{said:?}"); } +#[test] +fn layer_invert_ramp_says_when_symbol_by_expression_sets_colors() { + // #1355: Symbol by Expression colors are drawn over the class colors, so the reversal shows only + // where the expression gives none; with no color expression (a size one doesn't set colors) it is quiet. + let notices = |s: &mut Session| -> Vec { + let mark = s.notice_mark(); + s.run("layer.invertRamp", &json!({"layer": "Countries"})).unwrap(); + s.notices_since(mark).iter().map(|n| n.text.clone()).collect() + }; + let mut s = world(); + s.run("layer.symbology", &json!({"layer": "Countries", "type": "graduated", "field": "pop_est", "classes": 4})).unwrap(); + assert!(notices(&mut s).is_empty()); + s.run("layer.symbolBy", &json!({"layer": "Countries", "strokeWidth": "2"})).unwrap(); + assert!(notices(&mut s).is_empty(), "a width is no color"); + s.run("layer.symbolBy", &json!({"layer": "Countries", "fill": "if(pop_est > 1e8, '#ff0000', NULL)"})).unwrap(); + let said = notices(&mut s); + assert_eq!(said.len(), 1, "{said:?}"); + assert!(said[0].starts_with("The layer's Symbol by Expression sets some features' colors"), "{said:?}"); + s.run("layer.symbolBy", &json!({"layer": "Countries", "fill": null, "stroke": "'#00ff00'"})).unwrap(); + assert_eq!(notices(&mut s).len(), 1, "a stroke color counts too"); +} + #[test] fn layer_invert_ramp_says_when_there_is_nothing_to_invert() { // #1355: with fewer than two classes showing the scheme, an inversion would change nothing, so diff --git a/docs/user-guide/cartography.md b/docs/user-guide/cartography.md index e2ab98f4d..eee808d3d 100644 --- a/docs/user-guide/cartography.md +++ b/docs/user-guide/cartography.md @@ -201,6 +201,8 @@ When the layer's own stack (the one classes without a stack of their own are dra colour in every layer, the classes show those colours and little of the colour scheme: the colours are reversed all the same, and a notice says so. On a layer that holds several kinds of geometry, the reversal treats every class as the layer's main kind, as choosing a colour scheme does. +A fill or stroke colour from **Symbol by Expression** is drawn over the classes' colours wherever +the expression gives one, so the reversal shows only where it gives none, and a notice says so. Choosing a colour scheme again starts from the scheme's own order, but classing the layer again on the same field, ranges and scheme keeps the reversed colours, as it keeps other colours edited