Add Table<GradientStops> gradient rendering#3989
Add Table<GradientStops> gradient rendering#3989YohYamasaki wants to merge 2 commits intoGraphiteEditor:masterfrom
Conversation
* Add SVG and Vello renderers for Table<GradientStops> * Add thumbnail rendering for Table<GradientStops> * Use row transform to map (0,0), (1,0) unit line to document space * Set 100px width for the initially created gradient * Add support of table gradients for the gradient tool
There was a problem hiding this comment.
Code Review
This pull request introduces support for GradientTable (a table of GradientStops) throughout the editor and rendering pipeline. Key changes include the addition of the GradientTableSet message, updates to the gradient tool to support table-based gradients, and the implementation of Vello rendering for these tables using manual encoding. Furthermore, a thumbnail_bounding_box method has been added to the BoundingBox trait to refine bounding box calculations for thumbnails. Review feedback highlights an inconsistency in default transform scales and potential numeric stability issues in SVG rendering for infinite fills.
There was a problem hiding this comment.
5 issues found across 19 files
Confidence score: 3/5
- There is concrete user-facing regression risk: in
node-graph/libraries/graphic-types/src/artboard.rs,Artboard::thumbnail_bounding_boxcurrently mirrorsbounding_box, which can miss child thumbnail-specific finite bounds (such as gradients) and produce incorrect thumbnails. - In
node-graph/libraries/rendering/src/renderer.rs, skipping gradient layers in Outline mode can make content disappear in that view, which is a noticeable behavior break even if normal rendering still works. - The fallback transform mismatch in
editor/src/messages/portfolio/document/node_graph/node_properties.rsversus the canonical default invalue.rs(identity vs 100x scale) adds inconsistency risk for empty gradient tables; maintainability drift is also signaled by duplicated thumbnail-bound logic innode-graph/libraries/core-types/src/table.rs. - Pay close attention to
node-graph/libraries/graphic-types/src/artboard.rs,node-graph/libraries/rendering/src/renderer.rs,editor/src/messages/portfolio/document/node_graph/node_properties.rs,node-graph/libraries/core-types/src/table.rs, andeditor/src/messages/tool/tool_messages/gradient_tool.rs- thumbnail bounds, outline visibility, default transform consistency, and stale gradient tool UI state are the key risk areas.
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/renderer.rs">
<violation number="1" location="node-graph/libraries/rendering/src/renderer.rs:1698">
P2: Gradient layers are fully skipped in Outline mode, causing them to disappear instead of rendering any outline/preview representation.</violation>
</file>
<file name="node-graph/libraries/core-types/src/table.rs">
<violation number="1" location="node-graph/libraries/core-types/src/table.rs:150">
P2: `thumbnail_bounding_box` duplicates `bounding_box` aggregation logic, increasing risk of future behavioral drift between normal and thumbnail bounds.</violation>
</file>
<file name="node-graph/libraries/graphic-types/src/artboard.rs">
<violation number="1" location="node-graph/libraries/graphic-types/src/artboard.rs:59">
P1: `Artboard::thumbnail_bounding_box` is a no-op wrapper around `bounding_box`, bypassing child thumbnail-specific finite bounds (e.g., gradients).</violation>
</file>
<file name="editor/src/messages/portfolio/document/node_graph/node_properties.rs">
<violation number="1" location="editor/src/messages/portfolio/document/node_graph/node_properties.rs:1156">
P2: The default fallback transform here is `DAffine2::IDENTITY` (scale 1), but the canonical default for a `GradientTable` in `value.rs` uses `DAffine2::from_scale(DVec2::splat(100.))`. If the table is empty when this runs, the `on_update` closure will reconstruct it with a scale-1 transform, causing the gradient to render 100× smaller than expected.</violation>
</file>
<file name="editor/src/messages/tool/tool_messages/gradient_tool.rs">
<violation number="1" location="editor/src/messages/tool/tool_messages/gradient_tool.rs:140">
P2: Gradient-table mode can still show radial-only controls from stale global state, and those actions do not apply to table-backed gradients.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
* Thumbnail rendering of artboard with infinite gradient layer * Hide radial gradient's reverse direction button for gradient table * Remove unused imports
Part of #2779
This PR fixes the renderer of
Table<GradientStops>and allows using the gradient tool for the gradient position control.Demo
Screen.Recording.2026-04-03.at.13.52.06.mp4
Notes
Table's transform for the control geometry instead of the currentGradientstruct's start/end. The initial transform, created upon node insertion, places the gradient from (0, 0) to (100, 0).BoundingBox::bounding_boxfor infinite footprint layers yet correctly render the contents in a thumbnail, addedBoundingBox::thumbnail_bounding_box, which virtually applies the boundary based on the stops of a gradient.Table<GradientStops>andGradientinternally to reuse existing logic. This should be refactored when it becomes ready to useTable<GradientStops>for all gradient use cases.Table.