Merge #1024 (carry)

The automatic sampling margin was three quarters of the selection's larger side, computed
as `ext * 3` in u32 (edit.fill) or i32 (edit.contentAwareFill) before the window cap, so a
loaded document with a far-flung selection overflowed and panicked. edit.contentAwareFill
also had no window cap at all and could try to allocate a canvas-sized window.

The margin is now one shared function computed in u64 and saturated to i32, and both
commands check the window against the same 400 M-pixel cap before reading or allocating.
Content-Aware Move uses the shared margin too.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

crates/engine/src/channel_cmds.rs

@@ -1315,7 +1315,15 @@ fn merge(s: &mut Session, p: &Value) -> Result<Value> {
     if srcs.len() < mode.color_channels() {
         return Err(bad(cmd, format!("{mode:?} needs {} grayscale documents, got {}", mode.color_channels(), srcs.len())));
     }
-    let first = s.documents()[srcs[0]].doc.clone();
+    // Each source closes by its *current* index, so a repeated index would shift the later
+    // removals onto a document that was never named (#934). Refuse it rather than guess.
+    let mut seen = std::collections::HashSet::new();
+    if let Some(dup) = srcs.iter().find(|i| !seen.insert(**i)) {
+        return Err(bad(cmd, format!("document {dup} is listed more than once")));
+    }
+    let Some(first) = srcs.first().and_then(|&i| s.documents().get(i)).map(|d| d.doc.clone()) else {
+        return Err(bad(cmd, "bad document index"));
+    };
     let mut planes = Vec::new();
     for &i in &srcs {
         let d = &s.documents()[i].doc;

crates/engine/src/channel_cmds/tests.rs

@@ -450,6 +450,29 @@ fn split_and_merge_channels() {
     assert!(s.execute("channel.merge", json!({"mode": "cmyk"})).is_err());
 }
 
+#[test]
+fn merge_rejects_a_repeated_source_and_keeps_unrelated_documents() {
+    // #934: `documents: [0, 0, 1]` passed validation, then the close loop ran `close(1)`,
+    // `close(0)`, `close(0)` and the last call removed the unrelated RGB document at index 2.
+    let mut s = session();
+    s.execute("channel.split", json!({})).unwrap();
+    assert_eq!(s.documents().len(), 3);
+    s.execute("file.new", json!({"width": 40, "height": 20})).unwrap();
+    s.execute("edit.fill", json!({"color": "#00ff00"})).unwrap();
+    let keep = s.documents()[3].doc.id;
+    let before: Vec<_> = s.documents().iter().map(|d| d.doc.id).collect();
+    let err = s.execute("channel.merge", json!({"mode": "rgb", "documents": [0, 0, 1]})).unwrap_err();
+    assert!(err.to_string().contains("more than once"), "{err}");
+    let after: Vec<_> = s.documents().iter().map(|d| d.doc.id).collect();
+    assert_eq!(after, before, "a rejected merge closes nothing");
+    assert!(s.documents().iter().any(|d| d.doc.id == keep));
+    assert!(s.set_active(3) && s.undo(), "the unrelated document keeps its history");
+    // Distinct indices still merge and close exactly the named sources.
+    s.execute("channel.merge", json!({"mode": "rgb", "documents": [0, 1, 2]})).unwrap();
+    assert_eq!(s.documents().len(), 2);
+    assert!(s.documents().iter().any(|d| d.doc.id == keep));
+}
+
 #[test]
 fn duplicate_to_other_document() {
     let mut s = session();

crates/engine/src/edit_menu_cmds.rs

@@ -341,12 +341,13 @@ fn content_aware_fill_as(s: &mut Session, p: &Value, cmd: &'static str, label: &
     if hb.is_empty() {
         return Err(bad(cmd, "the selection is outside the canvas"));
     }
-    let ext = hb.width().max(hb.height()) as i32;
+    // The canvas is at most i32 wide, so the extent fits; saturate anyway rather than wrap (#963).
+    let ext = i32::try_from(hb.width().max(hb.height())).unwrap_or(i32::MAX);
     let sampling = str_or(p, "sampling", "auto").to_string();
     let custom_mask: Option<Surface> = p.get("channel").and_then(|v| channel_mask(&doc, v)).cloned();
     let custom_rect = if p.get("area").is_some() { Some(rect_param(p, "area", cmd)?) } else { None };
     let window = match sampling.as_str() {
-        "auto" => hb.inflate((ext * 3 / 4).max(32)),
+        "auto" => hb.inflate(crate::fill_cmds::sampling_margin(hb)),
         "rectangular" => hb.inflate(int(p, "margin").map_or(ext.max(16), |m| m.clamp(0, 100_000) as i32)),
         "custom" => {
             let r = match (custom_rect, custom_mask.as_ref()) {
@@ -373,7 +374,8 @@ fn content_aware_fill_as(s: &mut Session, p: &Value, cmd: &'static str, label: &
     let surf = doc.layer(id).and_then(|l| l.surface()).ok_or(EngineError::NoLayer(id))?;
     let fmt = surf.format();
     let n = fmt.channels();
-    let (w, h) = (window.width() as usize, window.height() as usize);
+    // Refuse a window too large to read before allocating for it, whatever the sampling (#963).
+    let (w, h) = crate::fill_cmds::window_size(window)?;
     // A background job when started with `Session::start` (#210): reading the window and the
     // PatchMatch fill run on a worker against the document snapshot, cancellable per row band.
     crate::jobs::run(

crates/engine/src/edit_menu_cmds/tests.rs

@@ -116,6 +116,65 @@ fn content_aware_fill_huge_area_neither_panics_nor_wraps() {
     }
 }
 
+/// `blob_session` with the canvas declared as `size` and a sparse selection of `rects`, as a
+/// loaded `.pcraft` can hold (#963). Nothing here allocates more than a few tiles.
+fn huge_selection(size: (u32, u32), rects: &[Rect]) -> Session {
+    let mut s = blob_session(8);
+    let st = s.active_mut().unwrap();
+    let doc = std::sync::Arc::make_mut(&mut st.doc);
+    doc.size = photocraft_geom::Size::new(size.0, size.1);
+    let mut sel = Surface::new(PixelFormat::GRAY8);
+    for r in rects {
+        sel.fill_rect(*r, &[1.0]);
+    }
+    doc.selection = Some(sel);
+    s
+}
+
+/// A sampling window too large to read, or whose margin overflows, is the size error: no panic,
+/// no huge allocation, the document and history untouched (#963).
+#[test]
+fn content_aware_fill_huge_window_is_an_error() {
+    // Largest extent whose `ext * 3` still fits in i32.
+    const EDGE: i32 = i32::MAX / 3;
+    let max = i32::MAX;
+    let big = (2_000_000_000, 2_000_000_000);
+    let dot = Rect::new(0, 0, 1, 1);
+    let cases = [
+        ("auto, wide, just above", big, vec![dot, Rect::new(EDGE, 0, EDGE + 1, 1)], json!({})),
+        ("auto, tall, just above", big, vec![dot, Rect::new(0, EDGE, 1, EDGE + 1)], json!({})),
+        ("auto, wide, just below", big, vec![dot, Rect::new(EDGE - 1, 0, EDGE, 1)], json!({})),
+        (
+            "auto, extreme coordinates",
+            (max as u32, max as u32),
+            vec![Rect::new(i32::MIN, i32::MIN, i32::MIN + 1, i32::MIN + 1), dot, Rect::new(max - 2, max - 2, max - 1, max - 1)],
+            json!({}),
+        ),
+        (
+            "rectangular, small selection",
+            (300_000, 300_000),
+            vec![Rect::new(150_000, 150_000, 150_001, 150_001)],
+            json!({"sampling": "rectangular", "margin": 100_000}),
+        ),
+        ("rectangular, huge extent", big, vec![dot, Rect::new(EDGE + 1, 0, EDGE + 2, 1)], json!({"sampling": "rectangular"})),
+        ("custom, whole canvas", big, vec![dot], json!({"sampling": "custom", "area": [0, 0, 2_000_000_000, 2_000_000_000]})),
+    ];
+    for (name, size, rects, p) in cases {
+        let mut s = huge_selection(size, &rects);
+        let before = s.active().unwrap().doc.clone();
+        let past = s.active().unwrap().history.past_len();
+        let err = s.execute("edit.contentAwareFill", p).expect_err(name).to_string();
+        assert!(err.contains("too large for Content-Aware fill"), "{name}: {err}");
+        let st = s.active().unwrap();
+        assert!(std::sync::Arc::ptr_eq(&before, &st.doc), "{name}: the document changed");
+        assert_eq!(st.history.past_len(), past, "{name}: a history step was recorded");
+    }
+    // Control: an ordinary selection still fills.
+    let mut s = huge_selection((64, 48), &[Rect::new(26, 16, 38, 30)]);
+    let r = s.execute("edit.contentAwareFill", json!({"colorAdaptation": "none"})).unwrap();
+    assert!(r["filled"].as_u64().unwrap() > 100);
+}
+
 #[test]
 fn content_aware_fill_removes_object_at_all_depths() {
     for depth in [8, 16, 32] {

crates/engine/src/fill_cmds.rs

@@ -176,15 +176,10 @@ fn content_aware(surf: &Surface, sel: &Surface, canvas: Rect, color_adaptation:
     if hole_bounds.is_empty() {
         return Err(EngineError::Other("the selection is outside the canvas".into()));
     }
-    let ext = hole_bounds.width().max(hole_bounds.height());
-    let window = hole_bounds.inflate(i32::try_from(ext * 3 / 4).unwrap_or(i32::MAX).max(32)).intersect(&canvas);
+    let window = hole_bounds.inflate(sampling_margin(hole_bounds)).intersect(&canvas);
     let fmt = surf.format();
     let n = fmt.channels();
-    let (w, h) = (window.width() as usize, window.height() as usize);
-    // Same cap as other whole-image operations: refuse absurd windows instead of allocating.
-    if w.saturating_mul(h) > 400_000_000 {
-        return Err(EngineError::Other("the selection is too large for Content-Aware fill".into()));
-    }
+    let (w, h) = window_size(window)?;
     let img = surf.read_region(window);
     let mut hole = vec![false; w * h];
     for (i, hv) in hole.iter_mut().enumerate() {
@@ -209,6 +204,25 @@ fn content_aware(surf: &Surface, sel: &Surface, canvas: Rect, color_adaptation:
     Ok(Source::Pixels(window, px))
 }
 
+/// The automatic Content-Aware sampling margin around `hole`: three quarters of its larger side,
+/// at least 32 pixels. Worked out in u64 because a loaded document can declare a selection whose
+/// extent times three overflows u32 (#963); a margin beyond i32 saturates, as `Rect::inflate` does.
+pub(crate) fn sampling_margin(hole: Rect) -> i32 {
+    let ext = u64::from(hole.width().max(hole.height()));
+    i32::try_from(ext * 3 / 4).unwrap_or(i32::MAX).max(32)
+}
+
+/// The size of a Content-Aware sampling window, or an error for a window too large to read and
+/// fill (#963). Same cap as other whole-image operations: refuse absurd windows instead of
+/// allocating.
+pub(crate) fn window_size(window: Rect) -> Result<(usize, usize)> {
+    let (w, h) = (window.width(), window.height());
+    if u64::from(w) * u64::from(h) > 400_000_000 {
+        return Err(EngineError::Other("the selection is too large for Content-Aware fill".into()));
+    }
+    Ok((w as usize, h as usize))
+}
+
 /// How the source goes onto the target.
 #[derive(Clone, Copy)]
 struct Blend {
@@ -499,4 +513,67 @@ mod tests {
         // Out-of-range opacity is clamped.
         s.execute(CMD, json!({"contents": "white", "opacity": 1e300})).unwrap();
     }
+
+    /// A session whose canvas is `size` and whose selection is the given (sparse) rectangles, as
+    /// a loaded `.pcraft` can declare (#963): the canvas size is not capped on load and the
+    /// selection tiles can lie far apart. Nothing here allocates more than a few tiles.
+    fn huge_selection(size: (u32, u32), rects: &[Rect]) -> Session {
+        let mut s = session();
+        s.execute(CMD, json!({"contents": "color", "color": "#336699"})).unwrap();
+        let st = s.active_mut().unwrap();
+        let doc = std::sync::Arc::make_mut(&mut st.doc);
+        doc.size = photocraft_geom::Size::new(size.0, size.1);
+        let mut sel = Surface::new(photocraft_color::PixelFormat::GRAY8);
+        for r in rects {
+            sel.fill_rect(*r, &[1.0]);
+        }
+        doc.selection = Some(sel);
+        s
+    }
+
+    /// Content-Aware fill on a selection whose extent overflows the sampling-window arithmetic
+    /// (#963) returns the size error and leaves the document and history untouched.
+    #[test]
+    fn content_aware_huge_extent_is_an_error() {
+        // Largest extent whose `ext * 3` still fits in u32.
+        const EDGE: i32 = (u32::MAX / 3) as i32;
+        let max = i32::MAX;
+        let cases: [(&str, (u32, u32), Vec<Rect>); 7] = [
+            // The issue's repro: a second selection tile 5 600 000 tiles to the right.
+            ("issue repro", (2_000_000_000, 2_000_000_000), vec![Rect::new(0, 0, 8, 8), Rect::new(5_600_000 * TILE_SIZE, 0, 5_600_000 * TILE_SIZE + 8, 8)]),
+            ("wide, just above", (2_000_000_000, 2_000_000_000), vec![Rect::new(0, 0, 1, 1), Rect::new(EDGE, 0, EDGE + 1, 1)]),
+            ("tall, just above", (2_000_000_000, 2_000_000_000), vec![Rect::new(0, 0, 1, 1), Rect::new(0, EDGE, 1, EDGE + 1)]),
+            ("wide, just below", (2_000_000_000, 2_000_000_000), vec![Rect::new(0, 0, 1, 1), Rect::new(EDGE - 1, 0, EDGE, 1)]),
+            ("tall, just below", (2_000_000_000, 2_000_000_000), vec![Rect::new(0, 0, 1, 1), Rect::new(0, EDGE - 1, 1, EDGE)]),
+            // The largest canvas a Rect can hold, selected corner to corner, plus a piece off the
+            // canvas at the most negative coordinate.
+            (
+                "extreme coordinates",
+                (max as u32, max as u32),
+                vec![Rect::new(i32::MIN, i32::MIN, i32::MIN + 1, i32::MIN + 1), Rect::new(0, 0, 1, 1), Rect::new(max - 2, max - 2, max - 1, max - 1)],
+            ),
+            // A canvas wider than a Rect can hold is treated as empty, never as a panic.
+            ("canvas beyond i32", (u32::MAX, u32::MAX), vec![Rect::new(0, 0, 1, 1), Rect::new(max - 2, 0, max - 1, 1)]),
+        ];
+        for (name, size, rects) in cases {
+            let mut s = huge_selection(size, &rects);
+            let before = s.active().unwrap().doc.clone();
+            let past = s.active().unwrap().history.past_len();
+            let r = s.execute(CMD, json!({"contents": "contentAware", "colorAdaptation": false}));
+            let err = r.expect_err(name).to_string();
+            assert!(!err.contains("internal error"), "{name}: {err}");
+            if name != "canvas beyond i32" {
+                assert!(err.contains("too large for Content-Aware fill"), "{name}: {err}");
+            }
+            let st = s.active().unwrap();
+            assert!(std::sync::Arc::ptr_eq(&before, &st.doc), "{name}: the document changed");
+            assert_eq!(st.history.past_len(), past, "{name}: a history step was recorded");
+            assert!(close(px(&s, 5, 5), [0x33 as f32 / 255.0, 0x66 as f32 / 255.0, 0x99 as f32 / 255.0, 1.0]), "{name}");
+        }
+        // Control: an ordinary selection still fills from its surroundings.
+        let mut s = huge_selection((40, 30), &[Rect::new(15, 10, 21, 16)]);
+        s.execute(CMD, json!({"contents": "white"})).unwrap();
+        s.execute(CMD, json!({"contents": "contentAware", "colorAdaptation": false})).unwrap();
+        assert!(close(px(&s, 17, 12), [0x33 as f32 / 255.0, 0x66 as f32 / 255.0, 0x99 as f32 / 255.0, 1.0]), "{:?}", px(&s, 17, 12));
+    }
 }

crates/engine/src/lens_cmds.rs

@@ -12,7 +12,7 @@ use photocraft_algo::lens::{self, EdgeMode, LensCorrection};
 use photocraft_algo::transform::Interp;
 use photocraft_algo::wideangle::{self, WideAngle};
 use photocraft_color::{ColorMode, SampleType};
-use photocraft_doc::{Document, LayerContent, LayerId, SmartFilter};
+use photocraft_doc::{Document, Layer, LayerContent, LayerId, SmartFilter};
 use photocraft_geom::Rect;
 use photocraft_raster::{Surface, from_rgba_into};
 use serde_json::{Value, json};
@@ -152,21 +152,36 @@ pub fn raw_params(cmd: &str, p: &Value) -> Result<CameraRaw> {
 
 // ---------- pixels ----------
 
-fn rgba_region(surf: &Surface, area: Rect) -> Vec<[f32; 4]> {
-    let mut v = vec![[0.0f32; 4]; (area.width() * area.height()) as usize];
+/// Largest area Camera Raw develops in one pass. The pipeline holds the whole region as RGBA
+/// f32 plus the source and result planes (≥ 48 bytes per pixel), so beyond this a document
+/// is refused with an error rather than attempted (#962).
+pub const MAX_RAW_PIXELS: u64 = 400_000_000;
+
+/// The number of pixels in `area` if Camera Raw can develop it in one pass.
+fn raw_pixels(area: Rect) -> Option<usize> {
+    let n = u64::from(area.width()) * u64::from(area.height());
+    (n <= MAX_RAW_PIXELS).then(|| usize::try_from(n).ok()).flatten()
+}
+
+fn rgba_region(surf: &Surface, area: Rect, len: usize) -> Vec<[f32; 4]> {
+    let mut v = vec![[0.0f32; 4]; len];
     surf.read_rgba_into(area, &mut v);
     v
 }
 
 /// Camera Raw on a surface over `area` (RGB / Gray via RGBA, at the surface's depth).
+///
+/// An area above [`MAX_RAW_PIXELS`] is returned unchanged: the command rejects it with an
+/// error first, and this keeps the preview and Smart Filter paths from overflowing.
 pub fn camera_raw_surface(surf: &Surface, area: Rect, p: &CameraRaw) -> Surface {
     let area = area.intersect(&surf.content_bounds().union(&area));
     if area.is_empty() || p.is_identity() {
         return surf.clone();
     }
+    let Some(len) = raw_pixels(area) else { return surf.clone() };
     let fmt = surf.format();
     let (w, h) = (area.width() as usize, area.height() as usize);
-    let mut px = rgba_region(surf, area);
+    let mut px = rgba_region(surf, area, len);
     camera_raw::develop(&mut px, w, h, p, fmt.sample == SampleType::F32);
     let n = fmt.channels();
     let orig = surf.read_region(area);
@@ -357,6 +372,19 @@ fn camera_raw_cmd(s: &mut Session, p: &Value) -> Result<Value> {
     let cr = raw_params(RAW, p)?;
     // New settings are strict; stored Smart Filters re-apply through the lenient `raw_params`.
     cr.validate().map_err(|e| bad(RAW, e))?;
+    // The pixel path works on the canvas plus the layer's content in one buffer; a legal
+    // 65536×65536 canvas is 2^32 pixels, which overflowed the u32 size arithmetic (#962).
+    if !cr.is_identity() {
+        let d = s.active().ok_or(EngineError::NoDocument)?;
+        let id = target(s, p)?;
+        let area = d.doc.layer(id).and_then(Layer::surface).map_or(d.doc.bounds(), |sf| d.doc.bounds().union(&sf.content_bounds()));
+        if raw_pixels(area).is_none() {
+            return Err(bad(
+                RAW,
+                format!("the image is too large for Camera Raw ({}×{} px; max {} megapixels)", area.width(), area.height(), MAX_RAW_PIXELS / 1_000_000),
+            ));
+        }
+    }
     let t0 = Stopwatch::start();
     let id = run_filter(s, RAW, "Camera Raw Filter", p.clone(), false, &|surf, canvas| camera_raw_surface(surf, canvas.union(&surf.content_bounds()), &cr))?;
     Ok(json!({"layer": id.0, "identity": cr.is_identity(), "ms": t0.ms()}))
@@ -554,6 +582,27 @@ mod tests {
         assert!((p[0] - p[2]).abs() < 0.01, "{p:?}");
     }
 
+    #[test]
+    fn camera_raw_rejects_an_area_too_large_to_develop() {
+        // #962: `area.width() * area.height()` overflowed u32 on a legal 65536×65536 canvas,
+        // so `run` panicked and dispatch reported an internal error.
+        let mut s = Session::new();
+        s.execute("file.new", json!({"width": 8, "height": 8, "background": "transparent"})).unwrap();
+        s.execute("image.canvasSize", json!({"width": 65536, "height": 65536, "extensionColor": "transparent"})).unwrap();
+        let err = s.execute(RAW, json!({"exposure": 1})).unwrap_err();
+        assert!(matches!(err, EngineError::BadParams { .. }), "{err}");
+        assert!(err.to_string().contains("too large"), "{err}");
+        // The surface function itself never overflows either: an oversized area is left alone.
+        let d = s.active().unwrap();
+        let surf = d.doc.layer(d.active_layer.unwrap()).unwrap().surface().unwrap();
+        let cr = raw_params(RAW, &json!({"exposure": 1})).unwrap();
+        let out = camera_raw_surface(surf, Rect::from_xywh(0, 0, 65536, 65536), &cr);
+        assert_eq!(out.content_bounds(), surf.content_bounds());
+        // A normal document still develops.
+        let mut s = session(8);
+        s.execute(RAW, json!({"exposure": 1})).unwrap();
+    }
+
     #[cfg(not(target_arch = "wasm32"))]
     #[test]
     fn lens_correction_batch_over_files() {

crates/engine/src/retouch_cmds/content_aware_move.rs

@@ -150,8 +150,7 @@ fn erode(w: usize, h: usize, m: &[bool], r: usize) -> Vec<bool> {
 /// Where a fill samples from around `r`: as Edit › Content-Aware Fill's automatic sampling area,
 /// three quarters of the extent around it (at least 32, at most [`MAX_MARGIN`] pixels).
 fn window(r: Rect, canvas: Rect) -> Rect {
-    let ext = r.width().max(r.height()) as i32;
-    r.inflate((ext * 3 / 4).clamp(32, MAX_MARGIN)).intersect(&canvas)
+    r.inflate(crate::fill_cmds::sampling_margin(r).min(MAX_MARGIN)).intersect(&canvas)
 }
 
 /// The pixels a step works on: the targeted surface, or with Sample All Layers the visible

crates/format/src/finite.rs

@@ -0,0 +1,335 @@
+//! Save-time check that the manifest holds only finite floats (issue #1101).
+//!
+//! JSON cannot spell NaN or the infinities: `serde_json` writes them as `null`, which the
+//! manifest's plain `f32`/`f64` fields refuse on load. Such a document would save (and autosave)
+//! with a success report and then never open again. So the save is refused instead, naming the
+//! value, by walking the manifest with a serializer that produces nothing and fails on the first
+//! non-finite float.
+
+use std::fmt;
+
+use serde::ser::{self, Serialize};
+
+/// Path of the first non-finite float, e.g. `document.layers[2].content.adjustment.exposure`
+/// (`Other` is reserved for a `Serialize` impl failing on its own, which the derived manifest
+/// never does).
+#[derive(Debug)]
+pub enum Problem {
+    NonFinite { path: String },
+    Other(String),
+}
+
+impl fmt::Display for Problem {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Problem::NonFinite { path } => write!(f, "`{path}` is NaN or infinite"),
+            Problem::Other(m) => f.write_str(m),
+        }
+    }
+}
+
+impl std::error::Error for Problem {}
+
+impl ser::Error for Problem {
+    fn custom<T: fmt::Display>(msg: T) -> Self {
+        Problem::Other(msg.to_string())
+    }
+}
+
+/// `Ok(())` when every float in `value`'s serialized form is finite; otherwise the path of the
+/// first one that is not.
+pub fn check<T: Serialize + ?Sized>(value: &T) -> Result<(), Problem> {
+    value.serialize(Check { path: String::new() })
+}
+
+struct Check {
+    path: String,
+}
+
+impl Check {
+    fn at(&self, seg: fmt::Arguments<'_>) -> Check {
+        Check { path: format!("{}{}", self.path, seg) }
+    }
+    fn float(self, finite: bool) -> Result<(), Problem> {
+        if finite { Ok(()) } else { Err(Problem::NonFinite { path: self.path.trim_start_matches('.').to_owned() }) }
+    }
+}
+
+type R = Result<(), Problem>;
+
+impl ser::Serializer for Check {
+    type Ok = ();
+    type Error = Problem;
+    type SerializeSeq = Seq;
+    type SerializeTuple = Seq;
+    type SerializeTupleStruct = Seq;
+    type SerializeTupleVariant = Seq;
+    type SerializeMap = Map;
+    type SerializeStruct = Struct;
+    type SerializeStructVariant = Struct;
+
+    fn serialize_bool(self, _: bool) -> R {
+        Ok(())
+    }
+    fn serialize_i8(self, _: i8) -> R {
+        Ok(())
+    }
+    fn serialize_i16(self, _: i16) -> R {
+        Ok(())
+    }
+    fn serialize_i32(self, _: i32) -> R {
+        Ok(())
+    }
+    fn serialize_i64(self, _: i64) -> R {
+        Ok(())
+    }
+    fn serialize_i128(self, _: i128) -> R {
+        Ok(())
+    }
+    fn serialize_u8(self, _: u8) -> R {
+        Ok(())
+    }
+    fn serialize_u16(self, _: u16) -> R {
+        Ok(())
+    }
+    fn serialize_u32(self, _: u32) -> R {
+        Ok(())
+    }
+    fn serialize_u64(self, _: u64) -> R {
+        Ok(())
+    }
+    fn serialize_u128(self, _: u128) -> R {
+        Ok(())
+    }
+    fn serialize_f32(self, v: f32) -> R {
+        self.float(v.is_finite())
+    }
+    fn serialize_f64(self, v: f64) -> R {
+        self.float(v.is_finite())
+    }
+    fn serialize_char(self, _: char) -> R {
+        Ok(())
+    }
+    fn serialize_str(self, _: &str) -> R {
+        Ok(())
+    }
+    fn serialize_bytes(self, _: &[u8]) -> R {
+        Ok(())
+    }
+    fn serialize_none(self) -> R {
+        Ok(())
+    }
+    fn serialize_some<T: Serialize + ?Sized>(self, v: &T) -> R {
+        v.serialize(self)
+    }
+    fn serialize_unit(self) -> R {
+        Ok(())
+    }
+    fn serialize_unit_struct(self, _: &'static str) -> R {
+        Ok(())
+    }
+    fn serialize_unit_variant(self, _: &'static str, _: u32, _: &'static str) -> R {
+        Ok(())
+    }
+    fn serialize_newtype_struct<T: Serialize + ?Sized>(self, _: &'static str, v: &T) -> R {
+        v.serialize(self)
+    }
+    fn serialize_newtype_variant<T: Serialize + ?Sized>(self, _: &'static str, _: u32, variant: &'static str, v: &T) -> R {
+        v.serialize(self.at(format_args!(".{variant}")))
+    }
+    fn serialize_seq(self, _: Option<usize>) -> Result<Seq, Problem> {
+        Ok(Seq { path: self.path, i: 0 })
+    }
+    fn serialize_tuple(self, _: usize) -> Result<Seq, Problem> {
+        Ok(Seq { path: self.path, i: 0 })
+    }
+    fn serialize_tuple_struct(self, _: &'static str, _: usize) -> Result<Seq, Problem> {
+        Ok(Seq { path: self.path, i: 0 })
+    }
+    fn serialize_tuple_variant(self, _: &'static str, _: u32, variant: &'static str, _: usize) -> Result<Seq, Problem> {
+        Ok(Seq { path: format!("{}.{variant}", self.path), i: 0 })
+    }
+    fn serialize_map(self, _: Option<usize>) -> Result<Map, Problem> {
+        Ok(Map { path: self.path, key: String::new() })
+    }
+    fn serialize_struct(self, _: &'static str, _: usize) -> Result<Struct, Problem> {
+        Ok(Struct { path: self.path })
+    }
+    fn serialize_struct_variant(self, _: &'static str, _: u32, variant: &'static str, _: usize) -> Result<Struct, Problem> {
+        Ok(Struct { path: format!("{}.{variant}", self.path) })
+    }
+}
+
+struct Seq {
+    path: String,
+    i: usize,
+}
+
+impl Seq {
+    fn next<T: Serialize + ?Sized>(&mut self, v: &T) -> R {
+        let c = Check { path: format!("{}[{}]", self.path, self.i) };
+        self.i += 1;
+        v.serialize(c)
+    }
+}
+
+impl ser::SerializeSeq for Seq {
+    type Ok = ();
+    type Error = Problem;
+    fn serialize_element<T: Serialize + ?Sized>(&mut self, v: &T) -> R {
+        self.next(v)
+    }
+    fn end(self) -> R {
+        Ok(())
+    }
+}
+
+impl ser::SerializeTuple for Seq {
+    type Ok = ();
+    type Error = Problem;
+    fn serialize_element<T: Serialize + ?Sized>(&mut self, v: &T) -> R {
+        self.next(v)
+    }
+    fn end(self) -> R {
+        Ok(())
+    }
+}
+
+impl ser::SerializeTupleStruct for Seq {
+    type Ok = ();
+    type Error = Problem;
+    fn serialize_field<T: Serialize + ?Sized>(&mut self, v: &T) -> R {
+        self.next(v)
+    }
+    fn end(self) -> R {
+        Ok(())
+    }
+}
+
+impl ser::SerializeTupleVariant for Seq {
+    type Ok = ();
+    type Error = Problem;
+    fn serialize_field<T: Serialize + ?Sized>(&mut self, v: &T) -> R {
+        self.next(v)
+    }
+    fn end(self) -> R {
+        Ok(())
+    }
+}
+
+struct Map {
+    path: String,
+    key: String,
+}
+
+impl ser::SerializeMap for Map {
+    type Ok = ();
+    type Error = Problem;
+    fn serialize_key<T: Serialize + ?Sized>(&mut self, key: &T) -> R {
+        // Keys are checked like any value; their text (when they have one) names the entry.
+        key.serialize(Check { path: format!("{}.<key>", self.path) })?;
+        self.key = serde_json::to_string(key).map_or_else(|_| "?".to_owned(), |k| k.trim_matches('"').to_owned());
+        Ok(())
+    }
+    fn serialize_value<T: Serialize + ?Sized>(&mut self, v: &T) -> R {
+        v.serialize(Check { path: format!("{}.{}", self.path, self.key) })
+    }
+    fn end(self) -> R {
+        Ok(())
+    }
+}
+
+struct Struct {
+    path: String,
+}
+
+impl Struct {
+    fn field<T: Serialize + ?Sized>(&mut self, key: &'static str, v: &T) -> R {
+        v.serialize(Check { path: format!("{}.{key}", self.path) })
+    }
+}
+
+impl ser::SerializeStruct for Struct {
+    type Ok = ();
+    type Error = Problem;
+    fn serialize_field<T: Serialize + ?Sized>(&mut self, key: &'static str, v: &T) -> R {
+        self.field(key, v)
+    }
+    fn end(self) -> R {
+        Ok(())
+    }
+}
+
+impl ser::SerializeStructVariant for Struct {
+    type Ok = ();
+    type Error = Problem;
+    fn serialize_field<T: Serialize + ?Sized>(&mut self, key: &'static str, v: &T) -> R {
+        self.field(key, v)
+    }
+    fn end(self) -> R {
+        Ok(())
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use serde::Serialize;
+
+    #[derive(Serialize)]
+    struct Inner {
+        gamma: f32,
+        offsets: Vec<f64>,
+    }
+    #[derive(Serialize)]
+    enum Content {
+        Exposure { exposure: f32 },
+        Levels(f64, f64),
+    }
+    #[derive(Serialize)]
+    struct Outer {
+        dpi: f32,
+        inner: Option<Inner>,
+        layers: Vec<Content>,
+        named: std::collections::BTreeMap<String, f32>,
+    }
+
+    fn outer() -> Outer {
+        Outer {
+            dpi: 72.0,
+            inner: Some(Inner { gamma: 1.0, offsets: vec![0.0, 1.5] }),
+            layers: vec![Content::Levels(0.0, 1.0), Content::Exposure { exposure: 0.5 }],
+            named: [("blur".to_owned(), 2.0)].into_iter().collect(),
+        }
+    }
+
+    fn path(v: &impl Serialize) -> String {
+        match check(v) {
+            Err(Problem::NonFinite { path }) => path,
+            other => panic!("expected a non-finite report, got {other:?}"),
+        }
+    }
+
+    #[test]
+    fn finite_values_pass_and_each_position_is_named() {
+        assert!(check(&outer()).is_ok());
+        let mut o = outer();
+        o.dpi = f32::NAN;
+        assert_eq!(path(&o), "dpi");
+        let mut o = outer();
+        if let Some(i) = o.inner.as_mut() {
+            i.offsets[1] = f64::INFINITY;
+        }
+        assert_eq!(path(&o), "inner.offsets[1]");
+        let mut o = outer();
+        o.layers[1] = Content::Exposure { exposure: f32::NEG_INFINITY };
+        assert_eq!(path(&o), "layers[1].Exposure.exposure");
+        let mut o = outer();
+        o.layers[0] = Content::Levels(0.0, f64::NAN);
+        assert_eq!(path(&o), "layers[0].Levels[1]");
+        let mut o = outer();
+        o.named.insert("blur".to_owned(), f32::NAN);
+        assert_eq!(path(&o), "named.blur");
+        assert_eq!(path(&f32::NAN), "");
+    }
+}

crates/format/src/lib.rs

@@ -25,6 +25,7 @@
 pub mod atomic;
 pub mod autosave;
 mod convert;
+mod finite;
 pub mod manifest;
 mod migrate;
 pub mod read;
@@ -58,6 +59,8 @@ pub enum FormatError {
     TooNew { found: u32, supported: u32 },
     #[error("limit exceeded: {0}")]
     LimitExceeded(String),
+    #[error("`{path}` is NaN or infinite, which a .pcraft manifest cannot hold; reset that value and save again")]
+    NonFinite { path: String },
     #[error("I/O: {0}")]
     Io(#[from] std::io::Error),
     #[error("manifest JSON: {0}")]

crates/format/src/store.rs

@@ -519,6 +519,12 @@ impl PcraftWriter {
             thumbnail: opts.thumbnail.as_ref().map(|_| THUMB.to_owned()),
             composite: opts.composite.as_ref().map(|_| COMPOSITE.to_owned()),
         };
+        // serde_json writes NaN and the infinities as `null`, which the manifest's float fields
+        // refuse on load (issue #1101): refuse the save instead, naming the value.
+        crate::finite::check(&manifest).map_err(|e| match e {
+            crate::finite::Problem::NonFinite { path } => FormatError::NonFinite { path },
+            crate::finite::Problem::Other(m) => FormatError::Unsupported(format!("manifest: {m}")),
+        })?;
         let manifest = serde_json::to_vec_pretty(&manifest)?;
         check_manifest_depth(&manifest)?;
         let stats = SaveStats { tiles_total: c.tiles.len(), blobs_total: c.blobs.len(), manifest_bytes: manifest.len(), ..Default::default() };

crates/format/tests/roundtrip.rs

@@ -425,3 +425,47 @@ fn video_layer_frames_survive_roundtrip() {
     v.frames[2].read_rgba_into(Rect::from_xywh(1, 1, 1, 1), &mut px);
     assert!(px[0][0] > 0.9 && px[0][2] < 0.1, "frame 2 is reddish: {:?}", px[0]);
 }
+
+/// Issue #1101: serde_json writes NaN and the infinities as `null`, which the manifest's float
+/// fields refuse on load, so such a document saved "successfully" and never opened again. The
+/// save is refused instead, naming the value; finite documents are unaffected.
+#[test]
+fn a_non_finite_float_is_refused_at_save_rather_than_breaking_every_later_load() {
+    use photocraft_doc::Adjustment;
+    type Mutate = Box<dyn Fn(&mut Document)>;
+    let base = rich_doc(ColorMode::Rgb, SampleType::U8);
+    let cases: Vec<(&str, Mutate)> = vec![
+        (
+            "exposure",
+            Box::new(|d| {
+                d.insert_above(None, Layer::new("adj", LayerContent::Adjustment(Adjustment::Exposure { exposure: f32::NAN, offset: 0.0, gamma: 1.0 })));
+            }),
+        ),
+        (
+            "gamma",
+            Box::new(|d| {
+                d.insert_above(
+                    None,
+                    Layer::new("adj", LayerContent::Adjustment(Adjustment::Exposure { exposure: 0.0, offset: 0.0, gamma: f32::NEG_INFINITY })),
+                );
+            }),
+        ),
+        ("resolution_dpi", Box::new(|d| d.resolution_dpi = f32::INFINITY)),
+    ];
+    for (field, mutate) in cases {
+        let mut doc = base.clone();
+        mutate(&mut doc);
+        let err = save_to_bytes(&doc, &SaveOptions::default()).expect_err(field);
+        assert!(matches!(err, FormatError::NonFinite { .. }), "{field}: {err:?}");
+        let msg = err.to_string();
+        assert!(msg.contains(field) && msg.contains("NaN or infinite"), "{field}: {msg}");
+        // The incremental writer shares the check, for autosave and directory bundles.
+        let mut w = PcraftWriter::new();
+        assert!(matches!(w.save_zip(&doc, &SaveOptions::default()), Err(FormatError::NonFinite { .. })), "{field}: PcraftWriter");
+    }
+    // The same document with finite values round-trips as before.
+    let mut doc = base.clone();
+    doc.insert_above(None, Layer::new("adj", LayerContent::Adjustment(Adjustment::Exposure { exposure: 0.5, offset: 0.0, gamma: 1.0 })));
+    let back = load_from_bytes(&save_to_bytes(&doc, &SaveOptions::default()).unwrap()).unwrap();
+    assert_eq!(back, doc);
+}

crates/io/src/annotations_map.rs

@@ -179,8 +179,11 @@ fn write_note(n: &Note) -> Vec<u8> {
     b.push(u8::from(n.open));
     b.push(0x1C);
     b.extend_from_slice(&1u16.to_be_bytes());
+    // The position comes from the file or a command and can sit anywhere in i32 (issue #939):
+    // the cast saturates, and the icon's far edge must saturate too rather than wrap (release) or
+    // overflow (debug) when the note is re-saved.
     let (x, y) = (n.position[0].round() as i32, n.position[1].round() as i32);
-    for v in [y, x, y + 20, x + 16] {
+    for v in [y, x, y.saturating_add(20), x.saturating_add(16)] {
         b.extend_from_slice(&v.to_be_bytes());
     }
     let pp = if n.popup == [0.0; 4] { [n.position[0] + 20.0, n.position[1] + 20.0, n.position[0] + 260.0, n.position[1] + 160.0] } else { n.popup };

crates/io/tests/annotations.rs

@@ -63,3 +63,87 @@ fn measurement_scale_roundtrips_through_psd() {
         assert_eq!(again.resources.iter().filter(|r| r.id == MEASUREMENT_SCALE).count(), 1);
     }
 }
+
+/// A PSD `Anno` block with one text note whose icon rectangle is `[top, left, 0, 0]` and whose
+/// popup is all zeros (issue #939's fixture).
+fn anno_with_icon(top: i32, left: i32) -> Vec<u8> {
+    let mut entry = Vec::new();
+    entry.extend_from_slice(b"txtA");
+    entry.push(1); // open
+    entry.push(0x1C); // flags
+    entry.extend_from_slice(&1u16.to_be_bytes()); // optional blocks
+    for v in [top, left, 0, 0] {
+        entry.extend_from_slice(&v.to_be_bytes()); // icon: top, left, bottom, right
+    }
+    for v in [0i32; 4] {
+        entry.extend_from_slice(&v.to_be_bytes()); // popup
+    }
+    entry.extend_from_slice(&0u16.to_be_bytes()); // colour space: RGB
+    for _ in 0..4 {
+        entry.extend_from_slice(&0u16.to_be_bytes());
+    }
+    entry.extend_from_slice(&[0u8, 0]); // author: empty Pascal string
+    entry.extend_from_slice(&[0u8, 0]); // name
+    entry.extend_from_slice(&[0u8, 0]); // modification date
+    let text = [0xFEu8, 0xFF, 0x00, b'A']; // UTF-16BE "A"
+    entry.extend_from_slice(&((text.len() + 12) as u32).to_be_bytes());
+    entry.extend_from_slice(b"txtC");
+    entry.extend_from_slice(&(text.len() as u32).to_be_bytes());
+    entry.extend_from_slice(&text);
+    let mut out = Vec::new();
+    out.extend_from_slice(&2u16.to_be_bytes());
+    out.extend_from_slice(&1u16.to_be_bytes());
+    out.extend_from_slice(&1u32.to_be_bytes()); // one annotation
+    out.extend_from_slice(&((entry.len() + 4) as u32).to_be_bytes());
+    out.extend_from_slice(&entry);
+    out
+}
+
+fn psd_with_anno(anno: Vec<u8>) -> Vec<u8> {
+    let mut doc = Document::new("notes", Size::new(16, 16), ColorMode::Rgb, SampleType::U8);
+    doc.layers.push(photocraft_doc::Layer::raster("Background", doc.pixel_format()));
+    let mut file = PsdFile::from_bytes(&to_psd(&doc)).unwrap();
+    let mut tb = photocraft_psd::TaggedBlock::new(*b"Anno", anno.clone());
+    tb.padding = Some(vec![0; (4 - anno.len() % 4) % 4]);
+    file.global_blocks.push(tb);
+    file.to_bytes().unwrap()
+}
+
+/// Icon rectangle `[top, left, bottom, right]` of the first note in the exported `Anno` block.
+fn exported_icon(doc: &Document) -> [i32; 4] {
+    let f = PsdFile::from_bytes(&to_psd(doc)).unwrap();
+    let d = &f.global_block(b"Anno").unwrap().data;
+    // header (2 + 2 + 4), entry length (4), `txtA` (4), open (1), flags (1), optional (2).
+    let mut out = [0i32; 4];
+    for (i, v) in out.iter_mut().enumerate() {
+        let at = 20 + i * 4;
+        *v = i32::from_be_bytes(d[at..at + 4].try_into().unwrap());
+    }
+    out
+}
+
+/// Issue #939: a note whose stored icon top sits near `i32::MAX` imported fine and then
+/// overflowed `y + 20` on every PSD save (a panic in debug, a wrapped coordinate in release). The
+/// far edge saturates instead, so the note survives a round trip with representable geometry.
+#[test]
+fn a_note_icon_near_i32_max_saves_with_representable_geometry() {
+    for (top, left) in [(i32::MAX - 5, 0), (0, i32::MAX - 3), (i32::MIN, i32::MIN)] {
+        let imp = import("notes.psd", &psd_with_anno(anno_with_icon(top, left))).expect("import");
+        assert_eq!(imp.document.notes[0].position, [f64::from(left), f64::from(top)]);
+        let icon = exported_icon(&imp.document);
+        assert_eq!(icon, [top, left, top.saturating_add(20), left.saturating_add(16)], "{top},{left}");
+        let back = import("notes.psd", &to_psd(&imp.document)).unwrap().document;
+        assert_eq!((back.notes[0].text.as_str(), back.notes[0].position), ("A", [f64::from(left), f64::from(top)]));
+    }
+    // The same through the model: a position a command could set beyond i32 saturates, NaN is 0.
+    let mut doc = Document::new("notes", Size::new(16, 16), ColorMode::Rgb, SampleType::U8);
+    doc.notes.push(Note { text: "far".into(), position: [1e300, f64::INFINITY], ..Default::default() });
+    assert_eq!(exported_icon(&doc), [i32::MAX, i32::MAX, i32::MAX, i32::MAX]);
+    doc.notes[0].position = [f64::NAN, -1e300];
+    assert_eq!(exported_icon(&doc), [i32::MIN, 0, i32::MIN + 20, 16]);
+    // Control: an ordinary note is unchanged.
+    doc.notes[0].position = [42.0, 28.0];
+    assert_eq!(exported_icon(&doc), [28, 42, 48, 58]);
+    let back = import("x.psd", &to_psd(&doc)).unwrap().document;
+    assert_eq!(back.notes[0].position, [42.0, 28.0]);
+}

crates/raw/src/orf.rs

@@ -72,7 +72,14 @@ pub(crate) fn decode(t: &Tiff, limits: &Limits) -> Result<Sensor> {
     let raw = t.ifd_at(t.first_ifd, 0).ok_or_else(|| RawError::malformed("ORF has no IFD0"))?;
     let width = u64::from(t.tag_uint(&raw, tag::IMAGE_WIDTH).unwrap_or(0));
     let height = u64::from(t.tag_uint(&raw, tag::IMAGE_LENGTH).unwrap_or(0));
-    let stored: u64 = t.tag_uints(&raw, tag::STRIP_BYTE_COUNTS).iter().map(|&c| u64::from(c)).sum();
+    // Every ORF, compressed or not, declares its strip byte counts. Missing or unreadable counts
+    // mean IFD0 is cut short (a partial download or a damaged file), not the compressed variant:
+    // an empty sum must not be mistaken for "fewer than 16 bits per sample".
+    let counts = t.tag_uints(&raw, tag::STRIP_BYTE_COUNTS);
+    if counts.is_empty() {
+        return Err(RawError::malformed("ORF IFD0 has no readable strip byte counts"));
+    }
+    let stored: u64 = counts.iter().map(|&c| u64::from(c)).sum();
     if t.tag_uint(&raw, tag::COMPRESSION).unwrap_or(1) != 1 || stored < width.saturating_mul(height).saturating_mul(2) {
         return Err(RawError::unsupported("Olympus compressed (or packed) ORF is not decoded"));
     }

crates/raw/tests/vendor_codecs.rs

@@ -177,6 +177,33 @@ fn orf_uncompressed_round_trip() {
     assert_eq!((p.width, p.height), (16, 8));
 }
 
+/// A file cut short inside IFD0 keeps its width, height and compression entries but loses the
+/// strip byte counts; their empty sum used to read as "fewer than 16 bits per sample" and the
+/// damaged file was reported as the undecoded compressed variant. Damage is `Malformed`,
+/// whichever byte it lands on.
+#[test]
+fn orf_truncated_in_ifd0_is_malformed_not_the_compressed_variant() {
+    let (w, h) = (40, 24);
+    let data = mosaic(&scene(w, h), w, [1, 0, 2, 1], 64, 4095);
+    let b = orf(w, h, &data);
+    assert!(decode(&b, &Limits::default()).is_ok());
+    let ifd = u32::from_le_bytes(b[4..8].try_into().unwrap()) as usize;
+    let n = u16::from_le_bytes([b[ifd], b[ifd + 1]]) as usize;
+    let entries: Vec<u16> = (0..n).map(|i| u16::from_le_bytes([b[ifd + 2 + 12 * i], b[ifd + 3 + 12 * i]])).collect();
+    let at = |tag: u16| ifd + 2 + 12 * entries.iter().position(|&t| t == tag).unwrap();
+    // Cut right before the StripByteCounts entry: width, height and compression survive.
+    let cut = at(279);
+    assert!(cut > at(259), "compression precedes the byte counts");
+    assert!(matches!(decode(&b[..cut], &Limits::default()), Err(RawError::Malformed(_))), "{:?}", decode(&b[..cut], &Limits::default()).err());
+    // No prefix of the file reads as the compressed variant: the compression declaration is
+    // intact or gone, never "compressed".
+    for cut in 0..b.len() {
+        if let Err(RawError::Unsupported(m)) = decode(&b[..cut], &Limits::default()) {
+            assert!(!m.contains("compressed"), "prefix {cut}: {m}");
+        }
+    }
+}
+
 #[test]
 fn orf_packed_or_compressed_falls_back() {
     let (w, h) = (40, 24);