Merge pull request #12165 from hnoerenberg/fix/rotate-dialog-two-decimals-12035

fix(ui): the Arbitrary rotation dialog keeps the ruler's two decimals (#12035)

crates/ui-egui/src/dialogs.rs

@@ -1259,6 +1259,8 @@ pub fn open_command_dialog(app: &mut PhotocraftApp, command: &str, label: &str)
 pub mod adjust_dialog;
 pub mod adjust_editors;
 pub mod adjust_preview;
+#[cfg(test)]
+mod arbitrary_ruler_prefill_tests;
 pub(crate) mod bitmap_dialog;
 pub mod blur_pins;
 pub mod brush_preset_dialog;

crates/ui-egui/src/dialogs/arbitrary_ruler_prefill_tests.rs

@@ -0,0 +1,113 @@
+//! #12035: Image Rotation › Arbitrary pre-fills the ruler's straightening angle at the
+//! precision the reference application shows. A ruler from 100,100 to 400,300 (A -33.7° on
+//! the Info panel) opens the Rotate Canvas dialog at 33.69°, Counter clockwise, and OK runs
+//! `image.rotation.arbitrary` with that two-decimal angle, not the field's old one-decimal
+//! round. The Angle range is -359.99..359.99, wide, so today's wide-range rounding to one
+//! decimal decided the dialog's value; only this field now keeps hundredths.
+
+use super::filter_dialog::{body, open, params_of};
+use crate::PhotocraftApp;
+use serde_json::{Map, Value, json};
+
+/// Draw a dialog body once, as a live dialog does every frame: the drawn fields are what
+/// the dialog shows and what OK sends.
+fn drawn(f: &mut Map<String, Value>) {
+    egui::Context::default().run_ui(Default::default(), |ui| body(ui, f)).textures_delta.clear();
+}
+
+/// A 318 px square document (the size `ok_runs_the_two_decimal_angle` measures on), a ruler
+/// from 100,100 to `end`, the opened Arbitrary dialog and its fields after one drawn frame.
+fn ruler_dialog(end: [i32; 2]) -> (PhotocraftApp, u64, Map<String, Value>) {
+    let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default());
+    app.run("file.new", json!({"width": 318, "height": 318})).unwrap();
+    app.run("image.analysis.rulerTool", json!({"start": [100, 100], "end": end})).unwrap();
+    let id = open(&mut app, "image.rotation.arbitrary").unwrap();
+    let mut f = app.ui.dialog_mut(id).unwrap().fields.clone();
+    drawn(&mut f);
+    (app, id, f)
+}
+
+/// The measured case: the Rotate Canvas dialog opens at 33.69°, Counter clockwise — the
+/// angle keeps two decimals where the wide-range rule used to round it to 33.7.
+#[test]
+fn ruler_prefill_keeps_the_measured_two_decimal_angle() {
+    let (_app, _id, f) = ruler_dialog([400, 300]);
+    let a = f["angle"].as_f64().unwrap();
+    assert!((a - 33.69).abs() < 1e-3, "the dialog opens at {a}, want the measured 33.69");
+    assert_eq!(crate::widgets::fmt_num2(a), "33.69", "the Angle field shows the measured string");
+    assert_eq!(f["direction"], json!("ccw"), "a ruler falling to the right straightens counter-clockwise");
+}
+
+/// A ruler that needs a clockwise turn pre-fills Clockwise, at the same two decimals;
+/// with no ruler the dialog opens at today's default (0, Clockwise).
+#[test]
+fn clockwise_ruler_prefills_clockwise_and_no_ruler_keeps_the_default() {
+    // (100,100) → (400,50) rises to the right: a 9.46° clockwise turn.
+    let (_app, _id, f) = ruler_dialog([400, 50]);
+    let a = f["angle"].as_f64().unwrap();
+    assert_eq!(f["direction"], json!("cw"), "a rising ruler pre-fills Clockwise");
+    assert!((a - 9.46).abs() < 1e-3, "the clockwise pre-fill is {a}, want 9.46 to two decimals");
+    // No ruler: today's default, unchanged.
+    let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default());
+    app.run("file.new", json!({"width": 318, "height": 318})).unwrap();
+    let id = open(&mut app, "image.rotation.arbitrary").unwrap();
+    let mut f = app.ui.dialog_mut(id).unwrap().fields.clone();
+    drawn(&mut f);
+    assert_eq!(f["angle"], json!(0.0), "without a ruler the dialog opens at today's 0");
+    assert_eq!(f["direction"], json!("cw"), "without a ruler the dialog opens Clockwise");
+}
+
+/// OK runs `image.rotation.arbitrary` with the two-decimal angle: a 318 px square rotated by
+/// 33.69° measures 441 px across, by 33.7° 442 px — the dialog's rounding must not decide.
+#[test]
+fn ok_runs_the_two_decimal_angle() {
+    let (mut app, id, f) = ruler_dialog([400, 300]);
+    let params = params_of(&f);
+    assert!((params["angle"].as_f64().unwrap() - 33.69).abs() < 1e-3, "OK sends {params}");
+    app.ui.dialog_mut(id).unwrap().fields = f;
+    let r = crate::dialogs::confirm(&mut app, id).unwrap();
+    assert_eq!((r["width"].as_u64(), r["height"].as_u64()), (Some(441), Some(441)), "442 would mean the 33.7 round ran");
+}
+
+/// The f32 double-rounding the review caught: a 43x27 ruler line (here 100,100 to 143,127)
+/// is 32.1249984 degrees, and the f32 cast of that full-precision pre-fill lands exactly on
+/// 32.125 - rounding the widened value half away from zero opened the dialog at 32.13. The
+/// hundredths round in f64 before the field narrows, so it opens at 32.12, Counter clockwise.
+#[test]
+fn the_ruler_angle_rounds_at_full_precision_before_the_field_narrows() {
+    let (_app, _id, f) = ruler_dialog([143, 127]);
+    let a = f["angle"].as_f64().unwrap();
+    assert!((a - 32.12).abs() < 1e-3, "the 43x27 ruler opens at {a}, want 32.12");
+    assert_eq!(crate::widgets::fmt_num2(a), "32.12", "the Angle field shows 32.12, not the 32.13 the f32 round gives");
+    assert!((a - 32.13).abs() > 1e-3, "the f32 double-round must not reach the dialog: {a}");
+    assert_eq!(f["direction"], json!("ccw"));
+}
+
+/// The fix is per-field: unrelated number fields keep the precision they have today —
+/// wide ranges round to one decimal (in another dialog, and under the same key name),
+/// small ranges keep their two.
+#[test]
+fn other_number_fields_keep_their_precision() {
+    // A wide range in another dialog: Gaussian Blur's radius (0.1..1000) rounds to one decimal.
+    let mut f = Map::new();
+    f.insert("__command".into(), json!("filter.blur.gaussianBlur"));
+    f.insert("radius".into(), json!(7.26));
+    drawn(&mut f);
+    let r = f["radius"].as_f64().unwrap();
+    assert!((r - 7.3).abs() < 1e-5, "the Gaussian Blur radius {r} must stay at one decimal");
+    // The same key the fix touches, in another dialog: Twirl's angle (-999..999) rounds to one.
+    let mut f = Map::new();
+    f.insert("__command".into(), json!("filter.distort.twirl"));
+    f.insert("angle".into(), json!(33.69));
+    drawn(&mut f);
+    let a = f["angle"].as_f64().unwrap();
+    assert!((a - 33.7).abs() < 1e-5, "Twirl's angle {a} keeps the one-decimal round");
+    // A small range keeps its two decimals (Radial Blur spins, so the centre fields stay plain).
+    let mut f = Map::new();
+    f.insert("__command".into(), json!("filter.blur.radialBlur"));
+    f.insert("method".into(), json!("spin"));
+    f.insert("centerX".into(), json!(0.123));
+    drawn(&mut f);
+    let x = f["centerX"].as_f64().unwrap();
+    assert!((x - 0.12).abs() < 1e-5, "the Radial Blur centre {x} keeps its two decimals");
+}

crates/ui-egui/src/dialogs/filter_dialog.rs

@@ -806,6 +806,26 @@ fn uses_angle_dial(command: &str, key: &str) -> bool {
     key == "angle" && ANGLE_DIAL.contains(&command)
 }
 
+/// Wide-range number fields that still show two decimals: the Rotate Canvas angle keeps
+/// hundredths of a degree, so the ruler's straightening angle reaches the dialog as 33.69 and
+/// not 33.7 (#12035). The arrow and drag step follow the shown precision, as elsewhere.
+fn wide_field_decimals(command: &str, key: &str) -> Option<u32> {
+    match (command, key) {
+        ("image.rotation.arbitrary", "angle") => Some(2),
+        _ => None,
+    }
+}
+
+/// The scale a drawn number field rounds to: two decimals for small ranges (0–1 thresholds and
+/// centres) and for the wide fields that show them (#12035), one decimal for the other wide ones.
+fn field_scale(command: &str, key: &str, min: f32, max: f32) -> f32 {
+    match wide_field_decimals(command, key) {
+        Some(d) => 10f32.powi(d.min(6) as i32),
+        None if max - min <= 10.0 => 100.0,
+        None => 10.0,
+    }
+}
+
 /// Replace Color (#4143): the dialog's "From" colour, sampled from the image with a canvas
 /// eyedropper like Photoshop's. While armed, the next click on the image writes the merged
 /// composite colour into the `color` field (the same sampler the Color Picker uses).
@@ -1031,7 +1051,10 @@ pub fn body(ui: &mut egui::Ui, f: &mut Map<String, Value>) {
         }
         match p.kind {
             Kind::Range { min, max, default } => {
-                let mut v = f.get(&p.key).and_then(Value::as_f64).unwrap_or(default as f64) as f32;
+                let src = f.get(&p.key).and_then(Value::as_f64);
+                let mut v = src.unwrap_or(default as f64) as f32;
+                // A widget edit this frame makes the field's f32 value authoritative (#12035).
+                let mut edited = false;
                 if cmd == "image.mode.indexedColor" && p.key == "colors" {
                     let mut count = v.round().clamp(2.0, 256.0) as u32;
                     crate::widgets::color_count_row(ui, &label(&p.key), &mut count);
@@ -1074,12 +1097,23 @@ pub fn body(ui: &mut egui::Ui, f: &mut Map<String, Value>) {
                     } else {
                         // A double-click on the slider puts it back to the filter's default.
                         let gestures = crate::widgets::RowGestures { reset: Some(default), wheel_step: None, ..Default::default() };
-                        crate::widgets::slider_row_with(ui, &label(&p.key), &mut v, min..=max, unit, None, gestures);
+                        edited =
+                            crate::widgets::slider_row_prec(ui, &label(&p.key), &mut v, min..=max, unit, None, gestures, wide_field_decimals(&cmd, &p.key))
+                                .changed();
                     }
                 });
-                // Small ranges (0–1 thresholds and centres) keep the two decimals their field shows.
-                let scale = if max - min <= 10.0 { 100.0 } else { 10.0 };
-                f.insert(p.key, json!((v * scale).round() / scale));
+                // The field's shown precision: small ranges keep their two decimals, and so
+                // does the Arbitrary angle; the other wide fields round to one (#12035).
+                let scale = field_scale(&cmd, &p.key, min, max);
+                // A field showing two decimals rounds while the value is still f64: a 43x27
+                // ruler is 32.1249984°, whose f32 cast lands exactly on 32.125, and rounding
+                // that widened value half-away shows 32.13 where the pre-fill says 32.12.
+                // Values the user typed or dragged are f32 already, so they round as before.
+                let w = match (wide_field_decimals(&cmd, &p.key), src) {
+                    (Some(_), Some(raw)) if !edited => ((raw * f64::from(scale)).round() / f64::from(scale)) as f32,
+                    _ => (v * scale).round() / scale,
+                };
+                f.insert(p.key, json!(w));
             }
             Kind::Choice { options, default } => {
                 ui.horizontal(|ui| {