From c8d39051e3debc70307e7d16aaed07e1a42385b1 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Tue, 9 Jun 2026 15:57:52 +0200 Subject: [PATCH 01/20] Add `partition_by` parameter to `GeomTrait::apply_projection` Extend the signature so geoms that need group-aware densification (line, path, polygon) will have access to the layer's grouping columns at projection time. Co-Authored-By: Claude Opus 4.6 --- src/plot/layer/geom/mod.rs | 7 +++++-- src/plot/layer/geom/point.rs | 1 + src/plot/layer/geom/spatial.rs | 1 + src/plot/layer/geom/text.rs | 1 + src/plot/projection/coord/map.rs | 1 + src/plot/projection/coord/mod.rs | 1 + 6 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/plot/layer/geom/mod.rs b/src/plot/layer/geom/mod.rs index 7b510a1d4..f3841183b 100644 --- a/src/plot/layer/geom/mod.rs +++ b/src/plot/layer/geom/mod.rs @@ -296,10 +296,11 @@ pub trait GeomTrait: std::fmt::Debug + std::fmt::Display + Send + Sync { /// Called after stat transforms, before data fetch. Each geom decides what /// projection means for its parameterization: /// - Spatial: ST_AsWKB (always), plus ST_Transform when Map coord has a CRS - /// - Future geoms: rectangles transform corners, lines segmentize, etc. + /// - Line/path/polygon: densify segments before ST_Transform /// /// `columns` lists all column names in the query (for portable column /// replacement on backends that don't support `SELECT * REPLACE`). + /// `partition_by` lists the grouping columns for the layer. /// /// The default is a no-op (returns query unchanged). fn apply_projection( @@ -309,6 +310,7 @@ pub trait GeomTrait: std::fmt::Debug + std::fmt::Display + Send + Sync { _dialect: &dyn SqlDialect, _clip: bool, _columns: &[String], + _partition_by: &[String], ) -> Result { Ok(query.to_string()) } @@ -645,9 +647,10 @@ impl Geom { dialect: &dyn SqlDialect, clip: bool, columns: &[String], + partition_by: &[String], ) -> Result { self.0 - .apply_projection(query, projection, dialect, clip, columns) + .apply_projection(query, projection, dialect, clip, columns, partition_by) } /// Adjust layer mappings and parameters based on geom-specific logic diff --git a/src/plot/layer/geom/point.rs b/src/plot/layer/geom/point.rs index 00850aa95..51d40ceea 100644 --- a/src/plot/layer/geom/point.rs +++ b/src/plot/layer/geom/point.rs @@ -62,6 +62,7 @@ impl GeomTrait for Point { dialect: &dyn SqlDialect, _clip: bool, columns: &[String], + _partition_by: &[String], ) -> Result { project_position_columns(query, projection, dialect, columns) } diff --git a/src/plot/layer/geom/spatial.rs b/src/plot/layer/geom/spatial.rs index f49badc5f..6e8d1f8bc 100644 --- a/src/plot/layer/geom/spatial.rs +++ b/src/plot/layer/geom/spatial.rs @@ -78,6 +78,7 @@ impl GeomTrait for Spatial { dialect: &dyn SqlDialect, clip: bool, columns: &[String], + _partition_by: &[String], ) -> crate::Result { let col = naming::quote_ident(&naming::aesthetic_column("geometry")); let is_map = projection.coord.coord_kind() == CoordKind::Map; diff --git a/src/plot/layer/geom/text.rs b/src/plot/layer/geom/text.rs index 5bc3f959f..be8a56714 100644 --- a/src/plot/layer/geom/text.rs +++ b/src/plot/layer/geom/text.rs @@ -77,6 +77,7 @@ impl GeomTrait for Text { dialect: &dyn SqlDialect, _clip: bool, columns: &[String], + _partition_by: &[String], ) -> Result { project_position_columns(query, projection, dialect, columns) } diff --git a/src/plot/projection/coord/map.rs b/src/plot/projection/coord/map.rs index d27b24e37..3e311fd6a 100644 --- a/src/plot/projection/coord/map.rs +++ b/src/plot/projection/coord/map.rs @@ -70,6 +70,7 @@ pub(crate) fn apply_map_transforms( dialect, clip, &columns, + &layer.partition_by, )?; } diff --git a/src/plot/projection/coord/mod.rs b/src/plot/projection/coord/mod.rs index 66d80cff9..09ec866e3 100644 --- a/src/plot/projection/coord/mod.rs +++ b/src/plot/projection/coord/mod.rs @@ -163,6 +163,7 @@ pub trait CoordTrait: std::fmt::Debug + Send + Sync { dialect, false, &columns, + &layer.partition_by, )?; } Ok(()) From 504d9a3c7d07e82689d91dd9c695536eccb82327 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Tue, 9 Jun 2026 16:23:43 +0200 Subject: [PATCH 02/20] --- src/plot/layer/geom/mod.rs | 171 +++++++++++++++++++++++++++++++++ src/plot/layer/geom/spatial.rs | 12 +-- 2 files changed, 177 insertions(+), 6 deletions(-) diff --git a/src/plot/layer/geom/mod.rs b/src/plot/layer/geom/mod.rs index f3841183b..3b5f7c8f0 100644 --- a/src/plot/layer/geom/mod.rs +++ b/src/plot/layer/geom/mod.rs @@ -407,6 +407,177 @@ pub(crate) fn project_position_columns( )) } +/// Subdivide edges in a tabular dataset by linear interpolation. +/// +/// Inserts intermediate vertices along edges longer than `max_segment`. +/// Continuous aesthetics (columns that are neither positions nor in `partition_by`) +/// are interpolated too. Discrete (partition) columns are carried through unchanged. +/// +/// - `domain_order`: aesthetic name for the ordering column (e.g. "pos1" for line). +/// When `None`, a synthetic row index is used (path/polygon). +/// - `close_ring`: when true, the last vertex connects back to the first (polygon). +/// - `segment_length`: target edge length after subdivision (in position units). +/// - `n_segments`: size of the integer series (must be at least as large as the +/// maximum number of subdivisions any single edge can produce). +pub(crate) fn densify_edges( + query: &str, + dialect: &dyn SqlDialect, + columns: &[String], + partition_by: &[String], + domain_order: Option<&str>, + close_ring: bool, + segment_length: f64, + n_segments: usize, +) -> String { + let pos1 = naming::quote_ident(&naming::aesthetic_column("pos1")); + let pos2 = naming::quote_ident(&naming::aesthetic_column("pos2")); + + // Continuous aesthetics to interpolate: columns - partition_by - positions + let pos1_col = naming::aesthetic_column("pos1"); + let pos2_col = naming::aesthetic_column("pos2"); + let continuous_cols: Vec<&String> = columns + .iter() + .filter(|c| *c != &pos1_col && *c != &pos2_col && !partition_by.contains(c)) + .collect(); + + // Ordering column + let order_col = match domain_order { + Some(aes) => naming::quote_ident(&naming::aesthetic_column(aes)), + None => "\"__ggsql_edge_idx__\"".to_string(), + }; + + // PARTITION BY clause for window functions + let partition_clause = if partition_by.is_empty() { + String::new() + } else { + let parts: Vec = partition_by.iter().map(|c| naming::quote_ident(c)).collect(); + format!("PARTITION BY {}", parts.join(", ")) + }; + + let window_def = if partition_clause.is_empty() { + format!("ORDER BY {order_col}") + } else { + format!("{partition_clause} ORDER BY {order_col}") + }; + + let seq_cte = dialect.sql_generate_series(n_segments); + + // Synthesize row ordering for path/polygon + let indexed_query = if domain_order.is_none() { + format!( + "SELECT *, ROW_NUMBER() OVER ({partition_clause} ORDER BY (SELECT NULL)) \ + AS \"__ggsql_edge_idx__\" FROM ({query})" + ) + } else { + query.to_string() + }; + + // LEAD expressions for positions — polygon closes the ring via FIRST_VALUE fallback + let pos1_lead = if close_ring { + format!( + "COALESCE(LEAD({pos1}) OVER w, FIRST_VALUE({pos1}) OVER w) AS \"__ggsql_next_pos1__\"" + ) + } else { + format!("LEAD({pos1}) OVER w AS \"__ggsql_next_pos1__\"") + }; + let pos2_lead = if close_ring { + format!( + "COALESCE(LEAD({pos2}) OVER w, FIRST_VALUE({pos2}) OVER w) AS \"__ggsql_next_pos2__\"" + ) + } else { + format!("LEAD({pos2}) OVER w AS \"__ggsql_next_pos2__\"") + }; + + // LEAD expressions for continuous aesthetics + let mut cont_leads = String::new(); + for c in &continuous_cols { + let qc = naming::quote_ident(c); + let alias = format!("\"__ggsql_next_{}\"", c.replace('"', "")); + if close_ring { + cont_leads.push_str(&format!( + ", COALESCE(LEAD({qc}) OVER w, FIRST_VALUE({qc}) OVER w) AS {alias}" + )); + } else { + cont_leads.push_str(&format!(", LEAD({qc}) OVER w AS {alias}")); + } + } + + // Segment length (Euclidean in source CRS units) + let seg_len = format!( + "SQRT(POWER(\"__ggsql_next_pos1__\" - {pos1}, 2) + \ + POWER(\"__ggsql_next_pos2__\" - {pos2}, 2))" + ); + + // Edges CTE: original rows + LEAD columns + segment length + let edges_query = format!( + "SELECT *, {pos1_lead}, {pos2_lead}{cont_leads}, \ + {seg_len} AS \"__ggsql_seg_len__\" \ + FROM ({indexed_query}) \"__ggsql_src__\" \ + WINDOW w AS ({window_def})" + ); + + // Interpolation: n / CEIL(seg_len / threshold) gives fraction [0, 1) + let threshold_lit = format!("{:.6}", segment_length); + let n_subdivs = format!("CEIL(\"__ggsql_seg_len__\" / {threshold_lit})"); + + // SELECT list + let mut select_parts: Vec = Vec::new(); + + // Discrete columns — unchanged + for c in partition_by { + select_parts.push(naming::quote_ident(c)); + } + + // Position columns — interpolated + select_parts.push(format!( + "{pos1} + (\"__ggsql_next_pos1__\" - {pos1}) * \ + (CAST(\"__ggsql_seq__\".n AS REAL) / {n_subdivs}) AS {pos1}" + )); + select_parts.push(format!( + "{pos2} + (\"__ggsql_next_pos2__\" - {pos2}) * \ + (CAST(\"__ggsql_seq__\".n AS REAL) / {n_subdivs}) AS {pos2}" + )); + + // Continuous aesthetics — interpolated + for c in &continuous_cols { + let qc = naming::quote_ident(c); + let next = format!("\"__ggsql_next_{}\"", c.replace('"', "")); + select_parts.push(format!( + "{qc} + ({next} - {qc}) * \ + (CAST(\"__ggsql_seq__\".n AS REAL) / {n_subdivs}) AS {qc}" + )); + } + + // WHERE: emit n < subdivisions per segment; for open geoms, keep last vertex + let where_clause = if close_ring { + format!("\"__ggsql_seq__\".n < {n_subdivs}") + } else { + format!( + "(\"__ggsql_next_pos1__\" IS NOT NULL AND \"__ggsql_seq__\".n < {n_subdivs}) \ + OR (\"__ggsql_next_pos1__\" IS NULL AND \"__ggsql_seq__\".n = 0)" + ) + }; + + // ORDER BY + let order_parts = if partition_by.is_empty() { + format!("{order_col}, \"__ggsql_seq__\".n") + } else { + let parts: Vec = partition_by.iter().map(|c| naming::quote_ident(c)).collect(); + format!("{}, {order_col}, \"__ggsql_seq__\".n", parts.join(", ")) + }; + + format!( + "WITH {seq_cte}, \ + \"__ggsql_edges__\" AS ({edges_query}) \ + SELECT {select} \ + FROM \"__ggsql_edges__\" \ + CROSS JOIN \"__ggsql_seq__\" \ + WHERE {where_clause} \ + ORDER BY {order_parts}", + select = select_parts.join(", "), + ) +} + /// True when `parameters["aggregate"]` is set to a non-null string or array. pub(crate) fn has_aggregate_param(parameters: &HashMap) -> bool { matches!( diff --git a/src/plot/layer/geom/spatial.rs b/src/plot/layer/geom/spatial.rs index 6e8d1f8bc..eb294c4e8 100644 --- a/src/plot/layer/geom/spatial.rs +++ b/src/plot/layer/geom/spatial.rs @@ -138,7 +138,7 @@ mod tests { let spatial = Spatial; let projection = Projection::cartesian(); let result = spatial - .apply_projection("SELECT * FROM t", &projection, &AnsiDialect, false, &[]) + .apply_projection("SELECT * FROM t", &projection, &AnsiDialect, false, &[], &[]) .unwrap(); assert!(result.contains("ST_AsBinary")); @@ -150,7 +150,7 @@ mod tests { let spatial = Spatial; let projection = Projection::map(); let result = spatial - .apply_projection("SELECT * FROM t", &projection, &AnsiDialect, false, &[]) + .apply_projection("SELECT * FROM t", &projection, &AnsiDialect, false, &[], &[]) .unwrap(); // Map without CRS passes through (ensure_geometry is identity for AnsiDialect) @@ -167,7 +167,7 @@ mod tests { ParameterValue::String("+proj=merc".to_string()), ); let result = spatial - .apply_projection("SELECT * FROM t", &projection, &AnsiDialect, false, &[]) + .apply_projection("SELECT * FROM t", &projection, &AnsiDialect, false, &[], &[]) .unwrap(); // Without clip=true, just ST_Transform @@ -186,7 +186,7 @@ mod tests { ParameterValue::String("+proj=merc".to_string()), ); let result = spatial - .apply_projection("SELECT * FROM t", &projection, &AnsiDialect, true, &[]) + .apply_projection("SELECT * FROM t", &projection, &AnsiDialect, true, &[], &[]) .unwrap(); assert!(result.contains("ST_Intersection")); @@ -203,7 +203,7 @@ mod tests { ParameterValue::String("+proj=ortho +lat_0=45 +lon_0=10".to_string()), ); let result = spatial - .apply_projection("SELECT * FROM t", &projection, &AnsiDialect, true, &[]) + .apply_projection("SELECT * FROM t", &projection, &AnsiDialect, true, &[], &[]) .unwrap(); assert!(result.contains("ST_Transform")); @@ -222,7 +222,7 @@ mod tests { ParameterValue::String("+proj=gnom +lat_0=90 +lon_0=0".to_string()), ); let result = spatial - .apply_projection("SELECT * FROM t", &projection, &AnsiDialect, true, &[]) + .apply_projection("SELECT * FROM t", &projection, &AnsiDialect, true, &[], &[]) .unwrap(); assert!(result.contains("ST_MakeValid")); From 7324de4db5d3bf2ab32cfab7db176c5bf1d6cfaa Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Tue, 9 Jun 2026 16:33:29 +0200 Subject: [PATCH 03/20] Implement edge densification for line, path, and polygon geoms When a map projection requires CRS transformation, these geoms now subdivide long edges before projecting. This produces smooth curves in projected space instead of straight-line segments between vertices. Adds `needs_projection()` to centralize the guard logic shared by all three geoms. Co-Authored-By: Claude Opus 4.6 --- src/plot/layer/geom/line.rs | 25 ++++++++++++++++++++++--- src/plot/layer/geom/mod.rs | 20 ++++++++++++++++++++ src/plot/layer/geom/path.rs | 22 +++++++++++++++++++++- src/plot/layer/geom/polygon.rs | 22 +++++++++++++++++++++- 4 files changed, 84 insertions(+), 5 deletions(-) diff --git a/src/plot/layer/geom/line.rs b/src/plot/layer/geom/line.rs index b40b975f5..a4183d799 100644 --- a/src/plot/layer/geom/line.rs +++ b/src/plot/layer/geom/line.rs @@ -3,12 +3,15 @@ use super::stat_aggregate; use super::types::wrap_with_order_by; use super::{ - has_aggregate_param, DefaultAesthetics, DefaultParamValue, GeomTrait, GeomType, - ParamConstraint, ParamDefinition, StatResult, + densify_edges, has_aggregate_param, needs_projection, project_position_columns, + DefaultAesthetics, DefaultParamValue, GeomTrait, GeomType, ParamConstraint, ParamDefinition, + StatResult, }; use crate::plot::layer::orientation::{ALIGNED, ORIENTATION_VALUES}; +use crate::plot::projection::Projection; use crate::plot::types::DefaultAestheticValue; -use crate::Mappings; +use crate::reader::SqlDialect; +use crate::{Mappings, Result}; /// Line geom - line charts with connected points #[derive(Debug, Clone, Copy)] @@ -77,6 +80,22 @@ impl GeomTrait for Line { // the Identity and Aggregate paths. Ok(wrap_with_order_by(query, result, "pos1")) } + + fn apply_projection( + &self, + query: &str, + projection: &Projection, + dialect: &dyn SqlDialect, + _clip: bool, + columns: &[String], + partition_by: &[String], + ) -> Result { + if !needs_projection(projection) { + return Ok(query.to_string()); + } + let densified = densify_edges(query, dialect, columns, partition_by, Some("pos1"), false, 1.0, 360); + project_position_columns(&densified, projection, dialect, columns) + } } impl std::fmt::Display for Line { diff --git a/src/plot/layer/geom/mod.rs b/src/plot/layer/geom/mod.rs index 3b5f7c8f0..78f57801f 100644 --- a/src/plot/layer/geom/mod.rs +++ b/src/plot/layer/geom/mod.rs @@ -407,6 +407,26 @@ pub(crate) fn project_position_columns( )) } +/// Returns true when the projection requires position transformation (Map coord +/// with distinct source and target CRS). Used to guard densification and +/// `project_position_columns`. +pub(crate) fn needs_projection(projection: &Projection) -> bool { + use crate::plot::projection::coord::CoordKind; + + if projection.coord.coord_kind() != CoordKind::Map { + return false; + } + let target = match projection.properties.get("target") { + Some(ParameterValue::String(s)) => s.as_str(), + _ => return false, + }; + let source = match projection.properties.get("source") { + Some(ParameterValue::String(s)) => s.as_str(), + _ => return false, + }; + source != target +} + /// Subdivide edges in a tabular dataset by linear interpolation. /// /// Inserts intermediate vertices along edges longer than `max_segment`. diff --git a/src/plot/layer/geom/path.rs b/src/plot/layer/geom/path.rs index 5e32a3be0..6687c61a6 100644 --- a/src/plot/layer/geom/path.rs +++ b/src/plot/layer/geom/path.rs @@ -2,9 +2,13 @@ use super::types::POSITION_VALUES; use super::{ - DefaultAesthetics, DefaultParamValue, GeomTrait, GeomType, ParamConstraint, ParamDefinition, + densify_edges, needs_projection, project_position_columns, DefaultAesthetics, + DefaultParamValue, GeomTrait, GeomType, ParamConstraint, ParamDefinition, }; +use crate::plot::projection::Projection; use crate::plot::types::DefaultAestheticValue; +use crate::reader::SqlDialect; +use crate::Result; /// Path geom - connected line segments in order #[derive(Debug, Clone, Copy)] @@ -36,6 +40,22 @@ impl GeomTrait for Path { }]; PARAMS } + + fn apply_projection( + &self, + query: &str, + projection: &Projection, + dialect: &dyn SqlDialect, + _clip: bool, + columns: &[String], + partition_by: &[String], + ) -> Result { + if !needs_projection(projection) { + return Ok(query.to_string()); + } + let densified = densify_edges(query, dialect, columns, partition_by, None, false, 1.0, 360); + project_position_columns(&densified, projection, dialect, columns) + } } impl std::fmt::Display for Path { diff --git a/src/plot/layer/geom/polygon.rs b/src/plot/layer/geom/polygon.rs index d1ed6841c..29f23af75 100644 --- a/src/plot/layer/geom/polygon.rs +++ b/src/plot/layer/geom/polygon.rs @@ -2,9 +2,13 @@ use super::types::POSITION_VALUES; use super::{ - DefaultAesthetics, DefaultParamValue, GeomTrait, GeomType, ParamConstraint, ParamDefinition, + densify_edges, needs_projection, project_position_columns, DefaultAesthetics, + DefaultParamValue, GeomTrait, GeomType, ParamConstraint, ParamDefinition, }; +use crate::plot::projection::Projection; use crate::plot::types::DefaultAestheticValue; +use crate::reader::SqlDialect; +use crate::Result; /// Polygon geom - arbitrary polygons #[derive(Debug, Clone, Copy)] @@ -37,6 +41,22 @@ impl GeomTrait for Polygon { }]; PARAMS } + + fn apply_projection( + &self, + query: &str, + projection: &Projection, + dialect: &dyn SqlDialect, + _clip: bool, + columns: &[String], + partition_by: &[String], + ) -> Result { + if !needs_projection(projection) { + return Ok(query.to_string()); + } + let densified = densify_edges(query, dialect, columns, partition_by, None, true, 1.0, 360); + project_position_columns(&densified, projection, dialect, columns) + } } impl std::fmt::Display for Polygon { From a77fcdb83621034e1a44029720590ec042903822 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Tue, 9 Jun 2026 16:38:27 +0200 Subject: [PATCH 04/20] Add tests for needs_projection, densify_edges, and line apply_projection Co-Authored-By: Claude Opus 4.6 --- src/plot/layer/geom/line.rs | 36 ++++++++ src/plot/layer/geom/mod.rs | 175 ++++++++++++++++++++++++++++++++++++ 2 files changed, 211 insertions(+) diff --git a/src/plot/layer/geom/line.rs b/src/plot/layer/geom/line.rs index a4183d799..2692d3d18 100644 --- a/src/plot/layer/geom/line.rs +++ b/src/plot/layer/geom/line.rs @@ -103,3 +103,39 @@ impl std::fmt::Display for Line { write!(f, "line") } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::naming; + use crate::plot::types::ParameterValue; + use crate::reader::AnsiDialect; + + #[test] + fn test_apply_projection_densifies_and_transforms() { + let line = Line; + let mut projection = Projection::map(); + projection.properties.insert( + "source".to_string(), + ParameterValue::String("EPSG:4326".to_string()), + ); + projection.properties.insert( + "target".to_string(), + ParameterValue::String("+proj=ortho +lat_0=0 +lon_0=0".to_string()), + ); + + let columns = vec![ + naming::aesthetic_column("pos1"), + naming::aesthetic_column("pos2"), + ]; + let result = line + .apply_projection("SELECT * FROM t", &projection, &AnsiDialect, false, &columns, &[]) + .unwrap(); + + // Densification happened + assert!(result.contains("__ggsql_seq__")); + assert!(result.contains("LEAD(")); + // Projection happened + assert!(result.contains("ST_Transform")); + } +} diff --git a/src/plot/layer/geom/mod.rs b/src/plot/layer/geom/mod.rs index 78f57801f..71067657b 100644 --- a/src/plot/layer/geom/mod.rs +++ b/src/plot/layer/geom/mod.rs @@ -1051,4 +1051,179 @@ mod tests { } } } + + #[test] + fn test_needs_projection_false_for_cartesian() { + let projection = Projection::cartesian(); + assert!(!needs_projection(&projection)); + } + + #[test] + fn test_needs_projection_false_without_target() { + let projection = Projection::map(); + assert!(!needs_projection(&projection)); + } + + #[test] + fn test_needs_projection_false_without_source() { + let mut projection = Projection::map(); + projection.properties.insert( + "target".to_string(), + ParameterValue::String("+proj=ortho".to_string()), + ); + assert!(!needs_projection(&projection)); + } + + #[test] + fn test_needs_projection_false_when_same_crs() { + let mut projection = Projection::map(); + projection.properties.insert( + "source".to_string(), + ParameterValue::String("EPSG:4326".to_string()), + ); + projection.properties.insert( + "target".to_string(), + ParameterValue::String("EPSG:4326".to_string()), + ); + assert!(!needs_projection(&projection)); + } + + #[test] + fn test_needs_projection_true_when_different_crs() { + let mut projection = Projection::map(); + projection.properties.insert( + "source".to_string(), + ParameterValue::String("EPSG:4326".to_string()), + ); + projection.properties.insert( + "target".to_string(), + ParameterValue::String("+proj=ortho".to_string()), + ); + assert!(needs_projection(&projection)); + } + + #[test] + fn test_densify_edges_basic_structure() { + use crate::reader::AnsiDialect; + + let columns = vec![ + naming::aesthetic_column("pos1"), + naming::aesthetic_column("pos2"), + ]; + let result = densify_edges( + "SELECT * FROM t", + &AnsiDialect, + &columns, + &[], + Some("pos1"), + false, + 1.0, + 360, + ); + + assert!(result.contains("__ggsql_seq__")); + assert!(result.contains("LEAD(")); + assert!(result.contains("__ggsql_seg_len__")); + assert!(result.contains("__ggsql_next_pos1__")); + assert!(result.contains("__ggsql_next_pos2__")); + } + + #[test] + fn test_densify_edges_with_partition() { + use crate::reader::AnsiDialect; + + let columns = vec![ + naming::aesthetic_column("pos1"), + naming::aesthetic_column("pos2"), + naming::aesthetic_column("stroke"), + ]; + let partition_by = vec![naming::aesthetic_column("stroke")]; + let result = densify_edges( + "SELECT * FROM t", + &AnsiDialect, + &columns, + &partition_by, + Some("pos1"), + false, + 1.0, + 360, + ); + + assert!(result.contains("PARTITION BY")); + assert!(result.contains("__ggsql_aes_stroke__")); + } + + #[test] + fn test_densify_edges_interpolates_continuous_aesthetics() { + use crate::reader::AnsiDialect; + + let columns = vec![ + naming::aesthetic_column("pos1"), + naming::aesthetic_column("pos2"), + naming::aesthetic_column("stroke"), + naming::aesthetic_column("opacity"), + ]; + let partition_by = vec![naming::aesthetic_column("stroke")]; + let result = densify_edges( + "SELECT * FROM t", + &AnsiDialect, + &columns, + &partition_by, + Some("pos1"), + false, + 1.0, + 360, + ); + + // opacity is continuous (not in partition_by, not a position) — should be interpolated + assert!(result.contains("__ggsql_next___ggsql_aes_opacity__")); + } + + #[test] + fn test_densify_edges_close_ring() { + use crate::reader::AnsiDialect; + + let columns = vec![ + naming::aesthetic_column("pos1"), + naming::aesthetic_column("pos2"), + ]; + let result = densify_edges( + "SELECT * FROM t", + &AnsiDialect, + &columns, + &[], + None, + true, + 1.0, + 360, + ); + + // Closed ring uses COALESCE(LEAD(...), FIRST_VALUE(...)) + assert!(result.contains("FIRST_VALUE(")); + // Uses synthetic row index + assert!(result.contains("__ggsql_edge_idx__")); + } + + #[test] + fn test_densify_edges_open_keeps_last_vertex() { + use crate::reader::AnsiDialect; + + let columns = vec![ + naming::aesthetic_column("pos1"), + naming::aesthetic_column("pos2"), + ]; + let result = densify_edges( + "SELECT * FROM t", + &AnsiDialect, + &columns, + &[], + Some("pos1"), + false, + 1.0, + 360, + ); + + // Open geom: WHERE clause keeps last vertex via IS NULL check + assert!(result.contains("IS NULL AND")); + } } From fdff1426d92604397b9ed62f1b445eec6d9d91f5 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Tue, 9 Jun 2026 16:39:41 +0200 Subject: [PATCH 05/20] Run cargo fmt and suppress clippy too_many_arguments Co-Authored-By: Claude Opus 4.6 --- src/plot/layer/geom/line.rs | 20 ++++++++++++++++++-- src/plot/layer/geom/mod.rs | 11 +++++++++-- src/plot/layer/geom/spatial.rs | 27 ++++++++++++++++++++++++--- 3 files changed, 51 insertions(+), 7 deletions(-) diff --git a/src/plot/layer/geom/line.rs b/src/plot/layer/geom/line.rs index 2692d3d18..92c23e818 100644 --- a/src/plot/layer/geom/line.rs +++ b/src/plot/layer/geom/line.rs @@ -93,7 +93,16 @@ impl GeomTrait for Line { if !needs_projection(projection) { return Ok(query.to_string()); } - let densified = densify_edges(query, dialect, columns, partition_by, Some("pos1"), false, 1.0, 360); + let densified = densify_edges( + query, + dialect, + columns, + partition_by, + Some("pos1"), + false, + 1.0, + 360, + ); project_position_columns(&densified, projection, dialect, columns) } } @@ -129,7 +138,14 @@ mod tests { naming::aesthetic_column("pos2"), ]; let result = line - .apply_projection("SELECT * FROM t", &projection, &AnsiDialect, false, &columns, &[]) + .apply_projection( + "SELECT * FROM t", + &projection, + &AnsiDialect, + false, + &columns, + &[], + ) .unwrap(); // Densification happened diff --git a/src/plot/layer/geom/mod.rs b/src/plot/layer/geom/mod.rs index 71067657b..ab1712332 100644 --- a/src/plot/layer/geom/mod.rs +++ b/src/plot/layer/geom/mod.rs @@ -439,6 +439,7 @@ pub(crate) fn needs_projection(projection: &Projection) -> bool { /// - `segment_length`: target edge length after subdivision (in position units). /// - `n_segments`: size of the integer series (must be at least as large as the /// maximum number of subdivisions any single edge can produce). +#[allow(clippy::too_many_arguments)] pub(crate) fn densify_edges( query: &str, dialect: &dyn SqlDialect, @@ -470,7 +471,10 @@ pub(crate) fn densify_edges( let partition_clause = if partition_by.is_empty() { String::new() } else { - let parts: Vec = partition_by.iter().map(|c| naming::quote_ident(c)).collect(); + let parts: Vec = partition_by + .iter() + .map(|c| naming::quote_ident(c)) + .collect(); format!("PARTITION BY {}", parts.join(", ")) }; @@ -582,7 +586,10 @@ pub(crate) fn densify_edges( let order_parts = if partition_by.is_empty() { format!("{order_col}, \"__ggsql_seq__\".n") } else { - let parts: Vec = partition_by.iter().map(|c| naming::quote_ident(c)).collect(); + let parts: Vec = partition_by + .iter() + .map(|c| naming::quote_ident(c)) + .collect(); format!("{}, {order_col}, \"__ggsql_seq__\".n", parts.join(", ")) }; diff --git a/src/plot/layer/geom/spatial.rs b/src/plot/layer/geom/spatial.rs index eb294c4e8..82366f0ad 100644 --- a/src/plot/layer/geom/spatial.rs +++ b/src/plot/layer/geom/spatial.rs @@ -138,7 +138,14 @@ mod tests { let spatial = Spatial; let projection = Projection::cartesian(); let result = spatial - .apply_projection("SELECT * FROM t", &projection, &AnsiDialect, false, &[], &[]) + .apply_projection( + "SELECT * FROM t", + &projection, + &AnsiDialect, + false, + &[], + &[], + ) .unwrap(); assert!(result.contains("ST_AsBinary")); @@ -150,7 +157,14 @@ mod tests { let spatial = Spatial; let projection = Projection::map(); let result = spatial - .apply_projection("SELECT * FROM t", &projection, &AnsiDialect, false, &[], &[]) + .apply_projection( + "SELECT * FROM t", + &projection, + &AnsiDialect, + false, + &[], + &[], + ) .unwrap(); // Map without CRS passes through (ensure_geometry is identity for AnsiDialect) @@ -167,7 +181,14 @@ mod tests { ParameterValue::String("+proj=merc".to_string()), ); let result = spatial - .apply_projection("SELECT * FROM t", &projection, &AnsiDialect, false, &[], &[]) + .apply_projection( + "SELECT * FROM t", + &projection, + &AnsiDialect, + false, + &[], + &[], + ) .unwrap(); // Without clip=true, just ST_Transform From 401ffaac22b0f52195afd256e8625c9a85e22148 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Tue, 9 Jun 2026 17:00:54 +0200 Subject: [PATCH 06/20] Fix NULL coordinates for last vertex in open densified paths Wrap the entire interpolation delta in COALESCE so that when the next vertex is NULL (last point in an open line/path), the expression collapses to the original position instead of propagating NULL. Co-Authored-By: Claude Opus 4.6 --- src/plot/layer/geom/mod.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/plot/layer/geom/mod.rs b/src/plot/layer/geom/mod.rs index ab1712332..623d06bce 100644 --- a/src/plot/layer/geom/mod.rs +++ b/src/plot/layer/geom/mod.rs @@ -552,14 +552,15 @@ pub(crate) fn densify_edges( select_parts.push(naming::quote_ident(c)); } - // Position columns — interpolated + // Interpolation fraction + let frac = format!("CAST(\"__ggsql_seq__\".n AS REAL) / {n_subdivs}"); + + // Position columns — interpolated; COALESCE handles the last vertex (NULL next) select_parts.push(format!( - "{pos1} + (\"__ggsql_next_pos1__\" - {pos1}) * \ - (CAST(\"__ggsql_seq__\".n AS REAL) / {n_subdivs}) AS {pos1}" + "{pos1} + COALESCE((\"__ggsql_next_pos1__\" - {pos1}) * ({frac}), 0.0) AS {pos1}" )); select_parts.push(format!( - "{pos2} + (\"__ggsql_next_pos2__\" - {pos2}) * \ - (CAST(\"__ggsql_seq__\".n AS REAL) / {n_subdivs}) AS {pos2}" + "{pos2} + COALESCE((\"__ggsql_next_pos2__\" - {pos2}) * ({frac}), 0.0) AS {pos2}" )); // Continuous aesthetics — interpolated @@ -567,8 +568,7 @@ pub(crate) fn densify_edges( let qc = naming::quote_ident(c); let next = format!("\"__ggsql_next_{}\"", c.replace('"', "")); select_parts.push(format!( - "{qc} + ({next} - {qc}) * \ - (CAST(\"__ggsql_seq__\".n AS REAL) / {n_subdivs}) AS {qc}" + "{qc} + COALESCE(({next} - {qc}) * ({frac}), 0.0) AS {qc}" )); } From 4d94e467376e60cab915535962df3b3b5083788a Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Wed, 10 Jun 2026 16:10:19 +0200 Subject: [PATCH 07/20] Implement edge densification for tile geom under map projection Continuous tiles (pos1min/pos1max/pos2min/pos2max) are expanded to polygon vertices, densified, and projected so rectangle edges curve correctly on map projections. The tile geom mutates its mappings after polygonization to reflect the new pos1/pos2 column structure, keeping the logic self-contained and eliminating redundant SQL wrappers. Co-Authored-By: Claude Opus 4.6 --- src/execute/mod.rs | 2 +- src/plot/layer/geom/line.rs | 37 +- src/plot/layer/geom/mod.rs | 36 +- src/plot/layer/geom/path.rs | 24 +- src/plot/layer/geom/point.rs | 13 +- src/plot/layer/geom/polygon.rs | 15 +- src/plot/layer/geom/spatial.rs | 56 ++- src/plot/layer/geom/text.rs | 13 +- src/plot/layer/geom/tile.rs | 373 ++++++++++++++++++- src/plot/projection/coord/map.rs | 14 +- src/plot/projection/coord/map_projections.rs | 2 +- src/plot/projection/coord/mod.rs | 16 +- src/plot/projection/types.rs | 2 +- src/writer/vegalite/layer.rs | 36 +- 14 files changed, 548 insertions(+), 91 deletions(-) diff --git a/src/execute/mod.rs b/src/execute/mod.rs index d18f4d0bb..66774ad30 100644 --- a/src/execute/mod.rs +++ b/src/execute/mod.rs @@ -1384,7 +1384,7 @@ pub fn prepare_data_with_reader(query: &str, reader: &dyn Reader) -> Result, ) -> Result { if !needs_projection(projection) { return Ok(query.to_string()); } + let columns: Vec = mappings + .aesthetics + .keys() + .map(|k| naming::aesthetic_column(k)) + .collect(); + let pos1_col = naming::aesthetic_column("pos1"); let densified = densify_edges( query, dialect, - columns, + &columns, partition_by, - Some("pos1"), + Some(&pos1_col), false, 1.0, 360, ); - project_position_columns(&densified, projection, dialect, columns) + project_position_columns(&densified, projection, dialect, &columns) } } @@ -122,6 +128,8 @@ mod tests { #[test] fn test_apply_projection_densifies_and_transforms() { + use crate::plot::types::AestheticValue; + let line = Line; let mut projection = Projection::map(); projection.properties.insert( @@ -133,18 +141,23 @@ mod tests { ParameterValue::String("+proj=ortho +lat_0=0 +lon_0=0".to_string()), ); - let columns = vec![ - naming::aesthetic_column("pos1"), - naming::aesthetic_column("pos2"), - ]; + let mut mappings = Mappings::new(); + mappings.insert( + "pos1".to_string(), + AestheticValue::standard_column(naming::aesthetic_column("pos1")), + ); + mappings.insert( + "pos2".to_string(), + AestheticValue::standard_column(naming::aesthetic_column("pos2")), + ); let result = line .apply_projection( "SELECT * FROM t", &projection, &AnsiDialect, false, - &columns, - &[], + &mut mappings, + &mut vec![], ) .unwrap(); diff --git a/src/plot/layer/geom/mod.rs b/src/plot/layer/geom/mod.rs index 623d06bce..7c5b15328 100644 --- a/src/plot/layer/geom/mod.rs +++ b/src/plot/layer/geom/mod.rs @@ -297,10 +297,13 @@ pub trait GeomTrait: std::fmt::Debug + std::fmt::Display + Send + Sync { /// projection means for its parameterization: /// - Spatial: ST_AsWKB (always), plus ST_Transform when Map coord has a CRS /// - Line/path/polygon: densify segments before ST_Transform + /// - Tile (continuous): expand to polygon corners, densify, project /// /// `columns` lists all column names in the query (for portable column /// replacement on backends that don't support `SELECT * REPLACE`). - /// `partition_by` lists the grouping columns for the layer. + /// `partition_by` is mutable: geoms that introduce new grouping columns + /// (e.g. tile adds `__ggsql_poly_id__`) push them here so they survive + /// downstream pruning. /// /// The default is a no-op (returns query unchanged). fn apply_projection( @@ -309,8 +312,8 @@ pub trait GeomTrait: std::fmt::Debug + std::fmt::Display + Send + Sync { _projection: &Projection, _dialect: &dyn SqlDialect, _clip: bool, - _columns: &[String], - _partition_by: &[String], + _mappings: &mut Mappings, + _partition_by: &mut Vec, ) -> Result { Ok(query.to_string()) } @@ -433,8 +436,9 @@ pub(crate) fn needs_projection(projection: &Projection) -> bool { /// Continuous aesthetics (columns that are neither positions nor in `partition_by`) /// are interpolated too. Discrete (partition) columns are carried through unchanged. /// -/// - `domain_order`: aesthetic name for the ordering column (e.g. "pos1" for line). -/// When `None`, a synthetic row index is used (path/polygon). +/// - `domain_order`: column name to ORDER BY within each partition (e.g. +/// `naming::aesthetic_column("pos1")` for line). When `None`, a synthetic +/// row index is used (path/polygon). /// - `close_ring`: when true, the last vertex connects back to the first (polygon). /// - `segment_length`: target edge length after subdivision (in position units). /// - `n_segments`: size of the integer series (must be at least as large as the @@ -461,9 +465,9 @@ pub(crate) fn densify_edges( .filter(|c| *c != &pos1_col && *c != &pos2_col && !partition_by.contains(c)) .collect(); - // Ordering column + // Ordering column (raw column name, already unquoted) let order_col = match domain_order { - Some(aes) => naming::quote_ident(&naming::aesthetic_column(aes)), + Some(col) => naming::quote_ident(col), None => "\"__ggsql_edge_idx__\"".to_string(), }; @@ -844,11 +848,11 @@ impl Geom { projection: &Projection, dialect: &dyn SqlDialect, clip: bool, - columns: &[String], - partition_by: &[String], + mappings: &mut Mappings, + partition_by: &mut Vec, ) -> Result { self.0 - .apply_projection(query, projection, dialect, clip, columns, partition_by) + .apply_projection(query, projection, dialect, clip, mappings, partition_by) } /// Adjust layer mappings and parameters based on geom-specific logic @@ -1117,12 +1121,13 @@ mod tests { naming::aesthetic_column("pos1"), naming::aesthetic_column("pos2"), ]; + let pos1_col = naming::aesthetic_column("pos1"); let result = densify_edges( "SELECT * FROM t", &AnsiDialect, &columns, &[], - Some("pos1"), + Some(&pos1_col), false, 1.0, 360, @@ -1145,12 +1150,13 @@ mod tests { naming::aesthetic_column("stroke"), ]; let partition_by = vec![naming::aesthetic_column("stroke")]; + let pos1_col = naming::aesthetic_column("pos1"); let result = densify_edges( "SELECT * FROM t", &AnsiDialect, &columns, &partition_by, - Some("pos1"), + Some(&pos1_col), false, 1.0, 360, @@ -1171,12 +1177,13 @@ mod tests { naming::aesthetic_column("opacity"), ]; let partition_by = vec![naming::aesthetic_column("stroke")]; + let pos1_col = naming::aesthetic_column("pos1"); let result = densify_edges( "SELECT * FROM t", &AnsiDialect, &columns, &partition_by, - Some("pos1"), + Some(&pos1_col), false, 1.0, 360, @@ -1219,12 +1226,13 @@ mod tests { naming::aesthetic_column("pos1"), naming::aesthetic_column("pos2"), ]; + let pos1_col = naming::aesthetic_column("pos1"); let result = densify_edges( "SELECT * FROM t", &AnsiDialect, &columns, &[], - Some("pos1"), + Some(&pos1_col), false, 1.0, 360, diff --git a/src/plot/layer/geom/path.rs b/src/plot/layer/geom/path.rs index 6687c61a6..eb2902cf2 100644 --- a/src/plot/layer/geom/path.rs +++ b/src/plot/layer/geom/path.rs @@ -8,7 +8,7 @@ use super::{ use crate::plot::projection::Projection; use crate::plot::types::DefaultAestheticValue; use crate::reader::SqlDialect; -use crate::Result; +use crate::{naming, Mappings, Result}; /// Path geom - connected line segments in order #[derive(Debug, Clone, Copy)] @@ -47,14 +47,28 @@ impl GeomTrait for Path { projection: &Projection, dialect: &dyn SqlDialect, _clip: bool, - columns: &[String], - partition_by: &[String], + mappings: &mut Mappings, + partition_by: &mut Vec, ) -> Result { if !needs_projection(projection) { return Ok(query.to_string()); } - let densified = densify_edges(query, dialect, columns, partition_by, None, false, 1.0, 360); - project_position_columns(&densified, projection, dialect, columns) + let columns: Vec = mappings + .aesthetics + .keys() + .map(|k| naming::aesthetic_column(k)) + .collect(); + let densified = densify_edges( + query, + dialect, + &columns, + partition_by, + None, + false, + 1.0, + 360, + ); + project_position_columns(&densified, projection, dialect, &columns) } } diff --git a/src/plot/layer/geom/point.rs b/src/plot/layer/geom/point.rs index 51d40ceea..0ef63c9dc 100644 --- a/src/plot/layer/geom/point.rs +++ b/src/plot/layer/geom/point.rs @@ -8,7 +8,7 @@ use super::{ use crate::plot::projection::Projection; use crate::plot::types::DefaultAestheticValue; use crate::reader::SqlDialect; -use crate::Result; +use crate::{naming, Mappings, Result}; /// Point geom - scatter plots and similar #[derive(Debug, Clone, Copy)] @@ -61,10 +61,15 @@ impl GeomTrait for Point { projection: &Projection, dialect: &dyn SqlDialect, _clip: bool, - columns: &[String], - _partition_by: &[String], + mappings: &mut Mappings, + _partition_by: &mut Vec, ) -> Result { - project_position_columns(query, projection, dialect, columns) + let columns: Vec = mappings + .aesthetics + .keys() + .map(|k| naming::aesthetic_column(k)) + .collect(); + project_position_columns(query, projection, dialect, &columns) } } diff --git a/src/plot/layer/geom/polygon.rs b/src/plot/layer/geom/polygon.rs index 29f23af75..3c19a5fba 100644 --- a/src/plot/layer/geom/polygon.rs +++ b/src/plot/layer/geom/polygon.rs @@ -8,7 +8,7 @@ use super::{ use crate::plot::projection::Projection; use crate::plot::types::DefaultAestheticValue; use crate::reader::SqlDialect; -use crate::Result; +use crate::{naming, Mappings, Result}; /// Polygon geom - arbitrary polygons #[derive(Debug, Clone, Copy)] @@ -48,14 +48,19 @@ impl GeomTrait for Polygon { projection: &Projection, dialect: &dyn SqlDialect, _clip: bool, - columns: &[String], - partition_by: &[String], + mappings: &mut Mappings, + partition_by: &mut Vec, ) -> Result { if !needs_projection(projection) { return Ok(query.to_string()); } - let densified = densify_edges(query, dialect, columns, partition_by, None, true, 1.0, 360); - project_position_columns(&densified, projection, dialect, columns) + let columns: Vec = mappings + .aesthetics + .keys() + .map(|k| naming::aesthetic_column(k)) + .collect(); + let densified = densify_edges(query, dialect, &columns, partition_by, None, true, 1.0, 360); + project_position_columns(&densified, projection, dialect, &columns) } } diff --git a/src/plot/layer/geom/spatial.rs b/src/plot/layer/geom/spatial.rs index 82366f0ad..ef59f8592 100644 --- a/src/plot/layer/geom/spatial.rs +++ b/src/plot/layer/geom/spatial.rs @@ -77,16 +77,21 @@ impl GeomTrait for Spatial { projection: &Projection, dialect: &dyn SqlDialect, clip: bool, - columns: &[String], - _partition_by: &[String], + mappings: &mut Mappings, + _partition_by: &mut Vec, ) -> crate::Result { + let columns: Vec = mappings + .aesthetics + .keys() + .map(|k| naming::aesthetic_column(k)) + .collect(); let col = naming::quote_ident(&naming::aesthetic_column("geometry")); let is_map = projection.coord.coord_kind() == CoordKind::Map; // WORKAROUND(duckdb-rs#714): normalize column to GEOMETRY since it may // be WKB BLOB from the Arrow export workaround. let ensure_geom = dialect.sql_ensure_geometry(&col); - let geom_query = dialect.sql_select_replace(&ensure_geom, &col, query, columns); + let geom_query = dialect.sql_select_replace(&ensure_geom, &col, query, &columns); let geom_expr = if let (true, Some(ParameterValue::String(crs))) = (is_map, projection.properties.get("target")) @@ -103,7 +108,7 @@ impl GeomTrait for Spatial { source, crs, dialect, - columns, + &columns, )); } @@ -114,11 +119,11 @@ impl GeomTrait for Spatial { } else { // Non-map coord — convert to WKB directly let wkb_expr = dialect.sql_geometry_to_wkb(&col); - return Ok(dialect.sql_select_replace(&wkb_expr, &col, &geom_query, columns)); + return Ok(dialect.sql_select_replace(&wkb_expr, &col, &geom_query, &columns)); }; // Map coord with CRS — output native projected geometry (WKB added by framing) - Ok(dialect.sql_select_replace(&geom_expr, &col, &geom_query, columns)) + Ok(dialect.sql_select_replace(&geom_expr, &col, &geom_query, &columns)) } } @@ -143,8 +148,8 @@ mod tests { &projection, &AnsiDialect, false, - &[], - &[], + &mut Mappings::new(), + &mut vec![], ) .unwrap(); @@ -162,8 +167,8 @@ mod tests { &projection, &AnsiDialect, false, - &[], - &[], + &mut Mappings::new(), + &mut vec![], ) .unwrap(); @@ -186,8 +191,8 @@ mod tests { &projection, &AnsiDialect, false, - &[], - &[], + &mut Mappings::new(), + &mut vec![], ) .unwrap(); @@ -207,7 +212,14 @@ mod tests { ParameterValue::String("+proj=merc".to_string()), ); let result = spatial - .apply_projection("SELECT * FROM t", &projection, &AnsiDialect, true, &[], &[]) + .apply_projection( + "SELECT * FROM t", + &projection, + &AnsiDialect, + true, + &mut Mappings::new(), + &mut vec![], + ) .unwrap(); assert!(result.contains("ST_Intersection")); @@ -224,7 +236,14 @@ mod tests { ParameterValue::String("+proj=ortho +lat_0=45 +lon_0=10".to_string()), ); let result = spatial - .apply_projection("SELECT * FROM t", &projection, &AnsiDialect, true, &[], &[]) + .apply_projection( + "SELECT * FROM t", + &projection, + &AnsiDialect, + true, + &mut Mappings::new(), + &mut vec![], + ) .unwrap(); assert!(result.contains("ST_Transform")); @@ -243,7 +262,14 @@ mod tests { ParameterValue::String("+proj=gnom +lat_0=90 +lon_0=0".to_string()), ); let result = spatial - .apply_projection("SELECT * FROM t", &projection, &AnsiDialect, true, &[], &[]) + .apply_projection( + "SELECT * FROM t", + &projection, + &AnsiDialect, + true, + &mut Mappings::new(), + &mut vec![], + ) .unwrap(); assert!(result.contains("ST_MakeValid")); diff --git a/src/plot/layer/geom/text.rs b/src/plot/layer/geom/text.rs index be8a56714..f5e3d2e9b 100644 --- a/src/plot/layer/geom/text.rs +++ b/src/plot/layer/geom/text.rs @@ -9,7 +9,7 @@ use crate::plot::projection::Projection; use crate::plot::types::DefaultAestheticValue; use crate::plot::{ArrayConstraint, NumberConstraint}; use crate::reader::SqlDialect; -use crate::{naming, DataFrame, Result}; +use crate::{naming, DataFrame, Mappings, Result}; use std::collections::HashMap; /// Text geom - text labels at positions @@ -76,10 +76,15 @@ impl GeomTrait for Text { projection: &Projection, dialect: &dyn SqlDialect, _clip: bool, - columns: &[String], - _partition_by: &[String], + mappings: &mut Mappings, + _partition_by: &mut Vec, ) -> Result { - project_position_columns(query, projection, dialect, columns) + let columns: Vec = mappings + .aesthetics + .keys() + .map(|k| naming::aesthetic_column(k)) + .collect(); + project_position_columns(query, projection, dialect, &columns) } fn post_process( diff --git a/src/plot/layer/geom/tile.rs b/src/plot/layer/geom/tile.rs index dac354f8d..665c2cc20 100644 --- a/src/plot/layer/geom/tile.rs +++ b/src/plot/layer/geom/tile.rs @@ -6,9 +6,11 @@ use super::stat_aggregate; use super::types::POSITION_VALUES; use super::types::{get_column_name, get_quoted_column_name}; use super::{ - has_aggregate_param, DefaultAesthetics, GeomTrait, GeomType, ParamConstraint, StatResult, + densify_edges, has_aggregate_param, needs_projection, project_position_columns, + DefaultAesthetics, GeomTrait, GeomType, ParamConstraint, StatResult, }; use crate::naming; +use crate::plot::projection::Projection; use crate::plot::types::{ColumnInfo, DefaultAestheticValue, ParameterValue}; use crate::plot::{DefaultParamValue, ParamDefinition}; use crate::reader::SqlDialect; @@ -206,6 +208,154 @@ impl GeomTrait for Tile { } Ok(tile_result) } + + fn apply_projection( + &self, + query: &str, + projection: &Projection, + dialect: &dyn SqlDialect, + _clip: bool, + mappings: &mut Mappings, + partition_by: &mut Vec, + ) -> Result { + if !needs_projection(projection) { + return Ok(query.to_string()); + } + + let columns: Vec = mappings + .aesthetics + .keys() + .map(|k| naming::aesthetic_column(k)) + .collect(); + + // Only densify continuous tiles (those parameterized by pos1min/pos1max/pos2min/pos2max). + // Discrete tiles use categorical positions and don't appear on maps. + let bound_aes = ["pos1min", "pos1max", "pos2min", "pos2max"]; + let is_continuous = bound_aes + .iter() + .all(|a| columns.contains(&naming::aesthetic_column(a))); + + if !is_continuous { + return project_position_columns(query, projection, dialect, &columns); + } + + let (expanded, expanded_columns) = expand_rect_to_polygon(query, &columns); + + let poly_id_col = "__ggsql_poly_id__".to_string(); + partition_by.push(poly_id_col.clone()); + + let densified = densify_edges( + &expanded, + dialect, + &expanded_columns, + partition_by, + Some("__ggsql_corner__"), + true, + 1.0, + 360, + ); + let projected = + project_position_columns(&densified, projection, dialect, &expanded_columns)?; + + // After polygonization, the data has pos1/pos2 columns (not pos1min/pos1max/pos2min/pos2max). + // Update mappings to reflect the new column structure so downstream stages + // (schema validation, encoding) reference the correct columns. + use crate::plot::types::AestheticValue; + for aes in &bound_aes { + mappings.aesthetics.remove(*aes); + } + mappings.insert( + "pos1", + AestheticValue::standard_column(naming::aesthetic_column("pos1")), + ); + mappings.insert( + "pos2", + AestheticValue::standard_column(naming::aesthetic_column("pos2")), + ); + + Ok(projected) + } +} + +/// Expand each continuous-scale rectangle into 4 corner vertices (polygon outline). +/// +/// Input: one row per rectangle with pos1min/pos1max/pos2min/pos2max bounds. +/// Output: four rows per rectangle with pos1/pos2 corner positions and a +/// `__ggsql_poly_id__` grouping column. Material aesthetics pass through unchanged. +/// The bound columns are dropped — callers that need them should re-derive them +/// from pos1/pos2 after densification and projection. +/// +/// Returns the expanded query and the new column list. +fn expand_rect_to_polygon(query: &str, columns: &[String]) -> (String, Vec) { + let pos1min_col = naming::aesthetic_column("pos1min"); + let pos1max_col = naming::aesthetic_column("pos1max"); + let pos2min_col = naming::aesthetic_column("pos2min"); + let pos2max_col = naming::aesthetic_column("pos2max"); + + // Columns to carry through unchanged (everything except the 4 bound columns) + let passthrough_cols: Vec<&String> = columns + .iter() + .filter(|c| { + *c != &pos1min_col && *c != &pos1max_col && *c != &pos2min_col && *c != &pos2max_col + }) + .collect(); + let passthrough: Vec = passthrough_cols + .iter() + .map(|c| naming::quote_ident(c)) + .collect(); + + // Step 1: Number each rectangle. + // ORDER BY (SELECT NULL) is a workaround: ROW_NUMBER requires ORDER BY + // syntactically, but we don't care about the order — just need unique IDs. + let numbered = format!( + "SELECT *, ROW_NUMBER() OVER (ORDER BY (SELECT NULL)) \ + AS \"__ggsql_poly_id__\" FROM ({query})" + ); + + // Step 2: Expand to 4 corners via CROSS JOIN with UNION ALL literal table. + // More portable than VALUES(...) whose aliasing syntax varies across backends. + // Corner order: bottom-left, bottom-right, top-right, top-left (CCW) + let corners_table = "(SELECT 1 AS \"__ggsql_corner__\" \ + UNION ALL SELECT 2 \ + UNION ALL SELECT 3 \ + UNION ALL SELECT 4)"; + + let pos1min_q = naming::quote_ident(&pos1min_col); + let pos1max_q = naming::quote_ident(&pos1max_col); + let pos2min_q = naming::quote_ident(&pos2min_col); + let pos2max_q = naming::quote_ident(&pos2max_col); + let pos1_q = naming::quote_ident(&naming::aesthetic_column("pos1")); + let pos2_q = naming::quote_ident(&naming::aesthetic_column("pos2")); + + let mut select_parts: Vec = passthrough; + select_parts.push("\"__ggsql_poly_id__\"".to_string()); + select_parts.push("\"__ggsql_corner__\"".to_string()); + select_parts.push(format!( + "CASE \"__ggsql_corner__\" \ + WHEN 1 THEN {pos1min_q} WHEN 2 THEN {pos1max_q} \ + WHEN 3 THEN {pos1max_q} WHEN 4 THEN {pos1min_q} END AS {pos1_q}" + )); + select_parts.push(format!( + "CASE \"__ggsql_corner__\" \ + WHEN 1 THEN {pos2min_q} WHEN 2 THEN {pos2min_q} \ + WHEN 3 THEN {pos2max_q} WHEN 4 THEN {pos2max_q} END AS {pos2_q}" + )); + + let sql = format!( + "SELECT {} FROM ({numbered}) \"__ggsql_rect__\" \ + CROSS JOIN {corners_table} \"__ggsql_corners__\"", + select_parts.join(", ") + ); + + // Output columns: passthrough + poly_id + pos1 + pos2 + // __ggsql_corner__ is in the SQL (for ordering) but not in the column list + // so densify_edges won't attempt to interpolate it. + let mut out_columns: Vec = passthrough_cols.into_iter().cloned().collect(); + out_columns.push("__ggsql_poly_id__".to_string()); + out_columns.push(naming::aesthetic_column("pos1")); + out_columns.push(naming::aesthetic_column("pos2")); + + (sql, out_columns) } /// Wrap an aggregated query so each `__ggsql_stat___` column is also @@ -477,6 +627,17 @@ mod tests { // ==================== Helper Functions ==================== + fn create_bound_mappings(aesthetics: &[&str]) -> Mappings { + let mut mappings = Mappings::new(); + for aes in aesthetics { + mappings.insert( + aes.to_string(), + AestheticValue::standard_column(naming::aesthetic_column(aes)), + ); + } + mappings + } + fn create_schema(discrete_cols: &[&str]) -> Schema { create_schema_with_extra(discrete_cols, &[]) } @@ -1213,4 +1374,214 @@ mod tests { assert!(query.contains("0.9 AS \"__ggsql_stat_height")); } } + + // ==================== Projection / Densification Tests ==================== + + #[test] + fn test_expand_rect_to_polygon_structure() { + let columns = vec![ + naming::aesthetic_column("pos1min"), + naming::aesthetic_column("pos1max"), + naming::aesthetic_column("pos2min"), + naming::aesthetic_column("pos2max"), + naming::aesthetic_column("fill"), + ]; + let (sql, out_cols) = expand_rect_to_polygon("SELECT * FROM t", &columns); + + // Should have poly_id assignment + assert!(sql.contains("__ggsql_poly_id__")); + // Should use CROSS JOIN with UNION ALL corner table + assert!(sql.contains("CROSS JOIN")); + assert!(sql.contains("UNION ALL")); + // Should produce CASE expressions for pos1/pos2 + assert!(sql.contains("CASE \"__ggsql_corner__\"")); + // Should emit pos1 and pos2 + let pos1_col = naming::aesthetic_column("pos1"); + let pos2_col = naming::aesthetic_column("pos2"); + assert!(out_cols.contains(&pos1_col)); + assert!(out_cols.contains(&pos2_col)); + // Should NOT contain bound columns in output (they are dropped) + assert!(!out_cols.contains(&naming::aesthetic_column("pos1min"))); + assert!(!out_cols.contains(&naming::aesthetic_column("pos1max"))); + // Should carry through fill + assert!(out_cols.contains(&naming::aesthetic_column("fill"))); + // Should include poly_id + assert!(out_cols.contains(&"__ggsql_poly_id__".to_string())); + } + + #[test] + fn test_apply_projection_no_op_without_map() { + let tile = Tile; + let projection = Projection::cartesian(); + let mut mappings = create_bound_mappings(&["pos1min", "pos1max", "pos2min", "pos2max"]); + let result = tile + .apply_projection( + "SELECT * FROM t", + &projection, + &crate::reader::AnsiDialect, + false, + &mut mappings, + &mut vec![], + ) + .unwrap(); + + assert_eq!(result, "SELECT * FROM t"); + } + + #[test] + fn test_apply_projection_densifies_continuous_tiles() { + let tile = Tile; + let mut projection = Projection::map(); + projection.properties.insert( + "source".to_string(), + ParameterValue::String("EPSG:4326".to_string()), + ); + projection.properties.insert( + "target".to_string(), + ParameterValue::String("+proj=ortho +lat_0=0 +lon_0=0".to_string()), + ); + + let mut mappings = + create_bound_mappings(&["pos1min", "pos1max", "pos2min", "pos2max", "fill"]); + let result = tile + .apply_projection( + "SELECT * FROM t", + &projection, + &crate::reader::AnsiDialect, + false, + &mut mappings, + &mut vec![], + ) + .unwrap(); + + // Polygon expansion happened + assert!(result.contains("__ggsql_poly_id__")); + assert!(result.contains("CROSS JOIN")); + // Densification happened + assert!(result.contains("__ggsql_seq__")); + assert!(result.contains("LEAD(")); + // Projection happened + assert!(result.contains("ST_Transform")); + // Mappings mutated: bound aesthetics replaced by pos1/pos2 + assert!(!mappings.contains_key("pos1min")); + assert!(!mappings.contains_key("pos1max")); + assert!(!mappings.contains_key("pos2min")); + assert!(!mappings.contains_key("pos2max")); + assert!(mappings.contains_key("pos1")); + assert!(mappings.contains_key("pos2")); + } + + #[cfg(feature = "duckdb")] + #[test] + fn test_densified_rectangle_vertex_order() { + use crate::reader::{DuckDBReader, Reader}; + + let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap(); + let dialect = reader.dialect(); + + // A simple 20°×20° rectangle + let input = "SELECT -80.0 AS \"__ggsql_aes_pos1min__\", \ + -60.0 AS \"__ggsql_aes_pos1max__\", \ + 30.0 AS \"__ggsql_aes_pos2min__\", \ + 50.0 AS \"__ggsql_aes_pos2max__\""; + + let mut mappings = create_bound_mappings(&["pos1min", "pos1max", "pos2min", "pos2max"]); + + let tile = Tile; + let mut projection = Projection::map(); + projection.properties.insert( + "source".to_string(), + ParameterValue::String("EPSG:4326".to_string()), + ); + projection.properties.insert( + "target".to_string(), + ParameterValue::String("+proj=ortho +lat_0=40 +lon_0=-70".to_string()), + ); + + for stmt in dialect.sql_spatial_setup() { + reader.execute_sql(&stmt).unwrap(); + } + + let projected_sql = tile + .apply_projection( + input, + &projection, + dialect, + false, + &mut mappings, + &mut vec![], + ) + .unwrap(); + + let df = reader.execute_sql(&projected_sql).unwrap(); + let n = df.inner().num_rows(); + assert!(n > 4, "expected densified vertices, got {n}"); + + // After polygonization, columns are pos1/pos2 (not pos1min/pos2min) + let pos1_col = df + .inner() + .column_by_name(&naming::aesthetic_column("pos1")) + .unwrap() + .as_any() + .downcast_ref::() + .unwrap(); + let pos2_col = df + .inner() + .column_by_name(&naming::aesthetic_column("pos2")) + .unwrap() + .as_any() + .downcast_ref::() + .unwrap(); + + // A bowtie would show the polygon self-intersecting: edges cross. + // Check that consecutive edges don't cross by computing signed area. + // A simple (non-self-intersecting) polygon has consistent winding → + // the signed area is non-zero with one sign. + let mut signed_area: f64 = 0.0; + for i in 0..n { + let j = (i + 1) % n; + let x0 = pos1_col.value(i); + let y0 = pos2_col.value(i); + let x1 = pos1_col.value(j); + let y1 = pos2_col.value(j); + signed_area += (x1 - x0) * (y1 + y0); + } + // Non-zero signed area means consistent winding (no bowtie) + assert!( + signed_area.abs() > 1e6, + "signed area too small ({signed_area}), likely a bowtie or degenerate polygon" + ); + } + + #[test] + fn test_apply_projection_discrete_tiles_only_project() { + let tile = Tile; + let mut projection = Projection::map(); + projection.properties.insert( + "source".to_string(), + ParameterValue::String("EPSG:4326".to_string()), + ); + projection.properties.insert( + "target".to_string(), + ParameterValue::String("+proj=merc".to_string()), + ); + + // Discrete tiles only have pos1/pos2 + let mut mappings = create_bound_mappings(&["pos1", "pos2"]); + let result = tile + .apply_projection( + "SELECT * FROM t", + &projection, + &crate::reader::AnsiDialect, + false, + &mut mappings, + &mut vec![], + ) + .unwrap(); + + // Should just project, no densification + assert!(result.contains("ST_Transform")); + assert!(!result.contains("__ggsql_poly_id__")); + assert!(!result.contains("CROSS JOIN")); + } } diff --git a/src/plot/projection/coord/map.rs b/src/plot/projection/coord/map.rs index 3e311fd6a..219e7c137 100644 --- a/src/plot/projection/coord/map.rs +++ b/src/plot/projection/coord/map.rs @@ -16,7 +16,7 @@ use crate::DataFrame; pub(crate) fn apply_map_transforms( map_proj: &dyn MapProjectionTrait, - layers: &[Layer], + layers: &mut [Layer], layer_queries: &mut [String], projection: &mut super::super::Projection, dialect: &dyn SqlDialect, @@ -57,20 +57,14 @@ pub(crate) fn apply_map_transforms( let clip = boundary_lonlat.is_some(); // Step 3: Apply per-layer projection (ST_Transform, clip to horizon) - for (idx, layer) in layers.iter().enumerate() { - let columns: Vec = layer - .mappings - .aesthetics - .keys() - .map(|k| naming::aesthetic_column(k)) - .collect(); + for (idx, layer) in layers.iter_mut().enumerate() { layer_queries[idx] = layer.geom.apply_projection( &layer_queries[idx], projection, dialect, clip, - &columns, - &layer.partition_by, + &mut layer.mappings, + &mut layer.partition_by, )?; } diff --git a/src/plot/projection/coord/map_projections.rs b/src/plot/projection/coord/map_projections.rs index bfe32756d..5c5b689b4 100644 --- a/src/plot/projection/coord/map_projections.rs +++ b/src/plot/projection/coord/map_projections.rs @@ -238,7 +238,7 @@ impl super::CoordTrait for T { fn apply_projection_transforms( &self, - layers: &[Layer], + layers: &mut [Layer], layer_queries: &mut [String], projection: &mut super::super::Projection, dialect: &dyn SqlDialect, diff --git a/src/plot/projection/coord/mod.rs b/src/plot/projection/coord/mod.rs index 09ec866e3..a4ef7996b 100644 --- a/src/plot/projection/coord/mod.rs +++ b/src/plot/projection/coord/mod.rs @@ -144,26 +144,20 @@ pub trait CoordTrait: std::fmt::Debug + Send + Sync { /// Override to add coord-specific setup (e.g., Map loads the spatial extension). fn apply_projection_transforms( &self, - layers: &[Layer], + layers: &mut [Layer], layer_queries: &mut [String], projection: &mut super::Projection, dialect: &dyn SqlDialect, _execute_query: &dyn Fn(&str) -> crate::Result, ) -> crate::Result<()> { - for (idx, layer) in layers.iter().enumerate() { - let columns: Vec = layer - .mappings - .aesthetics - .keys() - .map(|k| crate::naming::aesthetic_column(k)) - .collect(); + for (idx, layer) in layers.iter_mut().enumerate() { layer_queries[idx] = layer.geom.apply_projection( &layer_queries[idx], projection, dialect, false, - &columns, - &layer.partition_by, + &mut layer.mappings, + &mut layer.partition_by, )?; } Ok(()) @@ -247,7 +241,7 @@ impl Coord { /// Orchestrate projection transforms for all layers. pub fn apply_projection_transforms( &self, - layers: &[Layer], + layers: &mut [Layer], layer_queries: &mut [String], projection: &mut super::Projection, dialect: &dyn SqlDialect, diff --git a/src/plot/projection/types.rs b/src/plot/projection/types.rs index b8c4fb49a..d69474908 100644 --- a/src/plot/projection/types.rs +++ b/src/plot/projection/types.rs @@ -67,7 +67,7 @@ impl Projection { /// Orchestrate projection transforms for all layers. pub fn apply_projection_transforms( &mut self, - layers: &[Layer], + layers: &mut [Layer], layer_queries: &mut [String], dialect: &dyn SqlDialect, execute_query: &dyn Fn(&str) -> crate::Result, diff --git a/src/writer/vegalite/layer.rs b/src/writer/vegalite/layer.rs index 7ef0b0b5e..d24a38b6a 100644 --- a/src/writer/vegalite/layer.rs +++ b/src/writer/vegalite/layer.rs @@ -1429,9 +1429,36 @@ impl GeomRenderer for TileRenderer { fn modify_spec( &self, layer_spec: &mut Value, - _layer: &Layer, + layer: &Layer, context: &RenderContext, ) -> Result<()> { + let (pos1, pos1_end, _, pos2, pos2_end, _) = &context.channels; + + // Polygonized tile: densified rectangle rendered as closed line + if layer + .partition_by + .contains(&"__ggsql_poly_id__".to_string()) + { + layer_spec["mark"] = json!({ + "type": "line", + "interpolate": "linear-closed" + }); + + if let Some(encoding) = layer_spec + .get_mut("encoding") + .and_then(|e| e.as_object_mut()) + { + // Preserve vertex order + encoding.insert( + "order".to_string(), + json!({"field": ROW_INDEX_COLUMN, "type": "quantitative"}), + ); + } + + return Ok(()); + } + + // Discrete tile: rect mark with band-based width/height let encoding = layer_spec .get_mut("encoding") .and_then(|e| e.as_object_mut()); @@ -1440,13 +1467,11 @@ impl GeomRenderer for TileRenderer { return Ok(()); }; - let (pos1, pos1_end, _, pos2, pos2_end, _) = &context.channels; - // Check which directions are discrete let pos1_is_discrete = !encoding.contains_key(pos1_end.as_str()); let pos2_is_discrete = !encoding.contains_key(pos2_end.as_str()); - // Early return if both continuous + // Early return if both continuous (standard rect mark is fine) if !pos1_is_discrete && !pos2_is_discrete { return Ok(()); } @@ -1459,7 +1484,6 @@ impl GeomRenderer for TileRenderer { if pos1_is_discrete { if let Some(width_enc) = encoding.remove("width") { - // Check if it's a field encoding or literal value if let Some(field) = width_enc.get("field").and_then(|f| f.as_str()) { // Field encoding: use expression with datum reference mark["width"] = json!({ @@ -1474,7 +1498,6 @@ impl GeomRenderer for TileRenderer { if pos2_is_discrete { if let Some(height_enc) = encoding.remove("height") { - // Check if it's a field encoding or literal value if let Some(field) = height_enc.get("field").and_then(|f| f.as_str()) { // Field encoding: use expression with datum reference mark["height"] = json!({ @@ -1487,7 +1510,6 @@ impl GeomRenderer for TileRenderer { } } - // Only set mark if we added width or height if mark.get("width").is_some() || mark.get("height").is_some() { layer_spec["mark"] = mark; } From ac9b60ed7f2d85ce155cb4d2c4df245836a540e8 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Wed, 10 Jun 2026 16:37:54 +0200 Subject: [PATCH 08/20] Remove clip parameter from apply_projection signature MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The resolved clip state is already available in projection.properties — spatial now reads it directly from there instead of receiving it as a separate argument. map.rs writes the resolved value back after determining whether the boundary materialized. Co-Authored-By: Claude Opus 4.6 --- src/plot/layer/geom/line.rs | 2 -- src/plot/layer/geom/mod.rs | 4 +--- src/plot/layer/geom/path.rs | 1 - src/plot/layer/geom/point.rs | 1 - src/plot/layer/geom/polygon.rs | 1 - src/plot/layer/geom/spatial.rs | 20 +++++++++++++------- src/plot/layer/geom/text.rs | 1 - src/plot/layer/geom/tile.rs | 13 +------------ src/plot/projection/coord/map.rs | 6 ++++-- src/plot/projection/coord/mod.rs | 1 - 10 files changed, 19 insertions(+), 31 deletions(-) diff --git a/src/plot/layer/geom/line.rs b/src/plot/layer/geom/line.rs index 9006cd90f..d06cb3502 100644 --- a/src/plot/layer/geom/line.rs +++ b/src/plot/layer/geom/line.rs @@ -86,7 +86,6 @@ impl GeomTrait for Line { query: &str, projection: &Projection, dialect: &dyn SqlDialect, - _clip: bool, mappings: &mut Mappings, partition_by: &mut Vec, ) -> Result { @@ -155,7 +154,6 @@ mod tests { "SELECT * FROM t", &projection, &AnsiDialect, - false, &mut mappings, &mut vec![], ) diff --git a/src/plot/layer/geom/mod.rs b/src/plot/layer/geom/mod.rs index 7c5b15328..3ecb2776d 100644 --- a/src/plot/layer/geom/mod.rs +++ b/src/plot/layer/geom/mod.rs @@ -311,7 +311,6 @@ pub trait GeomTrait: std::fmt::Debug + std::fmt::Display + Send + Sync { query: &str, _projection: &Projection, _dialect: &dyn SqlDialect, - _clip: bool, _mappings: &mut Mappings, _partition_by: &mut Vec, ) -> Result { @@ -847,12 +846,11 @@ impl Geom { query: &str, projection: &Projection, dialect: &dyn SqlDialect, - clip: bool, mappings: &mut Mappings, partition_by: &mut Vec, ) -> Result { self.0 - .apply_projection(query, projection, dialect, clip, mappings, partition_by) + .apply_projection(query, projection, dialect, mappings, partition_by) } /// Adjust layer mappings and parameters based on geom-specific logic diff --git a/src/plot/layer/geom/path.rs b/src/plot/layer/geom/path.rs index eb2902cf2..de8d1213e 100644 --- a/src/plot/layer/geom/path.rs +++ b/src/plot/layer/geom/path.rs @@ -46,7 +46,6 @@ impl GeomTrait for Path { query: &str, projection: &Projection, dialect: &dyn SqlDialect, - _clip: bool, mappings: &mut Mappings, partition_by: &mut Vec, ) -> Result { diff --git a/src/plot/layer/geom/point.rs b/src/plot/layer/geom/point.rs index 0ef63c9dc..47c8ad1ea 100644 --- a/src/plot/layer/geom/point.rs +++ b/src/plot/layer/geom/point.rs @@ -60,7 +60,6 @@ impl GeomTrait for Point { query: &str, projection: &Projection, dialect: &dyn SqlDialect, - _clip: bool, mappings: &mut Mappings, _partition_by: &mut Vec, ) -> Result { diff --git a/src/plot/layer/geom/polygon.rs b/src/plot/layer/geom/polygon.rs index 3c19a5fba..16f32686c 100644 --- a/src/plot/layer/geom/polygon.rs +++ b/src/plot/layer/geom/polygon.rs @@ -47,7 +47,6 @@ impl GeomTrait for Polygon { query: &str, projection: &Projection, dialect: &dyn SqlDialect, - _clip: bool, mappings: &mut Mappings, partition_by: &mut Vec, ) -> Result { diff --git a/src/plot/layer/geom/spatial.rs b/src/plot/layer/geom/spatial.rs index ef59f8592..3217409da 100644 --- a/src/plot/layer/geom/spatial.rs +++ b/src/plot/layer/geom/spatial.rs @@ -76,7 +76,6 @@ impl GeomTrait for Spatial { query: &str, projection: &Projection, dialect: &dyn SqlDialect, - clip: bool, mappings: &mut Mappings, _partition_by: &mut Vec, ) -> crate::Result { @@ -87,6 +86,10 @@ impl GeomTrait for Spatial { .collect(); let col = naming::quote_ident(&naming::aesthetic_column("geometry")); let is_map = projection.coord.coord_kind() == CoordKind::Map; + let clip = matches!( + projection.properties.get("clip"), + Some(ParameterValue::Boolean(true)) + ); // WORKAROUND(duckdb-rs#714): normalize column to GEOMETRY since it may // be WKB BLOB from the Arrow export workaround. @@ -147,7 +150,6 @@ mod tests { "SELECT * FROM t", &projection, &AnsiDialect, - false, &mut Mappings::new(), &mut vec![], ) @@ -166,7 +168,6 @@ mod tests { "SELECT * FROM t", &projection, &AnsiDialect, - false, &mut Mappings::new(), &mut vec![], ) @@ -190,7 +191,6 @@ mod tests { "SELECT * FROM t", &projection, &AnsiDialect, - false, &mut Mappings::new(), &mut vec![], ) @@ -211,12 +211,14 @@ mod tests { "target".to_string(), ParameterValue::String("+proj=merc".to_string()), ); + projection + .properties + .insert("clip".to_string(), ParameterValue::Boolean(true)); let result = spatial .apply_projection( "SELECT * FROM t", &projection, &AnsiDialect, - true, &mut Mappings::new(), &mut vec![], ) @@ -235,12 +237,14 @@ mod tests { "target".to_string(), ParameterValue::String("+proj=ortho +lat_0=45 +lon_0=10".to_string()), ); + projection + .properties + .insert("clip".to_string(), ParameterValue::Boolean(true)); let result = spatial .apply_projection( "SELECT * FROM t", &projection, &AnsiDialect, - true, &mut Mappings::new(), &mut vec![], ) @@ -261,12 +265,14 @@ mod tests { "target".to_string(), ParameterValue::String("+proj=gnom +lat_0=90 +lon_0=0".to_string()), ); + projection + .properties + .insert("clip".to_string(), ParameterValue::Boolean(true)); let result = spatial .apply_projection( "SELECT * FROM t", &projection, &AnsiDialect, - true, &mut Mappings::new(), &mut vec![], ) diff --git a/src/plot/layer/geom/text.rs b/src/plot/layer/geom/text.rs index f5e3d2e9b..6f778d4e2 100644 --- a/src/plot/layer/geom/text.rs +++ b/src/plot/layer/geom/text.rs @@ -75,7 +75,6 @@ impl GeomTrait for Text { query: &str, projection: &Projection, dialect: &dyn SqlDialect, - _clip: bool, mappings: &mut Mappings, _partition_by: &mut Vec, ) -> Result { diff --git a/src/plot/layer/geom/tile.rs b/src/plot/layer/geom/tile.rs index 665c2cc20..c8fe2d105 100644 --- a/src/plot/layer/geom/tile.rs +++ b/src/plot/layer/geom/tile.rs @@ -214,7 +214,6 @@ impl GeomTrait for Tile { query: &str, projection: &Projection, dialect: &dyn SqlDialect, - _clip: bool, mappings: &mut Mappings, partition_by: &mut Vec, ) -> Result { @@ -1419,7 +1418,6 @@ mod tests { "SELECT * FROM t", &projection, &crate::reader::AnsiDialect, - false, &mut mappings, &mut vec![], ) @@ -1448,7 +1446,6 @@ mod tests { "SELECT * FROM t", &projection, &crate::reader::AnsiDialect, - false, &mut mappings, &mut vec![], ) @@ -1503,14 +1500,7 @@ mod tests { } let projected_sql = tile - .apply_projection( - input, - &projection, - dialect, - false, - &mut mappings, - &mut vec![], - ) + .apply_projection(input, &projection, dialect, &mut mappings, &mut vec![]) .unwrap(); let df = reader.execute_sql(&projected_sql).unwrap(); @@ -1573,7 +1563,6 @@ mod tests { "SELECT * FROM t", &projection, &crate::reader::AnsiDialect, - false, &mut mappings, &mut vec![], ) diff --git a/src/plot/projection/coord/map.rs b/src/plot/projection/coord/map.rs index 219e7c137..42ed896d7 100644 --- a/src/plot/projection/coord/map.rs +++ b/src/plot/projection/coord/map.rs @@ -54,7 +54,10 @@ pub(crate) fn apply_map_transforms( world_bbox = compute_world_bbox(&source, &target, dialect, execute_query); boundary_lonlat = Some(b); } - let clip = boundary_lonlat.is_some(); + projection.properties.insert( + "clip".to_string(), + ParameterValue::Boolean(boundary_lonlat.is_some()), + ); // Step 3: Apply per-layer projection (ST_Transform, clip to horizon) for (idx, layer) in layers.iter_mut().enumerate() { @@ -62,7 +65,6 @@ pub(crate) fn apply_map_transforms( &layer_queries[idx], projection, dialect, - clip, &mut layer.mappings, &mut layer.partition_by, )?; diff --git a/src/plot/projection/coord/mod.rs b/src/plot/projection/coord/mod.rs index a4ef7996b..7f6e6c576 100644 --- a/src/plot/projection/coord/mod.rs +++ b/src/plot/projection/coord/mod.rs @@ -155,7 +155,6 @@ pub trait CoordTrait: std::fmt::Debug + Send + Sync { &layer_queries[idx], projection, dialect, - false, &mut layer.mappings, &mut layer.partition_by, )?; From 65b3348bb14d643ebafe3e9027905ff80578fb2e Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Wed, 10 Jun 2026 16:49:49 +0200 Subject: [PATCH 09/20] Add Mappings::insert_column convenience method Co-Authored-By: Claude Opus 4.6 --- src/plot/layer/geom/line.rs | 12 ++---------- src/plot/layer/geom/tile.rs | 16 +++------------- src/plot/types.rs | 8 ++++++++ 3 files changed, 13 insertions(+), 23 deletions(-) diff --git a/src/plot/layer/geom/line.rs b/src/plot/layer/geom/line.rs index d06cb3502..75cfc3014 100644 --- a/src/plot/layer/geom/line.rs +++ b/src/plot/layer/geom/line.rs @@ -127,8 +127,6 @@ mod tests { #[test] fn test_apply_projection_densifies_and_transforms() { - use crate::plot::types::AestheticValue; - let line = Line; let mut projection = Projection::map(); projection.properties.insert( @@ -141,14 +139,8 @@ mod tests { ); let mut mappings = Mappings::new(); - mappings.insert( - "pos1".to_string(), - AestheticValue::standard_column(naming::aesthetic_column("pos1")), - ); - mappings.insert( - "pos2".to_string(), - AestheticValue::standard_column(naming::aesthetic_column("pos2")), - ); + mappings.insert_column("pos1", "pos1"); + mappings.insert_column("pos2", "pos2"); let result = line .apply_projection( "SELECT * FROM t", diff --git a/src/plot/layer/geom/tile.rs b/src/plot/layer/geom/tile.rs index c8fe2d105..3134d2f1e 100644 --- a/src/plot/layer/geom/tile.rs +++ b/src/plot/layer/geom/tile.rs @@ -259,18 +259,11 @@ impl GeomTrait for Tile { // After polygonization, the data has pos1/pos2 columns (not pos1min/pos1max/pos2min/pos2max). // Update mappings to reflect the new column structure so downstream stages // (schema validation, encoding) reference the correct columns. - use crate::plot::types::AestheticValue; for aes in &bound_aes { mappings.aesthetics.remove(*aes); } - mappings.insert( - "pos1", - AestheticValue::standard_column(naming::aesthetic_column("pos1")), - ); - mappings.insert( - "pos2", - AestheticValue::standard_column(naming::aesthetic_column("pos2")), - ); + mappings.insert_column("pos1", "pos1"); + mappings.insert_column("pos2", "pos2"); Ok(projected) } @@ -629,10 +622,7 @@ mod tests { fn create_bound_mappings(aesthetics: &[&str]) -> Mappings { let mut mappings = Mappings::new(); for aes in aesthetics { - mappings.insert( - aes.to_string(), - AestheticValue::standard_column(naming::aesthetic_column(aes)), - ); + mappings.insert_column(aes, aes); } mappings } diff --git a/src/plot/types.rs b/src/plot/types.rs index ba69fc8f1..2b44ed5a1 100644 --- a/src/plot/types.rs +++ b/src/plot/types.rs @@ -96,6 +96,14 @@ impl Mappings { self.aesthetics.insert(aesthetic.into(), value); } + /// Insert a standard column mapping using the internal naming convention. + pub fn insert_column(&mut self, aesthetic: &str, column: &str) { + self.insert( + aesthetic, + AestheticValue::standard_column(crate::naming::aesthetic_column(column)), + ); + } + /// Get an aesthetic value by name pub fn get(&self, aesthetic: &str) -> Option<&AestheticValue> { self.aesthetics.get(aesthetic) From da863f790272b554892c4d4a209a2feccafca56e Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Wed, 10 Jun 2026 16:55:24 +0200 Subject: [PATCH 10/20] Add Mappings::column_names and use it across geoms Co-Authored-By: Claude Opus 4.6 --- src/plot/layer/geom/line.rs | 6 +----- src/plot/layer/geom/path.rs | 8 ++------ src/plot/layer/geom/point.rs | 8 ++------ src/plot/layer/geom/polygon.rs | 8 ++------ src/plot/layer/geom/spatial.rs | 6 +----- src/plot/layer/geom/text.rs | 6 +----- src/plot/layer/geom/tile.rs | 6 +----- src/plot/projection/coord/map.rs | 7 +------ src/plot/types.rs | 8 ++++++++ 9 files changed, 19 insertions(+), 44 deletions(-) diff --git a/src/plot/layer/geom/line.rs b/src/plot/layer/geom/line.rs index 75cfc3014..a86c80bc6 100644 --- a/src/plot/layer/geom/line.rs +++ b/src/plot/layer/geom/line.rs @@ -92,11 +92,7 @@ impl GeomTrait for Line { if !needs_projection(projection) { return Ok(query.to_string()); } - let columns: Vec = mappings - .aesthetics - .keys() - .map(|k| naming::aesthetic_column(k)) - .collect(); + let columns = mappings.column_names(); let pos1_col = naming::aesthetic_column("pos1"); let densified = densify_edges( query, diff --git a/src/plot/layer/geom/path.rs b/src/plot/layer/geom/path.rs index de8d1213e..8accb166c 100644 --- a/src/plot/layer/geom/path.rs +++ b/src/plot/layer/geom/path.rs @@ -8,7 +8,7 @@ use super::{ use crate::plot::projection::Projection; use crate::plot::types::DefaultAestheticValue; use crate::reader::SqlDialect; -use crate::{naming, Mappings, Result}; +use crate::{Mappings, Result}; /// Path geom - connected line segments in order #[derive(Debug, Clone, Copy)] @@ -52,11 +52,7 @@ impl GeomTrait for Path { if !needs_projection(projection) { return Ok(query.to_string()); } - let columns: Vec = mappings - .aesthetics - .keys() - .map(|k| naming::aesthetic_column(k)) - .collect(); + let columns = mappings.column_names(); let densified = densify_edges( query, dialect, diff --git a/src/plot/layer/geom/point.rs b/src/plot/layer/geom/point.rs index 47c8ad1ea..58d2b734c 100644 --- a/src/plot/layer/geom/point.rs +++ b/src/plot/layer/geom/point.rs @@ -8,7 +8,7 @@ use super::{ use crate::plot::projection::Projection; use crate::plot::types::DefaultAestheticValue; use crate::reader::SqlDialect; -use crate::{naming, Mappings, Result}; +use crate::{Mappings, Result}; /// Point geom - scatter plots and similar #[derive(Debug, Clone, Copy)] @@ -63,11 +63,7 @@ impl GeomTrait for Point { mappings: &mut Mappings, _partition_by: &mut Vec, ) -> Result { - let columns: Vec = mappings - .aesthetics - .keys() - .map(|k| naming::aesthetic_column(k)) - .collect(); + let columns = mappings.column_names(); project_position_columns(query, projection, dialect, &columns) } } diff --git a/src/plot/layer/geom/polygon.rs b/src/plot/layer/geom/polygon.rs index 16f32686c..34db09ed1 100644 --- a/src/plot/layer/geom/polygon.rs +++ b/src/plot/layer/geom/polygon.rs @@ -8,7 +8,7 @@ use super::{ use crate::plot::projection::Projection; use crate::plot::types::DefaultAestheticValue; use crate::reader::SqlDialect; -use crate::{naming, Mappings, Result}; +use crate::{Mappings, Result}; /// Polygon geom - arbitrary polygons #[derive(Debug, Clone, Copy)] @@ -53,11 +53,7 @@ impl GeomTrait for Polygon { if !needs_projection(projection) { return Ok(query.to_string()); } - let columns: Vec = mappings - .aesthetics - .keys() - .map(|k| naming::aesthetic_column(k)) - .collect(); + let columns = mappings.column_names(); let densified = densify_edges(query, dialect, &columns, partition_by, None, true, 1.0, 360); project_position_columns(&densified, projection, dialect, &columns) } diff --git a/src/plot/layer/geom/spatial.rs b/src/plot/layer/geom/spatial.rs index 3217409da..ca0eb7537 100644 --- a/src/plot/layer/geom/spatial.rs +++ b/src/plot/layer/geom/spatial.rs @@ -79,11 +79,7 @@ impl GeomTrait for Spatial { mappings: &mut Mappings, _partition_by: &mut Vec, ) -> crate::Result { - let columns: Vec = mappings - .aesthetics - .keys() - .map(|k| naming::aesthetic_column(k)) - .collect(); + let columns = mappings.column_names(); let col = naming::quote_ident(&naming::aesthetic_column("geometry")); let is_map = projection.coord.coord_kind() == CoordKind::Map; let clip = matches!( diff --git a/src/plot/layer/geom/text.rs b/src/plot/layer/geom/text.rs index 6f778d4e2..d103f9134 100644 --- a/src/plot/layer/geom/text.rs +++ b/src/plot/layer/geom/text.rs @@ -78,11 +78,7 @@ impl GeomTrait for Text { mappings: &mut Mappings, _partition_by: &mut Vec, ) -> Result { - let columns: Vec = mappings - .aesthetics - .keys() - .map(|k| naming::aesthetic_column(k)) - .collect(); + let columns = mappings.column_names(); project_position_columns(query, projection, dialect, &columns) } diff --git a/src/plot/layer/geom/tile.rs b/src/plot/layer/geom/tile.rs index 3134d2f1e..9284d0b65 100644 --- a/src/plot/layer/geom/tile.rs +++ b/src/plot/layer/geom/tile.rs @@ -221,11 +221,7 @@ impl GeomTrait for Tile { return Ok(query.to_string()); } - let columns: Vec = mappings - .aesthetics - .keys() - .map(|k| naming::aesthetic_column(k)) - .collect(); + let columns = mappings.column_names(); // Only densify continuous tiles (those parameterized by pos1min/pos1max/pos2min/pos2max). // Discrete tiles use categorical positions and don't appear on maps. diff --git a/src/plot/projection/coord/map.rs b/src/plot/projection/coord/map.rs index 42ed896d7..aae93ccaa 100644 --- a/src/plot/projection/coord/map.rs +++ b/src/plot/projection/coord/map.rs @@ -97,12 +97,7 @@ pub(crate) fn apply_map_transforms( } layer_queries[idx] = if is_spatial { - let columns: Vec = layer - .mappings - .aesthetics - .keys() - .map(|k| naming::aesthetic_column(k)) - .collect(); + let columns = layer.mappings.column_names(); let geom_col_quoted = naming::quote_ident(&naming::aesthetic_column("geometry")); let wkb_expr = dialect.sql_geometry_to_wkb(&geom_col_quoted); dialect.sql_select_replace( diff --git a/src/plot/types.rs b/src/plot/types.rs index 2b44ed5a1..a76072783 100644 --- a/src/plot/types.rs +++ b/src/plot/types.rs @@ -104,6 +104,14 @@ impl Mappings { ); } + /// Return the internal column names for all mapped aesthetics. + pub fn column_names(&self) -> Vec { + self.aesthetics + .keys() + .map(|k| crate::naming::aesthetic_column(k)) + .collect() + } + /// Get an aesthetic value by name pub fn get(&self, aesthetic: &str) -> Option<&AestheticValue> { self.aesthetics.get(aesthetic) From 3f9446c2697738d24b066ec6becd3bcc6a5c84c3 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Thu, 11 Jun 2026 09:33:26 +0200 Subject: [PATCH 11/20] Implement edge densification for segment geom under map projection Co-Authored-By: Claude Opus 4.6 --- src/plot/layer/geom/segment.rs | 251 ++++++++++++++++++++++++++++++++- src/writer/vegalite/layer.rs | 45 +++++- 2 files changed, 293 insertions(+), 3 deletions(-) diff --git a/src/plot/layer/geom/segment.rs b/src/plot/layer/geom/segment.rs index 3a76dc97b..81f06f024 100644 --- a/src/plot/layer/geom/segment.rs +++ b/src/plot/layer/geom/segment.rs @@ -2,9 +2,13 @@ use super::types::POSITION_VALUES; use super::{ - DefaultAesthetics, DefaultParamValue, GeomTrait, GeomType, ParamConstraint, ParamDefinition, + densify_edges, needs_projection, project_position_columns, DefaultAesthetics, + DefaultParamValue, GeomTrait, GeomType, ParamConstraint, ParamDefinition, }; +use crate::plot::projection::Projection; use crate::plot::types::DefaultAestheticValue; +use crate::reader::SqlDialect; +use crate::{naming, Mappings, Result}; /// Segment geom - line segments between two points #[derive(Debug, Clone, Copy)] @@ -45,6 +49,99 @@ impl GeomTrait for Segment { fn aggregate_domain_aesthetics(&self) -> Option<&'static [&'static str]> { Some(&[]) } + + fn apply_projection( + &self, + query: &str, + projection: &Projection, + dialect: &dyn SqlDialect, + mappings: &mut Mappings, + partition_by: &mut Vec, + ) -> Result { + if !needs_projection(projection) { + return Ok(query.to_string()); + } + + let columns = mappings.column_names(); + let (expanded, expanded_columns) = expand_segment_to_vertices(query, &columns); + + let segment_id_col = "__ggsql_segment_id__".to_string(); + partition_by.push(segment_id_col); + + let densified = densify_edges( + &expanded, + dialect, + &expanded_columns, + partition_by, + Some("__ggsql_vertex__"), + false, + 1.0, + 360, + ); + let projected = + project_position_columns(&densified, projection, dialect, &expanded_columns)?; + + mappings.insert_column("pos1end", "pos1"); + mappings.insert_column("pos2end", "pos2"); + + Ok(projected) + } +} + +/// Expand each segment row into 2 vertex rows (start + end). +/// +/// Input: one row per segment with pos1/pos2 (start) and pos1end/pos2end (end). +/// Output: two rows per segment with pos1/pos2 vertex positions and a +/// `__ggsql_segment_id__` grouping column. Material aesthetics pass through unchanged. +fn expand_segment_to_vertices(query: &str, columns: &[String]) -> (String, Vec) { + let pos1_col = naming::aesthetic_column("pos1"); + let pos2_col = naming::aesthetic_column("pos2"); + let pos1end_col = naming::aesthetic_column("pos1end"); + let pos2end_col = naming::aesthetic_column("pos2end"); + + let passthrough_cols: Vec<&String> = columns + .iter() + .filter(|c| *c != &pos1_col && *c != &pos2_col && *c != &pos1end_col && *c != &pos2end_col) + .collect(); + let passthrough: Vec = passthrough_cols + .iter() + .map(|c| naming::quote_ident(c)) + .collect(); + + let numbered = format!( + "SELECT *, ROW_NUMBER() OVER (ORDER BY (SELECT NULL)) \ + AS \"__ggsql_segment_id__\" FROM ({query})" + ); + + let vertices_table = "(SELECT 0 AS \"__ggsql_vertex__\" UNION ALL SELECT 1)"; + + let pos1_q = naming::quote_ident(&pos1_col); + let pos2_q = naming::quote_ident(&pos2_col); + let pos1end_q = naming::quote_ident(&pos1end_col); + let pos2end_q = naming::quote_ident(&pos2end_col); + + let mut select_parts: Vec = passthrough; + select_parts.push("\"__ggsql_segment_id__\"".to_string()); + select_parts.push("\"__ggsql_vertex__\"".to_string()); + select_parts.push(format!( + "CASE \"__ggsql_vertex__\" WHEN 0 THEN {pos1_q} WHEN 1 THEN {pos1end_q} END AS {pos1_q}" + )); + select_parts.push(format!( + "CASE \"__ggsql_vertex__\" WHEN 0 THEN {pos2_q} WHEN 1 THEN {pos2end_q} END AS {pos2_q}" + )); + + let sql = format!( + "SELECT {} FROM ({numbered}) \"__ggsql_seg__\" \ + CROSS JOIN {vertices_table} \"__ggsql_vertices__\"", + select_parts.join(", ") + ); + + let mut out_columns: Vec = passthrough_cols.into_iter().cloned().collect(); + out_columns.push("__ggsql_segment_id__".to_string()); + out_columns.push(pos1_col); + out_columns.push(pos2_col); + + (sql, out_columns) } impl std::fmt::Display for Segment { @@ -55,9 +152,21 @@ impl std::fmt::Display for Segment { #[cfg(test)] mod tests { + use super::Segment; + use crate::plot::layer::geom::GeomTrait; + use crate::plot::projection::Projection; + use crate::plot::types::ParameterValue; use crate::plot::{AestheticContext, AestheticValue, Geom, Layer}; + use crate::{naming, Mappings}; + + fn create_segment_mappings() -> Mappings { + let mut mappings = Mappings::new(); + for aes in &["pos1", "pos2", "pos1end", "pos2end"] { + mappings.insert_column(aes, aes); + } + mappings + } - /// Helper function to create a layer with given mappings and validate it fn validate_segment(mappings: &[(&str, &str)]) -> Result<(), String> { let mut layer = Layer::new(Geom::segment()); for (aesthetic, column) in mappings { @@ -96,4 +205,142 @@ mod tests { result.err() ); } + + #[test] + fn test_apply_projection_no_op_without_map() { + let segment = Segment; + let projection = Projection::cartesian(); + let mut mappings = create_segment_mappings(); + let mut partition_by = vec![]; + + let result = segment + .apply_projection( + "SELECT * FROM t", + &projection, + &crate::reader::AnsiDialect, + &mut mappings, + &mut partition_by, + ) + .unwrap(); + + assert_eq!(result, "SELECT * FROM t"); + assert!(partition_by.is_empty()); + } + + #[test] + fn test_apply_projection_expands_and_densifies() { + let segment = Segment; + let mut projection = Projection::map(); + projection.properties.insert( + "source".to_string(), + ParameterValue::String("EPSG:4326".to_string()), + ); + projection.properties.insert( + "target".to_string(), + ParameterValue::String("+proj=merc".to_string()), + ); + + let mut mappings = create_segment_mappings(); + let mut partition_by = vec![]; + + let result = segment + .apply_projection( + "SELECT * FROM t", + &projection, + &crate::reader::AnsiDialect, + &mut mappings, + &mut partition_by, + ) + .unwrap(); + + assert!(result.contains("__ggsql_segment_id__")); + assert!(result.contains("CROSS JOIN")); + assert!(result.contains("ST_Transform")); + assert!(partition_by.contains(&"__ggsql_segment_id__".to_string())); + // pos1end/pos2end remapped to pos1/pos2 columns + assert!(mappings.contains_key("pos1end")); + assert!(mappings.contains_key("pos2end")); + } + + #[cfg(feature = "duckdb")] + #[test] + fn test_densified_segment_produces_intermediate_vertices() { + use crate::reader::{DuckDBReader, Reader}; + use arrow::array::Array; + + let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap(); + let dialect = reader.dialect(); + + // A segment spanning 40° of longitude at 45°N + let input = format!( + "SELECT -80.0 AS \"{}\", 45.0 AS \"{}\", \ + -40.0 AS \"{}\", 45.0 AS \"{}\"", + naming::aesthetic_column("pos1"), + naming::aesthetic_column("pos2"), + naming::aesthetic_column("pos1end"), + naming::aesthetic_column("pos2end"), + ); + + let segment = Segment; + let mut projection = Projection::map(); + projection.properties.insert( + "source".to_string(), + ParameterValue::String("EPSG:4326".to_string()), + ); + projection.properties.insert( + "target".to_string(), + ParameterValue::String("+proj=ortho +lat_0=45 +lon_0=-60".to_string()), + ); + + let mut mappings = create_segment_mappings(); + let mut partition_by = vec![]; + + for stmt in dialect.sql_spatial_setup() { + reader.execute_sql(&stmt).unwrap(); + } + + let projected_sql = segment + .apply_projection( + &input, + &projection, + dialect, + &mut mappings, + &mut partition_by, + ) + .unwrap(); + + let df = reader.execute_sql(&projected_sql).unwrap(); + let n = df.inner().num_rows(); + assert!( + n > 2, + "expected densified vertices (more than start+end), got {n}" + ); + + let pos1_col = df + .inner() + .column_by_name(&naming::aesthetic_column("pos1")) + .unwrap() + .as_any() + .downcast_ref::() + .unwrap(); + let pos2_col = df + .inner() + .column_by_name(&naming::aesthetic_column("pos2")) + .unwrap() + .as_any() + .downcast_ref::() + .unwrap(); + + // Verify no NULLs in the projected positions + assert_eq!(pos1_col.null_count(), 0); + assert_eq!(pos2_col.null_count(), 0); + + // First and last vertex should differ (segment has distinct endpoints) + let first_x = pos1_col.value(0); + let last_x = pos1_col.value(n - 1); + assert!( + (first_x - last_x).abs() > 1e-6, + "endpoints should differ after projection" + ); + } } diff --git a/src/writer/vegalite/layer.rs b/src/writer/vegalite/layer.rs index d24a38b6a..b44a32be4 100644 --- a/src/writer/vegalite/layer.rs +++ b/src/writer/vegalite/layer.rs @@ -1560,6 +1560,48 @@ impl GeomRenderer for PolygonRenderer { } } +// ============================================================================= +// Segment Renderer +// ============================================================================= + +/// Renderer for segment geom — normally a rule (x/y/x2/y2), but when densified +/// under map projection the segment is expanded to a multi-row line. +pub struct SegmentRenderer; + +impl GeomRenderer for SegmentRenderer { + fn modify_spec( + &self, + layer_spec: &mut Value, + layer: &Layer, + context: &RenderContext, + ) -> Result<()> { + if layer + .partition_by + .contains(&"__ggsql_segment_id__".to_string()) + { + layer_spec["mark"] = json!({ + "type": "line", + "clip": true + }); + + if let Some(encoding) = layer_spec + .get_mut("encoding") + .and_then(|e| e.as_object_mut()) + { + let (_, pos1_end, _, _, pos2_end, _) = &context.channels; + encoding.remove(pos1_end.as_str()); + encoding.remove(pos2_end.as_str()); + encoding.insert( + "order".to_string(), + json!({"field": ROW_INDEX_COLUMN, "type": "quantitative"}), + ); + } + } + + Ok(()) + } +} + // ============================================================================= // Violin Renderer // ============================================================================= @@ -2490,8 +2532,9 @@ pub fn get_renderer(geom: &Geom) -> Box { GeomType::Text => Box::new(TextRenderer), GeomType::Range => Box::new(RangeRenderer), GeomType::Rule => Box::new(RuleRenderer), + GeomType::Segment => Box::new(SegmentRenderer), GeomType::Spatial => Box::new(SpatialRenderer), - // All other geoms (Point, Area, Ribbon, Density, Segment, etc.) use the default renderer + // All other geoms (Point, Area, Ribbon, Density, etc.) use the default renderer _ => Box::new(DefaultRenderer), } } From 23da84841cba3b71799f54829a3d99892d73ee79 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Thu, 11 Jun 2026 10:19:04 +0200 Subject: [PATCH 12/20] Implement edge densification for ribbon geom under map projection Co-Authored-By: Claude Opus 4.6 --- src/plot/layer/geom/ribbon.rs | 288 +++++++++++++++++++++++++++++++++- src/writer/vegalite/layer.rs | 45 +++++- 2 files changed, 330 insertions(+), 3 deletions(-) diff --git a/src/plot/layer/geom/ribbon.rs b/src/plot/layer/geom/ribbon.rs index 47f9bc26d..9abb668ed 100644 --- a/src/plot/layer/geom/ribbon.rs +++ b/src/plot/layer/geom/ribbon.rs @@ -2,10 +2,15 @@ use super::stat_aggregate; use super::types::{wrap_with_order_by, POSITION_VALUES}; -use super::{has_aggregate_param, DefaultAesthetics, GeomTrait, GeomType, StatResult}; +use super::{ + densify_edges, has_aggregate_param, needs_projection, project_position_columns, + DefaultAesthetics, GeomTrait, GeomType, StatResult, +}; +use crate::plot::projection::Projection; use crate::plot::types::DefaultAestheticValue; use crate::plot::{DefaultParamValue, ParamConstraint, ParamDefinition}; -use crate::Mappings; +use crate::reader::SqlDialect; +use crate::{naming, Mappings, Result}; /// Ribbon geom - confidence bands and ranges #[derive(Debug, Clone, Copy)] @@ -47,6 +52,44 @@ impl GeomTrait for Ribbon { Some(&["pos1"]) } + fn apply_projection( + &self, + query: &str, + projection: &Projection, + dialect: &dyn SqlDialect, + mappings: &mut Mappings, + partition_by: &mut Vec, + ) -> Result { + if !needs_projection(projection) { + return Ok(query.to_string()); + } + + let columns = mappings.column_names(); + let (expanded, expanded_columns) = expand_ribbon_to_polygon(query, &columns, partition_by); + + let ribbon_id_col = "__ggsql_ribbon_id__".to_string(); + partition_by.push(ribbon_id_col); + + let densified = densify_edges( + &expanded, + dialect, + &expanded_columns, + partition_by, + Some("__ggsql_vertex__"), + true, + 1.0, + 360, + ); + let projected = + project_position_columns(&densified, projection, dialect, &expanded_columns)?; + + mappings.insert_column("pos2", "pos2"); + mappings.insert_column("pos2min", "pos2"); + mappings.insert_column("pos2max", "pos2"); + + Ok(projected) + } + fn apply_stat_transform( &self, query: &str, @@ -78,8 +121,249 @@ impl GeomTrait for Ribbon { } } +/// Expand a ribbon (pos1, pos2min, pos2max) into a closed polygon outline. +/// +/// The outline traces the upper edge forward (pos1 ascending, pos2max) then +/// the lower edge backward (pos1 descending, pos2min). Each row produces two +/// vertices; the vertex index (`__ggsql_vertex__`) orders upper edge first, +/// then lower edge reversed. +fn expand_ribbon_to_polygon( + query: &str, + columns: &[String], + partition_by: &[String], +) -> (String, Vec) { + let pos1_col = naming::aesthetic_column("pos1"); + let pos2min_col = naming::aesthetic_column("pos2min"); + let pos2max_col = naming::aesthetic_column("pos2max"); + let pos2_col = naming::aesthetic_column("pos2"); + + let passthrough_cols: Vec<&String> = columns + .iter() + .filter(|c| *c != &pos1_col && *c != &pos2min_col && *c != &pos2max_col) + .collect(); + let passthrough_quoted: Vec = passthrough_cols + .iter() + .map(|c| naming::quote_ident(c)) + .collect(); + + let pos1_q = naming::quote_ident(&pos1_col); + let pos2min_q = naming::quote_ident(&pos2min_col); + let pos2max_q = naming::quote_ident(&pos2max_col); + let pos2_q = naming::quote_ident(&pos2_col); + + // Number rows within each group by pos1 order and compute the group size. + let partition_clause = if partition_by.is_empty() { + String::new() + } else { + let parts: Vec = partition_by + .iter() + .map(|c| naming::quote_ident(c)) + .collect(); + format!("PARTITION BY {} ", parts.join(", ")) + }; + + // ribbon_id: unique per partition group (DENSE_RANK over partition columns). + // When no partition columns exist, every row belongs to one ribbon → constant 1. + let ribbon_id_expr = if partition_by.is_empty() { + "1".to_string() + } else { + let parts: Vec = partition_by + .iter() + .map(|c| naming::quote_ident(c)) + .collect(); + format!("DENSE_RANK() OVER (ORDER BY {})", parts.join(", ")) + }; + + let numbered = format!( + "SELECT *, \ + ROW_NUMBER() OVER ({partition_clause}ORDER BY {pos1_q}) AS \"__ggsql_row_idx__\", \ + COUNT(*) OVER ({partition_clause}) AS \"__ggsql_n_rows__\", \ + {ribbon_id_expr} AS \"__ggsql_ribbon_id__\" \ + FROM ({query})" + ); + + // Build select list for each half + let mut common_select: Vec = passthrough_quoted.clone(); + common_select.push("\"__ggsql_ribbon_id__\"".to_string()); + + // Upper edge: vertex index = row_idx (1..n), pos2 = pos2max + let mut upper_parts = common_select.clone(); + upper_parts.push("\"__ggsql_row_idx__\" AS \"__ggsql_vertex__\"".to_string()); + upper_parts.push(pos1_q.to_string()); + upper_parts.push(format!("{pos2max_q} AS {pos2_q}")); + + // Lower edge: vertex index = 2*n - row_idx + 1 (n+1..2n), pos2 = pos2min + let mut lower_parts = common_select; + lower_parts.push( + "(2 * \"__ggsql_n_rows__\" - \"__ggsql_row_idx__\" + 1) AS \"__ggsql_vertex__\"" + .to_string(), + ); + lower_parts.push(pos1_q.to_string()); + lower_parts.push(format!("{pos2min_q} AS {pos2_q}")); + + let sql = format!( + "SELECT {} FROM ({numbered}) \"__ggsql_r__\" \ + UNION ALL \ + SELECT {} FROM ({numbered}) \"__ggsql_r__\"", + upper_parts.join(", "), + lower_parts.join(", "), + ); + + let mut out_columns: Vec = passthrough_cols.into_iter().cloned().collect(); + out_columns.push("__ggsql_ribbon_id__".to_string()); + out_columns.push(pos1_col); + out_columns.push(pos2_col); + + (sql, out_columns) +} + impl std::fmt::Display for Ribbon { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "ribbon") } } + +#[cfg(test)] +mod tests { + use super::Ribbon; + use crate::plot::layer::geom::GeomTrait; + use crate::plot::projection::Projection; + use crate::plot::types::ParameterValue; + use crate::{naming, Mappings}; + + fn create_ribbon_mappings() -> Mappings { + let mut mappings = Mappings::new(); + for aes in &["pos1", "pos2min", "pos2max"] { + mappings.insert_column(aes, aes); + } + mappings + } + + #[test] + fn test_apply_projection_no_op_without_map() { + let ribbon = Ribbon; + let projection = Projection::cartesian(); + let mut mappings = create_ribbon_mappings(); + let mut partition_by = vec![]; + + let result = ribbon + .apply_projection( + "SELECT * FROM t", + &projection, + &crate::reader::AnsiDialect, + &mut mappings, + &mut partition_by, + ) + .unwrap(); + + assert_eq!(result, "SELECT * FROM t"); + assert!(partition_by.is_empty()); + } + + #[test] + fn test_apply_projection_expands_to_polygon() { + let ribbon = Ribbon; + let mut projection = Projection::map(); + projection.properties.insert( + "source".to_string(), + ParameterValue::String("EPSG:4326".to_string()), + ); + projection.properties.insert( + "target".to_string(), + ParameterValue::String("+proj=merc".to_string()), + ); + + let mut mappings = create_ribbon_mappings(); + let mut partition_by = vec![]; + + let result = ribbon + .apply_projection( + "SELECT * FROM t", + &projection, + &crate::reader::AnsiDialect, + &mut mappings, + &mut partition_by, + ) + .unwrap(); + + assert!(result.contains("__ggsql_ribbon_id__")); + assert!(result.contains("UNION ALL")); + assert!(result.contains("ST_Transform")); + assert!(partition_by.contains(&"__ggsql_ribbon_id__".to_string())); + assert!(mappings.contains_key("pos2")); + } + + #[cfg(feature = "duckdb")] + #[test] + fn test_densified_ribbon_produces_closed_polygon() { + use crate::reader::{DuckDBReader, Reader}; + use arrow::array::Array; + + let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap(); + let dialect = reader.dialect(); + + // A ribbon spanning 60° of longitude with a 10° band width + let input = format!( + "SELECT * FROM (VALUES \ + (-90.0, 40.0, 50.0), \ + (-60.0, 42.0, 52.0), \ + (-30.0, 38.0, 48.0)) \ + AS t(\"{}\", \"{}\", \"{}\")", + naming::aesthetic_column("pos1"), + naming::aesthetic_column("pos2min"), + naming::aesthetic_column("pos2max"), + ); + + let ribbon = Ribbon; + let mut projection = Projection::map(); + projection.properties.insert( + "source".to_string(), + ParameterValue::String("EPSG:4326".to_string()), + ); + projection.properties.insert( + "target".to_string(), + ParameterValue::String("+proj=ortho +lat_0=45 +lon_0=-60".to_string()), + ); + + let mut mappings = create_ribbon_mappings(); + let mut partition_by = vec![]; + + for stmt in dialect.sql_spatial_setup() { + reader.execute_sql(&stmt).unwrap(); + } + + let projected_sql = ribbon + .apply_projection( + &input, + &projection, + dialect, + &mut mappings, + &mut partition_by, + ) + .unwrap(); + + let df = reader.execute_sql(&projected_sql).unwrap(); + let n = df.inner().num_rows(); + // 3 input rows → 6 polygon vertices (upper + lower) before densification, + // densification adds intermediate vertices along edges > 1° + assert!(n > 6, "expected densified polygon vertices, got {n}"); + + let pos1_col = df + .inner() + .column_by_name(&naming::aesthetic_column("pos1")) + .unwrap() + .as_any() + .downcast_ref::() + .unwrap(); + let pos2_col = df + .inner() + .column_by_name(&naming::aesthetic_column("pos2")) + .unwrap() + .as_any() + .downcast_ref::() + .unwrap(); + + assert_eq!(pos1_col.null_count(), 0); + assert_eq!(pos2_col.null_count(), 0); + } +} diff --git a/src/writer/vegalite/layer.rs b/src/writer/vegalite/layer.rs index b44a32be4..b64808f30 100644 --- a/src/writer/vegalite/layer.rs +++ b/src/writer/vegalite/layer.rs @@ -1602,6 +1602,48 @@ impl GeomRenderer for SegmentRenderer { } } +// ============================================================================= +// Ribbon Renderer +// ============================================================================= + +/// Renderer for ribbon geom — normally an area (x/y/y2), but when densified +/// under map projection the ribbon is expanded to a closed polygon. +pub struct RibbonRenderer; + +impl GeomRenderer for RibbonRenderer { + fn modify_spec( + &self, + layer_spec: &mut Value, + layer: &Layer, + context: &RenderContext, + ) -> Result<()> { + if layer + .partition_by + .contains(&"__ggsql_ribbon_id__".to_string()) + { + layer_spec["mark"] = json!({ + "type": "line", + "interpolate": "linear-closed" + }); + + if let Some(encoding) = layer_spec + .get_mut("encoding") + .and_then(|e| e.as_object_mut()) + { + let (_, pos1_end, _, _, pos2_end, _) = &context.channels; + encoding.remove(pos1_end.as_str()); + encoding.remove(pos2_end.as_str()); + encoding.insert( + "order".to_string(), + json!({"field": ROW_INDEX_COLUMN, "type": "quantitative"}), + ); + } + } + + Ok(()) + } +} + // ============================================================================= // Violin Renderer // ============================================================================= @@ -2532,9 +2574,10 @@ pub fn get_renderer(geom: &Geom) -> Box { GeomType::Text => Box::new(TextRenderer), GeomType::Range => Box::new(RangeRenderer), GeomType::Rule => Box::new(RuleRenderer), + GeomType::Ribbon => Box::new(RibbonRenderer), GeomType::Segment => Box::new(SegmentRenderer), GeomType::Spatial => Box::new(SpatialRenderer), - // All other geoms (Point, Area, Ribbon, Density, etc.) use the default renderer + // All other geoms (Point, Area, Density, etc.) use the default renderer _ => Box::new(DefaultRenderer), } } From a0d18b8ec23f61b27c530e084760dcdef071433b Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Fri, 12 Jun 2026 09:18:38 +0200 Subject: [PATCH 13/20] Implement edge densification for rule geom under map projection Rules with clipping expand into densified line segments: each rule is cross-joined to its clip-boundary extent, densified along the spanning axis, clipped to the boundary, then projected. The writer renders the result as a line mark ordered by row index. The apply_projection trait method now accepts a mutable parameters map so Rule can signal the orientation override downstream. Co-Authored-By: Claude Opus 4.6 --- src/plot/layer/geom/line.rs | 3 +- src/plot/layer/geom/mod.rs | 17 +- src/plot/layer/geom/path.rs | 1 + src/plot/layer/geom/point.rs | 1 + src/plot/layer/geom/polygon.rs | 1 + src/plot/layer/geom/ribbon.rs | 4 + src/plot/layer/geom/rule.rs | 448 ++++++++++++++++++++++++++++++- src/plot/layer/geom/segment.rs | 4 + src/plot/layer/geom/spatial.rs | 7 + src/plot/layer/geom/text.rs | 1 + src/plot/layer/geom/tile.rs | 13 +- src/plot/layer/mod.rs | 3 +- src/plot/projection/coord/map.rs | 1 + src/plot/projection/coord/mod.rs | 1 + src/writer/vegalite/layer.rs | 24 +- 15 files changed, 518 insertions(+), 11 deletions(-) diff --git a/src/plot/layer/geom/line.rs b/src/plot/layer/geom/line.rs index a86c80bc6..dfb56efea 100644 --- a/src/plot/layer/geom/line.rs +++ b/src/plot/layer/geom/line.rs @@ -88,6 +88,7 @@ impl GeomTrait for Line { dialect: &dyn SqlDialect, mappings: &mut Mappings, partition_by: &mut Vec, + _parameters: &mut std::collections::HashMap, ) -> Result { if !needs_projection(projection) { return Ok(query.to_string()); @@ -117,7 +118,6 @@ impl std::fmt::Display for Line { #[cfg(test)] mod tests { use super::*; - use crate::naming; use crate::plot::types::ParameterValue; use crate::reader::AnsiDialect; @@ -144,6 +144,7 @@ mod tests { &AnsiDialect, &mut mappings, &mut vec![], + &mut std::collections::HashMap::new(), ) .unwrap(); diff --git a/src/plot/layer/geom/mod.rs b/src/plot/layer/geom/mod.rs index 3ecb2776d..422db2ef9 100644 --- a/src/plot/layer/geom/mod.rs +++ b/src/plot/layer/geom/mod.rs @@ -154,6 +154,7 @@ pub trait GeomTrait: std::fmt::Debug + std::fmt::Display + Send + Sync { &self, _mappings: &crate::Mappings, _aesthetic_ctx: &Option, + _partition_by: &[String], ) -> std::result::Result<(), String> { Ok(()) } @@ -313,6 +314,7 @@ pub trait GeomTrait: std::fmt::Debug + std::fmt::Display + Send + Sync { _dialect: &dyn SqlDialect, _mappings: &mut Mappings, _partition_by: &mut Vec, + _parameters: &mut std::collections::HashMap, ) -> Result { Ok(query.to_string()) } @@ -848,9 +850,16 @@ impl Geom { dialect: &dyn SqlDialect, mappings: &mut Mappings, partition_by: &mut Vec, + parameters: &mut std::collections::HashMap, ) -> Result { - self.0 - .apply_projection(query, projection, dialect, mappings, partition_by) + self.0.apply_projection( + query, + projection, + dialect, + mappings, + partition_by, + parameters, + ) } /// Adjust layer mappings and parameters based on geom-specific logic @@ -884,8 +893,10 @@ impl Geom { &self, mappings: &Mappings, aesthetic_ctx: &Option, + partition_by: &[String], ) -> std::result::Result<(), String> { - self.0.validate_aesthetics(mappings, aesthetic_ctx) + self.0 + .validate_aesthetics(mappings, aesthetic_ctx, partition_by) } } diff --git a/src/plot/layer/geom/path.rs b/src/plot/layer/geom/path.rs index 8accb166c..f8bc2c4ca 100644 --- a/src/plot/layer/geom/path.rs +++ b/src/plot/layer/geom/path.rs @@ -48,6 +48,7 @@ impl GeomTrait for Path { dialect: &dyn SqlDialect, mappings: &mut Mappings, partition_by: &mut Vec, + _parameters: &mut std::collections::HashMap, ) -> Result { if !needs_projection(projection) { return Ok(query.to_string()); diff --git a/src/plot/layer/geom/point.rs b/src/plot/layer/geom/point.rs index 58d2b734c..1df12d2e7 100644 --- a/src/plot/layer/geom/point.rs +++ b/src/plot/layer/geom/point.rs @@ -62,6 +62,7 @@ impl GeomTrait for Point { dialect: &dyn SqlDialect, mappings: &mut Mappings, _partition_by: &mut Vec, + _parameters: &mut std::collections::HashMap, ) -> Result { let columns = mappings.column_names(); project_position_columns(query, projection, dialect, &columns) diff --git a/src/plot/layer/geom/polygon.rs b/src/plot/layer/geom/polygon.rs index 34db09ed1..7ed7da94b 100644 --- a/src/plot/layer/geom/polygon.rs +++ b/src/plot/layer/geom/polygon.rs @@ -49,6 +49,7 @@ impl GeomTrait for Polygon { dialect: &dyn SqlDialect, mappings: &mut Mappings, partition_by: &mut Vec, + _parameters: &mut std::collections::HashMap, ) -> Result { if !needs_projection(projection) { return Ok(query.to_string()); diff --git a/src/plot/layer/geom/ribbon.rs b/src/plot/layer/geom/ribbon.rs index 9abb668ed..3760a66cf 100644 --- a/src/plot/layer/geom/ribbon.rs +++ b/src/plot/layer/geom/ribbon.rs @@ -59,6 +59,7 @@ impl GeomTrait for Ribbon { dialect: &dyn SqlDialect, mappings: &mut Mappings, partition_by: &mut Vec, + _parameters: &mut std::collections::HashMap, ) -> Result { if !needs_projection(projection) { return Ok(query.to_string()); @@ -253,6 +254,7 @@ mod tests { &crate::reader::AnsiDialect, &mut mappings, &mut partition_by, + &mut std::collections::HashMap::new(), ) .unwrap(); @@ -283,6 +285,7 @@ mod tests { &crate::reader::AnsiDialect, &mut mappings, &mut partition_by, + &mut std::collections::HashMap::new(), ) .unwrap(); @@ -339,6 +342,7 @@ mod tests { dialect, &mut mappings, &mut partition_by, + &mut std::collections::HashMap::new(), ) .unwrap(); diff --git a/src/plot/layer/geom/rule.rs b/src/plot/layer/geom/rule.rs index 90f7c4e9e..eafb459ec 100644 --- a/src/plot/layer/geom/rule.rs +++ b/src/plot/layer/geom/rule.rs @@ -1,7 +1,14 @@ //! Rule geom implementation -use super::{DefaultAesthetics, GeomTrait, GeomType, ParamDefinition}; -use crate::plot::types::DefaultAestheticValue; +use super::{ + densify_edges, needs_projection, project_position_columns, DefaultAesthetics, GeomTrait, + GeomType, ParamDefinition, +}; +use crate::plot::projection::coord::map::clip_boundary_table; +use crate::plot::projection::Projection; +use crate::plot::types::{DefaultAestheticValue, ParameterValue}; +use crate::reader::SqlDialect; +use crate::{naming, Mappings, Result}; /// Rule geom - horizontal and vertical reference lines #[derive(Debug, Clone, Copy)] @@ -34,12 +41,92 @@ impl GeomTrait for Rule { Some(&[]) } + fn apply_projection( + &self, + query: &str, + projection: &Projection, + dialect: &dyn SqlDialect, + mappings: &mut Mappings, + partition_by: &mut Vec, + parameters: &mut std::collections::HashMap, + ) -> Result { + if !needs_projection(projection) { + return Ok(query.to_string()); + } + + // Rules only densify when clipping is active (clip boundary table exists) + let clip_active = matches!( + projection.properties.get("clip"), + Some(ParameterValue::Boolean(true)) + ); + // The rule input always has one position column named __ggsql_aes_pos1__ + // regardless of whether the aesthetic key is "pos1" or "pos2" — the executor + // normalizes it to the pos1 slot. + let columns = vec![naming::aesthetic_column("pos1")]; + if !clip_active { + return project_position_columns(query, projection, dialect, &columns); + } + + let has_pos1 = mappings.contains_key("pos1"); + let (expanded, expanded_columns) = + expand_rule_to_segment(query, &columns, has_pos1, dialect); + + let rule_id_col = "__ggsql_rule_id__".to_string(); + partition_by.push(rule_id_col); + + let densified = densify_edges( + &expanded, + dialect, + &expanded_columns, + partition_by, + Some("__ggsql_vertex__"), + false, + 1.0, + 360, + ); + // Clip densified vertices to the clip boundary before projecting. + let pos1_q = naming::quote_ident(&naming::aesthetic_column("pos1")); + let pos2_q = naming::quote_ident(&naming::aesthetic_column("pos2")); + let clip_table = clip_boundary_table(); + let clipped = format!( + "SELECT * FROM ({densified}) WHERE ST_Contains(\ + (SELECT geom FROM {clip_table}), ST_Point({pos1_q}, {pos2_q}))" + ); + + let projected = project_position_columns(&clipped, projection, dialect, &expanded_columns)?; + + // Both pos1 and pos2 columns exist in the output SQL (the spanning axis + // was synthesized). Add the new axis to mappings so the writer encodes it. + if !has_pos1 { + mappings.aesthetics.remove("pos2"); + mappings.insert_column("pos1", "pos1"); + mappings.insert_column("pos2", "pos2"); + } else { + mappings.insert_column("pos2", "pos2"); + } + + // After densification both axes are explicit — disable the orientation flip + // that would otherwise swap the DataFrame columns. + parameters.insert( + "orientation".to_string(), + ParameterValue::String("aligned".to_string()), + ); + + Ok(projected) + } + fn validate_aesthetics( &self, mappings: &crate::Mappings, aesthetic_ctx: &Option, + partition_by: &[String], ) -> std::result::Result<(), String> { - // Rule requires exactly one of pos1 or pos2 (XOR logic) + // Rule requires exactly one of pos1 or pos2 (XOR logic). + // After densification both axes are present — skip the check. + if partition_by.contains(&"__ggsql_rule_id__".to_string()) { + return Ok(()); + } + let has_pos1 = mappings.contains_key("pos1"); let has_pos2 = mappings.contains_key("pos2"); @@ -116,6 +203,82 @@ impl GeomTrait for Rule { } } +/// Expand each rule into 2 vertex rows (start + end of the spanning axis). +/// +/// The input always has a single position column `__ggsql_aes_pos1__` (the +/// executor normalizes the rule's fixed axis into the pos1 slot). `has_pos1` +/// tells us the semantic meaning: +/// - true (vertical rule): pos1 is longitude (fixed), synthesize pos2 from bbox y. +/// - false (horizontal rule): pos1 is latitude (fixed), rename to pos2, synthesize +/// pos1 from bbox x. +fn expand_rule_to_segment( + query: &str, + columns: &[String], + has_pos1: bool, + dialect: &dyn SqlDialect, +) -> (String, Vec) { + let pos1_col = naming::aesthetic_column("pos1"); + let pos2_col = naming::aesthetic_column("pos2"); + let pos1_q = naming::quote_ident(&pos1_col); + let pos2_q = naming::quote_ident(&pos2_col); + + let boundary_table = clip_boundary_table(); + let bbox_expr = dialect.sql_geometry_bbox("geom", &boundary_table); + + // The input column is always __ggsql_aes_pos1__. Build the SELECT + // expressions that produce both pos1 and pos2 in the output. + let (fixed_expr, span_expr) = if has_pos1 { + // Vertical rule: input pos1 = longitude (keep as pos1), synthesize pos2 from y-extent + let fixed = pos1_q.clone(); + let span = format!( + "CASE \"__ggsql_vertex__\" WHEN 0 THEN (SELECT ymin FROM ({bbox_expr})) \ + WHEN 1 THEN (SELECT ymax FROM ({bbox_expr})) END AS {pos2_q}" + ); + (fixed, span) + } else { + // Horizontal rule: input pos1 = latitude (rename to pos2), synthesize pos1 from x-extent + let fixed = format!("{pos1_q} AS {pos2_q}"); + let span = format!( + "CASE \"__ggsql_vertex__\" WHEN 0 THEN (SELECT xmin FROM ({bbox_expr})) \ + WHEN 1 THEN (SELECT xmax FROM ({bbox_expr})) END AS {pos1_q}" + ); + (fixed, span) + }; + + // Passthrough: columns minus the input pos1 column (we handle it explicitly above) + let passthrough_cols: Vec<&String> = columns.iter().filter(|c| *c != &pos1_col).collect(); + let passthrough_quoted: Vec = passthrough_cols + .iter() + .map(|c| naming::quote_ident(c)) + .collect(); + + let numbered = format!( + "SELECT *, ROW_NUMBER() OVER (ORDER BY (SELECT NULL)) \ + AS \"__ggsql_rule_id__\" FROM ({query})" + ); + + let vertices_table = "(SELECT 0 AS \"__ggsql_vertex__\" UNION ALL SELECT 1)"; + + let mut select_parts: Vec = passthrough_quoted; + select_parts.push("\"__ggsql_rule_id__\"".to_string()); + select_parts.push("\"__ggsql_vertex__\"".to_string()); + select_parts.push(fixed_expr); + select_parts.push(span_expr); + + let sql = format!( + "SELECT {} FROM ({numbered}) \"__ggsql_rule__\" \ + CROSS JOIN {vertices_table} \"__ggsql_vertices__\"", + select_parts.join(", ") + ); + + let mut out_columns: Vec = passthrough_cols.into_iter().cloned().collect(); + out_columns.push("__ggsql_rule_id__".to_string()); + out_columns.push(pos1_col); + out_columns.push(pos2_col); + + (sql, out_columns) +} + impl std::fmt::Display for Rule { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "rule") @@ -124,9 +287,13 @@ impl std::fmt::Display for Rule { #[cfg(test)] mod tests { + use super::{expand_rule_to_segment, Rule}; + use crate::plot::layer::geom::{densify_edges, GeomTrait}; + use crate::plot::projection::Projection; + use crate::plot::types::ParameterValue; use crate::plot::{AestheticContext, AestheticValue, Geom, Layer}; + use crate::{naming, Mappings}; - /// Helper function to create a layer with given mappings and validate it fn validate_rule(mappings: &[(&str, &str)]) -> Result<(), String> { let mut layer = Layer::new(Geom::rule()); for (aesthetic, column) in mappings { @@ -173,4 +340,277 @@ mod tests { result.err() ); } + + #[test] + fn test_apply_projection_no_op_without_map() { + let rule = Rule; + let projection = Projection::cartesian(); + let mut mappings = Mappings::new(); + mappings.insert_column("pos1", "pos1"); + let mut partition_by = vec![]; + + let result = rule + .apply_projection( + "SELECT * FROM t", + &projection, + &crate::reader::AnsiDialect, + &mut mappings, + &mut partition_by, + &mut std::collections::HashMap::new(), + ) + .unwrap(); + + assert_eq!(result, "SELECT * FROM t"); + assert!(partition_by.is_empty()); + } + + #[test] + fn test_apply_projection_without_clip_only_projects() { + let rule = Rule; + let mut projection = Projection::map(); + projection.properties.insert( + "source".to_string(), + ParameterValue::String("EPSG:4326".to_string()), + ); + projection.properties.insert( + "target".to_string(), + ParameterValue::String("+proj=merc".to_string()), + ); + projection + .properties + .insert("clip".to_string(), ParameterValue::Boolean(false)); + + let mut mappings = Mappings::new(); + mappings.insert_column("pos1", "pos1"); + let mut partition_by = vec![]; + + let result = rule + .apply_projection( + "SELECT * FROM t", + &projection, + &crate::reader::AnsiDialect, + &mut mappings, + &mut partition_by, + &mut std::collections::HashMap::new(), + ) + .unwrap(); + + assert!(result.contains("ST_Transform")); + assert!(!result.contains("__ggsql_rule_id__")); + assert!(partition_by.is_empty()); + } + + #[test] + fn test_apply_projection_with_clip_expands_and_densifies() { + let rule = Rule; + let mut projection = Projection::map(); + projection.properties.insert( + "source".to_string(), + ParameterValue::String("EPSG:4326".to_string()), + ); + projection.properties.insert( + "target".to_string(), + ParameterValue::String("+proj=merc".to_string()), + ); + projection + .properties + .insert("clip".to_string(), ParameterValue::Boolean(true)); + + let mut mappings = Mappings::new(); + mappings.insert_column("pos1", "pos1"); + let mut partition_by = vec![]; + + let result = rule + .apply_projection( + "SELECT * FROM t", + &projection, + &crate::reader::AnsiDialect, + &mut mappings, + &mut partition_by, + &mut std::collections::HashMap::new(), + ) + .unwrap(); + + assert!(result.contains("__ggsql_rule_id__")); + assert!(result.contains("CROSS JOIN")); + assert!(result.contains("ST_Transform")); + assert!(partition_by.contains(&"__ggsql_rule_id__".to_string())); + // pos2 should be added to mappings (the spanning axis) + assert!(mappings.contains_key("pos2")); + } + + #[cfg(feature = "duckdb")] + #[test] + fn test_densified_rule_produces_intermediate_vertices() { + use crate::plot::projection::coord::map::clip_boundary_table; + use crate::reader::{DuckDBReader, Reader}; + use arrow::array::Array; + + let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap(); + let dialect = reader.dialect(); + + for stmt in dialect.sql_spatial_setup() { + reader.execute_sql(&stmt).unwrap(); + } + + // Create a clip boundary table simulating an orthographic hemisphere + let boundary_table = clip_boundary_table(); + let create_sql = format!( + "CREATE TEMP TABLE \"{boundary_table}\" AS \ + SELECT ST_GeomFromText(\ + 'POLYGON ((-90 -60, 90 -60, 90 60, -90 60, -90 -60))'\ + ) AS geom" + ); + reader.execute_sql(&create_sql).unwrap(); + + // A vertical rule at lon = -30 + let input = format!("SELECT -30.0 AS \"{}\"", naming::aesthetic_column("pos1"),); + + let rule = Rule; + let mut projection = Projection::map(); + projection.properties.insert( + "source".to_string(), + ParameterValue::String("EPSG:4326".to_string()), + ); + projection.properties.insert( + "target".to_string(), + ParameterValue::String("+proj=ortho +lat_0=0 +lon_0=0".to_string()), + ); + projection + .properties + .insert("clip".to_string(), ParameterValue::Boolean(true)); + + let mut mappings = Mappings::new(); + mappings.insert_column("pos1", "pos1"); + let mut partition_by = vec![]; + + let projected_sql = rule + .apply_projection( + &input, + &projection, + dialect, + &mut mappings, + &mut partition_by, + &mut std::collections::HashMap::new(), + ) + .unwrap(); + + let df = reader.execute_sql(&projected_sql).unwrap(); + let n = df.inner().num_rows(); + assert!( + n > 2, + "expected densified vertices (more than start+end), got {n}" + ); + + let pos1_col = df + .inner() + .column_by_name(&naming::aesthetic_column("pos1")) + .unwrap() + .as_any() + .downcast_ref::() + .unwrap(); + let pos2_col = df + .inner() + .column_by_name(&naming::aesthetic_column("pos2")) + .unwrap() + .as_any() + .downcast_ref::() + .unwrap(); + + assert_eq!(pos1_col.null_count(), 0); + assert_eq!(pos2_col.null_count(), 0); + } + + #[cfg(feature = "duckdb")] + #[test] + fn test_densified_horizontal_rule_keeps_latitude_constant() { + use crate::plot::projection::coord::map::clip_boundary_table; + use crate::reader::{DuckDBReader, Reader}; + + let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap(); + let dialect = reader.dialect(); + + for stmt in dialect.sql_spatial_setup() { + reader.execute_sql(&stmt).unwrap(); + } + + // Create a clip boundary table (simple rectangle in EPSG:4326) + let boundary_table = clip_boundary_table(); + let create_sql = format!( + "CREATE TEMP TABLE \"{boundary_table}\" AS \ + SELECT ST_GeomFromText(\ + 'POLYGON ((-90 -60, 90 -60, 90 60, -90 60, -90 -60))'\ + ) AS geom" + ); + reader.execute_sql(&create_sql).unwrap(); + + // A horizontal rule at lat = 20 (pos2 mapped, not pos1) + let input = format!("SELECT 20.0 AS \"{}\"", naming::aesthetic_column("pos1"),); + + let rule = Rule; + let mut projection = Projection::map(); + projection.properties.insert( + "source".to_string(), + ParameterValue::String("EPSG:4326".to_string()), + ); + projection.properties.insert( + "target".to_string(), + ParameterValue::String("+proj=ortho +lat_0=0 +lon_0=0".to_string()), + ); + projection + .properties + .insert("clip".to_string(), ParameterValue::Boolean(true)); + + // Horizontal rule: pos2 is mapped (latitude is the fixed axis) + let mut mappings = Mappings::new(); + mappings.insert_column("pos2", "pos2"); + let mut partition_by = vec![]; + + rule.apply_projection( + &input, + &projection, + dialect, + &mut mappings, + &mut partition_by, + &mut std::collections::HashMap::new(), + ) + .unwrap(); + + // Run expand + densify manually to verify latitude stays constant pre-projection. + let columns = vec![naming::aesthetic_column("pos1")]; + let has_pos1 = false; + let (expanded, expanded_columns) = + expand_rule_to_segment(&input, &columns, has_pos1, dialect); + let densified = densify_edges( + &expanded, + dialect, + &expanded_columns, + &["__ggsql_rule_id__".to_string()], + Some("__ggsql_vertex__"), + false, + 1.0, + 360, + ); + + let df = reader.execute_sql(&densified).unwrap(); + let n = df.inner().num_rows(); + assert!(n > 2, "expected densified vertices, got {n}"); + + let pos2_col = df + .inner() + .column_by_name(&naming::aesthetic_column("pos2")) + .unwrap() + .as_any() + .downcast_ref::() + .unwrap(); + + // All pos2 (latitude) values should be exactly 20.0 + for i in 0..n { + let val = pos2_col.value(i); + assert!( + (val - 20.0).abs() < 1e-10, + "row {i}: expected pos2=20.0, got {val}" + ); + } + } } diff --git a/src/plot/layer/geom/segment.rs b/src/plot/layer/geom/segment.rs index 81f06f024..14799c958 100644 --- a/src/plot/layer/geom/segment.rs +++ b/src/plot/layer/geom/segment.rs @@ -57,6 +57,7 @@ impl GeomTrait for Segment { dialect: &dyn SqlDialect, mappings: &mut Mappings, partition_by: &mut Vec, + _parameters: &mut std::collections::HashMap, ) -> Result { if !needs_projection(projection) { return Ok(query.to_string()); @@ -220,6 +221,7 @@ mod tests { &crate::reader::AnsiDialect, &mut mappings, &mut partition_by, + &mut std::collections::HashMap::new(), ) .unwrap(); @@ -250,6 +252,7 @@ mod tests { &crate::reader::AnsiDialect, &mut mappings, &mut partition_by, + &mut std::collections::HashMap::new(), ) .unwrap(); @@ -306,6 +309,7 @@ mod tests { dialect, &mut mappings, &mut partition_by, + &mut std::collections::HashMap::new(), ) .unwrap(); diff --git a/src/plot/layer/geom/spatial.rs b/src/plot/layer/geom/spatial.rs index ca0eb7537..13dcc4aae 100644 --- a/src/plot/layer/geom/spatial.rs +++ b/src/plot/layer/geom/spatial.rs @@ -78,6 +78,7 @@ impl GeomTrait for Spatial { dialect: &dyn SqlDialect, mappings: &mut Mappings, _partition_by: &mut Vec, + _parameters: &mut std::collections::HashMap, ) -> crate::Result { let columns = mappings.column_names(); let col = naming::quote_ident(&naming::aesthetic_column("geometry")); @@ -148,6 +149,7 @@ mod tests { &AnsiDialect, &mut Mappings::new(), &mut vec![], + &mut std::collections::HashMap::new(), ) .unwrap(); @@ -166,6 +168,7 @@ mod tests { &AnsiDialect, &mut Mappings::new(), &mut vec![], + &mut std::collections::HashMap::new(), ) .unwrap(); @@ -189,6 +192,7 @@ mod tests { &AnsiDialect, &mut Mappings::new(), &mut vec![], + &mut std::collections::HashMap::new(), ) .unwrap(); @@ -217,6 +221,7 @@ mod tests { &AnsiDialect, &mut Mappings::new(), &mut vec![], + &mut std::collections::HashMap::new(), ) .unwrap(); @@ -243,6 +248,7 @@ mod tests { &AnsiDialect, &mut Mappings::new(), &mut vec![], + &mut std::collections::HashMap::new(), ) .unwrap(); @@ -271,6 +277,7 @@ mod tests { &AnsiDialect, &mut Mappings::new(), &mut vec![], + &mut std::collections::HashMap::new(), ) .unwrap(); diff --git a/src/plot/layer/geom/text.rs b/src/plot/layer/geom/text.rs index d103f9134..f262af30c 100644 --- a/src/plot/layer/geom/text.rs +++ b/src/plot/layer/geom/text.rs @@ -77,6 +77,7 @@ impl GeomTrait for Text { dialect: &dyn SqlDialect, mappings: &mut Mappings, _partition_by: &mut Vec, + _parameters: &mut std::collections::HashMap, ) -> Result { let columns = mappings.column_names(); project_position_columns(query, projection, dialect, &columns) diff --git a/src/plot/layer/geom/tile.rs b/src/plot/layer/geom/tile.rs index 9284d0b65..d389b21c8 100644 --- a/src/plot/layer/geom/tile.rs +++ b/src/plot/layer/geom/tile.rs @@ -216,6 +216,7 @@ impl GeomTrait for Tile { dialect: &dyn SqlDialect, mappings: &mut Mappings, partition_by: &mut Vec, + _parameters: &mut std::collections::HashMap, ) -> Result { if !needs_projection(projection) { return Ok(query.to_string()); @@ -1406,6 +1407,7 @@ mod tests { &crate::reader::AnsiDialect, &mut mappings, &mut vec![], + &mut std::collections::HashMap::new(), ) .unwrap(); @@ -1434,6 +1436,7 @@ mod tests { &crate::reader::AnsiDialect, &mut mappings, &mut vec![], + &mut std::collections::HashMap::new(), ) .unwrap(); @@ -1486,7 +1489,14 @@ mod tests { } let projected_sql = tile - .apply_projection(input, &projection, dialect, &mut mappings, &mut vec![]) + .apply_projection( + input, + &projection, + dialect, + &mut mappings, + &mut vec![], + &mut std::collections::HashMap::new(), + ) .unwrap(); let df = reader.execute_sql(&projected_sql).unwrap(); @@ -1551,6 +1561,7 @@ mod tests { &crate::reader::AnsiDialect, &mut mappings, &mut vec![], + &mut std::collections::HashMap::new(), ) .unwrap(); diff --git a/src/plot/layer/mod.rs b/src/plot/layer/mod.rs index 47f1e7aaf..5284f7b26 100644 --- a/src/plot/layer/mod.rs +++ b/src/plot/layer/mod.rs @@ -305,7 +305,8 @@ impl Layer { } // Call geom-specific validation (e.g., XOR constraints for Rule) - self.geom.validate_aesthetics(&self.mappings, context)?; + self.geom + .validate_aesthetics(&self.mappings, context, &self.partition_by)?; Ok(()) } diff --git a/src/plot/projection/coord/map.rs b/src/plot/projection/coord/map.rs index aae93ccaa..c395217ae 100644 --- a/src/plot/projection/coord/map.rs +++ b/src/plot/projection/coord/map.rs @@ -67,6 +67,7 @@ pub(crate) fn apply_map_transforms( dialect, &mut layer.mappings, &mut layer.partition_by, + &mut layer.parameters, )?; } diff --git a/src/plot/projection/coord/mod.rs b/src/plot/projection/coord/mod.rs index 7f6e6c576..4ec2d35e9 100644 --- a/src/plot/projection/coord/mod.rs +++ b/src/plot/projection/coord/mod.rs @@ -157,6 +157,7 @@ pub trait CoordTrait: std::fmt::Debug + Send + Sync { dialect, &mut layer.mappings, &mut layer.partition_by, + &mut layer.parameters, )?; } Ok(()) diff --git a/src/writer/vegalite/layer.rs b/src/writer/vegalite/layer.rs index b64808f30..4a28894fa 100644 --- a/src/writer/vegalite/layer.rs +++ b/src/writer/vegalite/layer.rs @@ -710,7 +710,6 @@ impl GeomRenderer for RuleRenderer { // Regular horizontal/vertical rule - no special rendering needed return Ok(()); } - // Use layer's pre-computed orientation let (pos1, pos1_end, _, pos2, pos2_end, _) = &context.channels; let (primary, primary2, secondary, secondary2, extent_aes) = if is_transposed(layer) { @@ -759,6 +758,29 @@ impl GeomRenderer for RuleRenderer { layer: &Layer, context: &RenderContext, ) -> Result<()> { + // Densified rule: expanded to a multi-row line under map projection + if layer + .partition_by + .contains(&"__ggsql_rule_id__".to_string()) + { + layer_spec["mark"] = json!({ + "type": "line", + "clip": true + }); + + if let Some(encoding) = layer_spec + .get_mut("encoding") + .and_then(|e| e.as_object_mut()) + { + encoding.insert( + "order".to_string(), + json!({"field": ROW_INDEX_COLUMN, "type": "quantitative"}), + ); + } + + return Ok(()); + } + // Determine slope expression: either a literal value or a field reference let slope_expr = match layer.mappings.get("slope") { Some(AestheticValue::Literal(ParameterValue::Number(n))) if *n == 0.0 => { From c7438a65036a4ed5e7150c5140e247ee9b26490f Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Fri, 12 Jun 2026 10:00:26 +0200 Subject: [PATCH 14/20] block other geoms that have not implemented densification --- src/plot/layer/geom/mod.rs | 39 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/src/plot/layer/geom/mod.rs b/src/plot/layer/geom/mod.rs index 422db2ef9..b5919b647 100644 --- a/src/plot/layer/geom/mod.rs +++ b/src/plot/layer/geom/mod.rs @@ -306,16 +306,22 @@ pub trait GeomTrait: std::fmt::Debug + std::fmt::Display + Send + Sync { /// (e.g. tile adds `__ggsql_poly_id__`) push them here so they survive /// downstream pruning. /// - /// The default is a no-op (returns query unchanged). + /// The default returns an error for unsupported geoms under map projection. fn apply_projection( &self, query: &str, - _projection: &Projection, + projection: &Projection, _dialect: &dyn SqlDialect, _mappings: &mut Mappings, _partition_by: &mut Vec, _parameters: &mut std::collections::HashMap, ) -> Result { + if needs_projection(projection) { + return Err(crate::GgsqlError::ValidationError(format!( + "Layer '{}' is not supported under map projection.", + self.geom_type() + ))); + } Ok(query.to_string()) } @@ -1122,6 +1128,35 @@ mod tests { assert!(needs_projection(&projection)); } + #[test] + fn test_apply_projection_default_errors_for_unsupported_geom() { + let mut projection = Projection::map(); + projection.properties.insert( + "source".to_string(), + ParameterValue::String("EPSG:4326".to_string()), + ); + projection.properties.insert( + "target".to_string(), + ParameterValue::String("+proj=ortho".to_string()), + ); + + let geom = Geom::bar(); + let result = geom.apply_projection( + "SELECT * FROM t", + &projection, + &crate::reader::AnsiDialect, + &mut Mappings::new(), + &mut vec![], + &mut std::collections::HashMap::new(), + ); + + let err = result.unwrap_err(); + assert_eq!( + err.to_string(), + "Validation error: Layer 'bar' is not supported under map projection." + ); + } + #[test] fn test_densify_edges_basic_structure() { use crate::reader::AnsiDialect; From 086dd9a1e7c1c4c464e3418aa21d365a53edb1b6 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Fri, 12 Jun 2026 10:06:44 +0200 Subject: [PATCH 15/20] add comment --- src/plot/layer/geom/mod.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/plot/layer/geom/mod.rs b/src/plot/layer/geom/mod.rs index b5919b647..7caca75e2 100644 --- a/src/plot/layer/geom/mod.rs +++ b/src/plot/layer/geom/mod.rs @@ -448,6 +448,9 @@ pub(crate) fn needs_projection(projection: &Projection) -> bool { /// row index is used (path/polygon). /// - `close_ring`: when true, the last vertex connects back to the first (polygon). /// - `segment_length`: target edge length after subdivision (in position units). +/// Callers pass 1.0 assuming geographic (lon/lat) source coordinates. For +/// projected sources this over-densifies, but `n_segments` caps the vertex +/// count per edge so the cost stays bounded. /// - `n_segments`: size of the integer series (must be at least as large as the /// maximum number of subdivisions any single edge can produce). #[allow(clippy::too_many_arguments)] From 63b270a03ffa11bd95e31b4182468e07ebcee441 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Fri, 12 Jun 2026 10:41:05 +0200 Subject: [PATCH 16/20] Unify densification group ID column into naming::DENSIFY_ID_COLUMN All densifiable geoms (tile, segment, ribbon, rule) and the writer now reference a single constant instead of per-geom magic strings. The SQL column is uniformly __ggsql_densify_id__. Co-Authored-By: Claude Opus 4.6 --- src/naming.rs | 5 +++++ src/plot/layer/geom/mod.rs | 2 +- src/plot/layer/geom/ribbon.rs | 15 ++++++++------- src/plot/layer/geom/rule.rs | 21 +++++++++++---------- src/plot/layer/geom/segment.rs | 17 +++++++++-------- src/plot/layer/geom/tile.rs | 21 +++++++++++---------- src/writer/vegalite/layer.rs | 8 ++++---- 7 files changed, 49 insertions(+), 40 deletions(-) diff --git a/src/naming.rs b/src/naming.rs index 46ce8f8fb..6259f5bd3 100644 --- a/src/naming.rs +++ b/src/naming.rs @@ -88,6 +88,11 @@ pub const SOURCE_COLUMN: &str = concatcp!(GGSQL_PREFIX, "source", GGSQL_SUFFIX); /// Alias for schema extraction queries pub const SCHEMA_ALIAS: &str = concatcp!(GGSQL_SUFFIX, "schema", GGSQL_SUFFIX); +/// Column name for densification group ID. +/// Used by geoms that expand geometry (tile, segment, ribbon, rule) to group +/// densified vertices back into their original feature. +pub const DENSIFY_ID_COLUMN: &str = concatcp!(GGSQL_PREFIX, "densify_id", GGSQL_SUFFIX); + // ============================================================================ // Constructor Functions // ============================================================================ diff --git a/src/plot/layer/geom/mod.rs b/src/plot/layer/geom/mod.rs index 7caca75e2..2f7d1c6eb 100644 --- a/src/plot/layer/geom/mod.rs +++ b/src/plot/layer/geom/mod.rs @@ -303,7 +303,7 @@ pub trait GeomTrait: std::fmt::Debug + std::fmt::Display + Send + Sync { /// `columns` lists all column names in the query (for portable column /// replacement on backends that don't support `SELECT * REPLACE`). /// `partition_by` is mutable: geoms that introduce new grouping columns - /// (e.g. tile adds `__ggsql_poly_id__`) push them here so they survive + /// (e.g. tile adds `DENSIFY_ID_COLUMN`) push them here so they survive /// downstream pruning. /// /// The default returns an error for unsupported geoms under map projection. diff --git a/src/plot/layer/geom/ribbon.rs b/src/plot/layer/geom/ribbon.rs index 3760a66cf..5b2eb87af 100644 --- a/src/plot/layer/geom/ribbon.rs +++ b/src/plot/layer/geom/ribbon.rs @@ -68,8 +68,7 @@ impl GeomTrait for Ribbon { let columns = mappings.column_names(); let (expanded, expanded_columns) = expand_ribbon_to_polygon(query, &columns, partition_by); - let ribbon_id_col = "__ggsql_ribbon_id__".to_string(); - partition_by.push(ribbon_id_col); + partition_by.push(naming::DENSIFY_ID_COLUMN.to_string()); let densified = densify_edges( &expanded, @@ -175,17 +174,19 @@ fn expand_ribbon_to_polygon( format!("DENSE_RANK() OVER (ORDER BY {})", parts.join(", ")) }; + let densify_id_q = naming::quote_ident(naming::DENSIFY_ID_COLUMN); + let numbered = format!( "SELECT *, \ ROW_NUMBER() OVER ({partition_clause}ORDER BY {pos1_q}) AS \"__ggsql_row_idx__\", \ COUNT(*) OVER ({partition_clause}) AS \"__ggsql_n_rows__\", \ - {ribbon_id_expr} AS \"__ggsql_ribbon_id__\" \ + {ribbon_id_expr} AS {densify_id_q} \ FROM ({query})" ); // Build select list for each half let mut common_select: Vec = passthrough_quoted.clone(); - common_select.push("\"__ggsql_ribbon_id__\"".to_string()); + common_select.push(densify_id_q.to_string()); // Upper edge: vertex index = row_idx (1..n), pos2 = pos2max let mut upper_parts = common_select.clone(); @@ -211,7 +212,7 @@ fn expand_ribbon_to_polygon( ); let mut out_columns: Vec = passthrough_cols.into_iter().cloned().collect(); - out_columns.push("__ggsql_ribbon_id__".to_string()); + out_columns.push(naming::DENSIFY_ID_COLUMN.to_string()); out_columns.push(pos1_col); out_columns.push(pos2_col); @@ -289,10 +290,10 @@ mod tests { ) .unwrap(); - assert!(result.contains("__ggsql_ribbon_id__")); + assert!(result.contains(naming::DENSIFY_ID_COLUMN)); assert!(result.contains("UNION ALL")); assert!(result.contains("ST_Transform")); - assert!(partition_by.contains(&"__ggsql_ribbon_id__".to_string())); + assert!(partition_by.contains(&naming::DENSIFY_ID_COLUMN.to_string())); assert!(mappings.contains_key("pos2")); } diff --git a/src/plot/layer/geom/rule.rs b/src/plot/layer/geom/rule.rs index eafb459ec..015291693 100644 --- a/src/plot/layer/geom/rule.rs +++ b/src/plot/layer/geom/rule.rs @@ -71,8 +71,7 @@ impl GeomTrait for Rule { let (expanded, expanded_columns) = expand_rule_to_segment(query, &columns, has_pos1, dialect); - let rule_id_col = "__ggsql_rule_id__".to_string(); - partition_by.push(rule_id_col); + partition_by.push(naming::DENSIFY_ID_COLUMN.to_string()); let densified = densify_edges( &expanded, @@ -123,7 +122,7 @@ impl GeomTrait for Rule { ) -> std::result::Result<(), String> { // Rule requires exactly one of pos1 or pos2 (XOR logic). // After densification both axes are present — skip the check. - if partition_by.contains(&"__ggsql_rule_id__".to_string()) { + if partition_by.contains(&naming::DENSIFY_ID_COLUMN.to_string()) { return Ok(()); } @@ -252,15 +251,17 @@ fn expand_rule_to_segment( .map(|c| naming::quote_ident(c)) .collect(); + let densify_id_q = naming::quote_ident(naming::DENSIFY_ID_COLUMN); + let numbered = format!( "SELECT *, ROW_NUMBER() OVER (ORDER BY (SELECT NULL)) \ - AS \"__ggsql_rule_id__\" FROM ({query})" + AS {densify_id_q} FROM ({query})" ); let vertices_table = "(SELECT 0 AS \"__ggsql_vertex__\" UNION ALL SELECT 1)"; let mut select_parts: Vec = passthrough_quoted; - select_parts.push("\"__ggsql_rule_id__\"".to_string()); + select_parts.push(densify_id_q.to_string()); select_parts.push("\"__ggsql_vertex__\"".to_string()); select_parts.push(fixed_expr); select_parts.push(span_expr); @@ -272,7 +273,7 @@ fn expand_rule_to_segment( ); let mut out_columns: Vec = passthrough_cols.into_iter().cloned().collect(); - out_columns.push("__ggsql_rule_id__".to_string()); + out_columns.push(naming::DENSIFY_ID_COLUMN.to_string()); out_columns.push(pos1_col); out_columns.push(pos2_col); @@ -396,7 +397,7 @@ mod tests { .unwrap(); assert!(result.contains("ST_Transform")); - assert!(!result.contains("__ggsql_rule_id__")); + assert!(!result.contains(naming::DENSIFY_ID_COLUMN)); assert!(partition_by.is_empty()); } @@ -431,10 +432,10 @@ mod tests { ) .unwrap(); - assert!(result.contains("__ggsql_rule_id__")); + assert!(result.contains(naming::DENSIFY_ID_COLUMN)); assert!(result.contains("CROSS JOIN")); assert!(result.contains("ST_Transform")); - assert!(partition_by.contains(&"__ggsql_rule_id__".to_string())); + assert!(partition_by.contains(&naming::DENSIFY_ID_COLUMN.to_string())); // pos2 should be added to mappings (the spanning axis) assert!(mappings.contains_key("pos2")); } @@ -585,7 +586,7 @@ mod tests { &expanded, dialect, &expanded_columns, - &["__ggsql_rule_id__".to_string()], + &[naming::DENSIFY_ID_COLUMN.to_string()], Some("__ggsql_vertex__"), false, 1.0, diff --git a/src/plot/layer/geom/segment.rs b/src/plot/layer/geom/segment.rs index 14799c958..3872f8784 100644 --- a/src/plot/layer/geom/segment.rs +++ b/src/plot/layer/geom/segment.rs @@ -66,8 +66,7 @@ impl GeomTrait for Segment { let columns = mappings.column_names(); let (expanded, expanded_columns) = expand_segment_to_vertices(query, &columns); - let segment_id_col = "__ggsql_segment_id__".to_string(); - partition_by.push(segment_id_col); + partition_by.push(naming::DENSIFY_ID_COLUMN.to_string()); let densified = densify_edges( &expanded, @@ -93,7 +92,7 @@ impl GeomTrait for Segment { /// /// Input: one row per segment with pos1/pos2 (start) and pos1end/pos2end (end). /// Output: two rows per segment with pos1/pos2 vertex positions and a -/// `__ggsql_segment_id__` grouping column. Material aesthetics pass through unchanged. +/// `DENSIFY_ID_COLUMN` grouping column. Material aesthetics pass through unchanged. fn expand_segment_to_vertices(query: &str, columns: &[String]) -> (String, Vec) { let pos1_col = naming::aesthetic_column("pos1"); let pos2_col = naming::aesthetic_column("pos2"); @@ -109,9 +108,11 @@ fn expand_segment_to_vertices(query: &str, columns: &[String]) -> (String, Vec (String, Vec = passthrough; - select_parts.push("\"__ggsql_segment_id__\"".to_string()); + select_parts.push(densify_id_q.to_string()); select_parts.push("\"__ggsql_vertex__\"".to_string()); select_parts.push(format!( "CASE \"__ggsql_vertex__\" WHEN 0 THEN {pos1_q} WHEN 1 THEN {pos1end_q} END AS {pos1_q}" @@ -138,7 +139,7 @@ fn expand_segment_to_vertices(query: &str, columns: &[String]) -> (String, Vec = passthrough_cols.into_iter().cloned().collect(); - out_columns.push("__ggsql_segment_id__".to_string()); + out_columns.push(naming::DENSIFY_ID_COLUMN.to_string()); out_columns.push(pos1_col); out_columns.push(pos2_col); @@ -256,10 +257,10 @@ mod tests { ) .unwrap(); - assert!(result.contains("__ggsql_segment_id__")); + assert!(result.contains(naming::DENSIFY_ID_COLUMN)); assert!(result.contains("CROSS JOIN")); assert!(result.contains("ST_Transform")); - assert!(partition_by.contains(&"__ggsql_segment_id__".to_string())); + assert!(partition_by.contains(&naming::DENSIFY_ID_COLUMN.to_string())); // pos1end/pos2end remapped to pos1/pos2 columns assert!(mappings.contains_key("pos1end")); assert!(mappings.contains_key("pos2end")); diff --git a/src/plot/layer/geom/tile.rs b/src/plot/layer/geom/tile.rs index d389b21c8..4d3f519ad 100644 --- a/src/plot/layer/geom/tile.rs +++ b/src/plot/layer/geom/tile.rs @@ -237,8 +237,7 @@ impl GeomTrait for Tile { let (expanded, expanded_columns) = expand_rect_to_polygon(query, &columns); - let poly_id_col = "__ggsql_poly_id__".to_string(); - partition_by.push(poly_id_col.clone()); + partition_by.push(naming::DENSIFY_ID_COLUMN.to_string()); let densified = densify_edges( &expanded, @@ -270,7 +269,7 @@ impl GeomTrait for Tile { /// /// Input: one row per rectangle with pos1min/pos1max/pos2min/pos2max bounds. /// Output: four rows per rectangle with pos1/pos2 corner positions and a -/// `__ggsql_poly_id__` grouping column. Material aesthetics pass through unchanged. +/// `DENSIFY_ID_COLUMN` grouping column. Material aesthetics pass through unchanged. /// The bound columns are dropped — callers that need them should re-derive them /// from pos1/pos2 after densification and projection. /// @@ -296,9 +295,11 @@ fn expand_rect_to_polygon(query: &str, columns: &[String]) -> (String, Vec (String, Vec = passthrough; - select_parts.push("\"__ggsql_poly_id__\"".to_string()); + select_parts.push(densify_id_q.to_string()); select_parts.push("\"__ggsql_corner__\"".to_string()); select_parts.push(format!( "CASE \"__ggsql_corner__\" \ @@ -340,7 +341,7 @@ fn expand_rect_to_polygon(query: &str, columns: &[String]) -> (String, Vec = passthrough_cols.into_iter().cloned().collect(); - out_columns.push("__ggsql_poly_id__".to_string()); + out_columns.push(naming::DENSIFY_ID_COLUMN.to_string()); out_columns.push(naming::aesthetic_column("pos1")); out_columns.push(naming::aesthetic_column("pos2")); @@ -1375,7 +1376,7 @@ mod tests { let (sql, out_cols) = expand_rect_to_polygon("SELECT * FROM t", &columns); // Should have poly_id assignment - assert!(sql.contains("__ggsql_poly_id__")); + assert!(sql.contains(naming::DENSIFY_ID_COLUMN)); // Should use CROSS JOIN with UNION ALL corner table assert!(sql.contains("CROSS JOIN")); assert!(sql.contains("UNION ALL")); @@ -1392,7 +1393,7 @@ mod tests { // Should carry through fill assert!(out_cols.contains(&naming::aesthetic_column("fill"))); // Should include poly_id - assert!(out_cols.contains(&"__ggsql_poly_id__".to_string())); + assert!(out_cols.contains(&naming::DENSIFY_ID_COLUMN.to_string())); } #[test] @@ -1441,7 +1442,7 @@ mod tests { .unwrap(); // Polygon expansion happened - assert!(result.contains("__ggsql_poly_id__")); + assert!(result.contains(naming::DENSIFY_ID_COLUMN)); assert!(result.contains("CROSS JOIN")); // Densification happened assert!(result.contains("__ggsql_seq__")); @@ -1567,7 +1568,7 @@ mod tests { // Should just project, no densification assert!(result.contains("ST_Transform")); - assert!(!result.contains("__ggsql_poly_id__")); + assert!(!result.contains(naming::DENSIFY_ID_COLUMN)); assert!(!result.contains("CROSS JOIN")); } } diff --git a/src/writer/vegalite/layer.rs b/src/writer/vegalite/layer.rs index 4a28894fa..7182ee7e5 100644 --- a/src/writer/vegalite/layer.rs +++ b/src/writer/vegalite/layer.rs @@ -761,7 +761,7 @@ impl GeomRenderer for RuleRenderer { // Densified rule: expanded to a multi-row line under map projection if layer .partition_by - .contains(&"__ggsql_rule_id__".to_string()) + .contains(&naming::DENSIFY_ID_COLUMN.to_string()) { layer_spec["mark"] = json!({ "type": "line", @@ -1459,7 +1459,7 @@ impl GeomRenderer for TileRenderer { // Polygonized tile: densified rectangle rendered as closed line if layer .partition_by - .contains(&"__ggsql_poly_id__".to_string()) + .contains(&naming::DENSIFY_ID_COLUMN.to_string()) { layer_spec["mark"] = json!({ "type": "line", @@ -1599,7 +1599,7 @@ impl GeomRenderer for SegmentRenderer { ) -> Result<()> { if layer .partition_by - .contains(&"__ggsql_segment_id__".to_string()) + .contains(&naming::DENSIFY_ID_COLUMN.to_string()) { layer_spec["mark"] = json!({ "type": "line", @@ -1641,7 +1641,7 @@ impl GeomRenderer for RibbonRenderer { ) -> Result<()> { if layer .partition_by - .contains(&"__ggsql_ribbon_id__".to_string()) + .contains(&naming::DENSIFY_ID_COLUMN.to_string()) { layer_spec["mark"] = json!({ "type": "line", From e2632b5b87b5e6607cad3711254fc6ad8db154ac Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Fri, 12 Jun 2026 11:01:20 +0200 Subject: [PATCH 17/20] Replace partition_by magic-string checks with typed "densified" parameter Instead of detecting densification state by checking whether partition_by contains the densify ID column name, each geom now sets a typed parameters["densified"] = true during apply_projection. The writer and validate_aesthetics check this parameter directly. Co-Authored-By: Claude Opus 4.6 --- src/plot/layer/geom/mod.rs | 6 +++--- src/plot/layer/geom/ribbon.rs | 5 +++-- src/plot/layer/geom/rule.rs | 8 ++++++-- src/plot/layer/geom/segment.rs | 5 +++-- src/plot/layer/geom/tile.rs | 3 ++- src/plot/layer/mod.rs | 2 +- src/writer/vegalite/layer.rs | 32 ++++++++++++++++---------------- 7 files changed, 34 insertions(+), 27 deletions(-) diff --git a/src/plot/layer/geom/mod.rs b/src/plot/layer/geom/mod.rs index 2f7d1c6eb..ff462e0f1 100644 --- a/src/plot/layer/geom/mod.rs +++ b/src/plot/layer/geom/mod.rs @@ -154,7 +154,7 @@ pub trait GeomTrait: std::fmt::Debug + std::fmt::Display + Send + Sync { &self, _mappings: &crate::Mappings, _aesthetic_ctx: &Option, - _partition_by: &[String], + _parameters: &HashMap, ) -> std::result::Result<(), String> { Ok(()) } @@ -902,10 +902,10 @@ impl Geom { &self, mappings: &Mappings, aesthetic_ctx: &Option, - partition_by: &[String], + parameters: &HashMap, ) -> std::result::Result<(), String> { self.0 - .validate_aesthetics(mappings, aesthetic_ctx, partition_by) + .validate_aesthetics(mappings, aesthetic_ctx, parameters) } } diff --git a/src/plot/layer/geom/ribbon.rs b/src/plot/layer/geom/ribbon.rs index 5b2eb87af..42ba7280a 100644 --- a/src/plot/layer/geom/ribbon.rs +++ b/src/plot/layer/geom/ribbon.rs @@ -7,7 +7,7 @@ use super::{ DefaultAesthetics, GeomTrait, GeomType, StatResult, }; use crate::plot::projection::Projection; -use crate::plot::types::DefaultAestheticValue; +use crate::plot::types::{DefaultAestheticValue, ParameterValue}; use crate::plot::{DefaultParamValue, ParamConstraint, ParamDefinition}; use crate::reader::SqlDialect; use crate::{naming, Mappings, Result}; @@ -59,7 +59,7 @@ impl GeomTrait for Ribbon { dialect: &dyn SqlDialect, mappings: &mut Mappings, partition_by: &mut Vec, - _parameters: &mut std::collections::HashMap, + parameters: &mut std::collections::HashMap, ) -> Result { if !needs_projection(projection) { return Ok(query.to_string()); @@ -69,6 +69,7 @@ impl GeomTrait for Ribbon { let (expanded, expanded_columns) = expand_ribbon_to_polygon(query, &columns, partition_by); partition_by.push(naming::DENSIFY_ID_COLUMN.to_string()); + parameters.insert("densified".to_string(), ParameterValue::Boolean(true)); let densified = densify_edges( &expanded, diff --git a/src/plot/layer/geom/rule.rs b/src/plot/layer/geom/rule.rs index 015291693..cb277ef27 100644 --- a/src/plot/layer/geom/rule.rs +++ b/src/plot/layer/geom/rule.rs @@ -72,6 +72,7 @@ impl GeomTrait for Rule { expand_rule_to_segment(query, &columns, has_pos1, dialect); partition_by.push(naming::DENSIFY_ID_COLUMN.to_string()); + parameters.insert("densified".to_string(), ParameterValue::Boolean(true)); let densified = densify_edges( &expanded, @@ -118,11 +119,14 @@ impl GeomTrait for Rule { &self, mappings: &crate::Mappings, aesthetic_ctx: &Option, - partition_by: &[String], + parameters: &std::collections::HashMap, ) -> std::result::Result<(), String> { // Rule requires exactly one of pos1 or pos2 (XOR logic). // After densification both axes are present — skip the check. - if partition_by.contains(&naming::DENSIFY_ID_COLUMN.to_string()) { + if matches!( + parameters.get("densified"), + Some(ParameterValue::Boolean(true)) + ) { return Ok(()); } diff --git a/src/plot/layer/geom/segment.rs b/src/plot/layer/geom/segment.rs index 3872f8784..76126fa04 100644 --- a/src/plot/layer/geom/segment.rs +++ b/src/plot/layer/geom/segment.rs @@ -6,7 +6,7 @@ use super::{ DefaultParamValue, GeomTrait, GeomType, ParamConstraint, ParamDefinition, }; use crate::plot::projection::Projection; -use crate::plot::types::DefaultAestheticValue; +use crate::plot::types::{DefaultAestheticValue, ParameterValue}; use crate::reader::SqlDialect; use crate::{naming, Mappings, Result}; @@ -57,7 +57,7 @@ impl GeomTrait for Segment { dialect: &dyn SqlDialect, mappings: &mut Mappings, partition_by: &mut Vec, - _parameters: &mut std::collections::HashMap, + parameters: &mut std::collections::HashMap, ) -> Result { if !needs_projection(projection) { return Ok(query.to_string()); @@ -67,6 +67,7 @@ impl GeomTrait for Segment { let (expanded, expanded_columns) = expand_segment_to_vertices(query, &columns); partition_by.push(naming::DENSIFY_ID_COLUMN.to_string()); + parameters.insert("densified".to_string(), ParameterValue::Boolean(true)); let densified = densify_edges( &expanded, diff --git a/src/plot/layer/geom/tile.rs b/src/plot/layer/geom/tile.rs index 4d3f519ad..5bc571824 100644 --- a/src/plot/layer/geom/tile.rs +++ b/src/plot/layer/geom/tile.rs @@ -216,7 +216,7 @@ impl GeomTrait for Tile { dialect: &dyn SqlDialect, mappings: &mut Mappings, partition_by: &mut Vec, - _parameters: &mut std::collections::HashMap, + parameters: &mut std::collections::HashMap, ) -> Result { if !needs_projection(projection) { return Ok(query.to_string()); @@ -238,6 +238,7 @@ impl GeomTrait for Tile { let (expanded, expanded_columns) = expand_rect_to_polygon(query, &columns); partition_by.push(naming::DENSIFY_ID_COLUMN.to_string()); + parameters.insert("densified".to_string(), ParameterValue::Boolean(true)); let densified = densify_edges( &expanded, diff --git a/src/plot/layer/mod.rs b/src/plot/layer/mod.rs index 5284f7b26..69e837a33 100644 --- a/src/plot/layer/mod.rs +++ b/src/plot/layer/mod.rs @@ -306,7 +306,7 @@ impl Layer { // Call geom-specific validation (e.g., XOR constraints for Rule) self.geom - .validate_aesthetics(&self.mappings, context, &self.partition_by)?; + .validate_aesthetics(&self.mappings, context, &self.parameters)?; Ok(()) } diff --git a/src/writer/vegalite/layer.rs b/src/writer/vegalite/layer.rs index 7182ee7e5..8639a2793 100644 --- a/src/writer/vegalite/layer.rs +++ b/src/writer/vegalite/layer.rs @@ -759,10 +759,10 @@ impl GeomRenderer for RuleRenderer { context: &RenderContext, ) -> Result<()> { // Densified rule: expanded to a multi-row line under map projection - if layer - .partition_by - .contains(&naming::DENSIFY_ID_COLUMN.to_string()) - { + if matches!( + layer.parameters.get("densified"), + Some(ParameterValue::Boolean(true)) + ) { layer_spec["mark"] = json!({ "type": "line", "clip": true @@ -1457,10 +1457,10 @@ impl GeomRenderer for TileRenderer { let (pos1, pos1_end, _, pos2, pos2_end, _) = &context.channels; // Polygonized tile: densified rectangle rendered as closed line - if layer - .partition_by - .contains(&naming::DENSIFY_ID_COLUMN.to_string()) - { + if matches!( + layer.parameters.get("densified"), + Some(ParameterValue::Boolean(true)) + ) { layer_spec["mark"] = json!({ "type": "line", "interpolate": "linear-closed" @@ -1597,10 +1597,10 @@ impl GeomRenderer for SegmentRenderer { layer: &Layer, context: &RenderContext, ) -> Result<()> { - if layer - .partition_by - .contains(&naming::DENSIFY_ID_COLUMN.to_string()) - { + if matches!( + layer.parameters.get("densified"), + Some(ParameterValue::Boolean(true)) + ) { layer_spec["mark"] = json!({ "type": "line", "clip": true @@ -1639,10 +1639,10 @@ impl GeomRenderer for RibbonRenderer { layer: &Layer, context: &RenderContext, ) -> Result<()> { - if layer - .partition_by - .contains(&naming::DENSIFY_ID_COLUMN.to_string()) - { + if matches!( + layer.parameters.get("densified"), + Some(ParameterValue::Boolean(true)) + ) { layer_spec["mark"] = json!({ "type": "line", "interpolate": "linear-closed" From 7e45ff273fc07adf63fef2b92edb899544c2bd12 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Fri, 12 Jun 2026 11:07:40 +0200 Subject: [PATCH 18/20] fix double evaluation of window function --- src/plot/layer/geom/ribbon.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/plot/layer/geom/ribbon.rs b/src/plot/layer/geom/ribbon.rs index 42ba7280a..802d80145 100644 --- a/src/plot/layer/geom/ribbon.rs +++ b/src/plot/layer/geom/ribbon.rs @@ -205,9 +205,10 @@ fn expand_ribbon_to_polygon( lower_parts.push(format!("{pos2min_q} AS {pos2_q}")); let sql = format!( - "SELECT {} FROM ({numbered}) \"__ggsql_r__\" \ + "WITH \"__ggsql_r__\" AS ({numbered}) \ + SELECT {} FROM \"__ggsql_r__\" \ UNION ALL \ - SELECT {} FROM ({numbered}) \"__ggsql_r__\"", + SELECT {} FROM \"__ggsql_r__\"", upper_parts.join(", "), lower_parts.join(", "), ); From 3e03c5c320cc04bcae6d516906b3db49e5890f51 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Fri, 12 Jun 2026 15:56:38 +0200 Subject: [PATCH 19/20] mention projection name --- src/plot/layer/geom/mod.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/plot/layer/geom/mod.rs b/src/plot/layer/geom/mod.rs index ff462e0f1..e1389bd47 100644 --- a/src/plot/layer/geom/mod.rs +++ b/src/plot/layer/geom/mod.rs @@ -318,8 +318,9 @@ pub trait GeomTrait: std::fmt::Debug + std::fmt::Display + Send + Sync { ) -> Result { if needs_projection(projection) { return Err(crate::GgsqlError::ValidationError(format!( - "Layer '{}' is not supported under map projection.", - self.geom_type() + "Layer '{}' is not supported under '{}' projection.", + self.geom_type(), + projection.coord.name() ))); } Ok(query.to_string()) @@ -1156,7 +1157,7 @@ mod tests { let err = result.unwrap_err(); assert_eq!( err.to_string(), - "Validation error: Layer 'bar' is not supported under map projection." + "Validation error: Layer 'bar' is not supported under 'Unknown' projection." ); } From aae1c24c20025c7fbd239bb91ea71e166c1a94c3 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Fri, 12 Jun 2026 16:30:36 +0200 Subject: [PATCH 20/20] Separate spatial-specific steps from generic densification in rule layer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Decouple expand_rule_to_segment from clip_boundary_table by accepting a bbox_expr argument. Gate spatial steps (bbox source, ST_Contains clip) behind CoordKind::Map match arms so future non-map projections can reuse the generic expand→densify→project pipeline. Also include the projection name in the unsupported-geom error message. Co-Authored-By: Claude Opus 4.6 --- src/plot/layer/geom/rule.rs | 54 ++++++++++++++++++++++--------------- 1 file changed, 32 insertions(+), 22 deletions(-) diff --git a/src/plot/layer/geom/rule.rs b/src/plot/layer/geom/rule.rs index cb277ef27..ada1cbd91 100644 --- a/src/plot/layer/geom/rule.rs +++ b/src/plot/layer/geom/rule.rs @@ -5,6 +5,7 @@ use super::{ GeomType, ParamDefinition, }; use crate::plot::projection::coord::map::clip_boundary_table; +use crate::plot::projection::coord::CoordKind; use crate::plot::projection::Projection; use crate::plot::types::{DefaultAestheticValue, ParameterValue}; use crate::reader::SqlDialect; @@ -54,22 +55,27 @@ impl GeomTrait for Rule { return Ok(query.to_string()); } - // Rules only densify when clipping is active (clip boundary table exists) - let clip_active = matches!( - projection.properties.get("clip"), - Some(ParameterValue::Boolean(true)) - ); // The rule input always has one position column named __ggsql_aes_pos1__ // regardless of whether the aesthetic key is "pos1" or "pos2" — the executor // normalizes it to the pos1 slot. let columns = vec![naming::aesthetic_column("pos1")]; - if !clip_active { - return project_position_columns(query, projection, dialect, &columns); - } let has_pos1 = mappings.contains_key("pos1"); + let bbox_expr = match projection.coord.coord_kind() { + CoordKind::Map + if matches!( + projection.properties.get("clip"), + Some(ParameterValue::Boolean(true)) + ) => + { + let boundary_table = clip_boundary_table(); + dialect.sql_geometry_bbox("geom", &boundary_table) + } + // If we don't have a bbox, we cannot expand rule layer + _ => return project_position_columns(query, projection, dialect, &columns), + }; let (expanded, expanded_columns) = - expand_rule_to_segment(query, &columns, has_pos1, dialect); + expand_rule_to_segment(query, &columns, has_pos1, &bbox_expr); partition_by.push(naming::DENSIFY_ID_COLUMN.to_string()); parameters.insert("densified".to_string(), ParameterValue::Boolean(true)); @@ -84,14 +90,18 @@ impl GeomTrait for Rule { 1.0, 360, ); - // Clip densified vertices to the clip boundary before projecting. - let pos1_q = naming::quote_ident(&naming::aesthetic_column("pos1")); - let pos2_q = naming::quote_ident(&naming::aesthetic_column("pos2")); - let clip_table = clip_boundary_table(); - let clipped = format!( - "SELECT * FROM ({densified}) WHERE ST_Contains(\ - (SELECT geom FROM {clip_table}), ST_Point({pos1_q}, {pos2_q}))" - ); + let clipped = match projection.coord.coord_kind() { + CoordKind::Map => { + let pos1_q = naming::quote_ident(&naming::aesthetic_column("pos1")); + let pos2_q = naming::quote_ident(&naming::aesthetic_column("pos2")); + let clip_table = clip_boundary_table(); + format!( + "SELECT * FROM ({densified}) WHERE ST_Contains(\ + (SELECT geom FROM {clip_table}), ST_Point({pos1_q}, {pos2_q}))" + ) + } + _ => densified, + }; let projected = project_position_columns(&clipped, projection, dialect, &expanded_columns)?; @@ -214,20 +224,19 @@ impl GeomTrait for Rule { /// - true (vertical rule): pos1 is longitude (fixed), synthesize pos2 from bbox y. /// - false (horizontal rule): pos1 is latitude (fixed), rename to pos2, synthesize /// pos1 from bbox x. +/// +/// `bbox_expr` is a SQL expression that yields xmin, ymin, xmax, ymax columns. fn expand_rule_to_segment( query: &str, columns: &[String], has_pos1: bool, - dialect: &dyn SqlDialect, + bbox_expr: &str, ) -> (String, Vec) { let pos1_col = naming::aesthetic_column("pos1"); let pos2_col = naming::aesthetic_column("pos2"); let pos1_q = naming::quote_ident(&pos1_col); let pos2_q = naming::quote_ident(&pos2_col); - let boundary_table = clip_boundary_table(); - let bbox_expr = dialect.sql_geometry_bbox("geom", &boundary_table); - // The input column is always __ggsql_aes_pos1__. Build the SELECT // expressions that produce both pos1 and pos2 in the output. let (fixed_expr, span_expr) = if has_pos1 { @@ -584,8 +593,9 @@ mod tests { // Run expand + densify manually to verify latitude stays constant pre-projection. let columns = vec![naming::aesthetic_column("pos1")]; let has_pos1 = false; + let bbox_expr = dialect.sql_geometry_bbox("geom", &boundary_table); let (expanded, expanded_columns) = - expand_rule_to_segment(&input, &columns, has_pos1, dialect); + expand_rule_to_segment(&input, &columns, has_pos1, &bbox_expr); let densified = densify_edges( &expanded, dialect,