mirror of
https://github.com/storytold/photocraft.git
synced 2026-10-14 19:26:51 -04:00
Fix filter preview live during drag (#5549)
* Filter dialogs: paint the preview computed mid-drag instead of waiting for the pointer to stop
A preview that finished while the slider was still moving was thrown away before it
could reach the canvas, because the worker result had to match the request on every
field including the parameters. Every completion of a drag lands after the value has
moved on, so none of them matched, the worker was handed the newest value instead, and
that one went stale in turn. Nothing was ever painted while the pointer moved: the
effect appeared only once the slider had been still for about a frame of preview
time, which is what "I have to stop the slider and wait" looks like.
The display gate a few lines below already had the opposite rule. It compared the
document, revision, dialog, layer, channel target, command and proxy factor and
deliberately left the parameters out, so that a slightly old frame stays on screen
mid-drag. The publish gate disagreed with it and won, so the frame the display gate
was written to keep never existed.
Publish a successful result when the finished request matches the current one on
everything except the parameters, and let the existing display gate decide whether to
keep it. The two gates now share one predicate, FilterPreviewKey::same_target, so
they cannot drift apart again.
Errors keep the exact match. A value the pointer has already dragged past should not
raise a notice, and a failure is not something to show stale.
What this deliberately does not change:
- A reopened dialog has a new id, a committed document a new revision, a different
command or channel target a different key, so a result from another preview is still
never painted. Covered by a_reopened_dialog_discards_the_previous_dialogs_finished_preview.
- The freshly computed request is still what decides whether a new computation starts,
so the worker still converges on the value the dialog actually holds.
- One preview runs at a time and a stale one is still cancelled only once the values
settle, so this adds no work to the worker; it only stops discarding completed work.
- OK still recomputes at full resolution, so a frame shown mid-drag is never what gets
written to the document.
Test: a_preview_computed_mid_drag_reaches_the_canvas starts a blur at one radius, moves
the slider before that preview lands, and requires the completed frame to reach the
canvas while the slider is still on the new value. It fails on the old gate, timing out
after 10 s with no frame published.
* Filter dialogs: three guards on a mid-drag preview, after review
The first version of this let any completed frame through that matched the dialog
on everything but the parameters. Review found three ways that is wrong, and all
three are now closed and pinned by a test.
A stale result with nothing to show. Storing it replaces a good frame with an empty
one, which the display gate paints as "draw nothing": the canvas blinks back to the
unfiltered layer mid-drag. The exact request may still store an empty result, which
is how a filter that no longer applies clears the canvas, so the guard belongs on
the stale arm only. Splitting the match arm is what makes that separable; the
earlier single arm forced a false choice between the two.
A late frame replacing an exact one. Drag the slider away and straight back: the
frame on screen is already exact for the dialog, and the in-flight frame for the
value on the way out would have overwritten it, showing a blur the user has already
undone. A stale result is now dropped when the stored preview is already exact.
Errors were already scoped to the exact request and stay there: a value the pointer
has dragged past should not raise a notice, and the value it comes to rest on will
raise its own when it is recomputed.
Also pinned by test, because both are easy to undo by accident:
- The reopen test was not reaching the gate. discard_closed reclaims a closed
dialog's job a frame earlier in the real app, and the test called it too, so the
stale result never arrived and the test passed even with the dialog id removed
from the comparison. It now drives the gate directly. Verified by mutation.
- The mid-drag test asserted nothing about what the display gate does with the
published frame, so re-adding params there passed. It now asserts the canvas
actually returns something to draw, and it keeps polling until a frame for the
dialog's current value lands, which is what pins fresh to an exact match.
Tests are deterministic: the worker job is gated on a channel and signals when its
result is already in the worker channel, so nothing sleeps and nothing races the
150 ms settle timer that cancels a job whose values stopped changing.
Verified by mutating each guard and confirming a test catches it:
publish gate back to exact-match ................. killed
drop the is_some guard ........................... killed
drop the already-exact guard .................... killed
same_target ignores the dialog id ............... killed
display gate compares params again .............. killed
fresh relaxed to same_target .................... killed
same_target ignores k ........................... killed
k stays in the comparison on purpose. A 36 MP document previews at k=4, and
preview_factor lowers k as a pixel-sized parameter approaches its minimum, so k does
change under the slider. Relaxing it would paint a frame at a scale the preview
policy exists to avoid, and would loosen the display gate further than upstream
wrote it. The cost is a one-computation pause when the slider crosses a threshold
on a large document; that test keeps the choice visible.
* Filter dialogs: fix a race in the new tests that let CI pass them
The four new tests signalled each other through a channel the worker sends from inside
`compute`. But the worker only puts the result into the channel after `compute` returns:
let result = compute(&work);
let _ = tx.send(result);
So a test that woke on the signal and called `ensure_filter_preview` immediately could
reach `poll()` before that send. On a loaded CI runner it did: ubuntu failed
`a_mid_drag_empty_result_leaves_the_preview_alone` with no frame published at all. The
comment in the test claimed the result was already in the channel, which was the one
thing it was not.
The tests now drive the canvas until the state they need appears, which does not depend
on the order of that send. They also check their invariant on every pass rather than
only at the end: a wrongly published frame can be replaced by the next, correct one
before the loop stops, which had already left the reopen, late-frame and empty-result
tests unable to see their own mutation.
Unchanged: the assertions, the production code, and the four mutations that broke each
guard — re-verified, 7 of 7 still caught.
Runs: 30 back-to-back runs of the four tests on 12 cores, no failure; the full crate
twice, 2800 pass.
---------
Co-authored-by: agent-346022 <[email protected]>
This commit is contained in:
1 parent
64efae47c8
commit
602344bae0
3 files changed
+302
-62
No files matched your search
+234
-60
@@ -1733,15 +1733,29 @@ pub(crate) fn ensure_filter_preview(app: &mut PhotocraftApp, idx: usize, ctx: &e
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
if app.background_jobs {
|
||||
app.filter_preview_worker.supersede(&request, crate::gpu_canvas::now_ms());
|
||||
if let Some((finished, computed)) = app.filter_preview_worker.poll()
|
||||
&& finished == request
|
||||
{
|
||||
if let Some((finished, computed)) = app.filter_preview_worker.poll() {
|
||||
match computed {
|
||||
Ok(computed) => publish_filter_preview(app, idx, finished, computed.result, computed.ms),
|
||||
Err(error) => {
|
||||
// The dialog still holds these exact parameters. Publish as before, including an
|
||||
// empty result: that is how a filter which no longer applies clears the canvas.
|
||||
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.
|
||||
Ok(computed)
|
||||
if finished.same_target(&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);
|
||||
}
|
||||
Ok(_) => {}
|
||||
// Errors stay on the exact request: a value the pointer has already dragged past
|
||||
// must not raise a notice, and a still one will raise it when it is recomputed.
|
||||
Err(error) if finished == request => {
|
||||
app.filter_preview = Some(crate::filter_dialog::FilterPreview { key: finished, result: None, committing: false });
|
||||
crate::notices::error(app, error);
|
||||
}
|
||||
Err(_) => {}
|
||||
}
|
||||
}
|
||||
let fresh = app.filter_preview.as_ref().is_some_and(|p| p.key == request);
|
||||
@@ -1776,15 +1790,7 @@ pub(crate) fn ensure_filter_preview(app: &mut PhotocraftApp, idx: usize, ctx: &e
|
||||
}
|
||||
let preview = app.filter_preview.as_ref()?;
|
||||
// Keep the last preview during a drag, but never one from another source or dialog.
|
||||
// Completed worker results above must match every parameter before being uploaded.
|
||||
if preview.key.doc != request.doc
|
||||
|| preview.key.revision != request.revision
|
||||
|| preview.key.dialog != request.dialog
|
||||
|| preview.key.active != request.active
|
||||
|| preview.key.target != request.target
|
||||
|| preview.key.k != request.k
|
||||
|| preview.key.command != request.command
|
||||
{
|
||||
if !preview.key.same_target(&request) {
|
||||
return None;
|
||||
}
|
||||
let result = preview.result.as_ref()?;
|
||||
@@ -6078,57 +6084,225 @@ mod tests {
|
||||
assert!(app.filter_preview.as_ref().unwrap().result.is_some());
|
||||
}
|
||||
|
||||
// Starts a worker job that computes a real preview for the dialog as it stands, but only
|
||||
// finishes when the returned sender is signalled. Gating it here keeps these tests off the
|
||||
// clock: `supersede` cancels a running job once the values have been still for SETTLE_MS, so a
|
||||
// test that relies on a real blur beating a 150 ms timer is a test that fails on a loaded box.
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
fn gated_blur_preview(app: &mut PhotocraftApp, ctx: &egui::Context) -> (std::sync::mpsc::Sender<()>, f64) {
|
||||
let d = app.ui.dialogs.first().expect("a dialog is open");
|
||||
let st = app.session.active().expect("a document is active");
|
||||
let command = "filter.blur.gaussianBlur";
|
||||
let params = app.with_mask_target(command, crate::filter_dialog::params_of(&d.fields));
|
||||
assert_eq!(
|
||||
crate::filter_dialog::preview_factor(command, ¶ms, 1),
|
||||
1,
|
||||
"a 64x48 document previews at full resolution, and these tests rely on k staying 1"
|
||||
);
|
||||
let key = crate::filter_dialog::FilterPreviewKey {
|
||||
doc: st.doc.id,
|
||||
revision: st.revision,
|
||||
dialog: d.id,
|
||||
active: st.active_layer,
|
||||
target: st.channel_view.target,
|
||||
command: command.into(),
|
||||
params,
|
||||
k: 1,
|
||||
};
|
||||
let doc = app.session.documents()[0].doc.clone();
|
||||
let documents: Vec<_> = app.session.documents().iter().map(|d| d.doc.clone()).collect();
|
||||
let radius = key.params["radius"].as_f64().expect("the dialog seeds a radius");
|
||||
let (release, wait) = std::sync::mpsc::channel::<()>();
|
||||
app.filter_preview_worker
|
||||
.start(key.clone(), ctx.clone(), move |_| {
|
||||
wait.recv_timeout(std::time::Duration::from_secs(60)).expect("the test releases the job");
|
||||
let result =
|
||||
crate::filter_dialog::preview_document_for_target(&doc, key.active, &key.command, &key.params, key.k, None, key.target, &documents).map(
|
||||
|d| {
|
||||
let buffer = photocraft_compose::flatten(&d);
|
||||
(std::sync::Arc::new(d), buffer)
|
||||
},
|
||||
);
|
||||
crate::filter_preview_worker::Computed { result, ms: 0.0 }
|
||||
})
|
||||
.expect("the worker starts");
|
||||
(release, radius)
|
||||
}
|
||||
|
||||
// The worker hands its result to the channel only after `compute` returns, so a signal sent
|
||||
// from inside `compute` would still race that send. These tests drive the canvas until the
|
||||
// state they need appears instead of assuming a timing.
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
fn drive_until(app: &mut PhotocraftApp, ctx: &egui::Context, what: &str, done: impl Fn(&PhotocraftApp) -> bool) {
|
||||
let deadline = std::time::Instant::now() + std::time::Duration::from_secs(60);
|
||||
while !done(app) {
|
||||
assert!(std::time::Instant::now() < deadline, "timed out waiting for {what}");
|
||||
ensure_filter_preview(app, 0, ctx);
|
||||
std::thread::sleep(std::time::Duration::from_millis(1));
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
#[test]
|
||||
fn indexed_preview_discards_a_finished_job_after_slider_change_or_reopening() {
|
||||
for reopen in [false, true] {
|
||||
let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), Default::default());
|
||||
app.run("file.new", json!({"width": 8, "height": 8})).unwrap();
|
||||
app.background_jobs = true;
|
||||
crate::filter_dialog::open(&mut app, "image.mode.indexedColor").unwrap();
|
||||
let d = app.ui.dialogs.first().unwrap();
|
||||
let st = app.session.active().unwrap();
|
||||
let old = crate::filter_dialog::FilterPreviewKey {
|
||||
doc: st.doc.id,
|
||||
revision: st.revision,
|
||||
dialog: d.id,
|
||||
active: st.active_layer,
|
||||
target: st.channel_view.target,
|
||||
command: "image.mode.indexedColor".into(),
|
||||
params: app.with_mask_target("image.mode.indexedColor", crate::filter_dialog::params_of(&d.fields)),
|
||||
k: 1,
|
||||
};
|
||||
let (release, wait) = std::sync::mpsc::channel();
|
||||
let ctx = egui::Context::default();
|
||||
app.filter_preview_worker
|
||||
.start(old.clone(), ctx.clone(), move |_| {
|
||||
wait.recv_timeout(std::time::Duration::from_secs(5)).unwrap();
|
||||
crate::filter_preview_worker::Computed { result: None, ms: 0.0 }
|
||||
})
|
||||
.unwrap();
|
||||
if reopen {
|
||||
app.ui.dialogs.clear();
|
||||
crate::filter_preview_worker::discard_closed(&mut app);
|
||||
crate::filter_dialog::open(&mut app, "image.mode.indexedColor").unwrap();
|
||||
} else {
|
||||
app.ui.dialogs.first_mut().unwrap().fields.insert("colors".into(), json!(16));
|
||||
}
|
||||
assert!(ensure_filter_preview(&mut app, 0, &ctx).is_none());
|
||||
assert!(app.filter_preview.is_none());
|
||||
release.send(()).unwrap();
|
||||
let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5);
|
||||
while app.filter_preview.is_none() {
|
||||
assert!(std::time::Instant::now() < deadline);
|
||||
ensure_filter_preview(&mut app, 0, &ctx);
|
||||
if let Some(preview) = &app.filter_preview {
|
||||
assert_ne!(preview.key, old, "an outdated preview was published");
|
||||
}
|
||||
std::thread::sleep(std::time::Duration::from_millis(1));
|
||||
}
|
||||
assert_eq!(app.session.active().unwrap().doc.mode, photocraft_color::ColorMode::Rgb);
|
||||
fn a_preview_computed_mid_drag_reaches_the_canvas_and_then_converges() {
|
||||
// A drag used to paint nothing at all: every completion was dropped for carrying stale
|
||||
// parameters, so the canvas only ever showed an effect once the pointer had come to rest.
|
||||
let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), Default::default());
|
||||
app.run("file.new", json!({"width": 64, "height": 48})).unwrap();
|
||||
app.background_jobs = true;
|
||||
crate::filter_dialog::open(&mut app, "filter.blur.gaussianBlur").unwrap();
|
||||
let ctx = egui::Context::default();
|
||||
let (release, first) = gated_blur_preview(&mut app, &ctx);
|
||||
|
||||
// The pointer moves on before the frame lands.
|
||||
let moved = first + 4.0;
|
||||
app.ui.dialogs[0].fields.insert("radius".into(), json!(moved));
|
||||
release.send(()).expect("released");
|
||||
drive_until(&mut app, &ctx, "the mid-drag frame", |a| a.filter_preview.is_some());
|
||||
|
||||
// The finished frame is painted even though the dialog has moved on.
|
||||
let shown = app.filter_preview.as_ref().expect("the mid-drag frame was published");
|
||||
assert_eq!(shown.key.params["radius"].as_f64(), Some(first), "the frame is the one the worker finished");
|
||||
assert!(shown.result.is_some());
|
||||
assert_eq!(app.ui.dialogs[0].fields["radius"].as_f64(), Some(moved), "the slider kept moving");
|
||||
// The display gate lets it through, so the canvas has something to draw: this is what
|
||||
// re-adding `params` to that gate would break.
|
||||
assert!(ensure_filter_preview(&mut app, 0, &ctx).is_some(), "the canvas draws the mid-drag frame");
|
||||
|
||||
// And the preview still converges on the value the dialog holds. `fresh` has to stay an
|
||||
// exact match for that: relaxed, the canvas would believe it already had the current
|
||||
// request after the first frame of a drag and never compute again.
|
||||
let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10);
|
||||
while app.filter_preview.as_ref().is_none_or(|p| p.key.params["radius"].as_f64() != Some(moved)) {
|
||||
assert!(std::time::Instant::now() < deadline, "the preview never converged on the current radius");
|
||||
ensure_filter_preview(&mut app, 0, &ctx);
|
||||
std::thread::sleep(std::time::Duration::from_millis(1));
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
#[test]
|
||||
fn a_mid_drag_frame_does_not_replace_a_frame_that_is_already_exact() {
|
||||
let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), Default::default());
|
||||
app.run("file.new", json!({"width": 64, "height": 48})).unwrap();
|
||||
app.background_jobs = true;
|
||||
crate::filter_dialog::open(&mut app, "filter.blur.gaussianBlur").unwrap();
|
||||
let ctx = egui::Context::default();
|
||||
|
||||
// A frame for the radius the dialog holds right now reaches the canvas. This also proves
|
||||
// the hand-built key is the one the canvas builds: it can only be published by the exact
|
||||
// arm, which requires `finished == request`.
|
||||
let (release, first) = gated_blur_preview(&mut app, &ctx);
|
||||
release.send(()).expect("released");
|
||||
drive_until(&mut app, &ctx, "the first frame", |a| a.filter_preview.is_some());
|
||||
assert_eq!(app.filter_preview.as_ref().expect("published").key.params["radius"].as_f64(), Some(first));
|
||||
|
||||
// The slider moves away and a frame for the new radius is computed...
|
||||
let moved = first + 4.0;
|
||||
app.ui.dialogs[0].fields.insert("radius".into(), json!(moved));
|
||||
let (release_moved, _) = gated_blur_preview(&mut app, &ctx);
|
||||
// ...and back to `first` before that frame lands.
|
||||
app.ui.dialogs[0].fields.insert("radius".into(), json!(first));
|
||||
release_moved.send(()).expect("released");
|
||||
|
||||
// The frame for `moved` arrives late. It belongs to this preview, but the frame on screen
|
||||
// is already exact for the dialog, so it must never be shown in place of it. Checked on
|
||||
// every pass: a wrong frame published and then overwritten before the loop ends would
|
||||
// otherwise never be seen.
|
||||
drive_until(&mut app, &ctx, "the late frame to arrive", |a| {
|
||||
if let Some(shown) = &a.filter_preview {
|
||||
assert_ne!(shown.key.params["radius"].as_f64(), Some(moved), "a late frame replaced an exact one");
|
||||
}
|
||||
!a.filter_preview_worker.busy()
|
||||
});
|
||||
let shown = app.filter_preview.as_ref().expect("a frame is on screen");
|
||||
assert_eq!(shown.key.params["radius"].as_f64(), Some(first), "a late frame replaced an exact one");
|
||||
}
|
||||
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
#[test]
|
||||
fn a_mid_drag_empty_result_leaves_the_preview_alone() {
|
||||
let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), Default::default());
|
||||
app.run("file.new", json!({"width": 64, "height": 48})).unwrap();
|
||||
app.background_jobs = true;
|
||||
crate::filter_dialog::open(&mut app, "filter.blur.gaussianBlur").unwrap();
|
||||
let ctx = egui::Context::default();
|
||||
|
||||
let (release, first) = gated_blur_preview(&mut app, &ctx);
|
||||
release.send(()).expect("released");
|
||||
drive_until(&mut app, &ctx, "the first frame", |a| a.filter_preview.is_some());
|
||||
assert!(app.filter_preview.as_ref().expect("published").result.is_some());
|
||||
|
||||
// The slider moves and a job for the new radius is started, then the slider moves again
|
||||
// before that job lands: its result is stale by the time it arrives.
|
||||
let moved = first + 4.0;
|
||||
app.ui.dialogs[0].fields.insert("radius".into(), json!(moved));
|
||||
let d = app.ui.dialogs.first().expect("a dialog is open");
|
||||
let st = app.session.active().expect("a document is active");
|
||||
let key = crate::filter_dialog::FilterPreviewKey {
|
||||
doc: st.doc.id,
|
||||
revision: st.revision,
|
||||
dialog: d.id,
|
||||
active: st.active_layer,
|
||||
target: st.channel_view.target,
|
||||
command: "filter.blur.gaussianBlur".into(),
|
||||
params: json!({"radius": moved}),
|
||||
k: 1,
|
||||
};
|
||||
let (release_empty, wait) = std::sync::mpsc::channel::<()>();
|
||||
app.filter_preview_worker
|
||||
.start(key, ctx.clone(), move |_| {
|
||||
wait.recv_timeout(std::time::Duration::from_secs(60)).expect("released");
|
||||
crate::filter_preview_worker::Computed { result: None, ms: 0.0 }
|
||||
})
|
||||
.expect("the worker starts");
|
||||
app.ui.dialogs[0].fields.insert("radius".into(), json!(moved + 4.0));
|
||||
release_empty.send(()).expect("released");
|
||||
|
||||
// Storing an empty result here would drop the canvas back to the unfiltered layer for a
|
||||
// frame. The exact request may still store one; a stale one may not. Checked on every
|
||||
// pass, because the reopened preview that follows would otherwise repair the symptom.
|
||||
drive_until(&mut app, &ctx, "the empty frame to arrive", |a| {
|
||||
if let Some(shown) = &a.filter_preview {
|
||||
assert!(shown.result.is_some(), "an empty mid-drag result blanked the canvas");
|
||||
}
|
||||
!a.filter_preview_worker.busy()
|
||||
});
|
||||
assert!(app.filter_preview.as_ref().expect("a preview is still shown").result.is_some(), "an empty mid-drag result blanked the canvas");
|
||||
}
|
||||
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
#[test]
|
||||
fn a_reopened_dialog_never_paints_the_previous_dialogs_preview() {
|
||||
let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), Default::default());
|
||||
app.run("file.new", json!({"width": 64, "height": 48})).unwrap();
|
||||
app.background_jobs = true;
|
||||
crate::filter_dialog::open(&mut app, "filter.blur.gaussianBlur").unwrap();
|
||||
let ctx = egui::Context::default();
|
||||
let first_dialog = app.ui.dialogs.first().expect("a dialog is open").id;
|
||||
let (release, _) = gated_blur_preview(&mut app, &ctx);
|
||||
|
||||
// Close that dialog and open another one for the same layer and the same command. Only the
|
||||
// dialog id differs, which is exactly what `same_target` has to notice.
|
||||
app.ui.dialogs.clear();
|
||||
crate::filter_dialog::open(&mut app, "filter.blur.gaussianBlur").unwrap();
|
||||
let second_dialog = app.ui.dialogs.first().expect("a dialog is open").id;
|
||||
assert_ne!(second_dialog, first_dialog, "a reopened dialog gets a new id");
|
||||
release.send(()).expect("released");
|
||||
|
||||
// The first dialog's frame arrives late and must not be painted under the second one.
|
||||
// `discard_closed` usually reclaims it a frame earlier, which is why this drives the gate
|
||||
// directly: the id check is the part that has to hold when that cleanup misses. The
|
||||
// invariant is checked on every pass, not only at the end, because a wrongly published
|
||||
// frame can still be replaced by the reopened dialog's own preview before we stop.
|
||||
drive_until(&mut app, &ctx, "the first dialog's frame to arrive", |a| {
|
||||
if let Some(preview) = &a.filter_preview {
|
||||
assert_ne!(preview.key.dialog, first_dialog, "a reopened dialog painted the previous dialog's preview");
|
||||
}
|
||||
!a.filter_preview_worker.busy()
|
||||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn windows_uses_a_visible_crosshair() {
|
||||
// #737: Windows' inverting crosshair vanishes over mid-grey; use a black/white glyph.
|
||||
|
||||
@@ -962,7 +962,13 @@ pub struct FilterPreview {
|
||||
pub committing: bool,
|
||||
}
|
||||
|
||||
/// Match the full request before accepting a worker result, including a reopened dialog.
|
||||
/// Identity of one filter dialog's live preview.
|
||||
///
|
||||
/// `PartialEq` is the whole request: it says a worker result is exactly what the dialog is asking
|
||||
/// for now. [`Self::same_target`] is the looser question of whether a result belongs to this
|
||||
/// preview at all; two keys that agree on it name the same dialog, layer, document and preview
|
||||
/// resolution, and differ only in parameters — which is what a slider drag produces, since the
|
||||
/// worker computed for the value the pointer passed a moment ago.
|
||||
#[derive(Clone, Debug, PartialEq)]
|
||||
pub struct FilterPreviewKey {
|
||||
pub doc: photocraft_doc::DocId,
|
||||
@@ -975,6 +981,26 @@ pub struct FilterPreviewKey {
|
||||
pub k: u32,
|
||||
}
|
||||
|
||||
impl FilterPreviewKey {
|
||||
/// Everything except the parameters: same dialog, document, revision, layer, channel target,
|
||||
/// command and proxy factor.
|
||||
///
|
||||
/// The canvas shows whatever this preview was computed for, so a result whose parameters are
|
||||
/// a few frames old still belongs on screen while a slider is moving; matching on everything
|
||||
/// would mean no frame of a drag is ever shown, because every completion lands after the
|
||||
/// parameters moved on. A reopened dialog has a new id, a different command or a different
|
||||
/// document, and a committed document a new revision, so neither survives this test.
|
||||
pub fn same_target(&self, other: &Self) -> bool {
|
||||
self.doc == other.doc
|
||||
&& self.revision == other.revision
|
||||
&& self.dialog == other.dialog
|
||||
&& self.active == other.active
|
||||
&& self.target == other.target
|
||||
&& self.command == other.command
|
||||
&& self.k == other.k
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -988,6 +1014,46 @@ mod tests {
|
||||
assert_eq!(source_label("centerX"), "Center X");
|
||||
}
|
||||
|
||||
fn key() -> FilterPreviewKey {
|
||||
FilterPreviewKey {
|
||||
doc: photocraft_doc::DocId(7),
|
||||
revision: 11,
|
||||
dialog: 3,
|
||||
active: Some(photocraft_doc::LayerId(2)),
|
||||
target: Default::default(),
|
||||
command: "filter.blur.gaussianBlur".into(),
|
||||
params: json!({"radius": 4.0}),
|
||||
k: 1,
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn same_target_ignores_parameters_but_nothing_else() {
|
||||
// A drag only changes the parameters, and that must not hide the finished frame.
|
||||
let base = key();
|
||||
let mut moved = base.clone();
|
||||
moved.params = json!({"radius": 40.0});
|
||||
assert!(base.same_target(&moved));
|
||||
assert!(moved.same_target(&base));
|
||||
|
||||
// Anything else names a different preview and must not be shown.
|
||||
for break_it in [
|
||||
|k: &mut FilterPreviewKey| k.doc = photocraft_doc::DocId(8),
|
||||
|k: &mut FilterPreviewKey| k.revision = 12,
|
||||
|k: &mut FilterPreviewKey| k.dialog = 4,
|
||||
|k: &mut FilterPreviewKey| k.active = Some(photocraft_doc::LayerId(5)),
|
||||
|k: &mut FilterPreviewKey| k.active = None,
|
||||
|k: &mut FilterPreviewKey| k.target = photocraft_engine::channel_cmds::ChannelTarget::LayerMask,
|
||||
|k: &mut FilterPreviewKey| k.command = "filter.blur.motionBlur".into(),
|
||||
|k: &mut FilterPreviewKey| k.k = 2,
|
||||
] {
|
||||
let mut other = base.clone();
|
||||
break_it(&mut other);
|
||||
assert!(!other.same_target(&base), "{other:?} must not pass as {base:?}");
|
||||
assert!(!base.same_target(&other));
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn only_direction_angles_get_the_dial() {
|
||||
// Every listed command still has a degree `angle` range, so the list cannot go stale.
|
||||
|
||||
@@ -160,7 +160,7 @@ mod tests {
|
||||
let (old, result) = finish(&mut worker);
|
||||
assert!(result.is_ok());
|
||||
let latest = key(1, 0, 256);
|
||||
assert_ne!(old, latest); // The canvas must discard this result before uploading.
|
||||
assert_ne!(old, latest); // The canvas paints this as the drag's frame; the newest one is computed next.
|
||||
assert!(worker.start(latest.clone(), ctx, |_| Computed { result: None, ms: 1.0 }).unwrap());
|
||||
assert_eq!(finish(&mut worker).0, latest);
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user