From 3bdec5d005685a259d45650a6aa18dcca7b969d0 Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Sun, 13 Sep 2026 17:58:53 +0200 Subject: [PATCH] Fold attribute names through monitors and skip coercing name inputs --- editor/src/node_graph_executor/runtime.rs | 13 +++++- .../graph-craft/src/graphene_compiler.rs | 43 ++++++++++++++++--- node-graph/graph-craft/src/proto.rs | 42 +++++++++++++----- .../src/dynamic_executor.rs | 23 ++++++++++ node-graph/preprocessor/src/lib.rs | 40 ++++++++++++++++- 5 files changed, 140 insertions(+), 21 deletions(-) diff --git a/editor/src/node_graph_executor/runtime.rs b/editor/src/node_graph_executor/runtime.rs index a8e1880c08..a97943b2e9 100644 --- a/editor/src/node_graph_executor/runtime.rs +++ b/editor/src/node_graph_executor/runtime.rs @@ -5,7 +5,7 @@ use graph_craft::application_io::resource::ResourceRegistry; use graph_craft::application_io::{PlatformApplicationIo, PlatformEditorApi}; use graph_craft::document::value::{RenderOutput, RenderOutputType, TaggedValue}; use graph_craft::document::{NodeId, NodeNetwork}; -use graph_craft::graphene_compiler::Compiler; +use graph_craft::graphene_compiler::{CompileError, Compiler}; use graph_craft::proto::GraphErrors; use graphene_std::application_io::{ApplicationIo, ExportFormat, NodeGraphUpdateMessage, NodeGraphUpdateSender, RenderConfig, Texture}; use graphene_std::bounds::RenderBoundingBox; @@ -441,7 +441,12 @@ impl NodeRuntime { let c = Compiler {}; let proto_network = match c.compile_single(scoped_network, &interpreted_executor::node_registry::NODE_REGISTRY) { Ok(network) => network, - Err(e) => return Err((ResolvedDocumentNodeTypesDelta::default(), e)), + // Errors pinned to nodes (the attribute-name fold) show in the graph like type errors do + Err(CompileError::Graph(errors)) => { + self.node_graph_errors.clone_from(&errors); + return Err((ResolvedDocumentNodeTypesDelta::default(), format!("{errors:?}"))); + } + Err(error) => return Err((ResolvedDocumentNodeTypesDelta::default(), error.to_string())), }; self.monitor_nodes = proto_network .nodes @@ -499,6 +504,10 @@ impl NodeRuntime { let result = self.executor.introspect_with(monitor_node_path, |layout, batch, _arena| { use graphene_std::core_types::record::{Group, GroupItem, RunView}; let type_id = layout.element.type_id; + // A bare constant (a value input under a test monitor) has no run to read: only a level's records adopt as one + if layout.element.content_hash.is_none() { + return Some(()); + } // Graphic run: thumbnail (text-aware bounds, since the `BoundingBox` trait can't lay out `Graphic::Text` content) if type_id == std::any::TypeId::of::() { if update_thumbnails { diff --git a/node-graph/graph-craft/src/graphene_compiler.rs b/node-graph/graph-craft/src/graphene_compiler.rs index b84f8645eb..e68b577096 100644 --- a/node-graph/graph-craft/src/graphene_compiler.rs +++ b/node-graph/graph-craft/src/graphene_compiler.rs @@ -1,11 +1,42 @@ use crate::document::NodeNetwork; -use crate::proto::{ProtoNetwork, Registry}; +use crate::proto::{GraphErrors, ProtoNetwork, Registry}; use std::error::Error; +/// Why a network failed to compile: a structural message, or errors the +/// editor can pin to their nodes in the graph. +#[derive(Debug, Clone, PartialEq)] +pub enum CompileError { + Message(String), + Graph(GraphErrors), +} + +impl std::fmt::Display for CompileError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + CompileError::Message(message) => write!(f, "{message}"), + CompileError::Graph(errors) => write!(f, "{errors:?}"), + } + } +} + +impl Error for CompileError {} + +impl From for CompileError { + fn from(message: String) -> Self { + CompileError::Message(message) + } +} + +impl From for String { + fn from(error: CompileError) -> Self { + error.to_string() + } +} + pub struct Compiler {} impl Compiler { - pub fn compile<'r>(&self, mut network: NodeNetwork, registry: &'r Registry) -> impl Iterator> + 'r { + pub fn compile<'r>(&self, mut network: NodeNetwork, registry: &'r Registry) -> impl Iterator> + 'r { network.resolve_scope_inputs(); network.generate_node_paths(&[]); let node_ids = network.nodes.keys().copied().collect::>(); @@ -20,17 +51,15 @@ impl Compiler { proto_networks.map(move |mut proto_network| { proto_network.insert_context_nullification_nodes()?; let _ = proto_network.resolve_types(registry); - proto_network - .compute_layouts() - .map_err(|errors| errors.iter().map(|error| format!("{:?}", error.error)).collect::>().join("\n"))?; + proto_network.compute_layouts().map_err(CompileError::Graph)?; proto_network.generate_stable_node_ids(); Ok(proto_network) }) } - pub fn compile_single(&self, network: NodeNetwork, registry: &Registry) -> Result { + pub fn compile_single(&self, network: NodeNetwork, registry: &Registry) -> Result { assert_eq!(network.exports.len(), 1, "Graph with multiple outputs not yet handled"); let Some(proto_network) = self.compile(network, registry).next() else { - return Err("Failed to convert graph into proto graph".to_string()); + return Err(CompileError::Message("Failed to convert graph into proto graph".to_string())); }; proto_network } diff --git a/node-graph/graph-craft/src/proto.rs b/node-graph/graph-craft/src/proto.rs index d8622d6e08..db9eac46ae 100644 --- a/node-graph/graph-craft/src/proto.rs +++ b/node-graph/graph-craft/src/proto.rs @@ -424,6 +424,27 @@ impl ProtoNetwork { errors.is_empty().then_some(()).ok_or(errors) } + /// The constant a node serves, seen through the wrappers that pass a value + /// on unchanged (the editor's monitors, memo and nullification nodes, and + /// passthroughs); a node that computes its value names itself instead. + fn constant_behind(&self, id: NodeId) -> Result<&MemoHash, &str> { + let transparent = [ + graphene_core::memo::monitor::IDENTIFIER, + graphene_core::memo::memoize::IDENTIFIER, + graphene_core::memo::frame_memo::IDENTIFIER, + graphene_core::context_modification::context_modification::IDENTIFIER, + graphene_core::ops::passthrough::IDENTIFIER, + ]; + let mut node = &self.nodes[id.0 as usize].1; + loop { + match &node.construction_args { + ConstructionArgs::Value(value) => return Ok(value), + ConstructionArgs::Nodes(items) if transparent.contains(&node.identifier) && !items.is_empty() => node = &self.nodes[items[0].0 as usize].1, + _ => return Err(node.identifier.as_str()), + } + } + } + /// Resolves every name-from-input write against the constant its name /// input carries, leaving each node's meta indistinguishable from a marker /// node's. A name input that is not a constant is refused here, which is @@ -449,18 +470,17 @@ impl ProtoNetwork { .collect(); let names: Vec> = sources .iter() - .map( - |&(name_input, _)| match inputs.get(name_input as usize).map(|input| &self.nodes[input.0 as usize].1.construction_args) { - Some(ConstructionArgs::Value(value)) => match &**value { - value::TaggedValue::String(name) => Ok(core_types::attribute::intern_name(name)), - other => Err(GraphErrorType::AttributeName(format!("an attribute name must be text, but input {} is {}", name_input + 1, other.ty()))), - }, - _ => Err(GraphErrorType::AttributeName(format!( - "input {} must be a constant, since attribute names resolve when the graph compiles rather than when it runs", - name_input + 1 - ))), + .map(|&(name_input, _)| match inputs.get(name_input as usize).map(|input| self.constant_behind(*input)) { + Some(Ok(value)) => match &**value { + value::TaggedValue::String(name) => Ok(core_types::attribute::intern_name(name)), + other => Err(GraphErrorType::AttributeName(format!("an attribute name must be text, but input {} is {}", name_input + 1, other.ty()))), }, - ) + Some(Err(computed_by)) => Err(GraphErrorType::AttributeName(format!( + "input {} must be a constant, since attribute names resolve when the graph compiles rather than when it runs, but it is computed by {computed_by}", + name_input + 1 + ))), + None => Err(GraphErrorType::AttributeName(format!("input {} must be a constant, but the node has no such input", name_input + 1))), + }) .collect(); let node = &self.nodes[index].1; let mut folded: Vec<(&'static str, std::any::TypeId)> = Vec::new(); diff --git a/node-graph/interpreted-executor/src/dynamic_executor.rs b/node-graph/interpreted-executor/src/dynamic_executor.rs index 33d20e1840..f7f764e435 100644 --- a/node-graph/interpreted-executor/src/dynamic_executor.rs +++ b/node-graph/interpreted-executor/src/dynamic_executor.rs @@ -881,6 +881,29 @@ mod test { ); } + /// The editor's monitors and the compiler's memo and nullification nodes + /// pass a constant on unchanged, so a name reaches the fold through them. + #[test] + fn a_constant_name_folds_through_a_monitor() { + let mut network = ProtoNetwork { + stack_need: 0, + inputs: vec![], + output: NodeId(4), + nodes: vec![ + (NodeId(0), ProtoNode::value(ConstructionArgs::Value(TaggedValue::F64(7.).into()), vec![])), + (NodeId(1), string_value("novel:count")), + (NodeId(2), proto_node("graphene_core::memo::MonitorNode", vec![NodeId(1)])), + (NodeId(3), ProtoNode::value(ConstructionArgs::Value(TaggedValue::F64(2.5).into()), vec![])), + (NodeId(4), proto_node("graphic_nodes::graphic::WriteAttributeNode", vec![NodeId(0), NodeId(2), NodeId(3)])), + ], + }; + network.resolve_types(&node_registry::NODE_REGISTRY).unwrap(); + network.compute_layouts().expect("a monitored constant still folds"); + let executor = DynamicExecutor::new(network).unwrap(); + let layout = executor.tree().get(NodeId(4)).unwrap().layout().clone(); + assert!(layout.offset_of("novel:count", 0).is_some(), "the folded name names a field of the output layout"); + } + #[test] fn a_runtime_attribute_name_is_refused_when_the_graph_compiles() { // The outer node's name comes off another node rather than sitting on diff --git a/node-graph/preprocessor/src/lib.rs b/node-graph/preprocessor/src/lib.rs index 33616aaff5..54e3ac3e34 100644 --- a/node-graph/preprocessor/src/lib.rs +++ b/node-graph/preprocessor/src/lib.rs @@ -136,6 +136,17 @@ impl Preprocessor { if valid_call_args.len() > 1 { input_type = &const { generic!(D) }; } + // An attribute name folds to a constant when the graph compiles, so its input keeps the + // document's value in place rather than riding a coercion the fold could not see through. + let name_inputs: HashSet = implementations + .iter() + .filter_map(|entry| entry.layout_meta.as_ref()) + .flat_map(|meta| { + let writes = meta.named_writes.iter().map(|named| named.name_input as usize); + let reads = meta.named_reads.iter().map(|named| named.name_input as usize); + writes.chain(reads) + }) + .collect(); let mut inputs: Vec<_> = node_inputs(fields, first_node_io); let wrapper_input_count = inputs.len() - if *async_source_fields { 2 } else { 0 }; @@ -156,7 +167,7 @@ impl Preprocessor { ( NodeId(i as u64), match inputs.len() { - 1 => { + 1 if !name_inputs.contains(&i) => { let input = inputs.iter().next().unwrap(); let input_ty = input.nested_type(); let mut inputs = vec![NodeInput::import(input.clone(), i)]; @@ -311,3 +322,30 @@ impl std::fmt::Display for PreprocessorError { } } } + +#[cfg(test)] +mod tests { + use super::*; + use graph_craft::document::DocumentNodeImplementation; + + /// An attribute name folds when the graph compiles, so the preprocessor + /// must leave the name input as the document's constant rather than coerce it. + #[test] + fn an_attribute_name_input_is_not_coerced() { + let preprocessor = Preprocessor::new(); + let Some(substitution) = preprocessor.substitutions.get(&graphene_std::graphic::write_attribute::IDENTIFIER) else { + // Nothing to coerce on any input means no wrapper at all, which is also correct + return; + }; + let DocumentNodeImplementation::Network(wrapper) = &substitution.implementation else { + panic!("a substitution wraps the node in a network"); + }; + let name_slot = &wrapper.nodes[&NodeId(1)]; + assert_eq!( + name_slot.implementation, + DocumentNodeImplementation::ProtoNode(ops::passthrough::IDENTIFIER), + "the name input passes through untouched, got {:?}", + name_slot.implementation + ); + } +}