Skip to content

Add a gradient interpolation space attribute with sRGB Gamma (existing) and sRGB Linear (new) - #4412

Open
Keavon wants to merge 4 commits into
masterfrom
gradient-interpolation-space
Open

Add a gradient interpolation space attribute with sRGB Gamma (existing) and sRGB Linear (new)#4412
Keavon wants to merge 4 commits into
masterfrom
gradient-interpolation-space

Conversation

@Keavon

@Keavon Keavon commented Aug 5, 2026

Copy link
Copy Markdown
Member

Next comes the perceptual modes, at which point sRGB Linear won't be the default anymore, but this at least sets the stage for having a non-gamma default.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 issues found across 26 files

Confidence score: 3/5

  • In node-graph/libraries/vector-types/src/gradient.rs, treating missing gradient_interpolation as Default::default() (SrgbLinear) appears to erase the intended “field absent” meaning, which can silently change gradient behavior on read/round-trip — preserve an explicit missing/auto state instead of defaulting.
  • In editor/src/messages/portfolio/document/graph_operation/graph_operation_message_handler.rs, inline SVG styles with repeated color-interpolation can resolve to the first declaration instead of the last applicable one, so imported gradients may render in the wrong color space — update the resolver to follow CSS cascade order.
  • In editor/src/messages/portfolio/document/graph_operation/graph_operation_message_handler.rs, SVG import reads inline/presentation attributes but not CSS <style> blocks, so gradients from common exporters may import with incorrect interpolation — add <style> parsing support (or a clear fallback path with tests) to reduce compatibility risk.
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/graph_operation/graph_operation_message_handler.rs">

<violation number="1" location="editor/src/messages/portfolio/document/graph_operation/graph_operation_message_handler.rs:575">
P2: SVG gradients with repeated `color-interpolation` declarations in an inline style can import with the wrong interpolation space because the resolver keeps the first declaration; selecting the last applicable declaration would match the CSS cascade.</violation>

<violation number="2" location="editor/src/messages/portfolio/document/graph_operation/graph_operation_message_handler.rs:914">
P2: SVG import now resolves each gradient's interpolation from its inline `style` attribute and its `color-interpolation` presentation attribute, but it does not read CSS `<style>` blocks. Many exported SVGs (Illustrator/Inkscape) set `color-interpolation: linearRGB` through grouped stylesheet rules, and those gradients would fall through to the `SrgbGamma` default on import, silently discarding the author's declared linear interpolation. Consider also scanning `<style>` elements (and matching the selectors, or at least any `color-interpolation` declared there) so the import honors them rather than silently defaulting to gamma.</violation>
</file>

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

<violation number="1" location="node-graph/libraries/vector-types/src/gradient.rs:179">
P2: The item/attribute read path resolves a missing `gradient_interpolation` to `Default::default()` = `SrgbLinear`, which contradicts the design intent (and the doc comment on this struct) that the field's absence marks a pre-existing ramp and should read as the legacy `SrgbGamma`. The serde exchange form honors this via `serde(default = "GradientInterpolation::legacy_gamma")`, but the path the actual renderers and node graph use does not: `From<GradientRamp> for Item` writes the attribute only when non-default (`if !is_default()`), so both a newly created Linear ramp and every legacy gamma ramp leave the attribute absent, and both resolve to Linear. As a result, gradients in documents saved before this field existed will flip from gamma to linear rendering the moment one of the renderer paths (renderer.rs lines ~511/2181/2279 or render_ext.rs ~99) reads them - an unintended visual change to existing artwork. Because the attribute form can't distinguish "new Linear" from "legacy gamma", the fix needs a coordinated change (e.g. always write the attribute, or default the item read to legacy gamma and migrate), not just changing this one line.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic


while let Some(element) = next {
let from_style = element.attribute("style").and_then(|style| {
style.split(';').find_map(|declaration| {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: SVG gradients with repeated color-interpolation declarations in an inline style can import with the wrong interpolation space because the resolver keeps the first declaration; selecting the last applicable declaration would match the CSS cascade.

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 575:

<comment>SVG gradients with repeated `color-interpolation` declarations in an inline style can import with the wrong interpolation space because the resolver keeps the first declaration; selecting the last applicable declaration would match the CSS cascade.</comment>

<file context>
@@ -523,6 +525,72 @@ fn usvg_transform(c: usvg::Transform) -> DAffine2 {
+
+	while let Some(element) = next {
+		let from_style = element.attribute("style").and_then(|style| {
+			style.split(';').find_map(|declaration| {
+				let (property, value) = declaration.split_once(':')?;
+				(property.trim() == "color-interpolation").then(|| value.trim())
</file context>

let gradient_spread = convert_gradient_spread(linear.spread_method());
modify_inputs.fill_gradient_set(gradient, gradient_form, gradient_spread, transform);
// SVG interpolates between stops in gamma sRGB unless `color-interpolation` opts into linearRGB, carried explicitly rather than as the linear default
let gradient_interpolation = gradient_info.interpolations.get(linear.id()).copied().unwrap_or(GradientInterpolation::SrgbGamma);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: SVG import now resolves each gradient's interpolation from its inline style attribute and its color-interpolation presentation attribute, but it does not read CSS <style> blocks. Many exported SVGs (Illustrator/Inkscape) set color-interpolation: linearRGB through grouped stylesheet rules, and those gradients would fall through to the SrgbGamma default on import, silently discarding the author's declared linear interpolation. Consider also scanning <style> elements (and matching the selectors, or at least any color-interpolation declared there) so the import honors them rather than silently defaulting to gamma.

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 914:

<comment>SVG import now resolves each gradient's interpolation from its inline `style` attribute and its `color-interpolation` presentation attribute, but it does not read CSS `<style>` blocks. Many exported SVGs (Illustrator/Inkscape) set `color-interpolation: linearRGB` through grouped stylesheet rules, and those gradients would fall through to the `SrgbGamma` default on import, silently discarding the author's declared linear interpolation. Consider also scanning `<style>` elements (and matching the selectors, or at least any `color-interpolation` declared there) so the import honors them rather than silently defaulting to gamma.</comment>

<file context>
@@ -842,7 +910,9 @@ fn apply_usvg_fill(fill: &usvg::Fill, modify_inputs: &mut ModifyInputsContext, g
 			let gradient_spread = convert_gradient_spread(linear.spread_method());
-			modify_inputs.fill_gradient_set(gradient, gradient_form, gradient_spread, transform);
+			// SVG interpolates between stops in gamma sRGB unless `color-interpolation` opts into linearRGB, carried explicitly rather than as the linear default
+			let gradient_interpolation = gradient_info.interpolations.get(linear.id()).copied().unwrap_or(GradientInterpolation::SrgbGamma);
+			modify_inputs.fill_gradient_set(gradient, gradient_form, gradient_spread, gradient_interpolation, transform);
 		}
</file context>

Self {
stops: item.element().into(),
gradient_spread: item.attribute_cloned_or_default(ATTR_GRADIENT_SPREAD),
gradient_interpolation: item.attribute_cloned_or_default(ATTR_GRADIENT_INTERPOLATION),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The item/attribute read path resolves a missing gradient_interpolation to Default::default() = SrgbLinear, which contradicts the design intent (and the doc comment on this struct) that the field's absence marks a pre-existing ramp and should read as the legacy SrgbGamma. The serde exchange form honors this via serde(default = "GradientInterpolation::legacy_gamma"), but the path the actual renderers and node graph use does not: From<GradientRamp> for Item writes the attribute only when non-default (if !is_default()), so both a newly created Linear ramp and every legacy gamma ramp leave the attribute absent, and both resolve to Linear. As a result, gradients in documents saved before this field existed will flip from gamma to linear rendering the moment one of the renderer paths (renderer.rs lines ~511/2181/2279 or render_ext.rs ~99) reads them - an unintended visual change to existing artwork. Because the attribute form can't distinguish "new Linear" from "legacy gamma", the fix needs a coordinated change (e.g. always write the attribute, or default the item read to legacy gamma and migrate), not just changing this one line.

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/gradient.rs, line 179:

<comment>The item/attribute read path resolves a missing `gradient_interpolation` to `Default::default()` = `SrgbLinear`, which contradicts the design intent (and the doc comment on this struct) that the field's absence marks a pre-existing ramp and should read as the legacy `SrgbGamma`. The serde exchange form honors this via `serde(default = "GradientInterpolation::legacy_gamma")`, but the path the actual renderers and node graph use does not: `From<GradientRamp> for Item` writes the attribute only when non-default (`if !is_default()`), so both a newly created Linear ramp and every legacy gamma ramp leave the attribute absent, and both resolve to Linear. As a result, gradients in documents saved before this field existed will flip from gamma to linear rendering the moment one of the renderer paths (renderer.rs lines ~511/2181/2279 or render_ext.rs ~99) reads them - an unintended visual change to existing artwork. Because the attribute form can't distinguish "new Linear" from "legacy gamma", the fix needs a coordinated change (e.g. always write the attribute, or default the item read to legacy gamma and migrate), not just changing this one line.</comment>

<file context>
@@ -166,6 +176,7 @@ impl From<&Item<Gradient>> for GradientRamp {
 		Self {
 			stops: item.element().into(),
 			gradient_spread: item.attribute_cloned_or_default(ATTR_GRADIENT_SPREAD),
+			gradient_interpolation: item.attribute_cloned_or_default(ATTR_GRADIENT_INTERPOLATION),
 		}
 	}
</file context>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant