From 41e0348f157e097de40bc2c757343919fc2ab551 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hartmut=20N=C3=B6renberg?= Date: Sat, 7 Nov 2026 20:37:03 +0100 Subject: [PATCH 1/2] fix(ui): the Arbitrary rotation dialog keeps the ruler's two decimals (#12035) straighten_defaults put the ruler's straightening angle into the image.rotation.arbitrary dialog at full precision, but every number field whose range spans more than 10 rounds to one decimal as the dialog draws (filter_dialog.rs:930): the Angle range is -359.99..359.99 (mode_cmds.rs:684), so a ruler from 100,100 to 400,300 opened the Rotate Canvas dialog at 33.7, Counter clockwise, and OK ran the rotation at 33.7 degrees. The reference application keeps the ruler's angle to two decimals there: it opens at 33.69 degrees, Counter clockwise. The rounding rule gains a per-field precision beside the angle dial's list: only the Arbitrary angle field keeps hundredths, both in the value the field writes back and in the places it shows. It follows the existing slider_row_prec idiom (Exposure's stops, #4164), so the field's arrow and drag step follow its shown precision like theirs. Small ranges keep their two decimals, every other wide field its one; how the reference rounds a third decimal is not measured, so it still rounds half away from zero like today. A drawn 318 px square says 441 px across at 33.69 degrees and 442 at 33.7, which is what pins OK to the two-decimal angle end to end. Tests go in the new crates/ui-egui/src/dialogs/arbitrary_ruler_prefill_tests.rs (the measured pre-fill, the clockwise default pair, OK's angle, and a pin that Gaussian Blur's radius, Twirl's angle and a 0..1 centre keep the precision they have today); dialogs.rs only gains the mod line it needs. Fixes #12035 --- crates/ui-egui/src/dialogs.rs | 2 + .../dialogs/arbitrary_ruler_prefill_tests.rs | 99 +++++++++++++++++++ crates/ui-egui/src/dialogs/filter_dialog.rs | 27 ++++- 3 files changed, 125 insertions(+), 3 deletions(-) create mode 100644 crates/ui-egui/src/dialogs/arbitrary_ruler_prefill_tests.rs diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 82a1b6a0939..003b09ab5d8 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -1225,6 +1225,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; diff --git a/crates/ui-egui/src/dialogs/arbitrary_ruler_prefill_tests.rs b/crates/ui-egui/src/dialogs/arbitrary_ruler_prefill_tests.rs new file mode 100644 index 00000000000..cb947d0b0b3 --- /dev/null +++ b/crates/ui-egui/src/dialogs/arbitrary_ruler_prefill_tests.rs @@ -0,0 +1,99 @@ +//! #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) { + 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) { + 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 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"); +} diff --git a/crates/ui-egui/src/dialogs/filter_dialog.rs b/crates/ui-egui/src/dialogs/filter_dialog.rs index 540c1bb3b97..1aa350d53ba 100644 --- a/crates/ui-egui/src/dialogs/filter_dialog.rs +++ b/crates/ui-egui/src/dialogs/filter_dialog.rs @@ -763,6 +763,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 { + 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). @@ -924,11 +944,12 @@ pub fn body(ui: &mut egui::Ui, f: &mut Map) { } 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); + crate::widgets::slider_row_prec(ui, &label(&p.key), &mut v, min..=max, unit, None, gestures, wide_field_decimals(&cmd, &p.key)); } }); - // 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 }; + // 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); f.insert(p.key, json!((v * scale).round() / scale)); } Kind::Choice { options, default } => { From fe6399312f0d96525e603c16d1616f250f028aba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hartmut=20N=C3=B6renberg?= Date: Sun, 8 Nov 2026 00:46:26 +0100 Subject: [PATCH 2/2] fix(ui): round the ruler angle before the field narrows to f32 (#12035) A 43x27 ruler line is 32.1249984 degrees. straighten_defaults hands that full-precision value to the dialog, the Angle field reads it into an f32 that lands exactly on 32.125, and the f32 write-back round (half away from zero) turned it into 32.13: the dialog showed, and OK ran, an angle the ruler never was. The hundredths are a f64 question, so a field that shows two decimals now rounds the f64 value the field holds and only then narrows to f32; a value the user typed or dragged is f32 already and keeps today's arithmetic, so every other field is untouched. The review's 43x27 case is pinned in the test file: red shows the dialog opening at 32.130001, green at 32.12 (Counter clockwise), with the shown string asserted as "32.12". --- .../dialogs/arbitrary_ruler_prefill_tests.rs | 14 ++++++++++++++ crates/ui-egui/src/dialogs/filter_dialog.rs | 19 ++++++++++++++++--- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/crates/ui-egui/src/dialogs/arbitrary_ruler_prefill_tests.rs b/crates/ui-egui/src/dialogs/arbitrary_ruler_prefill_tests.rs index cb947d0b0b3..c139a4e1e52 100644 --- a/crates/ui-egui/src/dialogs/arbitrary_ruler_prefill_tests.rs +++ b/crates/ui-egui/src/dialogs/arbitrary_ruler_prefill_tests.rs @@ -69,6 +69,20 @@ fn ok_runs_the_two_decimal_angle() { 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. diff --git a/crates/ui-egui/src/dialogs/filter_dialog.rs b/crates/ui-egui/src/dialogs/filter_dialog.rs index 1aa350d53ba..ce2198d22c1 100644 --- a/crates/ui-egui/src/dialogs/filter_dialog.rs +++ b/crates/ui-egui/src/dialogs/filter_dialog.rs @@ -901,7 +901,10 @@ pub fn body(ui: &mut egui::Ui, f: &mut Map) { } 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); @@ -944,13 +947,23 @@ pub fn body(ui: &mut egui::Ui, f: &mut Map) { } 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_prec(ui, &label(&p.key), &mut v, min..=max, unit, None, gestures, wide_field_decimals(&cmd, &p.key)); + edited = + crate::widgets::slider_row_prec(ui, &label(&p.key), &mut v, min..=max, unit, None, gestures, wide_field_decimals(&cmd, &p.key)) + .changed(); } }); // 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); - f.insert(p.key, json!((v * scale).round() / scale)); + // 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| {