Replace node definition string-based lookups with DefinitionIdentifier instances (#3451)

* create definition identifier and integrate it

* Bug fixes and code review

* formatting

* Fix migrations

* Fix remove handles migration

* formatting

* Fix test

* Fix tests 2

* fix deserialization

* Code review

* Small fixes

* Consolidate 'Morph' node migrations

* Add old SamplePointsNode name to migrations list

* Fix tests

* Unrelated small fix

* Fix migration crashes

* Fix tests

* Final code review

* fmt

* Add metadata

---------

Co-authored-by: Keavon Chambers <keavon@keavon.com>
This commit is contained in:
Adam Gerhant
2026-01-12 23:09:43 -08:00
committed by GitHub
parent 4fea2b0fe7
commit a6052c5819
63 changed files with 843 additions and 802 deletions

View File

@@ -800,7 +800,7 @@ impl NodeNetwork {
let path = node.original_location.path.clone().unwrap_or_default();
// Replace value inputs with dedicated value nodes
if node.implementation != DocumentNodeImplementation::ProtoNode("core_types::value::ClonedNode".into()) {
if node.implementation != DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("core_types::value::ClonedNode")) {
Self::replace_value_inputs_with_nodes(&mut node.inputs, &mut self.nodes, &path, gen_id, map_ids, id);
}
@@ -842,7 +842,10 @@ impl NodeNetwork {
for (nested_node_id, mut nested_node) in inner_network.nodes.into_iter() {
for (nested_input_index, nested_input) in nested_node.clone().inputs.iter().enumerate() {
if let NodeInput::Import { import_index, .. } = nested_input {
let parent_input = node.inputs.get(*import_index).unwrap_or_else(|| panic!("Import index {import_index} should always exist"));
let parent_input = node
.inputs
.get(*import_index)
.unwrap_or_else(|| panic!("Import index {import_index} of network node implementation {:?} should always exist", nested_node.implementation));
match *parent_input {
// If the input to self is a node, connect the corresponding output of the inner network to it
NodeInput::Node { node_id, output_index } => {
@@ -936,7 +939,7 @@ impl NodeNetwork {
merged_node_id,
DocumentNode {
inputs: vec![NodeInput::Value { tagged_value, exposed }],
implementation: DocumentNodeImplementation::ProtoNode("core_types::value::ClonedNode".into()),
implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("core_types::value::ClonedNode")),
original_location,
..Default::default()
},
@@ -1035,7 +1038,7 @@ impl NodeNetwork {
assert_eq!(output_index, 0);
// TODO: check if we can read lambda checking?
let mut input_node = self.nodes.remove(&node_id).unwrap();
node.implementation = DocumentNodeImplementation::ProtoNode("core_types::value::ClonedNode".into());
node.implementation = DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("core_types::value::ClonedNode"));
if let Some(input) = input_node.inputs.get_mut(0) {
*input = match &input {
NodeInput::Node { .. } => NodeInput::import(generic!(T), 0),
@@ -1152,7 +1155,7 @@ mod test {
NodeId(0),
DocumentNode {
inputs: vec![NodeInput::import(concrete!(u32), 0), NodeInput::import(concrete!(u32), 1)],
implementation: DocumentNodeImplementation::ProtoNode("core_types::structural::ConsNode".into()),
implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("core_types::structural::ConsNode")),
..Default::default()
},
),
@@ -1160,7 +1163,7 @@ mod test {
NodeId(1),
DocumentNode {
inputs: vec![NodeInput::node(NodeId(0), 0)],
implementation: DocumentNodeImplementation::ProtoNode("core_types::ops::AddPairNode".into()),
implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("core_types::ops::AddPairNode")),
..Default::default()
},
),
@@ -1182,7 +1185,7 @@ mod test {
NodeId(1),
DocumentNode {
inputs: vec![NodeInput::import(concrete!(u32), 0), NodeInput::import(concrete!(u32), 1)],
implementation: DocumentNodeImplementation::ProtoNode("core_types::structural::ConsNode".into()),
implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("core_types::structural::ConsNode")),
..Default::default()
},
),
@@ -1190,7 +1193,7 @@ mod test {
NodeId(2),
DocumentNode {
inputs: vec![NodeInput::node(NodeId(1), 0)],
implementation: DocumentNodeImplementation::ProtoNode("core_types::ops::AddPairNode".into()),
implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("core_types::ops::AddPairNode")),
..Default::default()
},
),
@@ -1263,13 +1266,13 @@ mod test {
let document_node = DocumentNode {
inputs: vec![NodeInput::node(NodeId(0), 0)],
call_argument: concrete!(u32),
implementation: DocumentNodeImplementation::ProtoNode("core_types::structural::ConsNode".into()),
implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("core_types::structural::ConsNode")),
..Default::default()
};
let proto_node = document_node.resolve_proto_node();
let reference = ProtoNode {
identifier: "core_types::structural::ConsNode".into(),
identifier: ProtoNodeIdentifier::new("core_types::structural::ConsNode"),
call_argument: concrete!(u32),
construction_args: ConstructionArgs::Nodes(vec![NodeId(0)]),
..Default::default()
@@ -1286,7 +1289,7 @@ mod test {
(
NodeId(10),
ProtoNode {
identifier: "core_types::structural::ConsNode".into(),
identifier: ProtoNodeIdentifier::new("core_types::structural::ConsNode"),
call_argument: concrete!(u32),
construction_args: ConstructionArgs::Nodes(vec![NodeId(14)]),
original_location: OriginalLocation {
@@ -1302,7 +1305,7 @@ mod test {
(
NodeId(11),
ProtoNode {
identifier: "core_types::ops::AddPairNode".into(),
identifier: ProtoNodeIdentifier::new("core_types::ops::AddPairNode"),
call_argument: concrete!(Context),
construction_args: ConstructionArgs::Nodes(vec![NodeId(10)]),
original_location: OriginalLocation {
@@ -1317,7 +1320,7 @@ mod test {
(
NodeId(14),
ProtoNode {
identifier: "core_types::value::ClonedNode".into(),
identifier: ProtoNodeIdentifier::new("core_types::value::ClonedNode"),
call_argument: concrete!(core_types::Context),
construction_args: ConstructionArgs::Value(TaggedValue::U32(2).into()),
original_location: OriginalLocation {
@@ -1351,7 +1354,7 @@ mod test {
DocumentNode {
inputs: vec![NodeInput::node(NodeId(14), 0)],
call_argument: concrete!(u32),
implementation: DocumentNodeImplementation::ProtoNode("core_types::structural::ConsNode".into()),
implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("core_types::structural::ConsNode")),
original_location: OriginalLocation {
path: Some(vec![NodeId(1), NodeId(0)]),
inputs_source: [(Source { node: vec![NodeId(1)], index: 1 }, 1)].into(),
@@ -1365,7 +1368,7 @@ mod test {
NodeId(14),
DocumentNode {
inputs: vec![NodeInput::value(TaggedValue::U32(2), false)],
implementation: DocumentNodeImplementation::ProtoNode("core_types::value::ClonedNode".into()),
implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("core_types::value::ClonedNode")),
original_location: OriginalLocation {
path: Some(vec![NodeId(1), NodeId(4)]),
inputs_source: HashMap::new(),
@@ -1379,7 +1382,7 @@ mod test {
NodeId(11),
DocumentNode {
inputs: vec![NodeInput::node(NodeId(10), 0)],
implementation: DocumentNodeImplementation::ProtoNode("core_types::ops::AddPairNode".into()),
implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("core_types::ops::AddPairNode")),
original_location: OriginalLocation {
path: Some(vec![NodeId(1), NodeId(1)]),
inputs_source: HashMap::new(),

View File

@@ -30,7 +30,7 @@ impl core::fmt::Display for ProtoNetwork {
return f.write_str("{{Unknown Node}}");
};
f.write_str("Node: ")?;
f.write_str(&node.identifier.name)?;
f.write_str(node.identifier.as_str())?;
f.write_str("\n")?;
f.write_str(&"\t".repeat(indent))?;
@@ -156,7 +156,7 @@ impl ProtoNode {
use std::hash::Hasher;
let mut hasher = rustc_hash::FxHasher::default();
self.identifier.name.hash(&mut hasher);
self.identifier.as_str().hash(&mut hasher);
self.construction_args.hash(&mut hasher);
if self.skip_deduplication {
self.original_location.path.hash(&mut hasher);
@@ -612,7 +612,7 @@ impl GraphError {
pub fn new(node: &ProtoNode, text: impl Into<GraphErrorType>) -> Self {
Self {
node_path: node.original_location.path.clone().unwrap_or_default(),
identifier: node.identifier.name.clone(),
identifier: Cow::Owned(node.identifier.as_str().to_string()),
error: text.into(),
}
}
@@ -916,7 +916,7 @@ mod test {
let ids: Vec<_> = construction_network.nodes.iter().map(|(id, _)| *id).collect();
println!("{ids:#?}");
println!("nodes: {:#?}", construction_network.nodes);
assert_eq!(construction_network.nodes[0].1.identifier.name.as_ref(), "value");
assert_eq!(construction_network.nodes[0].1.identifier.as_str(), "value");
assert_eq!(ids, vec![NodeId(0), NodeId(1), NodeId(2), NodeId(3)]);
}
@@ -929,7 +929,7 @@ mod test {
assert_eq!(sorted, vec![NodeId(0), NodeId(1), NodeId(2), NodeId(3)]);
let ids: Vec<_> = construction_network.nodes.iter().map(|(id, _)| *id).collect();
println!("{ids:#?}");
assert_eq!(construction_network.nodes[0].1.identifier.name.as_ref(), "value");
assert_eq!(construction_network.nodes[0].1.identifier.as_str(), "value");
assert_eq!(ids, vec![NodeId(0), NodeId(1), NodeId(2), NodeId(3)]);
}
@@ -940,7 +940,7 @@ mod test {
.insert_context_nullification_nodes()
.expect("Error when calling 'insert_context_nullification_nodes' on 'construction_network.");
construction_network.generate_stable_node_ids();
assert_eq!(construction_network.nodes[0].1.identifier.name.as_ref(), "value");
assert_eq!(construction_network.nodes[0].1.identifier.as_str(), "value");
let ids: Vec<_> = construction_network.nodes.iter().map(|(id, _)| *id).collect();
// If this assert fails: These NodeIds seem to be changing when you modify TaggedValue, just update them.
@@ -958,7 +958,7 @@ mod test {
(
NodeId(7),
ProtoNode {
identifier: "id".into(),
identifier: ProtoNodeIdentifier::new("id"),
call_argument: concrete!(()),
construction_args: ConstructionArgs::Nodes(vec![NodeId(11)]),
..Default::default()
@@ -967,7 +967,7 @@ mod test {
(
NodeId(1),
ProtoNode {
identifier: "id".into(),
identifier: ProtoNodeIdentifier::new("id"),
call_argument: concrete!(()),
construction_args: ConstructionArgs::Nodes(vec![NodeId(11)]),
..Default::default()
@@ -976,7 +976,7 @@ mod test {
(
NodeId(10),
ProtoNode {
identifier: "cons".into(),
identifier: ProtoNodeIdentifier::new("cons"),
call_argument: concrete!(u32),
construction_args: ConstructionArgs::Nodes(vec![NodeId(14)]),
..Default::default()
@@ -985,7 +985,7 @@ mod test {
(
NodeId(11),
ProtoNode {
identifier: "add".into(),
identifier: ProtoNodeIdentifier::new("add"),
call_argument: concrete!(()),
construction_args: ConstructionArgs::Nodes(vec![NodeId(10)]),
..Default::default()
@@ -994,7 +994,7 @@ mod test {
(
NodeId(14),
ProtoNode {
identifier: "value".into(),
identifier: ProtoNodeIdentifier::new("value"),
call_argument: concrete!(()),
construction_args: ConstructionArgs::Value(value::TaggedValue::U32(2).into()),
..Default::default()
@@ -1014,7 +1014,7 @@ mod test {
(
NodeId(1),
ProtoNode {
identifier: "id".into(),
identifier: ProtoNodeIdentifier::new("id"),
call_argument: concrete!(()),
construction_args: ConstructionArgs::Nodes(vec![NodeId(2)]),
..Default::default()
@@ -1023,7 +1023,7 @@ mod test {
(
NodeId(2),
ProtoNode {
identifier: "id".into(),
identifier: ProtoNodeIdentifier::new("id"),
call_argument: concrete!(()),
construction_args: ConstructionArgs::Nodes(vec![NodeId(1)]),
..Default::default()

View File

@@ -98,9 +98,9 @@ async fn main() -> Result<(), Box<dyn Error>> {
Command::Export { ref document, .. } => document,
Command::ListNodeIdentifiers => {
let mut ids: Vec<_> = graphene_std::registry::NODE_METADATA.lock().unwrap().keys().cloned().collect();
ids.sort_by_key(|x| x.name.clone());
ids.sort_by_key(|x| x.as_str().to_string());
for id in ids {
println!("{}", id.name)
println!("{}", id.as_str());
}
return Ok(());
}
@@ -212,7 +212,7 @@ fn fix_nodes(network: &mut NodeNetwork) {
// https://github.com/GraphiteEditor/Graphite/blob/d68f91ccca69e90e6d2df78d544d36cd1aaf348e/editor/src/messages/portfolio/portfolio_message_handler.rs#L535
// Since the CLI doesn't have the document node definitions, a less robust method of just patching the inputs is used.
DocumentNodeImplementation::ProtoNode(proto_node_identifier)
if (proto_node_identifier.name.starts_with("graphene_core::ConstructLayerNode") || proto_node_identifier.name.starts_with("graphene_core::AddArtboardNode"))
if (proto_node_identifier.as_str().starts_with("graphene_core::ConstructLayerNode") || proto_node_identifier.as_str().starts_with("graphene_core::AddArtboardNode"))
&& node.inputs.len() < 3 =>
{
node.inputs.push(NodeInput::Reflection(DocumentNodeMetadata::DocumentNodePath));

View File

@@ -31,7 +31,7 @@ mod tests {
NodeId(1),
DocumentNode {
inputs: vec![NodeInput::node(NodeId(0), 0), NodeInput::node(NodeId(0), 0)],
implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("core_types::ops::AddNode")),
implementation: DocumentNodeImplementation::ProtoNode(graphene_std::math_nodes::add::IDENTIFIER),
..Default::default()
},
),

View File

@@ -3,14 +3,10 @@ use glam::{DAffine2, DVec2, IVec2};
use graph_craft::document::DocumentNode;
use graph_craft::document::value::RenderOutput;
use graph_craft::proto::{NodeConstructor, TypeErasedBox};
use graphene_std::Artboard;
use graphene_std::Context;
use graphene_std::Graphic;
use graphene_std::any::DynAnyNode;
use graphene_std::application_io::{ImageTexture, SurfaceFrame};
use graphene_std::brush::brush_cache::BrushCache;
use graphene_std::brush::brush_stroke::BrushStroke;
use graphene_std::concrete;
use graphene_std::gradient::GradientStops;
#[cfg(feature = "gpu")]
use graphene_std::raster::GPU;
@@ -25,9 +21,7 @@ use graphene_std::vector::Vector;
use graphene_std::wasm_application_io::WasmEditorApi;
#[cfg(feature = "gpu")]
use graphene_std::wasm_application_io::WasmSurfaceHandle;
use graphene_std::{Cow, ProtoNodeIdentifier};
use graphene_std::{NodeIO, NodeIOTypes};
use graphene_std::{fn_type_fut, future};
use graphene_std::{Artboard, Context, Graphic, NodeIO, NodeIOTypes, ProtoNodeIdentifier, concrete, fn_type_fut, future};
use node_registry_macros::{async_node, convert_node, into_node};
use once_cell::sync::Lazy;
use std::collections::HashMap;
@@ -269,11 +263,11 @@ fn node_registry() -> HashMap<ProtoNodeIdentifier, HashMap<NodeIOTypes, NodeCons
}
}
for (id, c, types) in node_types.into_iter() {
for (id, node_constructor, types) in node_types.into_iter() {
// TODO: this is a hack to remove the newline from the node new_name
// This occurs for the ChannelMixerNode presumably because of the long name.
// This might be caused by the stringify! macro
let mut new_name = id.name.replace('\n', " ");
let mut new_name = id.as_str().replace('\n', " ");
// Remove struct generics for all nodes except for the IntoNode and ConvertNode
if !(new_name.contains("IntoNode") || new_name.contains("ConvertNode"))
@@ -282,8 +276,7 @@ fn node_registry() -> HashMap<ProtoNodeIdentifier, HashMap<NodeIOTypes, NodeCons
new_name = path.to_string();
}
let nid = ProtoNodeIdentifier { name: Cow::Owned(new_name) };
map.entry(nid).or_default().insert(types.clone(), c);
map.entry(ProtoNodeIdentifier::with_owned_string(new_name)).or_default().insert(types.clone(), node_constructor);
}
map

View File

@@ -85,7 +85,7 @@ pub fn wrap_network_in_scope(mut network: NodeNetwork, editor_api: Arc<WasmEdito
if cfg!(feature = "gpu") {
nodes.push(DocumentNode {
implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::from("graphene_core::ops::IntoNode<&WgpuExecutor>")),
implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_core::ops::IntoNode<&WgpuExecutor>")),
inputs: vec![NodeInput::node(NodeId(2), 0)],
..Default::default()
});

View File

@@ -1,13 +1,12 @@
use crate::{ContextFeature, Node, NodeIO, NodeIOTypes, ProtoNodeIdentifier, Type, WasmNotSend};
use dyn_any::{DynAny, StaticType};
pub use no_std_types::registry::types;
use std::collections::HashMap;
use std::marker::PhantomData;
use std::ops::Deref;
use std::pin::Pin;
use std::sync::{LazyLock, Mutex};
pub use no_std_types::registry::types;
// Translation struct between macro and definition
#[derive(Clone, Debug)]
pub struct NodeMetadata {

View File

@@ -1,8 +1,6 @@
use std::any::TypeId;
pub use std::borrow::Cow;
use std::fmt::{Display, Formatter};
use std::ops::Deref;
#[macro_export]
macro_rules! concrete {
@@ -128,19 +126,7 @@ impl std::fmt::Debug for NodeIOTypes {
#[derive(Clone, Debug, PartialEq, Eq, Hash, specta::Type, serde::Serialize, serde::Deserialize)]
pub struct ProtoNodeIdentifier {
pub name: Cow<'static, str>,
}
impl From<String> for ProtoNodeIdentifier {
fn from(value: String) -> Self {
Self { name: Cow::Owned(value) }
}
}
impl From<&'static str> for ProtoNodeIdentifier {
fn from(s: &'static str) -> Self {
ProtoNodeIdentifier { name: Cow::Borrowed(s) }
}
name: Cow<'static, str>,
}
impl ProtoNodeIdentifier {
@@ -151,12 +137,8 @@ impl ProtoNodeIdentifier {
pub const fn with_owned_string(name: String) -> Self {
ProtoNodeIdentifier { name: Cow::Owned(name) }
}
}
impl Deref for ProtoNodeIdentifier {
type Target = str;
fn deref(&self) -> &Self::Target {
pub fn as_str(&self) -> &str {
self.name.as_ref()
}
}

View File

@@ -117,7 +117,7 @@ fn validate_implementations_for_generics(parsed: &ParsedNodeFn) {
quote!(#ty),
pat_ident.ident;
help = "Add #[implementations(ConcreteType1, ConcreteType2)] to field '{}'", pat_ident.ident;
help = "Or use #[skip_impl] if you want to manually implement the node"
help = "Or use #[node_macro::node(skip_impl)] if you want to manually implement the node"
);
}
}
@@ -133,7 +133,7 @@ fn validate_implementations_for_generics(parsed: &ParsedNodeFn) {
"Generic types in Node field `{}` require an #[implementations(...)] attribute",
pat_ident.ident;
help = "Add #[implementations(InputType1 -> OutputType1, InputType2 -> OutputType2)] to field '{}'", pat_ident.ident;
help = "Or use #[skip_impl] if you want to manually implement the node"
help = "Or use #[node_macro::node(skip_impl)] if you want to manually implement the node"
);
}
// Additional check for Node implementations

View File

@@ -4,7 +4,7 @@ use std::marker::PhantomData;
// Re-export TypeNode from core-types for convenience
pub use core_types::ops::TypeNode;
// TODO: Rename to "Passthrough"
// TODO: Rename to "Passthrough" and make this the node that users use, not the one defined in document_node_definitions.rs
/// Passes-through the input value without changing it.
/// This is useful for rerouting wires for organization purposes.
#[node_macro::node(skip_impl)]

View File

@@ -810,7 +810,7 @@ fn dot_product(
/// An operand of the dot product operation.
vector_a: DVec2,
/// The other operand of the dot product operation.
#[default((1., 0.))]
#[default(1., 0.)]
vector_b: DVec2,
/// Whether to normalize both input vectors so the calculation ranges in `[-1, 1]` by considering only their degree of directional alignment.
normalize: bool,

View File

@@ -69,17 +69,13 @@ pub fn generate_node_substitutions() -> HashMap<ProtoNodeIdentifier, DocumentNod
let input_ty = input.nested_type();
let mut inputs = vec![NodeInput::import(input.clone(), i)];
let into_node_identifier = ProtoNodeIdentifier {
name: format!("graphene_core::ops::IntoNode<{}>", input_ty.clone()).into(),
};
let convert_node_identifier = ProtoNodeIdentifier {
name: format!("graphene_core::ops::ConvertNode<{}>", input_ty.clone()).into(),
};
let into_node_identifier = ProtoNodeIdentifier::with_owned_string(format!("graphene_core::ops::IntoNode<{}>", input_ty.clone()));
let convert_node_identifier = ProtoNodeIdentifier::with_owned_string(format!("graphene_core::ops::ConvertNode<{}>", input_ty.clone()));
let proto_node = if into_node_registry.keys().any(|ident: &ProtoNodeIdentifier| ident.name.as_ref() == into_node_identifier.name.as_ref()) {
let proto_node = if into_node_registry.keys().any(|ident: &ProtoNodeIdentifier| ident.as_str() == into_node_identifier.as_str()) {
generated_nodes += 1;
into_node_identifier
} else if into_node_registry.keys().any(|ident| ident.name.as_ref() == convert_node_identifier.name.as_ref()) {
} else if into_node_registry.keys().any(|ident| ident.as_str() == convert_node_identifier.as_str()) {
generated_nodes += 1;
inputs.push(NodeInput::value(TaggedValue::None, false));
convert_node_identifier
@@ -162,7 +158,7 @@ pub fn node_inputs(fields: &[registry::FieldMetadata], first_node_io: &NodeIOTyp
return NodeInput::value(custom_default, exposed);
} else {
// It is incredibly useful to get a warning when the default type cannot be parsed rather than defaulting to `()`.
warn!("Failed to parse default value for type {ty:?} with data {data}");
warn!("Failed to parse default value for type `{ty:?}` with data `{data}`");
}
}
RegistryValueSource::Scope(data) => return NodeInput::scope(Cow::Borrowed(data)),