Add tests to the Ellipse, Artboard, and Fill tools (#2181)

* Add ellipse tests

* Add tests for fill tool and re-enable some other tests

* Code review

* Fix Rust crate advisory

---------

Co-authored-by: Keavon Chambers <keavon@keavon.com>
This commit is contained in:
James Lindsay 2025-03-07 02:13:15 +00:00 committed by GitHub
parent 1190e82322
commit b171eeba84
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
22 changed files with 1097 additions and 236 deletions

View file

@ -72,7 +72,7 @@ pub trait Node<'i, Input> {
}
/// Serialize the node which is used for the `introspect` function which can retrieve values from monitor nodes.
#[cfg(feature = "std")]
fn serialize(&self) -> Option<std::sync::Arc<dyn core::any::Any>> {
fn serialize(&self) -> Option<std::sync::Arc<dyn core::any::Any + Send + Sync>> {
log::warn!("Node::serialize not implemented for {}", core::any::type_name::<Self>());
None
}
@ -170,3 +170,25 @@ pub use crate::application_io::{SurfaceFrame, SurfaceId};
pub type WasmSurfaceHandle = application_io::SurfaceHandle<web_sys::HtmlCanvasElement>;
#[cfg(feature = "wasm")]
pub type WasmSurfaceHandleFrame = application_io::SurfaceHandleFrame<web_sys::HtmlCanvasElement>;
pub trait InputAccessorSource<'a, T>: InputAccessorSourceIdentifier + core::fmt::Debug {
fn get_input(&'a self, index: usize) -> Option<&'a T>;
fn set_input(&'a mut self, index: usize, value: T);
}
pub trait InputAccessorSourceIdentifier {
fn has_identifier(&self, identifier: &str) -> bool;
}
pub trait InputAccessor<'n, Source: 'n>
where
Self: Sized,
{
fn new_with_source(source: &'n Source) -> Option<Self>;
}
pub trait NodeInputDecleration {
const INDEX: usize;
fn identifier() -> &'static str;
type Result;
}

View file

@ -134,9 +134,9 @@ where
})
}
fn serialize(&self) -> Option<Arc<dyn core::any::Any>> {
fn serialize(&self) -> Option<Arc<dyn core::any::Any + Send + Sync>> {
let io = self.io.lock().unwrap();
(io).as_ref().map(|output| output.clone() as Arc<dyn core::any::Any>)
(io).as_ref().map(|output| output.clone() as Arc<dyn core::any::Any + Send + Sync>)
}
}

View file

@ -506,7 +506,7 @@ where
self.0.reset();
}
fn serialize(&self) -> Option<std::sync::Arc<dyn core::any::Any>> {
fn serialize(&self) -> Option<std::sync::Arc<dyn core::any::Any + Send + Sync>> {
self.0.serialize()
}
}
@ -581,4 +581,29 @@ mod test {
let fnn = FnNode::new(|(a, b)| (b, a));
assert_eq!(fnn.eval((1u32, 2u32)), (2, 1));
}
#[test]
pub fn add_vectors() {
assert_eq!(super::add((), DVec2::ONE, DVec2::ONE), DVec2::ONE * 2.);
}
#[test]
pub fn subtract_f64() {
assert_eq!(super::subtract((), 5_f64, 3_f64), 2.);
}
#[test]
pub fn divide_vectors() {
assert_eq!(super::divide((), DVec2::ONE, 2_f64), DVec2::ONE / 2.);
}
#[test]
pub fn modulo_positive() {
assert_eq!(super::modulo((), -5_f64, 2_f64, true), 1_f64);
}
#[test]
pub fn modulo_negative() {
assert_eq!(super::modulo((), -5_f64, 2_f64, false), -1_f64);
}
}

View file

@ -1461,7 +1461,7 @@ const WINDOW_SIZE: usize = 1024;
#[cfg(feature = "alloc")]
#[node_macro::node(category(""))]
fn generate_curves<C: Channel + super::Linear>(_: impl Ctx, curve: Curve, #[implementations(f32, f64)] _target_format: C) -> ValueMapperNode<C> {
fn generate_curves<C: Channel + crate::raster::Linear>(_: impl Ctx, curve: Curve, #[implementations(f32, f64)] _target_format: C) -> ValueMapperNode<C> {
use bezier_rs::{Bezier, TValue};
let [mut pos, mut param]: [[f32; 2]; 2] = [[0.; 2], curve.first_handle];

View file

@ -184,7 +184,7 @@ where
self.node.reset();
}
fn serialize(&self) -> Option<std::sync::Arc<dyn core::any::Any>> {
fn serialize(&self) -> Option<std::sync::Arc<dyn core::any::Any + Send + Sync>> {
self.node.serialize()
}
}
@ -217,7 +217,7 @@ where
}
#[inline(always)]
fn serialize(&self) -> Option<std::sync::Arc<dyn core::any::Any>> {
fn serialize(&self) -> Option<std::sync::Arc<dyn core::any::Any + Send + Sync>> {
self.node.serialize()
}
}
@ -273,7 +273,7 @@ where
self.node.reset();
}
fn serialize(&self) -> Option<std::sync::Arc<dyn core::any::Any>> {
fn serialize(&self) -> Option<std::sync::Arc<dyn core::any::Any + Send + Sync>> {
self.node.serialize()
}
}

View file

@ -19,6 +19,8 @@ use std::marker::PhantomData;
use std::str::FromStr;
pub use std::sync::Arc;
pub struct TaggedValueTypeError;
/// Macro to generate the tagged value enum.
macro_rules! tagged_value {
($ ($( #[$meta:meta] )* $identifier:ident ($ty:ty) ),* $(,)?) => {
@ -111,6 +113,26 @@ macro_rules! tagged_value {
Self::from_type(input).unwrap_or(TaggedValue::None)
}
}
$(
impl From<$ty> for TaggedValue {
fn from(value: $ty) -> Self {
Self::$identifier(value)
}
}
)*
$(
impl<'a> TryFrom<&'a TaggedValue> for &'a $ty {
type Error = TaggedValueTypeError;
fn try_from(value: &'a TaggedValue) -> Result<Self, Self::Error> {
match value{
TaggedValue::$identifier(value) => Ok(value),
_ => Err(TaggedValueTypeError),
}
}
}
)*
};
}
@ -128,6 +150,7 @@ tagged_value! {
#[cfg_attr(feature = "serde", serde(deserialize_with = "graphene_core::migrate_artboard_group"))]
ArtboardGroup(graphene_core::ArtboardGroupTable),
GraphicElement(graphene_core::GraphicElement),
Artboard(graphene_core::Artboard),
String(String),
U32(u32),
U64(u64),

View file

@ -923,12 +923,12 @@ mod test {
assert_eq!(
ids,
vec![
NodeId(907133870432995942),
NodeId(13049623730817360317),
NodeId(2177355904460308500),
NodeId(17479234042764485524),
NodeId(10988236038173832469),
NodeId(11097818235165626738),
NodeId(10795919842314709924),
NodeId(5986931472261716476),
NodeId(1689970140162147057),
NodeId(17084072420335757359),
NodeId(17163508657634907814),
NodeId(3540151743833532788)
]
);
}

View file

@ -95,7 +95,7 @@ impl DynamicExecutor {
}
/// Calls the `Node::serialize` for that specific node, returning for example the cached value for a monitor node. The node path must match the document node path.
pub fn introspect(&self, node_path: &[NodeId]) -> Result<Arc<dyn std::any::Any>, IntrospectError> {
pub fn introspect(&self, node_path: &[NodeId]) -> Result<Arc<dyn std::any::Any + Send + Sync + 'static>, IntrospectError> {
self.tree.introspect(node_path)
}
@ -217,7 +217,7 @@ impl BorrowTree {
}
/// Calls the `Node::serialize` for that specific node, returning for example the cached value for a monitor node. The node path must match the document node path.
pub fn introspect(&self, node_path: &[NodeId]) -> Result<Arc<dyn std::any::Any>, IntrospectError> {
pub fn introspect(&self, node_path: &[NodeId]) -> Result<Arc<dyn std::any::Any + Send + Sync + 'static>, IntrospectError> {
let (id, _) = self.source_map.get(node_path).ok_or_else(|| IntrospectError::PathNotFound(node_path.to_vec()))?;
let (node, _path) = self.nodes.get(id).ok_or(IntrospectError::ProtoNodeNotFound(*id))?;
node.serialize().ok_or(IntrospectError::NoData)

View file

@ -103,6 +103,28 @@ fn node_registry() -> HashMap<ProtoNodeIdentifier, HashMap<NodeIOTypes, NodeCons
node_io
},
),
async_node!(graphene_core::memo::MonitorNode<_, _, _>, input: Context, fn_params: [Context => graphene_core::RasterFrame]),
async_node!(graphene_core::memo::MonitorNode<_, _, _>, input: Context, fn_params: [Context => graphene_core::instances::Instances<Artboard>]),
async_node!(graphene_core::memo::MonitorNode<_, _, _>, input: Context, fn_params: [Context => String]),
async_node!(graphene_core::memo::MonitorNode<_, _, _>, input: Context, fn_params: [Context => glam::IVec2]),
async_node!(graphene_core::memo::MonitorNode<_, _, _>, input: Context, fn_params: [Context => glam::DVec2]),
async_node!(graphene_core::memo::MonitorNode<_, _, _>, input: Context, fn_params: [Context => bool]),
async_node!(graphene_core::memo::MonitorNode<_, _, _>, input: Context, fn_params: [Context => f64]),
async_node!(graphene_core::memo::MonitorNode<_, _, _>, input: Context, fn_params: [Context => u32]),
async_node!(graphene_core::memo::MonitorNode<_, _, _>, input: Context, fn_params: [Context => ()]),
async_node!(graphene_core::memo::MonitorNode<_, _, _>, input: Context, fn_params: [Context => Vec<f64>]),
async_node!(graphene_core::memo::MonitorNode<_, _, _>, input: Context, fn_params: [Context => BlendMode]),
async_node!(graphene_core::memo::MonitorNode<_, _, _>, input: Context, fn_params: [Context => graphene_core::vector::misc::BooleanOperation]),
async_node!(graphene_core::memo::MonitorNode<_, _, _>, input: Context, fn_params: [Context => Option<graphene_core::Color>]),
async_node!(graphene_core::memo::MonitorNode<_, _, _>, input: Context, fn_params: [Context => graphene_core::vector::style::Fill]),
async_node!(graphene_core::memo::MonitorNode<_, _, _>, input: Context, fn_params: [Context => graphene_core::vector::style::LineCap]),
async_node!(graphene_core::memo::MonitorNode<_, _, _>, input: Context, fn_params: [Context => graphene_core::vector::style::LineJoin]),
async_node!(graphene_core::memo::MonitorNode<_, _, _>, input: Context, fn_params: [Context => graphene_core::vector::style::Stroke]),
async_node!(graphene_core::memo::MonitorNode<_, _, _>, input: Context, fn_params: [Context => graphene_core::vector::style::Gradient]),
async_node!(graphene_core::memo::MonitorNode<_, _, _>, input: Context, fn_params: [Context => graphene_core::vector::style::GradientStops]),
async_node!(graphene_core::memo::MonitorNode<_, _, _>, input: Context, fn_params: [Context => Vec<graphene_core::uuid::NodeId>]),
async_node!(graphene_core::memo::MonitorNode<_, _, _>, input: Context, fn_params: [Context => graphene_core::Color]),
async_node!(graphene_core::memo::MonitorNode<_, _, _>, input: Context, fn_params: [Context => Box<graphene_core::vector::VectorModification>]),
#[cfg(feature = "gpu")]
(
ProtoNodeIdentifier::new("graphene_std::executor::MapGpuSingleImageNode"),

View file

@ -5,7 +5,10 @@ use convert_case::{Case, Casing};
use proc_macro2::TokenStream as TokenStream2;
use proc_macro_crate::FoundCrate;
use quote::{format_ident, quote};
use syn::{parse_quote, punctuated::Punctuated, spanned::Spanned, token::Comma, Error, Ident, Token, WhereClause, WherePredicate};
use syn::punctuated::Punctuated;
use syn::spanned::Spanned;
use syn::token::Comma;
use syn::{parse_quote, Error, Ident, PatIdent, Token, WhereClause, WherePredicate};
static NODE_ID: AtomicU64 = AtomicU64::new(0);
pub(crate) fn generate_node_code(parsed: &ParsedNodeFn) -> syn::Result<TokenStream2> {
@ -250,6 +253,7 @@ pub(crate) fn generate_node_code(parsed: &ParsedNodeFn) -> syn::Result<TokenStre
let properties = &attributes.properties_string.as_ref().map(|value| quote!(Some(#value))).unwrap_or(quote!(None));
let node_input_accessor = generate_node_input_references(parsed, fn_generics, &field_idents, &graphene_core, &identifier);
Ok(quote! {
/// Underlying implementation for [#struct_name]
#[inline]
@ -265,6 +269,9 @@ pub(crate) fn generate_node_code(parsed: &ParsedNodeFn) -> syn::Result<TokenStre
#[doc(inline)]
pub use #mod_name::#struct_name;
#[doc(hidden)]
#node_input_accessor
#[doc(hidden)]
mod #mod_name {
use super::*;
@ -326,6 +333,93 @@ pub(crate) fn generate_node_code(parsed: &ParsedNodeFn) -> syn::Result<TokenStre
})
}
/// Generates strongly typed utilites to access inputs
fn generate_node_input_references(parsed: &ParsedNodeFn, fn_generics: &[crate::GenericParam], field_idents: &[&PatIdent], graphene_core: &TokenStream2, identifier: &TokenStream2) -> TokenStream2 {
if parsed.attributes.skip_impl {
return quote! {};
}
let inputs_module_name = format_ident!("{}", parsed.struct_name.to_string().to_case(Case::Snake));
let (mut modified, mut generic_collector) = FilterUsedGenerics::new(fn_generics);
let mut generated_input_accessor = Vec::new();
for (input_index, (parsed_input, input_ident)) in parsed.fields.iter().zip(field_idents).enumerate() {
let mut ty = match parsed_input {
ParsedField::Regular { ty, .. } => ty,
ParsedField::Node { output_type, .. } => output_type,
}
.clone();
// We only want the necessary generics.
let used = generic_collector.filter_unnecessary_generics(&mut modified, &mut ty);
// TODO: figure out a better name that doesn't conflict with so many types
let struct_name = format_ident!("{}Input", input_ident.ident.to_string().to_case(Case::Pascal));
let (fn_generic_params, phantom_data_declerations) = generate_phantom_data(used.iter());
// Only create structs with phantom data where necessary.
generated_input_accessor.push(if phantom_data_declerations.is_empty() {
quote! {
pub struct #struct_name;
}
} else {
quote! {
pub struct #struct_name <#(#used),*>{
#(#phantom_data_declerations,)*
}
}
});
generated_input_accessor.push(quote! {
impl <#(#used),*> #graphene_core::NodeInputDecleration for #struct_name <#(#fn_generic_params),*> {
const INDEX: usize = #input_index;
fn identifier() -> &'static str {
protonode_identifier()
}
type Result = #ty;
}
})
}
quote! {
pub mod #inputs_module_name {
use super::*;
pub fn protonode_identifier() -> &'static str {
// Storing the string in a once lock should reduce allocations (since we call this in a loop)?
static NODE_NAME: std::sync::OnceLock<String> = std::sync::OnceLock::new();
NODE_NAME.get_or_init(|| #identifier )
}
#(#generated_input_accessor)*
}
}
}
/// It is necessary to generate PhantomData for each fn generic to avoid compiler errors.
fn generate_phantom_data<'a>(fn_generics: impl Iterator<Item = &'a crate::GenericParam>) -> (Vec<TokenStream2>, Vec<TokenStream2>) {
let mut phantom_data_declerations = Vec::new();
let mut fn_generic_params = Vec::new();
for fn_generic_param in fn_generics {
let field_name = format_ident!("phantom_{}", phantom_data_declerations.len());
match fn_generic_param {
crate::GenericParam::Lifetime(lifetime_param) => {
let lifetime = &lifetime_param.lifetime;
fn_generic_params.push(quote! {#lifetime});
phantom_data_declerations.push(quote! {#field_name: core::marker::PhantomData<&#lifetime ()>})
}
crate::GenericParam::Type(type_param) => {
let generic_name = &type_param.ident;
fn_generic_params.push(quote! {#generic_name});
phantom_data_declerations.push(quote! {#field_name: core::marker::PhantomData<#generic_name>});
}
_ => {}
}
}
(fn_generic_params, phantom_data_declerations)
}
fn generate_register_node_impl(parsed: &ParsedNodeFn, field_names: &[&Ident], struct_name: &Ident, identifier: &TokenStream2) -> Result<TokenStream2, syn::Error> {
if parsed.attributes.skip_impl {
return Ok(quote!());
@ -469,3 +563,83 @@ fn substitute_lifetimes(mut ty: Type, lifetime: &'static str) -> Type {
LifetimeReplacer(lifetime).visit_type_mut(&mut ty);
ty
}
/// Get only the necessary generics.
struct FilterUsedGenerics {
all: Vec<crate::GenericParam>,
used: Vec<bool>,
}
impl VisitMut for FilterUsedGenerics {
fn visit_lifetime_mut(&mut self, used_lifetime: &mut syn::Lifetime) {
for (generic, used) in self.all.iter().zip(self.used.iter_mut()) {
let crate::GenericParam::Lifetime(lifetime_param) = generic else { continue };
if used_lifetime == &lifetime_param.lifetime {
*used = true;
}
}
}
fn visit_path_mut(&mut self, path: &mut syn::Path) {
for (index, (generic, used)) in self.all.iter().zip(self.used.iter_mut()).enumerate() {
let crate::GenericParam::Type(type_param) = generic else { continue };
if path.leading_colon.is_none() && !path.segments.is_empty() && path.segments[0].arguments.is_none() && path.segments[0].ident == type_param.ident {
*used = true;
// Sometimes the generics conflict with the type name so we rename the generics.
path.segments[0].ident = format_ident!("G{index}");
}
}
for mut el in Punctuated::pairs_mut(&mut path.segments) {
self.visit_path_segment_mut(el.value_mut());
}
}
}
impl FilterUsedGenerics {
fn new(fn_generics: &[crate::GenericParam]) -> (Vec<crate::GenericParam>, Self) {
let mut all_possible_generics = fn_generics.to_vec();
// The 'n lifetime may also be needed; we must add it in
all_possible_generics.insert(0, syn::GenericParam::Lifetime(syn::LifetimeParam::new(Lifetime::new("'n", proc_macro2::Span::call_site()))));
let modified = all_possible_generics
.iter()
.cloned()
.enumerate()
.map(|(index, mut generic)| {
let crate::GenericParam::Type(type_param) = &mut generic else { return generic };
// Sometimes the generics conflict with the type name so we rename the generics.
type_param.ident = format_ident!("G{index}");
generic
})
.collect::<Vec<_>>();
let generic_collector = Self {
used: vec![false; all_possible_generics.len()],
all: all_possible_generics,
};
(modified, generic_collector)
}
fn used<'a>(&'a self, modified: &'a [crate::GenericParam]) -> impl Iterator<Item = &'a crate::GenericParam> {
modified.iter().zip(&self.used).filter(|(_, used)| **used).map(move |(value, _)| value)
}
fn filter_unnecessary_generics(&mut self, modified: &mut Vec<syn::GenericParam>, ty: &mut Type) -> Vec<syn::GenericParam> {
self.used.fill(false);
// Find out which generics are necessary to support the node input
self.visit_type_mut(ty);
// Sometimes generics may reference other generics. This is a non-optimal way of dealing with that.
for _ in 0..=self.all.len() {
for (index, item) in modified.iter_mut().enumerate() {
if self.used[index] {
self.visit_generic_param_mut(item);
}
}
}
self.used(&*modified).cloned().collect()
}
}