From cfd9c1e20f3facadf81acc85a1afd0af6c6db25c Mon Sep 17 00:00:00 2001 From: AeRo Date: Fri, 9 Oct 2026 15:57:38 +0200 Subject: [PATCH] DNG 1.7 deinterleave - fix per review --- crates/raw/src/dng.rs | 102 ++++++++++++++++++++++++++++++++++-------- docs/parity.md | 2 +- 2 files changed, 84 insertions(+), 20 deletions(-) diff --git a/crates/raw/src/dng.rs b/crates/raw/src/dng.rs index 14aa356a6..1b07ac772 100644 --- a/crates/raw/src/dng.rs +++ b/crates/raw/src/dng.rs @@ -20,34 +20,57 @@ pub(crate) fn raw_ifd(tiff: &Tiff) -> Option<&Ifd> { } /// Undo the DNG 1.7 row/column interleave, which reorders the stored raster without changing its -/// size: with `RowInterleaveFactor = n` the rows are stored as `n` consecutive groups, group `g` +/// size: with `RowInterleaveFactor = n` the rows are stored as `n` consecutive fields, field `g` /// holding rows `g, g+n, g+2n, …` of the image (and likewise for columns). /// -/// Left alone, a file like this decodes as `n × m` half-scale copies of the scene, one per -/// quadrant, with the CFA pattern then read across permuted samples — so it is not merely -/// misplaced, it is the wrong colour as well (`RowInterleaveFactor = ColumnInterleaveFactor = 2` -/// is what DNG 1.7 JPEG XL files from Adobe's converter carry). +/// The tag counts fields, not a divisor of the raster — see [`field_map`] — so a dimension the +/// factor does not divide is decoded, with the trailing fields short, rather than refused. +/// +/// Left alone, a file like this decodes as `n × m` scaled-down copies of the scene, one per +/// field, with the CFA pattern then read across permuted samples — so it is not merely misplaced, +/// it is the wrong colour as well (`RowInterleaveFactor = ColumnInterleaveFactor = 2` is what DNG +/// 1.7 JPEG XL files from Adobe's converter carry). fn deinterleave(data: &mut RawData, w: usize, h: usize, cpp: usize, rows: usize, cols: usize) -> Result<()> { if rows <= 1 && cols <= 1 { return Ok(()); } + // The spec sets no bound on the value (it is only "the number of interleaved fields"); this + // caps the field table we build and the permutation, and rejects a hostile tag. if !(1..=16).contains(&rows) || !(1..=16).contains(&cols) { return Err(RawError::Unsupported(format!("interleave factors {rows} × {cols}"))); } - if !h.is_multiple_of(rows) || !w.is_multiple_of(cols) { - return Err(RawError::Corrupt(format!("interleave {rows}×{cols} does not divide {w}×{h}"))); - } - // Stored row `r` belongs to group `r / (h / rows)` at position `r % (h / rows)` within it, and - // that position holds image row `group + rows * position`. Same for columns. - let (gh, gw) = (h / rows, w / cols); - let at = |i: usize, n: usize, g: usize| (i % g) * n + i / g; - let dest = |r: usize, c: usize| at(r, rows, gh) * w + at(c, cols, gw); + let (map_r, map_c) = (field_map(h, rows), field_map(w, cols)); + let dest = |r: usize, c: usize| map_r[r] * w + map_c[c]; match data { RawData::U16(v) => permute(v, w, h, cpp, dest), RawData::F32(v) => permute(v, w, h, cpp, dest), } } +/// The stored-line → image-line map for one axis: a factor of `n` splits `len` lines into `n` +/// interleaved fields, field `g` holding lines `g, g+n, g+2n, …`. The fields are stored +/// consecutively, so field `g` starts at stored line `offs[g]` and its `k`-th line is image line +/// `g + n*k`. Field sizes follow from the round-robin split — field `g` has `ceil((len - g) / n)` +/// lines — so when `n` does not divide `len` the trailing fields are short, and when `n` exceeds +/// `len` the trailing fields are empty. +fn field_map(len: usize, n: usize) -> Vec { + let mut offs = Vec::with_capacity(n + 1); + let mut acc = 0usize; + for g in 0..=n { + offs.push(acc); + if g < n && g < len { + acc += (len - 1 - g) / n + 1; + } + } + let mut map = vec![0usize; len]; + for g in 0..n { + for (k, i) in (offs[g]..offs[g + 1]).enumerate() { + map[i] = g + n * k; + } + } + map +} + /// Move every `cpp`-sample pixel of `v` from its stored position to `dest(r, c)`, the image /// position it belongs at. The permutation is not its own inverse, so it needs a destination. fn permute usize>(v: &mut Vec, w: usize, h: usize, cpp: usize, dest: F) -> Result<()> { @@ -260,7 +283,8 @@ mod tests { /// Store `image` the way DNG 1.7 interleaves it: row `r` of the image is written to stored row /// `(r % rows) * (h / rows) + r / rows`, and columns likewise. This is the inverse of - /// [`deinterleave`]'s destination map, i.e. what a writer produces. + /// [`deinterleave`]'s destination map for a raster the factor divides, i.e. what a writer + /// produces (the short-trailing-field case is covered by the hand-written fixtures below). fn interleave(image: &[u16], w: usize, h: usize, rows: usize, cols: usize) -> Vec { let (gh, gw) = (h / rows, w / cols); let mut out = vec![0u16; w * h]; @@ -279,6 +303,19 @@ mod tests { } } + /// The image `image` laid out in stored order, given the image line each stored row and column + /// holds. Written out by hand from the tag's plain reading, so it does not share a formula with + /// the code under test. + fn store(image: &[u16], w: usize, rows_img: &[usize], cols_img: &[usize]) -> Vec { + let mut out = vec![0u16; image.len()]; + for (sr, &ir) in rows_img.iter().enumerate() { + for (sc, &ic) in cols_img.iter().enumerate() { + out[sr * w + sc] = image[ir * w + ic]; + } + } + out + } + #[test] fn deinterleaves_an_interleaved_raster() { // 3×3 rather than 2×2 on purpose: the 2×2 permutation is its own inverse, so a 2×2 fixture @@ -302,16 +339,43 @@ mod tests { } #[test] - fn deinterleave_is_a_no_op_at_one_and_refuses_a_bad_factor() { + fn deinterleave_pins_the_field_direction() { + // Hand-computed ground truth for the tag's plain reading ("field g holds lines g, g+n, + // g+2n, …"). The 3×3 test above round-trips against a helper written from the same reading + // of the tag, so it fixes consistency but not the direction; and a 2×2 fixture cannot tell + // the two directions apart, being its own inverse. These maps are written out literally so + // a reversed permutation fails here. 3 row fields of h = 9 start at stored rows 0, 3, 6 and + // hold image rows [0,3,6,1,4,7,2,5,8]; 2 column fields of w = 4 give [0,2,1,3]. + let (w, h) = (4, 9); + let image: Vec = (0..(w * h) as u16).collect(); + let mut data = RawData::U16(store(&image, w, &[0, 3, 6, 1, 4, 7, 2, 5, 8], &[0, 2, 1, 3])); + assert_ne!(samples(&data), image, "the fixture has to actually be permuted"); + deinterleave(&mut data, w, h, 1, 3, 2).unwrap(); + assert_eq!(samples(&data), image); + } + + #[test] + fn deinterleaves_a_raster_the_factor_does_not_divide() { + // The spec fixes the number of fields, not that the size is a multiple of it: h = 7 over 3 + // row fields is 3 + 2 + 2 (the round-robin split), so field 0 starts at stored row 0 holding + // image rows 0, 3, 6; field 1 at stored row 3 holding 1, 4; field 2 at stored row 5 holding + // 2, 5. Columns: w = 5 over 2 fields is 3 + 2 → image columns [0,2,4,1,3]. + let (w, h) = (5, 7); + let image: Vec = (0..(w * h) as u16).collect(); + let mut data = RawData::U16(store(&image, w, &[0, 3, 6, 1, 4, 2, 5], &[0, 2, 4, 1, 3])); + assert_ne!(samples(&data), image, "the fixture has to actually be permuted"); + deinterleave(&mut data, w, h, 1, 3, 2).unwrap(); + assert_eq!(samples(&data), image); + } + + #[test] + fn deinterleave_is_a_no_op_at_one_and_refuses_a_hostile_factor() { let image: Vec = (0..48).collect(); let mut plain = RawData::U16(image.clone()); deinterleave(&mut plain, 8, 6, 1, 1, 1).unwrap(); assert_eq!(samples(&plain), image); - // A factor that does not divide the raster is an error, not a scrambled image. - let mut bad = RawData::U16(image.clone()); - assert!(deinterleave(&mut bad, 8, 6, 1, 4, 1).is_err()); - // so is a hostile factor + // A hostile factor is an error, not a scrambled image. let mut hostile = RawData::U16(image); assert!(deinterleave(&mut hostile, 8, 6, 1, 1 << 20, 1).is_err()); } diff --git a/docs/parity.md b/docs/parity.md index 5fc7068b9..91a3cebaa 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -142,7 +142,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins. | LR-IMP-PROFILES | Import profiles & presets | P1 | ✅ | `cmd:file.importPresets`, `cmd:preset.import`, `cmd:profile.import`, `crates/engine/src/preset_import.rs`, `crates/engine/src/preset_luminar.rs` | presets: .lcpreset, XMP, classic .lrtemplate, photos carrying edits ("DNG presets"), Luminar looks (.lmp, .mplumpack collections; sliders with a counterpart), .zip bundles, folders (folder → group), drag & drop; masks carried over; unmapped settings reported (fields that only describe the preset or photo, e.g. `Cluster`, `SortName`, `SupportsAmount2`, the as-shot white, empty Point Color slots, are not); profiles: .cube 3D LUTs; Adobe profile formats deliberately unsupported | | LR-IMP-LOCAL | Work on files in place | P0 | ✅ | `cmd:library.browse`, `cmd:photo.addToLibrary`, `cmd:library.import` (mode add), `cmd:local.addRoot`, `cmd:local.hide`, `cmd:local.restoreHidden`, `crates/engine/src/cmd/browse.rs`, `crates/ui-egui/src/panels/left.rs` (`local_section`), `crates/ui-egui/src/panels/grid.rs` (`folder_header`) | Local: browse Pictures / Desktop / Downloads / Home or any folder without adding it (breadcrumb, Include subfolders, Add N to My Photos); browsed photos stay out of All Photos, albums and counts; folders picked with Browse Folder… (or Keep in Local / Add to Local on a folder row) stay listed across restarts, and browsing a folder inside a listed one highlights it inside that tree (opened down to it, siblings reachable) instead of adding a root; edits go to XMP sidecars; importing promotes them; right-click a top-level location → Remove from Local hides the shortcut only (nothing on disk or in the catalog changes), saved with the UI state, and “Show N hidden locations” (or Browse Folder… on it) brings it back; a folder is one location however its path is spelled (`/` or `\`, trailing separator, `.`/`..`, drive-letter case; `folder_key` in `crates/catalog/src/query.rs`); browsed photos nobody changed are forgotten once their folder has not been browsed for 30 days (Settings → Performance → Local folders, 0 = never; checked when the library opens, or `cmd:library.forgetLocal` with `dryRun`; files and sidecars stay, browsing again brings them back; see `crates/catalog/src/local.rs` for what counts as a change) | | LR-IMP-SIDECAR-SPLIT | Separate XMP sidecar variants | P2 | ⬜ | `cmd:library.xmpPreferences` | sidecar naming option exists (stem/full), no split sidecars | -| LR-IMP-FORMATS | Supported formats | P0 | 🟡 | `crates/codecs/src/lib.rs`, `crates/raw/src/lib.rs`, `crates/raw/src/jxl.rs` | JPEG, PNG, TIFF (1- to 64-bit integer, 16- to 64-bit float, incl. bilevel and CCITT fax), WebP, JXL, PSD, GIF, BMP; DNG (incl. lossy / Smart Preview DNG and DNG 1.7 JPEG XL tiles/strips: lossless ones decode bit-exactly on synthetic CFA and LinearRaw files, lossy ones decode but no real *lossy* JPEG XL DNG has verified them yet, and DNG 1.7 row/column-interleaved rasters are undone before the mosaic is read — `RowInterleaveFactor` / `ColumnInterleaveFactor` reorder the stored raster without changing its size, so without that step a file from Adobe's converter decodes as `n × m` half-scale copies of the scene, one per quadrant, in the wrong colours; verified on 4 real ILCE-6400 JPEG XL DNGs rendering bit-identical to their non-interleaved twins, with the permutation unit-tested at a factor whose inverse differs from itself; a single-chunk JPEG XL preview IFD is offered as the embedded preview), CR2 (colour-filter layout from the file's `CR2CFAPattern` tag, issue #85), ARW (pre-2017 bodies: white balance and black level from enciphered maker-note / `SR2SubIFD` data, issue #148; downsized Sony lossless YCbCr 4:2:0 / 4:2:2 tiles decoded to linear RGB, verified on private ILCE-7M4 3:2 & 4:3 examples including 16-bit full-resolution export), NEF (uncompressed, lossless and lossy compressed 12/14-bit, `crates/raw/src/vendor/nefc.rs`), RAF (uncompressed, lossless/lossy compressed Bayer and X-Trans; `crates/raw/src/vendor/rafc.rs`, full sensor verification on 13 bodies, `docs/raf-compression.md`), RW2 / Leica RWL / Panasonic RAW in every raw format (compressed 4 and 6, the prefix-coded strips of format 8 from the GH6 on, packed 2/5/7, the 16-bit words of the 2005–2007 bodies; black levels, in-camera aspect crops and mapped-out defect pixels; 178 CC0 files from 118 bodies checked, `crates/raw/src/vendor/rw2.rs`), PEF, ORF (uncompressed; colour-filter layout from the file's Exif `CFAPattern`). 12-bit NEF black level: maker note `0x003d` is in 14-bit units (D750, D780, D850, D7500, Z 50 checked). Lossy type 2 NEF curves of version `0x44 0x40` (D780, D6, Z 5, Z 6, Z 7, Z 7_2) span a quarter of the sample range, read from the table's own point spacing. CR3 lossless CRX Bayer and version 0x100/0x200 C-RAW decodes to sensor pixels with timed white balance, black/white levels and file crop (the shot's aspect ratio from the maker note's `AspectInfo`); full sensor samples match the reference on M50/R100/R8 (`crates/raw/tests/cr3_corpus.rs`, `docs/cr3.md`). Unsupported coding variants retain metadata and use the embedded JPEG when available; HEVC preview decoding remains unsupported. Missing: CR3 unverified CRX variants, compressed ORF, NEFs labelled compressed (34713) but stored uncompressed without a `0x0096` table (Z 6 packed 14-bit, D850 12-bit uncompressed), NEF "lossy after split" (preview only: imported with `preview_only` = the decoder's reason, rendered as a rendered JPEG, flagged by a grid badge and loupe/Edit/Info notices, `previewOnly` in `catalog.query`; Reload clears it once the file decodes), TIFF-based raws whose raw data sits in a private block (Phase One / Leaf IIQ, Canon EOS-1D / 1Ds and Kodak DCS TIFFs: IFD0 is only a reduced RGB image next to a much larger declared picture) are recognised from that structure and import as `preview_only` with the reason, instead of opening as a thumbnail-sized ordinary image (`thumbnail_shell` in `crates/raw/src/lib.rs`), Recognised raws LightCraft cannot decode open as `preview_only` with any usable embedded JPEG, also Canon CRW, Minolta MRW and Sigma X3F (containers recognised by magic, preview found by a structural JPEG scan), Kodak KDC (IFD0 without `ImageWidth`), Leaf MOS and Epson ERF (scan), and any decode failure of a recognised raw, not only an unsupported variant; ones with no JPEG still fail with "without an embedded preview", AVIF decode (HEIC decodes only behind the non-default `heif` feature: `crates/heif`, heic-rs, same policy as PhotoCraft; 8/10-bit, alpha, irot/imir/clap, ICC/EXIF/XMP), palette (indexed-colour) TIFF (refused by the `tiff` crate) | +| LR-IMP-FORMATS | Supported formats | P0 | 🟡 | `crates/codecs/src/lib.rs`, `crates/raw/src/lib.rs`, `crates/raw/src/jxl.rs` | JPEG, PNG, TIFF (1- to 64-bit integer, 16- to 64-bit float, incl. bilevel and CCITT fax), WebP, JXL, PSD, GIF, BMP; DNG (incl. lossy / Smart Preview DNG and DNG 1.7 JPEG XL tiles/strips: lossless ones decode bit-exactly on synthetic CFA and LinearRaw files, lossy ones decode but no real *lossy* JPEG XL DNG has verified them yet, and DNG 1.7 row/column-interleaved rasters are undone before the mosaic is read — `RowInterleaveFactor` / `ColumnInterleaveFactor` reorder the stored raster without changing its size, so without that step a file from Adobe's converter (whose 2×2 interleave makes four half-scale quadrants) decodes as `n × m` scaled-down copies of the scene in the wrong colours; the fields follow the tag's count, so a dimension the factor does not divide decodes with short trailing fields rather than being refused; verified on 4 real ILCE-6400 JPEG XL DNGs rendering bit-identical to their non-interleaved twins, with the permutation unit-tested against a hand-written stored→image map at a factor whose inverse differs from itself (factors above 2 rest on the plain reading of the tag — no real file with one has turned up); a single-chunk JPEG XL preview IFD is offered as the embedded preview), CR2 (colour-filter layout from the file's `CR2CFAPattern` tag, issue #85), ARW (pre-2017 bodies: white balance and black level from enciphered maker-note / `SR2SubIFD` data, issue #148; downsized Sony lossless YCbCr 4:2:0 / 4:2:2 tiles decoded to linear RGB, verified on private ILCE-7M4 3:2 & 4:3 examples including 16-bit full-resolution export), NEF (uncompressed, lossless and lossy compressed 12/14-bit, `crates/raw/src/vendor/nefc.rs`), RAF (uncompressed, lossless/lossy compressed Bayer and X-Trans; `crates/raw/src/vendor/rafc.rs`, full sensor verification on 13 bodies, `docs/raf-compression.md`), RW2 / Leica RWL / Panasonic RAW in every raw format (compressed 4 and 6, the prefix-coded strips of format 8 from the GH6 on, packed 2/5/7, the 16-bit words of the 2005–2007 bodies; black levels, in-camera aspect crops and mapped-out defect pixels; 178 CC0 files from 118 bodies checked, `crates/raw/src/vendor/rw2.rs`), PEF, ORF (uncompressed; colour-filter layout from the file's Exif `CFAPattern`). 12-bit NEF black level: maker note `0x003d` is in 14-bit units (D750, D780, D850, D7500, Z 50 checked). Lossy type 2 NEF curves of version `0x44 0x40` (D780, D6, Z 5, Z 6, Z 7, Z 7_2) span a quarter of the sample range, read from the table's own point spacing. CR3 lossless CRX Bayer and version 0x100/0x200 C-RAW decodes to sensor pixels with timed white balance, black/white levels and file crop (the shot's aspect ratio from the maker note's `AspectInfo`); full sensor samples match the reference on M50/R100/R8 (`crates/raw/tests/cr3_corpus.rs`, `docs/cr3.md`). Unsupported coding variants retain metadata and use the embedded JPEG when available; HEVC preview decoding remains unsupported. Missing: CR3 unverified CRX variants, compressed ORF, NEFs labelled compressed (34713) but stored uncompressed without a `0x0096` table (Z 6 packed 14-bit, D850 12-bit uncompressed), NEF "lossy after split" (preview only: imported with `preview_only` = the decoder's reason, rendered as a rendered JPEG, flagged by a grid badge and loupe/Edit/Info notices, `previewOnly` in `catalog.query`; Reload clears it once the file decodes), TIFF-based raws whose raw data sits in a private block (Phase One / Leaf IIQ, Canon EOS-1D / 1Ds and Kodak DCS TIFFs: IFD0 is only a reduced RGB image next to a much larger declared picture) are recognised from that structure and import as `preview_only` with the reason, instead of opening as a thumbnail-sized ordinary image (`thumbnail_shell` in `crates/raw/src/lib.rs`), Recognised raws LightCraft cannot decode open as `preview_only` with any usable embedded JPEG, also Canon CRW, Minolta MRW and Sigma X3F (containers recognised by magic, preview found by a structural JPEG scan), Kodak KDC (IFD0 without `ImageWidth`), Leaf MOS and Epson ERF (scan), and any decode failure of a recognised raw, not only an unsupported variant; ones with no JPEG still fail with "without an embedded preview", AVIF decode (HEIC decodes only behind the non-default `heif` feature: `crates/heif`, heic-rs, same policy as PhotoCraft; 8/10-bit, alpha, irot/imir/clap, ICC/EXIF/XMP), palette (indexed-colour) TIFF (refused by the `tiff` crate) | | LR-IMP-CAMERA-COVERAGE | Camera coverage, verified per model | P0 | 🟡 | `crates/raw/tests/corpus.rs`, `xtask/src/main.rs` | a supported container is not the same as every camera that writes it decoding correctly: ~55 CC0 corpus files plus 6 exact-sensor CR3 cases (plus ~60 NEFs checked by hand and 178 Panasonic / Leica RW2, RWL and RAW files from 118 bodies, checked against each file's own JPEG) against the >1,000 models Lightroom lists; per-model differences still surface (e.g. the CR2 colour-filter layout differed by model until it was read from the file's own tag, issue #85, checked on 35 CR2 bodies; pre-2017 Sony ARWs opened green until their scrambled white balance / black level were read, issue #148, checked on 16 ARWs from 15 Sony bodies; Canon PowerShot and EOS M CR2s whose `ColorData` is stored as bytes opened green until the bytes were read as 16-bit words, 25 files from 25 bodies checked). Compressed RAF sensor arrays verified on 13 Fujifilm bodies (`docs/raf-compression.md`). Grow the CC0 corpus per model and test each sample decodes with plausible colour. Nikon CropArea and untagged-preview colour-space regression tests; private Z 8 crop verified. | | LR-IMP-CULL-AT-IMPORT | Culling analysis at import | P2 | 🟡 | `cmd:photo.analyze` | run Assisted Culling on the imported photos (they're selected after an import); not automatic | | LR-IMP-MOVE | Move on import [Classic] | P1 | ✅ | `cmd:library.import` (`mode` move), `crates/engine/src/import_move.rs`, `crates/engine/src/tests_import_move.rs`, `crates/ui-egui/src/import.rs` | Transfer ▸ Move with Copy's destination, folders (day / month / one folder / custom template), rename template and example destination; XMP sidecars (both namings) move along; each source is removed only after its destination is written (hard link on the same volume, else copied, synced and compared byte for byte) and its catalog record is saved; failed, duplicate, unchecked files keep their sources, taken names get -1, -2…, never overwritten; files already in the destination/library are added in place; sources that can't be removed (read-only card) are kept and reported (`kept`). Undo removes the photos from the library but leaves the moved files at the destination. No Copy as DNG while moving |