From 26de6912f78d8aab82e37153314a0a9716f864af Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Wed, 9 Sep 2026 21:23:24 +0000 Subject: [PATCH] Fix the fill transform toggle's name with a HasTransform newtype The fill's gradient placement input splits from one optional transform into the has-transform toggle and a plain transform, so the toggle's input name is fixed in the document format ahead of the transform toggle's own arrival. An unset placement keeps the toggle's false default and derives from the content's bounds at evaluation, which is also the unbaked placeholder the deferred gradient bake probes for. Documents saved with the optional form migrate by decomposing it, a wired transform source keeping its connection marked explicit, and the legacy fill migration now lands on the split shape directly. Co-Authored-By: Claude Fable 5 --- .../document/graph_operation/utility_types.rs | 9 ++- .../document/node_graph/node_properties.rs | 20 +++++- .../messages/portfolio/document_migration.rs | 72 ++++++++++++++----- .../graph_modification_utils.rs | 15 ++-- .../tool/tool_messages/gradient_tool.rs | 8 ++- editor/src/node_graph_executor.rs | 17 +++-- node-graph/graph-craft/src/document/value.rs | 1 + .../vector-types/src/vector/style.rs | 9 +++ node-graph/nodes/vector/src/vector_nodes.rs | 12 ++-- 9 files changed, 124 insertions(+), 39 deletions(-) diff --git a/editor/src/messages/portfolio/document/graph_operation/utility_types.rs b/editor/src/messages/portfolio/document/graph_operation/utility_types.rs index 638e2a133d..0230a30e9b 100644 --- a/editor/src/messages/portfolio/document/graph_operation/utility_types.rs +++ b/editor/src/messages/portfolio/document/graph_operation/utility_types.rs @@ -15,7 +15,7 @@ use graphene_std::raster::BlendMode; use graphene_std::raster_types::Image; use graphene_std::subpath::Subpath; use graphene_std::text::{Font, TypesettingConfig}; -use graphene_std::vector::style::{GradientSpreadMethod, GradientType, Stroke}; +use graphene_std::vector::style::{GradientSpreadMethod, GradientType, HasTransform, Stroke}; use graphene_std::vector::{GradientStops, PointId, Vector, VectorModification, VectorModificationType}; use graphene_std::{Artboard, Color, Graphic, NodeInputDecleration}; @@ -490,7 +490,12 @@ impl<'a> ModifyInputsContext<'a> { if transform_is_value { self.set_input_with_refresh( InputConnector::node(fill_node_id, graphene_std::vector::fill::TransformInput::INDEX), - NodeInput::value(TaggedValue::OptionalDAffine2(Some(transform)), false), + NodeInput::value(TaggedValue::DAffine2(transform), false), + true, + ); + self.set_input_with_refresh( + InputConnector::node(fill_node_id, graphene_std::vector::fill::HasTransformInput::INDEX), + NodeInput::value(TaggedValue::HasTransform(HasTransform(true)), false), true, ); } diff --git a/editor/src/messages/portfolio/document/node_graph/node_properties.rs b/editor/src/messages/portfolio/document/node_graph/node_properties.rs index a8e2c7a491..3eef8bf647 100644 --- a/editor/src/messages/portfolio/document/node_graph/node_properties.rs +++ b/editor/src/messages/portfolio/document/node_graph/node_properties.rs @@ -33,7 +33,7 @@ use graphene_std::transform::{Footprint, ReferencePoint, ScaleType, Transform}; use graphene_std::vector::misc::BooleanOperation; use graphene_std::vector::misc::{ArcType, CentroidType, ExtrudeJoiningAlgorithm, GridType, InterpolationDistribution, MergeByDistanceAlgorithm, PointSpacingType, RowsOrColumns, SpiralType}; use graphene_std::vector::style::{ - FillChoice, FillChoiceUI, GradientSpreadMethod, GradientStops, GradientStopsUI, GradientType, PaintOrder, StrokeAlign, StrokeCap, StrokeJoin, build_transform_with_y_preservation, + FillChoice, FillChoiceUI, GradientSpreadMethod, GradientStops, GradientStopsUI, GradientType, HasTransform, PaintOrder, StrokeAlign, StrokeCap, StrokeJoin, build_transform_with_y_preservation, }; use graphene_std::vector::{QRCodeErrorCorrectionLevel, VectorModification}; @@ -2677,7 +2677,23 @@ pub(crate) fn fill_properties(node_id: NodeId, context: &mut NodePropertiesConte } else { "Swap the start and end points of the gradient line." }) - .on_update(update_value(move |_| TaggedValue::OptionalDAffine2(Some(new_transform)), node_id, TransformInput::INDEX)) + .on_update(move |_: &IconButton| Message::Batched { + // The reversed placement is explicit even when the original was derived, so the toggle is set alongside it + messages: Box::new([ + NodeGraphMessage::SetInputValue { + node_id, + input_index: TransformInput::INDEX, + value: Box::new(TaggedValue::DAffine2(new_transform)), + } + .into(), + NodeGraphMessage::SetInputValue { + node_id, + input_index: HasTransformInput::INDEX, + value: Box::new(TaggedValue::HasTransform(HasTransform(true))), + } + .into(), + ]), + }) .widget_instance(); spread_methods_row.push(Separator::new(SeparatorStyle::Unrelated).widget_instance()); spread_methods_row.push(reverse_direction_button); diff --git a/editor/src/messages/portfolio/document_migration.rs b/editor/src/messages/portfolio/document_migration.rs index df2932ce85..10c8c67f38 100644 --- a/editor/src/messages/portfolio/document_migration.rs +++ b/editor/src/messages/portfolio/document_migration.rs @@ -15,7 +15,7 @@ use graphene_std::text::{TextAlign, TypesettingConfig}; use graphene_std::transform::ScaleType; use graphene_std::uuid::NodeId; use graphene_std::vector::graphic_types; -use graphene_std::vector::style::{PaintOrder, StrokeAlign}; +use graphene_std::vector::style::{HasTransform, PaintOrder, StrokeAlign}; use std::collections::HashMap; use std::f64::consts::PI; use std::ops::Range; @@ -1613,7 +1613,7 @@ fn migrate_node(node_id: &NodeId, node: &DocumentNode, network_path: &[NodeId], } // Upgrade the legacy 4-input Fill node (content, fill: Fill, _backup_color, _backup_gradient: Gradient) to the - // value-model 7-input shape (content, fill: generic paint list, _backup_color, _backup_gradient, _gradient_type, _spread_method, _transform). + // value-model 8-input shape (content, fill: generic paint list, _backup_color, _backup_gradient, _gradient_type, _spread_method, _has_transform, _transform). if reference == DefinitionIdentifier::ProtoNode(graphene_std::vector_nodes::fill::IDENTIFIER) && inputs_count == 4 { let mut node_template = resolve_document_node_type(&reference)?.default_node_template(); document.network_interface.replace_implementation(node_id, network_path, &mut node_template); @@ -1648,16 +1648,19 @@ fn migrate_node(node_id: &NodeId, node: &DocumentNode, network_path: &[NodeId], network_path, ); - let transform = if gradient.absolute { - Some(gradient.transform * gradient.to_transform()) + // An unbaked placement keeps the template's `HasTransform(false)` default, the probe the deferred bake looks for + if gradient.absolute { + let transform = gradient.transform * gradient.to_transform(); + document + .network_interface + .set_input(&InputConnector::node(*node_id, 6), NodeInput::value(TaggedValue::HasTransform(HasTransform(true)), false), network_path); + document + .network_interface + .set_input(&InputConnector::node(*node_id, 7), NodeInput::value(TaggedValue::DAffine2(transform), false), network_path); } else { // Baking a legacy bounding-box-relative gradient is deferred until the measurement pre-pass can supply the paint target's bounds document.pending_gradient_bbox_bake.push((network_path.to_vec(), *node_id, gradient.clone())); - None - }; - document - .network_interface - .set_input(&InputConnector::node(*node_id, 6), NodeInput::value(TaggedValue::OptionalDAffine2(transform), false), network_path); + } } } // Wired/exposed fill keeps the connection. @@ -1692,19 +1695,54 @@ fn migrate_node(node_id: &NodeId, node: &DocumentNode, network_path: &[NodeId], network_path, ); - let transform = if g.absolute { - Some(g.transform * g.to_transform()) + // As above: only an absolute placement writes the toggle pair, a deferred bake keeps the default + if g.absolute { + let transform = g.transform * g.to_transform(); + document + .network_interface + .set_input(&InputConnector::node(*node_id, 6), NodeInput::value(TaggedValue::HasTransform(HasTransform(true)), false), network_path); + document + .network_interface + .set_input(&InputConnector::node(*node_id, 7), NodeInput::value(TaggedValue::DAffine2(transform), false), network_path); } else { document.pending_gradient_bbox_bake.push((network_path.to_vec(), *node_id, g.clone())); - None - }; - document - .network_interface - .set_input(&InputConnector::node(*node_id, 6), NodeInput::value(TaggedValue::OptionalDAffine2(transform), false), network_path); + } } } - inputs_count = 7; + inputs_count = 8; + } + + // Upgrade Fill's optional placement transform into the has-transform toggle pair, fixing the input's name and count + if reference == DefinitionIdentifier::ProtoNode(graphene_std::vector_nodes::fill::IDENTIFIER) && inputs_count == 7 { + let mut node_template = resolve_document_node_type(&reference)?.default_node_template(); + let old_inputs = document.network_interface.replace_inputs(node_id, network_path, &mut node_template)?; + + for index in 0..6 { + document.network_interface.set_input(&InputConnector::node(*node_id, index), old_inputs[index].clone(), network_path); + } + match old_inputs[6].as_value() { + Some(TaggedValue::OptionalDAffine2(Some(transform))) => { + let transform = *transform; + document + .network_interface + .set_input(&InputConnector::node(*node_id, 6), NodeInput::value(TaggedValue::HasTransform(HasTransform(true)), false), network_path); + document + .network_interface + .set_input(&InputConnector::node(*node_id, 7), NodeInput::value(TaggedValue::DAffine2(transform), false), network_path); + } + // The unset placeholder becomes the toggle's default false, deriving placement from the bounds + Some(TaggedValue::OptionalDAffine2(None)) => {} + // A wired transform source keeps its connection, marked explicit + _ => { + document + .network_interface + .set_input(&InputConnector::node(*node_id, 6), NodeInput::value(TaggedValue::HasTransform(HasTransform(true)), false), network_path); + document.network_interface.set_input(&InputConnector::node(*node_id, 7), old_inputs[6].clone(), network_path); + } + } + + inputs_count = 8; } // Upgrade Stroke node to reorder parameters and add "Align" and "Paint Order" (#2644) diff --git a/editor/src/messages/tool/common_functionality/graph_modification_utils.rs b/editor/src/messages/tool/common_functionality/graph_modification_utils.rs index 0f58b12710..119fca7c28 100644 --- a/editor/src/messages/tool/common_functionality/graph_modification_utils.rs +++ b/editor/src/messages/tool/common_functionality/graph_modification_utils.rs @@ -14,7 +14,7 @@ use graphene_std::raster_types::{CPU, GPU, Image, Raster}; use graphene_std::subpath::Subpath; use graphene_std::text::{Font, TypesettingConfig}; use graphene_std::vector::misc::ManipulatorPointId; -use graphene_std::vector::style::{FillChoice, PaintOrder, StrokeAlign, StrokeCap, StrokeJoin, initial_gradient_transform_for_bounding_box}; +use graphene_std::vector::style::{FillChoice, HasTransform, PaintOrder, StrokeAlign, StrokeCap, StrokeJoin, initial_gradient_transform_for_bounding_box}; use graphene_std::vector::{GradientSpreadMethod, GradientStops, GradientType, PointId, SegmentId, VectorModificationType}; use std::collections::VecDeque; @@ -648,9 +648,14 @@ pub fn read_fill_node_gradient(fill_node: &DocumentNode, bounding_box: impl FnOn Some(&TaggedValue::GradientSpreadMethod(value)) => value, _ => GradientSpreadMethod::default(), }; + let has_transform = matches!( + fill_node.inputs.get(fill::HasTransformInput::INDEX).and_then(|input| input.as_value()), + Some(&TaggedValue::HasTransform(HasTransform(true))) + ); let transform_input = fill_node.inputs.get(fill::TransformInput::INDEX).and_then(|input| input.as_value()); let transform = match transform_input { - Some(&TaggedValue::OptionalDAffine2(value)) => value.unwrap_or_else(|| initial_gradient_transform_for_bounding_box(bounding_box())), + Some(&TaggedValue::DAffine2(value)) if has_transform => value, + Some(_) => initial_gradient_transform_for_bounding_box(bounding_box()), _ => DAffine2::IDENTITY, }; @@ -798,10 +803,10 @@ pub fn set_fill_for_selected_layers(fill_choice: FillChoice, document: &Document Some(TaggedValue::GradientSpreadMethod(value)) => *value, _ => GradientSpreadMethod::default(), }; + let has_transform = matches!(read(graphene_std::vector::fill::HasTransformInput::INDEX), Some(&TaggedValue::HasTransform(HasTransform(true)))); let transform = match read(graphene_std::vector::fill::TransformInput::INDEX) { - Some(TaggedValue::OptionalDAffine2(value)) => { - value.unwrap_or_else(|| initial_gradient_transform_for_bounding_box(document.network_interface.document_metadata().nonzero_bounding_box(layer))) - } + Some(&TaggedValue::DAffine2(value)) if has_transform => value, + Some(_) => initial_gradient_transform_for_bounding_box(document.network_interface.document_metadata().nonzero_bounding_box(layer)), _ => DAffine2::IDENTITY, }; diff --git a/editor/src/messages/tool/tool_messages/gradient_tool.rs b/editor/src/messages/tool/tool_messages/gradient_tool.rs index c5a1749e74..c63c080f5e 100644 --- a/editor/src/messages/tool/tool_messages/gradient_tool.rs +++ b/editor/src/messages/tool/tool_messages/gradient_tool.rs @@ -2014,7 +2014,7 @@ mod test_gradient { use graph_craft::document::value::TaggedValue; use graphene_std::NodeInputDecleration; use graphene_std::color::SRGBA8; - use graphene_std::vector::style::{GradientSpreadMethod, build_transform_with_y_preservation}; + use graphene_std::vector::style::{GradientSpreadMethod, HasTransform, build_transform_with_y_preservation}; use graphene_std::vector::{GradientStop, GradientStops, fill}; use super::gradient_space_transform; @@ -2067,8 +2067,12 @@ mod test_gradient { _ => GradientSpreadMethod::default(), }; + let has_transform = matches!( + fill_node.inputs.get(fill::HasTransformInput::INDEX).and_then(|input| input.as_value()), + Some(&TaggedValue::HasTransform(HasTransform(true))) + ); let local_transform = match fill_node.inputs.get(fill::TransformInput::INDEX).and_then(|input| input.as_value()) { - Some(&TaggedValue::OptionalDAffine2(Some(value))) => value, + Some(&TaggedValue::DAffine2(value)) if has_transform => value, _ => DAffine2::IDENTITY, }; diff --git a/editor/src/node_graph_executor.rs b/editor/src/node_graph_executor.rs index 8907e80c90..9f6afb54c0 100644 --- a/editor/src/node_graph_executor.rs +++ b/editor/src/node_graph_executor.rs @@ -13,6 +13,7 @@ use graphene_std::list::List; use graphene_std::raster::{CPU, Raster}; use graphene_std::renderer::{RenderMetadata, graphic_list_bounding_box}; use graphene_std::transform::Footprint; +use graphene_std::vector::style::HasTransform; use graphene_std::vector::{Vector, graphic_types}; use graphene_std::{ATTR_TRANSFORM, Graphic, NodeInputDecleration}; use interpreted_executor::dynamic_executor::ResolvedDocumentNodeTypesDelta; @@ -625,10 +626,14 @@ impl NodeGraphExecutor { if fill_transform_unbaked(document, &network_path, fill_node_id) { let absolute_gradient = gradient.to_absolute(bounding_box, item_transform); let gradient_transform = absolute_gradient.transform * absolute_gradient.to_transform(); - let input = InputConnector::node(fill_node_id, graphene_std::vector::fill::TransformInput::INDEX); + let transform_input = InputConnector::node(fill_node_id, graphene_std::vector::fill::TransformInput::INDEX); + let has_transform_input = InputConnector::node(fill_node_id, graphene_std::vector::fill::HasTransformInput::INDEX); document .network_interface - .set_input(&input, NodeInput::value(TaggedValue::OptionalDAffine2(Some(gradient_transform)), false), &network_path); + .set_input(&transform_input, NodeInput::value(TaggedValue::DAffine2(gradient_transform), false), &network_path); + document + .network_interface + .set_input(&has_transform_input, NodeInput::value(TaggedValue::HasTransform(HasTransform(true)), false), &network_path); } // The transform is settled, so its entry no longer needs to persist for a retry on the next open @@ -832,16 +837,16 @@ impl NodeGraphExecutor { } // TODO: Eventually remove this document upgrade code -/// Whether the fill node's transform input is still the unset `OptionalDAffine2(None)` placeholder that the migration leaves -/// behind, meaning its gradient placement has not yet been baked (or set by the user), so a measured bake may safely be written. +/// Whether the fill node's gradient placement is still the unset `HasTransform(false)` placeholder that the migration leaves +/// behind, meaning it has not yet been baked (or set by the user), so a measured bake may safely be written. fn fill_transform_unbaked(document: &DocumentMessageHandler, network_path: &[NodeId], fill_node_id: NodeId) -> bool { let Some(network) = document.network_interface.document_network().nested_network(network_path) else { return false; }; let Some(node) = network.nodes.get(&fill_node_id) else { return false }; matches!( - node.inputs.get(graphene_std::vector::fill::TransformInput::INDEX).and_then(|input| input.as_value()), - Some(TaggedValue::OptionalDAffine2(None)) + node.inputs.get(graphene_std::vector::fill::HasTransformInput::INDEX).and_then(|input| input.as_value()), + Some(TaggedValue::HasTransform(HasTransform(false))) ) } diff --git a/node-graph/graph-craft/src/document/value.rs b/node-graph/graph-craft/src/document/value.rs index 53d693d59f..3f817c6d74 100644 --- a/node-graph/graph-craft/src/document/value.rs +++ b/node-graph/graph-craft/src/document/value.rs @@ -580,6 +580,7 @@ tagged_value! { StrokeJoin(vector::style::StrokeJoin), StrokeAlign(vector::style::StrokeAlign), PaintOrder(vector::style::PaintOrder), + HasTransform(vector::style::HasTransform), GradientType(vector::style::GradientType), GradientSpreadMethod(vector::style::GradientSpreadMethod), ReferencePoint(vector::ReferencePoint), diff --git a/node-graph/libraries/vector-types/src/vector/style.rs b/node-graph/libraries/vector-types/src/vector/style.rs index 321828c450..bcb4828986 100644 --- a/node-graph/libraries/vector-types/src/vector/style.rs +++ b/node-graph/libraries/vector-types/src/vector/style.rs @@ -201,6 +201,15 @@ fn daffine2_identity() -> DAffine2 { DAffine2::IDENTITY } +/// Whether a fill's gradient placement transform is explicitly set, as opposed to derived +/// from the content's bounding box at evaluation. A newtype so the node input's name is +/// fixed in the document format ahead of the transform toggle's own arrival. +#[repr(C)] +#[cfg_attr(feature = "wasm", derive(tsify::Tsify))] +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Hash, graphene_hash::CacheHash, DynAny)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub struct HasTransform(pub bool); + #[repr(C)] #[cfg_attr(feature = "wasm", derive(tsify::Tsify))] #[derive(Debug, Clone, PartialEq, graphene_hash::CacheHash, DynAny)] diff --git a/node-graph/nodes/vector/src/vector_nodes.rs b/node-graph/nodes/vector/src/vector_nodes.rs index 4d82cfb218..fc20f114a9 100644 --- a/node-graph/nodes/vector/src/vector_nodes.rs +++ b/node-graph/nodes/vector/src/vector_nodes.rs @@ -38,7 +38,7 @@ use vector_types::vector::misc::{ CentroidType, ExtrudeJoiningAlgorithm, HandleId, InterpolationDistribution, MergeByDistanceAlgorithm, PointSpacingType, RowsOrColumns, bezpath_from_manipulator_groups, bezpath_to_manipulator_groups, handles_to_segment, is_linear, point_to_dvec2, segment_to_handles, }; -use vector_types::vector::style::{GradientStops, PaintOrder, Stroke, StrokeAlign, StrokeCap, StrokeJoin}; +use vector_types::vector::style::{GradientStops, HasTransform, PaintOrder, Stroke, StrokeAlign, StrokeCap, StrokeJoin}; use vector_types::vector::{FillId, PointId, RegionId, SegmentDomain, SegmentId, StrokeId, VectorExt}; use vector_types::{ATTR_GRADIENT_TYPE, ATTR_SPREAD_METHOD}; use vector_types::{GradientSpreadMethod, GradientType}; @@ -358,10 +358,11 @@ fn fill<'e>( _backup_gradient: IList, _gradient_type: GradientType, _spread_method: GradientSpreadMethod, - _transform: Option, + _has_transform: HasTransform, + _transform: DAffine2, ) -> Result<(Vector, Attr<'e, Fill>, Attr<'e, AppearanceMarker>), Interrupt> { let mut paint = paint_table(fill); - default_gradient_paint(&mut paint, element.bounding_box(), _gradient_type, _spread_method, _transform); + default_gradient_paint(&mut paint, element.bounding_box(), _gradient_type, _spread_method, _has_transform.0.then_some(_transform)); let appearance = stamped_appearance(*content_appearance, Coverage::new_fill(), &paint); let parked = park_paint(ctx.arena(), paint)?; let parked_appearance = park_appearance(ctx.arena(), appearance)?; @@ -380,14 +381,15 @@ fn fill_graphic_leveled<'e>( _backup_gradient: IList, _gradient_type: GradientType, _spread_method: GradientSpreadMethod, - _transform: Option, + _has_transform: HasTransform, + _transform: DAffine2, ) -> Result<(Graphic<'static>, Attr<'e, Fill>, Attr<'e, AppearanceMarker>), Interrupt> { let bounds = match BoundingBox::bounding_box(&element, DAffine2::IDENTITY, false) { RenderBoundingBox::Rectangle(bounds) => Some(bounds), _ => None, }; let mut paint = paint_table(fill); - default_gradient_paint(&mut paint, bounds, _gradient_type, _spread_method, _transform); + default_gradient_paint(&mut paint, bounds, _gradient_type, _spread_method, _has_transform.0.then_some(_transform)); let appearance = stamped_appearance(*content_appearance, Coverage::new_fill(), &paint); let parked = park_paint(ctx.arena(), paint)?; let parked_appearance = park_appearance(ctx.arena(), appearance)?;