diff --git a/editor/src/messages/portfolio/document/data_panel/data_panel_message_handler.rs b/editor/src/messages/portfolio/document/data_panel/data_panel_message_handler.rs index fe5d07b38d..f4e6ec4de1 100644 --- a/editor/src/messages/portfolio/document/data_panel/data_panel_message_handler.rs +++ b/editor/src/messages/portfolio/document/data_panel/data_panel_message_handler.rs @@ -334,6 +334,7 @@ impl TableItemLayout for Graphic<'_> { } fn identifier(&self) -> String { match self { + Self::None => "None".to_string(), Self::Graphic(list) => list.identifier(), Self::Vector(list) => list.identifier(), Self::RasterCPU(list) => list.identifier(), @@ -350,6 +351,7 @@ impl TableItemLayout for Graphic<'_> { } fn value_page(&self, data: &mut LayoutData) -> Vec { match self { + Self::None => label("None"), Self::Graphic(list) => list.layout_with_breadcrumb(data), Self::Vector(list) => list.layout_with_breadcrumb(data), Self::RasterCPU(list) => list.layout_with_breadcrumb(data), 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 2fcad7f069..ae2097cdf5 100644 --- a/editor/src/messages/portfolio/document/graph_operation/utility_types.rs +++ b/editor/src/messages/portfolio/document/graph_operation/utility_types.rs @@ -138,7 +138,7 @@ impl<'a> ModifyInputsContext<'a> { Some(NodeInput::type_default(descriptor!(List), true)), Some(NodeInput::value(TaggedValue::DVec2(location), false)), Some(NodeInput::value(TaggedValue::DVec2(dimensions), false)), - Some(NodeInput::value(TaggedValue::Color(Some(background)), false)), + Some(NodeInput::value(TaggedValue::Color(background), false)), Some(NodeInput::value(TaggedValue::Bool(clip), false)), ]); self.network_interface.insert_node(new_id, artboard_node_template, &[]); @@ -300,7 +300,7 @@ impl<'a> ModifyInputsContext<'a> { pub fn insert_color_value(&mut self, color: Color, layer: LayerNodeIdentifier) { let color_value = resolve_proto_node_type(graphene_std::math_nodes::color_value::IDENTIFIER) .expect("Color Value node does not exist") - .node_template_input_override([Some(NodeInput::value(TaggedValue::None, false)), Some(NodeInput::value(TaggedValue::Color(Some(color)), false))]); + .node_template_input_override([Some(NodeInput::value(TaggedValue::None, false)), Some(NodeInput::value(TaggedValue::Color(color), false))]); let color_value_id = NodeId::new(); self.network_interface.insert_node(color_value_id, color_value, &[]); @@ -460,8 +460,12 @@ impl<'a> ModifyInputsContext<'a> { let input_connector = InputConnector::node(fill_node_id, graphene_std::vector::fill::FillInput::INDEX); let backup_input_connector = InputConnector::node(fill_node_id, graphene_std::vector::fill::BackupColorInput::INDEX); - self.set_input_with_refresh(backup_input_connector, NodeInput::value(TaggedValue::Color(color), false), true); - self.set_input_with_refresh(input_connector, NodeInput::value(TaggedValue::Color(color), false), false); + // The backup remembers the last solid color, so the red-slash "none" choice leaves it untouched + if let Some(color) = color { + self.set_input_with_refresh(backup_input_connector, NodeInput::value(TaggedValue::Color(color), false), true); + } + let fill_value = color.map_or_else(TaggedValue::no_paint, TaggedValue::Color); + self.set_input_with_refresh(input_connector, NodeInput::value(fill_value, false), false); } pub fn fill_gradient_set(&mut self, gradient: Gradient, gradient_type: GradientType, spread_method: GradientSpreadMethod, transform: DAffine2) { @@ -716,7 +720,7 @@ impl<'a> ModifyInputsContext<'a> { }; let input_connector = InputConnector::node(stroke_node_id, graphene_std::vector::stroke::PaintInput::INDEX); - self.set_input_with_refresh(input_connector, NodeInput::value(TaggedValue::Color(color), false), true); + self.set_input_with_refresh(input_connector, NodeInput::value(color.map_or_else(TaggedValue::no_paint, TaggedValue::Color), false), true); let input_connector = InputConnector::node(stroke_node_id, graphene_std::vector::stroke::WeightInput::INDEX); self.set_input_with_refresh(input_connector, NodeInput::value(TaggedValue::F64(stroke.weight), false), true); let input_connector = InputConnector::node(stroke_node_id, graphene_std::vector::stroke::AlignInput::INDEX); diff --git a/editor/src/messages/portfolio/document/node_graph/document_node_definitions.rs b/editor/src/messages/portfolio/document/node_graph/document_node_definitions.rs index b8391b8f4f..9eb80d36a4 100644 --- a/editor/src/messages/portfolio/document/node_graph/document_node_definitions.rs +++ b/editor/src/messages/portfolio/document/node_graph/document_node_definitions.rs @@ -354,7 +354,7 @@ fn document_node_definitions() -> HashMap), true), NodeInput::value(TaggedValue::DVec2(DVec2::ZERO), false), NodeInput::value(TaggedValue::DVec2(DVec2::new(1920., 1080.)), false), - NodeInput::value(TaggedValue::Color(Some(Color::WHITE)), false), + NodeInput::value(TaggedValue::Color(Color::WHITE), false), NodeInput::value(TaggedValue::Bool(true), false), ], ..Default::default() 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 596d1b6dc2..de43b8567f 100644 --- a/editor/src/messages/portfolio/document/node_graph/node_properties.rs +++ b/editor/src/messages/portfolio/document/node_graph/node_properties.rs @@ -32,9 +32,7 @@ use graphene_std::text_nodes::StringCapitalization; 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, Gradient, GradientSpreadMethod, GradientType, GradientUI, PaintOrder, StrokeAlign, StrokeCap, StrokeJoin, build_transform_with_y_preservation, -}; +use graphene_std::vector::style::{FillChoiceUI, Gradient, GradientSpreadMethod, GradientType, GradientUI, PaintOrder, StrokeAlign, StrokeCap, StrokeJoin, build_transform_with_y_preservation}; use graphene_std::vector::{QRCodeErrorCorrectionLevel, VectorModification}; pub(crate) fn string_properties(text: &str) -> Vec { @@ -1175,30 +1173,37 @@ pub fn color_widget(parameter_widgets_info: ParameterWidgetsInfo, color_button: widgets.push(Separator::new(SeparatorStyle::Unrelated).widget_instance()); // Add the color input - match &**tagged_value { - TaggedValue::Color(color) => widgets.push( - color_button - .value(FillChoiceUI::from(&match color { - Some(color) => FillChoice::Solid(*color), - None => FillChoice::None, - })) - .on_update(update_value(|input: &ColorInput| TaggedValue::Color(input.value.as_solid().map(Color::from)), node_id, index)) - .on_commit(commit_value) - .widget_instance(), - ), - TaggedValue::Gradient(stops) => widgets.push( - color_button - .value(FillChoiceUI::from(&FillChoice::Gradient(stops.clone()))) - .on_update(update_value( - |input: &ColorInput| TaggedValue::Gradient(input.value.as_gradient().map(Gradient::from).unwrap_or_default()), - node_id, - index, - )) - .on_commit(commit_value) - .widget_instance(), - ), - x => warn!("Color {x:?}"), - } + let widget_value = match &**tagged_value { + TaggedValue::Color(color) => FillChoiceUI::Solid(SRGBA8::from(*color)), + TaggedValue::Gradient(stops) => FillChoiceUI::Gradient(GradientUI::from(stops)), + value if value.is_no_paint() => FillChoiceUI::None, + x => { + warn!("Color {x:?}"); + return LayoutGroup::row(widgets); + } + }; + + // A paint input (`allow_none`) stores the pick as a plain color, gradient, or no-paint type default, + // while a plain color or gradient input always keeps its own value type + let on_update: fn(&ColorInput) -> TaggedValue = if color_button.allow_none { + |input| match &input.value { + FillChoiceUI::None => TaggedValue::no_paint(), + FillChoiceUI::Solid(srgba) => TaggedValue::Color(Color::from(*srgba)), + FillChoiceUI::Gradient(gradient_ui) => TaggedValue::Gradient(Gradient::from(gradient_ui)), + } + } else if matches!(&**tagged_value, TaggedValue::Gradient(_)) { + |input| TaggedValue::Gradient(input.value.as_gradient().map(Gradient::from).unwrap_or_default()) + } else { + |input| TaggedValue::Color(input.value.as_solid().map(Color::from).unwrap_or(Color::TRANSPARENT)) + }; + + widgets.push( + color_button + .value(widget_value) + .on_update(update_value(on_update, node_id, index)) + .on_commit(commit_value) + .widget_instance(), + ); LayoutGroup::row(widgets) } @@ -2461,9 +2466,6 @@ pub(crate) fn fill_properties(node_id: NodeId, context: &mut NodePropertiesConte Other, } - let connector = InputConnector::node(node_id, FillInput::INDEX); - let input_type = context.network_interface.input_type(&connector, context.selection_network_path); - // Pass blank_assist=false because the assist slot is filled below ("Reverse Stops" button when in gradient mode) let mut widgets_first_row = start_widgets(ParameterWidgetsInfo::new(node_id, FillInput::INDEX, false, context)); @@ -2475,42 +2477,33 @@ pub(crate) fn fill_properties(node_id: NodeId, context: &mut NodePropertiesConte // bounding-box default transform needs the layer, and it falls back to a unit box when there isn't one. let layer = root_layer_for_chain_node(node_id, context); - let fill = match input_type.compiled_nested_type() { - Some(ty) if ty == &concrete!(List) => { - if let Ok(document_node) = get_document_node(node_id, context) { - let color = match document_node.inputs[FillInput::INDEX].as_value() { - Some(&TaggedValue::Color(c)) => c, - _ => None, - }; - ResolvedFill::Solid(color) - } else { - ResolvedFill::Other - } - } - Some(ty) if ty == &concrete!(List) => { - // Read this node's own inputs rather than the layer's nearest Fill, which may be a different node when Fills are chained - if let Ok(document_node) = get_document_node(node_id, context) - && let Some(gradient) = graph_modification_utils::read_fill_node_gradient(document_node, || { + let fill = match get_document_node(node_id, context) { + Ok(document_node) => match document_node.inputs[FillInput::INDEX].as_value() { + Some(TaggedValue::Color(color)) => ResolvedFill::Solid(Some(*color)), + Some(value) if value.is_no_paint() => ResolvedFill::Solid(None), + Some(TaggedValue::Gradient(_)) => { + match graph_modification_utils::read_fill_node_gradient(document_node, || { layer.map_or([DVec2::ZERO, DVec2::ONE], |layer| context.network_interface.document_metadata().nonzero_bounding_box(layer)) }) { - ResolvedFill::Gradient { - gradient: gradient.stops, - gradient_type: gradient.gradient_type, - spread_method: gradient.spread_method, - transform: gradient.transform, - transform_is_value: gradient.transform_is_value, + Some(gradient) => ResolvedFill::Gradient { + gradient: gradient.stops, + gradient_type: gradient.gradient_type, + spread_method: gradient.spread_method, + transform: gradient.transform, + transform_is_value: gradient.transform_is_value, + }, + None => ResolvedFill::Other, } - } else { - ResolvedFill::Other } - } - _ => ResolvedFill::Other, + _ => ResolvedFill::Other, + }, + Err(_) => ResolvedFill::Other, }; let (backup_color, backup_gradient) = match get_document_node(node_id, context) { Ok(document_node) => { let backup_color = match document_node.inputs[BackupColorInput::INDEX].as_value() { - Some(&TaggedValue::Color(color)) => color, + Some(&TaggedValue::Color(color)) => Some(color), _ => None, }; let backup_stops = match document_node.inputs[BackupGradientInput::INDEX].as_value() { @@ -2549,21 +2542,26 @@ pub(crate) fn fill_properties(node_id: NodeId, context: &mut NodePropertiesConte ResolvedFill::Other => FillChoiceUI::None, }; - let solid_set_messages = move |color: Option| Message::Batched { - messages: Box::new([ + let solid_set_messages = move |color: Option| { + let mut messages = vec![ NodeGraphMessage::SetInputValue { node_id, input_index: FillInput::INDEX, - value: Box::new(TaggedValue::Color(color)), + value: Box::new(color.map_or_else(TaggedValue::no_paint, TaggedValue::Color)), } .into(), - NodeGraphMessage::SetInputValue { - node_id, - input_index: BackupColorInput::INDEX, - value: Box::new(TaggedValue::Color(color)), - } - .into(), - ]), + ]; + if let Some(color) = color { + messages.push( + NodeGraphMessage::SetInputValue { + node_id, + input_index: BackupColorInput::INDEX, + value: Box::new(TaggedValue::Color(color)), + } + .into(), + ); + } + Message::Batched { messages: messages.into() } }; let gradient_set_messages = move |gradient: Gradient| Message::Batched { @@ -2611,7 +2609,7 @@ pub(crate) fn fill_properties(node_id: NodeId, context: &mut NodePropertiesConte let entries = vec![ RadioEntryData::new("solid") .label("Solid") - .on_update(update_value(move |_| TaggedValue::Color(backup_color), node_id, FillInput::INDEX)) + .on_update(update_value(move |_| backup_color.map_or_else(TaggedValue::no_paint, TaggedValue::Color), node_id, FillInput::INDEX)) .on_commit(commit_value), RadioEntryData::new("gradient") .label("Gradient") diff --git a/editor/src/messages/portfolio/document/storage_tests/round_trip_tests.rs b/editor/src/messages/portfolio/document/storage_tests/round_trip_tests.rs index 1c5a4853ea..3fc5d80b8d 100644 --- a/editor/src/messages/portfolio/document/storage_tests/round_trip_tests.rs +++ b/editor/src/messages/portfolio/document/storage_tests/round_trip_tests.rs @@ -708,3 +708,60 @@ async fn demo_artwork_edit_autosaves_and_round_trips() { // Autosaving the undone state still round-trips cleanly (no drift panic). editor.active_document_mut().commit_storage_snapshot(&byte_store, true); } + +/// The document's single Fill node, as `(network_path, node_id)`. +fn find_fill_node(document: &DocumentMessageHandler) -> (Vec, graph_craft::document::NodeId) { + node_paths(&document.network_interface) + .into_iter() + .find(|(network_path, node_id)| { + let Some(network) = document.network_interface.nested_network(network_path) else { return false }; + network.nodes[node_id].implementation == graph_craft::document::DocumentNodeImplementation::ProtoNode(graphene_std::vector_nodes::fill::IDENTIFIER) + }) + .expect("the document should contain a Fill node") +} + +/// The stored paint value of the document's single Fill node. +fn fill_paint_value(document: &DocumentMessageHandler) -> graph_craft::document::value::TaggedValue { + use graphene_std::NodeInputDecleration as _; + + let (network_path, node_id) = find_fill_node(document); + let network = document.network_interface.nested_network(&network_path).expect("the found network path should resolve"); + let input = network.nodes[&node_id] + .inputs + .get(graphene_std::vector::fill::FillInput::INDEX) + .expect("Fill should have a paint input"); + input.as_value().expect("the paint input should hold a value").clone() +} + +#[tokio::test] +async fn none_fill_survives_document_reopen() { + use graphene_std::NodeInputDecleration as _; + + let mut editor = EditorTestUtils::create(); + editor.new_document().await; + editor.drag_tool(ToolType::Rectangle, 0., 0., 100., 100., ModifierKeys::empty()).await; + + // Pick the red-slash "none" paint, stored the same way as the Fill widget's None choice + let (_, fill_node_id) = find_fill_node(editor.active_document()); + editor + .handle_message(NodeGraphMessage::SetInputValue { + node_id: fill_node_id, + input_index: graphene_std::vector::fill::FillInput::INDEX, + value: Box::new(graph_craft::document::value::TaggedValue::no_paint()), + }) + .await; + assert!(fill_paint_value(editor.active_document()).is_no_paint(), "the None pick should store as no_paint"); + + // Reopen through the editor's real open path, which runs the document migrations + let serialized = editor.active_document().serialize_document(); + editor + .handle_message(PortfolioMessage::OpenDocumentFile { + document_name: None, + document_path: None, + document_serialized_content: serialized, + }) + .await; + + let reopened_paint = fill_paint_value(editor.active_document()); + assert!(reopened_paint.is_no_paint(), "a none fill should survive reopening, but the stored paint became {reopened_paint:?}"); +} diff --git a/editor/src/messages/portfolio/document_migration.rs b/editor/src/messages/portfolio/document_migration.rs index a09e834164..83622851e7 100644 --- a/editor/src/messages/portfolio/document_migration.rs +++ b/editor/src/messages/portfolio/document_migration.rs @@ -10,6 +10,8 @@ use graph_craft::application_io::resource::{DataSource, Resource, ResourceHash, use graph_craft::descriptor; use graph_craft::document::DocumentNode; use graph_craft::document::{DocumentNodeImplementation, NodeInput, value::TaggedValue}; +use graphene_std::Color; +use graphene_std::NodeInputDecleration; use graphene_std::ProtoNodeIdentifier; use graphene_std::text::{TextAlign, TypesettingConfig}; use graphene_std::transform::ScaleType; @@ -1630,8 +1632,8 @@ fn migrate_node(node_id: &NodeId, node: &DocumentNode, network_path: &[NodeId], Some(TaggedValue::LegacyFill(old_fill)) => { let exposed = old_inputs[1].is_exposed(); let fill_value = match old_fill { - graphic_types::migrations::legacy::LegacyFill::None => TaggedValue::Color(None), - graphic_types::migrations::legacy::LegacyFill::Solid(color) => TaggedValue::Color(Some(*color)), + graphic_types::migrations::legacy::LegacyFill::None => TaggedValue::no_paint(), + graphic_types::migrations::legacy::LegacyFill::Solid(color) => TaggedValue::Color(*color), graphic_types::migrations::legacy::LegacyFill::Gradient(gradient) => TaggedValue::Gradient(gradient.stops.clone()), }; document @@ -1730,6 +1732,57 @@ fn migrate_node(node_id: &NodeId, node: &DocumentNode, network_path: &[NodeId], document.network_interface.set_input(&InputConnector::node(*node_id, 9), old_inputs[4].clone(), network_path); } + // TODO: Eventually remove this migration document upgrade code + // A legacy "no color" on a plain color connector (`TaggedValue::no_paint()` restored by the deserializer) becomes a color, + // since only paint connectors keep the no-paint choice + { + let migrate_color_input = |input: &NodeInput, fallback: Color| -> Option { + let NodeInput::Value { tagged_value, exposed } = input else { return None }; + if !tagged_value.is_no_paint() { + return None; + } + Some(NodeInput::value(TaggedValue::Color(fallback), *exposed)) + }; + + let conversions: &[(ProtoNodeIdentifier, usize, Color)] = &[ + (graphene_std::vector::fill::IDENTIFIER, graphene_std::vector::fill::BackupColorInput::INDEX, Color::BLACK), + ( + graphene_std::artboard::create_artboard::IDENTIFIER, + graphene_std::artboard::create_artboard::BackgroundInput::INDEX, + Color::WHITE, + ), + ( + graphene_std::math_nodes::color_value::IDENTIFIER, + graphene_std::math_nodes::color_value::ColorInput::INDEX, + Color::TRANSPARENT, + ), + ( + graphene_std::raster_nodes::adjustments::black_and_white::IDENTIFIER, + graphene_std::raster_nodes::adjustments::black_and_white::TintInput::INDEX, + Color::BLACK, + ), + ( + graphene_std::raster_nodes::blending_nodes::color_overlay::IDENTIFIER, + graphene_std::raster_nodes::blending_nodes::color_overlay::ColorInput::INDEX, + Color::BLACK, + ), + ( + graphene_std::raster_nodes::std_nodes::empty_image::IDENTIFIER, + graphene_std::raster_nodes::std_nodes::empty_image::ColorInput::INDEX, + Color::WHITE, + ), + ]; + for &(ref identifier, index, fallback) in conversions { + if reference != DefinitionIdentifier::ProtoNode(identifier.clone()) { + continue; + } + let Some(input) = node.inputs.get(index) else { continue }; + if let Some(migrated) = migrate_color_input(input, fallback) { + document.network_interface.set_input(&InputConnector::node(*node_id, index), migrated, network_path); + } + } + } + // Upgrade Text node to include line height and character spacing, which were previously hardcoded to 1, from https://github.com/GraphiteEditor/Graphite/pull/2016 if reference == DefinitionIdentifier::ProtoNode(ProtoNodeIdentifier::new("graphene_std::text::TextNode")) && inputs_count == 8 { let mut template: NodeTemplate = legacy_text_node_template()?; 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 c4ba053364..f8c22639f4 100644 --- a/editor/src/messages/tool/common_functionality/graph_modification_utils.rs +++ b/editor/src/messages/tool/common_functionality/graph_modification_utils.rs @@ -362,10 +362,10 @@ pub fn gradient_orientation_rightward(transform: glam::DAffine2) -> bool { /// Get the current fill of a layer from the closest "Fill" node. pub fn get_fill_color(layer: LayerNodeIdentifier, network_interface: &NodeNetworkInterface) -> Option { let inputs = NodeGraphLayer::new(layer, network_interface).find_node_inputs(&DefinitionIdentifier::ProtoNode(graphene_std::vector::fill::IDENTIFIER))?; - let &TaggedValue::Color(color) = inputs.get(graphene_std::vector::fill::FillInput::INDEX)?.as_value()? else { + let TaggedValue::Color(color) = inputs.get(graphene_std::vector::fill::FillInput::INDEX)?.as_value()? else { return None; }; - color + Some(*color) } /// Get the current blend mode of a layer from the closest upstream "Blend Mode" node. @@ -666,7 +666,11 @@ pub fn read_fill_node_gradient(fill_node: &DocumentNode, bounding_box: impl FnOn pub fn get_stroke_color(layer: LayerNodeIdentifier, network_interface: &NodeNetworkInterface) -> Option> { let color_index = graphene_std::vector::stroke::PaintInput::INDEX; let tagged = NodeGraphLayer::new(layer, network_interface).find_input(&DefinitionIdentifier::ProtoNode(graphene_std::vector::stroke::IDENTIFIER), color_index)?; - if let TaggedValue::Color(color) = tagged { Some(*color) } else { None } + match tagged { + TaggedValue::Color(color) => Some(Some(*color)), + value if value.is_no_paint() => Some(None), + _ => None, + } } /// Aggregated fill state across all selected non-artboard layers. @@ -699,8 +703,9 @@ pub fn selected_fill_state(document: &DocumentMessageHandler) -> Option Some(color.map_or(FillChoice::None, FillChoice::Solid)), + &TaggedValue::Color(color) => Some(FillChoice::Solid(color)), TaggedValue::Gradient(stops) => Some(FillChoice::Gradient(stops.clone())), + value if value.is_no_paint() => Some(FillChoice::None), _ => None, } })() @@ -825,7 +830,7 @@ pub fn set_stroke_color_for_selected_layers(color: Option, weight: f64, d for layer in layers { if let Some(node_id) = get_stroke_id(layer, &document.network_interface) { let input_index = graphene_std::vector::stroke::PaintInput::INDEX; - let value = Box::new(TaggedValue::Color(color)); + let value = Box::new(color.map_or_else(TaggedValue::no_paint, TaggedValue::Color)); responses.add(NodeGraphMessage::SetInputValue { node_id, input_index, value }); } else { let stroke = graphene_std::vector::style::Stroke::new(weight); diff --git a/node-graph/graph-craft/src/document/value.rs b/node-graph/graph-craft/src/document/value.rs index 111de224ce..35dba2b229 100644 --- a/node-graph/graph-craft/src/document/value.rs +++ b/node-graph/graph-craft/src/document/value.rs @@ -63,10 +63,11 @@ macro_rules! tagged_value { #[serde(deserialize_with = "core_types::misc::migrate_to_f64_array")] // TODO: Eventually remove this migration document upgrade code #[serde(alias = "F64Table", alias = "VecF64", alias = "VecF32", alias = "F64Array4")] F64Array(Vec), - /// Stored compactly as an `Option`, materializes as `List` at runtime via `to_dynany`/`to_any`. Aliases recover legacy on-disk shapes. - #[serde(deserialize_with = "core_types::misc::migrate_to_optional_color")] // TODO: Eventually remove this migration document upgrade code + /// A plain, always-present color. Aliases recover legacy on-disk shapes; a legacy `null` payload (the old "no color") + /// is routed to [`TaggedValue::no_paint`] by `deserialize_tagged_value_with_legacy_migration`. + #[serde(deserialize_with = "core_types::misc::migrate_to_color")] // TODO: Eventually remove this migration document upgrade code #[serde(alias = "ColorTable", alias = "OptionalColor", alias = "ColorNotInTable")] - Color(Option), + Color(Color), /// Stored compactly as a `Gradient`, materializes as a single-row `List` at runtime via `to_dynany`/`to_any`. Aliases recover legacy on-disk shapes. /// (Old documents that stored a full `Gradient` struct under this same `"Gradient"` tag are routed to `LegacyGradient` by `deserialize_tagged_value_with_legacy_migration`.) #[serde(deserialize_with = "graphic_types::vector_types::gradient::migrate_to_gradient")] // TODO: Eventually remove this migration document upgrade code @@ -156,10 +157,7 @@ macro_rules! tagged_value { let list: List = values.into_iter().map(core_types::list::Item::new_from_element).collect(); Box::new(list) } - Self::Color(color) => { - let list: List = color.into_iter().map(core_types::list::Item::new_from_element).collect(); - Box::new(list) - } + Self::Color(color) => Box::new(List::::new_from_element(color)), Self::Gradient(stops) => Box::new(List::::new_from_element(stops)), Self::BrushStrokes(strokes) => { let list: List = strokes.into_iter().map(core_types::list::Item::new_from_element).collect(); @@ -203,10 +201,7 @@ macro_rules! tagged_value { let list: List = values.into_iter().map(core_types::list::Item::new_from_element).collect(); Arc::new(list) } - Self::Color(color) => { - let list: List = color.into_iter().map(core_types::list::Item::new_from_element).collect(); - Arc::new(list) - } + Self::Color(color) => Arc::new(List::::new_from_element(color)), Self::Gradient(stops) => Arc::new(List::::new_from_element(stops)), Self::BrushStrokes(strokes) => { let list: List = strokes.into_iter().map(core_types::list::Item::new_from_element).collect(); @@ -339,7 +334,7 @@ macro_rules! tagged_value { Self::from_type_or_none(&Type::Concrete(td)).to_edge() } Self::F64Array(values) => Ok(leveled_record_value_source(values)), - Self::Color(color) => Ok(leveled_record_value_source(color.into_iter().collect::>())), + Self::Color(color) => Ok(leveled_record_value_source(vec![color])), Self::Gradient(stops) => Ok(leveled_record_value_source(vec![stops])), Self::BrushStrokes(strokes) => Ok(leveled_record_value_source(strokes)), // ======================= @@ -442,14 +437,14 @@ macro_rules! tagged_value { // Tries using the default for the tagged value type. If it not implemented, then uses the default used in document_node_types. If it is not used there, then TaggedValue::None is returned. if name == core_types::normalize_type_name(std::any::type_name::<()>()) { return Some(TaggedValue::None) } // List-wrapped types need a single-item default with the element's default, not an empty list - if name == core_types::normalize_type_name(std::any::type_name::>()) { return Some(TaggedValue::Color(Some(Color::default()))) } + if name == core_types::normalize_type_name(std::any::type_name::>()) { return Some(TaggedValue::Color(Color::default())) } if name == core_types::normalize_type_name(std::any::type_name::>()) { return Some(TaggedValue::Gradient(Gradient::default())) } $( if name == core_types::normalize_type_name(std::any::type_name::<$ty>()) { return Some(TaggedValue::$identifier(Default::default())) } )* if name == core_types::normalize_type_name(std::any::type_name::>()) { return Some(TaggedValue::F64Array(Vec::new())) } if name == core_types::normalize_type_name(std::any::type_name::>()) { return Some(TaggedValue::BrushStrokes(Vec::new())) } // Leveled inputs type by their element; each element name maps to the // same tagged default as its legacy list form. - if name == core_types::normalize_type_name(std::any::type_name::()) { return Some(TaggedValue::Color(Some(Color::default()))) } + if name == core_types::normalize_type_name(std::any::type_name::()) { return Some(TaggedValue::Color(Color::default())) } if name == core_types::normalize_type_name(std::any::type_name::()) { return Some(TaggedValue::Gradient(Gradient::default())) } if name == core_types::normalize_type_name(std::any::type_name::()) { return Some(TaggedValue::BrushStrokes(Vec::new())) } if name == core_types::normalize_type_name(std::any::type_name::()) { return Some(TaggedValue::TypeDefault(core_types::descriptor!(List))) } @@ -722,10 +717,10 @@ impl TaggedValue { () if ty == TypeId::of::() => to_dvec2(string).map(TaggedValue::DVec2)?, () if ty == TypeId::of::() => FromStr::from_str(string).map(TaggedValue::Bool).ok()?, // `Color` (not in a `List`) is still currently needed by `BlackAndWhiteNode` and `ColorOverlayNode` GPU `shader_node(PerPixelAdjust)` variants - () if ty == TypeId::of::() => to_color(string).map(|color| TaggedValue::Color(Some(color)))?, - () if ty == TypeId::of::>() => to_color(string).map(|color| TaggedValue::Color(Some(color)))?, + () if ty == TypeId::of::() => to_color(string).map(TaggedValue::Color)?, + () if ty == TypeId::of::>() => to_color(string).map(TaggedValue::Color)?, // The Fill and Stroke nodes' paint connectors default to `List`, their first registered implementation row - () if ty == TypeId::of::>() => to_color(string).map(|color| TaggedValue::Color(Some(color)))?, + () if ty == TypeId::of::>() => to_color(string).map(TaggedValue::Color)?, () if ty == TypeId::of::>() => to_gradient(string).map(TaggedValue::Gradient)?, () if ty == TypeId::of::() => to_reference_point(string).map(TaggedValue::ReferencePoint)?, _ => return None, @@ -743,6 +738,16 @@ impl TaggedValue { _ => panic!("Passed value is not of type u32"), } } + + /// The stored form of a paint input's red-slash "no paint" choice: the `List` type default, materializing as an empty paint list. + pub fn no_paint() -> Self { + TaggedValue::TypeDefault(descriptor!(List)) + } + + /// Whether this is the `List` type default created by [`Self::no_paint`] (and by disconnecting a paint wire). + pub fn is_no_paint(&self) -> bool { + matches!(self, TaggedValue::TypeDefault(td) if *td == descriptor!(List)) + } } /// Custom deserializer hooked onto `NodeInput::Value::tagged_value` that intercepts removed-variant tags before delegating to `TaggedValue`'s standard derive. @@ -758,6 +763,7 @@ impl TaggedValue { /// - `Vector` (or alias `VectorData`): /// - non-empty → `TaggedValue::VectorModification()` (the document_migration's Path pass disambiguates this between SVG-import legacy and a discardable modern baked value via the input's `exposed` flag) /// - empty → `TaggedValue::TypeDefault(descriptor!(List))` +/// - `FillChoice` → `TaggedValue::Color` (solid), `TaggedValue::Gradient` (gradient), or `TaggedValue::no_paint()` (none) /// /// All other tags (including ones with the modern shape) fall through to the standard derived `Deserialize` for `TaggedValue`. // TODO: Eventually remove this migration document upgrade code @@ -794,6 +800,31 @@ pub fn deserialize_tagged_value_with_legacy_migration<'de, D: serde::Deserialize } return Ok(MemoHash::new(TaggedValue::TypeDefault(descriptor!(List)))); } + // The `Color` tag used to carry `Option`, where a `null` payload (or an empty legacy color table) was the red-slash "no paint" choice + "Color" | "ColorTable" | "OptionalColor" | "ColorNotInTable" + if content.is_null() + || content + .as_object() + .and_then(|c| c.get("element").or_else(|| c.get("instance")).or_else(|| c.get("instances"))) + .and_then(|e| e.as_array()) + .is_some_and(|colors| colors.is_empty()) => + { + return Ok(MemoHash::new(TaggedValue::no_paint())); + } + // The removed `FillChoice` variant decomposes into the plain paint values + "FillChoice" => { + if let Some(payload) = content.as_object() { + if let Some(solid) = payload.get("Solid") { + let color: Color = serde_json::from_value(solid.clone()).map_err(serde::de::Error::custom)?; + return Ok(MemoHash::new(TaggedValue::Color(color))); + } + if let Some(gradient) = payload.get("Gradient") { + let gradient: Gradient = serde_json::from_value(gradient.clone()).map_err(serde::de::Error::custom)?; + return Ok(MemoHash::new(TaggedValue::Gradient(gradient))); + } + } + return Ok(MemoHash::new(TaggedValue::no_paint())); + } // The `Gradient` tag was reused: it used to carry a full `Gradient` struct (now `LegacyGradient`), and now carries an `Option`. // Disambiguate by payload shape: a Gradient struct has `start`/`end` keys; a `Gradient` has none of those (it has `position`/`midpoint`/`color`). "Gradient" if content.as_object().is_some_and(|c| c.contains_key("start") && c.contains_key("end")) => { @@ -927,7 +958,7 @@ mod leveled_edges { assert_eq!(edge.ty(), &record_source_type::()); assert_eq!(edge.layout().depth, 1); - let edge = TaggedValue::Color(Some(Color::default())).to_edge().unwrap(); + let edge = TaggedValue::Color(Color::default()).to_edge().unwrap(); assert_eq!(edge.ty(), &record_source_type::()); assert_eq!(edge.layout().depth, 1); @@ -973,3 +1004,19 @@ mod record_defaults { assert_eq!(TaggedValue::from_primitive_string("true", &record_source_type::()), Some(TaggedValue::Bool(true))); } } + +#[cfg(test)] +mod paint_default_parsing { + use super::*; + + /// Table-era documents stored the red-slash "no paint" fill as an empty color table, which must keep + /// deserializing to [`TaggedValue::no_paint`] rather than collapsing to a transparent color. + #[test] + fn empty_legacy_color_table_deserializes_to_no_paint() { + for payload in [r#"{"ColorTable": {"instances": []}}"#, r#"{"ColorTable": {"element": []}}"#, r#"{"Color": null}"#] { + let mut deserializer = serde_json::Deserializer::from_str(payload); + let value = deserialize_tagged_value_with_legacy_migration(&mut deserializer).expect("The legacy payload should deserialize"); + assert!(value.is_no_paint(), "The legacy payload `{payload}` should migrate to the no-paint choice"); + } + } +} diff --git a/node-graph/libraries/core-types/src/misc.rs b/node-graph/libraries/core-types/src/misc.rs index 8fb447662d..f66306f4cc 100644 --- a/node-graph/libraries/core-types/src/misc.rs +++ b/node-graph/libraries/core-types/src/misc.rs @@ -69,7 +69,7 @@ struct LegacyTable { } // TODO: Eventually remove this migration document upgrade code -pub fn migrate_to_optional_color<'de, D: serde::Deserializer<'de>>(deserializer: D) -> Result, D::Error> { +pub fn migrate_to_color<'de, D: serde::Deserializer<'de>>(deserializer: D) -> Result { use no_std_types::color::Color; use serde::Deserialize; @@ -81,8 +81,8 @@ pub fn migrate_to_optional_color<'de, D: serde::Deserializer<'de>>(deserializer: } Ok(match ColorFormat::deserialize(deserializer)? { - ColorFormat::OptionalColor(color) => color, - ColorFormat::List(list) => list.element.into_iter().next(), + ColorFormat::OptionalColor(color) => color.unwrap_or(Color::TRANSPARENT), + ColorFormat::List(list) => list.element.into_iter().next().unwrap_or(Color::TRANSPARENT), }) } diff --git a/node-graph/libraries/graphic-types/src/graphic/glue.rs b/node-graph/libraries/graphic-types/src/graphic/glue.rs index 799344e541..03ef634d42 100644 --- a/node-graph/libraries/graphic-types/src/graphic/glue.rs +++ b/node-graph/libraries/graphic-types/src/graphic/glue.rs @@ -11,6 +11,7 @@ use vector_types::Vector; /// [`map_groups_to_resident`] re-parks it into a serving arena. pub fn map_groups_to_owned<'out>(graphic: &Graphic<'_>) -> Graphic<'out> { match graphic { + Graphic::None => Graphic::None, Graphic::Group(group) => Graphic::Group(group.copy_out()), Graphic::Graphic(children) => { let mut out = List::new(); @@ -78,6 +79,7 @@ unsafe fn deep_repark_graphic(value: &(dyn std::any::Any + Send + Sync), dst: *m /// what this level newly produced. `None` reports arena exhaustion. pub fn map_groups_to_persistent<'p>(graphic: &Graphic<'_>, promotion: &core_types::record::Promotion<'p>) -> Option> { match graphic { + Graphic::None => Some(Graphic::None), Graphic::Group(group) => group.to_persistent(promotion).map(Graphic::Group), Graphic::Graphic(children) => { let mut out = List::new(); @@ -196,7 +198,7 @@ fn graphic_retained_heap(graphic: &Graphic<'_>) -> usize { Graphic::Text(text) => text.len(), Graphic::Gradient(gradient) => gradient.len() * size_of::<(f64, Color)>(), Graphic::Graphic(children) => (0..children.len()).filter_map(|index| children.element(index)).map(graphic_retained_heap).sum(), - Graphic::Group(_) | Graphic::RasterGPU(_) | Graphic::Color(_) => 0, + Graphic::None | Graphic::Group(_) | Graphic::RasterGPU(_) | Graphic::Color(_) => 0, } } diff --git a/node-graph/libraries/graphic-types/src/graphic/legacy.rs b/node-graph/libraries/graphic-types/src/graphic/legacy.rs index 057a2d10a1..d3ddfd0475 100644 --- a/node-graph/libraries/graphic-types/src/graphic/legacy.rs +++ b/node-graph/libraries/graphic-types/src/graphic/legacy.rs @@ -39,6 +39,7 @@ pub(crate) fn run_to_legacy_list(graphic: &Graphic<'_>) -> Graphic<'out> { match graphic { + Graphic::None => Graphic::None, Graphic::Group(group) => group_to_legacy_graphic(group), Graphic::Graphic(children) => { let mut out = List::new(); diff --git a/node-graph/libraries/graphic-types/src/graphic/mod.rs b/node-graph/libraries/graphic-types/src/graphic/mod.rs index e29046f33a..e22914587d 100644 --- a/node-graph/libraries/graphic-types/src/graphic/mod.rs +++ b/node-graph/libraries/graphic-types/src/graphic/mod.rs @@ -31,8 +31,11 @@ pub use vector_types::Vector; /// A leaf holds its element directly; its attributes ride the containing /// lane. Multi-element content is a [`core_types::record::Group`] run, or /// transitionally the legacy `Graphic` list. -#[derive(Clone, Debug, CacheHash, PartialEq, DynAny)] +#[derive(Clone, Debug, Default, CacheHash, PartialEq, DynAny)] pub enum Graphic<'e> { + /// The absence of graphical content, like CSS's `none` keyword: painting it produces nothing. + #[default] + None, Graphic(List>), Vector(Vector), RasterCPU(Raster), @@ -43,12 +46,6 @@ pub enum Graphic<'e> { Group(core_types::record::Group<'e>), } -impl Default for Graphic<'_> { - fn default() -> Self { - Self::Graphic(List::new()) - } -} - /// A typed legacy list as a legacy graphic list: each item de-tables to a /// leaf element, keeping its attributes on the containing lane. pub(in crate::graphic) fn detable_items<'e, T: Clone + Send + Sync + 'static>(list: List, leaf: fn(T) -> Graphic<'e>) -> List> { @@ -382,6 +379,7 @@ impl<'e> Graphic<'e> { } match self { + Graphic::None => true, Graphic::Graphic(list) => all_clipped(list), Graphic::Group(group) => group_all_clipped(group), _ => false, @@ -397,6 +395,7 @@ impl<'e> Graphic<'e> { pub fn is_opaque(&self) -> bool { match self { + Graphic::None => false, Graphic::Graphic(list) => !list.is_empty() && list.iter_element_values().all(Graphic::is_opaque), // A bare leaf carries no paint attribute, which rides its lane, so // nothing here claims opacity. @@ -410,6 +409,7 @@ impl<'e> Graphic<'e> { pub fn is_fully_transparent(&self) -> bool { match self { + Graphic::None => true, Graphic::Graphic(list) => list.iter_element_values().all(Graphic::is_fully_transparent), // A bare leaf carries no paint attribute, so only an unstroked // vector is invisible on its own. @@ -430,6 +430,7 @@ impl<'e> Graphic<'e> { /// Whether the graphic holds no content: a leaf always holds its element. pub fn is_empty(&self) -> bool { match self { + Graphic::None => true, Graphic::Graphic(list) => list.is_empty(), Graphic::Group(group) => group_is_empty(group), _ => false, @@ -440,6 +441,7 @@ impl<'e> Graphic<'e> { impl BoundingBox for Graphic<'_> { fn bounding_box(&self, transform: DAffine2, include_stroke: bool) -> RenderBoundingBox { match self { + Graphic::None => RenderBoundingBox::None, Graphic::Vector(vector) => BoundingBox::bounding_box(vector, transform, include_stroke), Graphic::RasterCPU(raster) => raster.bounding_box(transform, include_stroke), Graphic::RasterGPU(raster) => raster.bounding_box(transform, include_stroke), @@ -453,6 +455,7 @@ impl BoundingBox for Graphic<'_> { fn thumbnail_bounding_box(&self, transform: DAffine2, include_stroke: bool) -> RenderBoundingBox { match self { + Graphic::None => RenderBoundingBox::None, Graphic::Vector(vector) => vector.thumbnail_bounding_box(transform, include_stroke), Graphic::RasterCPU(raster) => raster.thumbnail_bounding_box(transform, include_stroke), Graphic::RasterGPU(raster) => raster.thumbnail_bounding_box(transform, include_stroke), @@ -484,6 +487,7 @@ impl<'e> ListConvert> for Raster { impl RenderComplexity for Graphic<'_> { fn render_complexity(&self) -> usize { match self { + Self::None => 0, Self::Graphic(list) => list.render_complexity(), Self::Vector(list) => list.render_complexity(), Self::RasterCPU(list) => list.render_complexity(), diff --git a/node-graph/libraries/rendering/src/render_ext.rs b/node-graph/libraries/rendering/src/render_ext.rs index ca50b373c9..62caad4d3c 100644 --- a/node-graph/libraries/rendering/src/render_ext.rs +++ b/node-graph/libraries/rendering/src/render_ext.rs @@ -256,6 +256,7 @@ impl RenderExt for List> { let gradient_id = render_gradient_paint(&core_types::lane::LeafLane::new(self, 0, gradient), svg_defs, item_transform, element_transform); format!(r##" {paint_attr}="url(#{gradient_id})""##) } + Some(Graphic::None) => format!(r#" {paint_attr}="none""#), Some(Graphic::Vector(_)) | Some(Graphic::RasterCPU(_)) | Some(Graphic::RasterGPU(_)) | Some(Graphic::Graphic(_)) | Some(Graphic::Text(_)) | Some(Graphic::Group(_)) => { let bounds = if target == PaintTarget::Stroke { // To prevent a wraparound artefact occurring when the tile boundary and the stroke region are perfectly aligned, the local coordinate is expanded slightly. diff --git a/node-graph/libraries/rendering/src/renderer.rs b/node-graph/libraries/rendering/src/renderer.rs index 7859397485..dafd83abe6 100644 --- a/node-graph/libraries/rendering/src/renderer.rs +++ b/node-graph/libraries/rendering/src/renderer.rs @@ -552,6 +552,7 @@ pub trait Render: BoundingBox + RenderComplexity { impl Render for Graphic<'_> { fn render_svg(&self, render: &mut SvgRender, render_params: &RenderParams) { match self { + Graphic::None => (), Graphic::Graphic(list) => list.render_svg(render, render_params), Graphic::Vector(vector) => render_vector_svg(&Single(vector), render, render_params), Graphic::RasterCPU(raster) => render_raster_cpu_svg(&Single(raster), render, render_params), @@ -565,6 +566,7 @@ impl Render for Graphic<'_> { fn render_to_vello(&self, scene: &mut Scene, transform: DAffine2, context: &mut RenderContext, render_params: &RenderParams) { match self { + Graphic::None => (), Graphic::Graphic(list) => list.render_to_vello(scene, transform, context, render_params), Graphic::Vector(vector) => render_vector_vello(&Single(vector), scene, transform, context, render_params), Graphic::RasterCPU(raster) => render_raster_cpu_vello(&Single(raster), scene, transform, render_params), @@ -590,6 +592,7 @@ impl Render for Graphic<'_> { fn contains_artboard(&self) -> bool { match self { + Graphic::None => false, Graphic::Graphic(list) => list.contains_artboard(), _ => false, } @@ -597,6 +600,7 @@ impl Render for Graphic<'_> { fn new_ids_from_hash(&mut self, reference: Option) { match self { + Graphic::None => (), Graphic::Graphic(list) => list.new_ids_from_hash(reference), Graphic::Vector(vector) => vector.vector_new_ids_from_hash(reference.map(|id| id.0).unwrap_or_default()), _ => (), @@ -660,6 +664,7 @@ fn collect_element_metadata<'a>( } match element { + Graphic::None => {} Graphic::Graphic(list) => collect_graphic_metadata_with(list, reach.nested(), metadata, footprint, element_id), Graphic::Vector(vector) if reach.applies() => collect_vector_metadata(&PaintOverlay::new(&Single(vector), reach.paint), metadata, footprint, element_id), Graphic::Vector(vector) => collect_vector_metadata(&Single(vector), metadata, footprint, element_id), @@ -703,6 +708,7 @@ fn collect_group_row_metadata(group: &Group, metadata: &mut RenderMetadata, elem fn add_element_upstream_click_targets<'a>(element: &'a Graphic, reach: PaintReach<'a>, click_targets: &mut Vec) { match element { + Graphic::None => (), Graphic::Graphic(list) => add_graphic_upstream_click_targets_with(list, reach.nested(), click_targets), Graphic::Vector(vector) if reach.applies() => add_vector_upstream_click_targets(&PaintOverlay::new(&Single(vector), reach.paint), click_targets), Graphic::Vector(vector) => add_vector_upstream_click_targets(&Single(vector), click_targets), @@ -715,6 +721,7 @@ fn add_element_upstream_click_targets<'a>(element: &'a Graphic, reach: PaintReac fn add_element_upstream_outline_targets<'a>(element: &'a Graphic, reach: PaintReach<'a>, outlines: &mut Vec) { match element { + Graphic::None => (), Graphic::Graphic(list) => add_graphic_upstream_outline_targets_with(list, reach.nested(), outlines), Graphic::Vector(vector) if reach.applies() => add_vector_upstream_outline_targets(&PaintOverlay::new(&Single(vector), reach.paint), outlines), Graphic::Vector(vector) => add_vector_upstream_outline_targets(&Single(vector), outlines), @@ -1563,6 +1570,7 @@ fn render_vector_vello>(source: &S, scene: &mut for paint_index in 0..fill_graphic.len() { let Some(paint) = fill_graphic.element(paint_index) else { continue }; match paint { + Graphic::None => continue, Graphic::Color(color) => { let fill = peniko::Brush::Solid(SRGBA8::from(*color).to_peniko_color()); scene.fill(fill_rule, kurbo::Affine::new(element_transform.to_cols_array()), &fill, None, path); @@ -1643,6 +1651,7 @@ fn render_vector_vello>(source: &S, scene: &mut }; match stroke_graphic { + Graphic::None => continue, Graphic::Color(color) => { let brush = peniko::Brush::Solid(SRGBA8::from(*color).to_peniko_color()); diff --git a/node-graph/nodes/path-bool/src/lib.rs b/node-graph/nodes/path-bool/src/lib.rs index 13e79c9a6a..adaf08b2af 100644 --- a/node-graph/nodes/path-bool/src/lib.rs +++ b/node-graph/nodes/path-bool/src/lib.rs @@ -376,6 +376,7 @@ fn flatten_vector_run_into<'a>(out: &mut List, level: GraphicLevel<'a>, let reach = inherited.for_lane(&columns, index); let composed = transform * level.attr::(index); match element { + Graphic::None => continue, Graphic::Vector(vector) => push_leaf_vector_row(out, level, index, vector, transform, reach), Graphic::Graphic(children) => push_union(out, flatten_vector_run(GraphicLevel::Legacy(children), composed, reach.nested())), Graphic::Group(group) => flatten_group(out, group, composed, reach),