diff --git a/crates/engine/src/cmd/buildcmds/trace.rs b/crates/engine/src/cmd/buildcmds/trace.rs index 9c6c28b900..11519b84be 100644 --- a/crates/engine/src/cmd/buildcmds/trace.rs +++ b/crates/engine/src/cmd/buildcmds/trace.rs @@ -527,8 +527,9 @@ fn color_groups(cmd: &str, d: &mut Document, items: Vec>) -> Result, + /// The encoded image the tracer decodes: the blob under the image's key, shared with the + /// document. Two blobs are equal when they are the same allocation or hold the same bytes. + bytes: Arc>, } /// What `imageTrace.make` and `imageTrace.makeAndExpand` take from the session before they trace. @@ -549,7 +553,6 @@ struct Prepared { params_json: Value, preset_label: String, library: Vec<([u8; 3], Paint)>, - bytes: Arc>, } /// What command `c` with params `p` traces, and how, from the session: everything but the trace. @@ -597,8 +600,8 @@ 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, swatches: params.swatches.clone() }; - Ok(Prepared { inputs, trace: params, view, params_json, preset_label, library, bytes }) + let inputs = TraceInputs { target, image, params: traced_with, swatches: params.swatches.clone(), bytes }; + Ok(Prepared { inputs, trace: params, view, params_json, preset_label, library }) } /// What a trace made of an image: the size in pixels it was decoded at, and the paths. @@ -616,11 +619,11 @@ impl Traced { } } -/// Decode the image `bytes` and trace it with `params`, telling `steps` how far it has got -/// ([`tr::trace_watched`]); [`EngineError::Cancelled`] once `steps` answers `false`. -fn trace_now(inputs: &TraceInputs, params: &tr::TraceParams, bytes: &[u8], steps: &(dyn Fn(u64, u64) -> bool + Sync)) -> Result { +/// Decode the image bytes of `inputs` and trace them with `params`, telling `steps` how far it has +/// got ([`tr::trace_watched`]); [`EngineError::Cancelled`] once `steps` answers `false`. +fn trace_now(inputs: &TraceInputs, params: &tr::TraceParams, steps: &(dyn Fn(u64, u64) -> bool + Sync)) -> Result { // An image over the tracer's size is traced scaled down (the paths are scaled back). - let raster = tr::Raster::decode_to_fit(bytes).map_err(|e| EngineError::Other(e.to_string()))?; + let raster = tr::Raster::decode_to_fit(&inputs.bytes).map_err(|e| EngineError::Other(e.to_string()))?; if !steps(0, 0) { return Err(EngineError::Cancelled); } @@ -682,7 +685,8 @@ impl TraceRun { (self.prepared.inputs.image.width, self.prepared.inputs.image.height) } - /// Does it still apply: would the command trace the same object, image and settings now? + /// Does it still apply: would the command trace the same object, image, pixels and settings + /// now? pub fn current(&self, s: &Session) -> bool { prepare(s, self.command, &self.params).is_ok_and(|now| now.inputs == self.prepared.inputs) } @@ -691,13 +695,13 @@ impl TraceRun { /// failed or stopped ([`EngineError::Cancelled`] once `steps` answers `false`). It touches no /// document, so it can run on any thread. pub fn run(&self, steps: &(dyn Fn(u64, u64) -> bool + Sync)) -> Result { - trace_now(&self.prepared.inputs, &self.prepared.trace, &self.prepared.bytes, steps) + trace_now(&self.prepared.inputs, &self.prepared.trace, steps) } } pub(super) fn trace_make(s: &mut Session, p: &Value, expand: bool) -> Result { let c = if expand { "imageTrace.makeAndExpand" } else { "imageTrace.make" }; - let Prepared { inputs, trace, view, params_json, preset_label, library, bytes } = prepare(s, c, p)?; + let Prepared { inputs, trace, view, params_json, preset_label, library } = prepare(s, c, p)?; let group_by_color = expand && by_color_param(c, p)?; // A saved preset's settings join the journal entry, so a replay traces as this run did after // the preset is changed, renamed or deleted. @@ -709,7 +713,7 @@ pub(super) fn trace_make(s: &mut Session, p: &Value, expand: bool) -> Result traced, - _ => trace_now(&inputs, &trace, &bytes, &|_, _| true)?, + _ => trace_now(&inputs, &trace, &|_, _| true)?, }; let TraceInputs { target, image: img, .. } = inputs; let mut warnings = keep_warnings(&trace, &res); diff --git a/crates/engine/src/tests/build.rs b/crates/engine/src/tests/build.rs index 49a806b258..21cbf678eb 100644 --- a/crates/engine/src/tests/build.rs +++ b/crates/engine/src/tests/build.rs @@ -3788,6 +3788,103 @@ fn a_trace_run_needs_the_library_colours_it_traced_with() { assert!(!run.current(&s), "{name} changed"); } +/// `image_doc`'s image with its disc 20 px to the left: the same size, other pixels. +fn moved_disc_png() -> Vec { + vectorcraft_trace::Raster::from_fn(100, 100, |x, y| { + let (dx, dy) = (x as f64 + 0.5 - 30.0, y as f64 + 0.5 - 50.0); + if dx * dx + dy * dy <= 900.0 { [0, 0, 0, 255] } else { [255, 255, 255, 255] } + }) + .encode_png() +} + +/// Puts `bytes` under the image key `key`, or takes the key's image data away (`None`). +fn set_image_data(s: &mut Session, key: &str, bytes: Option>) { + s.edit("image data", |d, _| { + match bytes { + Some(b) => d.images.insert(key.into(), ImageBlob::new("image/png", b)), + None => d.images.remove(key), + }; + Ok(()) + }) + .unwrap(); +} + +/// The middle of the first traced path of the Image Trace object the reply `r` names, in x. +fn traced_center_x(s: &Session, r: &Value) -> f64 { + outline(&traced_paths(&node(s, NodeId(r["id"].as_u64().unwrap())))[0]).0.bounds().unwrap().center().x +} + +/// A trace made away from its command no longer applies once the pixels under its image's key +/// change, though the object, its image record and the settings stay as they were; both commands +/// then trace the pixels there now. The same bytes in another allocation still apply. +#[test] +fn a_trace_run_needs_the_pixels_it_traced() { + let p = json!({"params": {"ignoreWhite": true}}); + for command in ["imageTrace.make", "imageTrace.makeAndExpand"] { + let mut s = session(); + image_doc(&mut s); + let run = cmd::TraceRun::prepare(&s, command, &p).unwrap(); + let traced = run.run(&|_, _| true).unwrap(); + // The same bytes, allocated anew: still current. + let same = s.doc().unwrap().doc.images["img1"].bytes.to_vec(); + set_image_data(&mut s, "img1", Some(same)); + assert!(run.current(&s), "{command}: equal bytes"); + // Other pixels under the same key: no longer current, and the hand-off is dropped. + set_image_data(&mut s, "img1", Some(moved_disc_png())); + assert!(!run.current(&s), "{command}: other pixels"); + traced.hand_in(&mut s); + let r = s.execute(command, run.params()).unwrap(); + assert!(s.trace_hand_off.is_none(), "{command}: dropped"); + // The disc is now at image x 30, which the ×2 placement at 50 puts at 110. + let x = traced_center_x(&s, &r); + assert!((x - 110.0).abs() < 1.5, "{command}: traced the pixels there now, at {x}"); + } +} + +/// Image data that is missing or empty: a run made before it went no longer applies, the run's +/// command gives the error it gives without a run, and nothing panics. +#[test] +fn a_trace_run_of_missing_or_empty_image_data() { + let p = json!({"params": {"ignoreWhite": true}}); + let mut s = session(); + image_doc(&mut s); + let run = cmd::TraceRun::prepare(&s, "imageTrace.make", &p).unwrap(); + let traced = run.run(&|_, _| true).unwrap(); + set_image_data(&mut s, "img1", None); + assert!(!run.current(&s), "missing"); + traced.hand_in(&mut s); + let e = s.execute(run.command(), run.params()).unwrap_err().to_string(); + assert!(e.contains("image data `img1` is missing"), "{e}"); + set_image_data(&mut s, "img1", Some(vec![])); + assert!(!run.current(&s), "empty"); + let empty = cmd::TraceRun::prepare(&s, "imageTrace.make", &p).unwrap(); + assert!(empty.current(&s), "two empty blobs are equal"); + assert!(matches!(empty.run(&|_, _| true), Err(EngineError::Other(_)))); + assert!(matches!(s.execute(run.command(), run.params()), Err(EngineError::Other(_)))); +} + +/// Tracing an Image Trace object again follows the same rule: once the pixels of the image it +/// holds change, a run made before no longer applies, and the command traces the pixels there now. +#[test] +fn a_trace_run_of_an_image_trace_object_needs_its_pixels() { + let p = json!({"params": {"ignoreWhite": true}}); + let mut s = session(); + image_doc(&mut s); + let g = s.execute("imageTrace.make", &p).unwrap()["id"].as_u64().unwrap(); + let again = json!({"id": g, "params": {"ignoreWhite": true}}); + let run = cmd::TraceRun::prepare(&s, "imageTrace.make", &again).unwrap(); + assert_eq!(run.target(), NodeId(g)); + let traced = run.run(&|_, _| true).unwrap(); + assert!(run.current(&s)); + set_image_data(&mut s, "img1", Some(moved_disc_png())); + assert!(!run.current(&s)); + traced.hand_in(&mut s); + let r = s.execute(run.command(), run.params()).unwrap(); + assert!(s.trace_hand_off.is_none(), "dropped"); + let x = traced_center_x(&s, &r); + assert!((x - 110.0).abs() < 1.5, "traced the pixels there now, at {x}"); +} + /// 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]