Images: read no more than the limit, keep plot bounds to what a viewport shows, free textures with no images left

Review fixes from CodeRabbit on #2749:
- the image file is read through read_capped, so a file that grows after the size check can't exhaust memory
- an image cut by a layout viewport widens the sheet's bounds only by the part inside it, as other primitives do
- uploaded tiles of closed drawings are dropped also when the next list has no images

crates/engine/src/cmd/image.rs

@@ -79,7 +79,11 @@ fn read_file(path: &str, host: Option<&str>) -> std::result::Result<Vec<u8>, Str
         if len > MAX_IMAGE_BYTES {
             return Err(format!("the file is larger than {} MB", MAX_IMAGE_BYTES >> 20));
         }
-        std::fs::read(&file).map_err(|e| e.to_string())
+        // Read no more than the limit: the file can grow or change after the check above.
+        let limit = usize::try_from(MAX_IMAGE_BYTES).unwrap_or(usize::MAX);
+        crate::capped_read::read_capped(&file, limit).map_err(|e| {
+            if e.kind() == std::io::ErrorKind::InvalidData { format!("the file is larger than {} MB", MAX_IMAGE_BYTES >> 20) } else { e.to_string() }
+        })
     }
     #[cfg(target_arch = "wasm32")]
     {

crates/render/src/viewports.rs

@@ -255,20 +255,24 @@ fn append_clipped(dst: &mut DisplayList, src: &DisplayList, clip: &Clip) {
 /// Append image `img` (primitive `p`, corners `corners`) to `dst`, to be cut to `clip` when it
 /// is drawn: the picture is not cut here, the outline is added to those it carries.
 fn push_clipped_image(dst: &mut DisplayList, p: &DPrim, img: &DImage, corners: &[Vec2], clip: &Clip) {
-    let outline = match clip {
-        Clip::Rect(rect) => {
-            if !Bounds2::from_points(corners.iter().copied()).intersects(rect) {
-                return;
-            }
-            vec![rect.min, Vec2::new(rect.max.x, rect.min.y), rect.max, Vec2::new(rect.min.x, rect.max.y)]
-        }
-        Clip::Poly(c) => c.polygon().to_vec(),
+    let (outline, area) = match clip {
+        Clip::Rect(rect) => (vec![rect.min, Vec2::new(rect.max.x, rect.min.y), rect.max, Vec2::new(rect.min.x, rect.max.y)], *rect),
+        Clip::Poly(c) => (c.polygon().to_vec(), c.bounds()),
     };
+    let own = Bounds2::from_points(corners.iter().copied());
+    if !own.intersects(&area) {
+        return;
+    }
     let Ok(index) = u32::try_from(dst.images.len()) else { return };
     let mut img = img.clone();
     img.clips.push(outline);
     dst.images.push(img);
+    // The list's bounds take only the part inside the clip, as for the other primitives.
+    let before = dst.bounds;
     push_raw(dst, p, Kind::Image { index }, corners);
+    dst.bounds = before;
+    dst.bounds.add(own.min.max(area.min));
+    dst.bounds.add(own.max.min(area.max));
 }
 
 fn push_raw(dst: &mut DisplayList, p: &DPrim, kind: Kind, pts: &[Vec2]) {

crates/render/tests/raster_image.rs

@@ -269,6 +269,9 @@ fn a_layout_viewport_cuts_the_image_to_its_outline() {
     assert_eq!(img.clips[1], [Vec2::new(15.0, 10.0), Vec2::new(105.0, 10.0), Vec2::new(105.0, 110.0), Vec2::new(15.0, 110.0)]);
     let p = list.prims.iter().find(|p| matches!(p.kind, Kind::Image { index: 0 })).unwrap();
     assert_eq!(list.points(p).len(), 4);
+    // The image (10..110 across) is wider than the viewport (15..105): only what shows counts
+    // towards the sheet's bounds, which a plot fits to.
+    assert!(list.bounds.min.x >= 15.0 - 1e-9 && list.bounds.max.x <= 105.0 + 1e-9, "{:?}", list.bounds);
 
     // Drawn: inside the viewport and the image's top row there is the picture; outside the
     // viewport, or in the clipped-away bottom row, there is none.

crates/ui-egui/src/canvas/images.rs

@@ -158,6 +158,11 @@ fn axis_rect(outline: &[Vec2]) -> Option<Bounds2> {
 
 /// Draw the images of `list`.
 pub(super) fn draw(p: &egui::Painter, xf: &Xf, list: &DisplayList) {
+    // Pyramids whose pixels are gone (drawing closed, image undone) go, also when the list has
+    // no images left to draw.
+    if let Ok(mut pyramids) = textures(p.ctx()).0.lock() {
+        pyramids.retain(|_, pyramid| pyramid.pixels.strong_count() > 0);
+    }
     if list.images.is_empty() {
         return;
     }
@@ -172,6 +177,11 @@ pub(super) fn draw(p: &egui::Painter, xf: &Xf, list: &DisplayList) {
     }
 }
 
+/// The image tiles uploaded so far, kept between frames.
+fn textures(ctx: &egui::Context) -> Textures {
+    ctx.data_mut(|d| d.get_temp_mut_or_default::<Textures>(egui::Id::unique("cadcraft-raster-textures")).clone())
+}
+
 /// Draw one image as textured meshes, one per tile on screen: its rectangle, or the outline it is
 /// clipped to.
 fn draw_one(p: &egui::Painter, xf: &Xf, img: &DImage, alpha: u8) {
@@ -184,10 +194,8 @@ fn draw_one(p: &egui::Painter, xf: &Xf, img: &DImage, alpha: u8) {
     // Where in the raster the triangles are.
     let Some(tris) = triangles(img, cut.outline).iter().map(|q| img.pixel_of(*q)).collect::<Option<Vec<Vec2>>>() else { return };
     let color = Color32::from_white_alpha((img.opacity(alpha) * 255.0) as u8);
-    let textures = ctx.data_mut(|d| d.get_temp_mut_or_default::<Textures>(egui::Id::unique("cadcraft-raster-textures")).clone());
+    let textures = textures(ctx);
     let Ok(mut pyramids) = textures.0.lock() else { return };
-    // Pyramids whose pixels are gone (drawing closed, image undone) go too.
-    pyramids.retain(|_, pyramid| pyramid.pixels.strong_count() > 0);
     let key = Arc::as_ptr(raster) as usize;
     let pyramid = match pyramids.entry(key) {
         Entry::Occupied(e) => e.into_mut(),
@@ -484,15 +492,17 @@ mod tests {
         assert!(frame(&l) >= 1, "the first frame also uploads the font atlas");
         assert_eq!(frame(&l), 0, "already uploaded");
         // The pixels gone (drawing closed, image undone): the pyramid goes on the next draw.
-        let count = |ctx: &egui::Context| {
-            let t = ctx.data_mut(|d| d.get_temp_mut_or_default::<Textures>(egui::Id::unique("cadcraft-raster-textures")).clone());
-            t.0.lock().unwrap().len()
-        };
+        let count = |ctx: &egui::Context| textures(ctx).0.lock().unwrap().len();
         assert_eq!(count(&ctx), 1);
         drop(l);
         drop(r);
-        frame(&list(|_| {}));
+        let next = list(|_| {});
+        frame(&next);
         assert_eq!(count(&ctx), 1, "only the new image's");
+        // A list with no images still lets go of the pictures that are gone.
+        drop(next);
+        frame(&DisplayList::default());
+        assert_eq!(count(&ctx), 0);
     }
 
     /// A known limit, kept visible here: on screen images are painted first, so they lie under