PSD: write pattern transparency where Photoshop keeps it (#8054) (#8756)

* PSD: write pattern transparency where Photoshop keeps it (#8054)

A pattern defined over transparency (delete the Background, Define
Pattern, then a Pattern Fill layer from it) was saved with its alpha
plane in the virtual memory array list's user mask slot (24), and
Photoshop refused the PSD. Photoshop keeps pattern transparency in the
last (sheet mask) slot and leaves the user mask unwritten (psd-tools
multichannel-pattern-fill.psd, written by Photoshop CC 2019). The same
writer serves `Patt`/`Pat2`/`Pat3`, `.pat` and `.abr`.

Files saved by older builds still open (the reader takes the first
written array after the colour channels as transparency), and saving
them again rewrites their pattern block instead of keeping the raw
bytes, which `block_has_user_mask` detects.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* PSD: rewrite only legacy pattern blocks, keep unreadable ones (#8054)

When the document's patterns are unchanged, a pattern block with
transparency in the user mask array is now rewritten in place from its
own patterns, and every other block (pattern blocks that fail to decode
included) is kept verbatim. Previously a legacy block forced the
replace-all path, which dropped an unreadable pattern block that an
unchanged save used to keep. Blocks are decoded in order with one shared
budget, as from_global_blocks does.

Addresses CodeRabbit review on #8756.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

crates/io/src/pattern_map.rs

@@ -9,7 +9,7 @@ use std::sync::Arc;
 use photocraft_color::{ColorMode, PixelFormat, SampleType};
 use photocraft_doc::{Document, Pattern};
 use photocraft_geom::Rect;
-use photocraft_psd::patterns::{PatternBudget, PsdPattern, block_key, mode_channels, parse_pattern_block_with, write_pattern_block};
+use photocraft_psd::patterns::{PatternBudget, PsdPattern, block_has_user_mask, block_key, mode_channels, parse_pattern_block_with, write_pattern_block};
 use photocraft_raster::Surface;
 
 use crate::pixels::{deinterleave, interleave, max_sample, psd_depth, sample_for_depth, zero_sample};
@@ -124,12 +124,13 @@ pub fn from_global_blocks(doc: &Document) -> Vec<Pattern> {
         .collect()
 }
 
-/// Global blocks to write: the raw ones while they still match [`Document::patterns`], else the
+/// Global blocks to write: the raw ones while they still match [`Document::patterns`] (see
-/// raw pattern blocks replaced by one written from the document (at the first one's position).
+/// [`repair_legacy_blocks`]), else the raw pattern blocks replaced by one written from the
+/// document (at the first one's position).
 pub fn export_global_blocks(doc: &Document) -> Vec<photocraft_doc::PsdGlobalBlock> {
     let raw = &doc.metadata.psd_global_blocks;
     if from_global_blocks(doc) == doc.patterns {
-        return raw.clone();
+        return repair_legacy_blocks(raw);
     }
     let key = block_key(psd_depth(doc.depth));
     let fresh = write_pattern_block(&doc.patterns.iter().map(to_psd).collect::<Vec<_>>()).unwrap_or_default();
@@ -151,6 +152,31 @@ pub fn export_global_blocks(doc: &Document) -> Vec<photocraft_doc::PsdGlobalBloc
     out
 }
 
+/// `raw` with each pattern block that keeps transparency where older PhotoCraft builds put it
+/// (which Photoshop refuses, #8054) rewritten in place from its own patterns. Every other block,
+/// unreadable pattern blocks included, is kept verbatim. Blocks are decoded in order with one
+/// shared budget, as [`from_global_blocks`] decodes them.
+fn repair_legacy_blocks(raw: &[photocraft_doc::PsdGlobalBlock]) -> Vec<photocraft_doc::PsdGlobalBlock> {
+    if !raw.iter().any(|(_, k, d)| PATTERN_KEYS.contains(k) && block_has_user_mask(d)) {
+        return raw.to_vec();
+    }
+    let mut budget = PatternBudget::default();
+    raw.iter()
+        .map(|b @ (sig, key, data)| {
+            if !PATTERN_KEYS.contains(key) {
+                return b.clone();
+            }
+            match parse_pattern_block_with(data, &mut budget) {
+                Ok(ps) if block_has_user_mask(data) => match write_pattern_block(&ps) {
+                    Ok(fresh) => (*sig, *key, Arc::new(fresh)),
+                    Err(_) => b.clone(),
+                },
+                _ => b.clone(),
+            }
+        })
+        .collect()
+}
+
 /// Reads a `.pat` pattern file.
 pub fn read_pat(bytes: &[u8]) -> Result<Vec<Pattern>, String> {
     let ps = photocraft_psd::patterns::parse_pat_file(bytes).map_err(|e| format!("not a readable .pat file: {e}"))?;
@@ -217,4 +243,38 @@ mod tests {
         assert_eq!(from_global_blocks(&d2), d.patterns);
         assert_eq!(read_pat(&write_pat(&d.patterns).unwrap()).unwrap(), d.patterns);
     }
+
+    #[test]
+    fn transparent_pattern_blocks_from_older_builds_are_rewritten_on_save() {
+        // #8054: Define Pattern over transparency, then a Pattern Fill layer; Photoshop refused
+        // the PSD because the pattern's transparency sat in the user mask array.
+        let mut d = Document::new("x", photocraft_geom::Size { width: 4, height: 4 }, ColorMode::Rgb, SampleType::U16);
+        d.patterns.push(pattern(ColorMode::Rgb, SampleType::U16, true));
+        let out = export_global_blocks(&d);
+        assert_eq!(out.len(), 1);
+        assert!(!block_has_user_mask(&out[0].2));
+        // Move the transparency into the user mask array, as older builds wrote it: a 16-bit
+        // pattern's array is raw, so it is the last 8 + 23 + 5 × 3 × 2 bytes of the pattern.
+        let mut legacy = out[0].2.to_vec();
+        let end = 4 + u32::from_be_bytes([legacy[0], legacy[1], legacy[2], legacy[3]]) as usize;
+        legacy[end - (8 + 23 + 30) - 4..end].rotate_left(4);
+        assert!(block_has_user_mask(&legacy));
+        // A pattern block that doesn't decode (pattern version 2) sits beside it.
+        let unreadable = (*b"8BIM", *b"Patt", Arc::new(vec![0, 0, 0, 8, 0, 0, 0, 2, 0, 0, 0, 0]));
+        let other = (*b"8BIM", *b"Txt2", Arc::new(vec![1]));
+        d.metadata.psd_global_blocks = vec![other.clone(), (*b"8BIM", *b"Pat2", Arc::new(legacy)), unreadable.clone()];
+        assert_eq!(from_global_blocks(&d), d.patterns, "older files still open");
+        // The patterns are unchanged: only the legacy block is rewritten, in place; the other
+        // blocks are kept verbatim.
+        let out = export_global_blocks(&d);
+        assert_eq!(out.len(), 3);
+        assert_eq!((&out[0], &out[2]), (&other, &unreadable));
+        assert_eq!(out[1].1, *b"Pat2");
+        assert!(!block_has_user_mask(&out[1].2));
+        let mut back = d.clone();
+        back.metadata.psd_global_blocks = out;
+        assert_eq!(from_global_blocks(&back), d.patterns);
+        // Saving again keeps every block as it is.
+        assert_eq!(export_global_blocks(&back), back.metadata.psd_global_blocks);
+    }
 }

crates/psd/src/patterns.rs

@@ -18,7 +18,9 @@
 //! `.pat` file is `8BPT`, version u16 (1), count u32, then the patterns without length prefixes.
 //! Photoshop writes the channel count as 24 and only marks the real channels as written; like
 //! ag-psd (MIT) we take the first written arrays as the colour channels and the next one as
-//! transparency.
+//! transparency. Photoshop stores a pattern's transparency in the last (sheet mask) array and
+//! leaves the user mask unwritten (psd-tools multichannel-pattern-fill.psd, Photoshop CC 2019); a
+//! file with transparency in the user mask array is refused by Photoshop (#8054).
 
 use crate::compression::{Compression, MAX_DECODED_BYTES, PlaneLayout, decode_planes, encode_planes};
 use crate::error::{PsdError, Result};
@@ -143,7 +145,15 @@ impl<'a> Rd<'a> {
     }
 }
 
-fn read_pattern(r: &mut Rd, budget: &mut PatternBudget) -> Result<PsdPattern> {
+/// A pattern's mode, name, id and palette, up to its virtual memory array list.
+struct PatternHeader {
+    mode: u32,
+    name: String,
+    id: String,
+    palette: Option<Vec<u8>>,
+}
+
+fn read_header(r: &mut Rd) -> Result<PatternHeader> {
     let version = r.u32()?;
     if version != 1 {
         return Err(PsdError::invalid(format!("pattern version {version}")));
@@ -169,12 +179,22 @@ fn read_pattern(r: &mut Rd, budget: &mut PatternBudget) -> Result<PsdPattern> {
     } else {
         None
     };
+    Ok(PatternHeader { mode, name, id, palette })
+}
+
+/// The body of a pattern's virtual memory array list (after its version and length).
+fn read_vma_body<'a>(r: &mut Rd<'a>) -> Result<&'a [u8]> {
     let vver = r.u32()?;
     if vver != 3 {
         return Err(PsdError::invalid(format!("virtual memory array list version {vver}")));
     }
     let len = r.u32()? as usize;
-    let body = r.take(len)?;
+    r.take(len)
+}
+
+fn read_pattern(r: &mut Rd, budget: &mut PatternBudget) -> Result<PsdPattern> {
+    let PatternHeader { mode, name, id, palette } = read_header(r)?;
+    let body = read_vma_body(r)?;
     let mut v = Rd { b: body, p: 0 };
     let [top, left, bottom, right] = v.rect()?;
     let w = right.checked_sub(left).ok_or(PsdError::LimitExceeded("pattern width overflow"))?;
@@ -413,12 +433,12 @@ fn write_pattern(p: &PsdPattern, out: &mut Vec<u8>) -> Result<()> {
     for _ in nc..SLOTS as usize {
         body.extend_from_slice(&0u32.to_be_bytes());
     }
-    // User mask slot: transparency; sheet mask slot: unused.
+    // User mask slot: unused; sheet mask slot: transparency, where Photoshop keeps it (#8054).
+    body.extend_from_slice(&0u32.to_be_bytes());
     match &p.alpha {
         Some(a) => array(a, &mut body)?,
         None => body.extend_from_slice(&0u32.to_be_bytes()),
     }
-    body.extend_from_slice(&0u32.to_be_bytes());
     out.extend_from_slice(&3u32.to_be_bytes());
     out.extend_from_slice(&(body.len() as u32).to_be_bytes());
     out.extend_from_slice(&body);
@@ -448,6 +468,48 @@ pub fn parse_pattern_block_with(data: &[u8], budget: &mut PatternBudget) -> Resu
     Ok(out)
 }
 
+/// Whether `Patt` / `Pat2` / `Pat3` block data keeps a pattern's transparency in the user mask
+/// array, as PhotoCraft wrote it before #8054. Photoshop refuses such a file, so a save must
+/// rewrite the block rather than keep it. Unreadable data counts as `false`.
+pub fn block_has_user_mask(data: &[u8]) -> bool {
+    let mut r = Rd { b: data, p: 0 };
+    while r.p + 4 <= data.len() {
+        let Ok(len) = r.u32() else { return false };
+        if len == 0 {
+            break;
+        }
+        let Ok(body) = r.take(len as usize) else { return false };
+        if pattern_has_user_mask(&mut Rd { b: body, p: 0 }).unwrap_or(false) {
+            return true;
+        }
+        let pad = (4 - len as usize % 4) % 4;
+        r.p = (r.p + pad).min(data.len());
+    }
+    false
+}
+
+fn pattern_has_user_mask(r: &mut Rd) -> Result<bool> {
+    read_header(r)?;
+    let mut v = Rd { b: read_vma_body(r)?, p: 0 };
+    v.rect()?;
+    let count = v.u32()?;
+    // Every array takes at least 4 bytes, so the body bounds this loop whatever `count` says.
+    for i in 0..count.saturating_add(2) {
+        if v.p >= v.b.len() {
+            break;
+        }
+        if v.u32()? == 0 {
+            continue;
+        }
+        let alen = v.u32()? as usize;
+        if i == count && alen != 0 {
+            return Ok(true);
+        }
+        v.take(alen)?;
+    }
+    Ok(false)
+}
+
 /// Serializes patterns as `Patt` / `Pat2` / `Pat3` block data.
 pub fn write_pattern_block(patterns: &[PsdPattern]) -> Result<Vec<u8>> {
     let mut out = Vec::new();
@@ -536,6 +598,47 @@ mod tests {
         }
     }
 
+    /// Offsets of the virtual memory arrays of a one-pattern block (each at its "written" flag),
+    /// with whether each is written, and the end of the array list.
+    fn vma_slots(block: &[u8], p: &PsdPattern) -> (Vec<(usize, bool)>, usize) {
+        let rect = pattern_vma_rect_offset(4, p);
+        let end = rect + u32::from_be_bytes(block[rect - 4..rect].try_into().unwrap()) as usize;
+        let (mut at, mut slots) = (rect + 20, Vec::new());
+        while at < end {
+            let written = u32::from_be_bytes(block[at..at + 4].try_into().unwrap()) != 0;
+            slots.push((at, written));
+            at += 4;
+            if written {
+                at += 4 + u32::from_be_bytes(block[at..at + 4].try_into().unwrap()) as usize;
+            }
+        }
+        (slots, end)
+    }
+
+    #[test]
+    fn transparency_goes_in_the_sheet_mask_array_like_photoshop() {
+        // #8054: transparency in the user mask array (slot 24) made Photoshop refuse the file;
+        // Photoshop keeps it in the sheet mask array (slot 25).
+        for depth in [8, 16, 32] {
+            let p = pat(depth, true);
+            let mut block = write_pattern_block(std::slice::from_ref(&p)).unwrap();
+            let (slots, end) = vma_slots(&block, &p);
+            let written: Vec<usize> = slots.iter().enumerate().filter(|(_, s)| s.1).map(|(i, _)| i).collect();
+            assert_eq!((slots.len(), written), (26, vec![0, 1, 2, 25]), "{depth}-bit");
+            assert!(!block_has_user_mask(&block));
+            // The layout older PhotoCraft builds wrote still opens, and is detected.
+            block[slots[24].0..end].rotate_left(4);
+            let (slots, _) = vma_slots(&block, &p);
+            assert!(slots[24].1 && !slots[25].1);
+            assert!(block_has_user_mask(&block));
+            assert_eq!(parse_pattern_block(&block).unwrap(), vec![p]);
+        }
+        assert!(!block_has_user_mask(&write_pattern_block(&[pat(8, false)]).unwrap()));
+        for junk in [&[][..], &[0, 0, 0, 9, 1], &[0xff; 64]] {
+            assert!(!block_has_user_mask(junk));
+        }
+    }
+
     #[test]
     fn malformed_input_errors_without_panicking() {
         let blk = write_pattern_block(&[pat(8, true)]).unwrap();