From cce34affb74819d76055bd9c363103b5fceda2dd Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Mon, 22 Jun 2026 14:17:02 -0700 Subject: [PATCH 1/2] Move NumberInput.svelte input processing from JS to Rust --- Cargo.lock | 2 +- editor/Cargo.toml | 1 + .../messages/layout/layout_message_handler.rs | 53 +++++++++++++++++-- .../src/components/widgets/WidgetSpan.svelte | 5 +- .../widgets/inputs/NumberInput.svelte | 26 ++++----- frontend/wrapper/Cargo.toml | 1 - frontend/wrapper/src/editor_wrapper.rs | 19 ------- libraries/math-parser/src/grammer.pest | 2 +- libraries/math-parser/src/lib.rs | 5 ++ 9 files changed, 69 insertions(+), 45 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 96b5bd89ed..946aaaa555 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2295,6 +2295,7 @@ dependencies = [ "js-sys", "kurbo", "log", + "math-parser", "num_enum", "once_cell", "preprocessor", @@ -2337,7 +2338,6 @@ dependencies = [ "graphite-editor", "js-sys", "log", - "math-parser", "node-macro", "ron", "serde", diff --git a/editor/Cargo.toml b/editor/Cargo.toml index dc9612ed74..a1d229e93c 100644 --- a/editor/Cargo.toml +++ b/editor/Cargo.toml @@ -23,6 +23,7 @@ graphene-hash = { workspace = true } interpreted-executor = { workspace = true } graphene-std = { workspace = true } # NOTE: `core-types` should not be added here because `graphene-std` re-exports its contents preprocessor = { workspace = true } +math-parser = { workspace = true } # Workspace dependencies js-sys = { workspace = true } diff --git a/editor/src/messages/layout/layout_message_handler.rs b/editor/src/messages/layout/layout_message_handler.rs index 40dd11689c..f7b0959810 100644 --- a/editor/src/messages/layout/layout_message_handler.rs +++ b/editor/src/messages/layout/layout_message_handler.rs @@ -310,11 +310,25 @@ impl LayoutMessageHandler { let callback_message = (number_input.on_update.callback)(number_input); responses.add(callback_message); } - // TODO: This crashes when the cursor is in a text box, such as in the Text node, and the transform node is clicked (https://github.com/GraphiteEditor/Graphite/issues/1761) - Value::String(str) => match str.as_str() { - "Increment" => responses.add((number_input.increment_callback_increase.callback)(number_input)), - "Decrement" => responses.add((number_input.increment_callback_decrease.callback)(number_input)), - _ => panic!("Invalid string found when updating `NumberInput`"), + // A text-field commit sends the user's raw entry as a math expression to evaluate and validate. + Value::String(expression) => { + let Some(evaluated) = evaluate_and_validate_number_input(&expression, number_input) else { return }; + + // Skip the update (and its history transaction) when the value is unchanged, since the network interface would short-circuit it anyway. + if number_input.value == Some(evaluated) { + return; + } + + // Snapshot for undo via `on_commit`, then apply the new value via `on_update`. + number_input.value = Some(evaluated); + responses.add((number_input.on_commit.callback)(&())); + responses.add((number_input.on_update.callback)(number_input)); + } + // The increment arrows send `{ "increment": "Increase" | "Decrease" }` to invoke the backend's directional step callback. + Value::Object(ref command) => match command.get("increment").and_then(Value::as_str) { + Some("Increase") => responses.add((number_input.increment_callback_increase.callback)(number_input)), + Some("Decrease") => responses.add((number_input.increment_callback_decrease.callback)(number_input)), + _ => error!("NumberInput received an unrecognized command: {value:?}"), }, _ => {} }, @@ -524,3 +538,32 @@ fn populate_computed_display_fields(layout: &mut Layout) { } } } + +/// Evaluates a math expression committed in a `NumberInput`'s text field, then clamps and rounds it to the widget's constraints. +/// Returns `None` if the expression fails to parse, fails to evaluate, or yields a non-real number (such as `sqrt(-1)`). +fn evaluate_and_validate_number_input(expression: &str, number_input: &NumberInput) -> Option { + let value = math_parser::evaluate(expression) + .inspect_err(|err| error!("Math parser error on \"{expression}\": {err}")) + .ok()? + .0 + .inspect_err(|err| error!("Math evaluate error on \"{expression}\": {err}")) + .ok()?; + + let real = value.as_real()?; + if real.is_nan() { + return None; + } + + let mut validated = real; + if let Some(min) = number_input.min { + validated = validated.max(min); + } + if let Some(max) = number_input.max { + validated = validated.min(max); + } + if number_input.is_integer { + validated = validated.round(); + } + + Some(validated) +} diff --git a/frontend/src/components/widgets/WidgetSpan.svelte b/frontend/src/components/widgets/WidgetSpan.svelte index 301fcf673b..aa37ad21f2 100644 --- a/frontend/src/components/widgets/WidgetSpan.svelte +++ b/frontend/src/components/widgets/WidgetSpan.svelte @@ -215,10 +215,11 @@ component: NumberInput, getProps: (props, index) => ({ ...props, - incrementCallbackIncrease: () => widgetValueCommitAndUpdate(index, "Increment", false), - incrementCallbackDecrease: () => widgetValueCommitAndUpdate(index, "Decrement", false), + incrementCallbackIncrease: () => widgetValueCommitAndUpdate(index, { increment: "Increase" }, false), + incrementCallbackDecrease: () => widgetValueCommitAndUpdate(index, { increment: "Decrease" }, false), $$events: { value: (e: CustomEvent) => widgetValueUpdate(index, e.detail, true), + commitText: (e: CustomEvent) => widgetValueUpdate(index, e.detail, true), startHistoryTransaction: () => widgetValueCommit(index, props.value), commitHistoryTransaction: () => editor.endTransaction(), }, diff --git a/frontend/src/components/widgets/inputs/NumberInput.svelte b/frontend/src/components/widgets/inputs/NumberInput.svelte index 4c88e28d2d..ffc14f0f2a 100644 --- a/frontend/src/components/widgets/inputs/NumberInput.svelte +++ b/frontend/src/components/widgets/inputs/NumberInput.svelte @@ -4,7 +4,6 @@ import FieldInput from "/src/components/widgets/inputs/FieldInput.svelte"; import { PRESS_REPEAT_DELAY_MS, PRESS_REPEAT_INTERVAL_MS } from "/src/managers/input"; import { browserVersion } from "/src/utility-functions/platform"; - import { evaluateMathExpression } from "/wrapper/pkg/graphite_wasm_wrapper"; import type { ActionShortcut, EditorWrapper, NumberInputIncrementBehavior, NumberInputMode } from "/wrapper/pkg/graphite_wasm_wrapper"; const BUTTONS_LEFT = 0b0000_0001; @@ -12,7 +11,12 @@ const BUTTON_LEFT = 0; const BUTTON_RIGHT = 2; - const dispatch = createEventDispatcher<{ value: number | undefined; startHistoryTransaction: undefined; commitHistoryTransaction: undefined }>(); + const dispatch = createEventDispatcher<{ + value: number | undefined; + commitText: string; + startHistoryTransaction: undefined; + commitHistoryTransaction: undefined; + }>(); const editor = getContext("editor"); @@ -247,21 +251,11 @@ // The `unFocus()` call at the bottom of this function and in `onTextChangeCanceled()` causes this function to be run again, so this check skips a second run. if (!editing) return; - // Insert a leading zero before all decimal points lacking a preceding digit, since the library doesn't realize that "point" means "zero point". - const textWithLeadingZeroes = text.replaceAll(/(?<=^|[^0-9])\./g, "0."); // Match any "." that is preceded by the start of the string (^) or a non-digit character ([^0-9]) + // The backend evaluates the math, validates against this widget's constraints, and (only when changed) applies it within a history transaction before resending the widget. + dispatch("commitText", text); - let newValue = evaluateMathExpression(textWithLeadingZeroes); - if (newValue !== undefined && isNaN(newValue)) newValue = undefined; // Rejects `sqrt(-1)` - - if (newValue !== undefined) { - const oldValue = value !== undefined && isInteger ? Math.round(value) : value; - if (newValue !== oldValue) { - dispatch("startHistoryTransaction"); - transactionInProgress = true; - } - } - updateValue(newValue); - commitTransactionIfInProgress(); + // Revert the field to the current value's canonical display; an accepted change resends the widget and re-runs `watchValue` to show the new value. + text = displayText(value, unit); editing = false; self?.unFocus(); diff --git a/frontend/wrapper/Cargo.toml b/frontend/wrapper/Cargo.toml index 381c1b6e65..57a13f03c9 100644 --- a/frontend/wrapper/Cargo.toml +++ b/frontend/wrapper/Cargo.toml @@ -33,7 +33,6 @@ wasm-bindgen = { workspace = true } serde-wasm-bindgen = { workspace = true } js-sys = { workspace = true } wasm-bindgen-futures = { workspace = true } -math-parser = { workspace = true } wgpu = { workspace = true } web-sys = { workspace = true } ron = { workspace = true } diff --git a/frontend/wrapper/src/editor_wrapper.rs b/frontend/wrapper/src/editor_wrapper.rs index 451b08c07e..2b91cf1125 100644 --- a/frontend/wrapper/src/editor_wrapper.rs +++ b/frontend/wrapper/src/editor_wrapper.rs @@ -979,22 +979,3 @@ impl EditorWrapper { self.dispatch(message); } } - -// ==================================================================== -// Static functions callable from JavaScript without an Editor instance -// ==================================================================== - -#[wasm_bindgen(js_name = evaluateMathExpression)] -pub fn evaluate_math_expression(expression: &str) -> Option { - let value = math_parser::evaluate(expression) - .inspect_err(|err| error!("Math parser error on \"{expression}\": {err}")) - .ok()? - .0 - .inspect_err(|err| error!("Math evaluate error on \"{expression}\": {err} ")) - .ok()?; - let Some(real) = value.as_real() else { - error!("{value} was not a real; skipping."); - return None; - }; - Some(real) -} diff --git a/libraries/math-parser/src/grammer.pest b/libraries/math-parser/src/grammer.pest index d7a61939df..f3d19b34af 100644 --- a/libraries/math-parser/src/grammer.pest +++ b/libraries/math-parser/src/grammer.pest @@ -26,7 +26,7 @@ fn_call = { ident ~ "(" ~ expr ~ ("," ~ expr)* ~ ")" } ident = @{ (ASCII_ALPHA | "_") ~ (ASCII_ALPHANUMERIC | "_")* } lit = { unit | ((float | int) ~ unit?) } -float = @{ int ~ "." ~ int? ~ exp? | int ~ exp } +float = @{ int ~ "." ~ int? ~ exp? | "." ~ int ~ exp? | int ~ exp } exp = _{ ^"e" ~ ("+" | "-")? ~ int } int = @{ ASCII_DIGIT+ } diff --git a/libraries/math-parser/src/lib.rs b/libraries/math-parser/src/lib.rs index 40e56ce04f..bf975c012e 100644 --- a/libraries/math-parser/src/lib.rs +++ b/libraries/math-parser/src/lib.rs @@ -139,6 +139,11 @@ mod tests { exponent_tau: "2^tau" => (2f64.powf(2. * std::f64::consts::PI), Unit::BASE_UNIT), infinity_subtract_large_number: "inf - 1000" => (f64::INFINITY, Unit::BASE_UNIT), + // Decimals with no leading digit before the point + leading_dot_decimal: ".5" => (0.5, Unit::BASE_UNIT), + leading_dot_in_expression: "1+.5" => (1.5, Unit::BASE_UNIT), + leading_dot_exponent: ".5e3" => (500., Unit::BASE_UNIT), + // Trigonometric functions trig_sin_pi: "sin(pi)" => (0., Unit::BASE_UNIT), trig_cos_zero: "cos(0)" => (1., Unit::BASE_UNIT), From 11332382d27ef92dd370bd5c46119e79da4c21ac Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Mon, 22 Jun 2026 14:37:57 -0700 Subject: [PATCH 2/2] Fix edge cases --- editor/src/messages/layout/layout_message_handler.rs | 4 ++-- libraries/math-parser/src/grammer.pest | 2 +- libraries/math-parser/src/lib.rs | 8 ++++++++ 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/editor/src/messages/layout/layout_message_handler.rs b/editor/src/messages/layout/layout_message_handler.rs index f7b0959810..6dc225d0d8 100644 --- a/editor/src/messages/layout/layout_message_handler.rs +++ b/editor/src/messages/layout/layout_message_handler.rs @@ -319,9 +319,9 @@ impl LayoutMessageHandler { return; } - // Snapshot for undo via `on_commit`, then apply the new value via `on_update`. - number_input.value = Some(evaluated); + // Snapshot the pre-change state for undo via `on_commit`, then apply the new value via `on_update`. responses.add((number_input.on_commit.callback)(&())); + number_input.value = Some(evaluated); responses.add((number_input.on_update.callback)(number_input)); } // The increment arrows send `{ "increment": "Increase" | "Decrease" }` to invoke the backend's directional step callback. diff --git a/libraries/math-parser/src/grammer.pest b/libraries/math-parser/src/grammer.pest index f3d19b34af..befa01cbc9 100644 --- a/libraries/math-parser/src/grammer.pest +++ b/libraries/math-parser/src/grammer.pest @@ -26,7 +26,7 @@ fn_call = { ident ~ "(" ~ expr ~ ("," ~ expr)* ~ ")" } ident = @{ (ASCII_ALPHA | "_") ~ (ASCII_ALPHANUMERIC | "_")* } lit = { unit | ((float | int) ~ unit?) } -float = @{ int ~ "." ~ int? ~ exp? | "." ~ int ~ exp? | int ~ exp } +float = @{ (int ~ "." ~ int? ~ exp? | "." ~ int ~ exp? | int ~ exp) ~ !("." | ASCII_DIGIT) } exp = _{ ^"e" ~ ("+" | "-")? ~ int } int = @{ ASCII_DIGIT+ } diff --git a/libraries/math-parser/src/lib.rs b/libraries/math-parser/src/lib.rs index bf975c012e..fb926e8ff1 100644 --- a/libraries/math-parser/src/lib.rs +++ b/libraries/math-parser/src/lib.rs @@ -27,6 +27,14 @@ mod tests { const EPSILON: f64 = 1e-10_f64; + #[test] + fn malformed_juxtaposed_numbers_fail_to_parse() { + // Two numbers cannot be glued together by a stray decimal point (they must not parse as implicit multiplication). + for input in ["1..5", "1.5.5", "1..", ".5.5"] { + assert!(evaluate(input).is_err(), "expected `{input}` to be a parse error"); + } + } + macro_rules! test_end_to_end{ ($($name:ident: $input:expr_2021 => ($expected_value:expr_2021, $expected_unit:expr_2021)),* $(,)?) => { $(