Add Gradient in Stroke Node#3905
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces comprehensive gradient support for strokes, aligning their painting capabilities with existing fill functionalities. The changes span across the editor's UI, SVG export logic, and the GPU renderer, ensuring a consistent and fully functional experience for applying gradients to vector strokes. This enhancement provides greater creative flexibility by allowing users to define complex visual styles for their vector paths. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
mohan-bee
force-pushed
the
feat/stroke-gradient
branch
from
4c876fd to
dd27358
Compare
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request successfully integrates gradient support for strokes, unifying the paint model across strokes and fills. The changes are well-implemented across the UI, SVG export, and GPU renderer, demonstrating a comprehensive approach to the feature. The refactoring of the Stroke struct to use the Fill enum is a key improvement, and the corresponding updates in various modules are consistent and correct. The addition of backup inputs for color and gradient in the node properties ensures a smooth user experience when switching between fill types. Overall, the code is clean, follows good practices, and effectively delivers the intended functionality.
|
@Keavon review pls |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
9 issues found across 8 files
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="editor/src/messages/portfolio/document/data_panel/data_panel_message_handler.rs">
<violation number="1" location="editor/src/messages/portfolio/document/data_panel/data_panel_message_handler.rs:392">
P2: Stroke paint is reduced to solid/none in the data panel, so gradient stroke paint is displayed inaccurately.</violation>
</file>
<file name="editor/src/messages/portfolio/document/graph_operation/graph_operation_message_handler.rs">
<violation number="1" location="editor/src/messages/portfolio/document/graph_operation/graph_operation_message_handler.rs:499">
P1: SVG import stroke handling ignores gradient paints, causing gradient-stroked paths to lose intended stroke styling on import.</violation>
</file>
<file name="node-graph/libraries/vector-types/src/vector/style.rs">
<violation number="1" location="node-graph/libraries/vector-types/src/vector/style.rs:155">
P2: `From<f64>`/`From<Vec<f64>>` for `Fill` silently drops values by always returning `Fill::None`, masking type/wiring errors through implicit `Into<Fill>` coercion.</violation>
<violation number="2" location="node-graph/libraries/vector-types/src/vector/style.rs:316">
P1: Renaming serialized `Stroke.color` to `paint` without deserialization alias/migration drops legacy stroke colors when loading older documents.</violation>
<violation number="3" location="node-graph/libraries/vector-types/src/vector/style.rs:501">
P2: `Stroke::default()` now initializes with `Fill::None`, contradicting the documented UX requirement to start with an opaque default stroke paint.</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:2056">
P2: Stroke color inline controls are rendered even when the input is exposed to the graph, creating conflicting UI and updates.</violation>
<violation number="2" location="editor/src/messages/portfolio/document/node_graph/node_properties.rs:2061">
P2: Backup stroke fill state is written from stale pre-edit `fill2` instead of the newly selected picker value, causing type-toggle restore to lose the latest edit.</violation>
</file>
<file name="node-graph/nodes/vector/src/vector_nodes.rs">
<violation number="1" location="node-graph/nodes/vector/src/vector_nodes.rs:203">
P2: Stroke node now defaults to `Fill::None`, causing newly added strokes to be invisible by default (regression from visible black stroke).</violation>
<violation number="2" location="node-graph/nodes/vector/src/vector_nodes.rs:1242">
P2: `solidify_stroke` copies gradient paint without remapping to the original stroke bounds, which can visually shift/stretch gradients after conversion.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| if let usvg::Paint::Color(color) = &stroke.paint() { | ||
| modify_inputs.stroke_set(Stroke { | ||
| color: Some(usvg_color(*color, stroke.opacity().get())), | ||
| paint: Fill::Solid(usvg_color(*color, stroke.opacity().get())), |
There was a problem hiding this comment.
P1: SVG import stroke handling ignores gradient paints, causing gradient-stroked paths to lose intended stroke styling on import.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At editor/src/messages/portfolio/document/graph_operation/graph_operation_message_handler.rs, line 499:
<comment>SVG import stroke handling ignores gradient paints, causing gradient-stroked paths to lose intended stroke styling on import.</comment>
<file context>
@@ -496,7 +496,7 @@ fn import_usvg_node(
if let usvg::Paint::Color(color) = &stroke.paint() {
modify_inputs.stroke_set(Stroke {
- color: Some(usvg_color(*color, stroke.opacity().get())),
+ paint: Fill::Solid(usvg_color(*color, stroke.opacity().get())),
weight: stroke.width().get() as f64,
dash_lengths: stroke.dasharray().as_ref().map(|lengths| lengths.iter().map(|&length| length as f64).collect()).unwrap_or_default(),
</file context>
| /// Stroke color | ||
| pub color: Option<Color>, | ||
| /// Stroke paint (solid color or gradient) | ||
| pub paint: Fill, |
There was a problem hiding this comment.
P1: Renaming serialized Stroke.color to paint without deserialization alias/migration drops legacy stroke colors when loading older documents.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/libraries/vector-types/src/vector/style.rs, line 316:
<comment>Renaming serialized `Stroke.color` to `paint` without deserialization alias/migration drops legacy stroke colors when loading older documents.</comment>
<file context>
@@ -300,8 +312,8 @@ fn daffine2_identity() -> DAffine2 {
- /// Stroke color
- pub color: Option<Color>,
+ /// Stroke paint (solid color or gradient)
+ pub paint: Fill,
/// Line thickness
pub weight: f64,
</file context>
editor/src/messages/portfolio/document/data_panel/data_panel_message_handler.rs
Show resolved
Hide resolved
| } | ||
| } | ||
|
|
||
| impl From<f64> for Fill { |
There was a problem hiding this comment.
P2: From<f64>/From<Vec<f64>> for Fill silently drops values by always returning Fill::None, masking type/wiring errors through implicit Into<Fill> coercion.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/libraries/vector-types/src/vector/style.rs, line 155:
<comment>`From<f64>`/`From<Vec<f64>>` for `Fill` silently drops values by always returning `Fill::None`, masking type/wiring errors through implicit `Into<Fill>` coercion.</comment>
<file context>
@@ -152,6 +152,18 @@ impl From<Gradient> for Fill {
}
}
+impl From<f64> for Fill {
+ fn from(_: f64) -> Fill {
+ Fill::None
</file context>
editor/src/messages/portfolio/document/node_graph/node_properties.rs
Outdated
Show resolved
Hide resolved
editor/src/messages/portfolio/document/node_graph/node_properties.rs
Outdated
Show resolved
Hide resolved
mohan-bee
force-pushed
the
feat/stroke-gradient
branch
from
dd27358 to
871468d
Compare
| let color = color_widget( | ||
| ParameterWidgetsInfo::new(node_id, ColorInput::INDEX, true, context), | ||
| crate::messages::layout::utility_types::widgets::button_widgets::ColorInput::default(), | ||
| let fill = document_node |
There was a problem hiding this comment.
This giant block of code starting here should not be duplicated.
There was a problem hiding this comment.
You're right @Keavon , thanks for catching that! I'll refactor this by extracting the shared fill/gradient widget-building logic into a common helper function (something like fill_color_widgets()) that both fill_properties and stroke_properties can call, so the code isn't duplicated. I hope this resolves.
Keavon
force-pushed
the
master
branch
2 times, most recently
from
d6228da to
e58c1de
Compare
|
!build (Run ID 23170598336) |
|
There was a problem hiding this comment.
1 issue found across 8 files (changes from recent commits).
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:1712">
P2: Standalone Fill::Gradient rendering to Vello collapses gradients to a single stop color, so GPU output renders a flat color instead of the intended gradient.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| let color = match &row.element { | ||
| vector_types::vector::style::Fill::None => continue, | ||
| vector_types::vector::style::Fill::Solid(color) => *color, | ||
| vector_types::vector::style::Fill::Gradient(gradient) => gradient.stops.color.first().copied().unwrap_or(Color::MAGENTA), |
There was a problem hiding this comment.
P2: Standalone Fill::Gradient rendering to Vello collapses gradients to a single stop color, so GPU output renders a flat color instead of the intended gradient.
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 1712:
<comment>Standalone Fill::Gradient rendering to Vello collapses gradients to a single stop color, so GPU output renders a flat color instead of the intended gradient.</comment>
<file context>
@@ -1607,11 +1607,153 @@ impl Render for Table<Color> {
+ let color = match &row.element {
+ vector_types::vector::style::Fill::None => continue,
+ vector_types::vector::style::Fill::Solid(color) => *color,
+ vector_types::vector::style::Fill::Gradient(gradient) => gradient.stops.color.first().copied().unwrap_or(Color::MAGENTA),
+ };
+
</file context>
There was a problem hiding this comment.
This is a known limitation that the renderer currently falls back to the first stop color for standalone gradient layers. A proper fix requires a different approach. Leaving this as a known stub for now.
2 participants
closes #3816
I added gradient support to strokes, unifying strokes and fills under the same paint model, and updated the ui, svg export, and GPU renderer to support it end-to-end.
demo:
Screen_Recording_2026-03-16_at_12.34.47_PM.mov