diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 4ab8a2eeb3f..b192880f9c1 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/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; 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..c139a4e1e52 --- /dev/null +++ b/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) { + 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 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"); +} diff --git a/crates/ui-egui/src/dialogs/filter_dialog.rs b/crates/ui-egui/src/dialogs/filter_dialog.rs index 4271cf21ed1..8a4963538ce 100644 --- a/crates/ui-egui/src/dialogs/filter_dialog.rs +++ b/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 { + 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) { } 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) { } 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| {