Grida Canvas - Curve Decorations#538
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
5 Skipped Deployments
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a stroke marker decoration system: new StrokeMarkerPreset types, per-node marker_start_shape/marker_end_shape fields, FlatBuffers/TS mappings, marker geometry and rendering (cutback-aware trimming), editor UI/tools/controls for selecting presets, example goldens, and related docs/config cleanup. Changes
Sequence DiagramsequenceDiagram
actor User
participant EditorUI as Editor UI
participant EditorStore as Editor Store
participant Painter as Painter
participant RenderEngine as Render Engine
User->>EditorUI: choose marker preset (start/end)
EditorUI->>EditorStore: changeNodePropertyStrokeDecoration*(node_id, preset)
EditorStore->>EditorStore: dispatch node/change marker_*_shape
EditorStore->>Painter: node updated (includes marker_start_shape/end)
Painter->>Painter: build layers (carry marker fields, stroke_width)
Painter->>RenderEngine: compute cutback = cutback_depth(preset, stroke_width)
RenderEngine->>RenderEngine: if cutback>0 trim path and compute stroke geometry
RenderEngine->>Painter: render stroke (trimmed/untrimmed as required)
Painter->>RenderEngine: draw_stroke_decorations(path, start, end, stroke_width)
RenderEngine->>RenderEngine: position & draw marker shapes at endpoints via PathMeasure
RenderEngine->>User: rasterized canvas with endpoint markers
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- Introduced a new document detailing the renderer-agnostic model for attaching glyphs (arrowheads, markers, ticks) to 2D paths. - Defined parameters for curve and arc-length placement, local frame orientation, and marker glyph properties. - Established policies for orientation, scale, and offset to enhance the rendering of decorations on paths. - Aimed to unify various marker types and improve compatibility with existing graphics standards.
- Introduced a new example demonstrating the placement model for curve decorations, showcasing various features such as endpoint markers, auto-start-reverse, join markers, and repeated placements. - Included functionality to generate and save a visual representation of the decorations applied to different path types (straight, polyline, curve). - Updated documentation to reflect the new example and its relevance to the curve-decoration feature.
- Enhanced the AGENTS.md file to present project structure in a tabular format for better clarity and organization. - Introduced a new format/AGENTS.md file detailing the guidelines for LLM agents working with the .grida file format, including schema evolution strategies and compatibility considerations.
- Added a new `StrokeDecoration` enum to represent various marker decorations at stroke endpoints, including arrows, circles, diamonds, and triangles. - Implemented functionality to draw these decorations in the rendering pipeline, ensuring they are correctly positioned and oriented based on stroke paths. - Created a new example demonstrating the use of stroke decorations in the Grida canvas, showcasing different decoration types and their visual representation. - Updated relevant data structures and serialization methods to support stroke decorations in line and vector nodes. - Enhanced the editor interface to allow users to set stroke decorations for line and vector nodes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@crates/grida-canvas/examples/golden_curve_decoration.rs`:
- Around line 94-104: The write to goldens/curve_decoration.png can panic if the
goldens directory doesn't exist; before calling std::fs::write (which is invoked
with the data variable after image.encode and before println!), ensure the
directory exists by creating it (use std::fs::create_dir_all on the parent
directory of the concat!(env!("CARGO_MANIFEST_DIR"),
"/goldens/curve_decoration.png") path) and handle or unwrap the result
appropriately so the example doesn't panic when run the first time.
In `@crates/grida-canvas/examples/golden_stroke_decoration.rs`:
- Around line 294-299: The code writes to the path stored in `out` without
ensuring its parent directory exists; before calling `std::fs::write(out, ...)`
(in the example around `out` and the write/println), call
`std::fs::create_dir_all` on the parent directory of `out` (use
`std::path::Path::new(out).parent()`) to create the `goldens` directory if
missing, then proceed with the write and keep existing error handling (e.g.,
unwrap) or propagate errors as appropriate.
In `@crates/grida-canvas/src/shape/marker.rs`:
- Line 14: Remove the unused import Point from the skia_safe import list in
crates/grida-canvas/src/shape/marker.rs: update the use statement that currently
references Canvas, Paint, PaintStyle, Path, PathBuilder, PathMeasure, Point to
drop Point so the import only includes symbols actually used (Canvas, Paint,
PaintStyle, Path, PathBuilder, PathMeasure), then run cargo build to confirm the
unused-import error is resolved.
In `@docs/wg/feat-2d/curve-decoration.md`:
- Around line 280-286: The circular cutback formula for c (c = r - sqrt(r^2 -
(w/2)^2)) currently jumps to c = 2r at the boundary w/2 = r; update the text to
avoid the discontinuity by either (a) changing the clamp wording to explicitly
state it's a conservative clamp and why, or (b) replace the branch with a
clamped expression c = min(2*r, r - sqrt(r^2 - (w/2)^2)) and optionally mention
smoothing for a visually continuous transition; reference the variables c, r,
and w in the description so readers can locate and understand the change.
In `@packages/grida-canvas-vn/vn.ts`:
- Around line 12-16: The doc comment for VectorNetworkVertex contains a dangling
{`@link` VectorNetworkVertexWithOverrides} that doesn't exist; fix by either
removing the {`@link` VectorNetworkVertexWithOverrides} reference from the JSDoc
above the export type VectorNetworkVertex, or if intended, add a new exported
type/interface named VectorNetworkVertexWithOverrides (matching the referenced
name) and update the comment to point to it; ensure the symbol you add or the
edited comment remains consistent with VectorNetworkVertex usage.
🧹 Nitpick comments (6)
crates/grida-canvas/src/vectornetwork/vn.rs (1)
5-13: AddPartialEqderive for consistency withVectorNetworkSegment.Since
StrokeDecorationderivesPartialEqandVectorNetworkSegment(line 15) derives it,VectorVertexOverrideshould too for consistency and to enable comparison-based testing.♻️ Suggested change
-#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct VectorVertexOverride { pub vertex_index: usize, pub stroke_decoration: StrokeDecoration, }crates/grida-canvas/src/io/io_grida.rs (1)
1828-1829: Prefer explicitStrokeDecoration::Noneoverunwrap_or_default()for clarity.
Keeps deserialization semantics stable ifDefaultever changes.♻️ Suggested change
- stroke_decoration_start: node.base.stroke_decoration_start.unwrap_or_default(), - stroke_decoration_end: node.base.stroke_decoration_end.unwrap_or_default(), + stroke_decoration_start: node + .base + .stroke_decoration_start + .unwrap_or(StrokeDecoration::None), + stroke_decoration_end: node + .base + .stroke_decoration_end + .unwrap_or(StrokeDecoration::None),- stroke_decoration_start: node.base.stroke_decoration_start.unwrap_or_default(), - stroke_decoration_end: node.base.stroke_decoration_end.unwrap_or_default(), + stroke_decoration_start: node + .base + .stroke_decoration_start + .unwrap_or(StrokeDecoration::None), + stroke_decoration_end: node + .base + .stroke_decoration_end + .unwrap_or(StrokeDecoration::None),Also applies to: 1886-1887
crates/grida-canvas/examples/golden_stroke_decoration.rs (1)
391-426: Consider using math2 primitives for the custom marker geometry.
The star/cross builders do manual trig; math2 vectors/angle helpers would keep geometry math consistent with the rest of the crate and ease reuse.
As per coding guidelines: Usemath2crate for geometry and common math operations.crates/grida-canvas/src/painter/layer.rs (1)
425-427: Consider a helper orDefaultimpl to reduce boilerplate for non-decoratable nodes.Every non-Line/non-Vector shape node repeats the same three lines:
stroke_decoration_start: StrokeDecoration::None, stroke_decoration_end: StrokeDecoration::None, stroke_width: 0.0,This appears ~10 times. A small constructor or
Default-based builder forPainterPictureShapeLayerwould reduce the surface area for future field additions and make adding new decoration-like features less error-prone.Also applies to: 488-490, 541-543, 588-590, 635-637, 682-684, 729-731, 910-912, 999-1001, 1032-1034
editor/scaffolds/sidecontrol/controls/icons/stroke-decoration-icons.tsx (2)
195-217:VerticalBarFilledIconuses stroke rendering while all other "filled" icons usefill="currentColor".This icon uses
fill="none"with a stroke path to draw the bar, whereas the other filled variants (triangle, circle, square, diamond) setfill="currentColor"and use closed<path>/<rect>/<circle>elements. The visual result may differ subtly (e.g., stroke join artifacts, line cap behavior). Consider using a filled<rect>for consistency — similar to howMarkerVerticalBarFilled(line 322) already does it with a filled rect.
366-388: Minor:if (!icon) return nullon line 376 is unreachable.
iconByValuehas an entry for everycg.StrokeDecorationvalue (including"none"), soiconwill always be a validReactNode. The guard is harmless but dead code.
| For a circle with radius $r$ and forward edge at the anchor: | ||
|
|
||
| $$ | ||
| c = r - \sqrt{r^2 - (w/2)^2} | ||
| $$ | ||
|
|
||
| When $w/2 \geq r$, the stroke is wider than the circle and $c = 2r$ (full diameter). |
There was a problem hiding this comment.
Minor: circular cutback formula has a discontinuity at w/2 = r.
The formula c = r − √(r² − (w/2)²) yields c = r at w/2 = r, but the clamped branch states c = 2r. This creates a jump from r to 2r at the exact boundary. Consider noting this is a conservative clamp, or switching to c = min(2r, ...) with a smooth transition — though in practice the visual difference at that threshold is negligible.
🤖 Prompt for AI Agents
In `@docs/wg/feat-2d/curve-decoration.md` around lines 280 - 286, The circular
cutback formula for c (c = r - sqrt(r^2 - (w/2)^2)) currently jumps to c = 2r at
the boundary w/2 = r; update the text to avoid the discontinuity by either (a)
changing the clamp wording to explicitly state it's a conservative clamp and
why, or (b) replace the branch with a clamped expression c = min(2*r, r -
sqrt(r^2 - (w/2)^2)) and optionally mention smoothing for a visually continuous
transition; reference the variables c, r, and w in the description so readers
can locate and understand the change.
| * | ||
| * For basic vertices, this is a simple `[x, y]` tuple (Vector2). | ||
| * For vertices with decoration overrides, use {@link VectorNetworkVertexWithOverrides}. | ||
| */ | ||
| export type VectorNetworkVertex = Vector2; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for VectorNetworkVertexWithOverrides definition across the codebase
rg -n 'VectorNetworkVertexWithOverrides' --type=tsRepository: gridaco/grida
Length of output: 181
🏁 Script executed:
# Search for VectorNetworkVertexWithOverrides in all file types
rg 'VectorNetworkVertexWithOverrides' --type-list | head -20
rg 'VectorNetworkVertexWithOverrides' | grep -v '\.ts:.*@link'Repository: gridaco/grida
Length of output: 446
🏁 Script executed:
# Search for VectorNetworkVertexWithOverrides in all files
rg 'VectorNetworkVertexWithOverrides'Repository: gridaco/grida
Length of output: 178
Remove dangling @link reference: VectorNetworkVertexWithOverrides is not defined in the codebase.
The doc comment at line 14 references {@link VectorNetworkVertexWithOverrides}, but this type does not exist anywhere in the repository. Either define the type or remove the @link to prevent broken references for TypeScript tooling.
🤖 Prompt for AI Agents
In `@packages/grida-canvas-vn/vn.ts` around lines 12 - 16, The doc comment for
VectorNetworkVertex contains a dangling {`@link` VectorNetworkVertexWithOverrides}
that doesn't exist; fix by either removing the {`@link`
VectorNetworkVertexWithOverrides} reference from the JSDoc above the export type
VectorNetworkVertex, or if intended, add a new exported type/interface named
VectorNetworkVertexWithOverrides (matching the referenced name) and update the
comment to point to it; ensure the symbol you add or the edited comment remains
consistent with VectorNetworkVertex usage.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@crates/grida-canvas/src/shape/marker.rs`:
- Around line 127-131: The ArrowLines branch in BuiltinMarker::ArrowLines sets
cutback to a fixed stroke_width * 0.5 which ignores the anchor parameter t;
change MarkerShape creation in that branch to compute cutback via
lerp_cutback(terminal_cutback, depth, t) (using the same depth = size as already
computed) so the cutback scales with the anchor, or if fixed cutback is
intentional add a clarifying comment on why ArrowLines does not use
lerp_cutback; update the ArrowLines arm that builds path with
shift_path(&build_arrow_lines(size), t * depth) to use the lerp_cutback result
instead of the hard-coded value.
🧹 Nitpick comments (3)
crates/grida-canvas/src/devtools/hit_overlay.rs (1)
90-90: Dead computation: path is built but immediately dropped.Lines 71–89 clone the path from cache and apply two matrix transforms, only for the result to be discarded here. Even in devtools code this is unnecessary work on every frame. Consider gating the computation behind a feature flag or
cfg, or simply commenting out lines 71–89 until the path stroking feature lands.crates/grida-canvas/src/shape/marker.rs (2)
126-165: Duplicated magic constants acrossmarker_shape,cutback_depth, and shape builders.The scaling factors (e.g.,
0.8,0.866,1.5,0.6,1.2,0.85) are repeated inmarker_shape,cutback_depth, and thebuild_*functions. If one is changed without updating the others, the cutback/shape geometry will silently go out of sync.Consider having each shape builder return a
MarkerShape(or at least export itsdepth/half_height) so the constants live in one place. Alternatively, define named constants per preset.Sketch: centralize constants per preset
// Example for TriangleFilled mod triangle { pub const SCALE: f32 = 0.8; pub const DEPTH_FACTOR: f32 = 1.5; // depth = size * SCALE * DEPTH_FACTOR pub const HEIGHT_FACTOR: f32 = 0.866; // half_h = size * SCALE * HEIGHT_FACTOR }Then reference
triangle::SCALEetc. inbuild_triangle_terminal,marker_shape, andcutback_depth.
367-389:draw_decoration_atduplicates transform logic fromdraw_marker_shape_at.The canvas save/translate/rotate/draw/restore sequence on lines 382–386 is identical to lines 203–207 in
draw_marker_shape_at. Consider havingdraw_decoration_atdelegate todraw_marker_shape_atafter resolving the marker path and paint:Proposed refactor
pub fn draw_decoration_at( canvas: &Canvas, path_measure: &mut PathMeasure, distance: f32, decoration: StrokeDecoration, stroke_width: f32, paint: &Paint, reverse: bool, ) { let size = marker_size(stroke_width); if let Some(marker) = marker_path(decoration, size, stroke_width) { - if let Some((pos, tangent)) = path_measure.pos_tan(distance) { - let angle = tangent.y.atan2(tangent.x); - let flip = if reverse { PI } else { 0.0 }; - let dp = decoration_paint(paint, decoration, stroke_width); - canvas.save(); - canvas.translate(pos); - canvas.rotate((angle + flip) * 180.0 / PI, None); - canvas.draw_path(&marker, &dp); - canvas.restore(); - } + let dp = decoration_paint(paint, decoration, stroke_width); + draw_marker_shape_at(canvas, path_measure, distance, &marker, &dp, reverse); } }
| BuiltinMarker::ArrowLines => { | ||
| let depth = size; | ||
| let path = shift_path(&build_arrow_lines(size), t * depth); | ||
| MarkerShape { path, cutback: stroke_width * 0.5 } | ||
| } |
There was a problem hiding this comment.
ArrowLines cutback ignores anchor parameter t.
Every other preset delegates to lerp_cutback(terminal_cutback, depth, t) so that the cutback scales with the anchor position. ArrowLines hard-codes stroke_width * 0.5 regardless of t. This means marker_shape(ArrowLines, Centroid, …) or marker_shape(ArrowLines, Offset(0.7), …) will shift the geometry but leave cutback unchanged—potentially causing a visible gap or overlap at non-terminal anchors.
If non-terminal anchors are not expected for ArrowLines, a comment explaining why would prevent future confusion. Otherwise, consider using lerp_cutback:
Proposed fix
BuiltinMarker::ArrowLines => {
let depth = size;
+ let terminal_cutback = stroke_width * 0.5;
let path = shift_path(&build_arrow_lines(size), t * depth);
- MarkerShape { path, cutback: stroke_width * 0.5 }
+ MarkerShape { path, cutback: lerp_cutback(terminal_cutback, depth, t) }
}🤖 Prompt for AI Agents
In `@crates/grida-canvas/src/shape/marker.rs` around lines 127 - 131, The
ArrowLines branch in BuiltinMarker::ArrowLines sets cutback to a fixed
stroke_width * 0.5 which ignores the anchor parameter t; change MarkerShape
creation in that branch to compute cutback via lerp_cutback(terminal_cutback,
depth, t) (using the same depth = size as already computed) so the cutback
scales with the anchor, or if fixed cutback is intentional add a clarifying
comment on why ArrowLines does not use lerp_cutback; update the ArrowLines arm
that builds path with shift_path(&build_arrow_lines(size), t * depth) to use the
lerp_cutback result instead of the hard-coded value.
| pub fn draw_endpoint_decorations( | ||
| canvas: &Canvas, | ||
| path: &Path, | ||
| start: StrokeDecoration, | ||
| end: StrokeDecoration, | ||
| stroke_width: f32, | ||
| stroke_paint: &Paint, | ||
| ) { | ||
| if !start.has_marker() && !end.has_marker() { | ||
| return; | ||
| } | ||
|
|
||
| let mut measure = PathMeasure::new(path, false, None); | ||
| let length = measure.length(); | ||
|
|
||
| if start.has_marker() { | ||
| draw_decoration_at( | ||
| canvas, | ||
| &mut measure, | ||
| 0.0, | ||
| start, | ||
| stroke_width, | ||
| stroke_paint, | ||
| true, | ||
| ); | ||
| } | ||
| if end.has_marker() { | ||
| draw_decoration_at( | ||
| canvas, | ||
| &mut measure, | ||
| length, | ||
| end, | ||
| stroke_width, | ||
| stroke_paint, | ||
| false, | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Decorations on zero-length or degenerate paths.
If the path has zero length (e.g., a single-point path), measure.length() returns 0.0. Both start and end decorations would call pos_tan(0.0), which on a degenerate path may return None (handled by the if let in draw_decoration_at), or may return a zero tangent vector causing atan2(0, 0) → undefined orientation. The PR spec mentions a deterministic fallback for degenerate tangents, but no fallback is implemented here.
This is unlikely to cause a crash (Skia will still render, just with an arbitrary rotation), but worth noting if the spec's fallback guarantee matters for visual consistency.
…lated functionality - Renamed `StrokeDecoration` to `StrokeMarkerPreset` for clarity and consistency across the codebase. - Updated examples and documentation to reflect the new naming convention. - Adjusted rendering logic to utilize the new marker presets, ensuring accurate visual representation of markers. - Enhanced the marker functionality by introducing new marker types and updating existing ones. - Modified serialization and data structures to accommodate the changes in marker representation.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@crates/grida-canvas/src/io/io_grida.rs`:
- Around line 834-843: The serde attributes on the fields marker_start_shape and
marker_end_shape currently only accept snake_case names and thus drop camelCase
JSON keys; update their #[serde(...)] on the structs containing pub
marker_start_shape: Option<StrokeMarkerPreset> and pub marker_end_shape:
Option<StrokeMarkerPreset> to include camelCase aliases (e.g., add alias =
"markerStartShape" for marker_start_shape and alias = "markerEndShape" for
marker_end_shape) while preserving rename = "..." and default so both snake_case
and camelCase payloads are accepted.
In `@docs/wg/feat-2d/curve-decoration.md`:
- Around line 397-410: The fenced code block containing the pseudo-schema
(showing MarkerGlyph and CurveDecoration) needs a language identifier to satisfy
MD040; update the block fence to use a plaintext identifier (e.g., ```text) so
the block starts with ```text followed by the existing
MarkerGlyph/CurveDecoration content and ends with ``` to silence the lint
warning.
🧹 Nitpick comments (6)
editor/scaffolds/sidecontrol/controls/icons/stroke-decoration-icons.tsx (1)
195-217: Inconsistent rendering technique between fixed and responsive vertical-bar icons.The fixed
StrokeDecorationVerticalBarFilledIconusesfill="none"with a stroked<path>(line 206–214), while the responsiveMarkerVerticalBarFilledusesfill="currentColor"with a<rect>(line 327–333). At small sizes the stroked variant will appear thicker/thinner than the filled rect depending on the stroke width, producing a visual mismatch between the dropdown item and the trigger preview.Consider aligning both to use the same approach (filled
<rect>is simpler and matches the other filled markers).♻️ Suggested: use a filled rect for the fixed icon as well
export function StrokeDecorationVerticalBarFilledIcon({ className, }: { className?: string; }) { const w = LINE_STROKE * 1.4; const h = w * 5; const left = 24 - w; - const top = LINE_Y - h / 2; - const bottom = LINE_Y + h / 2; return ( - <svg {...svgProps} className={cn(iconSlotClass, className)} fill="none"> + <svg {...svgProps} className={cn(iconSlotClass, className)} fill="currentColor"> <LineSegment /> - <path - d={`M${LINE_X2} ${LINE_Y} L${left} ${LINE_Y} L${left} ${top} L24 ${top} L24 ${bottom} L${left} ${bottom} L${left} ${LINE_Y}`} - stroke="currentColor" - strokeWidth={LINE_STROKE} - strokeLinecap="butt" - strokeLinejoin="miter" - /> + <rect x={left} y={LINE_Y - h / 2} width={w} height={h} /> </svg> ); }Also applies to: 322-336
packages/grida-canvas-schema/grida.ts (1)
2409-2419: Consider extracting a sharedIStrokeMarkerinterface.Both
VectorNode(lines 2409–2419) andLineNode(lines 2448–2457) declare identicalmarker_start_shape/marker_end_shapefields with the same types and docs. A small shared interface would reduce duplication and make it easier to add marker properties consistently to future node types (e.g., ifBooleanPathOperationNodeever needs markers).♻️ Example shared interface
// In the `i` namespace alongside IStroke: export interface IStrokeMarker { /** * Marker shape at the start endpoint. * `@default` "none" */ marker_start_shape?: cg.StrokeMarkerPreset; /** * Marker shape at the end endpoint. * `@default` "none" */ marker_end_shape?: cg.StrokeMarkerPreset; }Then have
VectorNodeandLineNodeextendi.IStrokeMarker.Also applies to: 2448-2457
editor/grida-canvas/editor.i.ts (1)
3626-3633: Stroke decoration methods look good; consider addingNodeID[]overload for batch operations.The methods follow the established
changeNodeProperty*naming convention, use the correctcg.StrokeMarkerPresettype, and are logically placed alongside other stroke properties. However,changeNodePropertyStrokesandchangeNodePropertyFillsboth supportNodeID[]overloads for batch operations, while these decoration methods do not. If batch marker changes are needed (e.g., applying the same marker to multiple selected nodes), adding the array overload would improve API consistency.crates/grida-canvas/examples/golden_stroke_decoration_presets.rs (1)
31-39: Avoid duplicated preset labels to prevent drift.Line 31/123: The labels are defined twice. Consider driving the label list from the same
presetsvector used to build the scene.Also applies to: 123-131
crates/grida-canvas/src/painter/layer.rs (2)
425-427: Consider aDefaultimpl or constructor to reduce boilerplate for non-decorated nodes.The triple
marker_start_shape: StrokeMarkerPreset::None, marker_end_shape: StrokeMarkerPreset::None, stroke_width: 0.0is repeated across ~10 node variants. A small helper (e.g., aDefaultderivation onPainterPictureShapeLayeror a builder method) would reduce noise and make it harder to forget updating all sites if new decoration fields are added later.Also applies to: 488-490, 541-543, 588-590, 635-637, 682-684, 729-731, 910-912, 999-1001, 1032-1034
157-162: Thestroke_widthfield onPainterPictureShapeLayermay be confusing given the existingstroke_pathcontext.This field is specifically for decoration sizing, not for stroke geometry computation. Consider renaming it to something like
decoration_stroke_widthormarker_stroke_widthto clarify its purpose and avoid ambiguity with the stroke width already baked intostroke_path. Note thatPainterPictureVectorLayeralready has its ownstroke_widthfield (line 194) used for stroke geometry, so the semantic overlap is limited to the shape layer — but a clearer name would help.
| #[serde( | ||
| rename = "marker_start_shape", | ||
| default | ||
| )] | ||
| pub marker_start_shape: Option<StrokeMarkerPreset>, | ||
| #[serde( | ||
| rename = "marker_end_shape", | ||
| default | ||
| )] | ||
| pub marker_end_shape: Option<StrokeMarkerPreset>, |
There was a problem hiding this comment.
Add camelCase aliases for marker_start_shape/end_shape to preserve JSON compatibility.
Line 834/839: Most JSON fields accept both snake_case and camelCase. Without aliases, payloads that use markerStartShape/markerEndShape will silently drop decorations.
🛠️ Proposed fix
#[serde(
- rename = "marker_start_shape",
+ rename = "marker_start_shape",
+ alias = "markerStartShape",
default
)]
pub marker_start_shape: Option<StrokeMarkerPreset>,
#[serde(
- rename = "marker_end_shape",
+ rename = "marker_end_shape",
+ alias = "markerEndShape",
default
)]
pub marker_end_shape: Option<StrokeMarkerPreset>,🤖 Prompt for AI Agents
In `@crates/grida-canvas/src/io/io_grida.rs` around lines 834 - 843, The serde
attributes on the fields marker_start_shape and marker_end_shape currently only
accept snake_case names and thus drop camelCase JSON keys; update their
#[serde(...)] on the structs containing pub marker_start_shape:
Option<StrokeMarkerPreset> and pub marker_end_shape: Option<StrokeMarkerPreset>
to include camelCase aliases (e.g., add alias = "markerStartShape" for
marker_start_shape and alias = "markerEndShape" for marker_end_shape) while
preserving rename = "..." and default so both snake_case and camelCase payloads
are accepted.
| ``` | ||
| MarkerGlyph | ||
| ├─ geometry: Path/Primitive | ||
| ├─ anchor: Point | ||
| ├─ forward_axis: Vec2 | ||
| └─ cutback_depth: f32 (computed from geometry + stroke width) | ||
|
|
||
| CurveDecoration | ||
| ├─ glyph: MarkerGlyph | ||
| ├─ placement: Placement | ||
| ├─ orient: OrientationPolicy | ||
| ├─ scale: ScalePolicy | ||
| └─ offset: Offset | ||
| ``` |
There was a problem hiding this comment.
Add a language identifier to the fenced code block.
Static analysis flags this block (MD040). Since it's a pseudo-schema rather than a specific language, use text or plaintext as the language identifier.
-```
+```text
MarkerGlyph🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 397-397: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@docs/wg/feat-2d/curve-decoration.md` around lines 397 - 410, The fenced code
block containing the pseudo-schema (showing MarkerGlyph and CurveDecoration)
needs a language identifier to satisfy MD040; update the block fence to use a
plaintext identifier (e.g., ```text) so the block starts with ```text followed
by the existing MarkerGlyph/CurveDecoration content and ends with ``` to silence
the lint warning.
1 participant
day-318-grida-canvas-curve-decoration.mp4
Curve Decorations (2D)
curve-decorationAbstract
This working group defines curve decorations: how arbitrary glyphs (arrowheads, dots, diamonds, dimension ticks) are attached to a 2D path and oriented by the path's local tangent/normal. Raster backends such as Skia only provide three native stroke caps (butt/round/square); custom endpoint styles are implemented as explicit marker geometry. The model unifies endpoint markers, mid-path markers, and repeated directional symbols under a single, arc-length-based placement and local-frame semantics, and is intended to be compatible with SVG markers, Skia, PDF, and CAD-style pipelines.
Motivation
Design tools commonly expose "line endpoints" like arrowheads, dots, diamonds, and dimension ticks. Those must be rendered as explicit marker geometry attached to a path, not as backend stroke caps.
This spec defines a renderer-agnostic model for attaching such glyphs to a 2D path using the path's local frame. It is intended to unify:
Path model and parameterization
Although we use "curve" informally, the engine deals with paths: piecewise contours comprised of line/quad/cubic segments.
We distinguish two parameterizations:
Curve parameter (segment parameter)
Each segment has its own parameter (e.g. Bézier$t\in[0,1]$ ). This is useful for geometry math but not stable for placement by distance.
Arc-length parameter (recommended for placement)
For placement, we use arc-length$\ell$ measured along a contour, or its normalized fraction $u\in[0,1]$ :
Unless explicitly stated otherwise, placement in this spec uses arc-length (absolute$\ell$ or normalized $u$ ).
Local frame on a path
For a placement position on a contour, we define:
Tangent
At arc-length$\ell$ , tangent is the direction of travel along the contour:
Normal (2D convention)
To make "normal offset" unambiguous, we define the left normal:
This means normal offsets are relative to the path direction. If orientation reverses$\hat{t}$ , $\hat{n}$ also flips.
Degenerate tangent fallback
Real paths can contain degenerate segments (zero length, repeated points) where the tangent is undefined. Implementations should:
This document treats the fallback as an implementation policy, but the behavior must be deterministic.
Marker glyph
A MarkerGlyph is geometry defined in its own local "marker space".
Required properties:
Optional properties:
This is conceptually similar to SVG
<marker>(refX/refYas anchor,orientas tangent alignment), but kept renderer-agnostic.Placement
A Placement determines where decorations appear.
Attachment domains
For a given contour:
Notes:
startandendcoincide; endpoint placement is typically ignored or treated asat(u=0).joinsimplies a piecewise path model; joins do not exist on a single analytic curve without segmentation.Arc-length vs parameter (important)
If a placement is specified by "parameter value" on a Bézier segment, it is not proportional to distance and is rarely what users expect. For design-tool semantics, arc-length placement should be the default.
If we ever expose curve-parameter placement, it should be a separate explicit mode (e.g.
at_param).Multi-contour paths
Paths may have multiple contours (subpaths). Placement resolution must specify whether it applies:
For initial 2D editor semantics, per contour is recommended.
Orientation policy
Orientation controls how marker space is rotated relative to the local frame.
Policies
orient="auto-start-reverse"Join tangent selection (for
joins)At a join there are two natural tangents:
For orientation at joins, define one of:
incomingoutgoingbisector(angle bisector between incoming/outgoing; may require miter-limit style clamping)Scale policy
Scale controls how marker glyphs size in world space.
If stroke width varies along the path (width profile),
stroke-relativeshould use the local effective width at the placement position.Offset
Offset is an optional translation relative to the local frame:
World translation contribution:
Offsets are essential for:
Transform composition (conceptual)
For an arc-length position$\ell$ , with position $p$ , tangent $\hat{t}$ , and marker anchor $a$ , the marker transform is conceptually:
Where:
autopoliciesnone,The exact multiplication order depends on engine conventions, but the intent is: anchor → scale → orient → place.
Rendering semantics (policy-level)
Marker glyphs are separate geometry. Two practical semantics matter:
Draw order
Default: stroke path first, then draw markers on top. This matches most design tools.
Cutback / trimming (optional)
Many arrowheads look visually incorrect if the base stroke runs under the marker. A common solution is to cut back the stroked path near endpoints by a marker-dependent distance before stroking.
Cutback is not required to define the decoration model, but should be acknowledged as a likely implementation policy.
Relationship to existing models
SVG markers
SVG is a specific instance of this model:
auto/auto-start-reverserefX/refYmarkerUnits(oftenstrokeWidth)Curve Decorations are intended to be at least as expressive, while remaining backend-agnostic.
Stroke caps (backend caps vs decorations)
Classic stroke caps (butt/round/square) are natively supported by many renderers (including Skia) and should usually remain a paint/stroke style property for performance and fidelity.
Custom "caps" (arrowheads, diamonds, circles, etc.) are best represented as endpoint placements of curve decorations.
In other words:
Minimal conceptual schema
Conceptually:
This decomposition covers common 2D editor needs without introducing overlapping primitives.
Implementation note (Skia viability)
Skia provides arc-length traversal utilities that return contour length$L$ and position plus tangent at distance $\ell$ . Endpoint and along-path placement are therefore straightforward: evaluate the local frame at $\ell=0$ and $\ell=L$ for endpoint markers, and negate the tangent for start markers when using
auto-start-reverse.Design goals and non-goals
Design goals
The model aims to be:
Non-goals (initial scope)
Out of scope for this spec version:
These can be layered later on top of the same "attach glyphs to a path" primitive.
Terminology
Summary by CodeRabbit
New Features
Documentation
Chores