From ce96ae66f2103a7803a39e82f67cbb82cb5791a9 Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Wed, 8 May 2024 01:36:25 +0200 Subject: [PATCH] Loosen the Graphene type system to allow contravariant function arguments (#1740) * Accept any input for nodes that expect () as input * Add comments * More comments --------- Co-authored-by: Keavon Chambers --- .../node_graph/document_node_types.rs | 289 +----------------- node-graph/graph-craft/src/document.rs | 2 +- node-graph/graph-craft/src/proto.rs | 30 +- node-graph/gstd/src/any.rs | 24 +- shell.nix | 2 +- 5 files changed, 54 insertions(+), 293 deletions(-) diff --git a/editor/src/messages/portfolio/document/node_graph/document_node_types.rs b/editor/src/messages/portfolio/document/node_graph/document_node_types.rs index 3ea1a5ce3..59deee957 100644 --- a/editor/src/messages/portfolio/document/node_graph/document_node_types.rs +++ b/editor/src/messages/portfolio/document/node_graph/document_node_types.rs @@ -326,7 +326,7 @@ fn static_nodes() -> Vec { category: "Structural", implementation: DocumentNodeImplementation::Network(NodeNetwork { imports: vec![NodeId(0), NodeId(0)], - exports: vec![NodeOutput::new(NodeId(2), 0)], + exports: vec![NodeOutput::new(NodeId(1), 0)], nodes: [ DocumentNode { name: "Load Resource".to_string(), @@ -340,13 +340,6 @@ fn static_nodes() -> Vec { implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_std::wasm_application_io::DecodeImageNode")), ..Default::default() }, - DocumentNode { - name: "Cull".to_string(), - inputs: vec![NodeInput::node(NodeId(1), 0)], - implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_core::transform::CullNode<_>")), - manual_composition: Some(concrete!(Footprint)), - ..Default::default() - }, ] .into_iter() .enumerate() @@ -646,64 +639,7 @@ fn static_nodes() -> Vec { DocumentNodeDefinition { name: "Noise Pattern", category: "General", - implementation: DocumentNodeImplementation::Network(NodeNetwork { - imports: vec![ - NodeId(0), - NodeId(0), - NodeId(0), - NodeId(0), - NodeId(0), - NodeId(0), - NodeId(0), - NodeId(0), - NodeId(0), - NodeId(0), - NodeId(0), - NodeId(0), - NodeId(0), - NodeId(0), - NodeId(0), - NodeId(0), - ], - exports: vec![NodeOutput::new(NodeId(1), 0)], - nodes: vec![ - DocumentNode { - name: "Noise Pattern".to_string(), - inputs: vec![ - NodeInput::Network(concrete!(())), - NodeInput::Network(concrete!(UVec2)), - NodeInput::Network(concrete!(u32)), - NodeInput::Network(concrete!(f64)), - NodeInput::Network(concrete!(graphene_core::raster::NoiseType)), - NodeInput::Network(concrete!(graphene_core::raster::FractalType)), - NodeInput::Network(concrete!(f64)), - NodeInput::Network(concrete!(graphene_core::raster::FractalType)), - NodeInput::Network(concrete!(u32)), - NodeInput::Network(concrete!(f64)), - NodeInput::Network(concrete!(f64)), - NodeInput::Network(concrete!(f64)), - NodeInput::Network(concrete!(f64)), - NodeInput::Network(concrete!(graphene_core::raster::CellularDistanceFunction)), - NodeInput::Network(concrete!(graphene_core::raster::CellularReturnType)), - NodeInput::Network(concrete!(f64)), - ], - implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_std::raster::NoisePatternNode<_, _, _, _, _, _, _, _, _, _, _, _, _, _, _>")), - ..Default::default() - }, - DocumentNode { - name: "Cull".to_string(), - inputs: vec![NodeInput::node(NodeId(0), 0)], - implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_core::transform::CullNode<_>")), - manual_composition: Some(concrete!(Footprint)), - ..Default::default() - }, - ] - .into_iter() - .enumerate() - .map(|(id, node)| (NodeId(id as u64), node)) - .collect(), - ..Default::default() - }), + implementation: DocumentNodeImplementation::proto("graphene_std::raster::NoisePatternNode<_, _, _, _, _, _, _, _, _, _, _, _, _, _, _>"), inputs: vec![ DocumentInputType::value("None", TaggedValue::None, false), // All @@ -945,6 +881,7 @@ fn static_nodes() -> Vec { // The input image feeds into the identity, then we take its passed-through value when the other channels are reading from it instead of the original input. // We do this for technical restrictions imposed by Graphene which doesn't allow an input to feed into multiple interior nodes in the subgraph. // Diagram: + // TODO: Remove this limitation by either making the `imports` above into a double-vec or making each of these DocumentNodes request their imported data based on its index. DocumentNode { name: "Identity".to_string(), inputs: vec![NodeInput::Network(concrete!(ImageFrame))], @@ -2230,30 +2167,7 @@ fn static_nodes() -> Vec { DocumentNodeDefinition { name: "Ellipse", category: "Vector", - implementation: DocumentNodeImplementation::Network(NodeNetwork { - imports: vec![NodeId(0), NodeId(0), NodeId(0)], - exports: vec![NodeOutput::new(NodeId(1), 0)], - nodes: vec![ - DocumentNode { - name: "Ellipse Generator".to_string(), - inputs: vec![NodeInput::Network(concrete!(())), NodeInput::Network(concrete!(f64)), NodeInput::Network(concrete!(f64))], - implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_core::vector::generator_nodes::EllipseGenerator<_, _>")), - ..Default::default() - }, - DocumentNode { - name: "Cull".to_string(), - inputs: vec![NodeInput::node(NodeId(0), 0)], - implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_core::transform::CullNode<_>")), - manual_composition: Some(concrete!(Footprint)), - ..Default::default() - }, - ] - .into_iter() - .enumerate() - .map(|(id, node)| (NodeId(id as u64), node)) - .collect(), - ..Default::default() - }), + implementation: DocumentNodeImplementation::proto("graphene_core::vector::generator_nodes::EllipseGenerator<_, _>"), inputs: vec![ DocumentInputType::none(), DocumentInputType::value("Radius X", TaggedValue::F64(50.), false), @@ -2266,37 +2180,7 @@ fn static_nodes() -> Vec { DocumentNodeDefinition { name: "Rectangle", category: "Vector", - implementation: DocumentNodeImplementation::Network(NodeNetwork { - imports: vec![NodeId(0), NodeId(0), NodeId(0), NodeId(0), NodeId(0), NodeId(0)], - exports: vec![NodeOutput::new(NodeId(1), 0)], - nodes: vec![ - DocumentNode { - name: "Rectangle Generator".to_string(), - inputs: vec![ - NodeInput::Network(concrete!(())), - NodeInput::Network(concrete!(f64)), - NodeInput::Network(concrete!(f64)), - NodeInput::Network(concrete!(bool)), - NodeInput::Network(generic!(T)), - NodeInput::Network(concrete!(bool)), - ], - implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_core::vector::generator_nodes::RectangleGenerator<_, _, _, _, _>")), - ..Default::default() - }, - DocumentNode { - name: "Cull".to_string(), - inputs: vec![NodeInput::node(NodeId(0), 0)], - implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_core::transform::CullNode<_>")), - manual_composition: Some(concrete!(Footprint)), - ..Default::default() - }, - ] - .into_iter() - .enumerate() - .map(|(id, node)| (NodeId(id as u64), node)) - .collect(), - ..Default::default() - }), + implementation: DocumentNodeImplementation::proto("graphene_core::vector::generator_nodes::RectangleGenerator<_, _, _, _, _>"), inputs: vec![ DocumentInputType::none(), DocumentInputType::value("Size X", TaggedValue::F64(100.), false), @@ -2312,30 +2196,7 @@ fn static_nodes() -> Vec { DocumentNodeDefinition { name: "Regular Polygon", category: "Vector", - implementation: DocumentNodeImplementation::Network(NodeNetwork { - imports: vec![NodeId(0), NodeId(0), NodeId(0)], - exports: vec![NodeOutput::new(NodeId(1), 0)], - nodes: vec![ - DocumentNode { - name: "Regular Polygon Generator".to_string(), - inputs: vec![NodeInput::Network(concrete!(())), NodeInput::Network(concrete!(u32)), NodeInput::Network(concrete!(f64))], - implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_core::vector::generator_nodes::RegularPolygonGenerator<_, _>")), - ..Default::default() - }, - DocumentNode { - name: "Cull".to_string(), - inputs: vec![NodeInput::node(NodeId(0), 0)], - implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_core::transform::CullNode<_>")), - manual_composition: Some(concrete!(Footprint)), - ..Default::default() - }, - ] - .into_iter() - .enumerate() - .map(|(id, node)| (NodeId(id as u64), node)) - .collect(), - ..Default::default() - }), + implementation: DocumentNodeImplementation::proto("graphene_core::vector::generator_nodes::RegularPolygonGenerator<_, _>"), inputs: vec![ DocumentInputType::none(), DocumentInputType::value("Sides", TaggedValue::U32(6), false), @@ -2348,35 +2209,7 @@ fn static_nodes() -> Vec { DocumentNodeDefinition { name: "Star", category: "Vector", - implementation: DocumentNodeImplementation::Network(NodeNetwork { - imports: vec![NodeId(0), NodeId(0), NodeId(0), NodeId(0)], - exports: vec![NodeOutput::new(NodeId(1), 0)], - nodes: vec![ - DocumentNode { - name: "Star Generator".to_string(), - inputs: vec![ - NodeInput::Network(concrete!(())), - NodeInput::Network(concrete!(u32)), - NodeInput::Network(concrete!(f64)), - NodeInput::Network(concrete!(f64)), - ], - implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_core::vector::generator_nodes::StarGenerator<_, _, _>")), - ..Default::default() - }, - DocumentNode { - name: "Cull".to_string(), - inputs: vec![NodeInput::node(NodeId(0), 0)], - implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_core::transform::CullNode<_>")), - manual_composition: Some(concrete!(Footprint)), - ..Default::default() - }, - ] - .into_iter() - .enumerate() - .map(|(id, node)| (NodeId(id as u64), node)) - .collect(), - ..Default::default() - }), + implementation: DocumentNodeImplementation::proto("graphene_core::vector::generator_nodes::StarGenerator<_, _, _>"), inputs: vec![ DocumentInputType::none(), DocumentInputType::value("Sides", TaggedValue::U32(5), false), @@ -2390,30 +2223,7 @@ fn static_nodes() -> Vec { DocumentNodeDefinition { name: "Line", category: "Vector", - implementation: DocumentNodeImplementation::Network(NodeNetwork { - imports: vec![NodeId(0), NodeId(0), NodeId(0)], - exports: vec![NodeOutput::new(NodeId(1), 0)], - nodes: vec![ - DocumentNode { - name: "Line Generator".to_string(), - inputs: vec![NodeInput::Network(concrete!(())), NodeInput::Network(concrete!(DVec2)), NodeInput::Network(concrete!(DVec2))], - implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_core::vector::generator_nodes::LineGenerator<_, _>")), - ..Default::default() - }, - DocumentNode { - name: "Cull".to_string(), - inputs: vec![NodeInput::node(NodeId(0), 0)], - implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_core::transform::CullNode<_>")), - manual_composition: Some(concrete!(Footprint)), - ..Default::default() - }, - ] - .into_iter() - .enumerate() - .map(|(id, node)| (NodeId(id as u64), node)) - .collect(), - ..Default::default() - }), + implementation: DocumentNodeImplementation::proto("graphene_core::vector::generator_nodes::LineGenerator<_, _>"), inputs: vec![ DocumentInputType::none(), DocumentInputType::value("Start", TaggedValue::DVec2(DVec2::new(0., -50.)), false), @@ -2426,30 +2236,7 @@ fn static_nodes() -> Vec { DocumentNodeDefinition { name: "Spline", category: "Vector", - implementation: DocumentNodeImplementation::Network(NodeNetwork { - imports: vec![NodeId(0), NodeId(0)], - exports: vec![NodeOutput::new(NodeId(1), 0)], - nodes: vec![ - DocumentNode { - name: "Spline Generator".to_string(), - inputs: vec![NodeInput::Network(concrete!(())), NodeInput::Network(concrete!(Vec))], - implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_core::vector::generator_nodes::SplineGenerator<_>")), - ..Default::default() - }, - DocumentNode { - name: "Cull".to_string(), - inputs: vec![NodeInput::node(NodeId(0), 0)], - implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_core::transform::CullNode<_>")), - manual_composition: Some(concrete!(Footprint)), - ..Default::default() - }, - ] - .into_iter() - .enumerate() - .map(|(id, node)| (NodeId(id as u64), node)) - .collect(), - ..Default::default() - }), + implementation: DocumentNodeImplementation::proto("graphene_core::vector::generator_nodes::SplineGenerator<_>"), inputs: vec![ DocumentInputType::none(), DocumentInputType::value("Points", TaggedValue::VecDVec2(vec![DVec2::new(0., -50.), DVec2::new(25., 0.), DVec2::new(0., 50.)]), false), @@ -2461,33 +2248,7 @@ fn static_nodes() -> Vec { DocumentNodeDefinition { name: "Shape", category: "Vector", - implementation: DocumentNodeImplementation::Network(NodeNetwork { - imports: vec![NodeId(0), NodeId(0)], - exports: vec![NodeOutput::new(NodeId(1), 0)], - nodes: vec![ - DocumentNode { - name: "Path Generator".to_string(), - inputs: vec![ - NodeInput::Network(concrete!(Vec>)), - NodeInput::Network(concrete!(Vec)), - ], - implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_core::vector::generator_nodes::PathGenerator<_>")), - ..Default::default() - }, - DocumentNode { - name: "Cull".to_string(), - inputs: vec![NodeInput::node(NodeId(0), 0)], - implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_core::transform::CullNode<_>")), - manual_composition: Some(concrete!(Footprint)), - ..Default::default() - }, - ] - .into_iter() - .enumerate() - .map(|(id, node)| (NodeId(id as u64), node)) - .collect(), - ..Default::default() - }), + implementation: DocumentNodeImplementation::proto("graphene_core::vector::generator_nodes::PathGenerator<_>"), inputs: vec![ DocumentInputType::value("Path Data", TaggedValue::Subpaths(vec![]), false), DocumentInputType::value("Colinear Manipulators", TaggedValue::ManipulatorGroupIds(vec![]), false), @@ -2525,35 +2286,7 @@ fn static_nodes() -> Vec { DocumentNodeDefinition { name: "Text", category: "Vector", - implementation: DocumentNodeImplementation::Network(NodeNetwork { - imports: vec![NodeId(0), NodeId(0), NodeId(0), NodeId(0)], - exports: vec![NodeOutput::new(NodeId(1), 0)], - nodes: vec![ - DocumentNode { - name: "Text Generator".to_string(), - inputs: vec![ - NodeInput::Network(concrete!(application_io::EditorApi)), - NodeInput::Network(concrete!(String)), - NodeInput::Network(concrete!(graphene_core::text::Font)), - NodeInput::Network(concrete!(f64)), - ], - implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_core::text::TextGeneratorNode<_, _, _>")), - ..Default::default() - }, - DocumentNode { - name: "Cull".to_string(), - inputs: vec![NodeInput::node(NodeId(0), 0)], - implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_core::transform::CullNode<_>")), - manual_composition: Some(concrete!(Footprint)), - ..Default::default() - }, - ] - .into_iter() - .enumerate() - .map(|(id, node)| (NodeId(id as u64), node)) - .collect(), - ..Default::default() - }), + implementation: DocumentNodeImplementation::proto("graphene_core::text::TextGeneratorNode<_, _, _>"), inputs: vec![ DocumentInputType::none(), DocumentInputType::value("Text", TaggedValue::String("Lorem ipsum".to_string()), false), diff --git a/node-graph/graph-craft/src/document.rs b/node-graph/graph-craft/src/document.rs index 63b3059ac..e5972ab90 100644 --- a/node-graph/graph-craft/src/document.rs +++ b/node-graph/graph-craft/src/document.rs @@ -969,7 +969,7 @@ impl NodeNetwork { return; } - // replace value inputs with value nodes + // Replace value inputs with value nodes for input in node.inputs.iter_mut() { // Skip inputs that are already value nodes if node.implementation == DocumentNodeImplementation::ProtoNode("graphene_core::value::ClonedNode".into()) { diff --git a/node-graph/graph-craft/src/proto.rs b/node-graph/graph-craft/src/proto.rs index 89553f313..50c0cd5a5 100644 --- a/node-graph/graph-craft/src/proto.rs +++ b/node-graph/graph-craft/src/proto.rs @@ -735,14 +735,28 @@ impl TypingContext { }) { return Err(vec![GraphError::new(node, GraphErrorType::UnexpectedGenerics { index, parameters })]); } - fn covariant(from: &Type, to: &Type) -> bool { + + /// Checks if a proposed input to a particular (primary or secondary) input is valid for its type signature. + /// `from` indicates the value given to a input, `to` indicates the input's allowed type as specified by its type signature. + fn valid_subtype(from: &Type, to: &Type) -> bool { match (from, to) { - (Type::Concrete(t1), Type::Concrete(t2)) => t1 == t2, - (Type::Fn(a1, b1), Type::Fn(a2, b2)) => covariant(a1, a2) && covariant(b1, b2), + // Direct comparison of two concrete types. + (Type::Concrete(type1), Type::Concrete(type2)) => type1 == type2, + // Loose comparison of function types, where loose means that functions are considered on a "greater than or equal to" basis of its function type's generality. + // That means we compare their types with a contravariant relationship, which means that a more general type signature may be substituted for a more specific type signature. + // For example, we allow `T -> V` to be substituted with `T' -> V` or `() -> V` where T' and () are more specific than T. + // This allows us to supply anything to a function that is satisfied with `()`. + // In other words, we are implementing these two relations, where the >= operator means that the left side is more general than the right side: + // - `T >= T' ⇒ (T' -> V) >= (T -> V)` (functions are contravariant in their input types) + // - `V >= V' ⇒ (T -> V) >= (T -> V')` (functions are covariant in their output types) + // While these two relations aren't a truth about the universe, they are a design decision that we are employing in our language design that is also common in other languages. + // For example, Rust implements these same relations as it describes here: + // More details explained here: + (Type::Fn(in1, out1), Type::Fn(in2, out2)) => valid_subtype(out1, out2) && (valid_subtype(in1, in2) || **in1 == concrete!(())), + // If either the proposed input or the allowed input are generic, we allow the substitution (meaning this is a valid subtype). // TODO: Add proper generic counting which is not based on the name - (Type::Generic(_), Type::Generic(_)) => true, - (Type::Generic(_), _) => true, - (_, Type::Generic(_)) => true, + (Type::Generic(_), _) | (_, Type::Generic(_)) => true, + // Reject unknown type relationships. _ => false, } } @@ -750,7 +764,7 @@ impl TypingContext { // List of all implementations that match the input and parameter types let valid_output_types = impls .keys() - .filter(|node_io| covariant(&input, &node_io.input) && parameters.iter().zip(node_io.parameters.iter()).all(|(p1, p2)| covariant(p1, p2))) + .filter(|node_io| valid_subtype(&input, &node_io.input) && parameters.iter().zip(node_io.parameters.iter()).all(|(p1, p2)| valid_subtype(p1, p2))) .collect::>(); // Attempt to substitute generic types with concrete types and save the list of results @@ -785,7 +799,7 @@ impl TypingContext { .cloned() .zip([&node_io.input].into_iter().chain(&node_io.parameters).cloned()) .enumerate() - .filter(|(_, (p1, p2))| !covariant(p1, p2)) + .filter(|(_, (p1, p2))| !valid_subtype(p1, p2)) .map(|(index, ty)| (node.original_location.inputs(index).min_by_key(|s| s.node.len()).map(|s| s.index).unwrap_or(index), ty)) .collect::>(); if current_errors.len() < best_errors { diff --git a/node-graph/gstd/src/any.rs b/node-graph/gstd/src/any.rs index 220a6eb80..ec0e96ed9 100644 --- a/node-graph/gstd/src/any.rs +++ b/node-graph/gstd/src/any.rs @@ -1,8 +1,10 @@ -use dyn_any::StaticType; pub use graph_craft::proto::{Any, NodeContainer, TypeErasedBox, TypeErasedNode}; use graph_craft::proto::{DynFuture, FutureAny, SharedNodeContainer}; use graphene_core::NodeIO; pub use graphene_core::{generic, ops, Node}; + +use dyn_any::StaticType; + use std::marker::PhantomData; pub struct DynAnyNode { @@ -19,12 +21,21 @@ where #[inline] fn eval(&'input self, input: Any<'input>) -> Self::Output { let node_name = core::any::type_name::(); - let input: Box<_I> = dyn_any::downcast(input).unwrap_or_else(|e| panic!("DynAnyNode Input, {0} in:\n{1}", e, node_name)); - let output = async move { - let result = self.node.eval(*input).await; + let output = |input| async move { + let result = self.node.eval(input).await; Box::new(result) as Any<'input> }; - Box::pin(output) + match dyn_any::downcast(input) { + Ok(input) => Box::pin(output(*input)), + // If the input type of the node is `()` and we supply an invalid type, we can still call the + // node and just ignore the input and call it with the unit type instead. + Err(_) if core::any::TypeId::of::<_I::Static>() == core::any::TypeId::of::<()>() => { + assert_eq!(std::mem::size_of::<_I>(), 0); + // Rust can't know, that `_I` and `()` are the same size, so we have to use a `transmute_copy()` here + Box::pin(output(unsafe { std::mem::transmute_copy(&()) })) + } + Err(e) => panic!("DynAnyNode Input, {0} in:\n{1}", e, node_name), + } } fn reset(&self) { @@ -67,6 +78,9 @@ where fn reset(&self) { self.node.reset(); } + fn serialize(&self) -> Option> { + self.node.serialize() + } } impl<_I, _O, S0> DynAnyRefNode<_I, _O, S0> { diff --git a/shell.nix b/shell.nix index d21fc3a07..07241e687 100644 --- a/shell.nix +++ b/shell.nix @@ -31,7 +31,7 @@ let rustc-wasm = pkgs.rust-bin.stable.latest.default.override { targets = [ "wasm32-unknown-unknown" ]; # wasm-pack needs this - extensions = [ "rust-src" ]; + extensions = [ "rust-src" "rust-analyzer" "clippy" ]; }; in # Make a shell with the dependencies we need