Add renderer support for painting vector with "fill" and "stroke" attributes with graphic List<T> types#4111
Merged
Keavon merged 35 commits intoJun 11, 2026
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces functionality to convert Fill styles into graphical tables, including a new to_transform method for the Gradient struct and associated unit tests. Feedback suggests refactoring the private fill_to_paint function into an implementation of the IntoGraphicTable trait for Fill to align with existing architectural patterns and optimize performance by avoiding unnecessary clones.
YohYamasaki
force-pushed
the
render-fill-via-graphic
branch
3 times, most recently
from
1e575b6 to
28b9fba
Compare
YohYamasaki
force-pushed
the
render-fill-via-graphic
branch
from
dd79824 to
212e088
Compare
Contributor
Author
|
/gemini review |
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors the vector fill system to use a List representation, enabling complex fills such as vectors and rasters via clipping. It updates the SVG and Vello renderers to support the ATTR_FILL_GRAPHIC attribute and introduces a fill_graphic debug node. Review feedback identifies high-severity performance bottlenecks caused by frequent cloning of the graphic list in both render loops. Additionally, there is a discrepancy where the SVG renderer only processes the first fill element compared to Vello's multi-fill support, and a concern regarding SVG bloat from redundant clip path generation in complex meshes.
YohYamasaki
force-pushed
the
render-fill-via-graphic
branch
from
e045d30 to
12d6335
Compare
Keavon
force-pushed
the
master
branch
2 times, most recently
from
4b7a823 to
847b8e9
Compare
timon-schelling
force-pushed
the
master
branch
2 times, most recently
from
15fcaac to
d5f0140
Compare
YohYamasaki
force-pushed
the
render-fill-via-graphic
branch
2 times, most recently
from
9df8439 to
ac3f317
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 6 files
Confidence score: 3/5
- There is a concrete user-facing rendering risk: in
node-graph/libraries/rendering/src/render_ext.rs, an emptyList<Color>produces an empty string rather thanfill="none", so SVG elements can render black instead of transparent. - Given the high confidence (9/10) and meaningful severity (7/10), this is more than a minor edge case and could cause visible regressions in generated SVG output.
- Pay close attention to
node-graph/libraries/rendering/src/render_ext.rs- ensure empty color lists map to explicit transparent fill behavior (fill="none").
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
YohYamasaki
changed the title
Contributor
There was a problem hiding this comment.
4 issues found across 13 files
Confidence score: 3/5
- There is concrete rendering-behavior risk in
node-graph/libraries/graphic-types/src/graphic.rs:Graphic::is_opaque/is_fully_transparentreportedly ignoreATTR_OPACITYandATTR_OPACITY_FILL, which can misclassify partially transparent graphics as opaque. node-graph/libraries/graphic-types/src/graphic.rsalso appears to evaluate only the first paint entry inList<Graphic>, so multi-layer fill/stroke paint setups may produce incorrect opacity/transparent decisions.- This looks mergeable with caution, but the combined medium-to-high severity issues suggest user-visible rendering mismatches are plausible (including the gradient-bounds mismatch noted in
node-graph/libraries/rendering/src/renderer.rs). - Pay close attention to
node-graph/libraries/graphic-types/src/graphic.rsandnode-graph/libraries/rendering/src/renderer.rs- opacity classification and stroke gradient bounds may diverge from expected/SVG behavior.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
This exposes List's attributes to message handlers, enabling them to access the necessary attribute data such as ATTR_STROKE_PAINT_GRAPHIC as `Fill` and `Stroke` will not have paint information in the future.
Also extracts `fill_graphic_list_at` / `stroke_paint_graphic_list_at` to share the row-attribute lookup across the existing call sites.
This reverts commit 4285243
Add `fill_attributes` / `stroke_paint_attributes` to `DocumentMetadata` so the `ExpandFillStrokeOnSelectedLayers` handler can read row paint visibility without exposing entire `List<Vector>`.
Keavon
force-pushed
the
render-fill-via-graphic
branch
from
9bcc640 to
46b9874
Compare
Keavon
approved these changes
Jun 11, 2026
timon-schelling
pushed a commit
that referenced
this pull request
Jun 12, 2026
…ributes with graphic List<T> types (#4111) * Add conversion from Fill to Table<Graphic> * Refactor Vector vello renderer for Gradient / Color * Refactor Vector SVG renderer for Gradient / Color * Fix conflicts * Add basic clipping-based fill for SVG rendering * Use Cow to avoid cloning graphic list for fill * Cleanup for Cow usage * format code * Use `<pattern>` instead of `<clipPath>` for clip This simplifies the future implementation of clipping-based rendering for strokes, as the stroke does not support the use of a clip path but rather paint sources from a paint server. * Move svg pattern rendering function to RenderExt * Fix comment * Fix empty fill list rendering as default black Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> * Move opaque check function to Graphic impl * Add color converter and debug node to use graphic * WIP: Use List<Graphic> to render Color & Gradient * Use `Arc<List<Vector>>` for vector_data metadata This exposes List's attributes to message handlers, enabling them to access the necessary attribute data such as ATTR_STROKE_PAINT_GRAPHIC as `Fill` and `Stroke` will not have paint information in the future. * Recurse opacity checks on nested `Graphic` Also extracts `fill_graphic_list_at` / `stroke_paint_graphic_list_at` to share the row-attribute lookup across the existing call sites. * Fix fill and stroke visibility check degradation * Fix clipping based stroke paint positioning * Refactor vello renderer for stroke to use graphic * Reduce `Fill` / `Stroke.color` to `List<Graphic>` allocations * Revert "Use `Arc<List<Vector>>` for vector_data metadata" This reverts commit 4285243 * Expose paint row attributes as dedicated metadata for vectors Add `fill_attributes` / `stroke_paint_attributes` to `DocumentMetadata` so the `ExpandFillStrokeOnSelectedLayers` handler can read row paint visibility without exposing entire `List<Vector>`. * Fix transparency check to consider fill opacity * Fix consistency of gradient placement for SVG stroke * Rename `stroke_paint_..` to `stroke_..` * Remove debug nodes * Allow to use any graphic type without casting * Rename `fill_graphic` / `stroke_graphic` to `fill` / `stroke` * Fix SVG pattern placement when stroke transform differs from item transform * Fix click target fill check for empty list in graphic * Fix blank fill/stroke attribute masking legacy style * Fix SVG's paint order trick for vector/raster fills * Add zero-division guard for pattern wraparound prevention * Code review --------- Co-authored-by: Keavon Chambers <keavon@keavon.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Partly closes #2779
This PR adds foundation of the renderer to handle
List<Graphic>as a paint source of fill and stroke.Demo
render-fill-and-stroke-via-list-graphic.mp4
The demo file can be found in below.
render-fill-and-stroke-via-list-graphic.graphite.zip
Notes
Fill/ColortoList<Graphic>to avoid breaking current node flow related to theFillandStrokenode. This should be removed after the future refactoring of the node.ATTR_FILL = "fill"andATTR_STROKE = "stroke"attribute keys for storing the paintsList<T>, where T is the graphic types, on aList<Vector>item.List<Graphic>as a fill and stroke paint source.RenderExtimplementations forPathStyle,Fillare no longer needed, now we have implementation forList<Graphic>instead. The same forStrokeis now shrunk to only generate the shape related attributes.<pattern>element instead of the<clipPath>.