Grida Canvas - Advanced Stroke Styles dasharray, join, cap and stroke rect width
#444
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
WalkthroughIntroduces a unified stroke system: per-side (rectangular) stroke widths, composite StrokeStyle (align/cap/join/miter/dash), new StrokeWidth/StrokeDashArray types, expanded stroke_geometry APIs and rendering paths, plus editor UI/commands and schema updates to expose the new stroke model. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Editor UI
participant Reducer as Reducer / Store
participant Editor as EditorDocumentStore
participant Painter as Painter
participant Shape as Shape Module
UI->>Reducer: changeNodePropertyStrokeTopWidth(nodeId, value)
Reducer->>Editor: dispatch node/change/strokeTopWidth
Editor->>Reducer: update node model (strokeTopWidth)
UI->>Painter: request renderScene()
Painter->>Shape: request stroke path for Rectangle node
Shape->>Shape: if rectangular_stroke_width() present
alt rectangular stroke
Shape->>Shape: stroke_geometry_rectangular(bounds, rect_stroke, radii, align, miter, dash)
else uniform stroke
Shape->>Shape: stroke_geometry(path, width, align, cap, join, miter, dash)
end
Shape-->>Painter: path (filled stroke geometry)
Painter-->>UI: rendered canvas
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Areas requiring extra attention:
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ 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 |
…e and add comprehensive tests. Improved handling of negative values, zero lengths, and odd-length duplication for dash patterns.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/grida-canvas-schema/grida.ts (1)
2557-2578: Rectangle default should also include miter limit.We now require
strokeJoinand supportstrokeMiterLimit, but rectangle prototypes still rely on implicit defaults. Please addstrokeMiterLimit: 4(or the canonical default) alongsidestrokeJoinso freshly created rectangles start from a valid state and match the renderer’s expectations.crates/grida-canvas/src/shape/stroke.rs (1)
88-113: Handle empty normalized dash patterns correctlyWhen
StrokeDashArray::normalized()returns an empty slice—e.g., for[0.0, 4.0]—we enter this branch, skip the path effect, and still strokesource_path. That produces a solid outline even though every dash length is zero, contradicting the new API docs (“all-zero dashes → invisible”) and SVG’s behavior (all dash lengths zero means nothing should render). Please detect this case before callingPathEffect::dashand return an empty path so open paths hide their stroke instead of reverting to solid. Something like:- if let Some(dashes) = stroke_dash_array { - if let Some(pe) = PathEffect::dash(&dashes.normalized(), 0.0) { + if let Some(dashes) = stroke_dash_array { + let normalized = dashes.normalized(); + if normalized.is_empty() { + if dashes.as_slice().iter().step_by(2).all(|&x| x == 0.0) { + return Path::new(); + } + } else if let Some(pe) = PathEffect::dash(&normalized, 0.0) { // Use a hairline StrokeRec for filtering to avoid double-width application let filter_rec = StrokeRec::new(InitStyle::Hairline); if let Some((dashed, _)) = - pe.filter_path(source_path, &filter_rec, source_path.bounds()) + pe.filter_path(source_path, &filter_rec, source_path.bounds()) { path_to_stroke = dashed.snapshot(); } } }This keeps invalid negative values (SVG says “render solid”) but makes zero-length dash arrays truly disappear.
crates/grida-canvas/src/node/schema.rs (1)
1207-1219: Missing rectangular_stroke_width() implementation for ImageNodeRec.ImageNodeRec uses
stroke_width: StrokeWidth(line 1159) which supports per-side rectangular strokes, but doesn't implementrectangular_stroke_width()like ContainerNodeRec and RectangleNodeRec do. This means the painter will always fall back to uniform stroke rendering even when rectangular stroke data is present.Apply this diff to add the missing implementation:
fn render_bounds_stroke_width(&self) -> f32 { if self.has_stroke_geometry() { self.stroke_width.max() } else { 0.0 } } + + fn rectangular_stroke_width(&self) -> Option<RectangularStrokeWidth> { + match &self.stroke_width { + StrokeWidth::Rectangular(rect_stroke) => Some(rect_stroke.clone()), + _ => None, + } + } }
🧹 Nitpick comments (6)
crates/grida-canvas/examples/golden_stroke_width_4.rs (1)
17-19: Remove unnecessary async wrapper.The
scene()function is markedasyncbut performs no asynchronous operations—it simply calls the synchronouscreate_stroke_width_4_scene(). This adds unnecessary complexity and may mislead readers about the function's behavior.Apply this diff to simplify:
-async fn scene() -> Scene { +fn scene() -> Scene { create_stroke_width_4_scene() }Then update
main:#[tokio::main] async fn main() { - let scene = scene().await; + let scene = scene();packages/grida-canvas-schema/grida.ts (1)
2089-2174: Confirm rectangle-like nodes always emit per-side widths.Extending
ImageNode,VideoNode, andContainerNodewithIRectangularStrokeWidthmakes sense, but none of these nodes currently expose stroke color/activation. Verify the renderer/editor consume these fields together; otherwise the extra surface might become dangling data. Consider lining this up with the planned stroke refactor before merging.packages/grida-cmath/index.ts (1)
5694-5729: Add domain guards for robustness in cmath.miter.Handle degenerate and non-finite inputs to avoid NaN/Infinity leaks and make behavior explicit.
export function ratio(angleDeg: number): number { - const halfRad = (angleDeg * Math.PI) / 360; - return 1 / Math.sin(halfRad); + if (!Number.isFinite(angleDeg)) return NaN; + if (angleDeg <= 0) return Infinity; // perfectly sharp corner + if (angleDeg >= 180) return 1; // flat join + const halfRad = (angleDeg * Math.PI) / 360; + const s = Math.sin(halfRad); + return s === 0 ? Infinity : 1 / s; } ... export function angle(ratio: number): number { - if (ratio <= 1) return 180; - return (2 * Math.asin(1 / ratio) * 180) / Math.PI; + if (!Number.isFinite(ratio) || ratio <= 0) return 180; + if (ratio <= 1) return 180; + const v = 1 / ratio; + // Clamp to [-1,1] for numerical safety + const clamped = Math.max(-1, Math.min(1, v)); + return (2 * Math.asin(clamped) * 180) / Math.PI; }crates/grida-canvas/examples/golden_vector_line.rs (1)
44-50: Minor polish: make miter limit explicit and reuse dash pattern.Clarity/perf tweaks, no behavior change.
- let dash_array = if i % 2 == 1 { - Some(StrokeDashArray::from(vec![10.0, 5.0])) - } else { - None - }; + // Reuse a shared dash pattern to avoid reallocations + static DASH: &[f32] = &[10.0, 5.0]; + let dash_array = if i % 2 == 1 { + Some(StrokeDashArray::from(DASH)) + } else { + None + }; let stroke_options = StrokeOptions { stroke_width: 2.0, stroke_align: StrokeAlign::Center, stroke_cap: StrokeCap::Round, stroke_join: StrokeJoin::Round, - stroke_miter_limit: StrokeMiterLimit::default(), + // Make the common miter limit explicit (≈29° cutoff) + stroke_miter_limit: StrokeMiterLimit::new(4.0), paints: Paints::new([Paint::from(color)]), width_profile: None, stroke_dash_array: dash_array, };Also applies to: 52-56, 59-60
editor/scaffolds/sidecontrol/controls/stroke-miter-limit.tsx (2)
35-46: Clamp input angle before converting to ratio.Defensive clamp avoids out-of-range values slipping through programmatically.
- const handleChange = (angleValue: number) => { + const handleChange = (angleValue: number) => { if (!onValueChange) return; - const ratio = cmath.miter.ratio(angleValue); + const a = cmath.clamp(angleValue, 1, 180); + const ratio = cmath.miter.ratio(a); onValueChange(ratio); };
48-57: Let me check howpaint.tsxandpercentage.tsxhandle mixed values with InputPropertyNumber:Let me check how other similar controls in the codebase handle mixed values with
InputPropertyNumber:Based on my verification, I can now provide the rewritten review comment:
Add degree icon and make step value explicit for UX clarity.
The
InputPropertyNumbercomponent already defaults tostep={1}and has full type support forgrida.mixedvalues. ThedisplayValuecorrectly returnsgrida.mixedunchanged, which the component'sTMixed<number | "">type accepts. The main enhancement is adding a visual degree indicator for better UX.return ( <InputPropertyNumber mode="fixed" type="number" placeholder="29" min={1} max={180} + step={1} + icon={<span>°</span>} value={displayValue} onValueChange={handleChange} /> );
| stroke_style: StrokeStyle { | ||
| stroke_align: Self::convert_stroke_align( | ||
| component | ||
| .stroke_align | ||
| .as_ref() | ||
| .map(|a| serde_json::to_string(a).unwrap_or_default()) | ||
| .unwrap_or_else(|| "CENTER".to_string()), | ||
| ), | ||
| stroke_cap: StrokeCap::default(), | ||
| stroke_join: StrokeJoin::default(), | ||
| stroke_miter_limit: StrokeMiterLimit::default(), | ||
| stroke_dash_array: component | ||
| .stroke_dashes | ||
| .clone() | ||
| .map(|v| v.into_iter().map(|x| x as f32).collect()), | ||
| }, | ||
| stroke_width: Self::build_unknown_stroke_width_from_figma(component.stroke_weight) | ||
| .into(), |
There was a problem hiding this comment.
Preserve Figma stroke cap/join/miter limit
Every stroke_style we build here (and in the other shape converters below) hardcodes StrokeCap::default(), StrokeJoin::default(), and StrokeMiterLimit::default(). That wipes out the real cap/join/miter data coming from Figma, so importing a file with round caps or bevel joins will silently revert to butt/miter/4.0. Please thread the actual stroke_cap, stroke_join, and stroke_miter_limit values from the Figma node into the new StrokeStyle, similar to how we already propagate stroke_dash_array, to avoid regressing fidelity.
🤖 Prompt for AI Agents
In crates/grida-canvas/src/io/io_figma.rs around lines 782 to 799, the
StrokeStyle is currently using StrokeCap::default(), StrokeJoin::default(), and
StrokeMiterLimit::default() which discards Figma's actual cap/join/miter
settings; thread the corresponding values from the Figma `component` into the
StrokeStyle instead (similar to stroke_dash_array): read component.stroke_cap,
component.stroke_join, and component.stroke_miter_limit, convert each to the
internal types (use or add conversion helpers like Self::convert_stroke_cap /
convert_stroke_join / convert_stroke_miter_limit or map the enums
appropriately), and assign those converted values to stroke_cap, stroke_join,
and stroke_miter_limit so the real Figma values are preserved.
| let size = Size { | ||
| width: bounds.width, | ||
| height: bounds.height, | ||
| }; | ||
| let stroke_path = Self::compute_rectangular_stroke_path( | ||
| &n.stroke_width, | ||
| &n.corner_radius, | ||
| &n.stroke_style, | ||
| &size, | ||
| &shape, |
There was a problem hiding this comment.
Rectangular stroke needs local-size input
size is built from get_world_bounds, but we later apply the node’s world transform again when drawing. For rotated or scaled containers, this double-applies the transform so per-side strokes end up inflated and misaligned. Please derive the dimensions in local space (e.g., from the pre-transform rect/shape or via a local-bounds query) before calling compute_rectangular_stroke_path.
🤖 Prompt for AI Agents
In crates/grida-canvas/src/painter/layer.rs around lines 356 to 365, the code
builds `size` from world bounds and passes it to
`compute_rectangular_stroke_path`, which double-applies the node's world
transform and inflates/misaligns per-side strokes for rotated/scaled containers;
change the code to derive the rectangle dimensions in local space (for example,
use the pre-transform rect/shape or query the node's local bounds, or transform
the world bounds by the inverse world transform) and construct `size` from those
local dimensions, then pass that local `size` (and any local corner radii) into
`compute_rectangular_stroke_path` so the stroke path is computed in node-local
space and not transformed twice.
| strokeWidth: [ | ||
| "rectangle", | ||
| "image", | ||
| "video", | ||
| "container", | ||
| "component", | ||
| "instance", | ||
| "vector", | ||
| "line", | ||
| "rectangle", | ||
| "ellipse", | ||
| "polygon", | ||
| "star", | ||
| ], |
There was a problem hiding this comment.
Remove duplicate "rectangle" entry.
The "rectangle" node type appears twice in the strokeWidth array (lines 93 and 101). This duplication is unnecessary and likely a copy-paste error.
Apply this diff to remove the duplicate:
strokeWidth: [
"rectangle",
"image",
"video",
"container",
"component",
"instance",
"vector",
"line",
- "rectangle",
"ellipse",
"polygon",
"star",
],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| strokeWidth: [ | |
| "rectangle", | |
| "image", | |
| "video", | |
| "container", | |
| "component", | |
| "instance", | |
| "vector", | |
| "line", | |
| "rectangle", | |
| "ellipse", | |
| "polygon", | |
| "star", | |
| ], | |
| strokeWidth: [ | |
| "rectangle", | |
| "image", | |
| "video", | |
| "container", | |
| "component", | |
| "instance", | |
| "vector", | |
| "line", | |
| "ellipse", | |
| "polygon", | |
| "star", | |
| ], |
🤖 Prompt for AI Agents
In editor/grida-canvas/utils/supports.ts around lines 92 to 105, the strokeWidth
array contains a duplicate "rectangle" entry; remove the redundant "rectangle"
so each node type appears only once in the array (delete the second occurrence)
and verify array formatting remains valid.
| strokeCap: [ | ||
| "container", | ||
| "rectangle", | ||
| "image", | ||
| "video", | ||
| "container", | ||
| "vector", | ||
| "line", | ||
| "rectangle", | ||
| "ellipse", | ||
| "polygon", | ||
| "star", | ||
| "text", | ||
| "component", | ||
| "instance", | ||
| "boolean", | ||
| ], |
There was a problem hiding this comment.
🧩 Analysis chain
Remove duplicate entries and verify strokeCap expansion.
There are duplicate node type entries in the strokeCap array:
- "container" appears on lines 218 and 222
- "rectangle" appears on lines 219 and 225
Additionally, the comment on lines 116-117 states that strokeCap "should be visible to editor only for polyline and line nodes (path-like nodes)," yet this canvas backend mapping includes many additional node types. Please verify this expansion is intentional for the canvas rendering backend.
Apply this diff to remove the duplicates:
strokeCap: [
"container",
"rectangle",
"image",
"video",
- "container",
"vector",
"line",
- "rectangle",
"ellipse",
"polygon",
"star",
"text",
"component",
"instance",
"boolean",
],🏁 Script executed:
#!/bin/bash
# Fetch the file and display the relevant sections
echo "=== Lines 110-120 (strokeCap comment context) ==="
sed -n '110,120p' editor/grida-canvas/utils/supports.ts
echo ""
echo "=== Lines 115-125 (dom_supports.strokeCap) ==="
sed -n '115,125p' editor/grida-canvas/utils/supports.ts
echo ""
echo "=== Lines 217-233 (canvas_supports.strokeCap) ==="
sed -n '217,233p' editor/grida-canvas/utils/supports.ts
echo ""
echo "=== Full strokeCap arrays with line numbers ==="
sed -n '116,130p' editor/grida-canvas/utils/supports.ts | cat -n
echo "---"
sed -n '217,233p' editor/grida-canvas/utils/supports.ts | cat -nLength of output: 2077
🏁 Script executed:
#!/bin/bash
# Search for comments or context around strokeCap in the file
echo "=== Search for strokeCap references with context ==="
rg -n -B3 -A3 "strokeCap" editor/grida-canvas/utils/supports.ts
echo ""
echo "=== Search for TODOs or FIXMEs related to stroke ==="
rg -n -i "todo|fixme" editor/grida-canvas/utils/supports.ts | grep -i stroke
echo ""
echo "=== Check if there are any comments about canvas-specific behavior ==="
sed -n '190,240p' editor/grida-canvas/utils/supports.ts | head -60Length of output: 1983
🏁 Script executed:
#!/bin/bash
# Check git history for strokeCap changes
echo "=== Git log for strokeCap changes ==="
git log -p --follow -S "strokeCap" -- editor/grida-canvas/utils/supports.ts | head -200
echo ""
echo "=== Check for any recent changes to this file ==="
git log --oneline -20 -- editor/grida-canvas/utils/supports.tsLength of output: 525
Remove duplicate entries in strokeCap array and document the canvas backend expansion.
Duplicates confirmed in editor/grida-canvas/utils/supports.ts lines 217-233:
- "container" appears twice (lines 218 and 222)
- "rectangle" appears twice (lines 219 and 225)
Additionally, the documented requirement (lines 116-117) states strokeCap "should be visible to editor only for polyline and line nodes," yet canvas_supports.strokeCap includes 15 node types. While this canvas-specific expansion may be intentional, it lacks documentation explaining why the canvas backend differs from the DOM backend constraint.
Apply the provided diff to remove duplicates and add a comment explaining the canvas backend expansion rationale.
🤖 Prompt for AI Agents
In editor/grida-canvas/utils/supports.ts around lines 217 to 233, remove the
duplicate "container" and "rectangle" entries from the strokeCap array and
collapse it to unique node types; then add a short inline comment above the
array explaining that the canvas backend intentionally expands strokeCap
visibility beyond the DOM/editor constraint (polyline/line) because canvas
rendering requires treating additional node types as strokable for accurate
visual output and hit-testing. Ensure the comment references the DOM/backend
difference and keep the array entries deduplicated and alphabetically or
logically ordered for readability.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
crates/grida-canvas/src/cg/types.rs (1)
1986-1988: Honor paint activity inPaints::is_visible.The implementation should use
paint.visible()instead of checking onlypaint.opacity() > 0.0. ThePaint::visible()method (lines 1867-1875) already combines bothactive()andopacity()checks, ensuring inactive paints with non-zero opacity are correctly treated as invisible.pub fn is_visible(&self) -> bool { - self.paints.iter().any(|paint| paint.opacity() > 0.0) + self.paints.iter().any(|paint| paint.visible()) }
🧹 Nitpick comments (4)
editor/grida-canvas/reducers/node.reducer.ts (2)
482-486: Consider clamping strokeMiterLimit to a minimum of 1.Per SVG and canvas graphics standards, miter limit should be >= 1. Other numeric properties with meaningful bounds (e.g.,
fontSizeat line 792,pointCountat line 340) are clamped usingranged()orcmath.clamp().Apply this diff to add minimum value validation:
strokeMiterLimit: defineNodeProperty<"strokeMiterLimit">({ apply: (draft, value, prev) => { - (draft as UN).strokeMiterLimit = value; + (draft as UN).strokeMiterLimit = ranged(1, value); }, }),
487-502: Consider adding defensive validation for strokeDashArray structure.While TypeScript types provide compile-time safety, adding runtime validation for array structure would be defensive against malformed data from external sources (e.g., imports, API responses).
Apply this diff to add optional runtime validation:
strokeDashArray: defineNodeProperty<"strokeDashArray">({ assert: (node) => node.type === "vector" || node.type === "line" || node.type === "rectangle" || node.type === "ellipse" || node.type === "polygon" || node.type === "star" || node.type === "svgpath" || node.type === "image" || node.type === "container" || node.type === "boolean", apply: (draft, value, prev) => { + // Validate array structure if provided + if (value !== null && value !== undefined) { + assert( + Array.isArray(value) && value.every((v) => typeof v === "number"), + "strokeDashArray must be an array of numbers" + ); + } (draft as UN).strokeDashArray = value; }, }),crates/grida-canvas/src/cg/types.rs (2)
544-544: AddEqandHashderives for consistency withStrokeCap.
StrokeJoinhas the same structure asStrokeCap(simple enum with no floating-point data) but is missing theEqandHashderives. This inconsistency limitsStrokeJoinfrom being used in hash-based collections and could confuse API users expecting parallel behavior.-#[derive(Debug, Clone, Copy, PartialEq, Deserialize)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Deserialize)] pub enum StrokeJoin {
610-642: ImproveStrokeMiterLimitencapsulation and safety.The current implementation has several design concerns:
Public field exposure: The tuple struct field is public (
pub f32), allowing direct mutation via.0which bypasses thevalue()accessor and any future validation. This breaks encapsulation.Missing validation: Neither
new()norFrom<f32>validate inputs. Negative or NaN values could create invalid state.Missing
Copyderive: As a simplef32wrapper, this type should deriveCopyfor ergonomics (likeStrokeCapandStrokeJoin).-#[derive(Debug, Clone, Copy, PartialEq, Deserialize)] -pub struct StrokeMiterLimit(#[serde(default = "StrokeMiterLimit::default_value")] pub f32); +#[derive(Debug, Clone, Copy, PartialEq, Deserialize)] +pub struct StrokeMiterLimit(#[serde(default = "StrokeMiterLimit::default_value")] f32); impl StrokeMiterLimit { pub const DEFAULT_VALUE: f32 = 4.0; /// Creates a new miter limit with the specified value. + /// + /// # Panics + /// Panics if limit is negative or NaN. pub const fn new(limit: f32) -> Self { + assert!(limit >= 0.0 && !limit.is_nan(), "Miter limit must be non-negative"); Self(limit) } impl From<f32> for StrokeMiterLimit { fn from(value: f32) -> Self { - Self(value) + Self::new(value) } }Note: The
assert!inconst fn newrequires Rust 1.57+. If targeting older versions, makenewnon-const and use runtime validation, or accept unchecked construction in const contexts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
crates/grida-canvas/src/cache/geometry.rs(6 hunks)crates/grida-canvas/src/cg/types.rs(2 hunks)editor/grida-canvas/reducers/node.reducer.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
crates/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep Rust crates under /crates; this is the monorepo’s Rust workspace for the Canvas implementation
Files:
crates/grida-canvas/src/cache/geometry.rscrates/grida-canvas/src/cg/types.rs
crates/grida-canvas/src/**/*.rs
📄 CodeRabbit inference engine (crates/grida-canvas/AGENTS.md)
crates/grida-canvas/src/**/*.rs: All internal structs (e.g., NodeRecs, SceneGraph, caches) must use NodeId (u64) for identifiers and not serialize them
Public APIs must accept and return UserNodeId (String) for stability and .grida serialization
Files:
crates/grida-canvas/src/cache/geometry.rscrates/grida-canvas/src/cg/types.rs
editor/grida-*/**
📄 CodeRabbit inference engine (AGENTS.md)
Use editor/grida-* directories to isolate domain-specific modules pending promotion to /packages
Files:
editor/grida-canvas/reducers/node.reducer.ts
🧠 Learnings (11)
📚 Learning: 2025-10-27T06:28:21.888Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas/AGENTS.md:0-0
Timestamp: 2025-10-27T06:28:21.888Z
Learning: Applies to crates/grida-canvas/src/**/*.rs : All internal structs (e.g., NodeRecs, SceneGraph, caches) must use NodeId (u64) for identifiers and not serialize them
Applied to files:
crates/grida-canvas/src/cache/geometry.rscrates/grida-canvas/src/cg/types.rs
📚 Learning: 2025-09-26T09:29:53.155Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas-wasm/AGENTS.md:0-0
Timestamp: 2025-09-26T09:29:53.155Z
Learning: Applies to crates/grida-canvas-wasm/{crates/grida-canvas-wasm/src/main.rs,**/grida-canvas-wasm.d.ts} : When introducing new public APIs in the WASM entrypoint (main.rs), update the TypeScript declarations in grida-canvas-wasm.d.ts to keep bindings in sync
Applied to files:
crates/grida-canvas/src/cache/geometry.rscrates/grida-canvas/src/cg/types.rseditor/grida-canvas/reducers/node.reducer.ts
📚 Learning: 2025-10-27T06:28:21.888Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas/AGENTS.md:0-0
Timestamp: 2025-10-27T06:28:21.888Z
Learning: Applies to crates/grida-canvas/src/**/*.rs : Public APIs must accept and return UserNodeId (String) for stability and .grida serialization
Applied to files:
crates/grida-canvas/src/cache/geometry.rscrates/grida-canvas/src/cg/types.rs
📚 Learning: 2025-09-15T10:31:32.864Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas-fonts/AGENTS.md:0-0
Timestamp: 2025-09-15T10:31:32.864Z
Learning: Applies to crates/grida-canvas-fonts/tests/ui_parser_test.rs : Place high-level UI API tests (parse_ui) in tests/ui_parser_test.rs
Applied to files:
crates/grida-canvas/src/cache/geometry.rs
📚 Learning: 2025-09-15T10:31:32.864Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas-fonts/AGENTS.md:0-0
Timestamp: 2025-09-15T10:31:32.864Z
Learning: Applies to crates/grida-canvas-fonts/tests/italic_level1.rs : Place core font selection and italic detection tests in tests/italic_level1.rs
Applied to files:
crates/grida-canvas/src/cache/geometry.rs
📚 Learning: 2025-09-15T10:31:32.864Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas-fonts/AGENTS.md:0-0
Timestamp: 2025-09-15T10:31:32.864Z
Learning: Applies to crates/grida-canvas-fonts/tests/scenario_*.rs : Name comprehensive scenario-specific integration tests as tests/scenario_*.rs
Applied to files:
crates/grida-canvas/src/cache/geometry.rs
📚 Learning: 2025-09-15T10:31:32.864Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas-fonts/AGENTS.md:0-0
Timestamp: 2025-09-15T10:31:32.864Z
Learning: Applies to crates/grida-canvas-fonts/tests/italic_name_checking.rs : Put name-based italic detection tests in tests/italic_name_checking.rs
Applied to files:
crates/grida-canvas/src/cache/geometry.rs
📚 Learning: 2025-10-14T08:23:46.382Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-14T08:23:46.382Z
Learning: Applies to docs/canvas/** : Avoid changes to docs/canvas (currently marked inactive) unless reactivated
Applied to files:
crates/grida-canvas/src/cg/types.rs
📚 Learning: 2025-10-16T02:50:45.759Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: editor/app/(tools)/tools/halftone/AGENTS.md:0-0
Timestamp: 2025-10-16T02:50:45.759Z
Learning: Applies to editor/app/(tools)/tools/halftone/app/(tools)/tools/halftone/_page.tsx : When adding a new shape, add a corresponding case in shapeToSVG() for SVG export
Applied to files:
editor/grida-canvas/reducers/node.reducer.ts
📚 Learning: 2025-10-16T02:50:45.759Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: editor/app/(tools)/tools/halftone/AGENTS.md:0-0
Timestamp: 2025-10-16T02:50:45.759Z
Learning: Applies to editor/app/(tools)/tools/halftone/app/(tools)/tools/halftone/_page.tsx : When adding a new shape, add a corresponding case in drawShape()
Applied to files:
editor/grida-canvas/reducers/node.reducer.ts
📚 Learning: 2025-10-16T02:50:45.759Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: editor/app/(tools)/tools/halftone/AGENTS.md:0-0
Timestamp: 2025-10-16T02:50:45.759Z
Learning: Applies to editor/app/(tools)/tools/halftone/app/(tools)/tools/halftone/_page.tsx : When adding a new shape, update the Shape type union
Applied to files:
editor/grida-canvas/reducers/node.reducer.ts
🧬 Code graph analysis (2)
crates/grida-canvas/src/cache/geometry.rs (3)
crates/grida-canvas-wasm/lib/index.ts (1)
Rectangle(33-38)packages/grida-canvas-cg/lib.ts (1)
StrokeAlign(230-230)crates/math2/src/rect.rs (1)
inflate(729-737)
crates/grida-canvas/src/cg/types.rs (2)
packages/grida-canvas-cg/lib.ts (2)
StrokeCap(212-212)StrokeJoin(224-224)crates/grida-canvas/src/node/schema.rs (5)
default(62-69)default(87-95)default(412-414)default(526-536)default(675-677)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: cargo test
- GitHub Check: test
🔇 Additional comments (9)
editor/grida-canvas/reducers/node.reducer.ts (1)
421-460: Per-side stroke widths are now properly clamped.All four per-side stroke width properties correctly use
ranged(0, value, editor.config.DEFAULT_MAX_STROKE_WIDTH), matching the existingstrokeWidthimplementation. This prevents negative or excessive width values from causing geometry issues.crates/grida-canvas/src/cache/geometry.rs (7)
14-14: LGTM: Import path updated to prelude.The import path change from
crate::cg::types::*tocrate::cg::prelude::*aligns with the unified stroke system introduced in this PR.
280-285: LGTM: BooleanOperation uses uniform stroke correctly.The render bounds calculation correctly uses
stroke_width.value_or_zero()andstroke_style.stroke_alignfor uniform stroke handling on boolean operations.
330-344: Previous issue resolved: Rectangular stroke widths now handled correctly.The Container render bounds computation now correctly branches based on whether rectangular stroke data exists, calling
compute_render_bounds_with_rectangular_strokewhen per-side widths are present. This addresses the previous review comment about dropped rectangular stroke widths on containers.
633-682: LGTM: Rectangular stroke bounds computation is correct.The new
compute_render_bounds_with_rectangular_strokefunction correctly implements per-side stroke bounds expansion for all three alignment modes:
- Center: Half-width inflation (stroke straddles boundary)
- Inside: No inflation (stroke contained within bounds)
- Outside: Full-width inflation (stroke extends outward)
The implementation uses
rect::inflateappropriately and applies effects after stroke adjustment, consistent with the uniform stroke path.
686-703: LGTM: Rectangle handles both uniform and per-side strokes.The Rectangle node correctly branches based on stroke type, using
compute_render_bounds_with_rectangular_strokefor per-side widths and falling back to uniform handling otherwise.
704-757: LGTM: Uniform stroke handling is consistent across shape types.All non-rectangular shape types (Ellipse, Polygon, Image, SVGPath, etc.) correctly use uniform stroke width via
render_bounds_stroke_width()or direct field access, paired withstroke_style.stroke_align. These shapes appropriately do not support per-side strokes.
758-775: LGTM: Container render bounds consistent across call sites.The
compute_render_boundsfunction correctly handles Container nodes with the same branching logic used inbuild_recursive(lines 330-344), ensuring rectangular stroke widths are properly computed regardless of the code path.crates/grida-canvas/src/cg/types.rs (1)
361-454: Excellent implementation with comprehensive documentation.The
StrokeCapenum is well-designed with:
- Clear, detailed documentation including visual examples and cross-platform references
- Appropriate derives for a public enum type
- Correct serde mappings that match the TypeScript definitions
- Standard default (Butt)
The extensive documentation is particularly valuable for a public API, making it easy for users to understand the visual differences between cap styles.
1 participant
Advanced Stroke models with lower level controls
Core
stroke_dash_arraystroke_capstroke_joinmiterbavelroundstroke_miter_limitstroke_width(each side)Editor
Impls
Pt1 stroke dash array
day-299-grida-canvas-stroke-pt1-dasharray.mp4
Pt2 stroke rect width
day-300-grida-canvas-stroke-pt2-stroke-rect.mp4
Pt3 join and miter limit
day-301-grida-canvas-stroke-pt3-stroke-join-and-miter-limit.mp4
Summary by CodeRabbit