Merge pull request #2749 from image-rs/upgrade-tiff-0.11

[v0.25] Upgrade tiff 0.11

Author: 197g

.github/workflows/rust.yml

@@ -39,14 +39,14 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        rust: ["1.85.0", nightly, beta]
+        rust: ["1.88.0", nightly, beta]
     steps:
     - uses: actions/checkout@v4
 
     - uses: dtolnay/rust-toolchain@nightly
-      if: ${{ matrix.rust == '1.85.0' }}
+      if: ${{ matrix.rust == '1.88.0' }}
     - name: Generate Cargo.lock with minimal-version dependencies
-      if: ${{ matrix.rust == '1.85.0' }}
+      if: ${{ matrix.rust == '1.88.0' }}
       run: |
         cargo -Zminimal-versions generate-lockfile
         cargo update --offline num-bigint --precise 0.4.2
@@ -61,7 +61,7 @@ jobs:
     - name: build
       run: cargo build -v
     - name: test
-      if: ${{ matrix.rust != '1.85.0' }}
+      if: ${{ matrix.rust != '1.88.0' }}
       run: cargo test -v && cargo doc -v
 
   test_other_archs:

Cargo.toml

@@ -5,7 +5,7 @@ edition = "2021"
 resolver = "2"
 
 # note: when changed, also update test runner in `.github/workflows/rust.yml`
-rust-version = "1.85.0"
+rust-version = "1.88.0"
 
 license = "MIT OR Apache-2.0"
 description = "Imaging library. Provides basic image processing and encoders/decoders for common image formats."
@@ -52,7 +52,7 @@ qoi = { version = "0.4", optional = true }
 ravif = { version = "0.12", default-features = false, optional = true }
 rayon = { version = "1.7.0", optional = true }
 rgb = { version = "0.8.48", default-features = false, optional = true }
-tiff = { version = "0.10.3", optional = true }
+tiff = { version = "0.11.2", optional = true }
 zune-core = { version = "0.5.0", default-features = false, optional = true }
 zune-jpeg = { version = "0.5.5", optional = true }
 serde = { version = "1.0.214", optional = true, features = ["derive"] }

src/codecs/avif/encoder.rs

@@ -188,7 +188,7 @@ impl<W: Write> AvifEncoder<W> {
                 Err(PodCastError::TargetAlignmentGreaterAndInputNotAligned) => {
                     // Sad, but let's allocate.
                     // bytemuck checks alignment _before_ slop but size mismatch before this..
-                    if buf.len() % size_of::<Channel>() != 0 {
+                    if !buf.len().is_multiple_of(size_of::<Channel>()) {
                         Err(ImageError::Parameter(ParameterError::from_kind(
                             ParameterErrorKind::DimensionMismatch,
                         )))

src/codecs/dxt.rs

@@ -80,7 +80,7 @@ impl<R: Read> DxtDecoder<R> {
         height: u32,
         variant: DxtVariant,
     ) -> Result<DxtDecoder<R>, ImageError> {
-        if width % 4 != 0 || height % 4 != 0 {
+        if !width.is_multiple_of(4) || !height.is_multiple_of(4) {
             // TODO: this is actually a bit of a weird case. We could return `DecodingError` but
             // it's not really the format that is wrong However, the encoder should surely return
             // `EncodingError` so it would be the logical choice for symmetry.
@@ -289,7 +289,7 @@ fn decode_dxt1_block(source: &[u8], dest: &mut [u8]) {
 /// Decode a row of DXT1 data to four rows of RGB data.
 /// `source.len()` should be a multiple of 8, otherwise this panics.
 fn decode_dxt1_row(source: &[u8], dest: &mut [u8]) {
-    assert!(source.len() % 8 == 0);
+    assert!(source.len().is_multiple_of(8));
     let block_count = source.len() / 8;
     assert!(dest.len() >= block_count * 48);
 
@@ -310,7 +310,7 @@ fn decode_dxt1_row(source: &[u8], dest: &mut [u8]) {
 /// Decode a row of DXT3 data to four rows of RGBA data.
 /// `source.len()` should be a multiple of 16, otherwise this panics.
 fn decode_dxt3_row(source: &[u8], dest: &mut [u8]) {
-    assert!(source.len() % 16 == 0);
+    assert!(source.len().is_multiple_of(16));
     let block_count = source.len() / 16;
     assert!(dest.len() >= block_count * 64);
 
@@ -331,7 +331,7 @@ fn decode_dxt3_row(source: &[u8], dest: &mut [u8]) {
 /// Decode a row of DXT5 data to four rows of RGBA data.
 /// `source.len()` should be a multiple of 16, otherwise this panics.
 fn decode_dxt5_row(source: &[u8], dest: &mut [u8]) {
-    assert!(source.len() % 16 == 0);
+    assert!(source.len().is_multiple_of(16));
     let block_count = source.len() / 16;
     assert!(dest.len() >= block_count * 64);
 

src/codecs/pnm/decoder.rs

@@ -786,7 +786,7 @@ impl Sample for PbmBit {
 
     fn bytelen(width: u32, height: u32, samples: u32) -> ImageResult<usize> {
         let count = width * samples;
-        let linelen = (count / 8) + u32::from((count % 8) != 0);
+        let linelen = (count / 8) + u32::from(!count.is_multiple_of(8));
         Ok((linelen * height) as usize)
     }
 

src/codecs/tiff.rs

@@ -33,6 +33,7 @@ where
 
     // We only use an Option here so we can call with_limits on the decoder without moving.
     inner: Option<Decoder<R>>,
+    buffer: DecodingResult,
 }
 
 impl<R> TiffDecoder<R>
@@ -56,21 +57,6 @@ where
             Err(other) => return Err(ImageError::from_tiff_decode(other)),
         }
 
-        let planar_config = inner
-            .find_tag(Tag::PlanarConfiguration)
-            .map(|res| res.and_then(|r| r.into_u16().ok()).unwrap_or_default())
-            .unwrap_or_default();
-
-        // Decode not supported for non Chunky Planar Configuration
-        if planar_config > 1 {
-            Err(ImageError::Unsupported(
-                UnsupportedError::from_format_and_kind(
-                    ImageFormat::Tiff.into(),
-                    UnsupportedErrorKind::GenericFeature(String::from("PlanarConfiguration = 2")),
-                ),
-            ))?;
-        }
-
         let color_type = match tiff_color_type {
             tiff::ColorType::Gray(1) => ColorType::L8,
             tiff::ColorType::Gray(8) => ColorType::L8,
@@ -118,6 +104,7 @@ where
             color_type,
             original_color_type,
             inner: Some(inner),
+            buffer: DecodingResult::U8(vec![]),
         })
     }
 
@@ -133,6 +120,66 @@ where
         };
         total_pixels.saturating_mul(bytes_per_pixel)
     }
+
+    /// Interleave planes in our `buffer` into `output`.
+    fn interleave_planes(
+        &mut self,
+        layout: tiff::decoder::BufferLayoutPreference,
+        output: &mut [u8],
+    ) -> ImageResult<()> {
+        if self.original_color_type != self.color_type.into() {
+            return Err(ImageError::Unsupported(
+                UnsupportedError::from_format_and_kind(
+                    ImageFormat::Tiff.into(),
+                    UnsupportedErrorKind::GenericFeature(
+                        "Planar TIFF with CMYK color type is not supported".to_string(),
+                    ),
+                ),
+            ));
+        }
+
+        // This only works if we and `tiff` agree on the layout, including the color type, of
+        // the sample matrix.
+        //
+        // TODO: triple buffer in the other case and fixup the planar layout independent of
+        // sample type. Problem description follows:
+        //
+        // That will suck since we can't call `interleave_planes` with a `ColorType` argument,
+        // Changing that parameter to `ExtendedColorType` is a can of worms, and exposing the
+        // underlying generic function is an optimization killer (we may want to help LLVM
+        // optimize this interleaving by SIMD). For LumaAlpha(1) colors we should do the bit
+        // expansion at the same time as interleaving to avoid wasting the memory traversal but
+        // expand-then-interleave is at least clear, albeit an extra buffer required. Meanwhile
+        // for `Cmyk8`/`Cmyk16` our output is smaller than the tiff buffer (4 samples to 3, or
+        // 5 to 4 if we had alpha) and not wanting multiple conversion function implementations
+        // we should interleave-then-expand?
+        //
+        // The hard part of the solution will be managing complexity.
+        let plane_stride = layout.plane_stride.map_or(0, |n| n.get());
+        let bytes = self.buffer.as_buffer(0);
+
+        let planes = bytes
+            .as_bytes()
+            .chunks_exact(plane_stride)
+            .collect::<Vec<_>>();
+
+        // Gracefully handle a mismatch of expectations. This should not occur in practice as we
+        // check that all planes have been read (see note on `read_image_to_buffer` usage below).
+        if planes.len() < usize::from(self.color_type.channel_count()) {
+            return Err(ImageError::Decoding(DecodingError::new(
+                ImageFormat::Tiff.into(),
+                "Not enough planes read from TIFF image".to_string(),
+            )));
+        }
+
+        utils::interleave_planes(
+            output,
+            self.color_type,
+            &planes[..usize::from(self.color_type.channel_count())],
+        );
+
+        Ok(())
+    }
 }
 
 fn check_sample_format(sample_format: u16, color_type: tiff::ColorType) -> Result<(), ImageError> {
@@ -319,15 +366,30 @@ impl<R: BufRead + Seek> ImageDecoder for TiffDecoder<R> {
         Ok(())
     }
 
-    fn read_image(self, buf: &mut [u8]) -> ImageResult<()> {
+    fn read_image(mut self, buf: &mut [u8]) -> ImageResult<()> {
         assert_eq!(u64::try_from(buf.len()), Ok(self.total_bytes()));
 
-        match self
+        let layout = self
             .inner
+            .as_mut()
             .unwrap()
-            .read_image()
-            .map_err(ImageError::from_tiff_decode)?
-        {
+            .read_image_to_buffer(&mut self.buffer)
+            .map_err(ImageError::from_tiff_decode)?;
+
+        // Check if we have all of the planes. Otherwise we ran into the allocation limit.
+        if self.buffer.as_buffer(0).as_bytes().len() < layout.complete_len {
+            return Err(ImageError::Limits(LimitError::from_kind(
+                LimitErrorKind::InsufficientMemory,
+            )));
+        }
+
+        if layout.planes > 1 {
+            // Note that we do not support planar layouts if we have to do conversion. Yet. See a
+            // more detailed comment in the implementation.
+            return self.interleave_planes(layout, buf);
+        }
+
+        match self.buffer {
             DecodingResult::U8(v) if self.original_color_type == ExtendedColorType::Cmyk8 => {
                 let mut out_cur = Cursor::new(buf);
                 for cmyk in v.chunks_exact(4) {
@@ -383,6 +445,7 @@ impl<R: BufRead + Seek> ImageDecoder for TiffDecoder<R> {
             }
             DecodingResult::F16(_) => unreachable!(),
         }
+
         Ok(())
     }
 

src/imageops/fast_blur.rs

@@ -128,7 +128,7 @@ fn boxes_for_gauss(sigma: f32, n: usize) -> Vec<usize> {
 
 #[inline]
 fn ceil_to_odd(x: usize) -> usize {
-    if x % 2 == 0 {
+    if x.is_multiple_of(2) {
         x + 1
     } else {
         x

src/imageops/sample.rs

@@ -1213,7 +1213,7 @@ impl GaussianBlurParameters {
     #[inline]
     fn round_to_nearest_odd(x: f32) -> u32 {
         let n = x.round() as u32;
-        if n % 2 != 0 {
+        if !n.is_multiple_of(2) {
             n
         } else {
             let lower = n - 1;
@@ -1237,7 +1237,7 @@ impl GaussianBlurParameters {
 
     fn kernel_size_from_sigma(sigma: f32) -> u32 {
         let possible_size = (((((sigma - 0.8) / 0.3) + 1.) * 2.) + 1.).max(3.) as u32;
-        if possible_size % 2 == 0 {
+        if possible_size.is_multiple_of(2) {
             return possible_size + 1;
         }
         possible_size

src/metadata/cicp.rs

@@ -1155,7 +1155,7 @@ impl CicpRgb {
     {
         use crate::traits::private::LayoutWithColor as Layout;
 
-        assert!(buffer.len() % from_layout.channels() == 0);
+        assert!(buffer.len().is_multiple_of(from_layout.channels()));
         let pixels = buffer.len() / from_layout.channels();
 
         let mut output: Vec<IntoSubpixel> = vec_try_with_capacity(pixels * into_layout.channels())

src/utils/mod.rs

@@ -40,7 +40,7 @@ pub(crate) fn expand_bits(bit_depth: u8, row_size: u32, buf: &[u8]) -> Vec<u8> {
     let mask = (1u8 << bit_depth as usize) - 1;
     let scaling_factor = 255 / ((1 << bit_depth as usize) - 1);
     let bit_width = row_size * u32::from(bit_depth);
-    let skip = if bit_width % 8 == 0 {
+    let skip = if bit_width.is_multiple_of(8) {
         0
     } else {
         (8 - bit_width % 8) / u32::from(bit_depth)
@@ -63,6 +63,53 @@ pub(crate) fn expand_bits(bit_depth: u8, row_size: u32, buf: &[u8]) -> Vec<u8> {
     p
 }
 
+#[inline(always)]
+pub(crate) fn interleave_planes(out: &mut [u8], color: crate::ColorType, planes: &[&[u8]]) {
+    #[track_caller]
+    pub(crate) fn trampoline<const PLANES: usize, const N: usize>(
+        out: &mut [u8],
+        planes: &[&[u8]],
+    ) {
+        interleave_planes_inner::<PLANES, N>(
+            out.as_chunks_mut::<N>().0,
+            <[_; PLANES]>::try_from(planes)
+                .unwrap()
+                .map(|arr| arr.as_chunks::<N>().0),
+        )
+    }
+
+    assert_eq!(planes.len(), usize::from(color.channel_count()));
+
+    match color {
+        crate::ColorType::L8 => trampoline::<1, 1>(out, planes),
+        crate::ColorType::La8 => trampoline::<2, 1>(out, planes),
+        crate::ColorType::Rgb8 => trampoline::<3, 1>(out, planes),
+        crate::ColorType::Rgba8 => trampoline::<4, 1>(out, planes),
+        crate::ColorType::L16 => trampoline::<1, 2>(out, planes),
+        crate::ColorType::La16 => trampoline::<2, 2>(out, planes),
+        crate::ColorType::Rgb16 => trampoline::<3, 2>(out, planes),
+        crate::ColorType::Rgba16 => trampoline::<4, 2>(out, planes),
+        crate::ColorType::Rgb32F => trampoline::<3, 4>(out, planes),
+        crate::ColorType::Rgba32F => trampoline::<4, 4>(out, planes),
+    }
+}
+
+#[inline(always)]
+fn interleave_planes_inner<const PLANES: usize, const N: usize>(
+    out: &mut [[u8; N]],
+    planes: [&[[u8; N]]; PLANES],
+) {
+    let mut iters = planes.map(|plane| plane.iter().copied());
+    for out in out.as_chunks_mut::<PLANES>().0 {
+        let vals = iters.each_mut().map(Iterator::next);
+
+        // I'd like to use array::zip once stable.
+        for i in 0..PLANES {
+            out[i] = vals[i].unwrap_or(out[i]);
+        }
+    }
+}
+
 /// Checks if the provided dimensions would cause an overflow.
 #[allow(dead_code)]
 // When no image formats that use it are enabled