diff --git a/crates/engine/src/cmd/fileio/tests_raster.rs b/crates/engine/src/cmd/fileio/tests_raster.rs index 53b35204c..110847ed5 100644 --- a/crates/engine/src/cmd/fileio/tests_raster.rs +++ b/crates/engine/src/cmd/fileio/tests_raster.rs @@ -217,6 +217,53 @@ fn an_image_opens_at_the_size_its_resolution_declares() { } } +#[test] +fn a_raster_export_says_nothing_of_images_another_thread_could_not_decode() { + // While another thread keeps exporting a document whose image can't be decoded (two exports at + // once, or the canvas drawing such a document), PNG exports of art with no image report + // nothing, and every export on the other thread still reports its own image. + use std::sync::atomic::{AtomicBool, Ordering}; + use vectorcraft_doc::{ImageBlob, ImageObject, Node, NodeKind}; + let mut broken = vectorcraft_doc::Document::new(40.0, 20.0); + broken.images.insert("broken".into(), ImageBlob::new("image/png", b"\x89PNG\r\n\x1a\n".to_vec())); + let im = ImageObject { + key: "broken".into(), + width: 2, + height: 1, + xf: vectorcraft_geom::Affine::scale(20.0), + link: None, + placement: Default::default(), + }; + let (id, layer) = (broken.alloc_id(), broken.layers[0].id); + broken.insert(Some(layer), 0, Node::new(id, NodeKind::Image(Box::new(im)))).unwrap(); + let mut s = session(400.0, 400.0, 1); + s.execute("shape.ellipse", &json!({"x": 20, "y": 20, "width": 360, "height": 360})).unwrap(); + let says_so = |w: &[Value]| w.iter().any(|w| w.as_str().is_some_and(|w| w.contains("couldn't be decoded"))); + let stop = AtomicBool::new(false); + let (tx, rx) = std::sync::mpsc::channel(); + // Nothing in the scope panics, so the other thread is always told to stop. + let (theirs, ours) = std::thread::scope(|scope| { + let other = scope.spawn(|| { + let mut warned = vec![]; + while !stop.load(Ordering::Relaxed) { + let w = encode_with_warnings(&broken, "png", &json!({})).map(|(_, w)| w.into_iter().map(Value::from).collect::>()); + warned.push(w.is_ok_and(|w| says_so(&w))); + let _ = tx.send(()); + } + warned + }); + let started = rx.recv_timeout(std::time::Duration::from_secs(60)).is_ok(); + let ours: Vec> = (0..5) + .map(|_| s.execute("document.export", &json!({"format": "png"})).ok().map(|r| says_so(r["warnings"].as_array().map_or(&[], |w| w)))) + .collect(); + stop.store(true, Ordering::Relaxed); + (other.join().ok(), started.then_some(ours)) + }); + let theirs = theirs.expect("the other thread's exports"); + assert!(!theirs.is_empty() && theirs.iter().all(|w| *w), "the other thread's exports: {theirs:?}"); + assert_eq!(ours.expect("the other thread started"), vec![Some(false); 5], "exports with no image"); +} + #[test] fn a_raster_export_says_when_an_image_could_not_be_decoded() { // A PNG whose header reads (so it places at its size) but whose pixel data is cut short: diff --git a/crates/render/src/paint.rs b/crates/render/src/paint.rs index d021a8f27..7a375462b 100644 --- a/crates/render/src/paint.rs +++ b/crates/render/src/paint.rs @@ -56,13 +56,17 @@ pub(crate) fn set_paint(ctx: &mut RenderContext, p: &Paint, bounds: Rect, f: &Fr /// 6614 x 59055 px, is 1.56 GB as RGBA), past the decoder's 512 MB default. The web keeps it. const MAX_DECODE_ALLOC: u64 = if cfg!(target_arch = "wasm32") { 512 << 20 } else { 2 << 30 }; -/// Images that couldn't be decoded (and so drew nothing), counted for exports to report. -static DECODE_FAILURES: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); +thread_local! { + /// Images that couldn't be decoded (and so drew nothing) on this thread, counted for exports to + /// report. + static DECODE_FAILURES: std::cell::Cell = const { std::cell::Cell::new(0) }; +} -/// How many images have failed to decode so far (process-wide): an export compares the count -/// before and after to say it left images out. +/// How many images have failed to decode on this thread so far: an export compares the count +/// before and after it draws to say it left images out. Images that other threads (another export, +/// the canvas, thumbnails) fail to decode meanwhile are not counted. pub fn decode_failures() -> u64 { - DECODE_FAILURES.load(std::sync::atomic::Ordering::Relaxed) + DECODE_FAILURES.with(std::cell::Cell::get) } /// `bytes` decoded within [`MAX_DECODE_ALLOC`] ([`decode_within_allowance`]); a failure is counted @@ -70,7 +74,7 @@ pub fn decode_failures() -> u64 { fn decode(bytes: &[u8]) -> Option { let img = decode_within_allowance(bytes).ok(); if img.is_none() { - DECODE_FAILURES.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + DECODE_FAILURES.with(|n| n.set(n.get() + 1)); } img } diff --git a/crates/render/src/tests.rs b/crates/render/src/tests.rs index 70b8a4559..57e0e5304 100644 --- a/crates/render/src/tests.rs +++ b/crates/render/src/tests.rs @@ -776,6 +776,23 @@ fn invalid_dash_pattern_strokes_solid() { } } +/// An image that can't be decoded is counted on the thread that tried to decode it: an export +/// compares [`decode_failures`] before and after it draws, so another thread's failure meanwhile is +/// not its own. +#[test] +fn decode_failures_counts_this_threads_images_only() { + let undecodable = || paint::decode_pixmap(b"\x89PNG\r\n\x1a\n").is_none(); + let before = decode_failures(); + let other = std::thread::spawn(move || { + let start = decode_failures(); + undecodable() && decode_failures() == start + 1 + }); + assert!(other.join().unwrap(), "counted on the other thread"); + assert_eq!(decode_failures(), before, "the other thread's failure"); + assert!(undecodable()); + assert_eq!(decode_failures(), before + 1, "this thread's failure"); +} + /// An image drawn larger than its pixels samples smoothly, or with `smooth_images` off each screen /// pixel takes its nearest image pixel (Pixel Preview with File Handling › Display Bitmaps as /// Anti-aliased Images off, #394).