diff --git a/node-graph/README.md b/node-graph/README.md index 0e7e381e1b..c93fcc79ad 100644 --- a/node-graph/README.md +++ b/node-graph/README.md @@ -102,7 +102,7 @@ Instead of manually implementing the `Node` trait with complex generics, one can ```rs #[node_macro::node(category("Raster: Adjustments"))] -fn opacity(_input: (), #[default(424242)] color: Color,#[min(0.1)] opacity_multiplier: f64) -> Color { +fn opacity(_input: (), #[default(424242)] color: Color, #[soft_min(0.1)] opacity_multiplier: f64) -> Color { let opacity_multiplier = opacity_multiplier as f32 / 100.; Color::from_rgbaf32_unchecked(color.r(), color.g(), color.b(), color.a() * opacity_multiplier) } diff --git a/node-graph/gcore/src/lib.rs b/node-graph/gcore/src/lib.rs index f4a8649e44..1aa71eed73 100644 --- a/node-graph/gcore/src/lib.rs +++ b/node-graph/gcore/src/lib.rs @@ -9,6 +9,7 @@ use core::future::Future; #[cfg(feature = "log")] extern crate log; pub use crate as graphene_core; +pub use num_traits; #[cfg(feature = "reflections")] pub use ctor; @@ -19,6 +20,7 @@ pub mod context; pub mod generic; pub mod instances; pub mod logic; +pub mod misc; pub mod ops; pub mod structural; #[cfg(feature = "std")] diff --git a/node-graph/gcore/src/misc.rs b/node-graph/gcore/src/misc.rs new file mode 100644 index 0000000000..9d70728ab1 --- /dev/null +++ b/node-graph/gcore/src/misc.rs @@ -0,0 +1,62 @@ +// TODO(TrueDoctor): Replace this with the more idiomatic approach instead of using `trait Clampable`. + +/// A trait for types that can be clamped within a min/max range defined by f64. +pub trait Clampable: Sized { + /// Clamps the value to be no less than `min`. + fn clamp_hard_min(self, min: f64) -> Self; + /// Clamps the value to be no more than `max`. + fn clamp_hard_max(self, max: f64) -> Self; +} + +// Implement for common numeric types +macro_rules! impl_clampable_float { + ($($ty:ty),*) => { + $( + impl Clampable for $ty { + #[inline(always)] + fn clamp_hard_min(self, min: f64) -> Self { + self.max(min as $ty) + } + #[inline(always)] + fn clamp_hard_max(self, max: f64) -> Self { + self.min(max as $ty) + } + } + )* + }; +} +impl_clampable_float!(f32, f64); + +macro_rules! impl_clampable_int { + ($($ty:ty),*) => { + $( + impl Clampable for $ty { + #[inline(always)] + fn clamp_hard_min(self, min: f64) -> Self { + // Using try_from to handle potential range issues safely, though min should ideally be valid. + // Consider using a different approach if f64 precision vs integer range is a concern. + <$ty>::try_from(min.ceil() as i64).ok().map_or(self, |min_val| self.max(min_val)) + } + #[inline(always)] + fn clamp_hard_max(self, max: f64) -> Self { + <$ty>::try_from(max.floor() as i64).ok().map_or(self, |max_val| self.min(max_val)) + } + } + )* + }; +} +// Add relevant integer types (adjust as needed) +impl_clampable_int!(u32, u64, i32, i64); + +// Implement for DVec2 (component-wise clamping) +use glam::DVec2; +impl Clampable for DVec2 { + #[inline(always)] + fn clamp_hard_min(self, min: f64) -> Self { + self.max(DVec2::splat(min)) + } + #[inline(always)] + fn clamp_hard_max(self, max: f64) -> Self { + self.min(DVec2::splat(max)) + } +} diff --git a/node-graph/gcore/src/raster/adjustments.rs b/node-graph/gcore/src/raster/adjustments.rs index 1b3c5c4e10..715beca35d 100644 --- a/node-graph/gcore/src/raster/adjustments.rs +++ b/node-graph/gcore/src/raster/adjustments.rs @@ -1399,7 +1399,7 @@ async fn posterize>( )] mut input: T, #[default(4)] - #[min(2.)] + #[hard_min(2.)] levels: u32, ) -> T { input.adjust(|color| { @@ -1435,6 +1435,7 @@ async fn exposure>( offset: f64, #[default(1.)] #[range((0.01, 10.))] + #[hard_min(0.0001)] gamma_correction: f64, ) -> T { input.adjust(|color| { diff --git a/node-graph/gcore/src/raster/color.rs b/node-graph/gcore/src/raster/color.rs index 3be18d1952..736019e2f1 100644 --- a/node-graph/gcore/src/raster/color.rs +++ b/node-graph/gcore/src/raster/color.rs @@ -930,6 +930,8 @@ impl Color { #[inline(always)] pub fn gamma(&self, gamma: f32) -> Color { + let gamma = gamma.max(0.0001); + // From https://www.dfstudios.co.uk/articles/programming/image-programming-algorithms/image-processing-algorithms-part-6-gamma-correction/ let inverse_gamma = 1. / gamma; self.map_rgb(|c: f32| c.powf(inverse_gamma)) diff --git a/node-graph/gcore/src/vector/generator_nodes.rs b/node-graph/gcore/src/vector/generator_nodes.rs index 762322f203..d6a07e66b1 100644 --- a/node-graph/gcore/src/vector/generator_nodes.rs +++ b/node-graph/gcore/src/vector/generator_nodes.rs @@ -101,7 +101,7 @@ fn regular_polygon( _: impl Ctx, _primary: (), #[default(6)] - #[min(3.)] + #[hard_min(3.)] #[implementations(u32, u64, f64)] sides: T, #[default(50)] radius: f64, @@ -116,7 +116,7 @@ fn star( _: impl Ctx, _primary: (), #[default(5)] - #[min(2.)] + #[hard_min(2.)] #[implementations(u32, u64, f64)] sides: T, #[default(50)] radius: f64, @@ -153,7 +153,7 @@ fn grid( _: impl Ctx, _primary: (), grid_type: GridType, - #[min(0.)] + #[hard_min(0.)] #[default(10)] #[implementations(f64, DVec2)] spacing: T, diff --git a/node-graph/gcore/src/vector/vector_nodes.rs b/node-graph/gcore/src/vector/vector_nodes.rs index 53e786fa9e..50cd60b87b 100644 --- a/node-graph/gcore/src/vector/vector_nodes.rs +++ b/node-graph/gcore/src/vector/vector_nodes.rs @@ -429,14 +429,18 @@ where async fn round_corners( _: impl Ctx, source: VectorDataTable, - #[min(0.)] + #[hard_min(0.)] #[default(10.)] radius: PixelLength, #[range((0., 1.))] + #[hard_min(0.)] + #[hard_max(1.)] #[default(0.5)] roundness: f64, #[default(100.)] edge_length_limit: Percentage, #[range((0., 180.))] + #[hard_min(0.)] + #[hard_max(180.)] #[default(5.)] min_angle_threshold: Angle, ) -> VectorDataTable { @@ -538,7 +542,7 @@ async fn spatial_merge_by_distance( _: impl Ctx, vector_data: VectorDataTable, #[default(0.1)] - #[min(0.0001)] + #[hard_min(0.0001)] distance: f64, ) -> VectorDataTable { let vector_data_transform = vector_data.transform(); @@ -748,7 +752,7 @@ async fn remove_handles( _: impl Ctx, vector_data: VectorDataTable, #[default(10.)] - #[min(0.)] + #[soft_min(0.)] max_handle_distance: f64, ) -> VectorDataTable { let vector_data_transform = vector_data.transform(); @@ -879,8 +883,8 @@ async fn generate_handles( // _: impl Ctx, // source: VectorDataTable, // #[default(1.)] -// #[min(1.)] -// #[max(8.)] +// #[hard_min(1.)] +// #[soft_max(8.)] // subdivisions: f64, // ) -> VectorDataTable { // let source_transform = source.transform(); @@ -1367,7 +1371,7 @@ async fn poisson_disk_points( _: impl Ctx, vector_data: VectorDataTable, #[default(10.)] - #[min(0.01)] + #[hard_min(0.01)] separation_disk_diameter: f64, seed: SeedValue, ) -> VectorDataTable { diff --git a/node-graph/gstd/src/filter.rs b/node-graph/gstd/src/filter.rs index dc08b1fba6..685a30c610 100644 --- a/node-graph/gstd/src/filter.rs +++ b/node-graph/gstd/src/filter.rs @@ -12,6 +12,7 @@ async fn blur( image_frame: ImageFrameTable, /// The radius of the blur kernel. #[range((0., 100.))] + #[hard_min(0.)] radius: PixelLength, /// Use a lower-quality box kernel instead of a circular Gaussian kernel. This is faster but produces boxy artifacts. box_blur: bool, diff --git a/node-graph/gstd/src/image_color_palette.rs b/node-graph/gstd/src/image_color_palette.rs index f817cc9a72..164db26208 100644 --- a/node-graph/gstd/src/image_color_palette.rs +++ b/node-graph/gstd/src/image_color_palette.rs @@ -5,8 +5,8 @@ use graphene_core::{Color, Ctx}; async fn image_color_palette( _: impl Ctx, image: ImageFrameTable, - #[min(1.)] - #[max(28.)] + #[hard_min(1.)] + #[soft_max(28.)] max_size: u32, ) -> Vec { const GRID: f32 = 3.; diff --git a/node-graph/node-macro/src/codegen.rs b/node-graph/node-macro/src/codegen.rs index af04dd8a0f..74d45ff0a2 100644 --- a/node-graph/node-macro/src/codegen.rs +++ b/node-graph/node-macro/src/codegen.rs @@ -2,7 +2,7 @@ use crate::parsing::*; use convert_case::{Case, Casing}; use proc_macro_crate::FoundCrate; use proc_macro2::TokenStream as TokenStream2; -use quote::{format_ident, quote}; +use quote::{format_ident, quote, quote_spanned}; use std::sync::atomic::AtomicU64; use syn::punctuated::Punctuated; use syn::spanned::Spanned; @@ -134,14 +134,22 @@ pub(crate) fn generate_node_code(parsed: &ParsedNodeFn) -> syn::Result = fields .iter() .map(|field| match field { - ParsedField::Regular { number_min: Some(number_min), .. } => quote!(Some(#number_min)), + ParsedField::Regular { number_soft_min, number_hard_min, .. } => match (number_soft_min, number_hard_min) { + (Some(soft_min), _) => quote!(Some(#soft_min)), + (None, Some(hard_min)) => quote!(Some(#hard_min)), + (None, None) => quote!(None), + }, _ => quote!(None), }) .collect(); let number_max_values: Vec<_> = fields .iter() .map(|field| match field { - ParsedField::Regular { number_max: Some(number_max), .. } => quote!(Some(#number_max)), + ParsedField::Regular { number_soft_max, number_hard_max, .. } => match (number_soft_max, number_hard_max) { + (Some(soft_max), _) => quote!(Some(#soft_max)), + (None, Some(hard_max)) => quote!(Some(#hard_max)), + (None, None) => quote!(None), + }, _ => quote!(None), }) .collect(); @@ -175,6 +183,33 @@ pub(crate) fn generate_node_code(parsed: &ParsedNodeFn) -> syn::Result { + let name = &pat_ident.ident; + let mut tokens = quote!(); + if let Some(min) = number_hard_min { + tokens.extend(quote_spanned! {min.span()=> + let #name = #graphene_core::misc::Clampable::clamp_hard_min(#name, #min); + }); + } + + if let Some(max) = number_hard_max { + tokens.extend(quote_spanned! {max.span()=> + let #name = #graphene_core::misc::Clampable::clamp_hard_max(#name, #max); + }); + } + tokens + } + ParsedField::Node { .. } => { + quote!() + } + }); + let all_implementation_types = fields.iter().flat_map(|field| match field { ParsedField::Regular { implementations, .. } => implementations.into_iter().cloned().collect::>(), ParsedField::Node { implementations, .. } => implementations @@ -186,13 +221,27 @@ pub(crate) fn generate_node_code(parsed: &ParsedNodeFn) -> syn::Result { + ( + ParsedField::Regular { + ty, number_hard_min, number_hard_max, .. + }, + _, + ) => { let all_lifetime_ty = substitute_lifetimes(ty.clone(), "all"); let id = future_idents.len(); let fut_ident = format_ident!("F{}", id); future_idents.push(fut_ident.clone()); + + // Add Clampable bound if this field uses hard_min or hard_max + if number_hard_min.is_some() || number_hard_max.is_some() { + // The bound applies to the Output type of the future, which is #ty + clampable_clauses.push(quote!(#ty: #graphene_core::misc::Clampable)); + } + quote!( #fut_ident: core::future::Future + #graphene_core::WasmNotSend + 'n, for<'all> #all_lifetime_ty: #graphene_core::WasmNotSend, @@ -220,6 +269,7 @@ pub(crate) fn generate_node_code(parsed: &ParsedNodeFn) -> syn::Result = parse_quote!( #(#clauses,)* + #(#clampable_clauses,)* #output_type: 'n, ); struct_where_clause.predicates.extend(extra_where); @@ -236,7 +286,10 @@ pub(crate) fn generate_node_code(parsed: &ParsedNodeFn) -> syn::Result Self::Output { Box::pin(async move { + use #graphene_core::misc::Clampable; + #(#eval_args)* + #(#min_max_args)* self::#fn_name(__input #(, #field_names)*) #await_keyword }) } diff --git a/node-graph/node-macro/src/parsing.rs b/node-graph/node-macro/src/parsing.rs index c66ae276d5..ee8030d55c 100644 --- a/node-graph/node-macro/src/parsing.rs +++ b/node-graph/node-macro/src/parsing.rs @@ -105,8 +105,10 @@ pub(crate) enum ParsedField { ty: Type, exposed: bool, value_source: ParsedValueSource, - number_min: Option, - number_max: Option, + number_soft_min: Option, + number_soft_max: Option, + number_hard_min: Option, + number_hard_max: Option, number_mode_range: Option, implementations: Punctuated, }, @@ -230,7 +232,7 @@ impl Parse for NodeFnAttributes { r#" Unsupported attribute in `node`. Supported attributes are 'category', 'path' and 'name'. - + Example usage: #[node_macro::node(category("Value"), name("Test Node"))] "# @@ -419,16 +421,29 @@ fn parse_field(pat_ident: PatIdent, ty: Type, attrs: &[Attribute]) -> syn::Resul _ => ParsedValueSource::None, }; - let number_min = extract_attribute(attrs, "min") + let number_soft_min = extract_attribute(attrs, "soft_min") + .map(|attr| { + attr.parse_args() + .map_err(|e| Error::new_spanned(attr, format!("Invalid numerical `soft_min` value for argument '{}': {}", ident, e))) + }) + .transpose()?; + let number_soft_max = extract_attribute(attrs, "soft_max") + .map(|attr| { + attr.parse_args() + .map_err(|e| Error::new_spanned(attr, format!("Invalid numerical `soft_max` value for argument '{}': {}", ident, e))) + }) + .transpose()?; + + let number_hard_min = extract_attribute(attrs, "hard_min") .map(|attr| { attr.parse_args() - .map_err(|e| Error::new_spanned(attr, format!("Invalid numerical `min` value for argument '{}': {}", ident, e))) + .map_err(|e| Error::new_spanned(attr, format!("Invalid numerical `hard_min` value for argument '{}': {}", ident, e))) }) .transpose()?; - let number_max = extract_attribute(attrs, "max") + let number_hard_max = extract_attribute(attrs, "hard_max") .map(|attr| { attr.parse_args() - .map_err(|e| Error::new_spanned(attr, format!("Invalid numerical `max` value for argument '{}': {}", ident, e))) + .map_err(|e| Error::new_spanned(attr, format!("Invalid numerical `hard_max` value for argument '{}': {}", ident, e))) }) .transpose()?; @@ -500,8 +515,10 @@ fn parse_field(pat_ident: PatIdent, ty: Type, attrs: &[Attribute]) -> syn::Resul description, widget_override, exposed, - number_min, - number_max, + number_soft_min, + number_soft_max, + number_hard_min, + number_hard_max, number_mode_range, ty, value_source, @@ -716,8 +733,10 @@ mod tests { ty: parse_quote!(f64), exposed: false, value_source: ParsedValueSource::None, - number_min: None, - number_max: None, + number_soft_min: None, + number_soft_max: None, + number_hard_min: None, + number_hard_max: None, number_mode_range: None, implementations: Punctuated::new(), }], @@ -781,8 +800,10 @@ mod tests { ty: parse_quote!(DVec2), exposed: false, value_source: ParsedValueSource::None, - number_min: None, - number_max: None, + number_soft_min: None, + number_soft_max: None, + number_hard_min: None, + number_hard_max: None, number_mode_range: None, implementations: Punctuated::new(), }, @@ -834,8 +855,10 @@ mod tests { ty: parse_quote!(f64), exposed: false, value_source: ParsedValueSource::Default(quote!(50.)), - number_min: None, - number_max: None, + number_soft_min: None, + number_soft_max: None, + number_hard_min: None, + number_hard_max: None, number_mode_range: None, implementations: Punctuated::new(), }], @@ -885,8 +908,10 @@ mod tests { ty: parse_quote!(f64), exposed: false, value_source: ParsedValueSource::None, - number_min: None, - number_max: None, + number_soft_min: None, + number_soft_max: None, + number_hard_min: None, + number_hard_max: None, number_mode_range: None, implementations: { let mut p = Punctuated::new(); @@ -911,8 +936,8 @@ mod tests { a: f64, /// b #[range((0., 100.))] - #[min(-500.)] - #[max(500.)] + #[soft_min(-500.)] + #[soft_max(500.)] b: f64, ) -> f64 { a + b @@ -948,8 +973,10 @@ mod tests { ty: parse_quote!(f64), exposed: false, value_source: ParsedValueSource::None, - number_min: Some(parse_quote!(-500.)), - number_max: Some(parse_quote!(500.)), + number_soft_min: Some(parse_quote!(-500.)), + number_soft_max: Some(parse_quote!(500.)), + number_hard_min: None, + number_hard_max: None, number_mode_range: Some(parse_quote!((0., 100.))), implementations: Punctuated::new(), }], @@ -999,8 +1026,10 @@ mod tests { widget_override: ParsedWidgetOverride::None, exposed: true, value_source: ParsedValueSource::None, - number_min: None, - number_max: None, + number_soft_min: None, + number_soft_max: None, + number_hard_min: None, + number_hard_max: None, number_mode_range: None, implementations: Punctuated::new(), }], diff --git a/node-graph/node-macro/src/validation.rs b/node-graph/node-macro/src/validation.rs index e6eece2128..9ce43a650e 100644 --- a/node-graph/node-macro/src/validation.rs +++ b/node-graph/node-macro/src/validation.rs @@ -9,6 +9,7 @@ pub fn validate_node_fn(parsed: &ParsedNodeFn) -> syn::Result<()> { // Add more validators here as needed validate_implementations_for_generics, validate_primary_input_expose, + validate_min_max, ]; for validator in validators { @@ -18,6 +19,64 @@ pub fn validate_node_fn(parsed: &ParsedNodeFn) -> syn::Result<()> { Ok(()) } +fn validate_min_max(parsed: &ParsedNodeFn) { + for field in &parsed.fields { + if let ParsedField::Regular { + number_hard_max, + number_hard_min, + number_soft_max, + number_soft_min, + pat_ident, + .. + } = field + { + if let (Some(soft_min), Some(hard_min)) = (number_soft_min, number_hard_min) { + let soft_min_value: f64 = soft_min.base10_parse().unwrap_or_default(); + let hard_min_value: f64 = hard_min.base10_parse().unwrap_or_default(); + if soft_min_value == hard_min_value { + emit_error!( + pat_ident.span(), + "Unnecessary #[soft_min] attribute on `{}`, as #[hard_min] has the same value.", + pat_ident.ident; + help = "You can safely remove the #[soft_min] attribute from this field."; + note = "#[soft_min] is redundant when it equals #[hard_min].", + ); + } else if soft_min_value < hard_min_value { + emit_error!( + pat_ident.span(), + "The #[soft_min] attribute on `{}` is incorrectly greater than #[hard_min].", + pat_ident.ident; + help = "You probably meant to reverse the two attribute values."; + note = "Allowing the possible slider range to preceed #[hard_min] doesn't make sense.", + ); + } + } + + if let (Some(soft_max), Some(hard_max)) = (number_soft_max, number_hard_max) { + let soft_max_value: f64 = soft_max.base10_parse().unwrap_or_default(); + let hard_max_value: f64 = hard_max.base10_parse().unwrap_or_default(); + if soft_max_value == hard_max_value { + emit_error!( + pat_ident.span(), + "Unnecessary #[soft_max] attribute on `{}`, as #[hard_max] has the same value.", + pat_ident.ident; + help = "You can safely remove the #[soft_max] attribute from this field."; + note = "#[soft_max] is redundant when it equals #[hard_max].", + ); + } else if soft_max_value < hard_max_value { + emit_error!( + pat_ident.span(), + "The #[soft_max] attribute on `{}` is incorrectly greater than #[hard_max].", + pat_ident.ident; + help = "You probably meant to reverse the two attribute values."; + note = "Allowing the possible slider range to exceed #[hard_max] doesn't make sense.", + ); + } + } + } + } +} + fn validate_primary_input_expose(parsed: &ParsedNodeFn) { if let Some(ParsedField::Regular { exposed: true, pat_ident, .. }) = parsed.fields.first() { emit_error!( diff --git a/proc-macros/src/widget_builder.rs b/proc-macros/src/widget_builder.rs index 09aee1f23a..f5c6d0a546 100644 --- a/proc-macros/src/widget_builder.rs +++ b/proc-macros/src/widget_builder.rs @@ -21,13 +21,13 @@ fn easier_string_assignment(field_ty: &Type, field_ident: &Ident) -> (TokenStrea // Based on https://stackoverflow.com/questions/66906261/rust-proc-macro-derive-how-do-i-check-if-a-field-is-of-a-primitive-type-like-b if last_segment.ident == Ident::new("String", last_segment.ident.span()) { return ( - quote::quote_spanned!(type_path.span() => impl Into), - quote::quote_spanned!(field_ident.span() => #field_ident.into()), + quote::quote_spanned!(type_path.span()=> impl Into), + quote::quote_spanned!(field_ident.span()=> #field_ident.into()), ); } } } - (quote::quote_spanned!(field_ty.span() => #field_ty), quote::quote_spanned!(field_ident.span() => #field_ident)) + (quote::quote_spanned!(field_ty.span()=> #field_ty), quote::quote_spanned!(field_ident.span()=> #field_ident)) } /// Extract the identifier of the field (which should always be present) @@ -54,8 +54,8 @@ fn find_type_and_assignment(field: &Field) -> syn::Result<(TokenStream2, TokenSt if let Some(first_generic) = generic_args.args.first() { if last_segment.ident == Ident::new("WidgetCallback", last_segment.ident.span()) { // Assign builder pattern to assign the closure directly - function_input_ty = quote::quote_spanned!(field_ty.span() => impl Fn(&#first_generic) -> crate::messages::message::Message + 'static + Send + Sync); - assignment = quote::quote_spanned!(field_ident.span() => crate::messages::layout::utility_types::layout_widget::WidgetCallback::new(#field_ident)); + function_input_ty = quote::quote_spanned!(field_ty.span()=> impl Fn(&#first_generic) -> crate::messages::message::Message + 'static + Send + Sync); + assignment = quote::quote_spanned!(field_ident.span()=> crate::messages::layout::utility_types::layout_widget::WidgetCallback::new(#field_ident)); } } } @@ -78,7 +78,7 @@ fn construct_builder(field: &Field) -> syn::Result { let (function_input_ty, assignment) = find_type_and_assignment(field)?; // Create builder function - Ok(quote::quote_spanned!(field.span() => + Ok(quote::quote_spanned!(field.span()=> #[doc = #doc_comment] pub fn #field_ident(mut self, #field_ident: #function_input_ty) -> Self{ self.#field_ident = #assignment;