From 2e54e882d6d35ea3afa2dc6504585f2e31bcb80f Mon Sep 17 00:00:00 2001 From: VoiceLessQ Date: Thu, 29 Oct 2026 14:00:13 -0200 Subject: [PATCH 1/2] Image Trace: a library colour changed meanwhile makes a background trace stale, and running traces keep frames coming From the review of #4605. TraceParams::swatches isn't in the settings' JSON (serde skip), so a Document Library trace whose swatches were edited while it ran still applied, with the old colours; the trace's inputs now hold them. The 100 ms repaint that keeps background work moving left traces out, so without input a finished trace could wait for the next event. --- crates/engine/src/cmd/buildcmds/trace.rs | 9 ++++++--- crates/engine/src/tests/build.rs | 15 +++++++++++++++ crates/ui-egui/src/app.rs | 1 + 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/cmd/buildcmds/trace.rs b/crates/engine/src/cmd/buildcmds/trace.rs index 8d72568e8..308e300ec 100644 --- a/crates/engine/src/cmd/buildcmds/trace.rs +++ b/crates/engine/src/cmd/buildcmds/trace.rs @@ -377,8 +377,11 @@ fn grouped(d: &mut Document, nodes: Vec, parents: &[Option]) -> Vec struct TraceInputs { target: NodeId, image: Box, - /// The settings traced with, as JSON, the library's colours in. + /// The settings traced with, as JSON. params: Value, + /// Document Library: the library's colours, which the JSON leaves out + /// ([`tr::TraceParams::swatches`]). + swatches: Vec<[u8; 3]>, } /// What `imageTrace.make` and `imageTrace.makeAndExpand` take from the session before they trace. @@ -431,7 +434,7 @@ fn prepare(s: &Session, c: &str, p: &Value) -> Result { let bytes = Arc::clone(&blob.bytes); let library = trace_library(c, s, &mut params)?; let traced_with = serde_json::to_value(¶ms).map_err(|e| EngineError::Other(e.to_string()))?; - let inputs = TraceInputs { target, image, params: traced_with }; + let inputs = TraceInputs { target, image, params: traced_with, swatches: params.swatches.clone() }; Ok(Prepared { inputs, trace: params, view, params_json, preset_label, library, bytes }) } @@ -544,7 +547,7 @@ pub(super) fn trace_make(s: &mut Session, p: &Value, expand: bool) -> Result traced, _ => trace_now(&inputs, &trace, &bytes, &|_, _| true)?, }; - let TraceInputs { target, image: img, params: _ } = inputs; + let TraceInputs { target, image: img, .. } = inputs; let warnings = trace_warnings((img.width, img.height), raster, res.worked_at); // Pixel space of the decoded raster → the image object's pixel space → document. let sx = img.width.max(1) as f64 / raster.0.max(1) as f64; diff --git a/crates/engine/src/tests/build.rs b/crates/engine/src/tests/build.rs index 5c7baad6f..cb4c2d018 100644 --- a/crates/engine/src/tests/build.rs +++ b/crates/engine/src/tests/build.rs @@ -2768,6 +2768,21 @@ fn a_trace_run_hands_its_trace_to_the_command() { assert!((bb.center().x - 160.0).abs() < 1.5, "traced where the image is now: {bb:?}"); } +/// A Document Library trace made away from its command no longer applies once the library's +/// colours change: the settings' JSON leaves them out (#4605 review). +#[test] +fn a_trace_run_needs_the_library_colours_it_traced_with() { + let mut s = session(); + image_doc(&mut s); + let p = json!({"preset": "6 Colors", "params": {"palette": "documentLibrary"}}); + let run = cmd::TraceRun::prepare(&s, "imageTrace.make", &p).unwrap(); + assert!(run.current(&s)); + let swatch = |w: &&vectorcraft_color::Swatch| matches!(w.paint, Paint::Solid { .. }) && !w.name.starts_with('['); + let name = s.doc().unwrap().doc.swatches_iter().find(swatch).map(|w| w.name.clone()).unwrap(); + s.execute("swatch.edit", &json!({"name": name, "color": "#123456"})).unwrap(); + assert!(!run.current(&s), "{name} changed"); +} + /// Shapes: the disc of the test image comes out as a live ellipse where the image lies, the same /// circle as its traced path; without the option it is a plain path. #[test] diff --git a/crates/ui-egui/src/app.rs b/crates/ui-egui/src/app.rs index 5a46f6157..6e2527fb6 100644 --- a/crates/ui-egui/src/app.rs +++ b/crates/ui-egui/src/app.rs @@ -298,6 +298,7 @@ impl VectorcraftApp { || !self.opening.pending.is_empty() || !self.plugin_runs.running.is_empty() || !self.placing.pending.is_empty() + || !self.trace_runs.running.is_empty() { // Keep the status bar's progress moving and pick the result up when it arrives. ctx.request_repaint_after(std::time::Duration::from_millis(100)); From 3301db5817aeb76aca3052b2d03924730d1829bd Mon Sep 17 00:00:00 2001 From: VoiceLessQ Date: Thu, 29 Oct 2026 14:48:55 -0200 Subject: [PATCH 2/2] Image Trace: starting a background trace asks for a frame The frame a trace starts in may already have asked for its next one before the trace started, so with no input the finished trace could wait. From the review of #4617. --- crates/ui-egui/src/tests/trace_runs.rs | 21 +++++++++++++++++++++ crates/ui-egui/src/trace_runs.rs | 4 ++++ 2 files changed, 25 insertions(+) diff --git a/crates/ui-egui/src/tests/trace_runs.rs b/crates/ui-egui/src/tests/trace_runs.rs index 9fbd1b2d6..d2de2f00f 100644 --- a/crates/ui-egui/src/tests/trace_runs.rs +++ b/crates/ui-egui/src/tests/trace_runs.rs @@ -76,6 +76,27 @@ fn make_traces_off_the_ui_thread_as_one_undo_step() { assert_eq!(app.session.doc().unwrap().history.undo.last().unwrap().label, "Image Trace (Make and Expand)"); } +/// Starting a trace asks for a frame, so its result is picked up without input: the frame that +/// started it may have asked for its next one before it did (#4617 review). +#[test] +fn starting_a_trace_asks_for_a_frame() { + let mut app = app(); + let ctx = egui::Context::default(); + app.adopt_context(&ctx); + for _ in 0..5 { + if !ctx.has_requested_repaint() { + break; + } + let mut out = ctx.run_ui(Default::default(), |_| {}); + out.textures_delta.clear(); + } + assert!(!ctx.has_requested_repaint(), "the context settled"); + crate::menus::invoke(&mut app, "imageTrace.make", json!({"preset": "Default"})); + assert_eq!(app.trace_runs.running.len(), 1); + assert!(ctx.has_requested_repaint()); + wait(&mut app); +} + /// Without threads (the web, `VECTORCRAFT_SYNC_OPEN`, unit tests that don't ask), Make traces at once. #[test] fn without_threads_make_traces_at_once() { diff --git a/crates/ui-egui/src/trace_runs.rs b/crates/ui-egui/src/trace_runs.rs index a5bf0fd5e..e05448b10 100644 --- a/crates/ui-egui/src/trace_runs.rs +++ b/crates/ui-egui/src/trace_runs.rs @@ -121,6 +121,10 @@ pub fn start(app: &mut VectorcraftApp, command: &str, p: Value, from_panel: bool let runs = &mut app.trace_runs; runs.next_id += 1; runs.running.push(Running { id: runs.next_id, doc, target, token, superseded: false, steps, from_panel, hidden: false, done }); + // This frame may have asked for its next one before the trace started (`tick`): ask now. + if let Some(ctx) = &app.ui_ctx { + ctx.request_repaint(); + } let status = format!("{}…", crate::i18n::msg(LABEL)); app.status(status.clone()); Ok(json!({ "background": true, "status": status }))