From 78527ed64c2c5a92faaf166cc5d4a87ec8f05681 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Mon, 29 Jun 2026 14:18:08 +0200 Subject: [PATCH 1/9] Integrate lon/lat scale limits and breaks into map projection pipeline SCALE lon/lat FROM [...] now overrides the data-derived bbox for viewport extent, and SCALE lon/lat SETTING breaks => [...] controls graticule line placement. Scale limits sit between data extent and PROJECT bounds in the precedence chain. Per-element fallback allows partial constraints (e.g. FROM (null, 40) constrains only the upper bound). OOB filtering is skipped for map position scales since their limits are in EPSG:4326 while data columns contain projected coordinates. Co-Authored-By: Claude Opus 4.6 --- src/execute/mod.rs | 3 + src/execute/scale.rs | 11 ++ src/plot/projection/coord/map.rs | 101 ++++++++++++++++++- src/plot/projection/coord/map_projections.rs | 3 + src/plot/projection/coord/mod.rs | 4 + src/plot/projection/types.rs | 11 +- 6 files changed, 129 insertions(+), 4 deletions(-) diff --git a/src/execute/mod.rs b/src/execute/mod.rs index 59be3b80..f1afd052 100644 --- a/src/execute/mod.rs +++ b/src/execute/mod.rs @@ -1383,12 +1383,15 @@ pub fn prepare_data_with_reader(query: &str, reader: &dyn Reader) -> Result( /// - The scale has an explicit input range, AND /// - NULL is not part of the explicit input range pub fn apply_scale_oob(spec: &Plot, data_map: &mut HashMap) -> Result<()> { + use crate::plot::projection::CoordKind; + let aesthetic_ctx = spec.get_aesthetic_context(); + let is_map = spec + .project + .as_ref() + .is_some_and(|p| p.coord.coord_kind() == CoordKind::Map); // First pass: apply OOB transformations (censor sets to NULL, squish clamps) for scale in &spec.scales { + // Map position scales have limits in geographic coordinates but data in projected + // CRS — viewport clipping is handled by the projection pipeline, not OOB. + if is_map && (scale.aesthetic == "pos1" || scale.aesthetic == "pos2") { + continue; + } // Get oob mode: // - If explicitly set, use that value (skip if "keep") // - If not set but has explicit input range, use default for aesthetic diff --git a/src/plot/projection/coord/map.rs b/src/plot/projection/coord/map.rs index 6d4ddae9..2ff527fc 100644 --- a/src/plot/projection/coord/map.rs +++ b/src/plot/projection/coord/map.rs @@ -4,6 +4,7 @@ use super::map_projections::MapProjectionTrait; use crate::naming; use crate::plot::layer::geom::GeomType; use crate::plot::scale::breaks::graticule_breaks; +use crate::plot::scale::Scale; use crate::plot::{DataSource, Layer, ParameterValue, Parameters}; use crate::reader::SqlDialect; use crate::DataFrame; @@ -17,6 +18,7 @@ pub(crate) fn apply_map_transforms( layers: &mut [Layer], layer_queries: &mut [String], projection: &mut super::super::Projection, + scales: &[Scale], dialect: &dyn SqlDialect, execute_query: &dyn Fn(&str) -> crate::Result, ) -> crate::Result<()> { @@ -111,7 +113,10 @@ pub(crate) fn apply_map_transforms( }; } - // Step 5: Resolve final frame bbox from user bounds + data bounds + world bounds + // Step 5: Resolve final frame bbox from user bounds + data bounds + world bounds. + // Scale limits (SCALE lon/lat FROM [...]) override data_bbox when present. + let data_bbox = + scale_override_bbox(scales, data_bbox, &source, &target, dialect, execute_query); let Some(bbox) = resolve_final_bbox(user_bbox, data_bbox, world_bbox) else { return Ok(()); }; @@ -125,6 +130,7 @@ pub(crate) fn apply_map_transforms( &bbox, boundary_lonlat.as_deref(), &target, + scales, dialect, execute_query, )?; @@ -142,6 +148,89 @@ pub(crate) fn apply_map_transforms( Ok(()) } +// --------------------------------------------------------------------------- +// Scale-to-projection integration +// --------------------------------------------------------------------------- + +/// If any position scale has explicit limits (FROM clause), build a geographic bbox +/// from those limits, project it to target CRS, and return it as the override for +/// data_bbox. Returns the original data_bbox unchanged when no scale limits are present. +fn scale_override_bbox( + scales: &[Scale], + data_bbox: Option, + source: &str, + target: &str, + dialect: &dyn SqlDialect, + execute_query: &dyn Fn(&str) -> crate::Result, +) -> Option { + let numeric_range = |aesthetic: &str| -> Option<&[crate::plot::types::ArrayElement]> { + let range = scales + .iter() + .find(|s| s.aesthetic == aesthetic && s.explicit_input_range)? + .input_range + .as_deref()?; + if range.len() == 2 && range.iter().any(|e| e.to_f64().is_some()) { + Some(range) + } else { + None + } + }; + + let lon = numeric_range("pos1"); + let lat = numeric_range("pos2"); + + if lon.is_none() && lat.is_none() { + return data_bbox; + } + + // Inverse-project data_bbox to EPSG:4326 as fallback for unconstrained elements + let geo_fallback = data_bbox + .as_ref() + .and_then(|b| b.reproject("EPSG:4326", dialect, execute_query)); + + let lon_min = lon + .and_then(|r| r.first()?.to_f64()) + .or_else(|| geo_fallback.as_ref().map(|b| b.xmin))?; + let lon_max = lon + .and_then(|r| r.last()?.to_f64()) + .or_else(|| geo_fallback.as_ref().map(|b| b.xmax))?; + let lat_min = lat + .and_then(|r| r.first()?.to_f64()) + .or_else(|| geo_fallback.as_ref().map(|b| b.ymin))?; + let lat_max = lat + .and_then(|r| r.last()?.to_f64()) + .or_else(|| geo_fallback.as_ref().map(|b| b.ymax))?; + + let geo_bbox = BBox { + xmin: lon_min, + ymin: lat_min, + xmax: lon_max, + ymax: lat_max, + crs: source.to_string(), + }; + + geo_bbox + .reproject(target, dialect, execute_query) + .or(data_bbox) +} + +/// Extract explicit break positions from a scale's properties. +/// Returns None if the scale has no explicit breaks (i.e. breaks is absent or a count). +fn scale_breaks(scales: &[Scale], aesthetic: &str) -> Option> { + let scale = scales.iter().find(|s| s.aesthetic == aesthetic)?; + match scale.properties.get("breaks") { + Some(ParameterValue::Array(arr)) => { + let breaks: Vec = arr.iter().filter_map(|e| e.to_f64()).collect(); + if breaks.is_empty() { + None + } else { + Some(breaks) + } + } + _ => None, + } +} + // --------------------------------------------------------------------------- // BBox // --------------------------------------------------------------------------- @@ -268,10 +357,14 @@ impl BBox { /// Build graticule lines: determine the visible lon/lat extent, generate densified /// meridians and parallels, clip and project them, and return projected WKT. +/// +/// When scales have explicit `BREAKS`, those are used directly instead of +/// auto-computing graticule positions. fn build_graticule( bbox: &BBox, clip_boundary_wkt: Option<&str>, crs: &str, + scales: &[Scale], dialect: &dyn SqlDialect, execute_query: &dyn Fn(&str) -> crate::Result, ) -> crate::Result<(Option, Option)> { @@ -281,8 +374,10 @@ fn build_graticule( let (lon_min, lon_max) = geo_bbox.xrange(); let (lat_min, lat_max) = geo_bbox.yrange(); - let lon_breaks = graticule_breaks(lon_min, lon_max, 7); - let lat_breaks = graticule_breaks(lat_min, lat_max, 7); + let lon_breaks = + scale_breaks(scales, "pos1").unwrap_or_else(|| graticule_breaks(lon_min, lon_max, 7)); + let lat_breaks = + scale_breaks(scales, "pos2").unwrap_or_else(|| graticule_breaks(lat_min, lat_max, 7)); if lon_breaks.is_empty() && lat_breaks.is_empty() { return Ok((None, None)); diff --git a/src/plot/projection/coord/map_projections.rs b/src/plot/projection/coord/map_projections.rs index 0358ba65..2ab74e36 100644 --- a/src/plot/projection/coord/map_projections.rs +++ b/src/plot/projection/coord/map_projections.rs @@ -2,6 +2,7 @@ use std::fmt; use std::sync::Arc; use super::CoordKind; +use crate::plot::scale::Scale; use crate::plot::types::{ validate_parameter, ArrayElement, DefaultParamValue, ParamConstraint, ParamDefinition, Parameters, TypeConstraint, @@ -240,6 +241,7 @@ impl super::CoordTrait for T { layers: &mut [Layer], layer_queries: &mut [String], projection: &mut super::super::Projection, + scales: &[Scale], dialect: &dyn SqlDialect, execute_query: &dyn Fn(&str) -> crate::Result, ) -> crate::Result<()> { @@ -248,6 +250,7 @@ impl super::CoordTrait for T { layers, layer_queries, projection, + scales, dialect, execute_query, ) diff --git a/src/plot/projection/coord/mod.rs b/src/plot/projection/coord/mod.rs index 39d08f2b..942fba47 100644 --- a/src/plot/projection/coord/mod.rs +++ b/src/plot/projection/coord/mod.rs @@ -23,6 +23,7 @@ use serde::{Deserialize, Serialize}; use std::sync::Arc; +use crate::plot::scale::Scale; use crate::plot::types::{validate_parameter, ParamDefinition, Parameters}; use crate::plot::Layer; use crate::reader::SqlDialect; @@ -146,6 +147,7 @@ pub trait CoordTrait: std::fmt::Debug + Send + Sync { layers: &mut [Layer], layer_queries: &mut [String], projection: &mut super::Projection, + _scales: &[Scale], dialect: &dyn SqlDialect, _execute_query: &dyn Fn(&str) -> crate::Result, ) -> crate::Result<()> { @@ -243,6 +245,7 @@ impl Coord { layers: &mut [Layer], layer_queries: &mut [String], projection: &mut super::Projection, + scales: &[Scale], dialect: &dyn SqlDialect, execute_query: &dyn Fn(&str) -> crate::Result, ) -> crate::Result<()> { @@ -250,6 +253,7 @@ impl Coord { layers, layer_queries, projection, + scales, dialect, execute_query, ) diff --git a/src/plot/projection/types.rs b/src/plot/projection/types.rs index d4b32009..e810e513 100644 --- a/src/plot/projection/types.rs +++ b/src/plot/projection/types.rs @@ -6,6 +6,7 @@ use serde::{Deserialize, Serialize}; use std::collections::HashMap; use super::coord::Coord; +use crate::plot::scale::Scale; use crate::plot::{Layer, Parameters}; use crate::reader::SqlDialect; use crate::DataFrame; @@ -69,10 +70,18 @@ impl Projection { &mut self, layers: &mut [Layer], layer_queries: &mut [String], + scales: &[Scale], dialect: &dyn SqlDialect, execute_query: &dyn Fn(&str) -> crate::Result, ) -> crate::Result<()> { let coord = self.coord.clone(); - coord.apply_projection_transforms(layers, layer_queries, self, dialect, execute_query) + coord.apply_projection_transforms( + layers, + layer_queries, + self, + scales, + dialect, + execute_query, + ) } } From d243432a034d00a0db82057bb8e992841d8c3e97 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Mon, 29 Jun 2026 14:18:36 +0200 Subject: [PATCH 2/9] Add tests for scale-to-projection integration Unit tests for scale_override_bbox (numeric limits, string limits, no explicit limits, mixed null/numeric) and scale_breaks (explicit array, numeric count ignored, absent, wrong aesthetic). Integration tests verify the full DuckDB path: scale limits constrain the projected bbox, and explicit breaks produce the expected number of graticule meridians. Co-Authored-By: Claude Opus 4.6 --- src/lib.rs | 88 +++++++++++++++ src/plot/projection/coord/map.rs | 182 +++++++++++++++++++++++++++++++ 2 files changed, 270 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index bdb8a521..c28198b1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1293,4 +1293,92 @@ mod integration_tests { assert!(x.abs() < 2000.0, "Expected x near 0, got {x}"); assert!(y.abs() < 2000.0, "Expected y near 0, got {y}"); } + + #[cfg(feature = "spatial")] + #[test] + fn test_scale_limits_override_map_bbox() { + use crate::plot::types::{ArrayElement, ParameterValue}; + + let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap(); + + // Query with explicit lon/lat scale limits on a mercator map + let query = r#" + VISUALISE FROM ggsql:world + DRAW spatial PROJECT TO mercator + SCALE lon FROM [5, 15] + SCALE lat FROM [45, 55] + "#; + + let prepared = execute::prepare_data_with_reader(query, &reader).unwrap(); + + let project = prepared.specs[0].project.as_ref().unwrap(); + let bbox_param = project + .computed + .get("bbox") + .expect("bbox should be computed"); + + // The bbox should reflect the projected [5,15] x [45,55] extent, + // not the full world data extent. + let ParameterValue::Array(bbox_arr) = bbox_param else { + panic!("bbox should be an Array"); + }; + let xmin = bbox_arr[0].to_f64().unwrap(); + let ymin = bbox_arr[1].to_f64().unwrap(); + let xmax = bbox_arr[2].to_f64().unwrap(); + let ymax = bbox_arr[3].to_f64().unwrap(); + + // In Web Mercator (EPSG:3857), lon 5° ≈ 556597, lon 15° ≈ 1669792 + // lat 45° ≈ 5621521, lat 55° ≈ 7361866 + // The bbox should be in that ballpark, not world-sized (~±20M). + assert!( + xmin > 400_000.0 && xmin < 700_000.0, + "xmin out of range: {xmin}" + ); + assert!( + xmax > 1_500_000.0 && xmax < 1_800_000.0, + "xmax out of range: {xmax}" + ); + assert!( + ymin > 5_000_000.0 && ymin < 6_000_000.0, + "ymin out of range: {ymin}" + ); + assert!( + ymax > 7_000_000.0 && ymax < 7_500_000.0, + "ymax out of range: {ymax}" + ); + } + + #[cfg(feature = "spatial")] + #[test] + fn test_scale_breaks_control_graticule() { + use crate::plot::types::ParameterValue; + + let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap(); + + let query = r#" + VISUALISE FROM ggsql:world + DRAW spatial PROJECT TO mercator + SCALE lon SETTING breaks => [0, 30, 60, 90] + "#; + + let prepared = execute::prepare_data_with_reader(query, &reader).unwrap(); + + let project = prepared.specs[0].project.as_ref().unwrap(); + let grat_lon = project + .computed + .get("graticule_lon") + .expect("graticule_lon should be set"); + + let ParameterValue::String(wkt) = grat_lon else { + panic!("graticule_lon should be a String (WKT)"); + }; + + // Each meridian in the MULTILINESTRING is a (...) group. + // Count by splitting on "), (" which separates individual lines. + let line_count = wkt.split("), (").count(); + assert_eq!( + line_count, 4, + "Expected 4 graticule meridians for breaks [0, 30, 60, 90], got {line_count}\nWKT: {wkt:.200}" + ); + } } diff --git a/src/plot/projection/coord/map.rs b/src/plot/projection/coord/map.rs index 2ff527fc..3d31a22e 100644 --- a/src/plot/projection/coord/map.rs +++ b/src/plot/projection/coord/map.rs @@ -1247,6 +1247,188 @@ mod tests { Err(crate::GgsqlError::InternalError("no db".into())) } + fn noop_dialect() -> crate::reader::duckdb::DuckDbDialect { + crate::reader::duckdb::DuckDbDialect + } + + mod scale_override_bbox_tests { + use super::*; + use crate::plot::scale::Scale; + use crate::plot::types::ArrayElement; + + fn make_scale(aesthetic: &str, range: Vec, explicit: bool) -> Scale { + let mut s = Scale::new(aesthetic); + s.input_range = Some(range); + s.explicit_input_range = explicit; + s + } + + #[test] + fn numeric_limits_override_data_bbox() { + let scales = vec![ + make_scale( + "pos1", + vec![ArrayElement::Number(10.0), ArrayElement::Number(20.0)], + true, + ), + make_scale( + "pos2", + vec![ArrayElement::Number(30.0), ArrayElement::Number(50.0)], + true, + ), + ]; + let data_bbox = Some(BBox { + xmin: 0.0, + ymin: 0.0, + xmax: 100.0, + ymax: 100.0, + crs: "EPSG:4326".to_string(), + }); + + let result = scale_override_bbox( + &scales, + data_bbox.clone(), + "EPSG:4326", + "EPSG:4326", + &noop_dialect(), + &noop_execute, + ); + + // reproject fails with noop_execute, so falls back to data_bbox + // But the function still enters the override path (does not early-return) + // With a real executor this would produce a new bbox from the scale limits + assert_eq!(result, data_bbox); + } + + #[test] + fn string_limits_ignored() { + let scales = vec![make_scale( + "pos1", + vec![ + ArrayElement::String("A".into()), + ArrayElement::String("B".into()), + ], + true, + )]; + let data_bbox = Some(BBox { + xmin: 0.0, + ymin: 0.0, + xmax: 100.0, + ymax: 100.0, + crs: "EPSG:4326".to_string(), + }); + + let result = scale_override_bbox( + &scales, + data_bbox.clone(), + "EPSG:4326", + "EPSG:4326", + &noop_dialect(), + &noop_execute, + ); + + assert_eq!(result, data_bbox); + } + + #[test] + fn no_explicit_limits_returns_data_bbox() { + let scales = vec![make_scale( + "pos1", + vec![ArrayElement::Number(10.0), ArrayElement::Number(20.0)], + false, // not explicit + )]; + let data_bbox = Some(BBox { + xmin: 5.0, + ymin: 5.0, + xmax: 50.0, + ymax: 50.0, + crs: "EPSG:4326".to_string(), + }); + + let result = scale_override_bbox( + &scales, + data_bbox.clone(), + "EPSG:4326", + "EPSG:4326", + &noop_dialect(), + &noop_execute, + ); + + assert_eq!(result, data_bbox); + } + + #[test] + fn mixed_null_numeric_without_fallback_returns_none() { + // pos1 has (null, 20), pos2 absent — no data_bbox to fall back on + let scales = vec![make_scale( + "pos1", + vec![ArrayElement::Null, ArrayElement::Number(20.0)], + true, + )]; + + let result = scale_override_bbox( + &scales, + None, + "EPSG:4326", + "EPSG:4326", + &noop_dialect(), + &noop_execute, + ); + + assert_eq!(result, None); + } + } + + mod scale_breaks_tests { + use super::*; + use crate::plot::scale::Scale; + use crate::plot::types::ArrayElement; + + #[test] + fn explicit_array_breaks_returned() { + let mut s = Scale::new("pos1"); + s.properties.insert( + "breaks".to_string(), + ParameterValue::Array(vec![ + ArrayElement::Number(0.0), + ArrayElement::Number(30.0), + ArrayElement::Number(60.0), + ]), + ); + let scales = vec![s]; + + assert_eq!(scale_breaks(&scales, "pos1"), Some(vec![0.0, 30.0, 60.0])); + } + + #[test] + fn numeric_break_count_ignored() { + let mut s = Scale::new("pos1"); + s.properties + .insert("breaks".to_string(), ParameterValue::Number(5.0)); + let scales = vec![s]; + + assert_eq!(scale_breaks(&scales, "pos1"), None); + } + + #[test] + fn absent_breaks_returns_none() { + let scales = vec![Scale::new("pos1")]; + assert_eq!(scale_breaks(&scales, "pos1"), None); + } + + #[test] + fn wrong_aesthetic_returns_none() { + let mut s = Scale::new("pos2"); + s.properties.insert( + "breaks".to_string(), + ParameterValue::Array(vec![ArrayElement::Number(10.0)]), + ); + let scales = vec![s]; + + assert_eq!(scale_breaks(&scales, "pos1"), None); + } + } + #[test] fn resolve_epsg_property_from_existing() { let mut props = Parameters::new(); From 279081faa90b5478a53c349b288540f9e0c28b65 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Mon, 29 Jun 2026 14:37:25 +0200 Subject: [PATCH 3/9] add news bullet --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b9eb1a45..e306862a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ ## [Unreleased] +### Changed + +- Position scales like `SCALE lon` and `SCALE lat` transfer their limits to + map projections, and transfer their `breaks` setting to the graticule (#492). + ## 0.4.1 - 2026-06-22 ### Changed From e27ebbb8bf175a63497d527a922639a87c5e0db9 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Fri, 3 Jul 2026 11:31:38 +0200 Subject: [PATCH 4/9] Add geographic scale transform for map position scales MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Identity transform whose `calculate_breaks` delegates to `graticule_breaks`, producing degree-aligned break positions (1°, 2°, 5°, 10°, 15°, 30°, etc.). Registered in the transform system with aliases "geographic" and "geo". Added to the Continuous scale type match arm so `VIA geographic` works. Co-Authored-By: Claude Opus 4.6 --- src/execute/scale.rs | 4 +- src/plot/scale/transform/geographic.rs | 88 ++++++++++++++++++++++++++ src/plot/scale/transform/mod.rs | 14 ++++ src/writer/vegalite/encoding.rs | 2 + 4 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 src/plot/scale/transform/geographic.rs diff --git a/src/execute/scale.rs b/src/execute/scale.rs index 233a3512..5ea30335 100644 --- a/src/execute/scale.rs +++ b/src/execute/scale.rs @@ -434,7 +434,9 @@ pub fn resolve_scale_types_and_transforms( | TransformKind::Asinh | TransformKind::PseudoLog // Integer transform uses Continuous scale - | TransformKind::Integer => ScaleType::continuous(), + | TransformKind::Integer + // Geographic transform uses Continuous scale + | TransformKind::Geographic => ScaleType::continuous(), // Discrete transforms (String, Bool) use Discrete scale TransformKind::String | TransformKind::Bool => ScaleType::discrete(), // Identity: fall back to dtype inference (considers aesthetic) diff --git a/src/plot/scale/transform/geographic.rs b/src/plot/scale/transform/geographic.rs new file mode 100644 index 00000000..a6432947 --- /dev/null +++ b/src/plot/scale/transform/geographic.rs @@ -0,0 +1,88 @@ +use super::{TransformKind, TransformTrait}; +use crate::plot::scale::breaks::graticule_breaks; + +#[derive(Debug, Clone, Copy)] +pub struct Geographic; + +impl TransformTrait for Geographic { + fn transform_kind(&self) -> TransformKind { + TransformKind::Geographic + } + + fn name(&self) -> &'static str { + "geographic" + } + + fn allowed_domain(&self) -> (f64, f64) { + (f64::NEG_INFINITY, f64::INFINITY) + } + + fn calculate_breaks(&self, min: f64, max: f64, n: usize, _pretty: bool) -> Vec { + graticule_breaks(min, max, n) + } + + fn calculate_minor_breaks( + &self, + _major_breaks: &[f64], + _n: usize, + _range: Option<(f64, f64)>, + ) -> Vec { + Vec::new() + } + + fn transform(&self, value: f64) -> f64 { + value + } + + fn inverse(&self, value: f64) -> f64 { + value + } +} + +impl std::fmt::Display for Geographic { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.name()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_breaks_world_longitude() { + let t = Geographic; + let breaks = t.calculate_breaks(-180.0, 180.0, 7, true); + assert!(!breaks.is_empty()); + for &b in &breaks { + assert!(b >= -180.0 && b <= 180.0); + } + // Should pick nice degree intervals (multiples of 30° or 45°) + assert!(breaks.iter().all(|b| b % 30.0 == 0.0 || b % 45.0 == 0.0)); + } + + #[test] + fn test_breaks_small_extent() { + let t = Geographic; + let breaks = t.calculate_breaks(5.0, 15.0, 5, true); + assert!(!breaks.is_empty()); + for &b in &breaks { + assert!(b > 5.0 && b < 15.0); + } + } + + #[test] + fn test_breaks_respects_count() { + let t = Geographic; + let breaks_few = t.calculate_breaks(-90.0, 90.0, 3, true); + let breaks_many = t.calculate_breaks(-90.0, 90.0, 10, true); + assert!(breaks_many.len() >= breaks_few.len()); + } + + #[test] + fn test_identity_transform() { + let t = Geographic; + assert_eq!(t.transform(42.0), 42.0); + assert_eq!(t.inverse(42.0), 42.0); + } +} diff --git a/src/plot/scale/transform/mod.rs b/src/plot/scale/transform/mod.rs index ed173eb1..a8b4ffc7 100644 --- a/src/plot/scale/transform/mod.rs +++ b/src/plot/scale/transform/mod.rs @@ -43,6 +43,7 @@ mod bool; mod date; mod datetime; mod exp; +mod geographic; mod identity; mod integer; mod log; @@ -57,6 +58,7 @@ pub use self::bool::Bool; pub use self::date::Date; pub use self::datetime::DateTime; pub use self::exp::Exp; +pub use self::geographic::Geographic; pub use self::identity::Identity; pub use self::integer::Integer; pub use self::log::Log; @@ -104,6 +106,8 @@ pub enum TransformKind { Bool, /// Integer transform (linear with integer casting) Integer, + /// Geographic transform (for map position scales — degree-aligned breaks) + Geographic, } impl TransformKind { @@ -136,6 +140,7 @@ impl std::fmt::Display for TransformKind { TransformKind::String => "string", TransformKind::Bool => "bool", TransformKind::Integer => "integer", + TransformKind::Geographic => "geographic", }; write!(f, "{}", name) } @@ -336,6 +341,11 @@ impl Transform { Self(Arc::new(Integer)) } + /// Create a Geographic transform (degree-aligned breaks for map projections) + pub fn geographic() -> Self { + Self(Arc::new(Geographic)) + } + /// Create a Transform from a string name /// /// Returns None if the name is not recognized. @@ -371,6 +381,7 @@ impl Transform { "string" | "str" | "varchar" => Some(Self::string()), "bool" | "boolean" => Some(Self::bool()), "integer" | "int" | "bigint" => Some(Self::integer()), + "geographic" | "geo" => Some(Self::geographic()), _ => None, } } @@ -395,6 +406,7 @@ impl Transform { TransformKind::String => Self::string(), TransformKind::Bool => Self::bool(), TransformKind::Integer => Self::integer(), + TransformKind::Geographic => Self::geographic(), } } @@ -602,6 +614,8 @@ pub const ALL_TRANSFORM_NAMES: &[&str] = &[ "integer", "int", // alias for integer "bigint", // alias for integer + "geographic", + "geo", // alias for geographic ]; #[cfg(test)] diff --git a/src/writer/vegalite/encoding.rs b/src/writer/vegalite/encoding.rs index ffe714bf..9020c7af 100644 --- a/src/writer/vegalite/encoding.rs +++ b/src/writer/vegalite/encoding.rs @@ -612,6 +612,8 @@ fn apply_transform_to_scale( TransformKind::String | TransformKind::Bool => {} // Integer transform: casting happens at SQL level TransformKind::Integer => {} + // Geographic transform: degree-aligned breaks, no VL equivalent + TransformKind::Geographic => {} } } From 7c4c42962b1cce010e0b9985dd1a8a9b0ec93dd1 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Fri, 3 Jul 2026 11:54:28 +0200 Subject: [PATCH 5/9] Auto-assign geographic transform to map position scales Extracts an `infer_transform` helper to unify the two duplicate transform-inference code paths. Map position scales (pos1/pos2) now receive the Geographic transform automatically when a map projection is active, so graticule breaks use degree-aligned logic. Co-Authored-By: Claude Opus 4.6 --- src/execute/scale.rs | 72 +++++++++++-------------- src/plot/scale/scale_type/continuous.rs | 1 + 2 files changed, 31 insertions(+), 42 deletions(-) diff --git a/src/execute/scale.rs b/src/execute/scale.rs index 5ea30335..38e8579c 100644 --- a/src/execute/scale.rs +++ b/src/execute/scale.rs @@ -6,9 +6,10 @@ use crate::naming; use crate::plot::aesthetic::AestheticContext; +use crate::plot::projection::CoordKind; use crate::plot::scale::{ default_oob, gets_default_scale, infer_scale_target_type, infer_transform_from_input_range, - is_facet_aesthetic, transform::Transform, OOB_CENSOR, OOB_KEEP, OOB_SQUISH, + is_facet_aesthetic, transform::Transform, TransformKind, OOB_CENSOR, OOB_KEEP, OOB_SQUISH, }; use crate::plot::{ AestheticValue, ArrayElement, ArrayElementType, ColumnInfo, Layer, ParameterValue, Plot, Scale, @@ -328,6 +329,10 @@ pub fn resolve_scale_types_and_transforms( use crate::plot::scale::coerce_dtypes; let aesthetic_ctx = spec.get_aesthetic_context(); + let is_map = spec + .project + .as_ref() + .is_some_and(|p| p.coord.coord_kind() == CoordKind::Map); for scale in &mut spec.scales { // Skip scales that already have explicit types (user specified) @@ -362,26 +367,7 @@ pub fn resolve_scale_types_and_transforms( // Resolve transform if not set if scale.transform.is_none() && !scale.explicit_transform { - // For Discrete/Ordinal scales, check input range first for transform inference - // This allows SCALE DISCRETE x FROM [true, false] to infer Bool transform - // even when the column is String - let transform_kind = if matches!( - scale_type.scale_type_kind(), - ScaleTypeKind::Discrete | ScaleTypeKind::Ordinal - ) { - if let Some(ref input_range) = scale.input_range { - if let Some(kind) = infer_transform_from_input_range(input_range) { - kind - } else { - scale_type - .default_transform(&scale.aesthetic, Some(&common_dtype)) - } - } else { - scale_type.default_transform(&scale.aesthetic, Some(&common_dtype)) - } - } else { - scale_type.default_transform(&scale.aesthetic, Some(&common_dtype)) - }; + let transform_kind = infer_transform(scale, &common_dtype, is_map); scale.transform = Some(Transform::from_kind(transform_kind)); } } @@ -416,7 +402,6 @@ pub fn resolve_scale_types_and_transforms( // If user specified VIA date/datetime/time/log/sqrt/etc., use Continuous scale let inferred_scale_type = if scale.explicit_transform { if let Some(ref transform) = scale.transform { - use crate::plot::scale::TransformKind; match transform.transform_kind() { // Temporal transforms require Continuous scale TransformKind::Date @@ -454,23 +439,7 @@ pub fn resolve_scale_types_and_transforms( // Infer transform if not explicit if scale.transform.is_none() && !scale.explicit_transform { - // For Discrete scales, check input range first for transform inference - // This allows SCALE DISCRETE x FROM [true, false] to infer Bool transform - // even when the column is String - let transform_kind = if inferred_scale_type.scale_type_kind() == ScaleTypeKind::Discrete - { - if let Some(ref input_range) = scale.input_range { - if let Some(kind) = infer_transform_from_input_range(input_range) { - kind - } else { - inferred_scale_type.default_transform(&scale.aesthetic, Some(&common_dtype)) - } - } else { - inferred_scale_type.default_transform(&scale.aesthetic, Some(&common_dtype)) - } - } else { - inferred_scale_type.default_transform(&scale.aesthetic, Some(&common_dtype)) - }; + let transform_kind = infer_transform(scale, &common_dtype, is_map); scale.transform = Some(Transform::from_kind(transform_kind)); } } @@ -478,6 +447,28 @@ pub fn resolve_scale_types_and_transforms( Ok(()) } +fn infer_transform( + scale: &Scale, + common_dtype: &arrow::datatypes::DataType, + is_map: bool, +) -> TransformKind { + if is_map && (scale.aesthetic == "pos1" || scale.aesthetic == "pos2") { + return TransformKind::Geographic; + } + let scale_type = scale.scale_type.as_ref().unwrap(); + if matches!( + scale_type.scale_type_kind(), + ScaleTypeKind::Discrete | ScaleTypeKind::Ordinal + ) { + if let Some(ref input_range) = scale.input_range { + if let Some(kind) = infer_transform_from_input_range(input_range) { + return kind; + } + } + } + scale_type.default_transform(&scale.aesthetic, Some(common_dtype)) +} + /// Collect all dtypes for an aesthetic across layers. pub fn collect_dtypes_for_aesthetic( layers: &[Layer], @@ -942,7 +933,6 @@ pub fn coerce_aesthetic_columns( /// /// Scales that were already resolved pre-stat (Binned scales) are skipped. pub fn resolve_scales(spec: &mut Plot, data_map: &mut HashMap) -> Result<()> { - use crate::plot::projection::CoordKind; use crate::plot::scale::ScaleDataContext; let aesthetic_ctx = spec.get_aesthetic_context(); @@ -1086,8 +1076,6 @@ pub fn find_columns_for_aesthetic<'a>( /// - The scale has an explicit input range, AND /// - NULL is not part of the explicit input range pub fn apply_scale_oob(spec: &Plot, data_map: &mut HashMap) -> Result<()> { - use crate::plot::projection::CoordKind; - let aesthetic_ctx = spec.get_aesthetic_context(); let is_map = spec .project diff --git a/src/plot/scale/scale_type/continuous.rs b/src/plot/scale/scale_type/continuous.rs index 22c26c93..22d0cd43 100644 --- a/src/plot/scale/scale_type/continuous.rs +++ b/src/plot/scale/scale_type/continuous.rs @@ -74,6 +74,7 @@ impl ScaleTypeTrait for Continuous { TransformKind::Date, TransformKind::DateTime, TransformKind::Time, + TransformKind::Geographic, ] } From c2479d5c8a8dc9651f78c77a9405dfc461a4b2f3 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Fri, 3 Jul 2026 12:33:55 +0200 Subject: [PATCH 6/9] Resolve map position scales during graticule generation Propagates `&mut [Scale]` through the projection trait chain so that `build_graticule` can call `resolve()` on position scales with a synthetic ScaleDataContext from the geographic bbox. After resolution, breaks are read via `Scale::numeric_breaks()`. Scales are marked `resolved = true` so the later `resolve_scales` pass skips them. Also adds `ScaleDataContext::from_range()` constructor. Co-Authored-By: Claude Opus 4.6 --- src/execute/mod.rs | 4 +- src/plot/projection/coord/map.rs | 114 ++++++++++--------- src/plot/projection/coord/map_projections.rs | 2 +- src/plot/projection/coord/mod.rs | 4 +- src/plot/projection/types.rs | 2 +- src/plot/scale/scale_type/binned.rs | 20 +--- src/plot/scale/scale_type/mod.rs | 13 +++ 7 files changed, 79 insertions(+), 80 deletions(-) diff --git a/src/execute/mod.rs b/src/execute/mod.rs index f1afd052..eaf34031 100644 --- a/src/execute/mod.rs +++ b/src/execute/mod.rs @@ -1383,11 +1383,11 @@ pub fn prepare_data_with_reader(query: &str, reader: &dyn Reader) -> Result crate::Result, ) -> crate::Result<()> { @@ -214,21 +213,25 @@ fn scale_override_bbox( .or(data_bbox) } -/// Extract explicit break positions from a scale's properties. -/// Returns None if the scale has no explicit breaks (i.e. breaks is absent or a count). -fn scale_breaks(scales: &[Scale], aesthetic: &str) -> Option> { - let scale = scales.iter().find(|s| s.aesthetic == aesthetic)?; - match scale.properties.get("breaks") { - Some(ParameterValue::Array(arr)) => { - let breaks: Vec = arr.iter().filter_map(|e| e.to_f64()).collect(); - if breaks.is_empty() { - None - } else { - Some(breaks) - } - } - _ => None, - } +/// Resolve a map position scale using the geographic bounding box as synthetic data. +/// +/// After this call, the scale's `properties["breaks"]` will be an Array of positions +/// (computed by the Geographic transform's `calculate_breaks`), and `resolved` will be +/// true so the later `resolve_scales` pass skips it. +fn resolve_map_scale(scale: &mut Scale, range: (f64, f64)) { + if scale.resolved { + return; + } + let Some(ref scale_type) = scale.scale_type.clone() else { + return; + }; + + let mut context = ScaleDataContext::from_range(range.0, range.1); + context.default_expand = Some((0.0, 0.0)); + + let aesthetic_owned = scale.aesthetic.clone(); + let _ = scale_type.resolve(scale, &context, &aesthetic_owned); + scale.resolved = true; } // --------------------------------------------------------------------------- @@ -364,7 +367,7 @@ fn build_graticule( bbox: &BBox, clip_boundary_wkt: Option<&str>, crs: &str, - scales: &[Scale], + scales: &mut [Scale], dialect: &dyn SqlDialect, execute_query: &dyn Fn(&str) -> crate::Result, ) -> crate::Result<(Option, Option)> { @@ -372,12 +375,20 @@ fn build_graticule( return Ok((None, None)); }; - let (lon_min, lon_max) = geo_bbox.xrange(); - let (lat_min, lat_max) = geo_bbox.yrange(); - let lon_breaks = - scale_breaks(scales, "pos1").unwrap_or_else(|| graticule_breaks(lon_min, lon_max, 7)); - let lat_breaks = - scale_breaks(scales, "pos2").unwrap_or_else(|| graticule_breaks(lat_min, lat_max, 7)); + let lon_breaks = match scales.iter_mut().find(|s| s.aesthetic == "pos1") { + Some(s) => { + resolve_map_scale(s, geo_bbox.xrange()); + s.numeric_breaks() + } + None => Vec::new(), + }; + let lat_breaks = match scales.iter_mut().find(|s| s.aesthetic == "pos2") { + Some(s) => { + resolve_map_scale(s, geo_bbox.yrange()); + s.numeric_breaks() + } + None => Vec::new(), + }; if lon_breaks.is_empty() && lat_breaks.is_empty() { return Ok((None, None)); @@ -1379,53 +1390,44 @@ mod tests { } } - mod scale_breaks_tests { + mod resolve_map_scale_tests { use super::*; - use crate::plot::scale::Scale; - use crate::plot::types::ArrayElement; + use crate::plot::scale::transform::Transform; + use crate::plot::scale::ScaleType; - #[test] - fn explicit_array_breaks_returned() { - let mut s = Scale::new("pos1"); - s.properties.insert( - "breaks".to_string(), - ParameterValue::Array(vec![ - ArrayElement::Number(0.0), - ArrayElement::Number(30.0), - ArrayElement::Number(60.0), - ]), - ); - let scales = vec![s]; - - assert_eq!(scale_breaks(&scales, "pos1"), Some(vec![0.0, 30.0, 60.0])); - } + use crate::plot::scale::Scale; #[test] - fn numeric_break_count_ignored() { + fn resolves_breaks_from_range() { let mut s = Scale::new("pos1"); + s.scale_type = Some(ScaleType::continuous()); + s.transform = Some(Transform::geographic()); s.properties .insert("breaks".to_string(), ParameterValue::Number(5.0)); - let scales = vec![s]; - assert_eq!(scale_breaks(&scales, "pos1"), None); - } + resolve_map_scale(&mut s, (-180.0, 180.0)); - #[test] - fn absent_breaks_returns_none() { - let scales = vec![Scale::new("pos1")]; - assert_eq!(scale_breaks(&scales, "pos1"), None); + assert!(s.resolved); + let breaks = s.numeric_breaks(); + assert!(!breaks.is_empty()); + assert!(breaks.iter().all(|&b| (-180.0..=180.0).contains(&b))); } #[test] - fn wrong_aesthetic_returns_none() { - let mut s = Scale::new("pos2"); + fn skips_already_resolved() { + let mut s = Scale::new("pos1"); + s.resolved = true; s.properties.insert( "breaks".to_string(), - ParameterValue::Array(vec![ArrayElement::Number(10.0)]), + ParameterValue::Array(vec![ + crate::plot::types::ArrayElement::Number(10.0), + crate::plot::types::ArrayElement::Number(20.0), + ]), ); - let scales = vec![s]; - assert_eq!(scale_breaks(&scales, "pos1"), None); + resolve_map_scale(&mut s, (-180.0, 180.0)); + + assert_eq!(s.numeric_breaks(), vec![10.0, 20.0]); } } diff --git a/src/plot/projection/coord/map_projections.rs b/src/plot/projection/coord/map_projections.rs index 2ab74e36..365f47ed 100644 --- a/src/plot/projection/coord/map_projections.rs +++ b/src/plot/projection/coord/map_projections.rs @@ -241,7 +241,7 @@ impl super::CoordTrait for T { layers: &mut [Layer], layer_queries: &mut [String], projection: &mut super::super::Projection, - scales: &[Scale], + scales: &mut [Scale], dialect: &dyn SqlDialect, execute_query: &dyn Fn(&str) -> crate::Result, ) -> crate::Result<()> { diff --git a/src/plot/projection/coord/mod.rs b/src/plot/projection/coord/mod.rs index 942fba47..6ec45246 100644 --- a/src/plot/projection/coord/mod.rs +++ b/src/plot/projection/coord/mod.rs @@ -147,7 +147,7 @@ pub trait CoordTrait: std::fmt::Debug + Send + Sync { layers: &mut [Layer], layer_queries: &mut [String], projection: &mut super::Projection, - _scales: &[Scale], + _scales: &mut [Scale], dialect: &dyn SqlDialect, _execute_query: &dyn Fn(&str) -> crate::Result, ) -> crate::Result<()> { @@ -245,7 +245,7 @@ impl Coord { layers: &mut [Layer], layer_queries: &mut [String], projection: &mut super::Projection, - scales: &[Scale], + scales: &mut [Scale], dialect: &dyn SqlDialect, execute_query: &dyn Fn(&str) -> crate::Result, ) -> crate::Result<()> { diff --git a/src/plot/projection/types.rs b/src/plot/projection/types.rs index e810e513..d5d3f309 100644 --- a/src/plot/projection/types.rs +++ b/src/plot/projection/types.rs @@ -70,7 +70,7 @@ impl Projection { &mut self, layers: &mut [Layer], layer_queries: &mut [String], - scales: &[Scale], + scales: &mut [Scale], dialect: &dyn SqlDialect, execute_query: &dyn Fn(&str) -> crate::Result, ) -> crate::Result<()> { diff --git a/src/plot/scale/scale_type/binned.rs b/src/plot/scale/scale_type/binned.rs index 561465d2..2bc19ad8 100644 --- a/src/plot/scale/scale_type/binned.rs +++ b/src/plot/scale/scale_type/binned.rs @@ -2009,15 +2009,7 @@ mod tests { scale.explicit_input_range = false; // Data context with narrower range than breaks - let context = ScaleDataContext { - range: Some(InputRange::Continuous(vec![ - ArrayElement::Number(2700.0), - ArrayElement::Number(6300.0), - ])), - dtype: Some(DataType::Float64), - is_discrete: false, - default_expand: None, - }; + let context = ScaleDataContext::from_range(2700.0, 6300.0); binned.resolve(&mut scale, &context, "fill").unwrap(); @@ -2066,15 +2058,7 @@ mod tests { ]); scale.explicit_input_range = true; - let context = ScaleDataContext { - range: Some(InputRange::Continuous(vec![ - ArrayElement::Number(2700.0), - ArrayElement::Number(6300.0), - ])), - dtype: Some(DataType::Float64), - is_discrete: false, - default_expand: None, - }; + let context = ScaleDataContext::from_range(2700.0, 6300.0); binned.resolve(&mut scale, &context, "fill").unwrap(); diff --git a/src/plot/scale/scale_type/mod.rs b/src/plot/scale/scale_type/mod.rs index 4b2c7df8..c0881a21 100644 --- a/src/plot/scale/scale_type/mod.rs +++ b/src/plot/scale/scale_type/mod.rs @@ -156,6 +156,19 @@ impl ScaleDataContext { } } + /// Create from an explicit numeric range. + pub fn from_range(min: f64, max: f64) -> Self { + Self { + range: Some(InputRange::Continuous(vec![ + ArrayElement::Number(min), + ArrayElement::Number(max), + ])), + dtype: Some(DataType::Float64), + is_discrete: false, + default_expand: None, + } + } + /// Get the continuous range as [min, max] if available. pub fn continuous_range(&self) -> Option<&[ArrayElement]> { match &self.range { From d6c3467da4a56ba9eb38f3fc0a0d815460fcaec0 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Fri, 3 Jul 2026 12:43:59 +0200 Subject: [PATCH 7/9] Remove redundant map position OOB skip Position aesthetics already default to oob_keep, so the explicit map-position check in apply_scale_oob was unreachable. Co-Authored-By: Claude Opus 4.6 --- src/execute/scale.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/execute/scale.rs b/src/execute/scale.rs index 38e8579c..628c108a 100644 --- a/src/execute/scale.rs +++ b/src/execute/scale.rs @@ -1077,18 +1077,9 @@ pub fn find_columns_for_aesthetic<'a>( /// - NULL is not part of the explicit input range pub fn apply_scale_oob(spec: &Plot, data_map: &mut HashMap) -> Result<()> { let aesthetic_ctx = spec.get_aesthetic_context(); - let is_map = spec - .project - .as_ref() - .is_some_and(|p| p.coord.coord_kind() == CoordKind::Map); // First pass: apply OOB transformations (censor sets to NULL, squish clamps) for scale in &spec.scales { - // Map position scales have limits in geographic coordinates but data in projected - // CRS — viewport clipping is handled by the projection pipeline, not OOB. - if is_map && (scale.aesthetic == "pos1" || scale.aesthetic == "pos2") { - continue; - } // Get oob mode: // - If explicitly set, use that value (skip if "keep") // - If not set but has explicit input range, use default for aesthetic From 6212873877694327aec33a5cdc944adceba6bc73 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Fri, 3 Jul 2026 13:01:42 +0200 Subject: [PATCH 8/9] Add integration tests for scale-driven graticule resolution Tests that `SCALE lon SETTING breaks => 4` produces exactly 4 graticule meridians (break count now resolved through geographic transform), and that map position scales are marked resolved after the projection step. Also ensures resolve_map_scale assigns Continuous type and Geographic transform when the scale has none (e.g. spatial-only queries where no column mapping creates position scales). Co-Authored-By: Claude Opus 4.6 --- src/lib.rs | 63 ++++++++++++++++++++++++++++- src/plot/projection/coord/map.rs | 13 ++++-- src/plot/scale/scale_type/binned.rs | 2 - 3 files changed, 72 insertions(+), 6 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index c28198b1..2efd0ca4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1297,7 +1297,7 @@ mod integration_tests { #[cfg(feature = "spatial")] #[test] fn test_scale_limits_override_map_bbox() { - use crate::plot::types::{ArrayElement, ParameterValue}; + use crate::plot::types::ParameterValue; let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap(); @@ -1381,4 +1381,65 @@ mod integration_tests { "Expected 4 graticule meridians for breaks [0, 30, 60, 90], got {line_count}\nWKT: {wkt:.200}" ); } + + #[cfg(feature = "spatial")] + #[test] + fn test_scale_break_count_controls_graticule() { + use crate::plot::types::ParameterValue; + + let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap(); + + let query = r#" + VISUALISE FROM ggsql:world + DRAW spatial PROJECT TO mercator + SCALE lon SETTING breaks => 4 + "#; + + let prepared = execute::prepare_data_with_reader(query, &reader).unwrap(); + + let project = prepared.specs[0].project.as_ref().unwrap(); + let grat_lon = project + .computed + .get("graticule_lon") + .expect("graticule_lon should be set"); + + let ParameterValue::String(wkt) = grat_lon else { + panic!("graticule_lon should be a String (WKT)"); + }; + + let line_count = wkt.split("), (").count(); + assert_eq!( + line_count, 4, + "Expected 4 graticule meridians for breaks => 4, got {line_count}\nWKT: {wkt:.200}" + ); + } + + #[cfg(feature = "spatial")] + #[test] + fn test_map_position_scales_resolved_after_projection() { + let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap(); + + let query = r#" + VISUALISE FROM ggsql:world + DRAW spatial PROJECT TO mercator + "#; + + let prepared = execute::prepare_data_with_reader(query, &reader).unwrap(); + + let spec = &prepared.specs[0]; + for scale in &spec.scales { + if scale.aesthetic == "pos1" || scale.aesthetic == "pos2" { + assert!( + scale.resolved, + "Map position scale '{}' should be marked resolved", + scale.aesthetic + ); + assert!( + !scale.numeric_breaks().is_empty(), + "Map position scale '{}' should have breaks", + scale.aesthetic + ); + } + } + } } diff --git a/src/plot/projection/coord/map.rs b/src/plot/projection/coord/map.rs index e9448053..df0549fd 100644 --- a/src/plot/projection/coord/map.rs +++ b/src/plot/projection/coord/map.rs @@ -219,12 +219,19 @@ fn scale_override_bbox( /// (computed by the Geographic transform's `calculate_breaks`), and `resolved` will be /// true so the later `resolve_scales` pass skips it. fn resolve_map_scale(scale: &mut Scale, range: (f64, f64)) { + use crate::plot::scale::transform::Transform; + use crate::plot::scale::ScaleType; + if scale.resolved { return; } - let Some(ref scale_type) = scale.scale_type.clone() else { - return; - }; + if scale.scale_type.is_none() { + scale.scale_type = Some(ScaleType::continuous()); + } + if scale.transform.is_none() { + scale.transform = Some(Transform::geographic()); + } + let scale_type = scale.scale_type.clone().unwrap(); let mut context = ScaleDataContext::from_range(range.0, range.1); context.default_expand = Some((0.0, 0.0)); diff --git a/src/plot/scale/scale_type/binned.rs b/src/plot/scale/scale_type/binned.rs index 2bc19ad8..1e747395 100644 --- a/src/plot/scale/scale_type/binned.rs +++ b/src/plot/scale/scale_type/binned.rs @@ -1989,7 +1989,6 @@ mod tests { // Issue: breaks like [2600, 3550, 4050, 4750, 6400] were getting terminal // breaks removed when data range was ~[2700, 6300]. use super::ScaleTypeTrait; - use arrow::datatypes::DataType; let binned = Binned; let mut scale = Scale::new("fill"); @@ -2035,7 +2034,6 @@ mod tests { // When BOTH explicit breaks AND explicit range are provided, // breaks should be filtered to the range. use super::ScaleTypeTrait; - use arrow::datatypes::DataType; let binned = Binned; let mut scale = Scale::new("fill"); From 3569ddeb6f62042554cf0e6497ef9777649fa183 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Fri, 3 Jul 2026 14:33:12 +0200 Subject: [PATCH 9/9] Fix graticule regression for spatial-only maps without explicit SCALE clauses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Create pos1/pos2 scales in resolve_scale_types_and_transforms when a map projection is active but no position scales exist (spatial layers only map "geometry"). Also fix .any() → .all() in numeric_range validation and strengthen the integration test to assert scale existence. Co-Authored-By: Claude Opus 4.6 --- src/execute/scale.rs | 11 +++++++++++ src/lib.rs | 34 ++++++++++++++++++++------------ src/plot/projection/coord/map.rs | 2 +- 3 files changed, 33 insertions(+), 14 deletions(-) diff --git a/src/execute/scale.rs b/src/execute/scale.rs index 628c108a..1bb86710 100644 --- a/src/execute/scale.rs +++ b/src/execute/scale.rs @@ -334,6 +334,17 @@ pub fn resolve_scale_types_and_transforms( .as_ref() .is_some_and(|p| p.coord.coord_kind() == CoordKind::Map); + if is_map { + for aes in ["pos1", "pos2"] { + if !spec.scales.iter().any(|s| s.aesthetic == aes) { + let mut scale = Scale::new(aes); + scale.scale_type = Some(ScaleType::continuous()); + scale.transform = Some(Transform::geographic()); + spec.scales.push(scale); + } + } + } + for scale in &mut spec.scales { // Skip scales that already have explicit types (user specified) if let Some(scale_type) = &scale.scale_type { diff --git a/src/lib.rs b/src/lib.rs index 2efd0ca4..17cabcef 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1427,19 +1427,27 @@ mod integration_tests { let prepared = execute::prepare_data_with_reader(query, &reader).unwrap(); let spec = &prepared.specs[0]; - for scale in &spec.scales { - if scale.aesthetic == "pos1" || scale.aesthetic == "pos2" { - assert!( - scale.resolved, - "Map position scale '{}' should be marked resolved", - scale.aesthetic - ); - assert!( - !scale.numeric_breaks().is_empty(), - "Map position scale '{}' should have breaks", - scale.aesthetic - ); - } + let pos_scales: Vec<_> = spec + .scales + .iter() + .filter(|s| s.aesthetic == "pos1" || s.aesthetic == "pos2") + .collect(); + assert_eq!( + pos_scales.len(), + 2, + "Map projection should create pos1 and pos2 scales even without explicit SCALE clauses" + ); + for scale in pos_scales { + assert!( + scale.resolved, + "Map position scale '{}' should be marked resolved", + scale.aesthetic + ); + assert!( + !scale.numeric_breaks().is_empty(), + "Map position scale '{}' should have breaks", + scale.aesthetic + ); } } } diff --git a/src/plot/projection/coord/map.rs b/src/plot/projection/coord/map.rs index df0549fd..f04c0d82 100644 --- a/src/plot/projection/coord/map.rs +++ b/src/plot/projection/coord/map.rs @@ -168,7 +168,7 @@ fn scale_override_bbox( .find(|s| s.aesthetic == aesthetic && s.explicit_input_range)? .input_range .as_deref()?; - if range.len() == 2 && range.iter().any(|e| e.to_f64().is_some()) { + if range.len() == 2 && range.iter().all(|e| e.to_f64().is_some()) { Some(range) } else { None