Move NumberInput.svelte input processing from JS to Rust (#4274)

* Move NumberInput.svelte input processing from JS to Rust

* Fix edge cases

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",

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 }

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)
+					// A text-field commit sends the user's raw entry as a math expression to evaluate and validate.
-					Value::String(str) => match str.as_str() {
+					Value::String(expression) => {
-						"Increment" => responses.add((number_input.increment_callback_increase.callback)(number_input)),
+						let Some(evaluated) = evaluate_and_validate_number_input(&expression, number_input) else { return };
-						"Decrement" => responses.add((number_input.increment_callback_decrease.callback)(number_input)),
+
-						_ => panic!("Invalid string found when updating `NumberInput`"),
+						// 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 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.
+					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<f64> {
+	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)
+}

frontend/src/components/widgets/WidgetSpan.svelte

@@ -215,10 +215,11 @@
 			component: NumberInput,
 			getProps: (props, index) => ({
 				...props,
-				incrementCallbackIncrease: () => widgetValueCommitAndUpdate(index, "Increment", false),
+				incrementCallbackIncrease: () => widgetValueCommitAndUpdate(index, { increment: "Increase" }, false),
-				incrementCallbackDecrease: () => widgetValueCommitAndUpdate(index, "Decrement", 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(),
 				},

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<EditorWrapper>("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".
+		// 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.
-		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])
+		dispatch("commitText", text);
 
-		let newValue = evaluateMathExpression(textWithLeadingZeroes);
+		// 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.
-		if (newValue !== undefined && isNaN(newValue)) newValue = undefined; // Rejects `sqrt(-1)`
+		text = displayText(value, unit);
-
-		if (newValue !== undefined) {
-			const oldValue = value !== undefined && isInteger ? Math.round(value) : value;
-			if (newValue !== oldValue) {
-				dispatch("startHistoryTransaction");
-				transactionInProgress = true;
-			}
-		}
-		updateValue(newValue);
-		commitTransactionIfInProgress();
 
 		editing = false;
 		self?.unFocus();

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 }

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<f64> {
-	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)
-}

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) ~ !("." | ASCII_DIGIT) }
 exp           =  _{ ^"e" ~ ("+" | "-")? ~ int }
 int           =  @{ ASCII_DIGIT+ }
 

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)),* $(,)?) => {
 			$(
@@ -139,6 +147,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),