Skip to content

Commit ef964ab

Browse files
teunbrandclaude
andcommitted
Unify line and path renderers into single PathRenderer
Remove LineRenderer and consolidate all line/path rendering logic into PathRenderer. Both geom types were already functionally identical: - Both map to "line" mark in Vega-Lite - Both add order encoding to preserve data order - Both need identical material aesthetic handling Changes: - Remove LineRenderer struct - PathRenderer now handles both line and path geoms - Updated get_renderer() to return PathRenderer for both GeomType::Line and GeomType::Path - Updated documentation to reflect unified renderer This eliminates code duplication while maintaining all functionality. The actual sorting/ordering happens in the execute phase via SQL ORDER BY, and both geoms preserve that order through the __ggsql_row_index__ column. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 49f42fd commit ef964ab

1 file changed

Lines changed: 13 additions & 30 deletions

File tree

src/writer/vegalite/layer.rs

Lines changed: 13 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -314,27 +314,18 @@ impl GeomRenderer for BarRenderer {
314314
// Path Renderer
315315
// =============================================================================
316316

317-
/// Renderer for path geom - adds order channel for natural data order
317+
/// Renderer for path and line geoms - preserves data order for correct rendering
318+
///
319+
/// Automatically detects when continuous material aesthetics (stroke, linewidth, opacity) vary
320+
/// within partition groups and converts to segmented rendering using detail encoding.
321+
/// Discrete material aesthetics (linetype, or discrete stroke) already define groups
322+
/// via partition_by and don't require special handling.
323+
///
324+
/// Handles both `line` and `path` geoms - the only difference is the mark type used.
318325
pub struct PathRenderer;
319326

320-
impl GeomRenderer for PathRenderer {
321-
fn modify_encoding(
322-
&self,
323-
encoding: &mut Map<String, Value>,
324-
_layer: &Layer,
325-
_context: &RenderContext,
326-
) -> Result<()> {
327-
// Use row index field to preserve natural data order
328-
encoding.insert(
329-
"order".to_string(),
330-
json!({"field": ROW_INDEX_COLUMN, "type": "quantitative"}),
331-
);
332-
Ok(())
333-
}
334-
}
335-
336327
// =============================================================================
337-
// Line Renderer
328+
// Helper functions for path/line segmentation
338329
// =============================================================================
339330

340331
/// Find row indices where any of the specified columns change value.
@@ -423,15 +414,7 @@ fn aesthetic_varies_within_groups(
423414
Ok(false)
424415
}
425416

426-
/// Renderer for line geom - preserves data order for correct line rendering
427-
///
428-
/// Automatically detects when continuous material aesthetics (stroke, linewidth, opacity) vary
429-
/// within partition groups and converts to segmented rendering using detail encoding.
430-
/// Discrete material aesthetics (linetype, or discrete stroke) already define groups
431-
/// via partition_by and don't require special handling.
432-
pub struct LineRenderer;
433-
434-
impl GeomRenderer for LineRenderer {
417+
impl GeomRenderer for PathRenderer {
435418
fn prepare_data(
436419
&self,
437420
df: &DataFrame,
@@ -507,10 +490,10 @@ impl GeomRenderer for LineRenderer {
507490
_data_key: &str,
508491
prepared: &PreparedData,
509492
) -> Result<Vec<Value>> {
510-
// Check if segmentation is needed via metadata
493+
// Get metadata from prepared data
511494
let PreparedData::Single { metadata, .. } = prepared else {
512495
return Err(GgsqlError::InternalError(
513-
"LineRenderer expects PreparedData::Single".to_string(),
496+
"PathRenderer expects PreparedData::Single".to_string(),
514497
));
515498
};
516499

@@ -2207,7 +2190,7 @@ impl GeomRenderer for BoxplotRenderer {
22072190
pub fn get_renderer(geom: &Geom) -> Box<dyn GeomRenderer> {
22082191
match geom.geom_type() {
22092192
GeomType::Path => Box::new(PathRenderer),
2210-
GeomType::Line => Box::new(LineRenderer),
2193+
GeomType::Line => Box::new(PathRenderer),
22112194
GeomType::Bar => Box::new(BarRenderer),
22122195
GeomType::Rect => Box::new(RectRenderer),
22132196
GeomType::Ribbon => Box::new(RibbonRenderer),

0 commit comments

Comments
 (0)