From 9d15e56ce186f2891757ec923db3225a8e2d07a8 Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Thu, 18 Sep 2025 16:37:01 -0700 Subject: [PATCH] Fix and clean up typing-related flaws with the Math category of nodes --- .../messages/portfolio/document_migration.rs | 1 + node-graph/gcore/src/graphic.rs | 2 +- node-graph/gcore/src/logic.rs | 28 ++- node-graph/gcore/src/ops.rs | 3 +- node-graph/gmath-nodes/src/lib.rs | 170 +++++++++--------- .../interpreted-executor/src/node_registry.rs | 4 + 6 files changed, 112 insertions(+), 96 deletions(-) diff --git a/editor/src/messages/portfolio/document_migration.rs b/editor/src/messages/portfolio/document_migration.rs index e0e04b2b..4a3143cc 100644 --- a/editor/src/messages/portfolio/document_migration.rs +++ b/editor/src/messages/portfolio/document_migration.rs @@ -1060,6 +1060,7 @@ fn migrate_node(node_id: &NodeId, node: &DocumentNode, network_path: &[NodeId], if reference == "Instance Index" && inputs_count == 0 { let mut node_template = resolve_document_node_type(reference)?.default_node_template(); document.network_interface.replace_implementation(node_id, network_path, &mut node_template); + document.network_interface.set_display_name(node_id, "Instance Index".to_string(), network_path); let mut node_path = network_path.to_vec(); node_path.push(*node_id); diff --git a/node-graph/gcore/src/graphic.rs b/node-graph/gcore/src/graphic.rs index 324c93e3..bd7a4677 100644 --- a/node-graph/gcore/src/graphic.rs +++ b/node-graph/gcore/src/graphic.rs @@ -338,7 +338,7 @@ async fn wrap_graphic + 'n>( /// Converts a table of graphical content into a graphic table by placing it into an element of a new wrapper graphic table. /// If it is already a graphic table, it is not wrapped again. Use the 'Wrap Graphic' node if wrapping is always desired. -#[node_macro::node(category("Type Conversion"))] +#[node_macro::node(category("General"))] async fn to_graphic> + 'n>( _: impl Ctx, #[implementations( diff --git a/node-graph/gcore/src/logic.rs b/node-graph/gcore/src/logic.rs index c131a0ec..efcd4a7f 100644 --- a/node-graph/gcore/src/logic.rs +++ b/node-graph/gcore/src/logic.rs @@ -9,9 +9,9 @@ use crate::vector::Vector; use crate::{Context, Ctx}; use glam::{DAffine2, DVec2}; -#[node_macro::node(category("Type Conversion"))] -fn to_string(_: impl Ctx, #[implementations(bool, f64, u32, u64, DVec2, DAffine2, String)] value: T) -> String { - format!("{value:?}") +#[node_macro::node(category("Debug"))] +fn to_string(_: impl Ctx, value: String) -> String { + value } #[node_macro::node(category("Text"))] @@ -34,10 +34,24 @@ fn string_replace(_: impl Ctx, string: String, from: TextArea, to: TextArea) -> #[node_macro::node(category("Text"))] fn string_slice(_: impl Ctx, string: String, start: f64, end: f64) -> String { - let start = if start < 0. { string.len() - start.abs() as usize } else { start as usize }; - let end = if end <= 0. { string.len() - end.abs() as usize } else { end as usize }; - let n = end.saturating_sub(start); - string.char_indices().skip(start).take(n).map(|(_, c)| c).collect() + let total_chars = string.chars().count(); + + let start = if start < 0. { + total_chars.saturating_sub(start.abs() as usize) + } else { + (start as usize).min(total_chars) + }; + let end = if end <= 0. { + total_chars.saturating_sub(end.abs() as usize) + } else { + (end as usize).min(total_chars) + }; + + if start >= end { + return String::new(); + } + + string.chars().skip(start).take(end - start).collect() } // TODO: Return u32, u64, or usize instead of f64 after #1621 is resolved and has allowed us to implement automatic type conversion in the node graph for nodes with generic type inputs. diff --git a/node-graph/gcore/src/ops.rs b/node-graph/gcore/src/ops.rs index c80a1adb..79de790b 100644 --- a/node-graph/gcore/src/ops.rs +++ b/node-graph/gcore/src/ops.rs @@ -1,6 +1,5 @@ -use graphene_core_shaders::Ctx; - use crate::Node; +use graphene_core_shaders::Ctx; use std::marker::PhantomData; // TODO: Rename to "Passthrough" diff --git a/node-graph/gmath-nodes/src/lib.rs b/node-graph/gmath-nodes/src/lib.rs index caf2e19c..88ced629 100644 --- a/node-graph/gmath-nodes/src/lib.rs +++ b/node-graph/gmath-nodes/src/lib.rs @@ -33,24 +33,24 @@ impl ValueProvider for MathNodeContext { /// Calculates a mathematical expression with input values "A" and "B" #[node_macro::node(category("Math: Arithmetic"), properties("math_properties"))] -fn math( +fn math( _: impl Ctx, /// The value of "A" when calculating the expression #[implementations(f64, f32)] - operand_a: U, + operand_a: T, /// A math expression that may incorporate "A" and/or "B", such as "sqrt(A + B) - B^2" #[default(A + B)] expression: String, /// The value of "B" when calculating the expression #[implementations(f64, f32)] #[default(1.)] - operand_b: U, -) -> U { + operand_b: T, +) -> T { let (node, _unit) = match ast::Node::try_parse_from_str(&expression) { Ok(expr) => expr, Err(e) => { warn!("Invalid expression: `{expression}`\n{e:?}"); - return U::from(0.).unwrap(); + return T::from(0.).unwrap(); } }; let context = EvalContext::new( @@ -65,14 +65,14 @@ fn math( Ok(value) => value, Err(e) => { warn!("Expression evaluation error: {e:?}"); - return U::from(0.).unwrap(); + return T::from(0.).unwrap(); } }; let Value::Number(num) = value; match num { - Number::Real(val) => U::from(val).unwrap(), - Number::Complex(c) => U::from(c.re).unwrap(), + Number::Real(val) => T::from(val).unwrap(), + Number::Complex(c) => T::from(c.re).unwrap(), } } @@ -109,11 +109,11 @@ fn subtract, T>( fn multiply, T>( _: impl Ctx, /// The left-hand side of the multiplication operation. - #[implementations(f64, f32, u32, f64, DVec2, DVec2, DAffine2)] + #[implementations(f64, f32, u32, DVec2, f64, DVec2, DAffine2)] multiplier: U, /// The right-hand side of the multiplication operation. #[default(1.)] - #[implementations(f64, f32, u32, DVec2, f64, DVec2, DAffine2)] + #[implementations(f64, f32, u32, DVec2, DVec2, f64, DAffine2)] multiplicand: T, ) -> >::Output { multiplier * multiplicand @@ -126,11 +126,11 @@ fn multiply, T>( fn divide + Default + PartialEq, T: Default + PartialEq>( _: impl Ctx, /// The left-hand side of the division operation. - #[implementations(f64, f64, f32, f32, u32, u32, DVec2, DVec2, f64)] + #[implementations(f64, f32, u32, DVec2, DVec2, f64)] numerator: U, /// The right-hand side of the division operation. #[default(1.)] - #[implementations(f64, f64, f32, f32, u32, u32, DVec2, f64, DVec2)] + #[implementations(f64, f32, u32, DVec2, f64, DVec2)] denominator: T, ) -> >::Output where @@ -162,58 +162,58 @@ fn modulo>>, T: Copy /// The exponent operation (^) calculates the result of raising a number to a power. #[node_macro::node(category("Math: Arithmetic"))] -fn exponent, T>( +fn exponent>( _: impl Ctx, /// The base number that will be raised to the power. #[implementations(f64, f32, u32)] - base: U, + base: T, /// The power to which the base number will be raised. - #[default(2.)] #[implementations(f64, f32, u32)] + #[default(2.)] power: T, -) -> >::Output { +) -> >::Output { base.pow(power) } /// The square root operation (√) calculates the nth root of a number, equivalent to raising the number to the power of 1/n. #[node_macro::node(category("Math: Arithmetic"))] -fn root( +fn root( _: impl Ctx, /// The number for which the nth root will be calculated. #[default(2.)] #[implementations(f64, f32)] - radicand: U, + radicand: T, /// The degree of the root to be calculated. Square root is 2, cube root is 3, and so on. #[default(2.)] #[implementations(f64, f32)] - degree: U, -) -> U { - if degree == U::from(2.).unwrap() { + degree: T, +) -> T { + if degree == T::from(2.).unwrap() { radicand.sqrt() - } else if degree == U::from(3.).unwrap() { + } else if degree == T::from(3.).unwrap() { radicand.cbrt() } else { - radicand.powf(U::from(1.).unwrap() / degree) + radicand.powf(T::from(1.).unwrap() / degree) } } /// The logarithmic function (log) calculates the logarithm of a number with a specified base. If the natural logarithm function (ln) is desired, set the base to "e". #[node_macro::node(category("Math: Arithmetic"))] -fn logarithm( +fn logarithm( _: impl Ctx, /// The number for which the logarithm will be calculated. #[implementations(f64, f32)] - value: U, + value: T, /// The base of the logarithm, such as 2 (binary), 10 (decimal), and e (natural logarithm). #[default(2.)] #[implementations(f64, f32)] - base: U, -) -> U { - if base == U::from(2.).unwrap() { + base: T, +) -> T { + if base == T::from(2.).unwrap() { value.log2() - } else if base == U::from(10.).unwrap() { + } else if base == T::from(10.).unwrap() { value.log10() - } else if base - U::from(std::f64::consts::E).unwrap() < U::epsilon() * U::from(1e6).unwrap() { + } else if base - T::from(std::f64::consts::E).unwrap() < T::epsilon() * T::from(1e6).unwrap() { value.ln() } else { value.log(base) @@ -222,66 +222,66 @@ fn logarithm( /// The sine trigonometric function (sin) calculates the ratio of the angle's opposite side length to its hypotenuse length. #[node_macro::node(category("Math: Trig"))] -fn sine( +fn sine( _: impl Ctx, /// The given angle. #[implementations(f64, f32)] - theta: U, + theta: T, /// Whether the given angle should be interpreted as radians instead of degrees. radians: bool, -) -> U { +) -> T { if radians { theta.sin() } else { theta.to_radians().sin() } } /// The cosine trigonometric function (cos) calculates the ratio of the angle's adjacent side length to its hypotenuse length. #[node_macro::node(category("Math: Trig"))] -fn cosine( +fn cosine( _: impl Ctx, /// The given angle. #[implementations(f64, f32)] - theta: U, + theta: T, /// Whether the given angle should be interpreted as radians instead of degrees. radians: bool, -) -> U { +) -> T { if radians { theta.cos() } else { theta.to_radians().cos() } } /// The tangent trigonometric function (tan) calculates the ratio of the angle's opposite side length to its adjacent side length. #[node_macro::node(category("Math: Trig"))] -fn tangent( +fn tangent( _: impl Ctx, /// The given angle. #[implementations(f64, f32)] - theta: U, + theta: T, /// Whether the given angle should be interpreted as radians instead of degrees. radians: bool, -) -> U { +) -> T { if radians { theta.tan() } else { theta.to_radians().tan() } } /// The inverse sine trigonometric function (asin) calculates the angle whose sine is the specified value. #[node_macro::node(category("Math: Trig"))] -fn sine_inverse( +fn sine_inverse( _: impl Ctx, /// The given value for which the angle will be calculated. Must be in the range [-1, 1] or else the result will be NaN. #[implementations(f64, f32)] - value: U, + value: T, /// Whether the resulting angle should be given in as radians instead of degrees. radians: bool, -) -> U { +) -> T { if radians { value.asin() } else { value.asin().to_degrees() } } /// The inverse cosine trigonometric function (acos) calculates the angle whose cosine is the specified value. #[node_macro::node(category("Math: Trig"))] -fn cosine_inverse( +fn cosine_inverse( _: impl Ctx, /// The given value for which the angle will be calculated. Must be in the range [-1, 1] or else the result will be NaN. #[implementations(f64, f32)] - value: U, + value: T, /// Whether the resulting angle should be given in as radians instead of degrees. radians: bool, -) -> U { +) -> T { if radians { value.acos() } else { value.acos().to_degrees() } } @@ -289,16 +289,16 @@ fn cosine_inverse( /// atan: the angle whose tangent is the specified scalar number. /// atan2: the angle of a ray from the origin to the specified vec2. /// -/// The resulting angle is always in the range [0°, 180°] or, in radians, [-π/2, π/2]. +/// The resulting angle is always in the range [-90°, 90°] or, in radians, [-π/2, π/2]. #[node_macro::node(category("Math: Trig"))] -fn tangent_inverse( +fn tangent_inverse( _: impl Ctx, /// The given value for which the angle will be calculated. #[implementations(f64, f32, DVec2)] - value: U, + value: T, /// Whether the resulting angle should be given in as radians instead of degrees. radians: bool, -) -> U::Output { +) -> T::Output { value.atan(radians) } @@ -327,88 +327,86 @@ impl TangentInverse for DVec2 { /// The random function (rand) converts a seed into a random number within the specified range, inclusive of the minimum and exclusive of the maximum. The minimum and maximum values are automatically swapped if they are reversed. #[node_macro::node(category("Math: Numeric"))] -fn random( +fn random( _: impl Ctx, _primary: (), /// Seed to determine the unique variation of which number will be generated. seed: u64, /// The smaller end of the range within which the random number will be generated. - #[implementations(f64, f32)] #[default(0.)] - min: U, + min: f64, /// The larger end of the range within which the random number will be generated. - #[implementations(f64, f32)] #[default(1.)] - max: U, + max: f64, ) -> f64 { let mut rng = rand::rngs::StdRng::seed_from_u64(seed); let result = rng.random::(); let (min, max) = if min < max { (min, max) } else { (max, min) }; - let (min, max) = (min.to_f64().unwrap(), max.to_f64().unwrap()); result * (max - min) + min } -/// Convert a number to an integer of the type u32, which may be the required type for certain node inputs. This will be removed in the future when automatic type conversion is implemented. -#[node_macro::node(name("To u32"), category("Type Conversion"))] -fn to_u32(_: impl Ctx, #[implementations(f64, f32)] value: U) -> u32 { - let value = U::clamp(value, U::from(0.).unwrap(), U::from(u32::MAX as f64).unwrap()); - value.to_u32().unwrap() +// TODO: Test that these are no longer needed in all circumstances, then remove them and add a migration to convert these into Passthrough nodes. Note: these act more as type annotations than as identity functions. +/// Convert a number to an integer of the type u32, which may be the required type for certain node inputs. +#[node_macro::node(name("To u32"), category("Debug"))] +fn to_u32(_: impl Ctx, value: u32) -> u32 { + value } -/// Convert a number to an integer of the type u64, which may be the required type for certain node inputs. This will be removed in the future when automatic type conversion is implemented. -#[node_macro::node(name("To u64"), category("Type Conversion"))] -fn to_u64(_: impl Ctx, #[implementations(f64, f32)] value: U) -> u64 { - let value = U::clamp(value, U::from(0.).unwrap(), U::from(u64::MAX as f64).unwrap()); - value.to_u64().unwrap() +// TODO: Test that these are no longer needed in all circumstances, then remove them and add a migration to convert these into Passthrough nodes. Note: these act more as type annotations than as identity functions. +/// Convert a number to an integer of the type u64, which may be the required type for certain node inputs. +#[node_macro::node(name("To u64"), category("Debug"))] +fn to_u64(_: impl Ctx, value: u64) -> u64 { + value } -/// Convert an integer to a decimal number of the type f64, which may be the required type for certain node inputs. This will be removed in the future when automatic type conversion is implemented. -#[node_macro::node(name("To f64"), category("Type Conversion"))] -fn to_f64(_: impl Ctx, #[implementations(u32, u64)] value: U) -> f64 { - value.to_f64().unwrap() +// TODO: Test that these are no longer needed in all circumstances, then remove them and add a migration to convert these into Passthrough nodes. Note: these act more as type annotations than as identity functions. +/// Convert an integer to a decimal number of the type f64, which may be the required type for certain node inputs. +#[node_macro::node(name("To f64"), category("Debug"))] +fn to_f64(_: impl Ctx, value: f64) -> f64 { + value } /// The rounding function (round) maps an input value to its nearest whole number. Halfway values are rounded away from zero. #[node_macro::node(category("Math: Numeric"))] -fn round( +fn round( _: impl Ctx, /// The number which will be rounded. #[implementations(f64, f32)] - value: U, -) -> U { + value: T, +) -> T { value.round() } /// The floor function (floor) rounds down an input value to the nearest whole number, unless the input number is already whole. #[node_macro::node(category("Math: Numeric"))] -fn floor( +fn floor( _: impl Ctx, /// The number which will be rounded down. #[implementations(f64, f32)] - value: U, -) -> U { + value: T, +) -> T { value.floor() } /// The ceiling function (ceil) rounds up an input value to the nearest whole number, unless the input number is already whole. #[node_macro::node(category("Math: Numeric"))] -fn ceiling( +fn ceiling( _: impl Ctx, /// The number which will be rounded up. #[implementations(f64, f32)] - value: U, -) -> U { + value: T, +) -> T { value.ceil() } /// The absolute value function (abs) removes the negative sign from an input value, if present. #[node_macro::node(category("Math: Numeric"))] -fn absolute_value( +fn absolute_value( _: impl Ctx, /// The number which will be made positive. - #[implementations(f64, f32)] - value: U, -) -> U { + #[implementations(f64, f32, i32, i64)] + value: T, +) -> T { value.abs() } @@ -540,28 +538,28 @@ fn binary_gcd + std::ops: /// The equality operation (==) compares two values and returns true if they are equal, or false if they are not. #[node_macro::node(category("Math: Logic"))] -fn equals, T>( +fn equals>( _: impl Ctx, /// One of the two numbers to compare for equality. #[implementations(f64, f32, u32, DVec2, &str, String)] value: T, /// The other of the two numbers to compare for equality. #[implementations(f64, f32, u32, DVec2, &str, String)] - other_value: U, + other_value: T, ) -> bool { other_value == value } /// The inequality operation (!=) compares two values and returns true if they are not equal, or false if they are. #[node_macro::node(category("Math: Logic"))] -fn not_equals, T>( +fn not_equals>( _: impl Ctx, /// One of the two numbers to compare for inequality. #[implementations(f64, f32, u32, DVec2, &str)] value: T, /// The other of the two numbers to compare for inequality. #[implementations(f64, f32, u32, DVec2, &str)] - other_value: U, + other_value: T, ) -> bool { other_value != value } diff --git a/node-graph/interpreted-executor/src/node_registry.rs b/node-graph/interpreted-executor/src/node_registry.rs index 65ed9439..08132004 100644 --- a/node-graph/interpreted-executor/src/node_registry.rs +++ b/node-graph/interpreted-executor/src/node_registry.rs @@ -57,6 +57,10 @@ fn node_registry() -> HashMap