Layer densification/munching for map projections by teunbrand · Pull Request #472 · posit-dev/ggsql

teunbrand · 2026-06-12T10:09:19Z

This PR aims to fix #163.

Briefly, from discussion it appeared we didn't need to implement generic densification, but would still need it for map projections.
This PR enables densification for line, path, polygon, tile, segment, ribbon and rule. Earlier we already had spatial, text and point. We do not implement densification for bar, histogram, boxplot, violin, density, smooth, area or range.

The general pattern is this:

apply_projection() checks if we have a map projection
expand_{layer}_to_polygon() for layers that need custom logic
densify_edges() for interpolation
- some bookkeeping for the partition_by columns and tracking the densified state via parameters["densified"].
Writer detects parameters["densified"] and writes a polygon/line instead of rect/ribbon/rule/segment etc.

The odd one out is the rule layer, because it needs the bounding box for knowing where to draw the line, so it has a bit more spatial specific logic than the rest of em.

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 <noreply@anthropic.com>

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 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

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 <noreply@anthropic.com>

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 <noreply@anthropic.com>

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 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

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 <noreply@anthropic.com>

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 <noreply@anthropic.com>

…eter 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 <noreply@anthropic.com>

thomasp85

This generally looks fine, but I low-key hate how much code and logic this requires...

Did you toy with converting the positions to a spatial column instead and let the db's spatial extension handle it? Or is densification not build into these extensions..?

thomasp85 · 2026-06-12T12:18:26Z

+    fn apply_projection(
+        &self,
+        query: &str,
+        projection: &Projection,
+        dialect: &dyn SqlDialect,
+        mappings: &mut Mappings,
+        partition_by: &mut Vec<String>,
+        parameters: &mut std::collections::HashMap<String, crate::plot::types::ParameterValue>,
+    ) -> Result<String> {
+        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);
+
+        partition_by.push(naming::DENSIFY_ID_COLUMN.to_string());
+        parameters.insert("densified".to_string(), ParameterValue::Boolean(true));
+
+        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)
+    }


I think I have a hard time figuring out how a ribbon would look like in combination with a spatial projection but I guess it doesn't hurt

Yeah my initial thought was that it could show things like tornado tracks, where the size increases with wind speeds. However, as this is anchored to horizontal/vertical orientation that doesn't make much sense for arbitrary tornado paths. But at the point I realised this, it was already working 🤷

teunbrand · 2026-06-12T12:58:52Z

This generally looks fine, but I low-key hate how much code and logic this requires...

Yeah doing geometry like interpolation and polygonisation in SQL sparks joy in zero people.

Did you toy with converting the positions to a spatial column instead and let the db's spatial extension handle it? Or is densification not build into these extensions..?

The main barrier for this is that only PostGIS and not DuckDB/SpatiaLite implement densification IIRC.

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 <noreply@anthropic.com>

teunbrand and others added 18 commits June 9, 2026 15:57

Add partition_by parameter to GeomTrait::apply_projection

c8d3905

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 <noreply@anthropic.com>

504d9a3

Add tests for needs_projection, densify_edges, and line apply_projection

a77fcdb

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Run cargo fmt and suppress clippy too_many_arguments

fdff142

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Add Mappings::insert_column convenience method

65b3348

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Add Mappings::column_names and use it across geoms

da863f7

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Implement edge densification for segment geom under map projection

3f9446c

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Implement edge densification for ribbon geom under map projection

23da848

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

block other geoms that have not implemented densification

c7438a6

add comment

086dd9a

fix double evaluation of window function

7e45ff2

teunbrand marked this pull request as ready for review June 12, 2026 10:27

teunbrand requested a review from thomasp85 June 12, 2026 10:27

teunbrand added this to the ggsql 0.4.0 milestone Jun 12, 2026

thomasp85 approved these changes Jun 12, 2026

View reviewed changes

teunbrand and others added 2 commits June 12, 2026 15:56

mention projection name

3e03c5c

teunbrand merged commit 714b34e into posit-dev:main Jun 12, 2026
2 checks passed

teunbrand deleted the spatial_densify branch June 12, 2026 14:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Layer densification/munching for map projections#472

Layer densification/munching for map projections#472
teunbrand merged 20 commits into
posit-dev:mainfrom
teunbrand:spatial_densify

teunbrand commented Jun 12, 2026 •

edited

Loading

Uh oh!

thomasp85 left a comment

Uh oh!

Uh oh!

thomasp85 Jun 12, 2026

Uh oh!

teunbrand Jun 12, 2026

Uh oh!

Uh oh!

teunbrand commented Jun 12, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

teunbrand commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thomasp85 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

thomasp85 Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

teunbrand Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

teunbrand commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

teunbrand commented Jun 12, 2026 •

edited

Loading

teunbrand commented Jun 12, 2026 •

edited

Loading