fix(ui): Roundness previews its radius within 1 px, and a held frame survives a factor change (#10474) (#11081)

* fix(ui): Roundness previews its radius within 1 px too, and a held frame survives a factor change (#10474)

Follow-up to #10524 from its last CodeRabbit review.

- Roundness's disc reaches floor(r) pixels along an axis, so at 6 px
  and k = 4 the proxy radius 1.5 reached 1 proxy px: a 4 px preview for
  a 6 px result. The factor now keeps floor(r / k) * k within 1 px of
  floor(r), as Squareness already does with round (k = 3 at 6 px, k = 2
  at 50 px). One whole-pixel function per mode replaces the special case.
- A canvas test drives the worker across a factor change: the frame for
  k = 2 stays on the canvas, at its own size, until the k = 1 frame lands.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test(ui): a fractional Roundness radius previews closer at its chosen factor (#10474)

CodeRabbit on #11081: only the factors were pinned. At 6.5 px the
Roundness preview at the chosen k = 3 is compared with OK's result and
must differ less than the k = 4 preview did.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(ui): a mid-drag frame for the previous proxy factor still reaches the canvas (#10474)

CodeRabbit on #11081: the display gate keeps a held frame at any factor,
but the mid-drag publish arm still required the same factor, so a frame
finished for the factor the drag just left was dropped. It now uses
same_target_at_any_scale too; a frame already exact for the dialog is
still never displaced.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
FredAppsandClaude Opus 5.5 authored and GitHub committed 2026-11-03 17:55:42 +01:00
1 parent d21c403e5f
commit d9dc644db1
4 files changed
+54 -12

No files matched your search

+7 -4
View File
@@ -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);
}
+25
View File
@@ -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() {
+5 -4
View File
@@ -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);
}
@@ -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");
}