Brush stroke types

[timon-schelling]

No description provided.

[cubic-dev-ai]

7 issues found across 18 files

Confidence score: 2/5

• node-graph/libraries/rendering/src/renderer.rs leaves List<Stroke> and Graphic::StrokeList rendering as no-ops, so brush strokes may disappear from SVG and Vello output; implement both rendering paths before relying on this representation.
• node-graph/libraries/brush-types/src/lib.rs computes bounds from centerline samples only, which can clip nonzero-diameter strokes and their layer extents; include the painted stroke footprint in the bounds calculation.
• node-graph/libraries/graphic-types/src/graphic.rs overwrites paths when flattening stroke lists from different layers, potentially assigning every stroke to item 0’s editor path; keep the synthetic wrapper anonymous so each stroke preserves its own path.
• node-graph/graph-craft/src/document/value.rs cannot create an empty default List<Stroke>, while sample_lerp and sample can panic at boundary or invalid indices; add the missing default conversion and guard these sampling APIs.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="node-graph/libraries/rendering/src/render_ext.rs">

<violation number="1" location="node-graph/libraries/rendering/src/render_ext.rs:298">
P2: Custom agent: **PR title enforcement**

The PR title `Brush stroke types` is a three-word noun phrase, not an imperative title, and falls below the five-word minimum. Rename it to `Add support for brush stroke types`.</violation>
</file>

<file name="node-graph/libraries/brush-types/src/lib.rs">

<violation number="1" location="node-graph/libraries/brush-types/src/lib.rs:73">
P2: Calling `sample` with an out-of-range index panics, including `Stroke::default().sample(0)`. Return an `Option<Sample>` or `Result` and handle invalid channel lengths instead of indexing unchecked.

(Based on your team's feedback about avoiding panics in application code.) [b5917bc3-2dbc-49a4-8246-5cad0a2e4976]</violation>

<violation number="2" location="node-graph/libraries/brush-types/src/lib.rs:85">
P2: `sample_lerp(index, t)` always indexes `index + 1`, so it panics when called with the last sample index (`index == len - 1`). Clamp or guard the upper bound so the method degrades to the last sample instead of panicking, and document the valid `index` range.</violation>

<violation number="3" location="node-graph/libraries/brush-types/src/lib.rs:105">
P2: When a brush stroke has nonzero diameter, this bound covers only centerline sample points. `BrushStrokes` stores diameter on the containing item, so clipping and layer bounds exclude the painted footprint; add style-aware inflation at the item level.</violation>
</file>

<file name="node-graph/libraries/graphic-types/src/graphic.rs">

<violation number="1" location="node-graph/libraries/graphic-types/src/graphic.rs:457">
P2: When a stroke list contains items from different layer paths, this wrapper overwrites every stroke's path with item 0's path during flattening. Keep the synthetic wrapper anonymous so each stroke retains its own editor routing metadata.</violation>
</file>

<file name="node-graph/graph-craft/src/document/value.rs">

<violation number="1" location="node-graph/graph-craft/src/document/value.rs:309">
P2: When a `List<Stroke>` input needs a default, `TaggedValue::from_type(&list!(Stroke))` returns `None`, so new brush nodes receive `TaggedValue::None` instead of an empty stroke list. Add a `Stroke` case in the `Type::List` branch returning `TaggedValue::Strokes(Vec::new())`.</violation>
</file>

<file name="node-graph/libraries/rendering/src/renderer.rs">

<violation number="1" location="node-graph/libraries/rendering/src/renderer.rs:2540">
P1: The new `Render for List<Stroke>` implements `render_svg` and `render_to_vello` as no-ops, and the `Graphic::StrokeList(_) => ()` arms in `Render for Graphic` do likewise. Because the brush node (`node-graph/nodes/brush/src/lib.rs::brush_strokes`) returns `Graphic::from(strokes)` as a `Graphic::StrokeList`, any brush output flowing through these arms paints nothing in both SVG and Vello while still reporting a bounds, complexity, and metadata. This is a footgun: callers reasonably expect a `Render` impl to draw something. Either implement stroke rendering or leave Graphic::StrokeList out of the render matches until the brush rasterization path exists.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P1: The new Render for List<Stroke> implements render_svg and render_to_vello as no-ops, and the Graphic::StrokeList(_) => () arms in Render for Graphic do likewise. Because the brush node (node-graph/nodes/brush/src/lib.rs::brush_strokes) returns Graphic::from(strokes) as a Graphic::StrokeList, any brush output flowing through these arms paints nothing in both SVG and Vello while still reporting a bounds, complexity, and metadata. This is a footgun: callers reasonably expect a Render impl to draw something. Either implement stroke rendering or leave Graphic::StrokeList out of the render matches until the brush rasterization path exists.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/libraries/rendering/src/renderer.rs, line 2540:

<comment>The new `Render for List<Stroke>` implements `render_svg` and `render_to_vello` as no-ops, and the `Graphic::StrokeList(_) => ()` arms in `Render for Graphic` do likewise. Because the brush node (`node-graph/nodes/brush/src/lib.rs::brush_strokes`) returns `Graphic::from(strokes)` as a `Graphic::StrokeList`, any brush output flowing through these arms paints nothing in both SVG and Vello while still reporting a bounds, complexity, and metadata. This is a footgun: callers reasonably expect a `Render` impl to draw something. Either implement stroke rendering or leave Graphic::StrokeList out of the render matches until the brush rasterization path exists.</comment>

<file context>
@@ -2522,6 +2537,12 @@ fn render_raster_gpu_item_to_vello(item: ItemRef<'_, Raster<GPU>>, scene: &mut S
 	}
 }
 
+impl Render for List<brush_types::Stroke> {
+	fn render_svg(&self, _render: &mut SvgRender, _render_params: &RenderParams) {}
+
</file context>

[cubic-dev-ai]

P2: Custom agent: PR title enforcement

The PR title Brush stroke types is a three-word noun phrase, not an imperative title, and falls below the five-word minimum. Rename it to Add support for brush stroke types.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/libraries/rendering/src/render_ext.rs, line 298:

<comment>The PR title `Brush stroke types` is a three-word noun phrase, not an imperative title, and falls below the five-word minimum. Rename it to `Add support for brush stroke types`.</comment>

<file context>
@@ -294,7 +294,8 @@ impl RenderExt for List<Graphic> {
 			| Some(Graphic::GraphicList(_))
-			| Some(Graphic::TextList(_)) => {
+			| Some(Graphic::TextList(_))
+			| Some(Graphic::StrokeList(_)) => {
 				let bounds = if target == PaintTarget::Stroke {
 					// To prevent a wraparound artefact occurring when the tile boundary and the stroke region are perfectly aligned, the local coordinate is expanded slightly.
</file context>

[cubic-dev-ai]

P2: Calling sample with an out-of-range index panics, including Stroke::default().sample(0). Return an Option<Sample> or Result and handle invalid channel lengths instead of indexing unchecked.

(Based on your team's feedback about avoiding panics in application code.) [b5917bc3-2dbc-49a4-8246-5cad0a2e4976]

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/libraries/brush-types/src/lib.rs, line 73:

<comment>Calling `sample` with an out-of-range index panics, including `Stroke::default().sample(0)`. Return an `Option<Sample>` or `Result` and handle invalid channel lengths instead of indexing unchecked.

(Based on your team's feedback about avoiding panics in application code.) [b5917bc3-2dbc-49a4-8246-5cad0a2e4976]</comment>

<file context>
@@ -0,0 +1,131 @@
+		[self.pressure.len(), self.tilt.len(), self.twist.len(), self.time.len()].into_iter().flatten().all(|len| len == n)
+	}
+
+	pub fn sample(&self, index: usize) -> Sample {
+		Sample {
+			position: self.position[index],
</file context>

[cubic-dev-ai]

P2: When a brush stroke has nonzero diameter, this bound covers only centerline sample points. BrushStrokes stores diameter on the containing item, so clipping and layer bounds exclude the painted footprint; add style-aware inflation at the item level.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/libraries/brush-types/src/lib.rs, line 105:

<comment>When a brush stroke has nonzero diameter, this bound covers only centerline sample points. `BrushStrokes` stores diameter on the containing item, so clipping and layer bounds exclude the painted footprint; add style-aware inflation at the item level.</comment>

<file context>
@@ -0,0 +1,131 @@
+}
+
+impl BoundingBox for Stroke {
+	fn bounding_box(&self, transform: DAffine2, _include_stroke: bool) -> RenderBoundingBox {
+		let Some(first) = self.position.first() else { return RenderBoundingBox::None };
+		let (min, max) = self.position.iter().fold((*first, *first), |(min, max), &point| (min.min(point), max.max(point)));
</file context>

[cubic-dev-ai]

P2: When a stroke list contains items from different layer paths, this wrapper overwrites every stroke's path with item 0's path during flattening. Keep the synthetic wrapper anonymous so each stroke retains its own editor routing metadata.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/libraries/graphic-types/src/graphic.rs, line 457:

<comment>When a stroke list contains items from different layer paths, this wrapper overwrites every stroke's path with item 0's path during flattening. Keep the synthetic wrapper anonymous so each stroke retains its own editor routing metadata.</comment>

<file context>
@@ -431,6 +452,17 @@ impl IntoGraphicList for List<Gradient> {
 
+impl IntoGraphicList for List<Stroke> {
+	fn into_graphic_list(self) -> List<Graphic> {
+		let layer_path: NodeIdPath = self.attribute_cloned_or_default(ATTR_EDITOR_LAYER_PATH, 0);
+		let mut graphic_list = List::new_from_element(Graphic::StrokeList(self));
+		if !layer_path.0.is_empty() {
</file context>

Suggested change

	let layer_path: NodeIdPath = self.attribute_cloned_or_default(ATTR_EDITOR_LAYER_PATH, 0);
	let mut graphic_list = List::new_from_element(Graphic::StrokeList(self));
	if !layer_path.0.is_empty() {
	graphic_list.set_attribute(ATTR_EDITOR_LAYER_PATH, 0, layer_path);
	}
	\t\tList::new_from_element(Graphic::StrokeList(self))

[cubic-dev-ai]

P2: When a List<Stroke> input needs a default, TaggedValue::from_type(&list!(Stroke)) returns None, so new brush nodes receive TaggedValue::None instead of an empty stroke list. Add a Stroke case in the Type::List branch returning TaggedValue::Strokes(Vec::new()).

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/graph-craft/src/document/value.rs, line 309:

<comment>When a `List<Stroke>` input needs a default, `TaggedValue::from_type(&list!(Stroke))` returns `None`, so new brush nodes receive `TaggedValue::None` instead of an empty stroke list. Add a `Stroke` case in the `Type::List` branch returning `TaggedValue::Strokes(Vec::new())`.</comment>

<file context>
@@ -295,6 +306,7 @@ macro_rules! tagged_value {
 					Self::BoxCorners(_) => item!(BoxCorners),
 					Self::GradientRamp(_) => item!(Gradient),
 					Self::BrushStrokes(_) => item!(BrushTrace),
+					Self::Strokes(_) => list!(Stroke),
 					// =======================
 					// AUTO-GENERATED VARIANTS
</file context>

[cubic-dev-ai]

P2: sample_lerp(index, t) always indexes index + 1, so it panics when called with the last sample index (index == len - 1). Clamp or guard the upper bound so the method degrades to the last sample instead of panicking, and document the valid index range.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/libraries/brush-types/src/lib.rs, line 85:

<comment>`sample_lerp(index, t)` always indexes `index + 1`, so it panics when called with the last sample index (`index == len - 1`). Clamp or guard the upper bound so the method degrades to the last sample instead of panicking, and document the valid `index` range.</comment>

<file context>
@@ -0,0 +1,131 @@
+
+	pub fn sample_lerp(&self, index: usize, t: f32) -> Sample {
+		let a = self.sample(index);
+		let b = self.sample(index + 1);
+		Sample {
+			position: a.position.lerp(b.position, t as f64),
</file context>