Render: an export no longer warns about images another thread could not decode

A raster export compared the process-wide count of images that could
not be decoded before and after it drew, and warned that "an image
couldn't be decoded and is missing from the picture" when the count
rose. Any other thread that failed to decode an image in that time
raised the count too: a second export, the UI thread drawing the
canvas, or the canvas, thumbnail and placed-document workers drawing a
document with such an image. The export then warned about an image it
does not contain. In the engine's tests,
options_not_applied_yet_and_document_features_warn failed in 5 of 60
runs of the tests_raster and tests_pdf* tests.

The count is now kept per thread. An export reads it on the thread it
draws on. That thread decodes every image the export draws that is not
already in the shared image store, and an image that failed to decode
is never stored.

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::<Vec<_>>());
+                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<Option<bool>> = (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:

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<u64> = 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<image::DynamicImage> {
     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
 }

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).