diff --git a/crates/ui-egui/src/canvas/filter_preview.rs b/crates/ui-egui/src/canvas/filter_preview.rs index 0144390b46..3a6ee54d97 100644 --- a/crates/ui-egui/src/canvas/filter_preview.rs +++ b/crates/ui-egui/src/canvas/filter_preview.rs @@ -84,11 +84,14 @@ pub(crate) fn ensure_filter_preview(app: &mut PhotocraftApp, idx: usize, ctx: &e Ok(computed) if finished == request => publish_filter_preview(app, idx, finished, computed.result, computed.ms), // Mid-drag, a frame for the value the pointer has just left is what makes the // canvas follow the drag. Three things have to hold for it: it belongs to this - // preview, it has a frame to show (an empty one would blank a good preview), and - // it must not displace a frame that is already exact for the dialog, which is - // what a slider dragged out and back would otherwise make the canvas do. + // preview (at any proxy factor: it is drawn at its own, and a radius drag can move + // the factor, #10474), it has a frame to show (an empty one would blank a good + // preview), and it must not displace a frame that is already exact for the dialog, + // which is what a slider dragged out and back would otherwise make the canvas do. Ok(computed) - if finished.same_target(&request) && computed.result.is_some() && !app.filter_preview.as_ref().is_some_and(|p| p.key == request) => + if finished.same_target_at_any_scale(&request) + && computed.result.is_some() + && !app.filter_preview.as_ref().is_some_and(|p| p.key == request) => { publish_filter_preview(app, idx, finished, computed.result, computed.ms); } diff --git a/crates/ui-egui/src/canvas/tests.rs b/crates/ui-egui/src/canvas/tests.rs index 6bce89cc0d..73ef696976 100644 --- a/crates/ui-egui/src/canvas/tests.rs +++ b/crates/ui-egui/src/canvas/tests.rs @@ -371,6 +371,31 @@ fn drive_until(app: &mut PhotocraftApp, ctx: &egui::Context, what: &str, done: i } } +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn a_held_preview_frame_stays_on_the_canvas_when_the_proxy_factor_changes() { + // #10474: Minimum and Maximum take their proxy factor from the radius, so a radius drag across + // a whole pixel changes `FilterPreviewKey::k`. The frame for the old factor stays on the + // canvas, drawn at its own factor, until the one for the new factor lands. + let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), Default::default()); + app.run("file.new", json!({"width": 2400, "height": 2200})).unwrap(); + app.background_jobs = true; + app.ui.views.get_mut(0).expect("the new document's view").zoom = 0.25; + crate::filter_dialog::open(&mut app, "filter.other.maximum").unwrap(); + app.ui.dialogs[0].fields.insert("radius".into(), json!(6.0)); + let ctx = egui::Context::default(); + drive_until(&mut app, &ctx, "the first frame", |a| a.filter_preview.as_ref().is_some_and(|p| p.result.is_some())); + assert_eq!(app.filter_preview.as_ref().map(|p| p.key.k), Some(2), "6 px on a reduced document previews on the proxy"); + + // 1.5 px previews at full resolution: a new request at a new factor, computed on the worker. + app.ui.dialogs[0].fields.insert("radius".into(), json!(1.5)); + let (k, _, size) = ensure_filter_preview(&mut app, 0, &ctx).expect("the held frame stays on the canvas"); + assert_eq!((k, size), (2, [1200, 1100]), "drawn at its own factor"); + drive_until(&mut app, &ctx, "the full-resolution frame", |a| a.filter_preview.as_ref().is_some_and(|p| p.key.k == 1 && p.result.is_some())); + let (k, _, size) = ensure_filter_preview(&mut app, 0, &ctx).expect("the new frame"); + assert_eq!((k, size), (1, [2400, 2200])); +} + #[cfg(not(target_arch = "wasm32"))] #[test] fn a_preview_computed_mid_drag_reaches_the_canvas_and_then_converges() { diff --git a/crates/ui-egui/src/dialogs/filter_dialog.rs b/crates/ui-egui/src/dialogs/filter_dialog.rs index 08ce0f6ab1..f2e97a80e8 100644 --- a/crates/ui-egui/src/dialogs/filter_dialog.rs +++ b/crates/ui-egui/src/dialogs/filter_dialog.rs @@ -279,16 +279,17 @@ pub fn preview_factor(command: &str, params: &Value, k: u32) -> u32 { // Minimum and Maximum measure the radius in whole pixels (Squareness rounds it, Roundness // floors it, and 0 is the identity), so at k = 4 a typed 1.5 px became 0.375 = 0 proxy px and // the preview showed the untouched document while OK applied a real effect (#10474). Keep at - // least one proxy pixel of radius, and for Squareness the largest k whose rounded proxy radius, - // scaled back, is within 1 px of the one OK applies (6 px at k = 4 rounded to 2 proxy px, 8 px). + // least one proxy pixel of radius, and the largest k whose whole-pixel proxy radius, scaled + // back, is within 1 px of the one OK applies: Squareness at 6 px and k = 4 rounds 1.5 to 2 proxy + // px (8 px), and Roundness's disc reaches floor(1.5) = 1 proxy px along an axis (4 px). // A large radius still previews on a proxy: at full size Roundness costs O(radius) per pixel. if matches!(command, "filter.other.minimum" | "filter.other.maximum") { let Some(r) = params.get("radius").and_then(Value::as_f64).filter(|r| r.is_finite()) else { return k }; let r = r.clamp(0.2, 500.0); - let square = params.get("preserve").and_then(Value::as_str) != Some("roundness"); + let whole: fn(f64) -> f64 = if params.get("preserve").and_then(Value::as_str) == Some("roundness") { f64::floor } else { f64::round }; let fits = |d: u32| { let d = f64::from(d); - d <= r.floor() && (!square || ((r / d).round() * d - r.round()).abs() <= 1.0) + d <= r.floor() && (whole(r / d) * d - whole(r)).abs() <= 1.0 }; return (1..=k).rev().find(|&d| fits(d)).unwrap_or(1); } diff --git a/crates/ui-egui/src/dialogs/filter_dialog/tests.rs b/crates/ui-egui/src/dialogs/filter_dialog/tests.rs index c5013adc25..dcaa32ddbd 100644 --- a/crates/ui-egui/src/dialogs/filter_dialog/tests.rs +++ b/crates/ui-egui/src/dialogs/filter_dialog/tests.rs @@ -1047,16 +1047,29 @@ fn proxy_previews_scale_only_lengths() { assert_eq!(preview_factor("filter.pixelate.mosaic", &json!({"cellSize": "x"}), 4), 4); let (k, diff) = error("filter.pixelate.mosaic", json!({"cellSize": 10.0})); assert!(k == 2 && diff < 0.01, "Mosaic: {diff}"); + // Roundness at a fractional radius: at 6.5 px, k = 3 reaches floor(2.17) = 2 proxy px along an + // axis (6 px), where k = 4 reached floor(1.63) = 1 (4 px), so the chosen factor previews closer + // to what OK applies than k = 4 did (#10474). + for cmd in ["filter.other.minimum", "filter.other.maximum"] { + let p = json!({"radius": 6.5, "preserve": "roundness"}); + let (k, diff) = error(cmd, p.clone()); + let applied = preview_document(&doc, Some(bg), cmd, &p, 1).unwrap(); + let at_four = preview_document(&doc, Some(bg), cmd, &p, 4).unwrap(); + let four = mean_abs(&every_kth(&applied, 4), &every_kth(&at_four, 1)); + assert!(k == 3 && diff < four, "{cmd}: k = {k} differs by {diff}, k = 4 by {four}"); + } // Minimum and Maximum keep at least one proxy pixel of radius (#10474), and a large radius - // keeps a proxy instead of a full-size preview that costs minutes at radius 500. Squareness - // also keeps its rounded radius within 1 px of OK's: 6 px is 2 proxy px at k = 3 (6 px), not - // at k = 4 (8 px); 50 px is 17 at k = 3 (51 px), not 13 at k = 4 (52 px). + // keeps a proxy instead of a full-size preview that costs minutes at radius 500. Both keep + // their whole-pixel radius within 1 px of OK's. Squareness rounds: 6 px is 2 proxy px at k = 3 + // (6 px), not at k = 4 (8 px); 50 px is 17 at k = 3 (51 px), not 13 at k = 4 (52 px). Roundness's + // disc reaches floor(r) along an axis: 6 px is 2 at k = 3 (6 px), not 1 at k = 4 (4 px); 50 px + // is 25 at k = 2, not 12 at k = 4 or 16 at k = 3 (48 px). let factors = |cmd: &str, preserve: &str| { [0.2, 1.5, 2.5, 3.9, 6.0, 50.0, 500.0, 9999.0].map(|radius| preview_factor(cmd, &json!({"radius": radius, "preserve": preserve}), 4)) }; for cmd in ["filter.other.minimum", "filter.other.maximum"] { assert_eq!(factors(cmd, "squareness"), [1, 1, 2, 3, 3, 3, 4, 4], "{cmd} squareness"); - assert_eq!(factors(cmd, "roundness"), [1, 1, 2, 3, 4, 4, 4, 4], "{cmd} roundness"); + assert_eq!(factors(cmd, "roundness"), [1, 1, 2, 3, 3, 2, 4, 4], "{cmd} roundness"); assert_eq!(preview_factor(cmd, &json!({"radius": 6.0}), 4), 3, "{cmd}: Squareness is the default"); assert_eq!(preview_factor(cmd, &json!({"radius": "x"}), 4), 4, "{cmd}: a junk radius keeps the proxy"); }