Skip to content

userSpaceOnUse/objectBoundingBox #4283

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
jsjgdh wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

userSpaceOnUse/objectBoundingBox #4283

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 77 additions & 8 deletions editor/src/messages/portfolio/document/graph_operation/graph_operation_message_handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use graph_craft::document::{NodeId, NodeInput};
use graphene_std::list::List;
use graphene_std::renderer::convert_usvg_path::convert_usvg_path;
use graphene_std::text::{Font, TypesettingConfig};
use graphene_std::vector::style::{Fill, Gradient, GradientSpreadMethod, GradientStop, GradientStops, GradientType, PaintOrder, Stroke, StrokeAlign, StrokeCap, StrokeJoin};
use graphene_std::vector::style::{Fill, Gradient, GradientSpreadMethod, GradientStop, GradientStops, GradientType, GradientUnits, PaintOrder, Stroke, StrokeAlign, StrokeCap, StrokeJoin};
use graphene_std::{Artboard, Color};

#[derive(ExtractField)]
Expand Down Expand Up @@ -452,6 +452,7 @@ impl MessageHandler<GraphOperationMessage, GraphOperationMessageContext<'_>> for
placement_transform.translation = placement_transform.translation.round();

let graphite_gradient_stops = extract_graphite_gradient_stops(&svg);
let svg_gradient_units = extract_svg_gradient_units(&svg);

@cubic-dev-ai cubic-dev-ai Bot Jun 25, 2026

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: Both extract_graphite_gradient_stops and extract_svg_gradient_units independently parse the same raw SVG XML string with usvg::roxmltree::Document::parse. For large SVG files this doubles the parsing cost. Consider refactoring to share a single parsed roxmltree::Document between the two extraction passes, or combining them into one function.

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

<comment>Both `extract_graphite_gradient_stops` and `extract_svg_gradient_units` independently parse the same raw SVG XML string with `usvg::roxmltree::Document::parse`. For large SVG files this doubles the parsing cost. Consider refactoring to share a single parsed `roxmltree::Document` between the two extraction passes, or combining them into one function.</comment>

<file context>
@@ -452,6 +452,7 @@ impl MessageHandler<GraphOperationMessage, GraphOperationMessageContext<'_>> for
 				placement_transform.translation = placement_transform.translation.round();
 
 				let graphite_gradient_stops = extract_graphite_gradient_stops(&svg);
+				let svg_gradient_units = extract_svg_gradient_units(&svg);
 
 				// Pass identity so each leaf layer receives only its SVG-native transform from `abs_transform`.
</file context>


// Pass identity so each leaf layer receives only its SVG-native transform from `abs_transform`.
// The placement offset is then applied once to the root group layer below.
Expand All @@ -462,6 +463,7 @@ impl MessageHandler<GraphOperationMessage, GraphOperationMessageContext<'_>> for
parent,
insert_index,
&graphite_gradient_stops,
&svg_gradient_units,
);

// After import, `layer_node` is set to the root group. Apply the placement transform to it
Expand Down Expand Up @@ -551,6 +553,50 @@ fn extract_graphite_gradient_stops(svg: &str) -> HashMap<String, GradientStops>
result
}

fn extract_svg_gradient_units(svg: &str) -> HashMap<String, GradientUnits> {
let mut result = HashMap::new();
let mut gradient_nodes = HashMap::new();

let doc = match usvg::roxmltree::Document::parse(svg) {
Ok(doc) => doc,
Err(_) => return result,
};
Comment on lines +556 to +563

@gemini-code-assist gemini-code-assist Bot Jun 25, 2026

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.

medium

Redundant XML Parsing

Both extract_graphite_gradient_stops and extract_svg_gradient_units parse the same raw SVG XML string independently using usvg::roxmltree::Document::parse. For large SVG files, parsing the XML tree multiple times introduces unnecessary CPU and memory overhead.

Consider refactoring these functions to accept a shared reference to the parsed roxmltree::Document, or combining them into a single pass over the XML descendants to extract both gradient stops and units simultaneously.


for node in doc.descendants() {
if matches!(node.tag_name().name(), "linearGradient" | "radialGradient")
&& let Some(gradient_id) = node.attribute("id")
{
gradient_nodes.insert(gradient_id, node);
}
}

fn resolve_units<'a>(node: usvg::roxmltree::Node<'a, 'a>, gradient_nodes: &HashMap<&'a str, usvg::roxmltree::Node<'a, 'a>>, seen: &mut Vec<&'a str>) -> Option<GradientUnits> {
match node.attribute("gradientUnits") {
Some("userSpaceOnUse") => return Some(GradientUnits::UserSpaceOnUse),
Some("objectBoundingBox") => return Some(GradientUnits::ObjectBoundingBox),
Some(_) => return None,
None => {}
}

let href = node.attribute("href").or_else(|| node.attribute(("http://www.w3.org/1999/xlink", "href")))?;
let referenced_id = href.strip_prefix('#')?;
if seen.contains(&referenced_id) {
return None;
}
let referenced_node = *gradient_nodes.get(referenced_id)?;
seen.push(referenced_id);
resolve_units(referenced_node, gradient_nodes, seen)
}

for (&gradient_id, &node) in &gradient_nodes {
// Per SVG spec, the default gradientUnits is objectBoundingBox.
let units = resolve_units(node, &gradient_nodes, &mut vec![gradient_id]).unwrap_or(GradientUnits::ObjectBoundingBox);
result.insert(gradient_id.to_string(), units);
}

result
}

fn parse_hex_stop_color(hex: &str, opacity: f32) -> Option<Color> {
let hex = hex.strip_prefix('#')?;
if hex.len() != 6 {
Expand All @@ -575,6 +621,7 @@ fn import_usvg_node(
parent: LayerNodeIdentifier,
insert_index: usize,
graphite_gradient_stops: &HashMap<String, GradientStops>,
svg_gradient_units: &HashMap<String, GradientUnits>,
) {
let layer = modify_inputs.create_layer(id);

Expand All @@ -595,7 +642,7 @@ fn import_usvg_node(
modify_inputs.import = true;

for child in group.children() {
let extent = import_usvg_node_inner(modify_inputs, child, NodeId::new(), layer, 0, graphite_gradient_stops, &mut group_extents_map);
let extent = import_usvg_node_inner(modify_inputs, child, NodeId::new(), layer, 0, graphite_gradient_stops, svg_gradient_units, &mut group_extents_map);
child_extents_svg_order.push(extent);
}

Expand All @@ -614,7 +661,7 @@ fn import_usvg_node(
modify_inputs.network_interface.unload_all_nodes_bounding_box(&[]);
}
usvg::Node::Path(path) => {
import_usvg_path(modify_inputs, node, path, layer, graphite_gradient_stops);
import_usvg_path(modify_inputs, node, path, layer, graphite_gradient_stops, svg_gradient_units);
}
usvg::Node::Image(_image) => {
warn!("Skip image");
Expand All @@ -639,6 +686,7 @@ fn import_usvg_node_inner(
parent: LayerNodeIdentifier,
insert_index: usize,
graphite_gradient_stops: &HashMap<String, GradientStops>,
svg_gradient_units: &HashMap<String, GradientUnits>,
group_extents_map: &mut HashMap<LayerNodeIdentifier, Vec<u32>>,
) -> u32 {
let layer = modify_inputs.create_layer(id);
Expand All @@ -649,7 +697,7 @@ fn import_usvg_node_inner(
usvg::Node::Group(group) => {
let mut child_extents: Vec<u32> = Vec::new();
for child in group.children() {
let extent = import_usvg_node_inner(modify_inputs, child, NodeId::new(), layer, 0, graphite_gradient_stops, group_extents_map);
let extent = import_usvg_node_inner(modify_inputs, child, NodeId::new(), layer, 0, graphite_gradient_stops, svg_gradient_units, group_extents_map);
child_extents.push(extent);
}
modify_inputs.layer_node = Some(layer);
Expand All @@ -664,7 +712,7 @@ fn import_usvg_node_inner(
total_extent
}
usvg::Node::Path(path) => {
import_usvg_path(modify_inputs, node, path, layer, graphite_gradient_stops);
import_usvg_path(modify_inputs, node, path, layer, graphite_gradient_stops, svg_gradient_units);
0
}
usvg::Node::Image(_image) => {
Expand All @@ -681,7 +729,14 @@ fn import_usvg_node_inner(
}

/// Helper to apply path data (vector geometry, fill, stroke, transform) to a layer.
fn import_usvg_path(modify_inputs: &mut ModifyInputsContext, node: &usvg::Node, path: &usvg::Path, layer: LayerNodeIdentifier, graphite_gradient_stops: &HashMap<String, GradientStops>) {
fn import_usvg_path(
modify_inputs: &mut ModifyInputsContext,
node: &usvg::Node,
path: &usvg::Path,
layer: LayerNodeIdentifier,
graphite_gradient_stops: &HashMap<String, GradientStops>,
svg_gradient_units: &HashMap<String, GradientUnits>,
) {
let subpaths = convert_usvg_path(path);

// Skip creating a Transform node entirely when the SVG-native transform is identity.
Expand All @@ -695,7 +750,7 @@ fn import_usvg_path(modify_inputs: &mut ModifyInputsContext, node: &usvg::Node,
}

if let Some(fill) = path.fill() {
apply_usvg_fill(fill, modify_inputs, graphite_gradient_stops);
apply_usvg_fill(fill, modify_inputs, graphite_gradient_stops, svg_gradient_units);
}
if let Some(stroke) = path.stroke() {
apply_usvg_stroke(stroke, modify_inputs, node_transform);
Expand Down Expand Up @@ -794,7 +849,15 @@ fn convert_spread_method(spread_method: usvg::SpreadMethod) -> GradientSpreadMet
}
}

fn apply_usvg_fill(fill: &usvg::Fill, modify_inputs: &mut ModifyInputsContext, graphite_gradient_stops: &HashMap<String, GradientStops>) {
/// Apply the SVG fill of a path to the layer being imported.
///
/// `units` is read from the original SVG XML (not from usvg) because usvg auto-converts
/// `objectBoundingBox` gradients to `userSpaceOnUse`, baking the bbox transform into
/// `linear.transform()`. The absolute `start`/`end` coordinates stored on the `Gradient`
/// therefore always live in user-space; `units` is kept solely as an **export hint** so
/// the SVG renderer can re-emit the correct `gradientUnits` attribute and apply
/// `bounds.inverse()` to recover the original bbox-relative coordinates on export.
fn apply_usvg_fill(fill: &usvg::Fill, modify_inputs: &mut ModifyInputsContext, graphite_gradient_stops: &HashMap<String, GradientStops>, svg_gradient_units: &HashMap<String, GradientUnits>) {
modify_inputs.fill_set(match &fill.paint() {
usvg::Paint::Color(color) => Fill::solid(usvg_color(*color, fill.opacity().get())),
usvg::Paint::LinearGradient(linear) => {
Expand All @@ -803,6 +866,8 @@ fn apply_usvg_fill(fill: &usvg::Fill, modify_inputs: &mut ModifyInputsContext, g
let (start, end) = (gradient_transform.transform_point2(start), gradient_transform.transform_point2(end));

let gradient_type = GradientType::Linear;
// Look up the original gradientUnits from the XML pass (usvg hides it after conversion).
let units = svg_gradient_units.get(linear.id()).copied().unwrap_or_default();

@cubic-dev-ai cubic-dev-ai Bot Jun 25, 2026

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: Using unwrap_or_default() for GradientUnits falls back to UserSpaceOnUse because that variant is marked #[default], but the SVG spec default is objectBoundingBox and the same file's extract_svg_gradient_units already uses ObjectBoundingBox as its explicit fallback. This creates an inconsistent, hidden cross-file dependency: missing/unmapped gradient IDs silently get the wrong units, which will corrupt re-exported SVGs.

A similar unwrap_or_default() bug exists on the radial gradient path a few lines below.

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

<comment>Using `unwrap_or_default()` for `GradientUnits` falls back to `UserSpaceOnUse` because that variant is marked `#[default]`, but the SVG spec default is `objectBoundingBox` and the same file's `extract_svg_gradient_units` already uses `ObjectBoundingBox` as its explicit fallback. This creates an inconsistent, hidden cross-file dependency: missing/unmapped gradient IDs silently get the wrong units, which will corrupt re-exported SVGs.

A similar `unwrap_or_default()` bug exists on the radial gradient path a few lines below.</comment>

<file context>
@@ -803,6 +866,8 @@ fn apply_usvg_fill(fill: &usvg::Fill, modify_inputs: &mut ModifyInputsContext, g
 
 			let gradient_type = GradientType::Linear;
+			// Look up the original gradientUnits from the XML pass (usvg hides it after conversion).
+			let units = svg_gradient_units.get(linear.id()).copied().unwrap_or_default();
 
 			let stops = match graphite_gradient_stops.get(linear.id()) {
</file context>
Suggested change
let units = svg_gradient_units.get(linear.id()).copied().unwrap_or_default();
let units = svg_gradient_units.get(linear.id()).copied().unwrap_or(GradientUnits::ObjectBoundingBox);


let stops = match graphite_gradient_stops.get(linear.id()) {
Some(graphite_stops) => graphite_stops.clone(),
Expand All @@ -821,6 +886,7 @@ fn apply_usvg_fill(fill: &usvg::Fill, modify_inputs: &mut ModifyInputsContext, g
start,
end,
gradient_type,
units,
stops,
spread_method,
// TODO: Eventually remove this document upgrade code
Expand All @@ -835,6 +901,8 @@ fn apply_usvg_fill(fill: &usvg::Fill, modify_inputs: &mut ModifyInputsContext, g
let (start, end) = (gradient_transform.transform_point2(center), gradient_transform.transform_point2(edge));

let gradient_type = GradientType::Radial;
// Look up the original gradientUnits from the XML pass (usvg hides it after conversion).
let units = svg_gradient_units.get(radial.id()).copied().unwrap_or_default();

let stops = match graphite_gradient_stops.get(radial.id()) {
Some(graphite_stops) => graphite_stops.clone(),
Expand All @@ -853,6 +921,7 @@ fn apply_usvg_fill(fill: &usvg::Fill, modify_inputs: &mut ModifyInputsContext, g
start,
end,
gradient_type,
units,
stops,
spread_method,
// TODO: Eventually remove this document upgrade code
Expand Down
1 change: 1 addition & 0 deletions editor/src/messages/tool/tool_messages/gradient_tool.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,7 @@ fn get_gradient(layer: LayerNodeIdentifier, network_interface: &NodeNetworkInter
stops,
gradient_type: chain_state.gradient_type,
spread_method: chain_state.spread_method,
units: Default::default(),
start: chain_state.transform.transform_point2(DVec2::ZERO),
end: chain_state.transform.transform_point2(DVec2::X),
// TODO: Eventually remove this document upgrade code
Expand Down
4 changes: 2 additions & 2 deletions node-graph/libraries/core-types/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ pub use graphene_hash;
pub use graphene_hash::CacheHash;
pub use list::{
ATTR_BACKGROUND, ATTR_BLEND_MODE, ATTR_CLIP, ATTR_CLIPPING_MASK, ATTR_DIMENSIONS, ATTR_EDITOR_CLICK_TARGET, ATTR_EDITOR_LAYER_PATH, ATTR_EDITOR_MERGED_LAYERS, ATTR_EDITOR_TEXT_FRAME, ATTR_END,
ATTR_FONT, ATTR_FONT_SIZE, ATTR_GRADIENT_LEGACY, ATTR_GRADIENT_TYPE, ATTR_LETTER_SPACING, ATTR_LETTER_TILT, ATTR_LINE_HEIGHT, ATTR_LOCATION, ATTR_MAX_HEIGHT, ATTR_MAX_WIDTH, ATTR_NAME,
ATTR_OPACITY, ATTR_OPACITY_FILL, ATTR_SPREAD_METHOD, ATTR_START, ATTR_TEXT_ALIGN, ATTR_TRANSFORM, ATTR_TYPE,
ATTR_FONT, ATTR_FONT_SIZE, ATTR_GRADIENT_LEGACY, ATTR_GRADIENT_TYPE, ATTR_GRADIENT_UNITS, ATTR_LETTER_SPACING, ATTR_LETTER_TILT, ATTR_LINE_HEIGHT, ATTR_LOCATION, ATTR_MAX_HEIGHT, ATTR_MAX_WIDTH,
ATTR_NAME, ATTR_OPACITY, ATTR_OPACITY_FILL, ATTR_SPREAD_METHOD, ATTR_START, ATTR_TEXT_ALIGN, ATTR_TRANSFORM, ATTR_TYPE,
};
pub use memo::MemoHash;
pub use no_std_types::AsU32;
Expand Down
2 changes: 2 additions & 0 deletions node-graph/libraries/core-types/src/list.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ pub const ATTR_CLIP: &str = "clip";
pub const ATTR_SPREAD_METHOD: &str = "spread_method";
/// Gradient's `GradientType` (`Linear` or `Radial`).
pub const ATTR_GRADIENT_TYPE: &str = "gradient_type";
/// Gradient's SVG coordinate system (`userSpaceOnUse` or `objectBoundingBox`).
pub const ATTR_GRADIENT_UNITS: &str = "gradient_units";
// TODO: Eventually remove this document upgrade code
/// `bool` runtime marker (never serialized) flagging a gradient that came from the legacy bounding-box-relative `Fill::Gradient`,
/// so the renderer reproduces the pre-#4241 positioning instead of the new absolute path.
Expand Down
5 changes: 4 additions & 1 deletion node-graph/libraries/graphic-types/src/graphic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ use core_types::list::{ATTR_FILL, ATTR_STROKE, Item, List};
use core_types::ops::{FromAnchorPosition, ListConvert};
use core_types::render_complexity::RenderComplexity;
use core_types::uuid::NodeId;
use core_types::{ATTR_CLIPPING_MASK, ATTR_EDITOR_LAYER_PATH, ATTR_GRADIENT_LEGACY, ATTR_GRADIENT_TYPE, ATTR_OPACITY, ATTR_OPACITY_FILL, ATTR_SPREAD_METHOD, ATTR_TRANSFORM, Color};
use core_types::{
ATTR_CLIPPING_MASK, ATTR_EDITOR_LAYER_PATH, ATTR_GRADIENT_LEGACY, ATTR_GRADIENT_TYPE, ATTR_GRADIENT_UNITS, ATTR_OPACITY, ATTR_OPACITY_FILL, ATTR_SPREAD_METHOD, ATTR_TRANSFORM, Color,
};
use dyn_any::DynAny;
use glam::{DAffine2, DVec2};
use raster_types::{CPU, GPU, Raster};
Expand Down Expand Up @@ -211,6 +213,7 @@ pub fn fill_to_graphic_list(fill: &Fill, bounding_box_transform: DAffine2) -> Op
let gradient_item = Item::new_from_element(gradient.stops.clone())
.with_attribute(ATTR_TRANSFORM, transform)
.with_attribute(ATTR_GRADIENT_TYPE, gradient.gradient_type)
.with_attribute(ATTR_GRADIENT_UNITS, gradient.units)

@cubic-dev-ai cubic-dev-ai Bot Jun 25, 2026

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: Writing ATTR_GRADIENT_UNITS unconditionally alongside the legacy bounding-box transform path creates a cross-file rendering contract mismatch. The Vello renderer (create_peniko_gradient_brush) correctly guards legacy gradients by checking ATTR_GRADIENT_LEGACY, but the SVG renderers (render_ext.rs and renderer.rs) read ATTR_GRADIENT_UNITS directly without checking the legacy flag. For ObjectBoundingBox units, SVG applies bounds.inverse() on top of an already-baked bbox transform, double-applying the coordinate conversion and producing incorrect gradient placement in SVG exports.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/libraries/graphic-types/src/graphic.rs, line 216:

<comment>Writing `ATTR_GRADIENT_UNITS` unconditionally alongside the legacy bounding-box transform path creates a cross-file rendering contract mismatch. The Vello renderer (`create_peniko_gradient_brush`) correctly guards legacy gradients by checking `ATTR_GRADIENT_LEGACY`, but the SVG renderers (`render_ext.rs` and `renderer.rs`) read `ATTR_GRADIENT_UNITS` directly without checking the legacy flag. For `ObjectBoundingBox` units, SVG applies `bounds.inverse()` on top of an already-baked bbox transform, double-applying the coordinate conversion and producing incorrect gradient placement in SVG exports.</comment>

<file context>
@@ -211,6 +213,7 @@ pub fn fill_to_graphic_list(fill: &Fill, bounding_box_transform: DAffine2) -> Op
 			let gradient_item = Item::new_from_element(gradient.stops.clone())
 				.with_attribute(ATTR_TRANSFORM, transform)
 				.with_attribute(ATTR_GRADIENT_TYPE, gradient.gradient_type)
+				.with_attribute(ATTR_GRADIENT_UNITS, gradient.units)
 				.with_attribute(ATTR_SPREAD_METHOD, gradient.spread_method)
 				.with_attribute(ATTR_GRADIENT_LEGACY, legacy);
</file context>

.with_attribute(ATTR_SPREAD_METHOD, gradient.spread_method)
.with_attribute(ATTR_GRADIENT_LEGACY, legacy);
let gradient_list = List::new_from_item(gradient_item);
Expand Down
29 changes: 22 additions & 7 deletions node-graph/libraries/rendering/src/render_ext.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ use crate::{Render, RenderSvgSegmentList, SvgRender};
use core_types::color::SRGBA8;
use core_types::list::List;
use core_types::uuid::generate_uuid;
use core_types::{ATTR_GRADIENT_TYPE, ATTR_SPREAD_METHOD, ATTR_TRANSFORM, Color};
use core_types::{ATTR_GRADIENT_TYPE, ATTR_GRADIENT_UNITS, ATTR_SPREAD_METHOD, ATTR_TRANSFORM, Color};
use glam::{DAffine2, DVec2};
use graphic_types::Graphic;
use graphic_types::vector_types::gradient::GradientType;
use graphic_types::vector_types::gradient::{GradientType, GradientUnits};
use graphic_types::vector_types::vector::style::{PaintOrder, Stroke, StrokeAlign, StrokeCap, StrokeJoin};
use std::fmt::Write;
use vector_types::GradientStops;
Expand All @@ -18,6 +18,16 @@ pub enum PaintTarget {
Stroke,
}

fn svg_gradient_transform(transform: DAffine2, bounds: DAffine2, units: GradientUnits) -> (GradientUnits, String) {
let (units, transform) = match units {
GradientUnits::UserSpaceOnUse => (GradientUnits::UserSpaceOnUse, transform),
GradientUnits::ObjectBoundingBox if transform_is_invertible(bounds) => (GradientUnits::ObjectBoundingBox, bounds.inverse() * transform),
GradientUnits::ObjectBoundingBox => (GradientUnits::UserSpaceOnUse, transform),
};

(units, format_transform_matrix(transform))
}

impl PaintTarget {
fn paint_attr(self) -> &'static str {
match self {
Expand Down Expand Up @@ -94,6 +104,7 @@ impl RenderExt for List<GradientStops> {

let Some(stops) = self.element(0) else { return 0 };
let gradient_type: GradientType = self.attribute_cloned_or_default(ATTR_GRADIENT_TYPE, 0);
let gradient_units: GradientUnits = self.attribute_cloned_or_default(ATTR_GRADIENT_UNITS, 0);
let local_gradient_transform: DAffine2 = self.attribute_cloned_or_default(ATTR_TRANSFORM, 0);
let spread_method: GradientSpreadMethod = self.attribute_cloned_or_default(ATTR_SPREAD_METHOD, 0);

Expand Down Expand Up @@ -122,7 +133,7 @@ impl RenderExt for List<GradientStops> {
let document_transform = item_transform * local_gradient_transform;

let placement = gradient_placement(document_transform, gradient_type);
let gradient_transform = format_transform_matrix(element_transform_inverse * placement);
let (gradient_units, gradient_transform) = svg_gradient_transform(element_transform_inverse * placement, _bounds, gradient_units);

@gemini-code-assist gemini-code-assist Bot Jun 25, 2026

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.

medium

Used Variable with Underscore Prefix

The _bounds variable is now used in the call to svg_gradient_transform. By Rust convention, variables prefixed with an underscore should remain unused. Since it is now used, consider renaming _bounds to bounds in the function signature of RenderExt::render for List<GradientStops> to improve code clarity and adhere to standard style guidelines.

@cubic-dev-ai cubic-dev-ai Bot Jun 25, 2026

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.

P3: The _bounds parameter is now used in the call to svg_gradient_transform, but the underscore prefix indicates an intentionally-unused binding. Rename it to bounds in the function signature to follow Rust naming conventions.

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/render_ext.rs, line 136:

<comment>The `_bounds` parameter is now used in the call to `svg_gradient_transform`, but the underscore prefix indicates an intentionally-unused binding. Rename it to `bounds` in the function signature to follow Rust naming conventions.</comment>

<file context>
@@ -122,7 +133,7 @@ impl RenderExt for List<GradientStops> {
 
 		let placement = gradient_placement(document_transform, gradient_type);
-		let gradient_transform = format_transform_matrix(element_transform_inverse * placement);
+		let (gradient_units, gradient_transform) = svg_gradient_transform(element_transform_inverse * placement, _bounds, gradient_units);
 		let gradient_transform = if gradient_transform.is_empty() {
 			String::new()
</file context>

let gradient_transform = if gradient_transform.is_empty() {
String::new()
} else {
Expand All @@ -141,15 +152,19 @@ impl RenderExt for List<GradientStops> {
GradientType::Linear => {
let _ = write!(
svg_defs,
r#"<linearGradient id="{}" gradientUnits="userSpaceOnUse" x1="0" y1="0" x2="1" y2="0"{spread_method}{gradient_transform}>{}</linearGradient>"#,
gradient_id, stop
r#"<linearGradient id="{}" gradientUnits="{}" x1="0" y1="0" x2="1" y2="0"{spread_method}{gradient_transform}>{}</linearGradient>"#,
gradient_id,
gradient_units.svg_name(),
stop
);
}
GradientType::Radial => {
let _ = write!(
svg_defs,
r#"<radialGradient id="{}" gradientUnits="userSpaceOnUse" cx="0" cy="0" r="1"{spread_method}{gradient_transform}>{}</radialGradient>"#,
gradient_id, stop
r#"<radialGradient id="{}" gradientUnits="{}" cx="0" cy="0" r="1"{spread_method}{gradient_transform}>{}</radialGradient>"#,
gradient_id,
gradient_units.svg_name(),
stop
);
}
}
Expand Down
Loading