From f05a644bd9900e59ec6a27b86b2fedbab872ffd9 Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Fri, 3 Jul 2026 18:48:58 -0700 Subject: [PATCH] Revamp the syntax to #[hard(a..b)] and #[soft(a..b)] bounds on node definitions (#4307) * Revamp the syntax to #[hard(a..b)] and #[soft(a..b)] bounds on node definitions * Address review feedback: error on empty bounds ranges and remove redundant count clamps * Fix NaN transform from Repeat Array with a count of 1 * Remove redundant manual clamps already enforced by #[hard(...)] bounds --- .../node_graph/document_node_definitions.rs | 17 +- .../document/node_graph/node_properties.rs | 80 ++++++--- node-graph/README.md | 4 +- .../libraries/core-types/src/registry.rs | 10 +- node-graph/node-macro/src/codegen.rs | 48 +++--- node-graph/node-macro/src/parsing.rs | 158 +++++++++++------- .../src/shader_nodes/per_pixel_adjust.rs | 2 +- node-graph/node-macro/src/validation.rs | 65 +++++-- node-graph/nodes/graphic/src/graphic.rs | 4 +- node-graph/nodes/gstd/src/text.rs | 11 +- node-graph/nodes/raster/src/adjustments.rs | 33 ++-- node-graph/nodes/raster/src/filter.rs | 10 +- .../nodes/raster/src/image_color_palette.rs | 2 +- node-graph/nodes/repeat/src/repeat_nodes.rs | 34 +++- node-graph/nodes/text/src/lib.rs | 4 +- .../nodes/vector/src/generator_nodes.rs | 13 +- node-graph/nodes/vector/src/vector_nodes.rs | 43 ++--- 17 files changed, 341 insertions(+), 197 deletions(-) diff --git a/editor/src/messages/portfolio/document/node_graph/document_node_definitions.rs b/editor/src/messages/portfolio/document/node_graph/document_node_definitions.rs index 4e33fcb250..c097025ac2 100644 --- a/editor/src/messages/portfolio/document/node_graph/document_node_definitions.rs +++ b/editor/src/messages/portfolio/document/node_graph/document_node_definitions.rs @@ -1664,15 +1664,18 @@ fn static_input_properties() -> InputProperties { if let Some(unit) = field.unit { number_input = number_input.unit(unit); } - if let Some(number_min) = field.number_min { - number_input = number_input.min(number_min); + // Typing is clamped only by the hard bounds; the slider extent prefers the soft bounds (see `property_from_type`) + if let Some(hard_min) = field.number_hard_min { + number_input = number_input.min(hard_min); } - if let Some(number_max) = field.number_max { - number_input = number_input.max(number_max); + if let Some(hard_max) = field.number_hard_max { + number_input = number_input.max(hard_max); } - if let Some((range_min, range_max)) = field.number_mode_range { - number_input = number_input.range_min(Some(range_min)); - number_input = number_input.range_max(Some(range_max)); + if field.number_mode_range { + number_input = number_input + .mode_range() + .range_min(field.number_soft_min.or(field.number_hard_min)) + .range_max(field.number_soft_max.or(field.number_hard_max)); } number_input = number_input.is_integer(false); if let Some(number_step) = field.number_step { diff --git a/editor/src/messages/portfolio/document/node_graph/node_properties.rs b/editor/src/messages/portfolio/document/node_graph/node_properties.rs index c726ca2568..7ce642cb4c 100644 --- a/editor/src/messages/portfolio/document/node_graph/node_properties.rs +++ b/editor/src/messages/portfolio/document/node_graph/node_properties.rs @@ -149,22 +149,36 @@ pub fn start_widgets(parameter_widgets_info: ParameterWidgetsInfo) -> Vec, + pub soft_max: Option, + pub hard_min: Option, + pub hard_max: Option, + pub slider: bool, +} + pub(crate) fn property_from_type( node_id: NodeId, index: usize, ty: &Type, - number_options: (Option, Option, Option<(f64, f64)>), + number_options: NumberOptions, unit: Option<&str>, display_decimal_places: Option, step: Option, context: &mut NodePropertiesContext, ) -> Result, Vec> { - let (mut number_min, mut number_max, range) = number_options; + let NumberOptions { + soft_min, + soft_max, + hard_min, + hard_max, + slider, + } = number_options; let mut number_input = NumberInput::default(); - if let Some((range_start, range_end)) = range { - number_min = Some(range_start); - number_max = Some(range_end); - number_input = number_input.mode_range().min(range_start).max(range_end); + if slider { + number_input = number_input.mode_range(); } if let Some(unit) = unit { number_input = number_input.unit(unit); @@ -176,8 +190,22 @@ pub(crate) fn property_from_type( number_input = number_input.step(step); } - let min = |x: f64| number_min.unwrap_or(x); - let max = |x: f64| number_max.unwrap_or(x); + // Applies the parameter's typing clamp and slider extent to the widget, given the type's own default bounds. + // Per end: the clamp is the hard bound (or unbounded if only a soft bound is given, since soft is a suggested + // extent rather than a limit), and the slider extent is the soft bound, each falling back to the hard bound + // and then to the type default when unspecified. An end with any explicit bound ignores the type default. + let bounded = |number_input: NumberInput, type_min: f64, type_max: f64| { + let clamp_min = hard_min.unwrap_or(if soft_min.is_some() { f64::NEG_INFINITY } else { type_min }); + let clamp_max = hard_max.unwrap_or(if soft_max.is_some() { f64::INFINITY } else { type_max }); + let extent_min = soft_min.or(hard_min).unwrap_or(type_min); + let extent_max = soft_max.or(hard_max).unwrap_or(type_max); + + number_input + .min(clamp_min) + .max(clamp_max) + .range_min(Some(extent_min).filter(|bound| bound.is_finite())) + .range_max(Some(extent_max).filter(|bound| bound.is_finite())) + }; let default_info = ParameterWidgetsInfo::new(node_id, index, true, context); @@ -186,16 +214,16 @@ pub(crate) fn property_from_type( Type::Concrete(concrete_type) => { match concrete_type.alias.as_ref().map(|x| x.as_ref()) { // Aliased types (ambiguous values) - Some("Percentage") | Some("PercentageF32") => number_widget(default_info, number_input.percentage().min(min(0.)).max(max(100.))).into(), - Some("SignedPercentage") | Some("SignedPercentageF32") => number_widget(default_info, number_input.percentage().min(min(-100.)).max(max(100.))).into(), - Some("Angle") | Some("AngleF32") => number_widget(default_info, number_input.mode_range().min(min(-180.)).max(max(180.)).unit(unit.unwrap_or("°"))).into(), - Some("Multiplier") => number_widget(default_info, number_input.unit(unit.unwrap_or("x"))).into(), - Some("PixelLength") => number_widget(default_info, number_input.min(min(0.)).unit(unit.unwrap_or(" px"))).into(), - Some("Length") => number_widget(default_info, number_input.min(min(0.))).into(), - Some("Fraction") => number_widget(default_info, number_input.mode_range().min(min(0.)).max(max(1.))).into(), - Some("Progression") => progression_widget(default_info, number_input.min(min(0.))).into(), - Some("SignedInteger") => number_widget(default_info, number_input.int()).into(), - Some("SeedValue") => number_widget(default_info, number_input.int().min(min(0.))).into(), + Some("Percentage") | Some("PercentageF32") => number_widget(default_info, bounded(number_input.percentage(), 0., 100.)).into(), + Some("SignedPercentage") | Some("SignedPercentageF32") => number_widget(default_info, bounded(number_input.percentage(), -100., 100.)).into(), + Some("Angle") | Some("AngleF32") => number_widget(default_info, bounded(number_input.mode_range(), -180., 180.).unit(unit.unwrap_or("°"))).into(), + Some("Multiplier") => number_widget(default_info, bounded(number_input, f64::NEG_INFINITY, f64::INFINITY).unit(unit.unwrap_or("x"))).into(), + Some("PixelLength") => number_widget(default_info, bounded(number_input, 0., f64::INFINITY).unit(unit.unwrap_or(" px"))).into(), + Some("Length") => number_widget(default_info, bounded(number_input, 0., f64::INFINITY)).into(), + Some("Fraction") => number_widget(default_info, bounded(number_input.mode_range(), 0., 1.)).into(), + Some("Progression") => progression_widget(default_info, bounded(number_input, 0., f64::INFINITY)).into(), + Some("SignedInteger") => number_widget(default_info, bounded(number_input.int(), f64::NEG_INFINITY, f64::INFINITY)).into(), + Some("SeedValue") => number_widget(default_info, bounded(number_input.int(), 0., f64::INFINITY)).into(), Some("PixelSize") => vec2_widget(default_info, "X", "Y", unit.unwrap_or(" px"), None, false), Some("TextArea") => text_area_widget(default_info).into(), @@ -206,9 +234,9 @@ pub(crate) fn property_from_type( // =============== // PRIMITIVE TYPES // =============== - Some(x) if x == TypeId::of::() || x == TypeId::of::() => number_widget(default_info, number_input.min(min(f64::NEG_INFINITY)).max(max(f64::INFINITY))).into(), - Some(x) if x == TypeId::of::() => number_widget(default_info, number_input.int().min(min(0.)).max(max(f64::from(u32::MAX)))).into(), - Some(x) if x == TypeId::of::() => number_widget(default_info, number_input.int().min(min(0.))).into(), + Some(x) if x == TypeId::of::() || x == TypeId::of::() => number_widget(default_info, bounded(number_input, f64::NEG_INFINITY, f64::INFINITY)).into(), + Some(x) if x == TypeId::of::() => number_widget(default_info, bounded(number_input.int(), 0., f64::from(u32::MAX))).into(), + Some(x) if x == TypeId::of::() => number_widget(default_info, bounded(number_input.int(), 0., f64::INFINITY)).into(), Some(x) if x == TypeId::of::() => bool_widget(default_info, CheckboxInput::default()).into(), Some(x) if x == TypeId::of::() => text_widget(default_info).into(), Some(x) if x == TypeId::of::() => vec2_widget(default_info, "X", "Y", "", None, false), @@ -2295,7 +2323,7 @@ pub(crate) fn generate_node_properties(node_id: NodeId, context: &mut NodeProper return Vec::new(); }; - let mut number_options = (None, None, None); + let mut number_options = NumberOptions::default(); let mut display_decimal_places = None; let mut step = None; let mut unit_suffix = None; @@ -2307,7 +2335,13 @@ pub(crate) fn generate_node_properties(node_id: NodeId, context: &mut NodeProper .get(proto_node_identifier) .and_then(|metadata| metadata.fields.get(input_index)) { - number_options = (field.number_min, field.number_max, field.number_mode_range); + number_options = NumberOptions { + soft_min: field.number_soft_min, + soft_max: field.number_soft_max, + hard_min: field.number_hard_min, + hard_max: field.number_hard_max, + slider: field.number_mode_range, + }; display_decimal_places = field.number_display_decimal_places; unit_suffix = field.unit; step = field.number_step; diff --git a/node-graph/README.md b/node-graph/README.md index 985929a395..bbb91870ba 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, #[soft_min(0.1)] opacity_multiplier: f64) -> Color { +fn opacity(_input: (), #[default(424242)] color: Color, #[range] #[soft(0..100)] 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) } @@ -110,7 +110,7 @@ fn opacity(_input: (), #[default(424242)] color: Color, #[soft_min(0.1)] opacity ## Additional Macro Options -The macro invocation can be extended with additional attributes. The currently supported attributes are (`name`, `path`, `skip_impl`, `category`). When using generics the `#[implementations()]` attribute can be used to automatically populate the node_registry for you. You can also use the `default`, `expose`, `min`, `max` and `range_mode` attributes to influence how the properties are generated. +The macro invocation can be extended with additional attributes. The currently supported attributes are (`name`, `path`, `skip_impl`, `category`). When using generics the `#[implementations()]` attribute can be used to automatically populate the node_registry for you. You can also use the `default`, `expose`, `soft`, `hard`, and `range` attributes to influence how the properties are generated. The `#[soft(a..b)]` and `#[hard(a..b)]` attributes set the slider's suggested extent and its enforced clamp, respectively (either endpoint may be omitted, e.g. `0..` or `..100`; both endpoints are inclusive, so there is no `..=` form), and `#[range]` renders the input as a draggable slider. Values typed into the input may exceed the soft extent but are clamped to the hard bounds, so `#[soft]` is only meaningful together with `#[range]`. ## Executing a document `NodeNetwork` diff --git a/node-graph/libraries/core-types/src/registry.rs b/node-graph/libraries/core-types/src/registry.rs index d9ead39db8..0d3e17cdbd 100644 --- a/node-graph/libraries/core-types/src/registry.rs +++ b/node-graph/libraries/core-types/src/registry.rs @@ -30,9 +30,13 @@ pub struct FieldMetadata { pub widget_override: RegistryWidgetOverride, pub value_source: RegistryValueSource, pub default_type: Option, - pub number_min: Option, - pub number_max: Option, - pub number_mode_range: Option<(f64, f64)>, + /// The slider's suggested extent, from `#[soft(a..b)]`. Typed values may exceed it. + pub number_soft_min: Option, + pub number_soft_max: Option, + /// The enforced clamp, from `#[hard(a..b)]`. Applied to typed values and at eval time. + pub number_hard_min: Option, + pub number_hard_max: Option, + pub number_mode_range: bool, pub number_display_decimal_places: Option, pub number_step: Option, pub unit: Option<&'static str>, diff --git a/node-graph/node-macro/src/codegen.rs b/node-graph/node-macro/src/codegen.rs index b3b04da441..b65024fe64 100644 --- a/node-graph/node-macro/src/codegen.rs +++ b/node-graph/node-macro/src/codegen.rs @@ -196,36 +196,24 @@ pub(crate) fn generate_node_code(crate_ident: &CrateIdent, parsed: &ParsedNodeFn }) .collect(); - let number_min_values: Vec<_> = regular_fields - .iter() - .map(|field| match &field.ty { - ParsedFieldType::Regular(RegularParsedField { 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<_> = regular_fields - .iter() - .map(|field| match &field.ty { - ParsedFieldType::Regular(RegularParsedField { 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(); + let bound_values = |select: fn(&RegularParsedField) -> &Option| -> Vec<_> { + regular_fields + .iter() + .map(|field| match &field.ty { + ParsedFieldType::Regular(regular) => select(regular).as_ref().map_or(quote!(None), |bound| quote!(Some(#bound))), + _ => quote!(None), + }) + .collect() + }; + let number_soft_min_values = bound_values(|field| &field.number_soft_min); + let number_soft_max_values = bound_values(|field| &field.number_soft_max); + let number_hard_min_values = bound_values(|field| &field.number_hard_min); + let number_hard_max_values = bound_values(|field| &field.number_hard_max); let number_mode_range_values: Vec<_> = regular_fields .iter() .map(|field| match &field.ty { - ParsedFieldType::Regular(RegularParsedField { - number_mode_range: Some(number_mode_range), - .. - }) => quote!(Some(#number_mode_range)), - _ => quote!(None), + ParsedFieldType::Regular(RegularParsedField { number_mode_range, .. }) => quote!(#number_mode_range), + _ => quote!(false), }) .collect(); let number_display_decimal_places: Vec<_> = regular_fields @@ -518,8 +506,10 @@ pub(crate) fn generate_node_code(crate_ident: &CrateIdent, parsed: &ParsedNodeFn exposed: #exposed, value_source: #value_sources, default_type: #default_types, - number_min: #number_min_values, - number_max: #number_max_values, + number_soft_min: #number_soft_min_values, + number_soft_max: #number_soft_max_values, + number_hard_min: #number_hard_min_values, + number_hard_max: #number_hard_max_values, number_mode_range: #number_mode_range_values, number_display_decimal_places: #number_display_decimal_places, number_step: #number_step, diff --git a/node-graph/node-macro/src/parsing.rs b/node-graph/node-macro/src/parsing.rs index f0641685aa..09c1217aeb 100644 --- a/node-graph/node-macro/src/parsing.rs +++ b/node-graph/node-macro/src/parsing.rs @@ -7,8 +7,8 @@ use syn::punctuated::Punctuated; use syn::spanned::Spanned; use syn::token::{Comma, RArrow}; use syn::{ - AttrStyle, Attribute, Error, Expr, ExprTuple, FnArg, GenericParam, Ident, ItemFn, Lit, LitFloat, LitInt, LitStr, Meta, Pat, PatIdent, PatType, Path, ReturnType, TraitBound, Type, TypeImplTrait, - TypeParam, TypeParamBound, Visibility, WhereClause, parse_quote, + AttrStyle, Attribute, Error, Expr, FnArg, GenericParam, Ident, ItemFn, Lit, LitFloat, LitInt, LitStr, Meta, Pat, PatIdent, PatType, Path, ReturnType, TraitBound, Type, TypeImplTrait, TypeParam, + TypeParamBound, Visibility, WhereClause, parse_quote, }; use crate::codegen::generate_node_code; @@ -126,7 +126,7 @@ pub enum ParsedFieldType { Node(NodeParsedField), } -/// A numeric bound value accepted by attributes like `#[soft_min]`, `#[hard_min]`, `#[soft_max]`, and `#[hard_max]`. +/// A single numeric endpoint within a `#[soft(..)]` or `#[hard(..)]` bounds range. /// Accepts both integer literals (e.g. `1`, `-1`) and float literals (e.g. `1.`, `-500.`). #[derive(Clone, Debug)] pub struct NumberBound { @@ -180,6 +180,52 @@ impl ToTokens for NumberBound { } } +/// A pair of numeric bounds parsed from the `#[soft(a..b)]` and `#[hard(a..b)]` attributes. +/// Either endpoint may be omitted for an open-ended bound (`a..` or `..b`), and each endpoint +/// independently accepts an integer or float literal (each cast to `f64`), so a mixed range like +/// `0..3.14159` is valid. +/// +/// The operator is always the bare `..`; both endpoints are treated as inclusive (clamping reaches them). +/// Unlike a Rust range there is no `..=` form, `..` is purely this attribute DSL's bounds operator. +#[derive(Clone, Debug)] +pub struct NumberRange { + start: Option, + end: Option, +} + +impl Parse for NumberRange { + fn parse(input: ParseStream) -> syn::Result { + if input.is_empty() { + return Err(input.error("expected a range like `0..100`, `..100`, or `0..`")); + } + + // A leading endpoint is present unless the range opens directly into the `..` operator. + let start = if input.peek(syn::Token![..=]) || input.peek(syn::Token![..]) { + None + } else { + Some(input.parse::()?) + }; + + // Only the bare `..` is accepted. `..=` is rejected even though both endpoints are inclusive here: + // this DSL treats `..` as its own bounds operator, deliberately diverging from Rust's range semantics. + if input.peek(syn::Token![..=]) { + return Err(input.error("use `..` rather than `..=` for number bounds; both endpoints are always inclusive (e.g. `0..100`)")); + } + if !input.peek(syn::Token![..]) { + return Err(input.error("expected a range like `0..100`, `..100`, or `0..`")); + } + input.parse::()?; + + let end = if input.is_empty() { None } else { Some(input.parse::()?) }; + + if start.is_none() && end.is_none() { + return Err(input.error("a bounds range must specify at least a lower or upper bound")); + } + + Ok(NumberRange { start, end }) + } +} + /// a param of any kind, either a concrete type or a generic type with a set of possible types specified via /// `#[implementation(type)]` #[derive(Clone, Debug)] @@ -191,7 +237,8 @@ pub struct RegularParsedField { pub number_soft_max: Option, pub number_hard_min: Option, pub number_hard_max: Option, - pub number_mode_range: Option, + /// Whether the number input renders as a draggable slider (the `#[range]` attribute) rather than the default increment field. + pub number_mode_range: bool, pub implementations: Punctuated, pub gpu_image: bool, } @@ -680,47 +727,27 @@ fn parse_field(pat_ident: PatIdent, ty: Type, attrs: &[Attribute]) -> syn::Resul _ => ParsedValueSource::None, }; - let number_soft_min = extract_attribute(attrs, "soft_min") + // The slider's interactive extent (`#[soft(a..b)]`) and the enforced clamp (`#[hard(a..b)]`), each an + // optionally open-ended range. They decompose into the four bound values used by codegen and the UI. + let number_soft_bounds = extract_attribute(attrs, "soft") .map(|attr| { - attr.parse_args() - .map_err(|e| Error::new_spanned(attr, format!("Invalid numerical `soft_min` value for argument '{ident}': {e}"))) + attr.parse_args::() + .map_err(|e| Error::new_spanned(attr, format!("Invalid `soft` bounds for argument '{ident}': {e}\nUSAGE EXAMPLE: #[soft(0..100)]"))) }) .transpose()?; - let number_soft_max = extract_attribute(attrs, "soft_max") + let number_hard_bounds = extract_attribute(attrs, "hard") .map(|attr| { - attr.parse_args() - .map_err(|e| Error::new_spanned(attr, format!("Invalid numerical `soft_max` value for argument '{ident}': {e}"))) + attr.parse_args::() + .map_err(|e| Error::new_spanned(attr, format!("Invalid `hard` bounds for argument '{ident}': {e}\nUSAGE EXAMPLE: #[hard(0..100)]"))) }) .transpose()?; + let number_soft_min = number_soft_bounds.as_ref().and_then(|range| range.start.clone()); + let number_soft_max = number_soft_bounds.as_ref().and_then(|range| range.end.clone()); + let number_hard_min = number_hard_bounds.as_ref().and_then(|range| range.start.clone()); + let number_hard_max = number_hard_bounds.as_ref().and_then(|range| range.end.clone()); - let number_hard_min = extract_attribute(attrs, "hard_min") - .map(|attr| { - attr.parse_args() - .map_err(|e| Error::new_spanned(attr, format!("Invalid numerical `hard_min` value for argument '{ident}': {e}"))) - }) - .transpose()?; - let number_hard_max = extract_attribute(attrs, "hard_max") - .map(|attr| { - attr.parse_args() - .map_err(|e| Error::new_spanned(attr, format!("Invalid numerical `hard_max` value for argument '{ident}': {e}"))) - }) - .transpose()?; - - let number_mode_range = extract_attribute(attrs, "range") - .map(|attr| { - attr.parse_args::().map_err(|e| { - Error::new_spanned( - attr, - format!("Invalid `range` tuple of min and max range slider values for argument '{ident}': {e}\nUSAGE EXAMPLE: #[range((0., 100.))]"), - ) - }) - }) - .transpose()?; - if let Some(range) = &number_mode_range - && range.elems.len() != 2 - { - return Err(Error::new_spanned(range, "Expected a tuple of two values for `range` for the min and max, respectively")); - } + // The `#[range]` marker selects the slider widget; its extent is derived from the soft (then hard) bounds. + let number_mode_range = extract_attribute(attrs, "range").is_some(); let unit = extract_attribute(attrs, "unit") .map(|attr| attr.parse_args::().map_err(|_e| Error::new_spanned(attr, "Expected a unit type as string".to_string()))) @@ -810,15 +837,15 @@ fn parse_field(pat_ident: PatIdent, ty: Type, attrs: &[Attribute]) -> syn::Resul .transpose()? .unwrap_or_default(); - // Error if a float literal is given for a bound attribute on an integer-typed field + // Error if a float literal is given for a bound on an integer-typed field if is_integer_type(&ty) { let bound_attrs = [ - (&number_soft_min, "soft_min"), - (&number_hard_min, "hard_min"), - (&number_soft_max, "soft_max"), - (&number_hard_max, "hard_max"), + (&number_soft_min, "soft", "lower"), + (&number_soft_max, "soft", "upper"), + (&number_hard_min, "hard", "lower"), + (&number_hard_max, "hard", "upper"), ]; - for (bound, attr_name) in bound_attrs { + for (bound, attr_name, end) in bound_attrs { if let Some(NumberBound { literal: NumberBoundLiteral::Float(_), .. @@ -826,7 +853,7 @@ fn parse_field(pat_ident: PatIdent, ty: Type, attrs: &[Attribute]) -> syn::Resul { return Err(Error::new_spanned( &pat_ident, - format!("Attribute `#[{attr_name}]` on `{ident}` has a float literal, but `{ident}` is an integer type. Use an integer literal without a decimal point."), + format!("The {end} `#[{attr_name}]` bound on `{ident}` is a float literal, but `{ident}` is an integer type. Use an integer literal without a decimal point."), )); } } @@ -1082,7 +1109,7 @@ mod tests { number_soft_max: None, number_hard_min: None, number_hard_max: None, - number_mode_range: None, + number_mode_range: false, implementations: Punctuated::new(), gpu_image: false, }), @@ -1168,7 +1195,7 @@ mod tests { number_soft_max: None, number_hard_min: None, number_hard_max: None, - number_mode_range: None, + number_mode_range: false, implementations: Punctuated::new(), gpu_image: false, }), @@ -1236,7 +1263,7 @@ mod tests { number_soft_max: None, number_hard_min: None, number_hard_max: None, - number_mode_range: None, + number_mode_range: false, implementations: Punctuated::new(), gpu_image: false, }), @@ -1302,7 +1329,7 @@ mod tests { number_soft_max: None, number_hard_min: None, number_hard_max: None, - number_mode_range: None, + number_mode_range: false, implementations: { let mut p = Punctuated::new(); p.push(parse_quote!(f32)); @@ -1330,9 +1357,9 @@ mod tests { fn add( a: f64, /// b - #[range((0., 100.))] - #[soft_min(-500.)] - #[soft_max(500.)] + #[range] + #[soft(0..100)] + #[hard(-500..500)] b: f64, ) -> f64 { a + b @@ -1376,11 +1403,11 @@ mod tests { ty: parse_quote!(f64), exposed: false, value_source: ParsedValueSource::None, - 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.))), + number_soft_min: Some(parse_quote!(0)), + number_soft_max: Some(parse_quote!(100)), + number_hard_min: Some(parse_quote!(-500)), + number_hard_max: Some(parse_quote!(500)), + number_mode_range: true, implementations: Punctuated::new(), gpu_image: false, }), @@ -1396,6 +1423,21 @@ mod tests { assert_parsed_node_fn(&parsed, &expected); } + #[test] + fn test_empty_bounds_range() { + let attr = quote!(category("Math: Arithmetic")); + let input = quote!( + fn add(a: f64, #[soft()] b: f64) -> f64 { + a + b + } + ); + + let result = parse_node_fn(attr, input); + assert!(result.is_err()); + let error_message = result.unwrap_err().to_string(); + assert!(error_message.contains("expected a range like `0..100`, `..100`, or `0..`")); + } + #[test] fn test_async_node() { let attr = quote!(category("IO")); @@ -1446,7 +1488,7 @@ mod tests { number_soft_max: None, number_hard_min: None, number_hard_max: None, - number_mode_range: None, + number_mode_range: false, implementations: Punctuated::new(), gpu_image: false, }), diff --git a/node-graph/node-macro/src/shader_nodes/per_pixel_adjust.rs b/node-graph/node-macro/src/shader_nodes/per_pixel_adjust.rs index 20ee1ec731..97f87fc4db 100644 --- a/node-graph/node-macro/src/shader_nodes/per_pixel_adjust.rs +++ b/node-graph/node-macro/src/shader_nodes/per_pixel_adjust.rs @@ -238,7 +238,7 @@ impl PerPixelAdjustCodegen<'_> { number_soft_max: None, number_hard_min: None, number_hard_max: None, - number_mode_range: None, + number_mode_range: false, implementations: Default::default(), gpu_image: false, }), diff --git a/node-graph/node-macro/src/validation.rs b/node-graph/node-macro/src/validation.rs index 79ad327910..7b51b31efa 100644 --- a/node-graph/node-macro/src/validation.rs +++ b/node-graph/node-macro/src/validation.rs @@ -10,6 +10,7 @@ pub fn validate_node_fn(parsed: &ParsedNodeFn) -> syn::Result<()> { validate_implementations_for_generics, validate_primary_input_expose, validate_min_max, + validate_range_slider_bounds, ]; for validator in validators { @@ -39,18 +40,18 @@ fn validate_min_max(parsed: &ParsedNodeFn) { if soft_min_value == hard_min_value { emit_error!( pat_ident.span(), - "Unnecessary #[soft_min] attribute on `{}`, as #[hard_min] has the same value.", + "Redundant lower bound on `{}`: the #[soft] and #[hard] lower bounds are equal.", 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].", + help = "Drop the lower bound from #[soft] and let the slider fall back to #[hard]."; + note = "A soft bound only matters when it sits inside the corresponding hard bound.", ); } else if soft_min_value < hard_min_value { emit_error!( pat_ident.span(), - "The #[soft_min] attribute on `{}` is incorrectly greater than #[hard_min].", + "The #[soft] lower bound on `{}` is below the #[hard] lower bound.", 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.", + help = "The soft (slider) range must stay within the hard (clamped) range."; + note = "Letting the slider range precede #[hard]'s lower bound doesn't make sense.", ); } } @@ -61,18 +62,18 @@ fn validate_min_max(parsed: &ParsedNodeFn) { if soft_max_value == hard_max_value { emit_error!( pat_ident.span(), - "Unnecessary #[soft_max] attribute on `{}`, as #[hard_max] has the same value.", + "Redundant upper bound on `{}`: the #[soft] and #[hard] upper bounds are equal.", 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].", + help = "Drop the upper bound from #[soft] and let the slider fall back to #[hard]."; + note = "A soft bound only matters when it sits inside the corresponding hard bound.", ); - } else if soft_max_value < hard_max_value { + } else if soft_max_value > hard_max_value { emit_error!( pat_ident.span(), - "The #[soft_max] attribute on `{}` is incorrectly greater than #[hard_max].", + "The #[soft] upper bound on `{}` is above the #[hard] upper bound.", 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.", + help = "The soft (slider) range must stay within the hard (clamped) range."; + note = "Letting the slider range exceed #[hard]'s upper bound doesn't make sense.", ); } } @@ -80,6 +81,44 @@ fn validate_min_max(parsed: &ParsedNodeFn) { } } +/// A `#[range]` slider needs a defined extent on both ends. The extent comes from `#[soft]` when present, +/// otherwise it falls back to `#[hard]`, so each end must be covered by at least one of the two attributes. +fn validate_range_slider_bounds(parsed: &ParsedNodeFn) { + for field in &parsed.fields { + if let ParsedField { + ty: ParsedFieldType::Regular(RegularParsedField { + number_mode_range: true, + number_soft_min, + number_soft_max, + number_hard_min, + number_hard_max, + .. + }), + pat_ident, + .. + } = field + { + let min_bounded = number_soft_min.is_some() || number_hard_min.is_some(); + let max_bounded = number_soft_max.is_some() || number_hard_max.is_some(); + + let missing = match (min_bounded, max_bounded) { + (true, true) => continue, + (false, false) => "lower and upper bounds", + (false, true) => "a lower bound", + (true, false) => "an upper bound", + }; + + emit_error!( + pat_ident.span(), + "The #[range] slider on `{}` is missing {}.", + pat_ident.ident, missing; + help = "A slider needs both ends defined; add the missing bound via #[soft(..)] or #[hard(..)], e.g. #[soft(0..100)]."; + note = "The slider's extent comes from #[soft] if present, otherwise #[hard].", + ); + } + } +} + fn validate_primary_input_expose(parsed: &ParsedNodeFn) { if let Some(ParsedField { ty: ParsedFieldType::Regular(RegularParsedField { exposed: true, .. }), diff --git a/node-graph/nodes/graphic/src/graphic.rs b/node-graph/nodes/graphic/src/graphic.rs index 9275be1332..74c003bc9c 100644 --- a/node-graph/nodes/graphic/src/graphic.rs +++ b/node-graph/nodes/graphic/src/graphic.rs @@ -156,7 +156,9 @@ async fn mirror( content: List, #[default(ReferencePoint::Center)] relative_to_bounds: ReferencePoint, #[unit(" px")] offset: f64, - #[range((-90., 90.))] angle: Angle, + #[range] + #[soft(-90..90)] + angle: Angle, #[default(true)] keep_original: bool, ) -> List where diff --git a/node-graph/nodes/gstd/src/text.rs b/node-graph/nodes/gstd/src/text.rs index 8f229def0b..1d537db016 100644 --- a/node-graph/nodes/gstd/src/text.rs +++ b/node-graph/nodes/gstd/src/text.rs @@ -22,13 +22,13 @@ fn text( /// The font size used to draw the text. #[unit(" px")] #[default(24.)] - #[hard_min(1.)] + #[hard(1..)] size: f64, /// The line height ratio, relative to the font size. Each line is drawn lower than its previous line by the distance of *Size* × *Line Height*. /// /// 0 means all lines overlap. 1 means all lines are spaced by just the font size. 1.2 is a common default for readable text. 2 means double-spaced text. #[unit("x")] - #[hard_min(0.)] + #[hard(0..)] #[step(0.1)] #[default(1.2)] line_height: f64, @@ -38,15 +38,14 @@ fn text( letter_spacing: f64, /// The angle of faux italic slant applied to each glyph. #[unit("°")] - #[hard_min(-85.)] - #[hard_max(85.)] + #[hard(-85..85)] letter_tilt: f64, /// Enables the maximum width constraint so lines can wrap. #[widget(ParsedWidgetOverride::Hidden)] has_max_width: bool, /// The maximum width that the text block can occupy before wrapping to a new line. Otherwise, lines do not wrap. #[unit(" px")] - #[hard_min(1.)] + #[hard(1..)] #[widget(ParsedWidgetOverride::Custom = "optional_f64")] max_width: f64, /// Whether the *Max Height* property is enabled so that lines beyond it are not drawn. @@ -54,7 +53,7 @@ fn text( has_max_height: bool, /// The maximum height that the text block can occupy. Excess lines are not drawn. #[unit(" px")] - #[hard_min(1.)] + #[hard(1..)] #[widget(ParsedWidgetOverride::Custom = "optional_f64")] max_height: f64, /// The horizontal alignment of each line of text within its surrounding box. To have an effect on a single line of text, *Max Width* must be set. diff --git a/node-graph/nodes/raster/src/adjustments.rs b/node-graph/nodes/raster/src/adjustments.rs index 34ea4fd4e0..ca76e82b65 100644 --- a/node-graph/nodes/raster/src/adjustments.rs +++ b/node-graph/nodes/raster/src/adjustments.rs @@ -85,8 +85,9 @@ fn gamma_correction>( #[gpu_image] mut input: T, #[default(2.2)] - #[range((0.01, 10.))] - #[hard_min(0.0001)] + #[range] + #[hard(0.0001..)] + #[soft(0.01..10)] gamma: f32, inverse: bool, ) -> T { @@ -344,22 +345,28 @@ fn black_and_white>( mut image: T, #[default(Color::BLACK)] tint: Color, #[default(40.)] - #[range((-200., 300.))] + #[range] + #[soft(-200..300)] reds: PercentageF32, #[default(60.)] - #[range((-200., 300.))] + #[range] + #[soft(-200..300)] yellows: PercentageF32, #[default(40.)] - #[range((-200., 300.))] + #[range] + #[soft(-200..300)] greens: PercentageF32, #[default(60.)] - #[range((-200., 300.))] + #[range] + #[soft(-200..300)] cyans: PercentageF32, #[default(20.)] - #[range((-200., 300.))] + #[range] + #[soft(-200..300)] blues: PercentageF32, #[default(80.)] - #[range((-200., 300.))] + #[range] + #[soft(-200..300)] magentas: PercentageF32, ) -> T { image.adjust(|color| { @@ -997,12 +1004,11 @@ fn posterize>( #[gpu_image] mut input: T, #[default(4)] - #[hard_min(2)] + #[hard(2..)] levels: u32, ) -> T { + let levels = levels as f32; input.adjust(|color| { - // `hard_min(2)` constrains the widget but doesn't bind the data-flow input (a saved doc or upstream node could still feed 0 or 1, producing inf/NaN below). - let levels = (levels as f32).max(2.); let number_of_areas = levels.recip(); let size_of_areas = (levels - 1.).recip(); color.map_gamma_rgb(|c| (c / number_of_areas).floor() * size_of_areas) @@ -1029,8 +1035,9 @@ fn exposure>( exposure: f32, offset: f32, #[default(1.)] - #[range((0.01, 10.))] - #[hard_min(0.0001)] + #[range] + #[hard(0.0001..)] + #[soft(0.01..10)] gamma_correction: f32, ) -> T { input.adjust(|color| { diff --git a/node-graph/nodes/raster/src/filter.rs b/node-graph/nodes/raster/src/filter.rs index 2e42ad9b66..373298a016 100644 --- a/node-graph/nodes/raster/src/filter.rs +++ b/node-graph/nodes/raster/src/filter.rs @@ -92,8 +92,9 @@ async fn blur( /// The image to be blurred. image_frame: List>, /// The radius of the blur kernel. - #[range((0., 100.))] - #[hard_min(0.)] + #[range] + #[hard(0..)] + #[soft(..100)] radius: PixelLength, /// Use a lower-quality box kernel instead of a circular Gaussian kernel. This is faster but produces boxy artifacts. box_blur: bool, @@ -128,8 +129,9 @@ async fn median_filter( /// The image to be filtered. image_frame: List>, /// The radius of the filter kernel. Larger values remove more noise but may blur fine details. - #[range((0., 50.))] - #[hard_min(0.)] + #[range] + #[hard(0..)] + #[soft(..50)] radius: PixelLength, ) -> List> { image_frame diff --git a/node-graph/nodes/raster/src/image_color_palette.rs b/node-graph/nodes/raster/src/image_color_palette.rs index 01e460f795..240e51d5ff 100644 --- a/node-graph/nodes/raster/src/image_color_palette.rs +++ b/node-graph/nodes/raster/src/image_color_palette.rs @@ -8,7 +8,7 @@ async fn image_color_palette( _: impl Ctx, image: List>, #[default(4)] - #[hard_min(1)] + #[hard(1..)] count: u32, ) -> List { const GRID: f32 = 3.; diff --git a/node-graph/nodes/repeat/src/repeat_nodes.rs b/node-graph/nodes/repeat/src/repeat_nodes.rs index b4ebefac84..a4137dc978 100644 --- a/node-graph/nodes/repeat/src/repeat_nodes.rs +++ b/node-graph/nodes/repeat/src/repeat_nodes.rs @@ -20,13 +20,13 @@ async fn repeat + Default + Send + Clone + 'static>( )] content: impl Node<'n, Context<'static>, Output = List>, #[default(1)] - #[hard_min(1)] + #[hard(1..)] count: u32, reverse: bool, ) -> List { // Someday this node can have the option to generate infinitely instead of a fixed count (basically `std::iter::repeat`). - let count = count.max(1) as usize; + let count = count as usize; let mut result_list = List::new(); @@ -60,12 +60,12 @@ pub async fn repeat_array + Default + Send + Clone + 'static>( direction: PixelSize, angle: Angle, #[default(5)] - #[hard_min(1)] + #[hard(1..)] count: u32, ) -> List { let angle = angle.to_radians(); - let count = count.max(1); - let total = (count - 1) as f64; + // A single copy has no steps between copies, so the denominator is kept at 1 to avoid `0. / 0.` producing a NaN transform + let total = (count - 1).max(1) as f64; let mut result_list = List::new(); @@ -108,11 +108,9 @@ async fn repeat_radial + Default + Send + Clone + 'static>( #[default(5)] radius: f64, #[default(5)] - #[hard_min(1)] + #[hard(1..)] count: u32, ) -> List { - let count = count.max(1); - let mut result_list = List::new(); for index in 0..count { @@ -265,6 +263,26 @@ mod test { } } + #[tokio::test] + async fn repeat_single_copy() { + let context = OwnedContextImpl::default().into_context(); + let repeated = super::repeat_array( + context, + &FutureWrapperNode(vector_node_from_bezpath(Rect::new(0., 0., 1., 1.).to_path(DEFAULT_ACCURACY))), + DVec2::new(12., 10.), + 45., + 1, + ) + .await; + let vector_list = vector_nodes::flatten_path(Footprint::default(), repeated).await; + let vector = vector_list.element(0).unwrap(); + assert_eq!(vector.region_manipulator_groups().count(), 1); + + let (_, manipulator_groups) = vector.region_manipulator_groups().next().unwrap(); + let anchor = manipulator_groups[0].anchor; + assert!(anchor.length() < 1e-5, "Expected the single copy to be untransformed, found anchor {anchor}"); + } + #[tokio::test] async fn repeat_transform_position() { let direction = DVec2::new(12., 10.); diff --git a/node-graph/nodes/text/src/lib.rs b/node-graph/nodes/text/src/lib.rs index 742e12cb04..489ce33d54 100644 --- a/node-graph/nodes/text/src/lib.rs +++ b/node-graph/nodes/text/src/lib.rs @@ -424,7 +424,7 @@ fn string_repeat( string: String, /// The number of times the string should appear in the output. #[default(2)] - #[hard_min(1)] + #[hard(1..)] count: u32, /// The string placed between each repetition. #[default("\\n")] @@ -436,7 +436,7 @@ fn string_repeat( ) -> String { let separator = if separator_escaping { unescape_string(separator) } else { separator }; - let count = count.max(1) as usize; + let count = count as usize; let mut result = String::with_capacity((string.len() + separator.len()) * count); for i in 0..count { diff --git a/node-graph/nodes/vector/src/generator_nodes.rs b/node-graph/nodes/vector/src/generator_nodes.rs index e3abf2005a..30c4e8c3d7 100644 --- a/node-graph/nodes/vector/src/generator_nodes.rs +++ b/node-graph/nodes/vector/src/generator_nodes.rs @@ -77,7 +77,8 @@ fn arc( radius: f64, start_angle: Angle, #[default(270.)] - #[range((0., 360.))] + #[range] + #[soft(0..360)] sweep_angle: Angle, arc_type: ArcType, ) -> List { @@ -167,7 +168,7 @@ fn regular_polygon( _: impl Ctx, _primary: (), #[default(6)] - #[hard_min(3.)] + #[hard(3..)] #[implementations(u32, u64, f64)] sides: T, #[unit(" px")] @@ -185,7 +186,7 @@ fn star( _: impl Ctx, _primary: (), #[default(5)] - #[hard_min(2.)] + #[hard(2..)] #[implementations(u32, u64, f64)] sides: T, #[unit(" px")] @@ -228,7 +229,7 @@ fn qr_code( text: String, #[widget(ParsedWidgetOverride::Hidden)] has_size: bool, #[unit(" px")] - #[hard_min(1.)] + #[hard(1..)] #[widget(ParsedWidgetOverride::Custom = "optional_f64")] size: f64, error_correction: QRCodeErrorCorrectionLevel, @@ -267,7 +268,7 @@ fn qr_code( }; if has_size { - vector.transform(glam::DAffine2::from_scale(DVec2::splat(size.max(1.) / qr_code.size() as f64))); + vector.transform(glam::DAffine2::from_scale(DVec2::splat(size / qr_code.size() as f64))); } List::new_from_element(vector) @@ -312,7 +313,7 @@ fn grid( _primary: (), grid_type: GridType, #[unit(" px")] - #[hard_min(0.)] + #[hard(0..)] #[default(10)] #[implementations(f64, DVec2)] spacing: T, diff --git a/node-graph/nodes/vector/src/vector_nodes.rs b/node-graph/nodes/vector/src/vector_nodes.rs index 47d5455e1b..dc06ec5870 100644 --- a/node-graph/nodes/vector/src/vector_nodes.rs +++ b/node-graph/nodes/vector/src/vector_nodes.rs @@ -263,21 +263,25 @@ async fn copy_to_points( content: List, /// Minimum range of randomized sizes given to each placed copy. #[default(1)] - #[range((0., 2.))] + #[range] + #[soft(0..2)] #[unit("x")] random_scale_min: Multiplier, /// Maximum range of randomized sizes given to each placed copy. #[default(1)] - #[range((0., 2.))] + #[range] + #[soft(0..2)] #[unit("x")] random_scale_max: Multiplier, /// Bias for the probability distribution of randomized sizes (0 is uniform, negatives favor more of small sizes, positives favor more of large sizes). - #[range((-50., 50.))] + #[range] + #[soft(-50..50)] random_scale_bias: f64, /// Seed to determine unique variations on all the randomized copy sizes. random_scale_seed: SeedValue, /// Range of randomized angles given to each placed copy, in degrees ranging from furthest clockwise to counterclockwise. - #[range((0., 360.))] + #[range] + #[soft(0..360)] random_rotation: Angle, /// Seed to determine unique variations on all the randomized copy angles. random_rotation_seed: SeedValue, @@ -337,18 +341,16 @@ async fn copy_to_points( async fn round_corners( _: impl Ctx, source: List, - #[hard_min(0.)] + #[hard(0..)] #[default(10.)] radius: PixelLength, - #[range((0., 1.))] - #[hard_min(0.)] - #[hard_max(1.)] + #[range] + #[hard(0..1)] #[default(0.5)] roundness: f64, #[default(100.)] edge_length_limit: Percentage, - #[range((0., 180.))] - #[hard_min(0.)] - #[hard_max(180.)] + #[range] + #[hard(0..180)] #[default(5.)] min_angle_threshold: Angle, ) -> List { @@ -452,7 +454,7 @@ pub fn merge_by_distance( _: impl Ctx, content: List, #[default(0.1)] - #[hard_min(0.0001)] + #[hard(0.0001..)] distance: PixelLength, algorithm: MergeByDistanceAlgorithm, ) -> List { @@ -892,8 +894,8 @@ async fn auto_tangents( source: List, /// The amount of spread for the auto-tangents, from 0 (sharp corner) to 1 (full spread). #[default(0.5)] - // TODO: Make this a soft range to allow any value to be typed in outside the slider range of 0 to 1 - #[range((0., 1.))] + #[range] + #[soft(0..1)] spread: f64, /// If active, existing non-zero handles won't be affected. #[default(true)] @@ -1376,16 +1378,16 @@ async fn sample_polyline( content: List, spacing: PointSpacingType, #[default(100.)] - #[hard_min(0.)] + #[hard(0..)] #[unit(" px")] separation: f64, #[default(100)] - #[hard_min(2)] + #[hard(2..)] quantity: u32, - #[hard_min(0.)] + #[hard(0..)] #[unit(" px")] start_offset: f64, - #[hard_min(0.)] + #[hard(0..)] #[unit(" px")] stop_offset: f64, adaptive_spacing: bool, @@ -1830,8 +1832,9 @@ async fn scatter_points( content: List, #[unit(" px")] #[default(10.)] - #[hard_min(0.01)] - #[range((1., 100.))] + #[range] + #[hard(0.01..)] + #[soft(1..100)] separation: f64, seed: SeedValue, ) -> List {