From 357cb32c9d3846f859f91d941a20afee79762074 Mon Sep 17 00:00:00 2001 From: JustJ01 Date: Sat, 14 Feb 2026 01:03:55 +0530 Subject: [PATCH] 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