Adopt the cascading "appearance" attribute in place of Vector::stroke and the "fill"/"paint" attributes - #4433
Adopt the cascading "appearance" attribute in place of Vector::stroke and the "fill"/"paint" attributes#4433Keavon wants to merge 10 commits into
Conversation
… all paint-writing nodes
…etadata, and editor, cascading from ancestors
…favor of the appearance attribute
…rder of the Fill and Stroke nodes
There was a problem hiding this comment.
All reported issues were addressed across 42 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Performance Benchmark Results
|
…reads into single walks
Performance Benchmark Results
|
…rit instead of blocking the cascade
There was a problem hiding this comment.
1 issue found across 10 files (changes from recent commits).
Confidence score: 3/5
- In
node-graph/nodes/path-bool/src/lib.rs, theGraphic::Graphicpropagation logic can drop a parent appearance when nestedAppearancecolumns contain empty rows, which can cause incorrect styling/render output in grouped vectors. Treat empty entries as missing and replace them with the parent appearance before the column-presence gate so inheritance is preserved.
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/nodes/path-bool/src/lib.rs">
<violation number="1" location="node-graph/nodes/path-bool/src/lib.rs:198">
P1: When a nested group has an `Appearance` column containing empty rows, its vectors lose the parent’s declared appearance. The `Graphic::Graphic` branch still gates propagation on column absence, so replace each empty row with the parent appearance, as the leaf branch and generic flattener do.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| let parent_opacity: f64 = graphic_list.attribute_cloned_or(ATTR_OPACITY, index, 1.); | ||
| let parent_fill: f64 = graphic_list.attribute_cloned_or(ATTR_OPACITY_FILL, index, 1.); | ||
| let layer_path: NodeIdPath = graphic_list.attribute_cloned_or_default(ATTR_EDITOR_LAYER_PATH, index); | ||
| let parent_appearance = graphic_list.attribute::<Appearance>(ATTR_APPEARANCE, index).and_then(Appearance::declared).cloned(); |
There was a problem hiding this comment.
P1: When a nested group has an Appearance column containing empty rows, its vectors lose the parent’s declared appearance. The Graphic::Graphic branch still gates propagation on column absence, so replace each empty row with the parent appearance, as the leaf branch and generic flattener do.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/nodes/path-bool/src/lib.rs, line 198:
<comment>When a nested group has an `Appearance` column containing empty rows, its vectors lose the parent’s declared appearance. The `Graphic::Graphic` branch still gates propagation on column absence, so replace each empty row with the parent appearance, as the leaf branch and generic flattener do.</comment>
<file context>
@@ -195,7 +195,7 @@ fn flatten_vector(graphic_list: &List<Graphic>) -> List<Vector> {
let parent_fill: f64 = graphic_list.attribute_cloned_or(ATTR_OPACITY_FILL, index, 1.);
let layer_path: NodeIdPath = graphic_list.attribute_cloned_or_default(ATTR_EDITOR_LAYER_PATH, index);
- let parent_appearance = graphic_list.attribute::<Appearance>(ATTR_APPEARANCE, index).cloned();
+ let parent_appearance = graphic_list.attribute::<Appearance>(ATTR_APPEARANCE, index).and_then(Appearance::declared).cloned();
let compose_parent = |mut item: Item<Vector>| {
</file context>
Performance Benchmark Results
|
Closes #2003
This introduces cascading inheritance of appearance, so any parent group will apply its appearance to shape layers (leaves in the tree) if it lacks an appearance, otherwise it does not. This matches SVG style inheritance, although it applies at the granularity of the appearance as a whole, not per-fill and per-stroke. Because this now opens the door to supporting multiple fills and strokes that was impossible until now.