Merge pull request #5535 from dotCooCoo/fix/placed-document-undecoded-marks

Render: placed-document bitmaps draw undecoded-image marks when the view does

crates/render/src/placed_document.rs

@@ -6,7 +6,10 @@
 //!
 //! On screen ([`RenderOptions::progressive_placed`]): a placed document can't be edited, so what
 //! it shows is cached as a bitmap per file and zoom step (a power of two, in device pixels per
-//! point), shared by every object showing that file. A bitmap not made yet is made on a
+//! point), shared by every object showing that file. Images inside it whose pixels can't be
+//! decoded are drawn as crossed boxes in the bitmap when the frame marks them
+//! ([`RenderOptions::mark_undecoded_images`]), and the bitmaps with and without marks are kept
+//! apart. A bitmap not made yet is made on a
 //! background worker while the frame shows another step's bitmap, or a light placeholder;
 //! [`generation`] changes when one is ready, [`made_between`] says for which files, and [`busy`]
 //! says whether any is coming, so the screen can draw again where those files show
@@ -45,8 +48,9 @@ const MAX_BYTES: usize = 4 * BUDGET;
 /// The smallest zoom step (device pixels per point).
 const MIN_STEP: f64 = 1.0 / 64.0;
 
-/// A bitmap's identity: the file shown (its key) and the zoom step's bits.
-type BitmapKey = (String, u64);
+/// A bitmap's identity: the file shown (its key), the zoom step's bits, and whether images whose
+/// pixels can't be decoded are marked ([`crate::RenderOptions::mark_undecoded_images`]).
+type BitmapKey = (String, u64, bool);
 
 struct Bitmap {
     pm: Arc<Pixmap>,
@@ -130,10 +134,19 @@ pub(crate) fn scheduled(key: &str) -> usize {
     lock(&SCHEDULED).get(key).copied().unwrap_or(0)
 }
 
-/// Mark the bitmap of file `key` at zoom `step` as not kept past [`MAX_BYTES`].
+/// Mark the bitmaps of file `key` at zoom `step`, with and without marks on undecoded images, as
+/// not kept past [`MAX_BYTES`].
 #[cfg(test)]
 pub(crate) fn mark_unkept(key: &str, step: f64) {
-    lock(&UNKEPT).insert((key.to_string(), step.to_bits()));
+    for mark in [false, true] {
+        lock(&UNKEPT).insert((key.to_string(), step.to_bits(), mark));
+    }
+}
+
+/// `object`'s file (`blob`) as a bitmap of `step` device pixels per point, made here and now.
+#[cfg(test)]
+pub(crate) fn bitmap(object: &PlacedDocument, blob: &ImageBlob, step: f64, mark_undecoded_images: bool) -> Option<Pixmap> {
+    make(&mut Renderer::new(), object, blob, step, mark_undecoded_images)
 }
 
 /// The zoom step for `scale` device pixels per point: the power of two at or above it.
@@ -150,11 +163,12 @@ impl Bitmaps {
         Some(b.pm.clone())
     }
 
-    /// The bitmap of another step of the same file, the nearest to `step`.
-    fn nearest(&self, key: &str, step: f64) -> Option<(Arc<Pixmap>, f64)> {
+    /// The bitmap of another step of the same file, with the same marks on undecoded images, the
+    /// nearest to `step`.
+    fn nearest(&self, key: &str, step: f64, mark: bool) -> Option<(Arc<Pixmap>, f64)> {
         self.map
             .iter()
-            .filter(|((k, _), _)| k == key)
+            .filter(|((k, _, m), _)| k == key && *m == mark)
             .min_by(|a, b| (a.1.step / step).log2().abs().total_cmp(&(b.1.step / step).log2().abs()))
             .map(|(_, b)| (b.pm.clone(), b.step))
     }
@@ -245,14 +259,30 @@ fn depth() -> usize {
 /// Render `art` of a placed document's file (`doc`, its resources) into `w`×`h` transparent
 /// pixels through `view`, the placed documents inside it one level deeper.
 pub fn render_inside(r: &mut Renderer, doc: &vectorcraft_doc::Document, art: &Arc<Node>, w: u16, h: u16, view: Affine) -> Option<crate::Rendered> {
+    render_inside_marked(r, doc, art, w, h, view, false)
+}
+
+/// [`render_inside`], drawing images whose pixels can't be decoded as crossed boxes when
+/// `mark_undecoded_images` is set ([`crate::RenderOptions::mark_undecoded_images`]).
+fn render_inside_marked(
+    r: &mut Renderer,
+    doc: &vectorcraft_doc::Document,
+    art: &Arc<Node>,
+    w: u16,
+    h: u16,
+    view: Affine,
+    mark_undecoded_images: bool,
+) -> Option<crate::Rendered> {
     let _inside = Inside::enter();
-    r.render_node(doc, art, w, h, view)
+    let opts = crate::RenderOptions { mark_undecoded_images, ..Default::default() };
+    r.render_node_with(doc, art, w, h, view, &opts)
 }
 
 /// `object`'s file (`blob`) as a bitmap of `step` device pixels per point of its box: from its
 /// preview when that is all there is and is fine enough, else from the file (read again when
-/// needed; the preview when it can't be).
-fn make(r: &mut Renderer, object: &PlacedDocument, blob: &ImageBlob, step: f64) -> Option<Pixmap> {
+/// needed; the preview when it can't be). Images whose pixels can't be decoded are drawn as
+/// crossed boxes when `mark_undecoded_images` is set.
+fn make(r: &mut Renderer, object: &PlacedDocument, blob: &ImageBlob, step: f64, mark_undecoded_images: bool) -> Option<Pixmap> {
     let (w, h) = (pixels(object.width * step), pixels(object.height * step));
     let preview = if blob.is_proxy() { Some(blob.bytes.clone()) } else { blob.proxy.clone() };
     let full = match blob.is_proxy() {
@@ -265,7 +295,7 @@ fn make(r: &mut Renderer, object: &PlacedDocument, blob: &ImageBlob, step: f64)
     let f = e.frame;
     let (sx, sy) = (w as f64 / f.width(), h as f64 / f.height());
     let view = Affine::scale_non_uniform(sx, sy) * Affine::translate((-f.x0, -f.y0));
-    let img = render_inside(r, &e.doc, &e.art, w, h, view)?;
+    let img = render_inside_marked(r, &e.doc, &e.art, w, h, view, mark_undecoded_images)?;
     let data = img.pixels.as_chunks::<4>().0.iter().map(|&[r, g, b, a]| vello_cpu::color::PremulRgba8 { r, g, b, a }).collect();
     Some(Pixmap::from_parts(data, w, h))
 }
@@ -281,7 +311,7 @@ fn pixels(v: f64) -> u16 {
 fn run(r: &mut Renderer, mut job: Job) {
     let scope = job.scope.take();
     let mut body = || {
-        match make(r, &job.object, &job.blob, job.step) {
+        match make(r, &job.object, &job.blob, job.step, job.key.2) {
             Some(pm) => {
                 if !lock(&BITMAPS).insert(job.key.clone(), pm, job.step, job.keep, (BUDGET, MAX_BYTES)) {
                     lock(&UNKEPT).insert(job.key.clone());
@@ -413,7 +443,7 @@ impl Renderer {
             return false;
         }
         let Some(blob) = f.doc.images.get(&p.key).cloned() else { return true };
-        let key = (p.key.clone(), step.to_bits());
+        let key = (p.key.clone(), step.to_bits(), f.opts.mark_undecoded_images);
         // Not kept past [`MAX_BYTES`]: drawn exactly.
         if lock(&UNKEPT).contains(&key) {
             return false;
@@ -423,7 +453,7 @@ impl Renderer {
             // The clock when this frame first drew a placed document's bitmap.
             let keep = *self.placed_frame.get_or_insert(b.clock);
             let found = b.get(&key);
-            let nearest = if found.is_none() { b.nearest(&p.key, step) } else { None };
+            let nearest = if found.is_none() { b.nearest(&p.key, step, f.opts.mark_undecoded_images) } else { None };
             (found, nearest, keep)
         };
         let shown = match found {
@@ -434,7 +464,7 @@ impl Renderer {
                 schedule(Job { key, object: p.clone(), blob, step, keep, scope: SCOPE_CAPTURE.get().map(|capture| capture()) });
                 // Made here and now (no threads) or meanwhile another step's.
                 let mut b = lock(&BITMAPS);
-                b.get(&(p.key.clone(), step.to_bits())).or(nearest.map(|n| n.0))
+                b.get(&(p.key.clone(), step.to_bits(), f.opts.mark_undecoded_images)).or(nearest.map(|n| n.0))
             }
         };
         let rect = Rect::new(0.0, 0.0, p.width, p.height);
@@ -480,7 +510,7 @@ mod tests {
         let mut b = Bitmaps::default();
         // 400 bytes each: a budget of 1,000 bytes and at most 2,000.
         let limits = (1000, 2000);
-        let key = |name: &str| (name.to_string(), 1f64.to_bits());
+        let key = |name: &str| (name.to_string(), 1f64.to_bits(), false);
         for name in ["a", "b", "c"] {
             assert!(b.insert(key(name), Pixmap::new(10, 10), 1.0, 0, limits));
         }
@@ -489,7 +519,7 @@ mod tests {
         let frame = b.clock;
         assert!(b.get(&key("a")).is_some());
         assert!(b.insert(key("d"), Pixmap::new(10, 10), 1.0, frame, limits));
-        let mut kept: Vec<&str> = b.map.keys().map(|(name, _)| name.as_str()).collect();
+        let mut kept: Vec<&str> = b.map.keys().map(|(name, _, _)| name.as_str()).collect();
         kept.sort_unstable();
         assert_eq!((kept, b.bytes), (vec!["a", "d"], 800), "b and c, not drawn since, are dropped");
         for name in ["e", "f", "g"] {
@@ -499,4 +529,17 @@ mod tests {
         assert!(!b.insert(key("h"), Pixmap::new(10, 10), 1.0, frame, limits), "past the most: not kept");
         assert!(!b.map.contains_key(&key("h")) && b.bytes == 2000);
     }
+
+    /// While a bitmap is made, another step's bitmap stands in only when it has the same marks on
+    /// undecoded images.
+    #[test]
+    fn a_stand_in_bitmap_has_the_same_marks() {
+        let mut b = Bitmaps::default();
+        let limits = (BUDGET, MAX_BYTES);
+        assert!(b.insert(("a".into(), 1f64.to_bits(), false), Pixmap::new(10, 10), 1.0, 0, limits));
+        assert!(b.insert(("a".into(), 4f64.to_bits(), true), Pixmap::new(40, 40), 4.0, 0, limits));
+        assert_eq!(b.nearest("a", 2.0, false).map(|n| n.1), Some(1.0));
+        assert_eq!(b.nearest("a", 2.0, true).map(|n| n.1), Some(4.0));
+        assert!(b.nearest("b", 2.0, false).is_none());
+    }
 }

crates/render/src/renderer.rs

@@ -307,15 +307,19 @@ impl Renderer {
 
     /// Render node `n` of `doc` (its resources) into `w`×`h` transparent pixels through `view`.
     pub fn render_node(&mut self, doc: &Document, n: &Arc<Node>, w: u16, h: u16, view: Affine) -> Option<Rendered> {
+        self.render_node_with(doc, n, w, h, view, &RenderOptions::default())
+    }
+
+    /// [`Self::render_node`] with `opts` (e.g. [`RenderOptions::mark_undecoded_images`]).
+    pub fn render_node_with(&mut self, doc: &Document, n: &Arc<Node>, w: u16, h: u16, view: Affine, opts: &RenderOptions) -> Option<Rendered> {
         self.check_plugins();
         self.check_symbols(doc);
         let inv = view.inverse();
         let visible = inv.transform_rect_bbox(Rect::new(0.0, 0.0, w as f64, h as f64));
         let px = 1.0 / view.determinant().abs().sqrt().max(1e-12);
         let mut ctx = single_threaded_context(w, h);
-        let opts = RenderOptions::default();
         let symbols = doc.symbol_boxes().quick();
-        let frame = Frame { mt: false, doc, symbols: &symbols, view, visible, px, opts: &opts, ink: Ink::Display, plates: None };
+        let frame = Frame { mt: false, doc, symbols: &symbols, view, visible, px, opts, ink: Ink::Display, plates: None };
         self.begin_frame();
         (self.knockout, self.nested, self.backdrop, self.picture) = (false, 0, None, None);
         self.clip_paths.clear();

crates/render/src/tests/placed_document.rs

@@ -8,7 +8,9 @@ use vectorcraft_geom::shapes;
 use crate::*;
 
 /// Every file reads as a 40×20 artboard filled blue; a file whose bytes start with `loop` also
-/// places itself (the same key) in its middle, and one whose bytes start with `bad` can't be read.
+/// places itself (the same key) in its middle, one whose bytes start with `cut` also shows an
+/// image whose pixels can't be decoded over its left half, and one whose bytes start with `bad`
+/// can't be read.
 fn blue_page(p: &PlacedDocument, bytes: &[u8]) -> Option<(Document, Node, Rect)> {
     if bytes.starts_with(b"bad") {
         return None;
@@ -17,6 +19,19 @@ fn blue_page(p: &PlacedDocument, bytes: &[u8]) -> Option<(Document, Node, Rect)>
     d.layers.clear();
     let blue = Appearance::basic(Paint::solid(Color::rgb(0.0, 0.0, 1.0)), Paint::None, 0.0);
     let mut children = vec![Arc::new(Node::path(NodeId(1), shapes::rectangle(Rect::new(0.0, 0.0, 40.0, 20.0)), blue))];
+    if bytes.starts_with(b"cut") {
+        d.images.insert("cut-image".into(), ImageBlob::new("image/png", b"not a PNG: placed document".to_vec()));
+        let im = vectorcraft_doc::ImageObject {
+            key: "cut-image".into(),
+            width: 20,
+            height: 20,
+            xf: Affine::IDENTITY,
+            link: None,
+            placement: Default::default(),
+            pixels: None,
+        };
+        children.push(Arc::new(Node::new(NodeId(3), NodeKind::Image(Box::new(im)))));
+    }
     if bytes.starts_with(b"loop") {
         d.images.insert(p.key.clone(), ImageBlob::new("application/json", bytes.to_vec()));
         let me = PlacedDocument { xf: Affine::translate((10.0, 5.0)) * Affine::scale(0.5), ..p.clone() };
@@ -254,6 +269,36 @@ fn a_bitmap_that_cant_be_made_is_not_asked_for_again() {
     assert!(shown != [255, 255, 255] && shown != [0, 0, 255], "the placeholder: {shown:?}");
 }
 
+/// A bitmap made for a frame that marks images whose pixels can't be decoded shows such an image
+/// in the file as a crossed box; one made for a frame that does not shows the art under it.
+#[test]
+fn a_bitmap_marks_undecoded_images_when_the_frame_does() {
+    let d = doc_with("cut-1", b"cut");
+    let Some(NodeKind::PlacedDocument(p)) = d.node(NodeId(50)).map(|n| &n.kind) else { panic!("a placed document") };
+    let Some(blob) = d.images.get("cut-1") else { panic!("the file's bytes") };
+    for mark in [false, true] {
+        // 40 × 20 pixels at one pixel a point; the image covers x 0 to 20.
+        let Some(pm) = placed_document::bitmap(p, blob, 1.0, mark) else { panic!("a bitmap") };
+        let pixel = |x: usize, y: usize| pm.data_as_u8_slice()[(y * 40 + x) * 4..][..4].to_vec();
+        assert_eq!(pixel(30, 10), [0, 0, 255, 255], "mark {mark}: the art beside the image");
+        assert_eq!(pixel(5, 10) != [0, 0, 255, 255], mark, "mark {mark}: the crossed box over the art: {:?}", pixel(5, 10));
+    }
+}
+
+/// Frames with and without marks on undecoded images ask for bitmaps of their own, so a bitmap
+/// made for one is not shown in the other. The file can't be read, so no bitmap is made after
+/// the test ends.
+#[test]
+fn bitmaps_with_and_without_undecoded_marks_are_asked_for_apart() {
+    let _serial = serial();
+    let d = doc_with("marks-1", b"bad");
+    for mark in [false, true] {
+        let opts = RenderOptions { progressive_placed: true, mark_undecoded_images: mark, ..Default::default() };
+        Renderer::new().render(&d, 100, 100, Affine::IDENTITY, &opts);
+    }
+    assert_eq!(placed_document::scheduled("marks-1"), 2, "one for each");
+}
+
 /// Once bitmaps are ready, drawing again where [`placed_area`] says gives what drawing the whole
 /// frame again gives. The area of a file covers every object showing it (a group's member and
 /// an object with a drop shadow too) and no object showing another file; art of a brush or a