decoder: reconstruct extended-depth samples modulo declared bit depth
Promoted extended-depth grayscale (9-15 bits -> u16, 17-31 bits -> u32) can carry bits above the declared depth after the horizontal predictor, which reconstructs prev+delta in the promoted storage width rather than modulo 2^BitsPerSample. This underflowed `max - v` in invert_colors (the image-rs/image-tiff#396 panic in debug, wrong pixel in release) and left junk in the high bits of the non-inverted output (#396 review, @197g: "clamp the channel values in the non-inverted color case"). An n-bit predictor delta is stored in exactly n bits, i.e. modulo 2^n, so reconstruction is defined modulo 2^n as well. Since 2^n divides the u16/u32 storage modulus, masking each promoted sample with (1<<n)-1 once after the predictor yields exactly the mod-2^n value -- including legitimate wrap-arounds -- and also bounds any out-of-range sample from malformed input. invert_colors' `max - v` is then always in range, superseding the targeted saturating_sub from #396. mask_samples_to_bit_depth runs in every integer decode branch of expand_chunk; outside the extended-depth unpacking branch it is a statically provable no-op (promoted layouts, declared < storage, occur only there -- sub-byte depths keep byte-granular storage and reject the horizontal predictor, standard depths have declared == storage, and the float predictor requires IEEEFP samples that are never promoted), so the branches cannot diverge. Adds exact-pixel regression tests on hand-built TIFFs: 12/20-bit wrap-around (plain and WhiteIsZero) and a 9-31-bit encode/decode identity sweep, alongside the two fuzzer-minimized panic seeds. A saturating clamp corrupts the wrap-around cases -- 12-bit [4095, 0, 1] is stored as deltas [4095, 1, 1] and masks back to [4095, 0, 1], but saturating produced [4095, 4095, 4095] -- so the tests lock the modular semantics.
This commit is contained in:
1 parent
a9cf5e17b9
commit
35c14f1e3c
5 files changed
+254
No files matched your search
@@ -1204,6 +1204,14 @@ impl Image {
|
||||
scratch,
|
||||
);
|
||||
|
||||
// An n-bit predictor delta is stored modulo 2^n, but the
|
||||
// reconstruction above accumulated it in the promoted u16/u32
|
||||
// storage width. Reduce back modulo the declared depth so both
|
||||
// the inverted and non-inverted paths see exact, in-range
|
||||
// values (2^n divides the storage modulus, so one mask after
|
||||
// the row is exact).
|
||||
super::mask_samples_to_bit_depth(out_row, tiff_bps, out_bps, self.sample_format);
|
||||
|
||||
if photometric_interpretation == PhotometricInterpretation::WhiteIsZero {
|
||||
super::invert_colors(out_row, color_type, self.sample_format)?;
|
||||
}
|
||||
@@ -1224,6 +1232,16 @@ impl Image {
|
||||
);
|
||||
}
|
||||
|
||||
// No-op in this branch: promoted layouts (declared depth < storage
|
||||
// width) are handled exclusively by the unpacking branch above.
|
||||
// Called in every integer branch so they cannot diverge.
|
||||
super::mask_samples_to_bit_depth(
|
||||
tile,
|
||||
layout.tiff_bits_per_sample,
|
||||
layout.output_bits_per_sample,
|
||||
self.sample_format,
|
||||
);
|
||||
|
||||
if photometric_interpretation == PhotometricInterpretation::WhiteIsZero {
|
||||
super::invert_colors(tile, color_type, self.sample_format)?;
|
||||
}
|
||||
@@ -1250,6 +1268,8 @@ impl Image {
|
||||
}
|
||||
_ => unreachable!(),
|
||||
}
|
||||
// No masking here: the floating point predictor requires
|
||||
// IEEEFP samples, which are never depth-promoted.
|
||||
if photometric_interpretation == PhotometricInterpretation::WhiteIsZero {
|
||||
super::invert_colors(row, color_type, self.sample_format)?;
|
||||
}
|
||||
@@ -1279,6 +1299,16 @@ impl Image {
|
||||
scratch,
|
||||
);
|
||||
|
||||
// No-op in this branch: promoted layouts only occur in the
|
||||
// unpacking branch. Called so the integer branches cannot
|
||||
// diverge.
|
||||
super::mask_samples_to_bit_depth(
|
||||
row,
|
||||
layout.tiff_bits_per_sample,
|
||||
layout.output_bits_per_sample,
|
||||
self.sample_format,
|
||||
);
|
||||
|
||||
if photometric_interpretation == PhotometricInterpretation::WhiteIsZero {
|
||||
super::invert_colors(row, color_type, self.sample_format)?;
|
||||
}
|
||||
@@ -1321,6 +1351,16 @@ impl Image {
|
||||
scratch,
|
||||
);
|
||||
|
||||
// No-op in this branch: promoted layouts only occur in the
|
||||
// unpacking branch. Called so the integer branches cannot
|
||||
// diverge.
|
||||
super::mask_samples_to_bit_depth(
|
||||
row,
|
||||
layout.tiff_bits_per_sample,
|
||||
layout.output_bits_per_sample,
|
||||
self.sample_format,
|
||||
);
|
||||
|
||||
if photometric_interpretation == PhotometricInterpretation::WhiteIsZero {
|
||||
super::invert_colors(row, color_type, self.sample_format)?;
|
||||
}
|
||||
|
||||
@@ -568,6 +568,56 @@ fn fix_endianness_and_predict(
|
||||
}
|
||||
}
|
||||
|
||||
/// Reduce promoted extended-depth samples (9–15 bits → `u16`, 17–31 bits →
|
||||
/// `u32`) modulo their declared bit depth, i.e. mask with `(1 << n) - 1`.
|
||||
///
|
||||
/// An n-bit horizontal-predicted sample stores its delta in exactly n bits,
|
||||
/// i.e. modulo `2^n`, so reconstruction is also defined modulo `2^n`. The
|
||||
/// predictor accumulates `prev + delta` in the promoted storage width
|
||||
/// (`u16`/`u32`, see `rev_hpredict_nsamp`); since `2^n` divides the storage
|
||||
/// modulus, masking once after the predictor yields exactly the mod-`2^n`
|
||||
/// value for every sample — including legitimate wrap-arounds (a 9-bit
|
||||
/// `511, 0` pair is stored as delta `1`, and `511 + 1 = 512` masks back to
|
||||
/// `0`). Masking also bounds any out-of-range sample in malformed input, so
|
||||
/// all downstream consumers (`invert_colors`' `max - v` and the non-inverted
|
||||
/// output path) see in-range values. For samples already in range it is a
|
||||
/// no-op.
|
||||
///
|
||||
/// This runs in every integer decode branch of `expand_chunk` so the
|
||||
/// branches cannot diverge. Outside the unpacking branch it reduces to the
|
||||
/// two guards below and touches nothing: promoted layouts (`declared <
|
||||
/// storage`) exist only there — standard depths have `declared == storage`,
|
||||
/// and sub-byte depths keep byte-granular storage.
|
||||
#[inline]
|
||||
fn mask_samples_to_bit_depth(
|
||||
buf: &mut [u8],
|
||||
declared_bits: u8,
|
||||
storage_bits: u8,
|
||||
sample_format: SampleFormat,
|
||||
) {
|
||||
// Only unsigned samples are promoted with zero-extension; signed
|
||||
// extended depths would need sign extension instead and are not
|
||||
// supported by the unpacking path.
|
||||
if sample_format != SampleFormat::Uint {
|
||||
return;
|
||||
}
|
||||
match storage_bits {
|
||||
16 if declared_bits < 16 => {
|
||||
let mask = (1u16 << declared_bits) - 1;
|
||||
for chunk in buf.as_chunks_mut::<2>().0 {
|
||||
*chunk = (u16::from_ne_bytes(*chunk) & mask).to_ne_bytes();
|
||||
}
|
||||
}
|
||||
32 if declared_bits < 32 => {
|
||||
let mask = (1u32 << declared_bits) - 1;
|
||||
for chunk in buf.as_chunks_mut::<4>().0 {
|
||||
*chunk = (u32::from_ne_bytes(*chunk) & mask).to_ne_bytes();
|
||||
}
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
||||
fn invert_colors(
|
||||
buf: &mut [u8],
|
||||
color_type: ColorType,
|
||||
|
||||
Reference in new issue
Block a user