mirrors/Graphite
1
0
Fork 0
mirror of https://github.com/GraphiteEditor/Graphite.git synced 2025-07-08 00:05:00 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

Remove subtyping for () from node graph type system (#2418)

Browse source 
* Remove subtyping for () from node graph type system

* Remove special-case in DynAnyNode for downcasting to ()

* Correct input type for CreateGpuSurfaceNode

* Remove unncessary imports

---------

Co-authored-by: Dennis Kobert <dennis@kobert.dev>
Co-authored-by: Keavon Chambers <keavon@keavon.com>
Co-authored-by: hypercube <0hypercube@gmail.com>
This commit is contained in:
bradrn 2025-03-27 08:26:07 +11:00 • committed by GitHub
parent dd27f4653d
commit 92132919d1
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 9 additions and 23 deletions
Download patch file Download diff file Expand all files Collapse all files

16
node-graph/gcore/src/registry.rs
View file

@ -1,4 +1,3 @@
use crate::transform::Footprint;
use crate::{Node, NodeIO, NodeIOTypes, Type, WasmNotSend};
use dyn_any::{DynAny, StaticType};
use std::collections::HashMap;
@ -251,21 +250,6 @@ where
};
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(&()) }))
}
// If the Node expects a footprint but we provide (). In this case construct the default Footprint and pass that
// This is pretty hacky pls fix
Err(_) if core::any::TypeId::of::<_I::Static>() == core::any::TypeId::of::<Footprint>() => {
assert_eq!(std::mem::size_of::<_I>(), std::mem::size_of::<Footprint>());
assert_eq!(std::mem::align_of::<_I>(), std::mem::align_of::<Footprint>());
// Rust can't know, that `_I` and `Footprint` are the same size, so we have to use a `transmute_copy()` here
Box::pin(output(unsafe { std::mem::transmute_copy(&Footprint::default()) }))
}
Err(e) => panic!("DynAnyNode Input, {0} in:\n{1}", e, node_name),
}
}

12
node-graph/graph-craft/src/proto.rs
View file

@ -691,13 +691,14 @@ impl TypingContext {
/// Checks if a proposed input to a particular (primary or secondary) input connector 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 {
fn valid_type(from: &Type, to: &Type) -> bool {
match (from, to) {
// Direct comparison of two concrete types.
(Type::Concrete(type1), Type::Concrete(type2)) => type1 == type2,
// Check inner type for futures
(Type::Future(type1), Type::Future(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.
// Direct comparison of two function types.
// Note: in the presence of subtyping, 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 `()`.
@ -706,8 +707,9 @@ impl TypingContext {
// - `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: <https://doc.rust-lang.org/nomicon/subtyping.html>
// Graphite doesn't have subtyping currently, but it used to have it, and may do so again, so we make sure to compare types in this way to make things easier.
// More details explained here: <https://github.com/GraphiteEditor/Graphite/issues/1741>
(Type::Fn(in1, out1), Type::Fn(in2, out2)) => valid_subtype(out2, out1) && (valid_subtype(in1, in2) || **in1 == concrete!(())),
(Type::Fn(in1, out1), Type::Fn(in2, out2)) => valid_type(out2, out1) && valid_type(in1, in2),
// 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,
@ -719,7 +721,7 @@ impl TypingContext {
// List of all implementations that match the input types
let valid_output_types = impls
.keys()
.filter(|node_io| valid_subtype(&node_io.call_argument, &primary_input_or_call_argument) && inputs.iter().zip(node_io.inputs.iter()).all(|(p1, p2)| valid_subtype(p1, p2)))
.filter(|node_io| valid_type(&node_io.call_argument, &primary_input_or_call_argument) && inputs.iter().zip(node_io.inputs.iter()).all(|(p1, p2)| valid_type(p1, p2)))
.collect::<Vec<_>>();
// Attempt to substitute generic types with concrete types and save the list of results
@ -753,7 +755,7 @@ impl TypingContext {
.cloned()
.zip([&node_io.call_argument].into_iter().chain(&node_io.inputs).cloned())
.enumerate()
.filter(|(_, (p1, p2))| !valid_subtype(p1, p2))
.filter(|(_, (p1, p2))| !valid_type(p1, p2))
.map(|(index, ty)| {
let i = node.original_location.inputs(index).min_by_key(|s| s.node.len()).map(|s| s.index).unwrap_or(index);
let i = if using_manual_composition { i } else { i + 1 };

4
node-graph/interpreted-executor/src/node_registry.rs
View file

@ -88,9 +88,9 @@ fn node_registry() -> HashMap<ProtoNodeIdentifier, HashMap<NodeIOTypes, NodeCons
ProtoNodeIdentifier::new(stringify!(wgpu_executor::CreateGpuSurfaceNode<_>)),
|args| {
Box::pin(async move {
let editor_api: DowncastBothNode<(), &WasmEditorApi> = DowncastBothNode::new(args[0].clone());
let editor_api: DowncastBothNode<Context, &WasmEditorApi> = DowncastBothNode::new(args[0].clone());
let node = <wgpu_executor::CreateGpuSurfaceNode<_>>::new(editor_api);
let any: DynAnyNode<(), _, _> = graphene_std::any::DynAnyNode::new(node);
let any: DynAnyNode<Context, _, _> = graphene_std::any::DynAnyNode::new(node);
Box::new(any) as TypeErasedBox
})
},