From 357cb32c9d3846f859f91d941a20afee79762074 Mon Sep 17 00:00:00 2001 From: JustJ01 Date: Sat, 14 Feb 2026 01:03:55 +0530 Subject: [PATCH 1/3] Fix hidden node flattening to correctly forward typed inputs instead of injecting () --- .../document_node_derive.rs | 2 +- .../node_graph/node_graph_message_handler.rs | 4 +- .../utility_types/network_interface.rs | 45 +++++++-- .../network_interface/resolved_types.rs | 2 +- node-graph/README.md | 2 +- node-graph/graph-craft/src/document.rs | 98 ++++++++++++++----- node-graph/preprocessor/src/lib.rs | 8 +- 7 files changed, 122 insertions(+), 39 deletions(-) diff --git a/editor/src/messages/portfolio/document/node_graph/document_node_definitions/document_node_derive.rs b/editor/src/messages/portfolio/document/node_graph/document_node_definitions/document_node_derive.rs index 72a4558a67..fc766c26ce 100644 --- a/editor/src/messages/portfolio/document/node_graph/document_node_definitions/document_node_derive.rs +++ b/editor/src/messages/portfolio/document/node_graph/document_node_definitions/document_node_derive.rs @@ -55,7 +55,7 @@ pub(super) fn post_process_nodes(custom: Vec) -> HashMap inputs, call_argument: input_type.clone(), implementation: DocumentNodeImplementation::ProtoNode(id.clone()), - visible: true, + visible: Visible::Passthrough, skip_deduplication: false, context_features: ContextDependencies::from(context_features.as_slice()), ..Default::default() diff --git a/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs b/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs index 98e3cae101..a8f886936a 100644 --- a/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs +++ b/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs @@ -2585,7 +2585,7 @@ impl NodeGraphMessageHandler { return Vec::new(); }; let mut nodes = Vec::new(); - for (node_id, visible) in network.nodes.iter().map(|(node_id, node)| (*node_id, node.visible)).collect::>() { + for (node_id, visible) in network.nodes.iter().map(|(node_id, node)| (*node_id, node.visible.is_visible())).collect::>() { let primary_input_connector = InputConnector::node(node_id, 0); let primary_input = if network_interface @@ -2735,7 +2735,7 @@ impl NodeGraphMessageHandler { let parents_visible = layer.ancestors(network_interface.document_metadata()).filter(|&ancestor| ancestor != layer).all(|layer| { if layer != LayerNodeIdentifier::ROOT_PARENT { - network_interface.document_node(&layer.to_node(), &[]).map(|node| node.visible).unwrap_or_default() + network_interface.document_node(&layer.to_node(), &[]).map(|node| node.visible.is_visible()).unwrap_or_default() } else { true } diff --git a/editor/src/messages/portfolio/document/utility_types/network_interface.rs b/editor/src/messages/portfolio/document/utility_types/network_interface.rs index cf6e487bc4..e5e08fa0c2 100644 --- a/editor/src/messages/portfolio/document/utility_types/network_interface.rs +++ b/editor/src/messages/portfolio/document/utility_types/network_interface.rs @@ -11,12 +11,15 @@ use crate::messages::portfolio::document::node_graph::document_node_definitions: use crate::messages::portfolio::document::node_graph::utility_types::{Direction, FrontendClickTargets, FrontendGraphDataType, FrontendGraphInput, FrontendGraphOutput}; use crate::messages::portfolio::document::overlays::utility_functions::text_width; use crate::messages::portfolio::document::utility_types::network_interface::resolved_types::ResolvedDocumentNodeTypes; +use crate::messages::portfolio::document::utility_types::network_interface::resolved_types::TypeSource; use crate::messages::portfolio::document::utility_types::wires::{GraphWireStyle, WirePath, WirePathUpdate, build_vector_wire}; use crate::messages::tool::common_functionality::graph_modification_utils; use crate::messages::tool::tool_messages::tool_prelude::NumberInputMode; use deserialization::deserialize_node_persistent_metadata; use glam::{DAffine2, DVec2, IVec2}; use graph_craft::Type; +use graph_craft::concrete; +use graph_craft::document::Visible; use graph_craft::document::value::TaggedValue; use graph_craft::document::{DocumentNode, DocumentNodeImplementation, NodeId, NodeInput, NodeNetwork, OldDocumentNodeImplementation, OldNodeNetwork}; use graphene_std::ContextDependencies; @@ -1049,7 +1052,7 @@ impl NodeNetworkInterface { log::error!("Could not get node in is_visible"); return false; }; - node.visible + node.visible.is_visible() } pub fn is_layer(&self, node_id: &NodeId, network_path: &[NodeId]) -> bool { @@ -1358,7 +1361,7 @@ impl NodeNetworkInterface { node.inputs = old_node.inputs; node.call_argument = old_node.manual_composition.unwrap(); - node.visible = old_node.visible; + node.visible = if old_node.visible { Visible::Passthrough } else { Visible::Value(node.call_argument.clone()) }; node.skip_deduplication = old_node.skip_deduplication; node.original_location = old_node.original_location; node_metadata.persistent_metadata.display_name = old_node.alias; @@ -4480,15 +4483,41 @@ impl NodeNetworkInterface { } pub fn set_visibility(&mut self, node_id: &NodeId, network_path: &[NodeId], is_visible: bool) { - let Some(network) = self.network_mut(network_path) else { - return; - }; - let Some(node) = network.nodes.get_mut(node_id) else { - log::error!("Could not get node {node_id} in set_visibility"); + if is_visible { + if let Some(network) = self.network_mut(network_path) { + if let Some(node) = network.nodes.get_mut(node_id) { + node.visible = Visible::Passthrough; + } + } + self.transaction_modified(); return; + } + + // Compute output_type BEFORE borrowing network mutably + let output_type = { + let output_connector = OutputConnector::Node { node_id: *node_id, output_index: 0 }; + + match self.output_type(&output_connector, network_path) { + TypeSource::Compiled(ty) => ty, + TypeSource::TaggedValue(ty) => ty, + TypeSource::Unknown | TypeSource::Invalid => { + log::error!("Output type unknown when hiding node"); + concrete!(()) // temporary fallback, but should rarely happen + } + TypeSource::Error(e) => { + log::error!("Error retrieving output type in set_visibility: {e}"); + concrete!(()) + } + } }; - node.visible = is_visible; + // Now borrow network mutably AFTER output_type is computed + if let Some(network) = self.network_mut(network_path) { + if let Some(node) = network.nodes.get_mut(node_id) { + node.visible = Visible::Value(output_type); + } + } + self.transaction_modified(); } diff --git a/editor/src/messages/portfolio/document/utility_types/network_interface/resolved_types.rs b/editor/src/messages/portfolio/document/utility_types/network_interface/resolved_types.rs index c24f73ea04..b630467424 100644 --- a/editor/src/messages/portfolio/document/utility_types/network_interface/resolved_types.rs +++ b/editor/src/messages/portfolio/document/utility_types/network_interface/resolved_types.rs @@ -203,7 +203,7 @@ impl NodeNetworkInterface { concrete!(()) } }; - TaggedValue::from_type_or_none(&guaranteed_type) + TaggedValue::from_type(&guaranteed_type).expect("Failed to construct TaggedValue for identity type") } /// A list of all valid input types for this specific node. diff --git a/node-graph/README.md b/node-graph/README.md index cf2c1e9bc1..2d5ad6115e 100644 --- a/node-graph/README.md +++ b/node-graph/README.md @@ -14,7 +14,7 @@ pub struct DocumentNode { pub call_argument: Type, pub implementation: DocumentNodeImplementation, pub skip_deduplication: bool, - pub visible: bool, + pub visible: Visible, pub original_location: OriginalLocation, } ``` diff --git a/node-graph/graph-craft/src/document.rs b/node-graph/graph-craft/src/document.rs index b54d56780d..aa5d915962 100644 --- a/node-graph/graph-craft/src/document.rs +++ b/node-graph/graph-craft/src/document.rs @@ -29,6 +29,25 @@ fn return_true() -> bool { true } +#[derive(Clone, Debug, PartialEq, Hash, serde::Serialize, serde::Deserialize)] +pub enum Visible { + Passthrough, + Value(Type), +} +impl Visible { + pub fn is_visible(&self) -> bool { + matches!(self, Visible::Passthrough) + } + + pub fn is_hidden(&self) -> bool { + matches!(self, Visible::Value(_)) + } +} + +fn return_visible_true() -> Visible { + Visible::Passthrough +} + /// An instance of a [`DocumentNodeDefinition`] that has been instantiated in a [`NodeNetwork`]. /// Currently, when an instance is made, it lives all on its own without any lasting connection to the definition. /// But we will want to change it in the future so it merely references its definition. @@ -50,8 +69,8 @@ pub struct DocumentNode { // A nested document network or a proto-node identifier. pub implementation: DocumentNodeImplementation, /// Represents the eye icon for hiding/showing the node in the graph UI. When hidden, a node gets replaced with an identity node during the graph flattening step. - #[serde(default = "return_true")] - pub visible: bool, + #[serde(default = "return_visible_true")] + pub visible: Visible, /// When two different proto nodes hash to the same value (e.g. two value nodes each containing `2_u32` or two multiply nodes that have the same node IDs as input), the duplicates are removed. /// See [`ProtoNetwork::generate_stable_node_ids`] for details. /// However sometimes this is not desirable, for example in the case of a [`graphene_core::memo::MonitorNode`] that needs to be accessed outside of the graph. @@ -94,7 +113,7 @@ impl Default for DocumentNode { inputs: Default::default(), call_argument: concrete!(Context), implementation: Default::default(), - visible: true, + visible: Visible::Passthrough, skip_deduplication: Default::default(), original_location: OriginalLocation::default(), context_features: Default::default(), @@ -785,16 +804,37 @@ impl NodeNetwork { return; }; - // If the node is hidden, replace it with an identity node - let identity_node = DocumentNodeImplementation::ProtoNode(graphene_core::ops::identity::IDENTIFIER); - if !node.visible && node.implementation != identity_node { - node.implementation = identity_node; + match node.visible.clone() { + Visible::Passthrough => {} - // Connect layer node to the group below - node.inputs.drain(1..); - node.call_argument = concrete!(()); - self.nodes.insert(id, node); - return; + Visible::Value(output_type) => { + let matching_input = node + .inputs + .iter() + .find_map(|input| match input { + NodeInput::Import { import_type, .. } if *import_type == output_type => Some(input.clone()), + NodeInput::Value { tagged_value, .. } if tagged_value.ty() == output_type => Some(input.clone()), + _ => None, + }) + .or_else(|| node.inputs.iter().find(|input| matches!(input, NodeInput::Node { .. })).cloned()); + + if let Some(primary) = matching_input { + node.implementation = DocumentNodeImplementation::ProtoNode(graphene_core::ops::identity::IDENTIFIER); + node.call_argument = concrete!(()); + node.inputs.clear(); + node.inputs.push(primary); + } else { + let tagged = TaggedValue::from_type(&output_type).unwrap_or_else(|| TaggedValue::None); + + node.implementation = DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("core_types::value::ClonedNode")); + node.call_argument = concrete!(()); + node.inputs.clear(); + node.inputs.push(NodeInput::value(tagged, false)); + } + + self.nodes.insert(id, node); + return; + } } let path = node.original_location.path.clone().unwrap_or_default(); @@ -878,21 +918,35 @@ impl NodeNetwork { // Connect all nodes that were previously connected to this node to the nodes of the inner network for (i, export) in inner_network.exports.into_iter().enumerate() { - if let NodeInput::Node { node_id, output_index, .. } = &export { - for deps in &node.original_location.dependants { - for dep in deps { - self.replace_node_inputs(*dep, (id, i), (*node_id, *output_index)); + match export { + NodeInput::Node { node_id, output_index } => { + for deps in &node.original_location.dependants { + for dep in deps { + self.replace_node_inputs(*dep, (id, i), (node_id, output_index)); + } } - } - if let Some(new_output_node) = self.nodes.get_mut(node_id) { - for dep in &node.original_location.dependants[i] { - new_output_node.original_location.dependants[*output_index].push(*dep); + if let Some(new_output_node) = self.nodes.get_mut(&node_id) { + for dep in &node.original_location.dependants[i] { + new_output_node.original_location.dependants[output_index].push(*dep); + } } + + self.replace_network_outputs(NodeInput::node(id, i), NodeInput::node(node_id, output_index)); + } + + NodeInput::Import { import_index, .. } => { + let parent_input = node.inputs.get(import_index).expect("Import index must exist on parent"); + + self.replace_network_outputs(NodeInput::node(id, i), parent_input.clone()); + } + + NodeInput::Value { .. } => { + self.replace_network_outputs(NodeInput::node(id, i), export); } - } - self.replace_network_outputs(NodeInput::node(id, i), export); + _ => {} + } } for node_id in new_nodes { diff --git a/node-graph/preprocessor/src/lib.rs b/node-graph/preprocessor/src/lib.rs index 0f9bf398e4..4ddb0397e4 100644 --- a/node-graph/preprocessor/src/lib.rs +++ b/node-graph/preprocessor/src/lib.rs @@ -87,7 +87,7 @@ pub fn generate_node_substitutions() -> HashMap HashMap DocumentNode { inputs: vec![NodeInput::import(generic!(X), i)], implementation: DocumentNodeImplementation::ProtoNode(identity_node.clone()), - visible: false, + visible: Visible::Passthrough, ..Default::default() }, }, @@ -111,7 +111,7 @@ pub fn generate_node_substitutions() -> HashMap HashMap Date: Sat, 14 Feb 2026 14:32:18 +0530 Subject: [PATCH 2/3] Use tagged_value_from_input for hidden node handling to preserve types --- .../utility_types/network_interface.rs | 28 +++++---- node-graph/graph-craft/src/document.rs | 58 ++++++++++++++++++- 2 files changed, 74 insertions(+), 12 deletions(-) diff --git a/editor/src/messages/portfolio/document/utility_types/network_interface.rs b/editor/src/messages/portfolio/document/utility_types/network_interface.rs index e5e08fa0c2..ad3e7f2a87 100644 --- a/editor/src/messages/portfolio/document/utility_types/network_interface.rs +++ b/editor/src/messages/portfolio/document/utility_types/network_interface.rs @@ -19,9 +19,9 @@ use deserialization::deserialize_node_persistent_metadata; use glam::{DAffine2, DVec2, IVec2}; use graph_craft::Type; use graph_craft::concrete; -use graph_craft::document::Visible; use graph_craft::document::value::TaggedValue; use graph_craft::document::{DocumentNode, DocumentNodeImplementation, NodeId, NodeInput, NodeNetwork, OldDocumentNodeImplementation, OldNodeNetwork}; +use graph_craft::document::{HiddenNodeInput, Visible}; use graphene_std::ContextDependencies; use graphene_std::math::quad::Quad; use graphene_std::subpath::Subpath; @@ -1361,7 +1361,7 @@ impl NodeNetworkInterface { node.inputs = old_node.inputs; node.call_argument = old_node.manual_composition.unwrap(); - node.visible = if old_node.visible { Visible::Passthrough } else { Visible::Value(node.call_argument.clone()) }; + node.visible = if old_node.visible { Visible::Passthrough } else { Visible::TaggedValues(Vec::new()) }; node.skip_deduplication = old_node.skip_deduplication; node.original_location = old_node.original_location; node_metadata.persistent_metadata.display_name = old_node.alias; @@ -4493,28 +4493,36 @@ impl NodeNetworkInterface { return; } - // Compute output_type BEFORE borrowing network mutably - let output_type = { + let input_count = self.document_node(node_id, network_path).map_or(0, |node| node.inputs.len()); + let tagged_inputs: Vec = (0..input_count) + .map(|input_index| HiddenNodeInput { + input_index, + tagged_value: self.tagged_value_from_input(&InputConnector::node(*node_id, input_index), network_path), + }) + .collect(); + let has_non_unit_input = tagged_inputs.iter().any(|hidden_input: &HiddenNodeInput| hidden_input.tagged_value.ty() != concrete!(())); + let visibility = if has_non_unit_input { + Visible::TaggedValues(tagged_inputs) + } else { let output_connector = OutputConnector::Node { node_id: *node_id, output_index: 0 }; - - match self.output_type(&output_connector, network_path) { + let output_type = match self.output_type(&output_connector, network_path) { TypeSource::Compiled(ty) => ty, TypeSource::TaggedValue(ty) => ty, TypeSource::Unknown | TypeSource::Invalid => { log::error!("Output type unknown when hiding node"); - concrete!(()) // temporary fallback, but should rarely happen + concrete!(()) } TypeSource::Error(e) => { log::error!("Error retrieving output type in set_visibility: {e}"); concrete!(()) } - } + }; + Visible::Value(output_type) }; - // Now borrow network mutably AFTER output_type is computed if let Some(network) = self.network_mut(network_path) { if let Some(node) = network.nodes.get_mut(node_id) { - node.visible = Visible::Value(output_type); + node.visible = visibility; } } diff --git a/node-graph/graph-craft/src/document.rs b/node-graph/graph-craft/src/document.rs index aa5d915962..5f19c4f8f8 100644 --- a/node-graph/graph-craft/src/document.rs +++ b/node-graph/graph-craft/src/document.rs @@ -29,10 +29,17 @@ fn return_true() -> bool { true } +#[derive(Clone, Debug, PartialEq, Hash, serde::Serialize, serde::Deserialize)] +pub struct HiddenNodeInput { + pub input_index: usize, + pub tagged_value: TaggedValue, +} + #[derive(Clone, Debug, PartialEq, Hash, serde::Serialize, serde::Deserialize)] pub enum Visible { Passthrough, - Value(Type), + Value(Type), //kept for backward compatibility + TaggedValues(Vec), } impl Visible { pub fn is_visible(&self) -> bool { @@ -40,7 +47,7 @@ impl Visible { } pub fn is_hidden(&self) -> bool { - matches!(self, Visible::Value(_)) + !self.is_visible() } } @@ -835,6 +842,53 @@ impl NodeNetwork { self.nodes.insert(id, node); return; } + + Visible::TaggedValues(tagged_values) => { + let passthrough_input = tagged_values + .iter() + .filter(|hidden_input| hidden_input.tagged_value.ty() != concrete!(())) + .find_map(|hidden_input| { + let input = node.inputs.get(hidden_input.input_index)?; + match input { + NodeInput::Node { .. } => Some(input.clone()), + NodeInput::Import { import_type, .. } if *import_type == hidden_input.tagged_value.ty() => Some(input.clone()), + NodeInput::Value { tagged_value, .. } if tagged_value.ty() == hidden_input.tagged_value.ty() => Some(input.clone()), + NodeInput::Import { .. } | NodeInput::Value { .. } | NodeInput::Scope(_) | NodeInput::Reflection(_) | NodeInput::Inline(_) => None, + } + }) + .or_else(|| { + tagged_values.iter().find_map(|hidden_input| { + let input = node.inputs.get(hidden_input.input_index)?; + match input { + NodeInput::Node { .. } | NodeInput::Import { .. } | NodeInput::Value { .. } => Some(input.clone()), + NodeInput::Scope(_) | NodeInput::Reflection(_) | NodeInput::Inline(_) => None, + } + }) + }); + + if let Some(primary) = passthrough_input { + node.implementation = DocumentNodeImplementation::ProtoNode(graphene_core::ops::identity::IDENTIFIER); + node.call_argument = concrete!(()); + node.inputs.clear(); + node.inputs.push(primary); + } else { + let tagged = tagged_values + .iter() + .find(|hidden_input| hidden_input.tagged_value.ty() != concrete!(())) + .or_else(|| tagged_values.first()) + .map(|hidden_input| hidden_input.tagged_value.clone()) + .or_else(|| TaggedValue::from_type(&node.call_argument)) + .unwrap_or(TaggedValue::None); + + node.implementation = DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("core_types::value::ClonedNode")); + node.call_argument = concrete!(()); + node.inputs.clear(); + node.inputs.push(NodeInput::value(tagged, false)); + } + + self.nodes.insert(id, node); + return; + } } let path = node.original_location.path.clone().unwrap_or_default(); From 7f9154288d03f09a5acc260690628140724082ea Mon Sep 17 00:00:00 2001 From: JustJ01 Date: Sun, 15 Feb 2026 18:39:27 +0530 Subject: [PATCH 3/3] made some changes --- .../document_node_derive.rs | 2 +- .../node_graph/node_graph_message_handler.rs | 4 +- .../utility_types/network_interface.rs | 147 ++++++++++++++---- .../network_interface/resolved_types.rs | 45 ++++-- node-graph/README.md | 2 +- node-graph/graph-craft/src/document.rs | 120 +++++--------- node-graph/preprocessor/src/lib.rs | 8 +- 7 files changed, 190 insertions(+), 138 deletions(-) diff --git a/editor/src/messages/portfolio/document/node_graph/document_node_definitions/document_node_derive.rs b/editor/src/messages/portfolio/document/node_graph/document_node_definitions/document_node_derive.rs index fc766c26ce..c9fe1d5eda 100644 --- a/editor/src/messages/portfolio/document/node_graph/document_node_definitions/document_node_derive.rs +++ b/editor/src/messages/portfolio/document/node_graph/document_node_definitions/document_node_derive.rs @@ -55,7 +55,7 @@ pub(super) fn post_process_nodes(custom: Vec) -> HashMap inputs, call_argument: input_type.clone(), implementation: DocumentNodeImplementation::ProtoNode(id.clone()), - visible: Visible::Passthrough, + visible: None, skip_deduplication: false, context_features: ContextDependencies::from(context_features.as_slice()), ..Default::default() diff --git a/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs b/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs index 1ebc58c59c..3d26dd053b 100644 --- a/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs +++ b/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs @@ -2571,7 +2571,7 @@ impl NodeGraphMessageHandler { return Vec::new(); }; let mut nodes = Vec::new(); - for (node_id, visible) in network.nodes.iter().map(|(node_id, node)| (*node_id, node.visible.is_visible())).collect::>() { + for (node_id, visible) in network.nodes.iter().map(|(node_id, node)| (*node_id, node.visible.is_none())).collect::>() { let primary_input_connector = InputConnector::node(node_id, 0); let primary_input = if network_interface @@ -2721,7 +2721,7 @@ impl NodeGraphMessageHandler { let parents_visible = layer.ancestors(network_interface.document_metadata()).filter(|&ancestor| ancestor != layer).all(|layer| { if layer != LayerNodeIdentifier::ROOT_PARENT { - network_interface.document_node(&layer.to_node(), &[]).map(|node| node.visible.is_visible()).unwrap_or_default() + network_interface.document_node(&layer.to_node(), &[]).map(|node| node.visible.is_none()).unwrap_or_default() } else { true } diff --git a/editor/src/messages/portfolio/document/utility_types/network_interface.rs b/editor/src/messages/portfolio/document/utility_types/network_interface.rs index 95f8392b0d..e69cf52732 100644 --- a/editor/src/messages/portfolio/document/utility_types/network_interface.rs +++ b/editor/src/messages/portfolio/document/utility_types/network_interface.rs @@ -10,18 +10,16 @@ use crate::messages::portfolio::document::graph_operation::utility_types::Modify use crate::messages::portfolio::document::node_graph::document_node_definitions::{DefinitionIdentifier, resolve_document_node_type}; use crate::messages::portfolio::document::node_graph::utility_types::{Direction, FrontendClickTargets, FrontendGraphDataType, FrontendGraphInput, FrontendGraphOutput}; use crate::messages::portfolio::document::overlays::utility_functions::text_width; -use crate::messages::portfolio::document::utility_types::network_interface::resolved_types::ResolvedDocumentNodeTypes; -use crate::messages::portfolio::document::utility_types::network_interface::resolved_types::TypeSource; +use crate::messages::portfolio::document::utility_types::network_interface::resolved_types::{ResolvedDocumentNodeTypes, TypeSource}; use crate::messages::portfolio::document::utility_types::wires::{GraphWireStyle, WirePath, WirePathUpdate, build_vector_wire}; use crate::messages::tool::common_functionality::graph_modification_utils; use crate::messages::tool::tool_messages::tool_prelude::NumberInputMode; use deserialization::deserialize_node_persistent_metadata; use glam::{DAffine2, DVec2, IVec2}; use graph_craft::Type; -use graph_craft::concrete; use graph_craft::document::value::TaggedValue; use graph_craft::document::{DocumentNode, DocumentNodeImplementation, NodeId, NodeInput, NodeNetwork, OldDocumentNodeImplementation, OldNodeNetwork}; -use graph_craft::document::{HiddenNodeInput, Visible}; +use graph_craft::document::{Hidden, HiddenNodeInput}; use graphene_std::ContextDependencies; use graphene_std::math::quad::Quad; use graphene_std::subpath::Subpath; @@ -1052,7 +1050,7 @@ impl NodeNetworkInterface { log::error!("Could not get node in is_visible"); return false; }; - node.visible.is_visible() + node.visible.is_none() } pub fn is_layer(&self, node_id: &NodeId, network_path: &[NodeId]) -> bool { @@ -1361,7 +1359,7 @@ impl NodeNetworkInterface { node.inputs = old_node.inputs; node.call_argument = old_node.manual_composition.unwrap(); - node.visible = if old_node.visible { Visible::Passthrough } else { Visible::TaggedValues(Vec::new()) }; + node.visible = if old_node.visible { None } else { Some(Hidden::Passthrough) }; node.skip_deduplication = old_node.skip_deduplication; node.original_location = old_node.original_location; node_metadata.persistent_metadata.display_name = old_node.alias; @@ -4487,7 +4485,7 @@ impl NodeNetworkInterface { if is_visible { if let Some(network) = self.network_mut(network_path) { if let Some(node) = network.nodes.get_mut(node_id) { - node.visible = Visible::Passthrough; + node.visible = None; } } self.transaction_modified(); @@ -4495,30 +4493,58 @@ impl NodeNetworkInterface { } let input_count = self.document_node(node_id, network_path).map_or(0, |node| node.inputs.len()); - let tagged_inputs: Vec = (0..input_count) - .map(|input_index| HiddenNodeInput { - input_index, - tagged_value: self.tagged_value_from_input(&InputConnector::node(*node_id, input_index), network_path), - }) - .collect(); - let has_non_unit_input = tagged_inputs.iter().any(|hidden_input: &HiddenNodeInput| hidden_input.tagged_value.ty() != concrete!(())); - let visibility = if has_non_unit_input { - Visible::TaggedValues(tagged_inputs) + + let visibility = if input_count > 0 { + // Nodes with inputs: use Passthrough (become identity node passing first input) + Some(Hidden::Passthrough) } else { + // Nodes with NO inputs - these generate values dynamically + // We need to check if they're connected to anything let output_connector = OutputConnector::Node { node_id: *node_id, output_index: 0 }; - let output_type = match self.output_type(&output_connector, network_path) { - TypeSource::Compiled(ty) => ty, - TypeSource::TaggedValue(ty) => ty, - TypeSource::Unknown | TypeSource::Invalid => { - log::error!("Output type unknown when hiding node"); - concrete!(()) - } - TypeSource::Error(e) => { - log::error!("Error retrieving output type in set_visibility: {e}"); - concrete!(()) + + // Get all downstream connections + let downstream_connectors = self + .outward_wires(network_path) + .and_then(|outward_wires| outward_wires.get(&output_connector)) + .cloned() + .unwrap_or_default(); + + if downstream_connectors.is_empty() { + // No downstream connections - safe to hide with a dummy value + // Use Passthrough which will make it output unit type + Some(Hidden::Passthrough) + } else { + // Has downstream connections - need to provide actual values + // For dynamic nodes, we try to infer from downstream what values they expect + let tagged_inputs: Vec = downstream_connectors + .into_iter() + .filter_map(|connector| { + let InputConnector::Node { node_id: downstream_node_id, input_index } = connector else { + return None; + }; + let input_connector = InputConnector::node(downstream_node_id, input_index); + let tagged_value = self.tagged_value_from_downstream_connector(&input_connector, network_path); + if matches!(tagged_value, TaggedValue::None) { + log::warn!("Could not infer hidden replacement value for node {downstream_node_id} input {input_index}"); + return None; + } + + Some(HiddenNodeInput { + node_id: downstream_node_id, + input_index, + tagged_value, + }) + }) + .collect(); + + if tagged_inputs.is_empty() { + // Couldn't infer any valid values - fall back to Passthrough + log::warn!("Cannot properly hide node {node_id} with no inputs - using Passthrough fallback"); + Some(Hidden::Passthrough) + } else { + Some(Hidden::TaggedValues(tagged_inputs)) } - }; - Visible::Value(output_type) + } }; if let Some(network) = self.network_mut(network_path) { @@ -4529,6 +4555,71 @@ impl NodeNetworkInterface { self.transaction_modified(); } + + fn tagged_value_from_downstream_connector(&mut self, input_connector: &InputConnector, network_path: &[NodeId]) -> TaggedValue { + let tagged_value = self.tagged_value_from_input(input_connector, network_path); + if !matches!(tagged_value, TaggedValue::None) { + return tagged_value; + } + + // If inference falls back to `None`, try extracting the concrete type directly from this connector. + if let TypeSource::Compiled(ty) | TypeSource::TaggedValue(ty) = self.input_type_not_invalid(input_connector, network_path) { + let typed = TaggedValue::from_type_or_none(&ty); + if !matches!(typed, TaggedValue::None) { + return typed; + } + } + + let InputConnector::Node { node_id, input_index } = *input_connector else { + return tagged_value; + }; + let Some(DocumentNodeImplementation::Network(_)) = self.implementation(&node_id, network_path) else { + // If this isn't a nested network input, try candidates directly on this connector. + let mut candidate_types = self.complete_valid_input_types(input_connector, network_path); + if candidate_types.is_empty() { + candidate_types = self.potential_valid_input_types(input_connector, network_path); + } + candidate_types.sort_by_key(|ty| ty.nested_type().identifier_name()); + + if let Some(candidate) = candidate_types.iter().find_map(|ty| { + let typed = TaggedValue::from_type_or_none(ty); + (!matches!(typed, TaggedValue::None)).then_some(typed) + }) { + return candidate; + } + + return tagged_value; + }; + + let nested_path = [network_path, &[node_id]].concat(); + let nested_downstream = self + .outward_wires(&nested_path) + .and_then(|outward_wires| outward_wires.get(&OutputConnector::Import(input_index))) + .cloned() + .unwrap_or_default(); + + for nested_input_connector in nested_downstream { + let nested_tagged_value = self.tagged_value_from_downstream_connector(&nested_input_connector, &nested_path); + if !matches!(nested_tagged_value, TaggedValue::None) { + return nested_tagged_value; + } + } + + // Nested traversal could not infer; try candidates at this connector as a final fallback. + let mut candidate_types = self.complete_valid_input_types(input_connector, network_path); + if candidate_types.is_empty() { + candidate_types = self.potential_valid_input_types(input_connector, network_path); + } + candidate_types.sort_by_key(|ty| ty.nested_type().identifier_name()); + + candidate_types + .iter() + .find_map(|ty| { + let typed = TaggedValue::from_type_or_none(ty); + (!matches!(typed, TaggedValue::None)).then_some(typed) + }) + .unwrap_or(tagged_value) + } pub fn set_locked(&mut self, node_id: &NodeId, network_path: &[NodeId], locked: bool) { let Some(node_metadata) = self.node_metadata_mut(node_id, network_path) else { diff --git a/editor/src/messages/portfolio/document/utility_types/network_interface/resolved_types.rs b/editor/src/messages/portfolio/document/utility_types/network_interface/resolved_types.rs index b630467424..8592c74031 100644 --- a/editor/src/messages/portfolio/document/utility_types/network_interface/resolved_types.rs +++ b/editor/src/messages/portfolio/document/utility_types/network_interface/resolved_types.rs @@ -182,28 +182,39 @@ impl NodeNetworkInterface { /// Gets the default tagged value for an input. If its not compiled, then it tries to get a valid type. If there are no valid types, then it picks a random implementation. pub fn tagged_value_from_input(&mut self, input_connector: &InputConnector, network_path: &[NodeId]) -> TaggedValue { - let guaranteed_type = match self.input_type(input_connector, network_path) { - TypeSource::Compiled(compiled) => compiled, - TypeSource::TaggedValue(value) => value, + match self.input_type(input_connector, network_path) { + TypeSource::Compiled(compiled) | TypeSource::TaggedValue(compiled) => { + let tagged_value = TaggedValue::from_type_or_none(&compiled); + if matches!(tagged_value, TaggedValue::None) { + log::error!("Failed to construct TaggedValue for {compiled:?} in tagged_value_from_input"); + } + tagged_value + } TypeSource::Unknown | TypeSource::Invalid => { - // Pick a random type from the complete valid types - // TODO: Add a NodeInput::Indeterminate which can be resolved at compile time to be any type that prevents an error. This may require bidirectional typing. - self.complete_valid_input_types(input_connector, network_path) - .into_iter() - .min_by_key(|ty| ty.nested_type().identifier_name()) - // Pick a random type from the potential valid types - .or_else(|| { - self.potential_valid_input_types(input_connector, network_path) - .into_iter() - .min_by_key(|ty| ty.nested_type().identifier_name()) - }).unwrap_or(concrete!(())) + // Try complete valid types first, then potential valid types + let mut candidate_types = self.complete_valid_input_types(input_connector, network_path); + if candidate_types.is_empty() { + candidate_types = self.potential_valid_input_types(input_connector, network_path); + } + candidate_types.sort_by_key(|ty| ty.nested_type().identifier_name()); + + // Try each candidate type in order until we find one that works + if let Some(tagged_value) = candidate_types.iter().find_map(|ty| { + let tagged = TaggedValue::from_type_or_none(ty); + (!matches!(tagged, TaggedValue::None)).then_some(tagged) + }) { + return tagged_value; + } + + let fallback_type = candidate_types.first().cloned().unwrap_or(concrete!(())); + log::error!("Failed to construct TaggedValue for any valid type on {input_connector:?}; fallback type: {fallback_type:?}"); + TaggedValue::from_type_or_none(&fallback_type) } TypeSource::Error(e) => { log::error!("Error getting tagged_value_from_input for {input_connector:?} {e}"); - concrete!(()) + TaggedValue::None } - }; - TaggedValue::from_type(&guaranteed_type).expect("Failed to construct TaggedValue for identity type") + } } /// A list of all valid input types for this specific node. diff --git a/node-graph/README.md b/node-graph/README.md index 2d5ad6115e..e0394f27d3 100644 --- a/node-graph/README.md +++ b/node-graph/README.md @@ -14,7 +14,7 @@ pub struct DocumentNode { pub call_argument: Type, pub implementation: DocumentNodeImplementation, pub skip_deduplication: bool, - pub visible: Visible, + pub visible: Option, pub original_location: OriginalLocation, } ``` diff --git a/node-graph/graph-craft/src/document.rs b/node-graph/graph-craft/src/document.rs index 5f19c4f8f8..241e0090f5 100644 --- a/node-graph/graph-craft/src/document.rs +++ b/node-graph/graph-craft/src/document.rs @@ -31,29 +31,16 @@ fn return_true() -> bool { #[derive(Clone, Debug, PartialEq, Hash, serde::Serialize, serde::Deserialize)] pub struct HiddenNodeInput { + pub node_id: NodeId, pub input_index: usize, pub tagged_value: TaggedValue, } #[derive(Clone, Debug, PartialEq, Hash, serde::Serialize, serde::Deserialize)] -pub enum Visible { +pub enum Hidden { Passthrough, - Value(Type), //kept for backward compatibility TaggedValues(Vec), } -impl Visible { - pub fn is_visible(&self) -> bool { - matches!(self, Visible::Passthrough) - } - - pub fn is_hidden(&self) -> bool { - !self.is_visible() - } -} - -fn return_visible_true() -> Visible { - Visible::Passthrough -} /// An instance of a [`DocumentNodeDefinition`] that has been instantiated in a [`NodeNetwork`]. /// Currently, when an instance is made, it lives all on its own without any lasting connection to the definition. @@ -76,8 +63,8 @@ pub struct DocumentNode { // A nested document network or a proto-node identifier. pub implementation: DocumentNodeImplementation, /// Represents the eye icon for hiding/showing the node in the graph UI. When hidden, a node gets replaced with an identity node during the graph flattening step. - #[serde(default = "return_visible_true")] - pub visible: Visible, + #[serde(default)] + pub visible: Option, /// When two different proto nodes hash to the same value (e.g. two value nodes each containing `2_u32` or two multiply nodes that have the same node IDs as input), the duplicates are removed. /// See [`ProtoNetwork::generate_stable_node_ids`] for details. /// However sometimes this is not desirable, for example in the case of a [`graphene_core::memo::MonitorNode`] that needs to be accessed outside of the graph. @@ -120,7 +107,7 @@ impl Default for DocumentNode { inputs: Default::default(), call_argument: concrete!(Context), implementation: Default::default(), - visible: Visible::Passthrough, + visible: None, skip_deduplication: Default::default(), original_location: OriginalLocation::default(), context_features: Default::default(), @@ -812,80 +799,43 @@ impl NodeNetwork { }; match node.visible.clone() { - Visible::Passthrough => {} - - Visible::Value(output_type) => { - let matching_input = node - .inputs - .iter() - .find_map(|input| match input { - NodeInput::Import { import_type, .. } if *import_type == output_type => Some(input.clone()), - NodeInput::Value { tagged_value, .. } if tagged_value.ty() == output_type => Some(input.clone()), - _ => None, - }) - .or_else(|| node.inputs.iter().find(|input| matches!(input, NodeInput::Node { .. })).cloned()); - - if let Some(primary) = matching_input { - node.implementation = DocumentNodeImplementation::ProtoNode(graphene_core::ops::identity::IDENTIFIER); - node.call_argument = concrete!(()); - node.inputs.clear(); - node.inputs.push(primary); - } else { - let tagged = TaggedValue::from_type(&output_type).unwrap_or_else(|| TaggedValue::None); - - node.implementation = DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("core_types::value::ClonedNode")); - node.call_argument = concrete!(()); - node.inputs.clear(); - node.inputs.push(NodeInput::value(tagged, false)); + None => {} + Some(Hidden::Passthrough) => { + node.implementation = DocumentNodeImplementation::ProtoNode(graphene_core::ops::identity::IDENTIFIER); + if node.inputs.len() > 1 { + node.inputs.drain(1..); } - + node.call_argument = concrete!(()); self.nodes.insert(id, node); return; } + Some(Hidden::TaggedValues(tagged_values)) => { + let mut replaced_any_downstream = false; + let replacement_tagged = tagged_values.first().map(|hidden_input| hidden_input.tagged_value.clone()).unwrap_or(TaggedValue::None); + + for hidden_input in &tagged_values { + let Some(downstream_node) = self.nodes.get_mut(&hidden_input.node_id) else { + continue; + }; + let Some(downstream_input) = downstream_node.inputs.get_mut(hidden_input.input_index) else { + continue; + }; + if matches!(downstream_input, NodeInput::Node { node_id, output_index } if *node_id == id && *output_index == 0) { + *downstream_input = NodeInput::value(hidden_input.tagged_value.clone(), false); + replaced_any_downstream = true; + } + } - Visible::TaggedValues(tagged_values) => { - let passthrough_input = tagged_values - .iter() - .filter(|hidden_input| hidden_input.tagged_value.ty() != concrete!(())) - .find_map(|hidden_input| { - let input = node.inputs.get(hidden_input.input_index)?; - match input { - NodeInput::Node { .. } => Some(input.clone()), - NodeInput::Import { import_type, .. } if *import_type == hidden_input.tagged_value.ty() => Some(input.clone()), - NodeInput::Value { tagged_value, .. } if tagged_value.ty() == hidden_input.tagged_value.ty() => Some(input.clone()), - NodeInput::Import { .. } | NodeInput::Value { .. } | NodeInput::Scope(_) | NodeInput::Reflection(_) | NodeInput::Inline(_) => None, - } - }) - .or_else(|| { - tagged_values.iter().find_map(|hidden_input| { - let input = node.inputs.get(hidden_input.input_index)?; - match input { - NodeInput::Node { .. } | NodeInput::Import { .. } | NodeInput::Value { .. } => Some(input.clone()), - NodeInput::Scope(_) | NodeInput::Reflection(_) | NodeInput::Inline(_) => None, - } - }) - }); - - if let Some(primary) = passthrough_input { - node.implementation = DocumentNodeImplementation::ProtoNode(graphene_core::ops::identity::IDENTIFIER); - node.call_argument = concrete!(()); - node.inputs.clear(); - node.inputs.push(primary); - } else { - let tagged = tagged_values - .iter() - .find(|hidden_input| hidden_input.tagged_value.ty() != concrete!(())) - .or_else(|| tagged_values.first()) - .map(|hidden_input| hidden_input.tagged_value.clone()) - .or_else(|| TaggedValue::from_type(&node.call_argument)) - .unwrap_or(TaggedValue::None); - - node.implementation = DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("core_types::value::ClonedNode")); - node.call_argument = concrete!(()); - node.inputs.clear(); - node.inputs.push(NodeInput::value(tagged, false)); + if replaced_any_downstream { + self.replace_network_outputs(NodeInput::node(id, 0), NodeInput::value(replacement_tagged, false)); + return; } + node.implementation = DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("core_types::value::ClonedNode")); + node.call_argument = concrete!(()); + node.inputs.clear(); + node.inputs.push(NodeInput::value(replacement_tagged, false)); + self.nodes.insert(id, node); return; } diff --git a/node-graph/preprocessor/src/lib.rs b/node-graph/preprocessor/src/lib.rs index 4ddb0397e4..544ac40eea 100644 --- a/node-graph/preprocessor/src/lib.rs +++ b/node-graph/preprocessor/src/lib.rs @@ -87,7 +87,7 @@ pub fn generate_node_substitutions() -> HashMap HashMap DocumentNode { inputs: vec![NodeInput::import(generic!(X), i)], implementation: DocumentNodeImplementation::ProtoNode(identity_node.clone()), - visible: Visible::Passthrough, + visible: None, ..Default::default() }, }, @@ -111,7 +111,7 @@ pub fn generate_node_substitutions() -> HashMap HashMap