Merge pull request #7432 from aiongg/integration-2

Integration batch: 11 PRs on main

crates/algo/src/camera_raw/local_tone.rs

@@ -173,6 +173,39 @@ fn up(p: &Plane, w: usize, h: usize) -> Plane {
     Plane { w, h, v: out }
 }
 
+/// The source rows of [`up`]`(p, w, h)` at output row `y`, one per tap (`None` where the
+/// zero-stuffed row is all zeros).
+fn up_rows(p: &Plane, h: usize, y: usize) -> [Option<usize>; 5] {
+    let lh = p.h.min(h.div_ceil(2));
+    std::array::from_fn(|k| {
+        let m = mirror(y as isize + k as isize - 2, h);
+        (m.is_multiple_of(2) && m / 2 < lh).then_some(m / 2)
+    })
+}
+
+/// One sample of [`up`]`(p, w, h)`: column `x` of the output row whose [`up_rows`] are `rows`.
+/// It adds the same terms in the same order as `up`, so it gives the same bits.
+fn up_at(p: &Plane, w: usize, rows: &[Option<usize>; 5], x: usize) -> f32 {
+    let lw = p.w.min(w.div_ceil(2));
+    let col = |j: usize| -> f32 { (0..5).filter_map(|k| rows[k].map(|r| TAPS[k] * (p.v[r * p.w + j] * 4.0))).sum() };
+    if x >= 2 && x + 2 < w {
+        // Away from the edges no tap mirrors.
+        (0..5)
+            .filter_map(|k| {
+                let i = x + k - 2;
+                (i.is_multiple_of(2) && i / 2 < lw).then(|| TAPS[k] * col(i / 2))
+            })
+            .sum()
+    } else {
+        (0..5)
+            .filter_map(|k| {
+                let m = mirror(x as isize + k as isize - 2, w);
+                (m.is_multiple_of(2) && m / 2 < lw).then(|| TAPS[k] * col(m / 2))
+            })
+            .sum()
+    }
+}
+
 /// Levels down to about 2 pixels on the short side.
 fn depth(w: usize, h: usize) -> usize {
     let mut s = w.min(h);
@@ -217,14 +250,14 @@ fn llf(u: &Plane, remap: impl Fn(f32, f32) -> f32 + Sync) -> Plane {
         });
         let gr = gaussian(m, n);
         for l in 0..n - 1 {
-            let coarse = up(&gr[l + 1], gr[l].w, gr[l].h);
-            let (gl, fine, lw) = (&g[l].v, &gr[l].v, g[l].w);
+            let (gl, fine, lw, lh, coarse) = (&g[l].v, &gr[l].v, g[l].w, g[l].h, &gr[l + 1]);
             rows(&mut lap[l], lw, |y, row| {
+                let taps = up_rows(coarse, lh, y);
                 for (x, o) in row.iter_mut().enumerate() {
                     let i = y * lw + x;
                     let wgt = (1.0 - (gl[i].clamp(lo, hi) - r).abs() / step).max(0.0);
                     if wgt > 0.0 {
-                        *o += wgt * (fine[i] - coarse.v[i]);
+                        *o += wgt * (fine[i] - up_at(coarse, lw, &taps, x));
                     }
                 }
             });
@@ -244,8 +277,10 @@ fn llf(u: &Plane, remap: impl Fn(f32, f32) -> f32 + Sync) -> Plane {
 /// The `q` quantile (0 … 1) of `v`.
 fn quantile(v: &[f32], q: f32) -> f32 {
     let mut s = v.to_vec();
-    s.sort_by(f32::total_cmp);
-    s[((s.len() - 1) as f32 * q).round() as usize]
+    // Selecting the one element is linear, a full sort is not (5 sorts of the work copy were a
+    // fifteenth of a render). `total_cmp` ties are bit-identical, so it is the sorted element.
+    let i = ((s.len() - 1) as f32 * q).round() as usize;
+    *s.select_nth_unstable_by(i, f32::total_cmp).1
 }
 
 /// 1st and 99th percentiles of the log luminance on a copy at most 256 pixels across.
@@ -444,11 +479,33 @@ mod tests {
                 assert_eq!(blur_step(&p, step).v, blur_step_reference(&p, step).v, "blur {w}×{h} step {step}");
             }
             for (uw, uh) in [(w * 2, h * 2), (w * 2 - 1, h * 2 - 1), (w * 2, h * 2 - 1), (w.max(2) * 2 - 3, h * 2)] {
-                assert_eq!(up(&p, uw, uh).v, up_reference(&p, uw, uh).v, "up {w}×{h} → {uw}×{uh}");
+                let full = up(&p, uw, uh);
+                assert_eq!(full.v, up_reference(&p, uw, uh).v, "up {w}×{h} → {uw}×{uh}");
+                // The filter samples `up` one pixel at a time; every sample has the same bits.
+                for y in 0..uh {
+                    let taps = up_rows(&p, uh, y);
+                    for x in 0..uw {
+                        assert_eq!(up_at(&p, uw, &taps, x).to_bits(), full.v[y * uw + x].to_bits(), "up_at {w}×{h} → {uw}×{uh} at {x},{y}");
+                    }
+                }
             }
         }
     }
 
+    #[test]
+    fn quantile_is_the_sorted_element() {
+        let p = noise_plane(301, 211, 9);
+        let mut v = p.v.clone();
+        // Ties and signed zeros, which `total_cmp` orders by their bits.
+        v.extend([0.0, -0.0, 0.0, -0.0, 3.5, 3.5, f32::MIN_POSITIVE]);
+        let mut sorted = v.clone();
+        sorted.sort_by(f32::total_cmp);
+        for q in [0.0, 0.001, 0.01, 0.25, 0.5, 0.99, 1.0] {
+            let want = sorted[((sorted.len() - 1) as f32 * q).round() as usize];
+            assert_eq!(quantile(&v, q).to_bits(), want.to_bits(), "q {q}");
+        }
+    }
+
     fn grey(w: usize, h: usize, f: impl Fn(usize, usize) -> f32) -> Vec<[f32; 4]> {
         (0..w * h).map(|i| f(i % w, i / w)).map(|v| [v, v, v, 1.0]).collect()
     }

crates/algo/src/segment/grabcut.rs

@@ -126,7 +126,9 @@ pub fn object_select(sampler: &dyn Sampler, canvas: Rect, rect: Rect, work_px: u
     }
     let (fg, bg) = grabcut(&img, &mut trimap, 8)?;
     let low: Vec<bool> = trimap.iter().map(|t| is_fg(*t)).collect();
-    let low = super::clean_mask(&low, lw, lh, 0.1, 0.002);
+    let low = super::clean_mask(&low, lw, lh, 0.1, 0.0);
+    // Every enclosed hole, however small, fills only when it looks like the object, so background seen through it stays open.
+    let low = super::fill_holes_by_model(&low, &img, &fg, &bg, 0.0, 0.05);
     super::finish_region(sampler, window, step, &low, lw, lh, Some((&fg, &bg)))
 }
 
@@ -209,7 +211,9 @@ pub fn object_at(sampler: &dyn Sampler, canvas: Rect, x: f32, y: f32, work_px: u
     let mut seeds = vec![false; w * h];
     seeds[sy * w + sx] = true;
     let low = super::keep_seeded(&low, &seeds, w, h);
-    let low = super::clean_mask(&low, w, h, 0.1, 0.01);
+    let low = super::clean_mask(&low, w, h, 0.1, 0.0);
+    // As in the rectangle mode: enclosed holes fill only when they look like the object.
+    let low = super::fill_holes_by_model(&low, &img, &fg, &bg, 0.0, 0.05);
     if !low.iter().any(|v| *v) {
         return None;
     }

crates/algo/src/segment/tests.rs

@@ -912,3 +912,61 @@ fn select_subject_dump_pictures() {
     write("ring", &img, None);
     write("ring-selected", &img, reg.as_ref());
 }
+
+/// A dark square on a light background with a square hole that shows the background colour.
+fn square_with_hole(hole: usize) -> RgbImage {
+    let (w, h) = (400usize, 300usize);
+    let mut rng = Rng::new(11);
+    let (x0, y0) = (100usize, 50usize);
+    RgbImage::from_fn(w, h, |x, y| {
+        let n = 0.02 * rng.normal();
+        let in_square = x >= x0 && x < x0 + 200 && y >= y0 && y < y0 + 200;
+        let in_hole = x >= x0 + 96 && x < x0 + 96 + hole && y >= y0 + 96 && y < y0 + 96 + hole;
+        let c = if in_square && !in_hole { [0.1, 0.12, 0.15] } else { [0.9, 0.9, 0.85] };
+        c.map(|v: f32| (v + n).clamp(0.0, 1.0))
+    })
+}
+
+#[test]
+fn object_select_keeps_small_background_hole_open() {
+    let img = square_with_hole(8);
+    let s = ImageSampler { img: &img, origin: (0, 0) };
+    let canvas = Rect::new(0, 0, 400, 300);
+    let reg = grabcut::object_select(&s, canvas, Rect::new(80, 30, 320, 270), 1_000_000).unwrap();
+    assert!(reg.at(150, 100) >= 0.5, "the square itself is selected");
+    assert!(reg.at(200, 150) < 0.5, "background seen through the object stays unselected");
+}
+
+#[test]
+fn object_at_keeps_small_background_hole_open() {
+    let img = square_with_hole(16);
+    let s = ImageSampler { img: &img, origin: (0, 0) };
+    let canvas = Rect::new(0, 0, 400, 300);
+    let reg = grabcut::object_at(&s, canvas, 130.0, 80.0, 1_000_000).unwrap();
+    assert!(reg.at(150, 100) >= 0.5, "the square itself is selected");
+    assert!(reg.at(203, 153) < 0.5, "background seen through the object stays unselected");
+}
+
+#[test]
+fn hole_fill_without_specks_fills_object_coloured_holes_only() {
+    // The hole check Object Selection uses: no hole fills just for being small, a speck of the object's colour still does.
+    let (w, h) = (40usize, 40usize);
+    let mut rng = Rng::new(5);
+    let in_square = |x: usize, y: usize| (5..35).contains(&x) && (5..35).contains(&y);
+    let bg_hole = |x: usize, y: usize| (10..12).contains(&x) && (10..12).contains(&y);
+    let fg_hole = |x: usize, y: usize| (25..27).contains(&x) && (25..27).contains(&y);
+    let img = RgbImage::from_fn(w, h, |x, y| {
+        let n = 0.02 * rng.normal();
+        let c = if in_square(x, y) && !bg_hole(x, y) { [0.1, 0.12, 0.15] } else { [0.9, 0.9, 0.85] };
+        c.map(|v: f32| (v + n).clamp(0.0, 1.0))
+    });
+    let mask: Vec<bool> = (0..w * h).map(|i| in_square(i % w, i / w) && !bg_hole(i % w, i / w) && !fg_hole(i % w, i / w)).collect();
+    // The colour models, as GrabCut would fit them: the object colour vs. the backdrop colour.
+    let dark: Vec<bool> = (0..w * h).map(|i| in_square(i % w, i / w) && !bg_hole(i % w, i / w)).collect();
+    let (fs, bs): (Vec<_>, Vec<_>) = img.px.iter().zip(&dark).partition(|(_, m)| **m);
+    let fit = |v: Vec<(&[f32; 3], &bool)>| Gmm::fit(&v.iter().map(|(p, _)| **p).collect::<Vec<_>>(), 3, gmm::DEFAULT_REG).unwrap();
+    let (fg, bg) = (fit(fs), fit(bs));
+    let out = fill_holes_by_model(&mask, &img, &fg, &bg, 0.0, 0.05);
+    assert!(out[25 * w + 25], "a speck of the object's colour fills");
+    assert!(!out[10 * w + 10], "a speck of the background's colour stays open");
+}

crates/engine/src/analysis_cmds.rs

@@ -757,7 +757,12 @@ fn count_add(s: &mut Session, p: &Value) -> Result<Value> {
     if !at[0].is_finite() || !at[1].is_finite() {
         return Err(bad("count.add", "coordinates must be finite"));
     }
-    let gi = p.get("group").and_then(Value::as_u64).map(|v| v as usize);
+    let gi = p.get("group").and_then(Value::as_u64).map(|v| usize::try_from(v).map_err(|_| bad("count.add", "no such group"))).transpose()?;
+    // The first marker creates group zero, but an explicit target must never be redirected.
+    let group_count = doc(s)?.measurement.count_groups.len().max(1);
+    if gi.is_some_and(|i| i >= group_count) {
+        return Err(bad("count.add", "no such group"));
+    }
     let n = s.edit("Count", |d, _| {
         if d.measurement.count_groups.is_empty() {
             let g = new_group(d, None, None);
@@ -858,7 +863,9 @@ fn count_delete_group(s: &mut Session, p: &Value) -> Result<Value> {
     s.edit("Delete Count Group", |d, _| {
         d.measurement.count_groups.remove(g);
         let n = d.measurement.count_groups.len();
-        d.measurement.active_count_group = d.measurement.active_count_group.min(n.saturating_sub(1));
+        // Removing a group before the active one shifts it down a slot.
+        let a = d.measurement.active_count_group;
+        d.measurement.active_count_group = (if g < a { a - 1 } else { a }).min(n.saturating_sub(1));
         Ok(())
     })?;
     Ok(count_json(doc(s)?.as_ref()))

crates/engine/src/analysis_cmds/tests.rs

@@ -291,6 +291,27 @@ fn count_groups_markers_and_undo() {
     assert!(s.execute("count.add", json!({"x": 1})).is_err());
 }
 
+#[test]
+fn deleting_an_earlier_count_group_keeps_the_active_group() {
+    let mut s = session(100, 100, 8);
+    s.execute("count.add", json!({"x": 10, "y": 10})).unwrap();
+    for _ in 0..3 {
+        s.execute("count.newGroup", json!({})).unwrap();
+    }
+    s.execute("count.setGroup", json!({"group": 2})).unwrap();
+    let r = s.execute("count.deleteGroup", json!({"group": 0})).unwrap();
+    assert_eq!(r["activeGroup"], 1);
+    assert_eq!(r["groups"][1]["name"], "Count Group 3");
+    // A later group leaves the active index alone.
+    let r = s.execute("count.deleteGroup", json!({"group": 2})).unwrap();
+    assert_eq!(r["activeGroup"], 1);
+    assert_eq!(r["groups"][1]["name"], "Count Group 3");
+    // Deleting the active last group selects the new last group.
+    let r = s.execute("count.deleteGroup", json!({})).unwrap();
+    assert_eq!(r["activeGroup"], 0);
+    assert_eq!(r["groups"][0]["name"], "Count Group 2");
+}
+
 #[test]
 fn log_delete_and_csv_export() {
     let mut s = session(10, 10, 8);
@@ -470,3 +491,47 @@ fn count_set_group_rejects_sizes_that_are_not_whole_numbers() {
     assert_eq!((g.marker_size, g.label_size), (3, 12), "a rejected call changes nothing");
     assert_eq!(s.active().unwrap().history.past_len(), past);
 }
+
+#[test]
+fn count_add_rejects_unknown_groups_without_changing_the_document() {
+    for groups in [0, 1, 3] {
+        for group in [(groups as u64).max(1), u64::MAX] {
+            let mut s = session(10, 10, 8);
+            for _ in 0..groups {
+                s.execute("count.newGroup", json!({})).unwrap();
+            }
+            let before = d(&s);
+            let history = s.active().unwrap().history.past_len();
+            let journal = s.journal.len();
+            let error = s.execute("count.add", json!({"x": 2, "y": 3, "group": group})).unwrap_err();
+            assert!(error.to_string().contains("no such group"), "{error}");
+            assert!(Arc::ptr_eq(&before, &d(&s)), "an invalid target must not create a group or add a marker");
+            assert_eq!(s.active().unwrap().history.past_len(), history);
+            assert_eq!(s.journal.len(), journal);
+        }
+    }
+}
+
+#[test]
+fn count_add_keeps_first_group_creation_and_targets_the_requested_group() {
+    let mut s = session(10, 10, 8);
+    // Group zero can be requested before the first marker creates it.
+    let r = s.execute("count.add", json!({"x": 2, "y": 3, "group": 0})).unwrap();
+    assert_eq!(r["groups"][0]["points"], json!([[2.0, 3.0]]));
+    s.execute("count.newGroup", json!({})).unwrap();
+    let before = d(&s).measurement.clone();
+    let history = s.active().unwrap().history.past_len();
+    let r = s.execute("count.add", json!({"x": 4, "y": 5, "group": 0})).unwrap();
+    assert_eq!(r["activeGroup"], 1);
+    assert_eq!(r["groups"][0]["points"], json!([[2.0, 3.0], [4.0, 5.0]]));
+    assert_eq!(r["groups"][1]["count"], 0);
+    assert_eq!(s.active().unwrap().history.past_len(), history + 1);
+    let after = d(&s).measurement.clone();
+    assert!(s.undo());
+    assert_eq!(d(&s).measurement, before);
+    assert!(s.redo());
+    assert_eq!(d(&s).measurement, after);
+    // Omitting the group still uses the active group.
+    let r = s.execute("count.add", json!({"x": 6, "y": 7})).unwrap();
+    assert_eq!(r["groups"][1]["points"], json!([[6.0, 7.0]]));
+}

crates/engine/src/filters.rs

@@ -313,7 +313,7 @@ pub(crate) fn run_filter(s: &mut Session, id: &str, p: &Value) -> Result<Value>
 }
 
 /// Lock transparency: put the layer's alpha back after a filter, in the tiles it changed.
-fn keep_alpha(old: &photocraft_raster::Surface, new: &mut photocraft_raster::Surface) {
+pub(crate) fn keep_alpha(old: &photocraft_raster::Surface, new: &mut photocraft_raster::Surface) {
     if old.format() != new.format() || !new.format().alpha {
         return;
     }

crates/engine/src/mode_cmds.rs

@@ -76,7 +76,8 @@ pub(crate) fn rotate_arbitrary(s: &mut Session, p: &Value) -> Result<Value> {
     if deg.rem_euclid(360.0).abs() < 1e-9 {
         return Ok(json!({"width": s.active().map(|d| d.doc.size.width), "height": s.active().map(|d| d.doc.size.height)}));
     }
-    let bg = s.tools.background;
+    // The Background stays opaque, whatever the background colour's alpha.
+    let bg = [s.tools.background[0], s.tools.background[1], s.tools.background[2], 1.0];
     // Color Sampler points turn with the canvas (the same map the edit applies to the geometry).
     // They are pixel indices, so map the pixel's centre, as the warp samples the pixels.
     {

crates/engine/src/mode_cmds/tests.rs

@@ -81,6 +81,15 @@ fn rotate_arbitrary_other_modes() {
     }
 }
 
+#[test]
+fn rotate_arbitrary_keeps_the_background_opaque_with_a_translucent_background_colour() {
+    let mut s = session(40, 20, 8, "rgb");
+    s.execute("tools.setColors", json!({"background": "#00ff0080"})).unwrap();
+    s.execute("image.rotation.arbitrary", json!({"angle": 30})).unwrap();
+    let corner = doc(&s).layers[0].surface().unwrap().rgba(0, 0);
+    assert_eq!(corner, [0.0, 1.0, 0.0, 1.0], "the uncovered corner is opaque background colour");
+}
+
 #[test]
 fn rotate_arbitrary_keeps_the_full_precision_angle() {
     // Rounded to f32 this angle crosses a whole pixel: 1003 px instead of 1002, so a rotated crop

crates/engine/src/perspective_crop_cmds.rs

@@ -188,7 +188,8 @@ fn warp_layers(layers: &mut [Layer], h: &Homography, interp: Interp, clip: Rect,
 fn apply(doc: &mut Document, q: &[[f64; 2]; 4], h: &Homography, size: (u32, u32), interp: Interp, bg: [f32; 4]) -> Result<()> {
     let (w, ht) = size;
     let clip = Rect::from_xywh(0, 0, w, ht);
-    let bg = photocraft_raster::from_rgba(&doc.pixel_format(), bg);
+    // The Background stays opaque, whatever the background colour's alpha.
+    let bg = photocraft_raster::from_rgba(&doc.pixel_format(), [bg[0], bg[1], bg[2], 1.0]);
     let background_id = doc.background().map(|l| l.id);
     warp_layers(&mut doc.layers, h, interp, clip, &bg, background_id)?;
     for ch in doc.channels.iter_mut().chain(doc.quick_mask.as_mut()) {
@@ -298,6 +299,18 @@ mod tests {
         }
     }
 
+    /// Corners past the canvas: the uncovered Background is the background colour, opaque even
+    /// when that colour has alpha (the Background stays opaque).
+    #[test]
+    fn uncovered_background_is_opaque() {
+        let q = [[-20.0, -20.0], [180.0, -20.0], [180.0, 140.0], [-20.0, 140.0]];
+        let mut s = skewed_ramp(SampleType::U8, &q, 100.0, 80.0);
+        s.execute("tools.setColors", json!({"background": "#00ff0080"})).unwrap();
+        s.execute(CMD, json!({"corners": q, "width": 100, "height": 80})).unwrap();
+        let corner = s.active().unwrap().doc.layers[0].surface().unwrap().rgba(0, 0);
+        assert_eq!(corner, [0.0, 1.0, 0.0, 1.0]);
+    }
+
     /// Bad quads and params are refused with an error, and the document is untouched.
     #[test]
     fn perspective_crop_rejects_bad_quads() {

crates/engine/src/plugin_cmds.rs

@@ -77,10 +77,17 @@ fn run(s: &mut Session, p: &Value) -> Result<Value> {
             *surf = plugin.apply(surf, canvas, selection.as_ref(), &resolved).map_err(err)?;
             return Ok(());
         }
+        crate::commands::check_pixels_unlocked(doc, layer)?;
+        let keep_alpha = doc.effective_locks(layer).transparency;
         let l = doc.layer_mut(layer).ok_or(EngineError::NoLayer(layer))?;
         match &mut l.content {
             LayerContent::Raster(surf) => {
+                // Lock Transparency: the plug-in may change colour but not alpha (as built-in filters).
+                let before = keep_alpha.then(|| surf.clone());
                 *surf = plugin.apply(surf, canvas, selection.as_ref(), &resolved).map_err(err)?;
+                if let Some(before) = &before {
+                    crate::filters::keep_alpha(before, surf);
+                }
                 Ok(())
             }
             LayerContent::Smart(_) => {

crates/engine/src/plugin_cmds/tests.rs

@@ -155,6 +155,56 @@ fn params_are_validated_and_passed() {
     Session::new().execute("plugin.remove", json!({"id": "test.cmds.grey"})).unwrap();
 }
 
+#[test]
+fn locked_layers_refuse_plugin_filters() {
+    // Like built-in filters: locked pixels (or Lock All) refuse the run with no undo step.
+    let id = "test.cmds.locks";
+    Session::new().execute("plugin.install", json!({"data": b64(&wat_plugin(id, "{}", GREY))})).unwrap();
+    for lock_all in [false, true] {
+        let mut s = doc(16);
+        let layer = s.active().unwrap().active_layer.unwrap();
+        s.edit("lock", |doc, _| {
+            let l = doc.layer_mut(layer).unwrap();
+            if lock_all {
+                l.locks.all = true;
+            } else {
+                l.locks.pixels = true;
+            }
+            Ok(())
+        })
+        .unwrap();
+        let before = pixels(&s);
+        let r = s.execute("plugin.run", json!({"id": id}));
+        assert!(r.as_ref().is_err_and(|e| e.to_string().contains("is locked")), "lock_all {lock_all}: {r:?}");
+        assert_eq!(pixels(&s), before, "lock_all {lock_all}");
+        s.execute("edit.undo", json!({})).unwrap();
+        let l = s.active().unwrap().doc.layer(layer).unwrap();
+        assert!(!l.locks.pixels && !l.locks.all, "the refused run left no undo step");
+        // Unlocked, the plug-in runs.
+        s.execute("plugin.run", json!({"id": id})).unwrap();
+        assert_ne!(pixels(&s), before);
+    }
+    // Lock Transparency: the run changes colour but keeps every pixel's alpha (GREY sets alpha too).
+    let mut s = doc(16);
+    let layer = s.active().unwrap().active_layer.unwrap();
+    s.edit("lock", |doc, _| {
+        doc.layer_mut(layer).unwrap().locks.transparency = true;
+        Ok(())
+    })
+    .unwrap();
+    let before = pixels(&s);
+    s.execute("plugin.run", json!({"id": id})).unwrap();
+    let after = pixels(&s);
+    assert_ne!(after, before);
+    for (a, b) in after.as_chunks::<4>().0.iter().zip(before.as_chunks::<4>().0) {
+        assert_eq!(a[3], b[3], "alpha kept");
+        if b[3] > 0.0 {
+            assert!((a[0] - 0.5).abs() < 1e-3, "colour changed: {a:?}");
+        }
+    }
+    Session::new().execute("plugin.remove", json!({"id": id})).unwrap();
+}
+
 #[test]
 fn hostile_plugins_fail_without_touching_the_document() {
     for (id, body) in [

crates/engine/src/vector_cmds.rs

@@ -1091,9 +1091,25 @@ fn path_rename(s: &mut Session, p: &Value) -> Result<Value> {
 
 fn path_to_selection(s: &mut Session, p: &Value) -> Result<Value> {
     let path = resolve_path(s, p.get("name").and_then(Value::as_str))?;
-    let feather = f64p(p, "feather").unwrap_or(0.0) as f32;
-    let aa = p.get("antiAlias").and_then(Value::as_bool).unwrap_or(true);
-    let mode = SelectionMode::parse(p.get("mode").and_then(Value::as_str).unwrap_or("replace"));
+    // Validate before rasterizing or touching selection history. An unchecked feather value
+    // could silently become zero (negative/string), or expand into an excessive blur.
+    let feather = match p.get("feather") {
+        None => 0.0,
+        Some(v) => v
+            .as_f64()
+            .filter(|n| n.is_finite() && (0.0..=1000.0).contains(n))
+            .ok_or_else(|| bad("path.toSelection", "`feather` must be a finite number within 0..1000"))? as f32,
+    };
+    let aa = match p.get("antiAlias") {
+        None => true,
+        Some(Value::Bool(v)) => *v,
+        _ => return Err(bad("path.toSelection", "`antiAlias` must be a boolean")),
+    };
+    let mode = match p.get("mode") {
+        None => SelectionMode::Replace,
+        Some(Value::String(v)) if matches!(v.as_str(), "replace" | "add" | "subtract" | "intersect") => SelectionMode::parse(v),
+        _ => return Err(bad("path.toSelection", "`mode` must be replace, add, subtract or intersect")),
+    };
     let area = s.active().ok_or(EngineError::NoDocument)?.doc.bounds();
     let mut mask = vector::fill_rasterizer(&path, vector::DEFAULT_TOLERANCE).render(area);
     if !aa {

crates/engine/src/vector_cmds/tests.rs

@@ -335,6 +335,63 @@ fn path_to_selection_matches_analytic_shape() {
     assert!(m[100 * 200 + 160] > 0.2 && m[100 * 200 + 160] < 0.8);
 }
 
+#[test]
+fn path_make_selection_rejects_invalid_options_without_changing_history() {
+    let mut s = session(80, 80, 8);
+    s.execute("path.set", json!({"path": {"subpaths": [{"knots": [[12, 12], [60, 12], [60, 60], [12, 60]]}]}})).unwrap();
+    s.execute("select.rect", json!({"x": 20, "y": 20, "width": 12, "height": 12})).unwrap();
+    let before = doc(&s).clone();
+    let history = s.active().unwrap().history.past_len();
+    for opts in [
+        json!({"feather": -1}),
+        json!({"feather": 1001}),
+        json!({"feather": 1e100}),
+        json!({"feather": "4"}),
+        json!({"feather": true}),
+        json!({"antiAlias": "false"}),
+        json!({"antiAlias": 1}),
+        json!({"mode": "union"}),
+        json!({"mode": 42}),
+    ] {
+        assert!(s.execute("path.toSelection", opts.clone()).is_err(), "accepted invalid Make Selection options: {opts}");
+        assert_eq!(*doc(&s), before, "invalid Make Selection modified the document: {opts}");
+        assert_eq!(s.active().unwrap().history.past_len(), history, "invalid Make Selection changed history: {opts}");
+    }
+}
+
+#[test]
+fn open_pen_paths_make_the_same_selection_as_closed_paths() {
+    // The pen's unfinished work path should be implicitly closed when rasterized into a selection.
+    let mut s = session(80, 80, 8);
+    let knots = json!([[12, 12], [52, 12], [52, 52], [12, 52]]);
+    for (aa, feather) in [(false, 0.0), (true, 0.0), (false, 3.0), (true, 3.0)] {
+        let mut masks = Vec::new();
+        for closed in [false, true] {
+            s.execute("path.set", json!({"path": {"subpaths": [{"closed": closed, "knots": knots}]}})).unwrap();
+            s.execute("path.toSelection", json!({"antiAlias": aa, "feather": feather, "mode": "replace"})).unwrap();
+            masks.push(selection_mask(&s));
+        }
+        assert_eq!(masks[0], masks[1], "open and closed paths differ (antiAlias={aa}, feather={feather})");
+        assert!(masks[0][32 * 80 + 32] > 0.99);
+        assert_eq!(masks[0][4 * 80 + 4], 0.0);
+        if feather > 0.0 {
+            assert!(masks[0].iter().any(|v| *v > 0.0 && *v < 1.0), "feather did not soften the selection");
+        }
+    }
+
+    // All boolean operations should use the same selection semantics for an open Pen path.
+    for mode in ["replace", "add", "subtract", "intersect"] {
+        s.execute("select.rect", json!({"x": 30, "y": 30, "width": 34, "height": 25})).unwrap();
+        s.execute("path.set", json!({"path": {"subpaths": [{"closed": false, "knots": knots}]}})).unwrap();
+        s.execute("path.toSelection", json!({"mode": mode, "antiAlias": false})).unwrap();
+        let mask = selection_mask(&s);
+        let at = |x: usize, y: usize| mask[y * 80 + x];
+        assert_eq!(at(20, 20), if matches!(mode, "replace" | "add") { 1.0 } else { 0.0 }, "{mode}: path-only");
+        assert_eq!(at(60, 40), if matches!(mode, "add" | "subtract") { 1.0 } else { 0.0 }, "{mode}: previous-only");
+        assert_eq!(at(40, 40), if mode == "subtract" { 0.0 } else { 1.0 }, "{mode}: overlap");
+    }
+}
+
 #[test]
 fn selection_to_work_path_roundtrip_iou() {
     let mut s = session(240, 180, 8);

crates/engine/src/video_cmds.rs

@@ -426,7 +426,9 @@ pub fn render_video_with(
     if frames > crate::timeline_cmds::MAX_DURATION {
         return Err(EngineError::Other(format!("the timeline is {frames} frames; Render Video is limited to {} frames", crate::timeline_cmds::MAX_DURATION)));
     }
-    let fps = base.timeline.as_ref().map_or(30.0, |t| t.fps).max(1.0);
+    // Only the GIF delay reads the rate (frames come from the duration); rates under 1 fps are
+    // valid timelines, so only an unusable loaded rate falls back to 30.
+    let fps = base.timeline.as_ref().map(|t| t.fps).filter(|f| f.is_finite() && *f > 0.0).unwrap_or(30.0);
     let stem = base.name.rsplit_once('.').map_or(base.name.as_str(), |(a, _)| a).to_string();
     let fmt = base.pixel_format();
     let bounds = base.bounds();

crates/engine/src/video_cmds/tests.rs

@@ -151,6 +151,39 @@ fn a_fully_transparent_animated_gif_frame_encodes() {
     assert!(f.indices.iter().all(|&i| Some(i) == f.transparent));
 }
 
+/// The GIF frame delays (centiseconds, from each Graphic Control Extension) of a Render Video.
+fn rendered_gif_delays(s: &Session) -> Vec<u16> {
+    let mut gif = Vec::new();
+    render_video_with(
+        s,
+        &json!({"format": "gif"}),
+        |_, bytes| {
+            gif = bytes.to_vec();
+            Ok(())
+        },
+        |_, _| true,
+    )
+    .unwrap();
+    gif.windows(6).filter(|w| w[..3] == [0x21, 0xF9, 0x04]).map(|w| u16::from_le_bytes([w[4], w[5]])).collect()
+}
+
+#[test]
+fn animated_gif_delay_follows_timelines_under_one_fps() {
+    // 0.5 fps is a valid timeline rate; its frames last 2 s, not the 1 s a 1 fps floor gave.
+    let mut s = session();
+    s.execute("timeline.create", json!({"duration": 2, "fps": 0.5})).unwrap();
+    assert_eq!(rendered_gif_delays(&s), [200, 200]);
+    // A loaded timeline with an unusable rate falls back to 30 fps (3 cs), as with no timeline.
+    let mut doc = (*s.active().unwrap().doc).clone();
+    doc.timeline = Some(Timeline::new(2, f32::NAN));
+    {
+        let st = s.active_mut().unwrap();
+        st.doc = std::sync::Arc::new(doc);
+        st.revision += 1;
+    }
+    assert_eq!(rendered_gif_delays(&s), [3, 3]);
+}
+
 #[test]
 fn inspect_reports_video_layer() {
     let dir = tmpdir("vinsp");

crates/format/src/store.rs

@@ -163,6 +163,10 @@ pub(crate) fn read_manifest(src: &dyn ByteSource, opts: &LoadOptions) -> Result<
     let layers = take_array(&mut v, "/document/layers");
     let mut m: Manifest = serde_json::from_value(v)?;
     m.document.layers = layers_from_json(layers)?;
+    // A number past f32's range decodes as infinity, which no save could write back (#6862).
+    if let Err(crate::finite::Problem::NonFinite { path }) = crate::finite::check(&m) {
+        return Err(FormatError::corrupt(format!("`{path}` is out of range")));
+    }
     Ok(m)
 }
 

crates/format/tests/corruption.rs

@@ -94,6 +94,24 @@ fn bad_manifest_json() {
     assert!(matches!(load_from_bytes(&b), Err(FormatError::Json(_))));
 }
 
+/// A number past f32's range would load as infinity, and every later save would then fail
+/// (#6862): it is refused when the file is opened, naming the value.
+#[test]
+fn out_of_range_number_refused_on_open() {
+    let b = rebuild(&sample(), |n, d| {
+        Some(if n == "manifest.json" {
+            let mut v: serde_json::Value = serde_json::from_slice(&d).unwrap();
+            v["document"]["layers"][0]["opacity"] = serde_json::json!(1e39);
+            serde_json::to_vec(&v).unwrap()
+        } else {
+            d
+        })
+    });
+    let Err(e) = load_from_bytes(&b) else { panic!("a manifest with opacity 1e39 opened") };
+    assert!(matches!(e, FormatError::Corrupt(_)), "{e}");
+    assert!(e.to_string().contains("document.layers[0].opacity"), "{e}");
+}
+
 /// Hostile nesting fails with an error instead of overflowing the stack; nesting as deep as the
 /// deepest groups a bundle may hold (three JSON levels each) parses, and is then rejected for
 /// lacking a format version.

crates/io/tests/psd16_merged_transparency.rs

@@ -0,0 +1,63 @@
+//! Opening and re-saving a Photoshop 16/32-bit file whose merged transparency is marked only by
+//! its name keeps the channel count: the "Transparency" channel is the composite's alpha, not a
+//! user alpha channel that the next save writes a second time (#4654).
+//!
+//! Photoshop's 16-bit PSB in the report has four channels, a positive layer count in `Lr16`, no
+//! `Mt16` block, and one alpha channel name, "Transparency". Re-saving it wrote five channels.
+
+mod common;
+
+use common::raster;
+use photocraft_color::{ColorMode, SampleType};
+use photocraft_doc::Document;
+use photocraft_geom::{Rect, Size};
+use photocraft_io::*;
+use photocraft_psd::PsdFile;
+
+/// The unicode alpha channel names (resource 1045), empty without it.
+fn names(file: &PsdFile) -> Vec<String> {
+    let Some(data) = file.resources.iter().find(|r| r.id == 1045).map(|r| &r.data) else { return Vec::new() };
+    let mut out = Vec::new();
+    let mut at = 0;
+    while at + 4 <= data.len() {
+        let n = u32::from_be_bytes(data[at..at + 4].try_into().unwrap()) as usize;
+        let units: Vec<u16> = data[at + 4..at + 4 + n * 2].chunks(2).map(|c| u16::from_be_bytes([c[0], c[1]])).collect();
+        out.push(String::from_utf16_lossy(&units).trim_end_matches('\0').to_string());
+        at += 4 + n * 2;
+    }
+    out
+}
+
+#[test]
+fn resave_keeps_a_name_marked_merged_transparency() {
+    for depth in [SampleType::U16, SampleType::F32] {
+        for path in ["x.psd", "x.psb"] {
+            let case = format!("{depth:?} {path}");
+            let mut doc = Document::new("t", Size::new(8, 6), ColorMode::Rgb, depth);
+            let fmt = doc.pixel_format();
+            doc.layers.push(raster("shape", fmt, Rect::new(1, 1, 5, 4), 3, true));
+            let ours = PsdFile::from_bytes(&export(&doc, path, &ExportOptions::default()).expect("export").bytes).expect("parse");
+            assert_eq!(ours.header.channels, 4, "{case}: colour + merged transparency");
+
+            // Photoshop's form: a positive layer count, no `Mt16`/`Mt32` marker, and the merged
+            // transparency named in the alpha channel names.
+            let mut ps = ours.clone();
+            ps.layer_info.as_mut().expect("layers").merged_alpha = false;
+            ps.global_blocks.retain(|b| !matches!(&b.key, b"Mtrn" | b"Mt16" | b"Mt32"));
+            assert!(!ps.merged_has_alpha(), "{case}: nothing marks it yet");
+            let name: Vec<u8> = [0u8, 0, 0, 13].into_iter().chain("Transparency\0".encode_utf16().flat_map(u16::to_be_bytes)).collect();
+            ps.resources.retain(|r| !matches!(r.id, 1006 | 1045));
+            ps.resources.push(photocraft_psd::ImageResource::new(1045, name));
+            assert!(ps.merged_has_alpha(), "{case}: the name marks the merged transparency");
+
+            let opened = import(path, &ps.to_bytes().expect("write")).expect("import").document;
+            let user: Vec<_> = opened.channels.iter().map(|c| c.name.as_str()).collect();
+            assert!(user.is_empty(), "{case}: no user alpha channel from the transparency: {user:?}");
+
+            let resaved = PsdFile::from_bytes(&export(&opened, path, &ExportOptions::default()).expect("re-export").bytes).expect("reparse");
+            assert_eq!(resaved.header.channels, 4, "{case}: a save with no edits keeps the channel count");
+            let resaved_names = names(&resaved);
+            assert!(resaved_names.iter().filter(|n| *n == "Transparency").count() <= 1, "{case}: no duplicate Transparency: {resaved_names:?}");
+        }
+    }
+}

crates/io/tests/psd32_export.rs

@@ -226,11 +226,11 @@ fn mt32_marks_the_merged_transparency() {
     let plane = |v: [f32; 2]| v.into_iter().flat_map(f32::to_be_bytes).collect::<Vec<u8>>();
     file.image_data.data = [plane([0.0, 1.0]), plane([0.0, 1.0]), plane([0.0, 1.0]), plane([1.0, 0.0])].concat();
     let name: Vec<u8> = [0u8, 0, 0, 13].into_iter().chain("Transparency\0".encode_utf16().flat_map(u16::to_be_bytes)).collect();
-    file.resources.retain(|r| r.id != 1045);
-    file.resources.push(photocraft_psd::ImageResource::new(1045, name));
+    file.resources.retain(|r| !matches!(r.id, 1006 | 1045));
     assert!(!file.merged_has_alpha(), "a positive layer count alone");
     file.global_blocks.push(photocraft_psd::TaggedBlock::new(*b"Mt32", Vec::new()));
     assert!(file.merged_has_alpha());
+    file.resources.push(photocraft_psd::ImageResource::new(1045, name));
     let merged = merged_composite(&file).unwrap();
     assert_eq!(merged.iter().map(|p| p[3]).collect::<Vec<_>>(), vec![1.0, 0.0]);
     let opened = import("transparent.psd", &file.to_bytes().unwrap()).unwrap();

crates/psd/src/file.rs

@@ -354,7 +354,10 @@ impl PsdFile {
     /// `true` if the merged image carries a transparency channel: its first extra channel, when
     /// the layer count was stored negative or a "Saving Merged Transparency" global block (`Mtrn`,
     /// `Mt16`, `Mt32`) is present. Photoshop marks 32-bit documents with `Mt32` and a positive
-    /// layer count (psd-tools transparentbg.psd).
+    /// layer count (psd-tools transparentbg.psd). Photoshop also names the merged transparency
+    /// "Transparency" and lists it first among the alpha channel names, and some of its 16-bit
+    /// PSBs carry no other mark (#4654): when the layers sit in a global `Lr16`/`Lr32` block, that
+    /// name with one name per extra channel marks it too.
     pub fn merged_has_alpha(&self) -> bool {
         let Some(color) = self.header.color_mode.color_channels() else {
             return false;
@@ -364,11 +367,40 @@ impl PsdFile {
         }
         let marked = self.global_blocks.iter().any(|b| matches!(&b.key, b"Mtrn" | b"Mt16" | b"Mt32"));
         match &self.layer_info {
-            Some(li) => li.merged_alpha || marked,
+            Some(li) => {
+                let in_block = matches!(self.layer_info_placement, LayerInfoPlacement::GlobalBlock { .. });
+                li.merged_alpha || marked || (in_block && self.names_merged_transparency(self.header.channels - color))
+            }
             None => true,
         }
     }
 
+    /// `true` when the alpha channel names (resource 1045, else 1006) hold exactly `extra` names
+    /// and the first is "Transparency", Photoshop's name for the merged transparency.
+    fn names_merged_transparency(&self, extra: u16) -> bool {
+        const NAME: &str = "Transparency";
+        let mut names = Vec::new();
+        if let Some(r) = self.resource(1045) {
+            let mut at = 0usize;
+            while let Some(len) = r.data.get(at..at.saturating_add(4)).and_then(|b| b.try_into().ok()).map(u32::from_be_bytes) {
+                at = at.saturating_add(4);
+                let end = (len as usize).checked_mul(2).and_then(|n| at.checked_add(n));
+                let Some(units) = end.and_then(|end| r.data.get(at..end)) else { break };
+                let units: Vec<u16> = units.as_chunks::<2>().0.iter().map(|c| u16::from_be_bytes(*c)).collect();
+                names.push(String::from_utf16_lossy(&units).trim_end_matches('\0').to_string());
+                at = at.saturating_add(units.len() * 2);
+            }
+        } else if let Some(r) = self.resource(1006) {
+            let mut at = 0usize;
+            while let Some(&len) = r.data.get(at) {
+                let Some(name) = r.data.get(at + 1..at + 1 + usize::from(len)) else { break };
+                names.push(String::from_utf8_lossy(name).into_owned());
+                at += 1 + usize::from(len);
+            }
+        }
+        names.len() == usize::from(extra) && names.first().is_some_and(|n| n == NAME)
+    }
+
     /// Checks structural invariants that the writer relies on.
     pub fn validate(&self) -> Result<()> {
         self.header.validate()?;

crates/ui-egui/src/menu_nav.rs

@@ -336,7 +336,10 @@ impl Nav {
         let Some(top) = self.top.filter(|_| !tops.is_empty()) else { return };
         let level = self.level.min(self.rows.len().saturating_sub(1));
         self.level = level;
-        let press = |k: Key| ctx.input_mut(|i| i.consume_key(Modifiers::NONE, k));
+        // A text field in the menu (Help › Search) keeps the keys that edit it: ← / → move its
+        // cursor and Space types a space.
+        let typing = ctx.text_edit_focused();
+        let press = |k: Key| !(typing && matches!(k, Key::ArrowLeft | Key::ArrowRight | Key::Space)) && ctx.input_mut(|i| i.consume_key(Modifiers::NONE, k));
         let current = self.highlighted(level).and_then(|i| self.rows.get(level)?.get(i)).cloned();
         let switch = |by: usize| {
             let n = tops.len();

crates/ui-egui/src/menu_nav_tests.rs

@@ -265,6 +265,44 @@ fn left_and_right_switch_menus_and_shortcuts_wait_while_one_is_open() {
     assert!(!is_open(&h.ctx), "shortcuts work again");
 }
 
+/// In Help › Search, ← / → move the field's cursor instead of switching to Window / File, and
+/// Space types a space instead of running the lit row.
+#[test]
+fn help_search_keeps_the_arrows_and_space_that_edit_it() {
+    let mut h = harness((1280.0, 720.0, 1.0));
+    open(&mut h, "Help");
+    let field = egui::Id::new("help-menu-search").with("field");
+    assert_eq!(h.ctx.memory(|m| m.focused()), Some(field), "the search field has the keyboard");
+    let text = |h: &Harness<'static, PhotocraftApp>| h.ctx.data(|d| d.get_temp::<String>(egui::Id::new("help-menu-search"))).unwrap_or_default();
+    let typing = |h: &mut Harness<'static, PhotocraftApp>, s: &str| {
+        h.event(egui::Event::Text(s.to_string()));
+        h.run_steps(3);
+    };
+    typing(&mut h, "levls");
+    key(&mut h, egui::Key::ArrowLeft);
+    assert!(h.query_by_label_contains("About PhotoCraft").is_some(), "← keeps Help open");
+    key(&mut h, egui::Key::ArrowLeft);
+    typing(&mut h, "e");
+    assert_eq!(text(&h), "levels", "← moved the cursor");
+    // The cursor sits at `leve|ls`: two → take it to the end.
+    key(&mut h, egui::Key::ArrowRight);
+    assert!(h.query_by_label_contains("About PhotoCraft").is_some(), "→ keeps Help open");
+    key(&mut h, egui::Key::ArrowRight);
+    // The pointer resting on a result lights it; a space typed then is text, not a click.
+    // "levels" finds both Image › Adjustments › Levels… and the Levels adjustment layer.
+    let levels = h.get_all_by_label_contains("› Levels").next().expect("a Levels result").rect().center();
+    h.hover_at(levels);
+    h.run_steps(3);
+    assert!(Nav::current(&h.ctx).highlighted(0).is_some(), "a result is lit");
+    h.event(egui::Event::Key { key: egui::Key::Space, physical_key: None, pressed: true, repeat: false, modifiers: egui::Modifiers::NONE });
+    h.event(egui::Event::Text(" ".into()));
+    h.event(egui::Event::Key { key: egui::Key::Space, physical_key: None, pressed: false, repeat: false, modifiers: egui::Modifiers::NONE });
+    h.run_steps(3);
+    assert!(is_open(&h.ctx) && h.query_by_label_contains("About PhotoCraft").is_some(), "Space keeps Help open");
+    assert!(h.state().ui.dialogs.is_empty(), "Space ran no command: {:?}", h.state().ui.dialogs);
+    assert_eq!(text(&h), "levels ");
+}
+
 /// A level's visible height (its view) and the full height of its rows: equal means no scrolling.
 fn fits(h: &Harness<'static, PhotocraftApp>, level: usize) -> (f32, f32) {
     let nav = Nav::current(&h.ctx);