Skip to content

Implement node output destructuring #3806

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Open
JustJ01 wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Implement node output destructuring #3806

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2e97e81
Add `Destruct` macro and trait for automatically acessing struct fields
TrueDoctor Apr 14, 2025
e215927
Complete node output destructuring implementation
JustJ01 Feb 22, 2026
f05970c
implemenet preprocessing expansion
JustJ01 Feb 23, 2026
f81c4da
formatted files
JustJ01 Feb 23, 2026
9db03a5
convert Split Vec2 and Split Channels to proto nodes
JustJ01 Feb 23, 2026
051b690
removed dead code
JustJ01 Feb 23, 2026
e5e4936
Merge branch 'master' into finish-deconstruct-outputs
Keavon Apr 3, 2026
958810d
Potentially solve outstanding issues
Keavon Apr 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
182 changes: 1 addition & 181 deletions editor/src/messages/portfolio/document/node_graph/document_node_definitions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ use graph_craft::concrete;
use graph_craft::document::value::*;
use graph_craft::document::*;
use graphene_std::brush::brush_cache::BrushCache;
use graphene_std::extract_xy::XY;
use graphene_std::raster::{CellularDistanceFunction, CellularReturnType, Color, DomainWarpType, FractalType, NoiseType, RedGreenBlueAlpha};
use graphene_std::raster::{CellularDistanceFunction, CellularReturnType, Color, DomainWarpType, FractalType, NoiseType};
use graphene_std::raster_types::{CPU, Raster};
use graphene_std::table::Table;
#[allow(unused_imports)]
Expand Down Expand Up @@ -1197,185 +1196,6 @@ fn document_node_definitions() -> HashMap<DefinitionIdentifier, DocumentNodeDefi
description: Cow::Borrowed("Generates customizable procedural noise patterns."),
properties: None,
},
DocumentNodeDefinition {
identifier: "Split Channels",
category: "Raster: Channels",
node_template: NodeTemplate {
document_node: DocumentNode {
implementation: DocumentNodeImplementation::Network(NodeNetwork {
exports: vec![
NodeInput::value(TaggedValue::None, false),
NodeInput::node(NodeId(0), 0),
NodeInput::node(NodeId(1), 0),
NodeInput::node(NodeId(2), 0),
NodeInput::node(NodeId(3), 0),
],
nodes: [
DocumentNode {
inputs: vec![
NodeInput::import(concrete!(Table<Raster<CPU>>), 0),
NodeInput::value(TaggedValue::RedGreenBlueAlpha(RedGreenBlueAlpha::Red), false),
],
implementation: DocumentNodeImplementation::ProtoNode(raster_nodes::adjustments::extract_channel::IDENTIFIER),
call_argument: generic!(T),
..Default::default()
},
DocumentNode {
inputs: vec![
NodeInput::import(concrete!(Table<Raster<CPU>>), 0),
NodeInput::value(TaggedValue::RedGreenBlueAlpha(RedGreenBlueAlpha::Green), false),
],
implementation: DocumentNodeImplementation::ProtoNode(raster_nodes::adjustments::extract_channel::IDENTIFIER),
call_argument: generic!(T),
..Default::default()
},
DocumentNode {
inputs: vec![
NodeInput::import(concrete!(Table<Raster<CPU>>), 0),
NodeInput::value(TaggedValue::RedGreenBlueAlpha(RedGreenBlueAlpha::Blue), false),
],
implementation: DocumentNodeImplementation::ProtoNode(raster_nodes::adjustments::extract_channel::IDENTIFIER),
call_argument: generic!(T),
..Default::default()
},
DocumentNode {
inputs: vec![
NodeInput::import(concrete!(Table<Raster<CPU>>), 0),
NodeInput::value(TaggedValue::RedGreenBlueAlpha(RedGreenBlueAlpha::Alpha), false),
],
implementation: DocumentNodeImplementation::ProtoNode(raster_nodes::adjustments::extract_channel::IDENTIFIER),
call_argument: generic!(T),
..Default::default()
},
]
.into_iter()
.enumerate()
.map(|(id, node)| (NodeId(id as u64), node))
.collect(),
..Default::default()
}),
inputs: vec![NodeInput::value(TaggedValue::Raster(Default::default()), true)],
..Default::default()
},
persistent_node_metadata: DocumentNodePersistentMetadata {
input_metadata: vec![("Image", "TODO").into()],
output_names: vec!["".to_string(), "Red".to_string(), "Green".to_string(), "Blue".to_string(), "Alpha".to_string()],
network_metadata: Some(NodeNetworkMetadata {
persistent_metadata: NodeNetworkPersistentMetadata {
node_metadata: [
DocumentNodeMetadata {
persistent_metadata: DocumentNodePersistentMetadata {
node_type_metadata: NodeTypePersistentMetadata::node(IVec2::new(0, 0)),
..Default::default()
},
..Default::default()
},
DocumentNodeMetadata {
persistent_metadata: DocumentNodePersistentMetadata {
node_type_metadata: NodeTypePersistentMetadata::node(IVec2::new(0, 2)),
..Default::default()
},
..Default::default()
},
DocumentNodeMetadata {
persistent_metadata: DocumentNodePersistentMetadata {
node_type_metadata: NodeTypePersistentMetadata::node(IVec2::new(0, 4)),
..Default::default()
},
..Default::default()
},
DocumentNodeMetadata {
persistent_metadata: DocumentNodePersistentMetadata {
node_type_metadata: NodeTypePersistentMetadata::node(IVec2::new(0, 6)),
..Default::default()
},
..Default::default()
},
]
.into_iter()
.enumerate()
.map(|(id, node)| (NodeId(id as u64), node))
.collect(),
..Default::default()
},
..Default::default()
}),
..Default::default()
},
},
description: Cow::Borrowed("TODO"),
properties: None,
},
DocumentNodeDefinition {
identifier: "Split Vec2",
category: "Math: Vector",
node_template: NodeTemplate {
document_node: DocumentNode {
implementation: DocumentNodeImplementation::Network(NodeNetwork {
exports: vec![NodeInput::value(TaggedValue::None, false), NodeInput::node(NodeId(0), 0), NodeInput::node(NodeId(1), 0)],
nodes: [
DocumentNode {
inputs: vec![NodeInput::import(concrete!(DVec2), 0), NodeInput::value(TaggedValue::XY(XY::X), false)],
implementation: DocumentNodeImplementation::ProtoNode(extract_xy::extract_xy::IDENTIFIER),
call_argument: generic!(T),
..Default::default()
},
DocumentNode {
inputs: vec![NodeInput::import(concrete!(DVec2), 0), NodeInput::value(TaggedValue::XY(XY::Y), false)],
implementation: DocumentNodeImplementation::ProtoNode(extract_xy::extract_xy::IDENTIFIER),
call_argument: generic!(T),
..Default::default()
},
]
.into_iter()
.enumerate()
.map(|(id, node)| (NodeId(id as u64), node))
.collect(),

..Default::default()
}),
inputs: vec![NodeInput::value(TaggedValue::DVec2(DVec2::ZERO), true)],
..Default::default()
},
persistent_node_metadata: DocumentNodePersistentMetadata {
input_metadata: vec![("Vec2", "TODO").into()],
output_names: vec!["".to_string(), "X".to_string(), "Y".to_string()],
network_metadata: Some(NodeNetworkMetadata {
persistent_metadata: NodeNetworkPersistentMetadata {
node_metadata: [
DocumentNodeMetadata {
persistent_metadata: DocumentNodePersistentMetadata {
node_type_metadata: NodeTypePersistentMetadata::node(IVec2::new(0, 0)),
..Default::default()
},
..Default::default()
},
DocumentNodeMetadata {
persistent_metadata: DocumentNodePersistentMetadata {
node_type_metadata: NodeTypePersistentMetadata::node(IVec2::new(0, 2)),
..Default::default()
},
..Default::default()
},
]
.into_iter()
.enumerate()
.map(|(id, node)| (NodeId(id as u64), node))
.collect(),
..Default::default()
},
..Default::default()
}),
..Default::default()
},
},
description: Cow::Borrowed(
"Decomposes the X and Y components of a vec2.\n\
\n\
The inverse of this node is \"Vec2 Value\", which can have either or both its X and Y parameters exposed as graph inputs.",
),
properties: None,
},
// TODO: Remove this and just use the proto node definition directly
DocumentNodeDefinition {
identifier: "Brush",
Expand Down
138 changes: 137 additions & 1 deletion .../messages/portfolio/document/node_graph/document_node_definitions/document_node_derive.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
use super::DocumentNodeDefinition;
use crate::messages::portfolio::document::node_graph::document_node_definitions::DefinitionIdentifier;
use crate::messages::portfolio::document::utility_types::network_interface::{DocumentNodePersistentMetadata, InputMetadata, NodeTemplate, WidgetOverride};
use crate::messages::portfolio::document::utility_types::network_interface::{
DocumentNodeMetadata, DocumentNodePersistentMetadata, InputMetadata, NodeNetworkMetadata, NodeNetworkPersistentMetadata, NodeTemplate, NodeTypePersistentMetadata, WidgetOverride,
};
use graph_craft::ProtoNodeIdentifier;
use graph_craft::document::value::TaggedValue;
use graph_craft::document::*;
use graphene_std::registry::*;
use graphene_std::*;
Expand Down Expand Up @@ -36,6 +40,7 @@ pub(super) fn post_process_nodes(custom: Vec<DocumentNodeDefinition>) -> HashMap
description,
properties,
context_features,
output_fields,
} = metadata;

let Some(implementations) = &node_registry.get(id) else { continue };
Expand Down Expand Up @@ -71,6 +76,12 @@ pub(super) fn post_process_nodes(custom: Vec<DocumentNodeDefinition>) -> HashMap
RegistryWidgetOverride::Custom(str) => InputMetadata::with_name_description_override(f.name, f.description, WidgetOverride::Custom(str.to_string())),
})
.collect(),
output_names: if output_fields.is_empty() {
Vec::new()
} else {
// A leading field with empty node_path is a hidden primary output placeholder, included as ""
output_fields.iter().map(|field| field.name.to_string()).collect()
},
locked: false,
..Default::default()
},
Expand All @@ -81,6 +92,120 @@ pub(super) fn post_process_nodes(custom: Vec<DocumentNodeDefinition>) -> HashMap
},
);
}
drop(node_registry);

// For nodes with destructured outputs, build a Network implementation so the UI knows about
// the multiple output connectors. The network contains the source proto node plus one extractor
// proto node per output field, wired directly (no identity pass-through or memo nodes).
let destructured_info = {
let node_metadata_registry = NODE_METADATA.lock().unwrap();
let node_registry = NODE_REGISTRY.lock().unwrap();
definitions_map
.iter()
.filter_map(|(_, def)| {
let DocumentNodeImplementation::ProtoNode(id) = &def.node_template.document_node.implementation else {
return None;
};
let meta = node_metadata_registry.get(id)?;
if meta.output_fields.is_empty() {
return None;
}

let has_hidden_primary = meta.output_fields.first().is_some_and(|f| f.node_path.is_empty());
let real_fields = if has_hidden_primary { &meta.output_fields[1..] } else { meta.output_fields };

let extractors: Vec<_> = real_fields
.iter()
.filter_map(|field| {
let extractor_id = ProtoNodeIdentifier::with_owned_string(field.node_path.to_string());
let call_arg = node_registry.get(&extractor_id).and_then(|impls| impls.first().map(|(_, node_io)| node_io.call_argument.clone()))?;
Some((extractor_id, call_arg))
})
.collect();

if extractors.is_empty() {
return None;
}

Some((id.clone(), has_hidden_primary, extractors))
})
.collect::<Vec<_>>()
};

// Build the definition-time network for each destructured node
for (id, has_hidden_primary, extractors) in &destructured_info {
let identifier = DefinitionIdentifier::ProtoNode(id.clone());
let Some(definition) = definitions_map.get_mut(&identifier) else { continue };

// The source node runs the original proto node (e.g. split_vec2)
let source_node_id = NodeId(0);
let source_node = DocumentNode {
inputs: definition
.node_template
.document_node
.inputs
.iter()
.enumerate()
.map(|(i, input)| NodeInput::import(input.ty(), i))
.collect(),
implementation: DocumentNodeImplementation::ProtoNode(id.clone()),
call_argument: definition.node_template.document_node.call_argument.clone(),
..Default::default()
};

// One extractor node per output field, each taking input from the source node
let mut inner_nodes: Vec<(NodeId, DocumentNode)> = vec![(source_node_id, source_node)];

let mut exports = if *has_hidden_primary { vec![NodeInput::value(TaggedValue::None, false)] } else { Vec::new() };

for (i, (extractor_id, call_arg)) in extractors.iter().enumerate() {
let extractor_node_id = NodeId((i + 1) as u64);
inner_nodes.push((
extractor_node_id,
DocumentNode {
inputs: vec![NodeInput::node(source_node_id, 0)],
implementation: DocumentNodeImplementation::ProtoNode(extractor_id.clone()),
call_argument: call_arg.clone(),
..Default::default()
},
));
exports.push(NodeInput::node(extractor_node_id, 0));
}

// Generate network metadata for each inner node
let node_metadata: HashMap<_, _> = inner_nodes
.iter()
.map(|(nid, _)| {
(
*nid,
DocumentNodeMetadata {
persistent_metadata: DocumentNodePersistentMetadata {
node_type_metadata: NodeTypePersistentMetadata::node(glam::IVec2::ZERO),
..Default::default()
},
..Default::default()
},
)
})
.collect();

definition.node_template.persistent_node_metadata.network_metadata = Some(NodeNetworkMetadata {
persistent_metadata: NodeNetworkPersistentMetadata {
reference: Some(definition.identifier.to_string()),
node_metadata,
..Default::default()
},
..Default::default()
});
definition.node_template.persistent_node_metadata.display_name = definition.identifier.to_string();

definition.node_template.document_node.implementation = DocumentNodeImplementation::Network(NodeNetwork {
exports,
nodes: inner_nodes.into_iter().collect(),
scope_injections: Default::default(),
generated: false,
});
}

// If any protonode does not have metadata then set its display name to its identifier string
for definition in definitions_map.values_mut() {
Expand All @@ -92,6 +217,17 @@ pub(super) fn post_process_nodes(custom: Vec<DocumentNodeDefinition>) -> HashMap
}
}

// Fill in inner proto node metadata for the destructured output network definitions
for (id, _, _) in destructured_info {
let identifier = DefinitionIdentifier::ProtoNode(id);
// We need to split the borrow: extract the node and metadata, traverse, then put metadata back
let Some(definition) = definitions_map.get(&identifier) else { continue };
let document_node = definition.node_template.document_node.clone();
let mut persistent_metadata = definition.node_template.persistent_node_metadata.clone();
traverse_node(&document_node, &mut persistent_metadata, &definitions_map);
definitions_map.get_mut(&identifier).unwrap().node_template.persistent_node_metadata = persistent_metadata;
}

// Add the rest of the network nodes to the map and add the metadata for their internal protonodes
for mut network_node in network_nodes {
traverse_node(&network_node.node_template.document_node, &mut network_node.node_template.persistent_node_metadata, &definitions_map);
Expand Down
4 changes: 3 additions & 1 deletion editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,9 @@ impl<'a> MessageHandler<NodeGraphMessage, NodeGraphMessageContext<'a>> for NodeG
{
return;
};
if let Some(DocumentNodeImplementation::Network(_)) = network_interface.implementation(&node_id, selection_network_path) {
if let Some(DocumentNodeImplementation::Network(network)) = network_interface.implementation(&node_id, selection_network_path)
&& !network.generated
{
responses.add(DocumentMessage::EnterNestedNetwork { node_id });
}
}
Expand Down
9 changes: 8 additions & 1 deletion editor/src/node_graph_executor/runtime.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,14 @@ impl NodeRuntime {
node_graph_errors: Vec::new(),
monitor_nodes: Vec::new(),

substitutions: preprocessor::generate_node_substitutions(),
substitutions: {
// Exclude nodes with destructured outputs since their definition networks already contain the complete structure
let mut substitutions = preprocessor::generate_node_substitutions();
let node_metadata = graphene_std::registry::NODE_METADATA.lock().unwrap();
substitutions.retain(|id, _| node_metadata.get(id).is_none_or(|meta| meta.output_fields.is_empty()));
drop(node_metadata);
substitutions
Comment on lines +144 to +149
Copy link
Copy Markdown
Member

@TrueDoctor TrueDoctor Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this done here instea of in the generate_node_substitutions method?

Copy link
Copy Markdown
Member

@Keavon Keavon Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is all out of my wheelhouse. If you're able to take it from here, I'd appreciate that. I basically just attempted to get it to work from a QA perspective without really understanding what's going on internally, since the alternative would have been to close the PR outright. Sorry I don't know the answer to the question, although it's probably worth fixing if it doesn't match your expectations.

Copy link
Copy Markdown
Member

@TrueDoctor TrueDoctor Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean you made the changes you should be able to address comments regarding them. the generate substitutions functions is used in other places as well, and it looks to me like the changes should probably apply to all callers so it would likely make sense to integrate the changes in the function (or even better instead of first generating the substitutions and then filtering them out, we should just not generate them in the first place) I won't have time to take over the pr myself

},

thumbnail_renders: Default::default(),
vector_modify: Default::default(),
Expand Down
Loading
Loading