From 5a9c6e2e063d3d8f2f1deff165c7c51cff8715d5 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Sun, 11 Oct 2026 02:06:19 +0700 Subject: [PATCH] Review #1012: whole numbers typed in command dialogs are integers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PR kept a field's integer kind only while its value had been an integer: typing 2.5 in Columns and then 3 sent 3.0, and the mosaic fell back to 10 × 10 again. A whole number typed in any command dialog is now an integer (`typed_number`) whatever the field held before; commands that take any number read integers as well, and a fraction stays one. Test: Object › Create Object Mosaic… opened from its menu item, 3 typed over Columns and Enter makes a 3 × 10 mosaic (it made 10 × 10 before, #1000). Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/dialogs/form.rs | 50 +++++++++-------------------- crates/ui-egui/src/dialogs/tests.rs | 32 ++++++++++++++++++ 2 files changed, 47 insertions(+), 35 deletions(-) diff --git a/crates/ui-egui/src/dialogs/form.rs b/crates/ui-egui/src/dialogs/form.rs index 9902faa8..2b3ace0d 100644 --- a/crates/ui-egui/src/dialogs/form.rs +++ b/crates/ui-egui/src/dialogs/form.rs @@ -248,7 +248,7 @@ pub(super) fn param_fields( match v { Value::Number(n) => { if let Some(x) = crate::widgets::plain_field(ui, ("fx-num", &k), n.as_f64().unwrap_or(0.0), "", 3, 140.0) { - d.fields.insert(k, numeric_edit_value(&n, x)); + d.fields.insert(k, typed_number(x)); changed = true; } } @@ -291,20 +291,11 @@ pub(super) fn param_fields( changed } -/// Keep integer command parameters integral when a numeric field is edited. The editor reads -/// numbers through f64, but a JSON float such as 12.0 is not accepted by Value::as_u64(). -/// Counts (Object Mosaic rows/columns, grid repeats, etc.) would otherwise fall back to their -/// defaults after the user changed them. Fractional input stays fractional for command validation. -fn numeric_edit_value(original: &serde_json::Number, value: f64) -> Value { - if value.is_finite() && value.fract() == 0.0 { - if original.is_u64() && value >= 0.0 && value < u64::MAX as f64 { - return json!(value as u64); - } - if original.is_i64() && value >= i64::MIN as f64 && value < i64::MAX as f64 { - return json!(value as i64); - } - } - json!(value) +/// A number typed in a command's dialog as JSON: a whole number as an integer, as counts are read +/// (`as_u64`: Object Mosaic's columns and rows, #1000), which `12.0` isn't; a fraction stays one. +/// Commands that take any number read either. +fn typed_number(x: f64) -> Value { + if x.fract() == 0.0 && x.abs() < 1e15 { json!(x as i64) } else { json!(x) } } /// Editor for a plug-in's parameters from its schema (plug-in filter and effect dialogs): numbers @@ -570,28 +561,17 @@ pub(super) fn caption(ui: &mut egui::Ui, text: &str) { } #[cfg(test)] -mod numeric_edit_tests { +mod typed_number_tests { use super::*; #[test] - fn integer_command_parameters_remain_json_integers() { - // Object Mosaic's command parses counts with Value::as_u64(). - let count = json!(10); - let original = count.as_number().unwrap(); - let changed = numeric_edit_value(original, 24.0); - assert_eq!(changed.as_u64(), Some(24)); - assert_eq!(numeric_edit_value(original, 1.0), json!(1)); - // A decimal must not be silently converted to a different integer. - assert_eq!(numeric_edit_value(original, 2.5), json!(2.5)); - } - - #[test] - fn signed_counts_and_float_parameters_retain_their_number_kind() { - let signed = json!(-10); - assert_eq!(numeric_edit_value(signed.as_number().unwrap(), -4.0), json!(-4)); - let float = json!(10.5); - let updated = numeric_edit_value(float.as_number().unwrap(), 12.0); - assert!(updated.is_f64()); - assert_eq!(updated, json!(12.0)); + fn whole_numbers_are_integers_and_fractions_stay_fractions() { + // Whatever the field held before: a count typed after a fraction is a count again. + assert_eq!(typed_number(24.0).as_u64(), Some(24)); + assert_eq!(typed_number(-4.0), json!(-4)); + assert_eq!(typed_number(0.0), json!(0)); + assert_eq!(typed_number(2.5), json!(2.5)); + // Numbers past exact integers stay floats. + assert!(typed_number(1e20).is_f64()); } } diff --git a/crates/ui-egui/src/dialogs/tests.rs b/crates/ui-egui/src/dialogs/tests.rs index f9534577..f0df5c69 100644 --- a/crates/ui-egui/src/dialogs/tests.rs +++ b/crates/ui-egui/src/dialogs/tests.rs @@ -434,6 +434,38 @@ fn a_dialog_opens_with_its_first_field_focused_so_typing_and_enter_apply() { assert_eq!(node.appearance.effects.last().map(|e| e.params["scaleH"].clone()), Some(json!(50.0))); } +/// #1000: a count typed in Object › Create Object Mosaic… reaches the command (a whole number typed +/// in a command's dialog is an integer), so the mosaic has that many columns, not the 10 × 10 grid. +#[test] +fn object_mosaic_makes_the_grid_typed_in_its_dialog() { + let mut app = app(); + app.run("shape.rectangle", json!({"x": 10, "y": 10, "width": 60, "height": 40})).unwrap(); + app.run("object.rasterize", json!({"ppi": 72})).unwrap(); + let ctx = egui::Context::default(); + theme::install_fonts(&ctx); + theme::apply(&ctx, Default::default()); + let frame = |app: &mut VectorcraftApp, events: Vec| { + let mut out = ctx.run_ui(egui::RawInput { events, ..Default::default() }, |ui| show(app, ui.ctx())); + out.textures_delta.clear(); + }; + let entry = crate::menus::menu_entries(&app).into_iter().find(|e| e.label == "Create Object Mosaic…").expect("menu item"); + let (cmd, params) = crate::menus::click_target(&entry.label, entry.command.as_deref().unwrap(), &entry.params); + crate::menus::invoke(&mut app, &cmd, params); + // Its first field, Columns, is focused: type 3 over it and press Enter. + frame(&mut app, vec![]); + frame(&mut app, vec![egui::Event::Text("3".into())]); + let enter = egui::Event::Key { key: egui::Key::Enter, physical_key: None, pressed: true, repeat: false, modifiers: Default::default() }; + frame(&mut app, vec![enter]); + assert!(app.ui.dialog.is_none()); + let mut tiles = None; + app.session.doc().unwrap().doc.walk(|n| { + if n.name.as_deref() == Some("Object Mosaic") { + tiles = n.children().map(|c| c.len()); + } + }); + assert_eq!(tiles, Some(3 * 10)); +} + /// #793: Artboard Options lists its fields as Name, Width, Height, not by key. #[test] fn artboard_options_lists_name_then_width_and_height() {