From 65662c48348e2b92b021cf31df2f212691eb1c6d Mon Sep 17 00:00:00 2001 From: "A. Molzer" <5550310+197g@users.noreply.github.com> Date: Wed, 12 Nov 2025 23:19:15 +0100 Subject: [PATCH 01/19] Expand requested plane when reading chunk --- src/decoder/image.rs | 97 ++++++++++++++++++++++++++++++++++-- src/decoder/mod.rs | 116 ++++++++++++++++++------------------------- 2 files changed, 140 insertions(+), 73 deletions(-) diff --git a/src/decoder/image.rs b/src/decoder/image.rs index 05e82e1..a0d41eb 100644 --- a/src/decoder/image.rs +++ b/src/decoder/image.rs @@ -30,6 +30,23 @@ pub(crate) struct TileAttributes { pub tile_length: usize, } +pub(crate) struct PlaneLayout { + /// Total chunks per plane. + pub plane_chunks: usize, + /// Buffer offset from one row of the image to its next, in bytes. + pub output_row_stride: usize, + /// Buffer offset from the start of one chunk to the next within a row. + pub chunk_row_stride: usize, + /// Buffer offset per strip of chunks. + pub chunks_stride: usize, + /// Number of chunks in one strip of chunks. + pub chunks_across: usize, + /// Number of bytes in a plane. + pub plane_stride: usize, + /// Buffer offset from one plane of output to the next. + pub plane_offsets: Vec, +} + impl TileAttributes { pub fn tiles_across(&self) -> usize { self.image_width.div_ceil(self.tile_width) @@ -594,6 +611,77 @@ impl Image { } } + /// Get the layout for each output plane (i.e. one plane for non-planar tiff). + pub(crate) fn preferred_output_layout(&self) -> TiffResult { + let Image { width, height, .. } = *self; + + let chunk_dimensions = self.chunk_dimensions()?; + + let chunk_dimensions = ( + chunk_dimensions.0.min(width), + chunk_dimensions.1.min(height), + ); + + if chunk_dimensions.0 == 0 || chunk_dimensions.1 == 0 { + return Err(TiffError::FormatError( + TiffFormatError::InconsistentSizesEncountered, + )); + } + + let color = self.colortype()?; + let samples = self.samples_per_out_texel(color); + + let bits_per_sample = u64::from(self.bits_per_sample); + + let output_row_bits = (u64::from(width) * bits_per_sample) + .checked_mul(samples as u64) + .ok_or(TiffError::LimitsExceeded)?; + let output_row_stride: usize = output_row_bits.div_ceil(8).try_into()?; + + let chunk_row_bits = (chunk_dimensions.0 as u64 * bits_per_sample) + .checked_mul(samples as u64) + .ok_or(TiffError::LimitsExceeded)?; + let chunk_row_stride: usize = chunk_row_bits.div_ceil(8).try_into()?; + + let chunks_stride = output_row_bits + .div_ceil(8) + .checked_mul(u64::from(chunk_dimensions.1)) + .ok_or(TiffError::LimitsExceeded)? + .try_into()?; + + let plane_stride = output_row_bits + .div_ceil(8) + .checked_mul(u64::from(height)) + .ok_or(TiffError::LimitsExceeded)? + .try_into()?; + + let plane_offsets = (0..=usize::MAX) + .step_by(plane_stride) + .take(usize::from(color.num_samples() / samples)) + .collect(); + + let chunks_across: usize = ((width - 1) / chunk_dimensions.0 + 1).try_into()?; + + // We would not get an offset in byte units, sorry, no bit interleaving. + if chunks_across > 1 && chunk_row_bits % 8 != 0 { + return Err(TiffError::UnsupportedError( + TiffUnsupportedError::MisalignedTileBoundaries, + )); + } + + let plane_chunks = self.chunk_offsets.len() / self.strips_per_pixel(); + + Ok(PlaneLayout { + output_row_stride, + plane_chunks, + chunks_stride, + chunks_across, + chunk_row_stride, + plane_stride, + plane_offsets, + }) + } + pub(crate) fn chunk_data_dimensions(&self, chunk_index: u32) -> TiffResult<(u32, u32)> { let dims = self.chunk_dimensions()?; @@ -640,7 +728,6 @@ impl Image { } = reader; let byte_order = reader.byte_order; - let reader = reader.inner(); // Validate that the color type is supported. let color_type = self.colortype()?; @@ -740,17 +827,17 @@ impl Image { assert!(output_row_stride >= data_row_bytes); assert!(buf.len() >= output_row_stride * (data_dims.1 as usize - 1) + data_row_bytes); + let is_all_bits = samples == usize::from(data_samples); + let is_output_chunk_rows = output_row_stride == chunk_row_bytes; + let mut reader = Self::create_reader( - reader, + reader.inner(), compression_method, *compressed_bytes, self.jpeg_tables.as_deref().map(|a| &**a), chunk_dims, )?; - let is_all_bits = samples == usize::from(data_samples); - let is_output_chunk_rows = output_row_stride == chunk_row_bytes; - if is_output_chunk_rows && is_all_bits { // Here we can read directly into the output buffer itself. let tile = &mut buf[..chunk_row_bytes * data_dims.1 as usize]; diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs index 7b42575..2813da4 100644 --- a/src/decoder/mod.rs +++ b/src/decoder/mod.rs @@ -210,6 +210,10 @@ pub struct BufferLayoutPreference { pub len: usize, /// Minimum number of bytes to represent a row of image data of the requested content. pub row_stride: Option, + /// Number of planes in the image. + pub planes: usize, + /// Number of bytes used to represent one plane. + pub plane_stride: Option, } /// The count and matching discriminant for a `DecodingBuffer`. @@ -1101,15 +1105,22 @@ impl Decoder { chunk_index: u32, ) -> TiffResult { let data_dims = self.image().chunk_data_dimensions(chunk_index)?; + let layout = self .result_extent(data_dims.0 as usize, data_dims.1 as usize)? .preferred_layout()?; let row_stride = self.image.minimum_row_stride(data_dims); + let plane_stride = row_stride + .and_then(|v| v.get().checked_mul(data_dims.1 as usize)) + .and_then(core::num::NonZeroUsize::new); + Ok(BufferLayoutPreference { len: layout.size(), row_stride, + planes: 1, + plane_stride, }) } @@ -1183,17 +1194,13 @@ impl Decoder { /// indicate the layout needed to read the first data plane. This will be fixed in a future /// major version of `tiff`. pub fn image_buffer_layout(&mut self) -> TiffResult { - let data_dims @ (width, height) = (self.image.width, self.image.height); - - let layout = self - .result_extent(width as usize, height as usize)? - .preferred_layout()?; - - let row_stride = self.image.minimum_row_stride(data_dims); + let layout = self.image().preferred_output_layout()?; Ok(BufferLayoutPreference { - len: layout.size(), - row_stride, + len: layout.plane_stride, + row_stride: core::num::NonZeroUsize::new(layout.output_row_stride), + planes: layout.plane_offsets.len(), + plane_stride: core::num::NonZeroUsize::new(layout.plane_stride), }) } @@ -1224,73 +1231,46 @@ impl Decoder { /// When the image is stored as a planar configuration, this method will currently only read /// the first sample's plane. This will be fixed in a future major version of `tiff`. pub fn read_image_bytes(&mut self, buffer: &mut [u8]) -> TiffResult<()> { - let width = self.image().width; - let height = self.image().height; + let image::PlaneLayout { + plane_chunks, + output_row_stride, + chunk_row_stride, + chunks_stride, + chunks_across, + plane_stride, + plane_offsets, + } = self.image().preferred_output_layout()?; - let extent = self.result_extent(width as usize, height as usize)?; - extent.assert_layout(buffer.len())?; + // Find how many planes fit into the output buffer. + let used_plane_offsets = plane_offsets + .iter() + .enumerate() + // Find the first plane that would not fit completely at its offset. + .skip_while(|(_, &offset)| buffer.len().checked_sub(plane_stride) >= Some(offset)) + .nth(0) + // If all planes fit, use all of them. + .map_or(plane_offsets.len(), |(idx, _)| idx); - if width == 0 || height == 0 { - return Ok(()); - } - - let chunk_dimensions = self.image().chunk_dimensions()?; - let chunk_dimensions = ( - chunk_dimensions.0.min(width), - chunk_dimensions.1.min(height), - ); - - if chunk_dimensions.0 == 0 || chunk_dimensions.1 == 0 { - return Err(TiffError::FormatError( - TiffFormatError::InconsistentSizesEncountered, - )); - } - - let color = self.image().colortype()?; - let samples = self.image().samples_per_out_texel(color); - - if samples == 0 { - return Err(TiffError::FormatError( - TiffFormatError::InconsistentSizesEncountered, - )); - } - - let output_row_bits = (width as u64 * self.image.bits_per_sample as u64) - .checked_mul(samples as u64) - .ok_or(TiffError::LimitsExceeded)?; - let output_row_stride: usize = output_row_bits.div_ceil(8).try_into()?; - - let chunk_row_bits = (chunk_dimensions.0 as u64 * self.image.bits_per_sample as u64) - .checked_mul(samples as u64) - .ok_or(TiffError::LimitsExceeded)?; - let chunk_row_bytes: usize = chunk_row_bits.div_ceil(8).try_into()?; - - let chunks_across = ((width - 1) / chunk_dimensions.0 + 1) as usize; - - if chunks_across > 1 && chunk_row_bits % 8 != 0 { - return Err(TiffError::UnsupportedError( - TiffUnsupportedError::MisalignedTileBoundaries, - )); - } - - let image_chunks = self.image().chunk_offsets.len() / self.image().strips_per_pixel(); // For multi-band images, only the first band is read. // Possible improvements: // * pass requested band as parameter // * collect bands to a RGB encoding result in case of RGB bands - for chunk in 0..image_chunks { - self.goto_offset_u64(self.image().chunk_offsets[chunk])?; - + for chunk in 0..plane_chunks { let x = chunk % chunks_across; let y = chunk / chunks_across; - let buffer_offset = - y * output_row_stride * chunk_dimensions.1 as usize + x * chunk_row_bytes; - self.image.expand_chunk( - &mut self.value_reader, - &mut buffer[buffer_offset..], - output_row_stride, - chunk as u32, - )?; + let buffer_offset = y * chunks_stride + x * chunk_row_stride; + + for (idx, &plane_offset) in plane_offsets[..used_plane_offsets].iter().enumerate() { + let chunk = chunk + idx * plane_chunks; + self.goto_offset_u64(self.image().chunk_offsets[chunk])?; + + self.image.expand_chunk( + &mut self.value_reader, + &mut buffer[plane_offset..][buffer_offset..], + output_row_stride, + chunk as u32, + )?; + } } Ok(()) From 61b3027f38f49150e39baebaa1c0d8513e983ddd Mon Sep 17 00:00:00 2001 From: "A. Molzer" <5550310+197g@users.noreply.github.com> Date: Thu, 13 Nov 2025 23:38:15 +0100 Subject: [PATCH 02/19] Test reading planes of a whole image --- tests/decode_planes.rs | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 tests/decode_planes.rs diff --git a/tests/decode_planes.rs b/tests/decode_planes.rs new file mode 100644 index 0000000..fcaff01 --- /dev/null +++ b/tests/decode_planes.rs @@ -0,0 +1,31 @@ +use tiff::decoder::Decoder; + +use std::fs::File; +use std::path::PathBuf; + +const TEST_IMAGE_DIR: &str = "./tests/images/"; + +#[test] +fn read_all_planes() { + let path = PathBuf::from(TEST_IMAGE_DIR).join("planar-rgb-u8.tif"); + let file = File::open(path).unwrap(); + let io = std::io::BufReader::new(file); + + let mut tif = Decoder::new(io).unwrap(); + let layout = tif.image_buffer_layout().unwrap(); + assert_eq!(layout.planes, 3); + let plane_stride = layout.plane_stride.map_or(0, |x| x.get()); + + let mut buffer = vec![0u8; layout.planes * plane_stride]; + tif.read_image_bytes(&mut buffer).unwrap(); + + // Mainly you can see: these are different and non-zero. Otherwise these magic constants just + // depend on the file itself that is being used. + const PLANE_EXCPECTED_SUMS: [u32; 3] = [15417630, 13007788, 11103530]; + + for (plane, expected) in (0..layout.planes).zip(PLANE_EXCPECTED_SUMS) { + let plane_data = &buffer[plane_stride * plane..][..layout.len]; + let sum: u32 = plane_data.iter().copied().map(u32::from).sum(); + assert_eq!(sum, expected); + } +} From f9eaf9632a4cb152c129d3735857ddee6c2b68da Mon Sep 17 00:00:00 2001 From: "A. Molzer" <5550310+197g@users.noreply.github.com> Date: Mon, 17 Nov 2025 21:11:28 +0100 Subject: [PATCH 03/19] Expand PreferredLayout for planar information --- src/decoder/image.rs | 17 ++++++++++++++- src/decoder/mod.rs | 49 +++++++++++++++++++++++++++++++++++++----- tests/decode_planes.rs | 3 ++- 3 files changed, 62 insertions(+), 7 deletions(-) diff --git a/src/decoder/image.rs b/src/decoder/image.rs index a0d41eb..a0586c8 100644 --- a/src/decoder/image.rs +++ b/src/decoder/image.rs @@ -45,6 +45,8 @@ pub(crate) struct PlaneLayout { pub plane_stride: usize, /// Buffer offset from one plane of output to the next. pub plane_offsets: Vec, + /// Total number of bytes for all planes in given order. + pub total_bytes: usize, } impl TileAttributes { @@ -655,11 +657,23 @@ impl Image { .ok_or(TiffError::LimitsExceeded)? .try_into()?; - let plane_offsets = (0..=usize::MAX) + // Using the standard range iterator as checked_add on steroids. + let mut offset = 0..=usize::MAX; + + let plane_offsets = offset + .by_ref() + // Note: for supporting subsampling, adjust as required. .step_by(plane_stride) .take(usize::from(color.num_samples() / samples)) .collect(); + // Get the past-the-end of the last plane. + // + // This also verifies the `take` above was not short. + let Some(total_bytes) = offset.nth(plane_stride) else { + return Err(TiffError::LimitsExceeded); + }; + let chunks_across: usize = ((width - 1) / chunk_dimensions.0 + 1).try_into()?; // We would not get an offset in byte units, sorry, no bit interleaving. @@ -679,6 +693,7 @@ impl Image { chunk_row_stride, plane_stride, plane_offsets, + total_bytes, }) } diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs index 2813da4..2eb3b6d 100644 --- a/src/decoder/mod.rs +++ b/src/decoder/mod.rs @@ -205,6 +205,7 @@ impl<'a> DecodingBuffer<'a> { /// caller provided buffer must fit the expectations of the decoder to be filled with data from the /// current image. #[non_exhaustive] +#[derive(Debug, Clone)] pub struct BufferLayoutPreference { /// Minimum byte size of the buffer to read image data. pub len: usize, @@ -214,6 +215,8 @@ pub struct BufferLayoutPreference { pub planes: usize, /// Number of bytes used to represent one plane. pub plane_stride: Option, + /// Number of bytes of data when reading all planes. + pub complete_len: usize, } /// The count and matching discriminant for a `DecodingBuffer`. @@ -992,16 +995,47 @@ impl Decoder { let output_row_stride = (output_width as u64) .saturating_mul(self.image.samples_per_out_texel(color) as u64) .saturating_mul(self.image.bits_per_sample as u64) - .div_ceil(8); + .div_ceil(8) + .try_into()?; - self.read_chunk_to_bytes(buffer.as_bytes_mut(), chunk_index, output_row_stride) + self.read_chunk_to_bytes(buffer.as_bytes_mut(), chunk_index, output_row_stride)?; + + Ok(()) + } + + /// Read all planes from a specific tile. + /// + /// This behaves in one of two ways depending on the planar configuration of the image: + /// 1. For chunky images, this reads the specified chunk index as if by + /// [`Self::read_chunk_bytes`]. + /// 2. For planar images, this reads the chunks of each complete plane to appropriate offsets + /// in the output buffer. The size of the buffer determines the number of planes being read. + /// The `chunk_index` determines the index of the first plane being read. + pub fn read_plane_bytes(&mut self, chunk_index: u32, buffer: &mut [u8]) -> TiffResult<()> { + let layout = self.image().preferred_output_layout()?; + + let plane_offsets = [0usize]; + let used_plane_offsets = 1; + let plane_chunks = 0u32; + + for (idx, &plane_offset) in plane_offsets[..used_plane_offsets].iter().enumerate() { + let chunk_index = chunk_index + idx as u32 * plane_chunks; + + self.read_chunk_to_bytes( + &mut buffer[plane_offset..], + chunk_index, + layout.output_row_stride, + )?; + } + + Ok(()) } fn read_chunk_to_bytes( &mut self, buffer: &mut [u8], chunk_index: u32, - output_row_stride: u64, + output_row_stride: usize, ) -> TiffResult<()> { let offset = self.image.chunk_file_range(chunk_index)?.0; self.goto_offset_u64(offset)?; @@ -1009,7 +1043,7 @@ impl Decoder { self.image.expand_chunk( &mut self.value_reader, buffer, - output_row_stride.try_into()?, + output_row_stride, chunk_index, )?; @@ -1121,6 +1155,7 @@ impl Decoder { row_stride, planes: 1, plane_stride, + complete_len: layout.size(), }) } @@ -1159,7 +1194,8 @@ impl Decoder { let output_row_stride = u64::from(data_dims.0) .saturating_mul(self.image.samples_per_pixel() as u64) .saturating_mul(self.image.bits_per_sample as u64) - .div_ceil(8); + .div_ceil(8) + .try_into()?; self.read_chunk_to_bytes(buffer, chunk_index, output_row_stride)?; @@ -1201,6 +1237,7 @@ impl Decoder { row_stride: core::num::NonZeroUsize::new(layout.output_row_stride), planes: layout.plane_offsets.len(), plane_stride: core::num::NonZeroUsize::new(layout.plane_stride), + complete_len: layout.total_bytes, }) } @@ -1239,6 +1276,8 @@ impl Decoder { chunks_across, plane_stride, plane_offsets, + // We assume that is correct, so really it can be ignored. + total_bytes: _, } = self.image().preferred_output_layout()?; // Find how many planes fit into the output buffer. diff --git a/tests/decode_planes.rs b/tests/decode_planes.rs index fcaff01..5b221da 100644 --- a/tests/decode_planes.rs +++ b/tests/decode_planes.rs @@ -16,7 +16,8 @@ fn read_all_planes() { assert_eq!(layout.planes, 3); let plane_stride = layout.plane_stride.map_or(0, |x| x.get()); - let mut buffer = vec![0u8; layout.planes * plane_stride]; + eprintln!("{:?}", layout); + let mut buffer = vec![0u8; layout.complete_len]; tif.read_image_bytes(&mut buffer).unwrap(); // Mainly you can see: these are different and non-zero. Otherwise these magic constants just From 95f2d475fa14d4c599e2fb6f0f83b4624f392852 Mon Sep 17 00:00:00 2001 From: "A. Molzer" <5550310+197g@users.noreply.github.com> Date: Sun, 23 Nov 2025 19:51:26 +0100 Subject: [PATCH 04/19] Consolidate layout information type We currently have one type in decoder/image.rs and another in decoder/mod.rs which each contributes part of the computation for laying out the output buffer. Confusingly though they rely on shared implementation details such as determining the number of channels that are written and their bit depth. We should have one shared method and resulting struct instead. --- src/decoder/image.rs | 194 ++++++++++++++++++++++++++++++------------- src/decoder/mod.rs | 52 ++++++++---- 2 files changed, 170 insertions(+), 76 deletions(-) diff --git a/src/decoder/image.rs b/src/decoder/image.rs index a0586c8..3f6a4a1 100644 --- a/src/decoder/image.rs +++ b/src/decoder/image.rs @@ -107,6 +107,41 @@ pub(crate) struct Image { pub chunk_bytes: Vec, } +/// Describes how to read a tile-aligned portion of the image. +pub(crate) struct ReadoutLayout { + /// The planar configuration, which applies to both the underlying image and the output buffer. + /// This may be relaxed if we find a clean enough way to provide it. + pub planar_config: PlanarConfiguration, + + /// The sample interpretation (interpret with planar_config). + pub color: ColorType, + /// The number of bytes from one row to another. + pub minimum_row_stride: usize, + + /// Number of bytes to advance in output per row. + pub row_stride: usize, + /// Number of bytes to advance in output per chunk in width. + pub chunk_row_stride: usize, + /// Number of bytes to advance in output per chunk in height. + pub chunk_col_stride: usize, + /// Number of bytes in output from one plane to another. + pub plane_stride: usize, + + /// Bits per sample in the encoded data. + pub tiff_bits_per_sample: u8, + /// Number of samples in the encoded data. + pub tiff_samples: u16, + /// Dimensions of the underlying rectangular chunks (tile or strips). + pub tiff_chunk_dimensions: (u32, u32), + + /// Chunks until wrapping to the next row of chunks. + pub chunks_across: u32, + /// Chunks to advance to get to the next row of chunks. + pub chunks_stride: u32, + /// Chunks to advance to get to the next plane of chunks. + pub chunks_per_plane: u32, +} + impl Image { pub fn from_reader( decoder: &mut ValueReader, @@ -209,7 +244,7 @@ impl Image { // Technically bits_per_sample.len() should be *equal* to samples, but libtiff also allows // it to be a single value that applies to all samples. - if bits_per_sample.len() != samples.into() && bits_per_sample.len() != 1 { + if bits_per_sample.len() != usize::from(samples) && bits_per_sample.len() != 1 { return Err(TiffError::FormatError( TiffFormatError::InconsistentSizesEncountered, )); @@ -557,9 +592,9 @@ impl Image { /// Example with `bits_per_sample = [8, 8, 8]` and `PhotometricInterpretation::RGB`: /// * `PlanarConfiguration::Chunky` -> 3 (RGBRGBRGB...) /// * `PlanarConfiguration::Planar` -> 1 (RRR...) (GGG...) (BBB...) - pub(crate) fn samples_per_pixel(&self) -> usize { + pub(crate) fn samples_per_pixel(&self) -> u16 { match self.planar_config { - PlanarConfiguration::Chunky => self.samples.into(), + PlanarConfiguration::Chunky => self.samples, PlanarConfiguration::Planar => 1, } } @@ -572,10 +607,10 @@ impl Image { } /// Number of strips per pixel. - pub(crate) fn strips_per_pixel(&self) -> usize { + pub(crate) fn strips_per_pixel(&self) -> u16 { match self.planar_config { PlanarConfiguration::Chunky => 1, - PlanarConfiguration::Planar => self.samples.into(), + PlanarConfiguration::Planar => self.samples, } } @@ -616,65 +651,59 @@ impl Image { /// Get the layout for each output plane (i.e. one plane for non-planar tiff). pub(crate) fn preferred_output_layout(&self) -> TiffResult { let Image { width, height, .. } = *self; + self.preferred_output_layout_for(width, height) + } - let chunk_dimensions = self.chunk_dimensions()?; - - let chunk_dimensions = ( - chunk_dimensions.0.min(width), - chunk_dimensions.1.min(height), - ); - - if chunk_dimensions.0 == 0 || chunk_dimensions.1 == 0 { - return Err(TiffError::FormatError( - TiffFormatError::InconsistentSizesEncountered, - )); - } + pub(crate) fn preferred_output_layout_for( + &self, + width: u32, + height: u32, + ) -> TiffResult { + self.readout_for_size(width, height)?.to_plane_layout() + } + /// Get the layout for reading out a tile-aligned portion of the image. + /// + /// The provided width and height should be less than or equal to the image dimensions. + pub(crate) fn readout_for_size(&self, width: u32, height: u32) -> TiffResult { let color = self.colortype()?; - let samples = self.samples_per_out_texel(color); - let bits_per_sample = u64::from(self.bits_per_sample); + let tiff_samples = self.samples_per_pixel(); + let tiff_bits_per_sample = self.bits_per_sample; + let data_samples = self.samples_per_out_texel(color); + let tiff_chunk_dimensions = self.chunk_dimensions()?; + let strips_per_pixel = self.strips_per_pixel(); - let output_row_bits = (u64::from(width) * bits_per_sample) - .checked_mul(samples as u64) + let tiff_dimensions = (self.width, self.height); + let data_dimensions = (width, height); + + let chunk_row_bits = (u64::from(tiff_chunk_dimensions.0) * u64::from(tiff_bits_per_sample)) + .checked_mul(u64::from(tiff_samples)) .ok_or(TiffError::LimitsExceeded)?; - let output_row_stride: usize = output_row_bits.div_ceil(8).try_into()?; + let chunk_row_bytes: usize = chunk_row_bits.div_ceil(8).try_into()?; - let chunk_row_bits = (chunk_dimensions.0 as u64 * bits_per_sample) - .checked_mul(samples as u64) + let data_row_bits = (u64::from(data_dimensions.0) * u64::from(tiff_bits_per_sample)) + .checked_mul(u64::from(data_samples)) .ok_or(TiffError::LimitsExceeded)?; - let chunk_row_stride: usize = chunk_row_bits.div_ceil(8).try_into()?; + let data_row_bytes: usize = data_row_bits.div_ceil(8).try_into()?; - let chunks_stride = output_row_bits + let chunk_col_stride: usize = data_row_bits .div_ceil(8) - .checked_mul(u64::from(chunk_dimensions.1)) + .checked_mul(u64::from(tiff_chunk_dimensions.1)) .ok_or(TiffError::LimitsExceeded)? .try_into()?; - let plane_stride = output_row_bits + let plane_stride: usize = data_row_bits .div_ceil(8) - .checked_mul(u64::from(height)) + .checked_mul(u64::from(data_dimensions.1)) .ok_or(TiffError::LimitsExceeded)? .try_into()?; - // Using the standard range iterator as checked_add on steroids. - let mut offset = 0..=usize::MAX; + let minimum_row_stride = data_row_bytes; - let plane_offsets = offset - .by_ref() - // Note: for supporting subsampling, adjust as required. - .step_by(plane_stride) - .take(usize::from(color.num_samples() / samples)) - .collect(); - - // Get the past-the-end of the last plane. - // - // This also verifies the `take` above was not short. - let Some(total_bytes) = offset.nth(plane_stride) else { - return Err(TiffError::LimitsExceeded); - }; - - let chunks_across: usize = ((width - 1) / chunk_dimensions.0 + 1).try_into()?; + let chunks_across: u32 = data_dimensions.0.div_ceil(tiff_chunk_dimensions.0); + let chunks_stride: u32 = tiff_dimensions.0.div_ceil(tiff_chunk_dimensions.0); + let chunks_per_plane = (self.chunk_offsets.len() as u32) / u32::from(strips_per_pixel); // We would not get an offset in byte units, sorry, no bit interleaving. if chunks_across > 1 && chunk_row_bits % 8 != 0 { @@ -683,17 +712,20 @@ impl Image { )); } - let plane_chunks = self.chunk_offsets.len() / self.strips_per_pixel(); - - Ok(PlaneLayout { - output_row_stride, - plane_chunks, - chunks_stride, - chunks_across, - chunk_row_stride, + Ok(ReadoutLayout { + planar_config: self.planar_config, + color, + minimum_row_stride, + row_stride: data_row_bytes, + chunk_row_stride: chunk_row_bytes, + chunk_col_stride, plane_stride, - plane_offsets, - total_bytes, + tiff_bits_per_sample, + tiff_samples, + tiff_chunk_dimensions, + chunks_across, + chunks_stride, + chunks_per_plane, }) } @@ -702,9 +734,9 @@ impl Image { match self.chunk_type { ChunkType::Strip => { - let strip_attrs = self.strip_decoder.as_ref().unwrap(); - let strips_per_band = - self.height.saturating_sub(1) / strip_attrs.rows_per_strip + 1; + let rows_per_strip = dims.1; + let strips_per_band = self.height.div_ceil(rows_per_strip); + let strip_height_without_padding = (chunk_index % strips_per_band) .checked_mul(dims.1) .and_then(|x| self.height.checked_sub(x)) @@ -816,7 +848,7 @@ impl Image { let photometric_interpretation = self.photometric_interpretation; let predictor = self.predictor; - let samples = self.samples_per_pixel(); + let samples = usize::from(self.samples_per_pixel()); let data_samples = self.samples_per_out_texel(color_type); // We have two dimensions: the 2d rectangle of encoded data and the 2d rectangle this @@ -973,3 +1005,45 @@ impl Image { }); } } + +impl ReadoutLayout { + pub(crate) fn samples_per_out_texel(&self) -> u16 { + match self.planar_config { + PlanarConfiguration::Chunky => self.color.num_samples(), + PlanarConfiguration::Planar => 1, + } + } + + /// Reduce this down to the layout of the output planes. + pub(crate) fn to_plane_layout(&self) -> Result { + let num_planes = self.color.num_samples() / self.samples_per_out_texel(); + + // Using the standard range iterator as checked_add on steroids. + let mut offset = 0..=usize::MAX; + + let plane_offsets = offset + .by_ref() + // Note: for supporting subsampling, adjust as required. + .step_by(self.plane_stride) + .take(usize::from(num_planes)) + .collect(); + + // Get the past-the-end of the last plane. + // + // This also verifies the `take` above was not short. + let Some(total_bytes) = offset.nth(self.plane_stride) else { + return Err(TiffError::LimitsExceeded); + }; + + Ok(PlaneLayout { + plane_chunks: self.chunks_per_plane as usize, + output_row_stride: self.row_stride, + chunk_row_stride: self.chunk_row_stride, + chunks_stride: self.chunk_col_stride, + chunks_across: self.chunks_across.try_into()?, + plane_stride: self.plane_stride, + plane_offsets, + total_bytes, + }) + } +} diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs index 2eb3b6d..ac34e0f 100644 --- a/src/decoder/mod.rs +++ b/src/decoder/mod.rs @@ -1050,7 +1050,7 @@ impl Decoder { Ok(()) } - fn result_buffer(&self, width: usize, height: usize) -> TiffResult { + fn result_buffer(&self, width: u32, height: u32) -> TiffResult { self.result_extent(width, height)? .to_result_buffer(&self.value_reader.limits) } @@ -1058,29 +1058,30 @@ impl Decoder { // FIXME: it's unusual for this method to take a `usize` dimension parameter since those would // typically come from the image data. The preferred consistent representation should be `u32` // and that would avoid some unusual casts `usize -> u64` with a `u64::from` instead. - fn result_extent(&self, width: usize, height: usize) -> TiffResult { - let bits_per_sample = self.image.bits_per_sample; + fn result_extent(&self, width: u32, height: u32) -> TiffResult { + let color = self.image.color_or_fallback(); + + let samples = self.image.samples_per_out_texel(color); + let bits_per_sample = color.bit_depth(); // If samples take up more than one byte, we expand it to a full number (integer or float) // and the number of samples in that integer type just counts the number in the underlying // image itself. Otherwise, a row is represented as the byte slice of bitfields without // expansion. let row_samples = if bits_per_sample >= 8 { - width + u64::from(width) } else { - ((width as u64) * bits_per_sample as u64) + (u64::from(width) * bits_per_sample as u64) .div_ceil(8) .try_into() .map_err(|_| TiffError::LimitsExceeded)? }; - let color = self.image.color_or_fallback(); - let samples = self.image.samples_per_out_texel(color); - - let buffer_size = row_samples - .checked_mul(height) + let buffer_size: usize = row_samples + .checked_mul(u64::from(height)) .and_then(|x| x.checked_mul(samples.into())) - .ok_or(TiffError::LimitsExceeded)?; + .ok_or(TiffError::LimitsExceeded)? + .try_into()?; Ok(match self.image().sample_format { SampleFormat::Uint => match bits_per_sample { @@ -1141,7 +1142,7 @@ impl Decoder { let data_dims = self.image().chunk_data_dimensions(chunk_index)?; let layout = self - .result_extent(data_dims.0 as usize, data_dims.1 as usize)? + .result_extent(data_dims.0, data_dims.1)? .preferred_layout()?; let row_stride = self.image.minimum_row_stride(data_dims); @@ -1159,6 +1160,26 @@ impl Decoder { }) } + /// Return the layout preferred to read planes corresponding to the specified chunk. + /// + /// This is similar to [`Self::image_chunk_buffer_layout`] but iterates all planes with chunks + /// at the corresponding coordinates of the image. + /// + /// # Bugs + /// + /// Sub-sampled images are not yet supported properly. + pub fn image_plane_buffer_layout( + &mut self, + chunk_index: u32, + ) -> TiffResult { + match self.image().planar_config { + PlanarConfiguration::Chunky => return self.image_chunk_buffer_layout(chunk_index), + PlanarConfiguration::Planar => {} + } + + todo!() + } + /// Read the specified chunk (at index `chunk_index`) and return the binary data as a Vector. /// /// # Bugs @@ -1167,8 +1188,7 @@ impl Decoder { /// the first sample's plane. This will be fixed in a future major version of `tiff`. pub fn read_chunk(&mut self, chunk_index: u32) -> TiffResult { let data_dims = self.image().chunk_data_dimensions(chunk_index)?; - - let mut result = self.result_buffer(data_dims.0 as usize, data_dims.1 as usize)?; + let mut result = self.result_buffer(data_dims.0, data_dims.1)?; self.read_chunk_to_buffer(result.as_buffer(0), chunk_index, data_dims.0 as usize)?; @@ -1188,7 +1208,7 @@ impl Decoder { pub fn read_chunk_bytes(&mut self, chunk_index: u32, buffer: &mut [u8]) -> TiffResult<()> { let data_dims = self.image().chunk_data_dimensions(chunk_index)?; - let extent = self.result_extent(data_dims.0 as usize, data_dims.1 as usize)?; + let extent = self.result_extent(data_dims.0, data_dims.1)?; extent.assert_layout(buffer.len())?; let output_row_stride = u64::from(data_dims.0) @@ -1251,7 +1271,7 @@ impl Decoder { let width = self.image().width; let height = self.image().height; - let mut result = self.result_buffer(width as usize, height as usize)?; + let mut result = self.result_buffer(width, height)?; self.read_image_bytes(result.as_buffer(0).as_bytes_mut())?; Ok(result) From 58543962f341e8d4e7fe9967c94e1135b543bb88 Mon Sep 17 00:00:00 2001 From: "A. Molzer" <5550310+197g@users.noreply.github.com> Date: Sun, 23 Nov 2025 19:54:21 +0100 Subject: [PATCH 05/19] Add example that exercises decoding (no output) --- examples/decode.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 examples/decode.rs diff --git a/examples/decode.rs b/examples/decode.rs new file mode 100644 index 0000000..e40476c --- /dev/null +++ b/examples/decode.rs @@ -0,0 +1,19 @@ +use tiff::decoder::Decoder; + +fn main() -> Result<(), Box> { + let Some(image) = std::env::args_os().nth(1) else { + eprintln!("Usage: decode FILE"); + return Ok(()); + }; + + let file = std::fs::File::open(image)?; + let io = std::io::BufReader::new(file); + let mut reader = Decoder::new(io)?; + + while { + reader.read_image()?; + reader.more_images() + } {} + + Ok(()) +} From 5d3a5242eaae57b85b2696c4becb747d4907ab5b Mon Sep 17 00:00:00 2001 From: "A. Molzer" <5550310+197g@users.noreply.github.com> Date: Wed, 26 Nov 2025 19:18:27 +0100 Subject: [PATCH 06/19] Add plane selection to allocating result_extent --- src/decoder/image.rs | 13 ++-- src/decoder/mod.rs | 170 +++++++++++++++++++++++------------------- src/encoder/writer.rs | 8 +- src/error.rs | 3 + 4 files changed, 109 insertions(+), 85 deletions(-) diff --git a/src/decoder/image.rs b/src/decoder/image.rs index 3f6a4a1..3468b9f 100644 --- a/src/decoder/image.rs +++ b/src/decoder/image.rs @@ -1019,19 +1019,16 @@ impl ReadoutLayout { let num_planes = self.color.num_samples() / self.samples_per_out_texel(); // Using the standard range iterator as checked_add on steroids. - let mut offset = 0..=usize::MAX; + // + // Note: for supporting subsampling, adjust as required. + let mut offset = (0..=usize::MAX).step_by(self.plane_stride); - let plane_offsets = offset - .by_ref() - // Note: for supporting subsampling, adjust as required. - .step_by(self.plane_stride) - .take(usize::from(num_planes)) - .collect(); + let plane_offsets = offset.by_ref().take(usize::from(num_planes)).collect(); // Get the past-the-end of the last plane. // // This also verifies the `take` above was not short. - let Some(total_bytes) = offset.nth(self.plane_stride) else { + let Some(total_bytes) = offset.next() else { return Err(TiffError::LimitsExceeded); }; diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs index ac34e0f..07d9bdc 100644 --- a/src/decoder/mod.rs +++ b/src/decoder/mod.rs @@ -197,6 +197,10 @@ impl<'a> DecodingBuffer<'a> { DecodingBuffer::F64(buf) => bytecast::f64_as_ne_mut_bytes(buf), } } + + pub fn byte_len(&mut self) -> usize { + self.as_bytes_mut().len() + } } /// Information on the byte buffer that should be supplied to the decoder. @@ -219,6 +223,32 @@ pub struct BufferLayoutPreference { pub complete_len: usize, } +impl BufferLayoutPreference { + fn from_planes(layout: &image::PlaneLayout) -> Self { + BufferLayoutPreference { + len: layout.plane_stride, + row_stride: core::num::NonZeroUsize::new(layout.output_row_stride), + planes: layout.plane_offsets.len(), + plane_stride: core::num::NonZeroUsize::new(layout.plane_stride), + complete_len: layout.total_bytes, + } + } + + #[inline(always)] + fn assert_min_layout(self, buffer: &[T]) -> TiffResult<()> { + if core::mem::size_of_val(buffer) < self.len { + Err(TiffError::UsageError( + UsageError::InsufficientOutputBufferSize { + needed: self.len, + provided: buffer.len(), + }, + )) + } else { + Ok(()) + } + } +} + /// The count and matching discriminant for a `DecodingBuffer`. #[derive(Clone)] enum DecodingExtent { @@ -273,13 +303,14 @@ impl DecodingExtent { .map_err(overflow) } - fn assert_layout(self, buffer: usize) -> TiffResult<()> { + #[inline(always)] + fn assert_min_layout(self, buffer: &[T]) -> TiffResult<()> { let needed_bytes = self.preferred_layout()?.size(); - if buffer < needed_bytes { + if core::mem::size_of_val(buffer) < needed_bytes { Err(TiffError::UsageError( UsageError::InsufficientOutputBufferSize { needed: needed_bytes, - provided: buffer, + provided: buffer.len(), }, )) } else { @@ -1003,34 +1034,6 @@ impl Decoder { Ok(()) } - /// Read all planes from a specific tile. - /// - /// This behaves in one of two ways depending on the planar configuration of the image: - /// 1. For chunky images, this reads the specified chunk index as if by - /// [`Self::read_chunk_bytes`]. - /// 2. For planar images, this reads the chunks of each complete plane to appropriate offsets - /// in the output buffer. The size of the buffer determines the number of planes being read. - /// The `chunk_index` determines the index of the first plane being read. - pub fn read_plane_bytes(&mut self, chunk_index: u32, buffer: &mut [u8]) -> TiffResult<()> { - let layout = self.image().preferred_output_layout()?; - - let plane_offsets = [0usize]; - let used_plane_offsets = 1; - let plane_chunks = 0u32; - - for (idx, &plane_offset) in plane_offsets[..used_plane_offsets].iter().enumerate() { - let chunk_index = chunk_index + idx as u32 * plane_chunks; - - self.read_chunk_to_bytes( - &mut buffer[plane_offset..], - chunk_index, - layout.output_row_stride, - )?; - } - - Ok(()) - } - fn read_chunk_to_bytes( &mut self, buffer: &mut [u8], @@ -1051,44 +1054,54 @@ impl Decoder { } fn result_buffer(&self, width: u32, height: u32) -> TiffResult { - self.result_extent(width, height)? + self.result_extent(width, height, 0..1)? .to_result_buffer(&self.value_reader.limits) } // FIXME: it's unusual for this method to take a `usize` dimension parameter since those would // typically come from the image data. The preferred consistent representation should be `u32` // and that would avoid some unusual casts `usize -> u64` with a `u64::from` instead. - fn result_extent(&self, width: u32, height: u32) -> TiffResult { - let color = self.image.color_or_fallback(); - - let samples = self.image.samples_per_out_texel(color); - let bits_per_sample = color.bit_depth(); + // FIXME: when planes are not homogenous (i.e. subsampled or differing depths) then the + // `readout_for_size` or `to_plane_layout` needs a parameter to determine the planes being + // read instead of assuming a constant repeated size for them. + fn result_extent( + &self, + width: u32, + height: u32, + planes: core::ops::Range, + ) -> TiffResult { + let read = self.image.readout_for_size(width, height)?; + let bits_per_sample = read.color.bit_depth(); // If samples take up more than one byte, we expand it to a full number (integer or float) // and the number of samples in that integer type just counts the number in the underlying // image itself. Otherwise, a row is represented as the byte slice of bitfields without - // expansion. - let row_samples = if bits_per_sample >= 8 { - u64::from(width) - } else { - (u64::from(width) * bits_per_sample as u64) - .div_ceil(8) - .try_into() - .map_err(|_| TiffError::LimitsExceeded)? + // expansion. The output did that computation for us. + let buffer = read.to_plane_layout()?; + + // The layout is for all planes. So restrict ourselves to the planes that were requested. + let offset = match buffer.plane_offsets.get(usize::from(planes.start)) { + Some(n) => *n, + None => { + return Err(TiffError::UsageError(UsageError::InvalidPlaneIndex( + planes.start, + ))) + } }; - let buffer_size: usize = row_samples - .checked_mul(u64::from(height)) - .and_then(|x| x.checked_mul(samples.into())) - .ok_or(TiffError::LimitsExceeded)? - .try_into()?; + let end = match buffer.plane_offsets.get(usize::from(planes.end)) { + Some(n) => *n, + None => buffer.total_bytes, + }; + + let buffer_bytes = end - offset; Ok(match self.image().sample_format { SampleFormat::Uint => match bits_per_sample { - n if n <= 8 => DecodingExtent::U8(buffer_size), - n if n <= 16 => DecodingExtent::U16(buffer_size), - n if n <= 32 => DecodingExtent::U32(buffer_size), - n if n <= 64 => DecodingExtent::U64(buffer_size), + n if n <= 8 => DecodingExtent::U8(buffer_bytes), + n if n <= 16 => DecodingExtent::U16(buffer_bytes / 2), + n if n <= 32 => DecodingExtent::U32(buffer_bytes / 4), + n if n <= 64 => DecodingExtent::U64(buffer_bytes / 8), n => { return Err(TiffError::UnsupportedError( TiffUnsupportedError::UnsupportedBitsPerChannel(n), @@ -1096,9 +1109,9 @@ impl Decoder { } }, SampleFormat::IEEEFP => match bits_per_sample { - 16 => DecodingExtent::F16(buffer_size), - 32 => DecodingExtent::F32(buffer_size), - 64 => DecodingExtent::F64(buffer_size), + 16 => DecodingExtent::F16(buffer_bytes / 2), + 32 => DecodingExtent::F32(buffer_bytes / 4), + 64 => DecodingExtent::F64(buffer_bytes / 8), n => { return Err(TiffError::UnsupportedError( TiffUnsupportedError::UnsupportedBitsPerChannel(n), @@ -1106,10 +1119,10 @@ impl Decoder { } }, SampleFormat::Int => match bits_per_sample { - n if n <= 8 => DecodingExtent::I8(buffer_size), - n if n <= 16 => DecodingExtent::I16(buffer_size), - n if n <= 32 => DecodingExtent::I32(buffer_size), - n if n <= 64 => DecodingExtent::I64(buffer_size), + n if n <= 8 => DecodingExtent::I8(buffer_bytes), + n if n <= 16 => DecodingExtent::I16(buffer_bytes / 2), + n if n <= 32 => DecodingExtent::I32(buffer_bytes / 4), + n if n <= 64 => DecodingExtent::I64(buffer_bytes / 8), n => { return Err(TiffError::UnsupportedError( TiffUnsupportedError::UnsupportedBitsPerChannel(n), @@ -1142,7 +1155,7 @@ impl Decoder { let data_dims = self.image().chunk_data_dimensions(chunk_index)?; let layout = self - .result_extent(data_dims.0, data_dims.1)? + .result_extent(data_dims.0, data_dims.1, 0..1)? .preferred_layout()?; let row_stride = self.image.minimum_row_stride(data_dims); @@ -1177,7 +1190,11 @@ impl Decoder { PlanarConfiguration::Planar => {} } - todo!() + let data_dims = self.image().chunk_data_dimensions(chunk_index)?; + let layout = self + .image() + .preferred_output_layout_for(data_dims.0, data_dims.1)?; + Ok(BufferLayoutPreference::from_planes(&layout)) } /// Read the specified chunk (at index `chunk_index`) and return the binary data as a Vector. @@ -1208,8 +1225,8 @@ impl Decoder { pub fn read_chunk_bytes(&mut self, chunk_index: u32, buffer: &mut [u8]) -> TiffResult<()> { let data_dims = self.image().chunk_data_dimensions(chunk_index)?; - let extent = self.result_extent(data_dims.0, data_dims.1)?; - extent.assert_layout(buffer.len())?; + let extent = self.result_extent(data_dims.0, data_dims.1, 0..1)?; + extent.assert_min_layout(buffer)?; let output_row_stride = u64::from(data_dims.0) .saturating_mul(self.image.samples_per_pixel() as u64) @@ -1251,14 +1268,7 @@ impl Decoder { /// major version of `tiff`. pub fn image_buffer_layout(&mut self) -> TiffResult { let layout = self.image().preferred_output_layout()?; - - Ok(BufferLayoutPreference { - len: layout.plane_stride, - row_stride: core::num::NonZeroUsize::new(layout.output_row_stride), - planes: layout.plane_offsets.len(), - plane_stride: core::num::NonZeroUsize::new(layout.plane_stride), - complete_len: layout.total_bytes, - }) + Ok(BufferLayoutPreference::from_planes(&layout)) } /// Decodes the entire image and return it as a Vector @@ -1287,19 +1297,27 @@ impl Decoder { /// /// When the image is stored as a planar configuration, this method will currently only read /// the first sample's plane. This will be fixed in a future major version of `tiff`. + /// + /// # Error + /// + /// Returns an error if the buffer fits less than one plane. In particular, for non-planar + /// images returns an error if the buffer does not fit the required size. pub fn read_image_bytes(&mut self, buffer: &mut [u8]) -> TiffResult<()> { - let image::PlaneLayout { + let ref layout @ image::PlaneLayout { plane_chunks, output_row_stride, chunk_row_stride, chunks_stride, chunks_across, plane_stride, - plane_offsets, + ref plane_offsets, // We assume that is correct, so really it can be ignored. total_bytes: _, } = self.image().preferred_output_layout()?; + let preference = BufferLayoutPreference::from_planes(&layout); + preference.assert_min_layout(buffer)?; + // Find how many planes fit into the output buffer. let used_plane_offsets = plane_offsets .iter() @@ -1310,6 +1328,8 @@ impl Decoder { // If all planes fit, use all of them. .map_or(plane_offsets.len(), |(idx, _)| idx); + debug_assert!(used_plane_offsets >= 1, "Should have errored"); + // For multi-band images, only the first band is read. // Possible improvements: // * pass requested band as parameter diff --git a/src/encoder/writer.rs b/src/encoder/writer.rs index e3585cc..bd9c603 100644 --- a/src/encoder/writer.rs +++ b/src/encoder/writer.rs @@ -240,8 +240,12 @@ fn encoder_on_short_writes() { write.inner.set_position(0); let mut tiff_reader = crate::decoder::Decoder::new(&mut write.inner).unwrap(); - let data = tiff_reader.read_image().unwrap(); + let mut data = tiff_reader.read_image().unwrap(); - assert!(matches!(data, crate::decoder::DecodingResult::U8(v) if v == imgdata)); + assert!( + matches!(&data, crate::decoder::DecodingResult::U8(v) if *v == imgdata), + "{data:?} @ {}", + data.as_buffer(0).byte_len() + ); } } diff --git a/src/error.rs b/src/error.rs index e8977e2..7c45ce3 100644 --- a/src/error.rs +++ b/src/error.rs @@ -175,6 +175,9 @@ quick_error! { InvalidChunkIndex(index: u32) { display("invalid chunk index ({index}) requested") } + InvalidPlaneIndex(index: u16) { + display("invalid plane index ({index}) requested") + } PredictorCompressionMismatch { display("requested predictor is not compatible with the requested compression") } From dce1c40bd5af776bd20004e3010dbb9471ec2eb4 Mon Sep 17 00:00:00 2001 From: "A. Molzer" <5550310+197g@users.noreply.github.com> Date: Thu, 27 Nov 2025 12:08:30 +0100 Subject: [PATCH 07/19] Move layout responsibility into ReadoutLayout --- src/decoder/image.rs | 149 ++++++++++++++++++++++--------------------- src/decoder/mod.rs | 126 +++++++++++++++++------------------- src/error.rs | 3 + 3 files changed, 138 insertions(+), 140 deletions(-) diff --git a/src/decoder/image.rs b/src/decoder/image.rs index 3468b9f..2571b07 100644 --- a/src/decoder/image.rs +++ b/src/decoder/image.rs @@ -12,7 +12,6 @@ use crate::{ }; use std::io::{self, Cursor, Read, Seek}; -use std::num::NonZeroUsize; use std::sync::Arc; #[derive(Debug)] @@ -31,16 +30,8 @@ pub(crate) struct TileAttributes { } pub(crate) struct PlaneLayout { - /// Total chunks per plane. - pub plane_chunks: usize, /// Buffer offset from one row of the image to its next, in bytes. pub output_row_stride: usize, - /// Buffer offset from the start of one chunk to the next within a row. - pub chunk_row_stride: usize, - /// Buffer offset per strip of chunks. - pub chunks_stride: usize, - /// Number of chunks in one strip of chunks. - pub chunks_across: usize, /// Number of bytes in a plane. pub plane_stride: usize, /// Buffer offset from one plane of output to the next. @@ -133,11 +124,11 @@ pub(crate) struct ReadoutLayout { pub tiff_samples: u16, /// Dimensions of the underlying rectangular chunks (tile or strips). pub tiff_chunk_dimensions: (u32, u32), + /// Number of bytes in the underlying data with all samples per row of chunks. + pub tiff_row_bytes: usize, /// Chunks until wrapping to the next row of chunks. pub chunks_across: u32, - /// Chunks to advance to get to the next row of chunks. - pub chunks_stride: u32, /// Chunks to advance to get to the next plane of chunks. pub chunks_per_plane: u32, } @@ -464,38 +455,6 @@ impl Image { } } - /// Get the multiband color describing this with its extra samples. - pub(crate) fn color_multiband_with_extras(&self) -> ColorType { - ColorType::Multiband { - bit_depth: self.bits_per_sample, - num_samples: self.samples, - } - } - - /// Describe this with an accurate color or a multiband. - pub(crate) fn color_or_fallback(&self) -> ColorType { - self.colortype() - .unwrap_or_else(|_| self.color_multiband_with_extras()) - } - - pub(crate) fn minimum_row_stride(&self, dims: (u32, u32)) -> Option { - let (width, height) = dims; - - let color = self.color_or_fallback(); - - let row_stride = u64::from(width) - .saturating_mul(u64::from(self.samples_per_out_texel(color))) - .saturating_mul(u64::from(self.bits_per_sample)) - .div_ceil(8); - - // Note: row stride should be smaller than the len if we have an actual buffer. If there - // are no pixels in the buffer (height _or_ width is 0) then the stride is not well defined - // and we return `None`. - (height > 0) - .then_some(row_stride as usize) - .and_then(NonZeroUsize::new) - } - fn create_reader<'r, R: 'r + Read>( reader: R, compression_method: CompressionMethod, @@ -662,6 +621,11 @@ impl Image { self.readout_for_size(width, height)?.to_plane_layout() } + pub(crate) fn readout_for_image(&self) -> TiffResult { + let Image { width, height, .. } = *self; + self.readout_for_size(width, height) + } + /// Get the layout for reading out a tile-aligned portion of the image. /// /// The provided width and height should be less than or equal to the image dimensions. @@ -674,12 +638,16 @@ impl Image { let tiff_chunk_dimensions = self.chunk_dimensions()?; let strips_per_pixel = self.strips_per_pixel(); - let tiff_dimensions = (self.width, self.height); let data_dimensions = (width, height); - let chunk_row_bits = (u64::from(tiff_chunk_dimensions.0) * u64::from(tiff_bits_per_sample)) + let tiff_row_bits = (u64::from(tiff_chunk_dimensions.0) * u64::from(tiff_bits_per_sample)) .checked_mul(u64::from(tiff_samples)) .ok_or(TiffError::LimitsExceeded)?; + let tiff_row_bytes: usize = tiff_row_bits.div_ceil(8).try_into()?; + + let chunk_row_bits = (u64::from(tiff_chunk_dimensions.0) * u64::from(tiff_bits_per_sample)) + .checked_mul(u64::from(data_samples)) + .ok_or(TiffError::LimitsExceeded)?; let chunk_row_bytes: usize = chunk_row_bits.div_ceil(8).try_into()?; let data_row_bits = (u64::from(data_dimensions.0) * u64::from(tiff_bits_per_sample)) @@ -702,10 +670,9 @@ impl Image { let minimum_row_stride = data_row_bytes; let chunks_across: u32 = data_dimensions.0.div_ceil(tiff_chunk_dimensions.0); - let chunks_stride: u32 = tiff_dimensions.0.div_ceil(tiff_chunk_dimensions.0); let chunks_per_plane = (self.chunk_offsets.len() as u32) / u32::from(strips_per_pixel); - // We would not get an offset in byte units, sorry, no bit interleaving. + // We would not get an offset in byte units, sorry, no bit interleaving in the output. if chunks_across > 1 && chunk_row_bits % 8 != 0 { return Err(TiffError::UnsupportedError( TiffUnsupportedError::MisalignedTileBoundaries, @@ -723,8 +690,8 @@ impl Image { tiff_bits_per_sample, tiff_samples, tiff_chunk_dimensions, + tiff_row_bytes, chunks_across, - chunks_stride, chunks_per_plane, }) } @@ -765,7 +732,7 @@ impl Image { &self, reader: &mut ValueReader, buf: &mut [u8], - output_row_stride: usize, + layout: &ReadoutLayout, chunk_index: u32, ) -> TiffResult<()> { let ValueReader { @@ -777,7 +744,8 @@ impl Image { let byte_order = reader.byte_order; // Validate that the color type is supported. - let color_type = self.colortype()?; + let color_type = layout.color; + match color_type { ColorType::RGB(n) | ColorType::RGBA(n) @@ -848,8 +816,8 @@ impl Image { let photometric_interpretation = self.photometric_interpretation; let predictor = self.predictor; - let samples = usize::from(self.samples_per_pixel()); - let data_samples = self.samples_per_out_texel(color_type); + let samples = layout.tiff_samples; + let data_samples = layout.samples_per_out_texel(); // We have two dimensions: the 2d rectangle of encoded data and the 2d rectangle this // takes up in the output. Each has an associated count of bits per pixel. The first @@ -860,22 +828,15 @@ impl Image { let chunk_dims = self.chunk_dimensions()?; let data_dims = self.chunk_data_dimensions(chunk_index)?; - let chunk_row_bits = (u64::from(chunk_dims.0) * u64::from(self.bits_per_sample)) - .checked_mul(samples as u64) - .ok_or(TiffError::LimitsExceeded)?; - let chunk_row_bytes: usize = chunk_row_bits.div_ceil(8).try_into()?; - - let data_row_bits = (u64::from(data_dims.0) * u64::from(self.bits_per_sample)) - .checked_mul(data_samples as u64) - .ok_or(TiffError::LimitsExceeded)?; - let data_row_bytes: usize = data_row_bits.div_ceil(8).try_into()?; + let chunk_row_bytes: usize = layout.tiff_row_bytes; + let data_row_bytes: usize = layout.chunk_row_bytes(data_dims.0)?; // TODO: Should these return errors instead? - assert!(output_row_stride >= data_row_bytes); - assert!(buf.len() >= output_row_stride * (data_dims.1 as usize - 1) + data_row_bytes); + assert!(layout.minimum_row_stride >= data_row_bytes); + assert!(buf.len() >= layout.row_stride * (data_dims.1 as usize - 1) + data_row_bytes); - let is_all_bits = samples == usize::from(data_samples); - let is_output_chunk_rows = output_row_stride == chunk_row_bytes; + let is_all_bits = samples == data_samples; + let is_output_chunk_rows = layout.row_stride == chunk_row_bytes; let mut reader = Self::create_reader( reader.inner(), @@ -907,7 +868,7 @@ impl Image { // The floating point predictor shuffles the padding bytes into the encoded output, so // this case is handled specially when needed. let mut encoded = vec![0u8; chunk_row_bytes]; - for row in buf.chunks_mut(output_row_stride).take(data_dims.1 as usize) { + for row in buf.chunks_mut(layout.row_stride).take(data_dims.1 as usize) { reader.read_exact(&mut encoded)?; let row = &mut row[..data_row_bytes]; @@ -923,10 +884,14 @@ impl Image { } } else if is_all_bits { // We read row-by-row but each row fits in its output buffer. - for row in buf.chunks_mut(output_row_stride).take(data_dims.1 as usize) { + for row in buf.chunks_mut(layout.row_stride).take(data_dims.1 as usize) { let row = &mut row[..data_row_bytes]; - reader.read_exact(row)?; + let used = data_row_bytes.min(chunk_row_bytes); + // Two ways how we get here: we have more bytes in our chunk data than in the image + // we are to read. Then we need to skip the rest of the data. Or we have a bigger + // row stride than the chunk contains data, then we need to fill only the front. + reader.read_exact(&mut row[..used])?; // Skip horizontal padding if chunk_row_bytes > data_row_bytes { let len = u64::try_from(chunk_row_bytes - data_row_bytes)?; @@ -969,7 +934,7 @@ impl Image { let photo_range = photometric_bit_end / 8..bits_per_pixel / 8; let mut encoded = vec![0u8; chunk_row_bytes]; - for row in buf.chunks_mut(output_row_stride).take(data_dims.1 as usize) { + for row in buf.chunks_mut(layout.row_stride).take(data_dims.1 as usize) { reader.read_exact(&mut encoded)?; Self::compact_photometric_bytes(&mut encoded, row, &photo_range); @@ -1014,6 +979,48 @@ impl ReadoutLayout { } } + // For a concrete chunk, which may be a partial border chunk, the byte length of one row of its + // pixel data. + pub(crate) fn chunk_row_bytes(&self, width: u32) -> TiffResult { + let data_samples = self.samples_per_out_texel(); + let data_row_bits = (u64::from(width) * u64::from(self.tiff_bits_per_sample)) + .checked_mul(u64::from(data_samples)) + .ok_or(TiffError::LimitsExceeded)?; + Ok(data_row_bits.div_ceil(8).try_into()?) + } + + pub(crate) fn set_row_stride(&mut self, row_stride: usize) -> Result<(), TiffError> { + if row_stride < self.minimum_row_stride { + return Err(TiffError::UsageError( + UsageError::InsufficientOutputRowStride { + needed: self.minimum_row_stride, + requested: row_stride, + }, + )); + } + + let data_row_bytes = u64::try_from(row_stride)?; + + let chunk_col_stride = data_row_bytes + .checked_mul(u64::from(self.tiff_chunk_dimensions.1)) + .ok_or(TiffError::LimitsExceeded)? + .try_into()?; + + let height = self.plane_stride.checked_div(self.row_stride); + + let plane_stride = height + .and_then(|h| data_row_bytes.checked_mul(h as u64)) + // If height was zero, or the previous stride was zero, there are no bytes in a plane + .unwrap_or(0) + .try_into()?; + + self.row_stride = row_stride; + self.chunk_col_stride = chunk_col_stride; + self.plane_stride = plane_stride; + + Ok(()) + } + /// Reduce this down to the layout of the output planes. pub(crate) fn to_plane_layout(&self) -> Result { let num_planes = self.color.num_samples() / self.samples_per_out_texel(); @@ -1033,11 +1040,7 @@ impl ReadoutLayout { }; Ok(PlaneLayout { - plane_chunks: self.chunks_per_plane as usize, output_row_stride: self.row_stride, - chunk_row_stride: self.chunk_row_stride, - chunks_stride: self.chunk_col_stride, - chunks_across: self.chunks_across.try_into()?, plane_stride: self.plane_stride, plane_offsets, total_bytes, diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs index 07d9bdc..97edfbf 100644 --- a/src/decoder/mod.rs +++ b/src/decoder/mod.rs @@ -405,7 +405,7 @@ pub struct IfdDecoder<'lt> { inner: tag_reader::TagReader<'lt, dyn tag_reader::EntryDecoder + 'lt>, } -fn rev_hpredict_nsamp(buf: &mut [u8], bit_depth: u8, samples: usize) { +fn rev_hpredict_nsamp(buf: &mut [u8], bit_depth: u8, samples: u16) { fn one_byte_predict(buf: &mut [u8]) { for i in N..buf.len() { buf[i] = buf[i].wrapping_add(buf[i - N]); @@ -428,6 +428,8 @@ fn rev_hpredict_nsamp(buf: &mut [u8], bit_depth: u8, samples: usize) { } } + let samples = usize::from(samples); + match (bit_depth, samples) { // Note we can't use `windows` or so due to the overlap between each iteration. We split // the cases by the samples / lookback constant so that each is optimized individually. @@ -494,7 +496,9 @@ fn rev_hpredict_nsamp(buf: &mut [u8], bit_depth: u8, samples: usize) { } } -fn predict_f32(input: &mut [u8], output: &mut [u8], samples: usize) { +fn predict_f32(input: &mut [u8], output: &mut [u8], samples: u16) { + let samples = usize::from(samples); + for i in samples..input.len() { input[i] = input[i].wrapping_add(input[i - samples]); } @@ -509,7 +513,9 @@ fn predict_f32(input: &mut [u8], output: &mut [u8], samples: usize) { } } -fn predict_f16(input: &mut [u8], output: &mut [u8], samples: usize) { +fn predict_f16(input: &mut [u8], output: &mut [u8], samples: u16) { + let samples = usize::from(samples); + for i in samples..input.len() { input[i] = input[i].wrapping_add(input[i - samples]); } @@ -522,7 +528,9 @@ fn predict_f16(input: &mut [u8], output: &mut [u8], samples: usize) { } } -fn predict_f64(input: &mut [u8], output: &mut [u8], samples: usize) { +fn predict_f64(input: &mut [u8], output: &mut [u8], samples: u16) { + let samples = usize::from(samples); + for i in samples..input.len() { input[i] = input[i].wrapping_add(input[i - samples]); } @@ -544,7 +552,7 @@ fn predict_f64(input: &mut [u8], output: &mut [u8], samples: usize) { fn fix_endianness_and_predict( buf: &mut [u8], bit_depth: u8, - samples: usize, + samples: u16, byte_order: ByteOrder, predictor: Predictor, ) { @@ -1021,15 +1029,12 @@ impl Decoder { chunk_index: u32, output_width: usize, ) -> TiffResult<()> { - let color = self.image.color_or_fallback(); + let (width, height) = self.image().chunk_data_dimensions(chunk_index)?; - let output_row_stride = (output_width as u64) - .saturating_mul(self.image.samples_per_out_texel(color) as u64) - .saturating_mul(self.image.bits_per_sample as u64) - .div_ceil(8) - .try_into()?; + let mut layout = self.image().readout_for_size(width, height)?; + layout.set_row_stride(output_width)?; - self.read_chunk_to_bytes(buffer.as_bytes_mut(), chunk_index, output_row_stride)?; + self.read_chunk_to_bytes(buffer.as_bytes_mut(), chunk_index, &layout)?; Ok(()) } @@ -1038,26 +1043,17 @@ impl Decoder { &mut self, buffer: &mut [u8], chunk_index: u32, - output_row_stride: usize, + layout: &image::ReadoutLayout, ) -> TiffResult<()> { let offset = self.image.chunk_file_range(chunk_index)?.0; self.goto_offset_u64(offset)?; - self.image.expand_chunk( - &mut self.value_reader, - buffer, - output_row_stride, - chunk_index, - )?; + self.image + .expand_chunk(&mut self.value_reader, buffer, layout, chunk_index)?; Ok(()) } - fn result_buffer(&self, width: u32, height: u32) -> TiffResult { - self.result_extent(width, height, 0..1)? - .to_result_buffer(&self.value_reader.limits) - } - // FIXME: it's unusual for this method to take a `usize` dimension parameter since those would // typically come from the image data. The preferred consistent representation should be `u32` // and that would avoid some unusual casts `usize -> u64` with a `u64::from` instead. @@ -1066,11 +1062,9 @@ impl Decoder { // read instead of assuming a constant repeated size for them. fn result_extent( &self, - width: u32, - height: u32, + read: &image::ReadoutLayout, planes: core::ops::Range, ) -> TiffResult { - let read = self.image.readout_for_size(width, height)?; let bits_per_sample = read.color.bit_depth(); // If samples take up more than one byte, we expand it to a full number (integer or float) @@ -1153,16 +1147,12 @@ impl Decoder { chunk_index: u32, ) -> TiffResult { let data_dims = self.image().chunk_data_dimensions(chunk_index)?; + let readout = self.image().readout_for_size(data_dims.0, data_dims.1)?; - let layout = self - .result_extent(data_dims.0, data_dims.1, 0..1)? - .preferred_layout()?; + let layout = self.result_extent(&readout, 0..1)?.preferred_layout()?; - let row_stride = self.image.minimum_row_stride(data_dims); - - let plane_stride = row_stride - .and_then(|v| v.get().checked_mul(data_dims.1 as usize)) - .and_then(core::num::NonZeroUsize::new); + let row_stride = core::num::NonZeroUsize::new(readout.minimum_row_stride); + let plane_stride = core::num::NonZeroUsize::new(readout.plane_stride); Ok(BufferLayoutPreference { len: layout.size(), @@ -1201,13 +1191,19 @@ impl Decoder { /// /// # Bugs /// - /// When the image is stored as a planar configuration, this method will currently only read - /// the first sample's plane. This will be fixed in a future major version of `tiff`. + /// When the image is stored as a planar configuration, this method will currently only + /// correctly read the first sample's plane. This will be fixed in a future major version of + /// `tiff`. pub fn read_chunk(&mut self, chunk_index: u32) -> TiffResult { - let data_dims = self.image().chunk_data_dimensions(chunk_index)?; - let mut result = self.result_buffer(data_dims.0, data_dims.1)?; + let (width, height) = self.image().chunk_data_dimensions(chunk_index)?; - self.read_chunk_to_buffer(result.as_buffer(0), chunk_index, data_dims.0 as usize)?; + let readout = self.image().readout_for_size(width, height)?; + + let mut result = self + .result_extent(&readout, 0..1)? + .to_result_buffer(&self.value_reader.limits)?; + + self.read_chunk_to_bytes(result.as_buffer(0).as_bytes_mut(), chunk_index, &readout)?; Ok(result) } @@ -1223,18 +1219,13 @@ impl Decoder { /// When the image is stored as a planar configuration, this method will currently only read /// the first sample's plane. This will be fixed in a future major version of `tiff`. pub fn read_chunk_bytes(&mut self, chunk_index: u32, buffer: &mut [u8]) -> TiffResult<()> { - let data_dims = self.image().chunk_data_dimensions(chunk_index)?; + let (width, height) = self.image().chunk_data_dimensions(chunk_index)?; - let extent = self.result_extent(data_dims.0, data_dims.1, 0..1)?; + let layout = self.image().readout_for_size(width, height)?; + let extent = self.result_extent(&layout, 0..1)?; extent.assert_min_layout(buffer)?; - let output_row_stride = u64::from(data_dims.0) - .saturating_mul(self.image.samples_per_pixel() as u64) - .saturating_mul(self.image.bits_per_sample as u64) - .div_ceil(8) - .try_into()?; - - self.read_chunk_to_bytes(buffer, chunk_index, output_row_stride)?; + self.read_chunk_to_bytes(buffer, chunk_index, &layout)?; Ok(()) } @@ -1278,10 +1269,12 @@ impl Decoder { /// When the image is stored as a planar configuration, this method will currently only read /// the first sample's plane. This will be fixed in a future major version of `tiff`. pub fn read_image(&mut self) -> TiffResult { - let width = self.image().width; - let height = self.image().height; + let readout = self.image().readout_for_image()?; + + let mut result = self + .result_extent(&readout, 0..1)? + .to_result_buffer(&self.value_reader.limits)?; - let mut result = self.result_buffer(width, height)?; self.read_image_bytes(result.as_buffer(0).as_bytes_mut())?; Ok(result) @@ -1303,19 +1296,17 @@ impl Decoder { /// Returns an error if the buffer fits less than one plane. In particular, for non-planar /// images returns an error if the buffer does not fit the required size. pub fn read_image_bytes(&mut self, buffer: &mut [u8]) -> TiffResult<()> { + let readout = self.image().readout_for_image()?; + let ref layout @ image::PlaneLayout { - plane_chunks, - output_row_stride, - chunk_row_stride, - chunks_stride, - chunks_across, + output_row_stride: _, plane_stride, ref plane_offsets, // We assume that is correct, so really it can be ignored. total_bytes: _, - } = self.image().preferred_output_layout()?; + } = readout.to_plane_layout()?; - let preference = BufferLayoutPreference::from_planes(&layout); + let preference = BufferLayoutPreference::from_planes(layout); preference.assert_min_layout(buffer)?; // Find how many planes fit into the output buffer. @@ -1334,20 +1325,21 @@ impl Decoder { // Possible improvements: // * pass requested band as parameter // * collect bands to a RGB encoding result in case of RGB bands - for chunk in 0..plane_chunks { - let x = chunk % chunks_across; - let y = chunk / chunks_across; - let buffer_offset = y * chunks_stride + x * chunk_row_stride; + for chunk in 0..readout.chunks_per_plane { + let x = (chunk % readout.chunks_across) as usize; + let y = (chunk / readout.chunks_across) as usize; + + let buffer_offset = y * readout.chunk_col_stride + x * readout.chunk_row_stride; for (idx, &plane_offset) in plane_offsets[..used_plane_offsets].iter().enumerate() { - let chunk = chunk + idx * plane_chunks; - self.goto_offset_u64(self.image().chunk_offsets[chunk])?; + let chunk = chunk + idx as u32 * readout.chunks_per_plane; + self.goto_offset_u64(self.image().chunk_offsets[chunk as usize])?; self.image.expand_chunk( &mut self.value_reader, &mut buffer[plane_offset..][buffer_offset..], - output_row_stride, - chunk as u32, + &readout, + chunk, )?; } } diff --git a/src/error.rs b/src/error.rs index 7c45ce3..25de149 100644 --- a/src/error.rs +++ b/src/error.rs @@ -190,6 +190,9 @@ quick_error! { InsufficientOutputBufferSize { needed: usize, provided: usize } { display("the borrowed output buffer is not large enough for the decoded data, needed {needed} but have {provided}") } + InsufficientOutputRowStride { needed: usize, requested: usize } { + display("the provided output row stride would alias rows of decoded data, needed {needed} but have {requested}") + } ZeroIfdPointer { display("the offset 0 can not point to a valid IFD") } From 6439880e3db7c731e03d0f3b5f68d0df7280d82e Mon Sep 17 00:00:00 2001 From: "A. Molzer" <5550310+197g@users.noreply.github.com> Date: Thu, 27 Nov 2025 22:26:07 +0100 Subject: [PATCH 08/19] Simplify layout structs --- src/decoder/image.rs | 10 ++++------ src/decoder/mod.rs | 14 ++++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/decoder/image.rs b/src/decoder/image.rs index 2571b07..59046a9 100644 --- a/src/decoder/image.rs +++ b/src/decoder/image.rs @@ -30,10 +30,8 @@ pub(crate) struct TileAttributes { } pub(crate) struct PlaneLayout { - /// Buffer offset from one row of the image to its next, in bytes. - pub output_row_stride: usize, - /// Number of bytes in a plane. - pub plane_stride: usize, + /// The underlying readout layout. + pub readout: ReadoutLayout, /// Buffer offset from one plane of output to the next. pub plane_offsets: Vec, /// Total number of bytes for all planes in given order. @@ -99,6 +97,7 @@ pub(crate) struct Image { } /// Describes how to read a tile-aligned portion of the image. +#[derive(Clone)] pub(crate) struct ReadoutLayout { /// The planar configuration, which applies to both the underlying image and the output buffer. /// This may be relaxed if we find a clean enough way to provide it. @@ -1040,10 +1039,9 @@ impl ReadoutLayout { }; Ok(PlaneLayout { - output_row_stride: self.row_stride, - plane_stride: self.plane_stride, plane_offsets, total_bytes, + readout: self.clone(), }) } } diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs index 97edfbf..ae361ee 100644 --- a/src/decoder/mod.rs +++ b/src/decoder/mod.rs @@ -226,10 +226,10 @@ pub struct BufferLayoutPreference { impl BufferLayoutPreference { fn from_planes(layout: &image::PlaneLayout) -> Self { BufferLayoutPreference { - len: layout.plane_stride, - row_stride: core::num::NonZeroUsize::new(layout.output_row_stride), + len: layout.readout.plane_stride, + row_stride: core::num::NonZeroUsize::new(layout.readout.row_stride), planes: layout.plane_offsets.len(), - plane_stride: core::num::NonZeroUsize::new(layout.plane_stride), + plane_stride: core::num::NonZeroUsize::new(layout.readout.plane_stride), complete_len: layout.total_bytes, } } @@ -1299,22 +1299,24 @@ impl Decoder { let readout = self.image().readout_for_image()?; let ref layout @ image::PlaneLayout { - output_row_stride: _, - plane_stride, ref plane_offsets, // We assume that is correct, so really it can be ignored. total_bytes: _, + ref readout, } = readout.to_plane_layout()?; let preference = BufferLayoutPreference::from_planes(layout); preference.assert_min_layout(buffer)?; + // Note: with differently sized planes this is dependent on the plane. + let last_plane_start = buffer.len().checked_sub(readout.plane_stride); + // Find how many planes fit into the output buffer. let used_plane_offsets = plane_offsets .iter() .enumerate() // Find the first plane that would not fit completely at its offset. - .skip_while(|(_, &offset)| buffer.len().checked_sub(plane_stride) >= Some(offset)) + .skip_while(|(_, &offset)| last_plane_start >= Some(offset)) .nth(0) // If all planes fit, use all of them. .map_or(plane_offsets.len(), |(idx, _)| idx); From 0de1261948c5a7e2708bc3866e5b8a099dd90303 Mon Sep 17 00:00:00 2001 From: "A. Molzer" <5550310+197g@users.noreply.github.com> Date: Thu, 27 Nov 2025 22:37:37 +0100 Subject: [PATCH 09/19] Simplify responsibility of testing buffer size --- src/decoder/mod.rs | 105 +++++++++++++-------------------------------- 1 file changed, 29 insertions(+), 76 deletions(-) diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs index ae361ee..d7e8290 100644 --- a/src/decoder/mod.rs +++ b/src/decoder/mod.rs @@ -50,92 +50,60 @@ pub enum DecodingResult { } impl DecodingResult { - fn new_u8(size: usize, limits: &Limits) -> TiffResult { - if size > limits.decoding_buffer_size { + fn new( + size: usize, + limits: &Limits, + from_fn: fn(Vec) -> Self, + ) -> TiffResult { + if size > limits.decoding_buffer_size / core::mem::size_of::() { Err(TiffError::LimitsExceeded) } else { - Ok(DecodingResult::U8(vec![0; size])) + Ok(from_fn(vec![T::default(); size])) } } + fn new_u8(size: usize, limits: &Limits) -> TiffResult { + Self::new(size, limits, DecodingResult::U8) + } + fn new_u16(size: usize, limits: &Limits) -> TiffResult { - if size > limits.decoding_buffer_size / 2 { - Err(TiffError::LimitsExceeded) - } else { - Ok(DecodingResult::U16(vec![0; size])) - } + Self::new(size, limits, DecodingResult::U16) } fn new_u32(size: usize, limits: &Limits) -> TiffResult { - if size > limits.decoding_buffer_size / 4 { - Err(TiffError::LimitsExceeded) - } else { - Ok(DecodingResult::U32(vec![0; size])) - } + Self::new(size, limits, DecodingResult::U32) } fn new_u64(size: usize, limits: &Limits) -> TiffResult { - if size > limits.decoding_buffer_size / 8 { - Err(TiffError::LimitsExceeded) - } else { - Ok(DecodingResult::U64(vec![0; size])) - } + Self::new(size, limits, DecodingResult::U64) } fn new_f32(size: usize, limits: &Limits) -> TiffResult { - if size > limits.decoding_buffer_size / std::mem::size_of::() { - Err(TiffError::LimitsExceeded) - } else { - Ok(DecodingResult::F32(vec![0.0; size])) - } + Self::new(size, limits, DecodingResult::F32) } fn new_f64(size: usize, limits: &Limits) -> TiffResult { - if size > limits.decoding_buffer_size / std::mem::size_of::() { - Err(TiffError::LimitsExceeded) - } else { - Ok(DecodingResult::F64(vec![0.0; size])) - } + Self::new(size, limits, DecodingResult::F64) } fn new_f16(size: usize, limits: &Limits) -> TiffResult { - if size > limits.decoding_buffer_size / std::mem::size_of::() { - Err(TiffError::LimitsExceeded) - } else { - Ok(DecodingResult::F16(vec![f16::ZERO; size])) - } + Self::new(size, limits, DecodingResult::F16) } fn new_i8(size: usize, limits: &Limits) -> TiffResult { - if size > limits.decoding_buffer_size / std::mem::size_of::() { - Err(TiffError::LimitsExceeded) - } else { - Ok(DecodingResult::I8(vec![0; size])) - } + Self::new(size, limits, DecodingResult::I8) } fn new_i16(size: usize, limits: &Limits) -> TiffResult { - if size > limits.decoding_buffer_size / 2 { - Err(TiffError::LimitsExceeded) - } else { - Ok(DecodingResult::I16(vec![0; size])) - } + Self::new(size, limits, DecodingResult::I16) } fn new_i32(size: usize, limits: &Limits) -> TiffResult { - if size > limits.decoding_buffer_size / 4 { - Err(TiffError::LimitsExceeded) - } else { - Ok(DecodingResult::I32(vec![0; size])) - } + Self::new(size, limits, DecodingResult::I32) } fn new_i64(size: usize, limits: &Limits) -> TiffResult { - if size > limits.decoding_buffer_size / 8 { - Err(TiffError::LimitsExceeded) - } else { - Ok(DecodingResult::I64(vec![0; size])) - } + Self::new(size, limits, DecodingResult::I64) } pub fn as_buffer(&mut self, start: usize) -> DecodingBuffer<'_> { @@ -233,13 +201,15 @@ impl BufferLayoutPreference { complete_len: layout.total_bytes, } } +} +impl image::ReadoutLayout { #[inline(always)] - fn assert_min_layout(self, buffer: &[T]) -> TiffResult<()> { - if core::mem::size_of_val(buffer) < self.len { + fn assert_min_layout(&self, buffer: &[T]) -> TiffResult<()> { + if core::mem::size_of_val(buffer) < self.plane_stride { Err(TiffError::UsageError( UsageError::InsufficientOutputBufferSize { - needed: self.len, + needed: self.plane_stride, provided: buffer.len(), }, )) @@ -302,21 +272,6 @@ impl DecodingExtent { } .map_err(overflow) } - - #[inline(always)] - fn assert_min_layout(self, buffer: &[T]) -> TiffResult<()> { - let needed_bytes = self.preferred_layout()?.size(); - if core::mem::size_of_val(buffer) < needed_bytes { - Err(TiffError::UsageError( - UsageError::InsufficientOutputBufferSize { - needed: needed_bytes, - provided: buffer.len(), - }, - )) - } else { - Ok(()) - } - } } #[derive(Debug, Copy, Clone, PartialEq)] @@ -1222,8 +1177,7 @@ impl Decoder { let (width, height) = self.image().chunk_data_dimensions(chunk_index)?; let layout = self.image().readout_for_size(width, height)?; - let extent = self.result_extent(&layout, 0..1)?; - extent.assert_min_layout(buffer)?; + layout.assert_min_layout(buffer)?; self.read_chunk_to_bytes(buffer, chunk_index, &layout)?; @@ -1305,8 +1259,7 @@ impl Decoder { ref readout, } = readout.to_plane_layout()?; - let preference = BufferLayoutPreference::from_planes(layout); - preference.assert_min_layout(buffer)?; + layout.readout.assert_min_layout(buffer)?; // Note: with differently sized planes this is dependent on the plane. let last_plane_start = buffer.len().checked_sub(readout.plane_stride); @@ -1340,7 +1293,7 @@ impl Decoder { self.image.expand_chunk( &mut self.value_reader, &mut buffer[plane_offset..][buffer_offset..], - &readout, + readout, chunk, )?; } From 6fe21da979e95b4717e190002602d81d9128521b Mon Sep 17 00:00:00 2001 From: "A. Molzer" <5550310+197g@users.noreply.github.com> Date: Thu, 27 Nov 2025 22:45:13 +0100 Subject: [PATCH 10/19] Simplify layout proxy methods --- src/decoder/image.rs | 14 -------------- src/decoder/mod.rs | 7 +++++-- 2 files changed, 5 insertions(+), 16 deletions(-) diff --git a/src/decoder/image.rs b/src/decoder/image.rs index 59046a9..113a66c 100644 --- a/src/decoder/image.rs +++ b/src/decoder/image.rs @@ -606,20 +606,6 @@ impl Image { } } - /// Get the layout for each output plane (i.e. one plane for non-planar tiff). - pub(crate) fn preferred_output_layout(&self) -> TiffResult { - let Image { width, height, .. } = *self; - self.preferred_output_layout_for(width, height) - } - - pub(crate) fn preferred_output_layout_for( - &self, - width: u32, - height: u32, - ) -> TiffResult { - self.readout_for_size(width, height)?.to_plane_layout() - } - pub(crate) fn readout_for_image(&self) -> TiffResult { let Image { width, height, .. } = *self; self.readout_for_size(width, height) diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs index d7e8290..e303e01 100644 --- a/src/decoder/mod.rs +++ b/src/decoder/mod.rs @@ -1136,9 +1136,12 @@ impl Decoder { } let data_dims = self.image().chunk_data_dimensions(chunk_index)?; + let layout = self .image() - .preferred_output_layout_for(data_dims.0, data_dims.1)?; + .readout_for_size(data_dims.0, data_dims.1)? + .to_plane_layout()?; + Ok(BufferLayoutPreference::from_planes(&layout)) } @@ -1212,7 +1215,7 @@ impl Decoder { /// indicate the layout needed to read the first data plane. This will be fixed in a future /// major version of `tiff`. pub fn image_buffer_layout(&mut self) -> TiffResult { - let layout = self.image().preferred_output_layout()?; + let layout = self.image().readout_for_image()?.to_plane_layout()?; Ok(BufferLayoutPreference::from_planes(&layout)) } From c3882b252c9f43c79d21e0f11a2b2dee3095a750 Mon Sep 17 00:00:00 2001 From: "A. Molzer" <5550310+197g@users.noreply.github.com> Date: Thu, 27 Nov 2025 23:54:47 +0100 Subject: [PATCH 11/19] Cleanup documentation --- src/decoder/image.rs | 19 ++++++++++--------- src/decoder/mod.rs | 10 +--------- 2 files changed, 11 insertions(+), 18 deletions(-) diff --git a/src/decoder/image.rs b/src/decoder/image.rs index 113a66c..849550b 100644 --- a/src/decoder/image.rs +++ b/src/decoder/image.rs @@ -29,15 +29,6 @@ pub(crate) struct TileAttributes { pub tile_length: usize, } -pub(crate) struct PlaneLayout { - /// The underlying readout layout. - pub readout: ReadoutLayout, - /// Buffer offset from one plane of output to the next. - pub plane_offsets: Vec, - /// Total number of bytes for all planes in given order. - pub total_bytes: usize, -} - impl TileAttributes { pub fn tiles_across(&self) -> usize { self.image_width.div_ceil(self.tile_width) @@ -132,6 +123,16 @@ pub(crate) struct ReadoutLayout { pub chunks_per_plane: u32, } +/// A `ReadoutLayout` with pre-calculated plane information. +pub(crate) struct PlaneLayout { + /// The underlying readout layout. + pub readout: ReadoutLayout, + /// Buffer offset from one plane of output to the next. + pub plane_offsets: Vec, + /// Total number of bytes for all planes in given order. + pub total_bytes: usize, +} + impl Image { pub fn from_reader( decoder: &mut ValueReader, diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs index e303e01..3fa2d4a 100644 --- a/src/decoder/mod.rs +++ b/src/decoder/mod.rs @@ -1009,9 +1009,6 @@ impl Decoder { Ok(()) } - // FIXME: it's unusual for this method to take a `usize` dimension parameter since those would - // typically come from the image data. The preferred consistent representation should be `u32` - // and that would avoid some unusual casts `usize -> u64` with a `u64::from` instead. // FIXME: when planes are not homogenous (i.e. subsampled or differing depths) then the // `readout_for_size` or `to_plane_layout` needs a parameter to determine the planes being // read instead of assuming a constant repeated size for them. @@ -1020,12 +1017,6 @@ impl Decoder { read: &image::ReadoutLayout, planes: core::ops::Range, ) -> TiffResult { - let bits_per_sample = read.color.bit_depth(); - - // If samples take up more than one byte, we expand it to a full number (integer or float) - // and the number of samples in that integer type just counts the number in the underlying - // image itself. Otherwise, a row is represented as the byte slice of bitfields without - // expansion. The output did that computation for us. let buffer = read.to_plane_layout()?; // The layout is for all planes. So restrict ourselves to the planes that were requested. @@ -1044,6 +1035,7 @@ impl Decoder { }; let buffer_bytes = end - offset; + let bits_per_sample = read.color.bit_depth(); Ok(match self.image().sample_format { SampleFormat::Uint => match bits_per_sample { From 5b9dc309194a55de67678df14b9c4b6deb8373cb Mon Sep 17 00:00:00 2001 From: "A. Molzer" <5550310+197g@users.noreply.github.com> Date: Wed, 10 Dec 2025 21:59:03 +0100 Subject: [PATCH 12/19] Remove badly defined layout method --- src/decoder/mod.rs | 27 --------------------------- 1 file changed, 27 deletions(-) diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs index 3fa2d4a..91d2592 100644 --- a/src/decoder/mod.rs +++ b/src/decoder/mod.rs @@ -1110,33 +1110,6 @@ impl Decoder { }) } - /// Return the layout preferred to read planes corresponding to the specified chunk. - /// - /// This is similar to [`Self::image_chunk_buffer_layout`] but iterates all planes with chunks - /// at the corresponding coordinates of the image. - /// - /// # Bugs - /// - /// Sub-sampled images are not yet supported properly. - pub fn image_plane_buffer_layout( - &mut self, - chunk_index: u32, - ) -> TiffResult { - match self.image().planar_config { - PlanarConfiguration::Chunky => return self.image_chunk_buffer_layout(chunk_index), - PlanarConfiguration::Planar => {} - } - - let data_dims = self.image().chunk_data_dimensions(chunk_index)?; - - let layout = self - .image() - .readout_for_size(data_dims.0, data_dims.1)? - .to_plane_layout()?; - - Ok(BufferLayoutPreference::from_planes(&layout)) - } - /// Read the specified chunk (at index `chunk_index`) and return the binary data as a Vector. /// /// # Bugs From 1f02ff8ca1e761e2b34a9512a92d4d1301ff3316 Mon Sep 17 00:00:00 2001 From: "A. Molzer" <5550310+197g@users.noreply.github.com> Date: Thu, 11 Dec 2025 00:13:26 +0100 Subject: [PATCH 13/19] Add method to retrieve planes of a tile This is well-defined even if we add sub-sampling support later on. We can adjust the layout type accordingly with factors per each plane. Note that the layout, in theory, does not depend on the index of the unit if we describe all planes for a multi-planar layout. The specification requires that the subsampling factors divide the tile sizes. However we have to handle non-planar images here and this leaves us open to other potential parameters. --- src/decoder/image.rs | 49 ++++++++++++++---- src/decoder/mod.rs | 109 +++++++++++++++++++++++++++++++++++------ src/error.rs | 3 ++ tests/decode_planes.rs | 46 ++++++++++++++++- 4 files changed, 181 insertions(+), 26 deletions(-) diff --git a/src/decoder/image.rs b/src/decoder/image.rs index 849550b..1845f80 100644 --- a/src/decoder/image.rs +++ b/src/decoder/image.rs @@ -123,16 +123,6 @@ pub(crate) struct ReadoutLayout { pub chunks_per_plane: u32, } -/// A `ReadoutLayout` with pre-calculated plane information. -pub(crate) struct PlaneLayout { - /// The underlying readout layout. - pub readout: ReadoutLayout, - /// Buffer offset from one plane of output to the next. - pub plane_offsets: Vec, - /// Total number of bytes for all planes in given order. - pub total_bytes: usize, -} - impl Image { pub fn from_reader( decoder: &mut ValueReader, @@ -1032,3 +1022,42 @@ impl ReadoutLayout { }) } } + +/// A `ReadoutLayout` with pre-calculated plane information. +pub(crate) struct PlaneLayout { + /// The underlying readout layout. + pub readout: ReadoutLayout, + /// Buffer offset from one plane of output to the next. + pub plane_offsets: Vec, + /// Total number of bytes for all planes in given order. + pub total_bytes: usize, +} + +impl PlaneLayout { + /// Return the number of planes to extract into the provided buffer. + pub(crate) fn used_planes(&self, buffer: &[impl Sized]) -> TiffResult { + self.readout.assert_min_layout(buffer)?; + let buffer_len = core::mem::size_of_val(buffer); + + // Note: with differently sized planes this is dependent on the plane. + let last_plane_start = buffer_len.checked_sub(self.readout.plane_stride); + + // Find how many planes fit into the output buffer. + let used_plane_offsets = self + .plane_offsets + .iter() + .enumerate() + // Find the first plane that would not fit completely at its offset. + .skip_while(|(_, &offset)| last_plane_start >= Some(offset)) + .nth(0) + // If all planes fit, use all of them. + .map_or(self.plane_offsets.len(), |(idx, _)| idx); + + debug_assert!( + used_plane_offsets <= usize::from(u16::MAX), + "Planes limited by number of samples, which is encoded as u16" + ); + + Ok(used_plane_offsets as u16) + } +} diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs index 91d2592..2c4a8ac 100644 --- a/src/decoder/mod.rs +++ b/src/decoder/mod.rs @@ -22,6 +22,10 @@ mod image; mod stream; mod tag_reader; +/// An index referring to a (rectangular) region of an image. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +pub struct TiffCodingUnit(pub u32); + /// Result of a decoding process #[derive(Debug)] pub enum DecodingResult { @@ -1110,6 +1114,40 @@ impl Decoder { }) } + /// Return the layout preferred to read several planes corresponding to the specified region. + /// + /// This is similar to [`Self::image_chunk_buffer_layout`] but can read chunks from all planes + /// at the corresponding coordinates of the image. + /// + /// # Bugs + /// + /// Sub-sampled images are not yet supported properly. + pub fn image_coding_unit_layout( + &mut self, + code_unit: TiffCodingUnit, + ) -> TiffResult { + match self.image().planar_config { + PlanarConfiguration::Chunky => return self.image_chunk_buffer_layout(code_unit.0), + PlanarConfiguration::Planar => {} + } + + let (width, height) = self.image().chunk_data_dimensions(code_unit.0)?; + + let layout = self + .image() + .readout_for_size(width, height)? + .to_plane_layout()?; + + if code_unit.0 >= layout.readout.chunks_per_plane { + return Err(TiffError::UsageError(UsageError::InvalidCodingUnit( + code_unit.0, + layout.readout.chunks_per_plane, + ))); + } + + Ok(BufferLayoutPreference::from_planes(&layout)) + } + /// Read the specified chunk (at index `chunk_index`) and return the binary data as a Vector. /// /// # Bugs @@ -1152,6 +1190,61 @@ impl Decoder { Ok(()) } + /// Read chunks corresponding to several planes of a region of pixels. + /// + /// For non planar images this is equivalent to [`Self::read_chunk_bytes`] as there is only one + /// plane in the image. For planar images the planes are stored consecutively into the output + /// buffer. Returns an error if not enough space for at least one plane is provided. Otherwise + /// reads all planes that can be stored completely in the provided output buffer. + /// + /// A region is a rectangular assortment of pixels in the image, depending on the chunk type + /// either strips or tiles. Borrowing terminology from JPEG we call the collection of all + /// chunks from all planes that encode samples from the same region a "coding unit". + /// + /// # Bugs + /// + /// Sub-sampled images are not yet supported properly. + pub fn read_coding_unit_bytes( + &mut self, + slice: TiffCodingUnit, + buffer: &mut [u8], + ) -> TiffResult<()> { + let (width, height) = self.image().chunk_data_dimensions(slice.0)?; + let readout = self.image().readout_for_size(width, height)?; + + let ref layout @ image::PlaneLayout { + ref plane_offsets, + // We assume that is correct, so really it can be ignored. + total_bytes: _, + ref readout, + } = readout.to_plane_layout()?; + + if slice.0 >= readout.chunks_per_plane { + return Err(TiffError::UsageError(UsageError::InvalidCodingUnit( + slice.0, + readout.chunks_per_plane, + ))); + } + + // No subsamples planes support, for now. + let used_plane_offsets = usize::from(layout.used_planes(buffer)?); + debug_assert!(used_plane_offsets >= 1, "Should have errored"); + + for (idx, &plane_offset) in plane_offsets[..used_plane_offsets].iter().enumerate() { + let chunk = slice.0 + idx as u32 * readout.chunks_per_plane; + self.goto_offset_u64(self.image().chunk_offsets[chunk as usize])?; + + self.image.expand_chunk( + &mut self.value_reader, + &mut buffer[plane_offset..], + readout, + chunk, + )?; + } + + Ok(()) + } + /// Returns the default chunk size for the current image. Any given chunk in the image is at most as large as /// the value returned here. For the size of the data (chunk minus padding), use `chunk_data_dimensions`. pub fn chunk_dimensions(&self) -> (u32, u32) { @@ -1227,21 +1320,7 @@ impl Decoder { ref readout, } = readout.to_plane_layout()?; - layout.readout.assert_min_layout(buffer)?; - - // Note: with differently sized planes this is dependent on the plane. - let last_plane_start = buffer.len().checked_sub(readout.plane_stride); - - // Find how many planes fit into the output buffer. - let used_plane_offsets = plane_offsets - .iter() - .enumerate() - // Find the first plane that would not fit completely at its offset. - .skip_while(|(_, &offset)| last_plane_start >= Some(offset)) - .nth(0) - // If all planes fit, use all of them. - .map_or(plane_offsets.len(), |(idx, _)| idx); - + let used_plane_offsets = usize::from(layout.used_planes(buffer)?); debug_assert!(used_plane_offsets >= 1, "Should have errored"); // For multi-band images, only the first band is read. diff --git a/src/error.rs b/src/error.rs index 25de149..e0c06cb 100644 --- a/src/error.rs +++ b/src/error.rs @@ -178,6 +178,9 @@ quick_error! { InvalidPlaneIndex(index: u16) { display("invalid plane index ({index}) requested") } + InvalidCodingUnit(index: u32, have: u32) { + display("out of bounds coding unit ({index}) requested, only {have} available") + } PredictorCompressionMismatch { display("requested predictor is not compatible with the requested compression") } diff --git a/tests/decode_planes.rs b/tests/decode_planes.rs index 5b221da..ea9770c 100644 --- a/tests/decode_planes.rs +++ b/tests/decode_planes.rs @@ -1,4 +1,4 @@ -use tiff::decoder::Decoder; +use tiff::decoder::{Decoder, TiffCodingUnit}; use std::fs::File; use std::path::PathBuf; @@ -30,3 +30,47 @@ fn read_all_planes() { assert_eq!(sum, expected); } } + +#[test] +fn read_first_coding_unit() { + let path = PathBuf::from(TEST_IMAGE_DIR).join("planar-rgb-u8.tif"); + let file = File::open(path).unwrap(); + let io = std::io::BufReader::new(file); + + let mut tif = Decoder::new(io).unwrap(); + let layout = tif.image_coding_unit_layout(TiffCodingUnit(0)).unwrap(); + + assert_eq!(layout.planes, 3); + let plane_stride = layout.plane_stride.map_or(0, |x| x.get()); + eprintln!("{:?}", layout); + let mut buffer = vec![0u8; layout.complete_len]; + + struct Expected { + plane_sums: [u32; 3], + unit: TiffCodingUnit, + } + + // Mainly you can see: these are different and non-zero. Otherwise these magic constants just + // depend on the file itself that is being used. Also this varies by what unit we are reading. + const PLANE_EXCPECTED_SUMS: [Expected; 2] = [ + Expected { + plane_sums: [856853, 767686, 688532], + unit: TiffCodingUnit(0), + }, + Expected { + plane_sums: [883679, 797329, 711086], + unit: TiffCodingUnit(1), + }, + ]; + + for expected in PLANE_EXCPECTED_SUMS { + tif.read_coding_unit_bytes(expected.unit, &mut buffer) + .unwrap(); + + for (plane, expected) in (0..layout.planes).zip(expected.plane_sums) { + let plane_data = &buffer[plane_stride * plane..][..layout.len]; + let sum: u32 = plane_data.iter().copied().map(u32::from).sum(); + assert_eq!(sum, expected); + } + } +} From b1c74495ce1c50adf7b7ede6b7c24508ae70c588 Mon Sep 17 00:00:00 2001 From: "A. Molzer" <5550310+197g@users.noreply.github.com> Date: Sun, 28 Dec 2025 15:09:42 +0100 Subject: [PATCH 14/19] Allow extraneous extra samples beyond alpha --- src/decoder/image.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/decoder/image.rs b/src/decoder/image.rs index 1845f80..4a7ec76 100644 --- a/src/decoder/image.rs +++ b/src/decoder/image.rs @@ -377,7 +377,7 @@ impl Image { pub(crate) fn colortype(&self) -> TiffResult { let is_alpha_extra_samples = matches!( self.extra_samples.as_slice(), - [ExtraSamples::AssociatedAlpha] | [ExtraSamples::UnassociatedAlpha] + [ExtraSamples::AssociatedAlpha, ..] | [ExtraSamples::UnassociatedAlpha, ..] ); match self.photometric_interpretation { From af8a8d660f8395ad61f2920696cc2d086e9b3739 Mon Sep 17 00:00:00 2001 From: "A. Molzer" <5550310+197g@users.noreply.github.com> Date: Sun, 28 Dec 2025 18:48:30 +0100 Subject: [PATCH 15/19] Write decoded planar data in example --- examples/decode.rs | 93 +++++++++++++++++++++++++++++++++++++++++++--- src/decoder/mod.rs | 25 +++++++++++-- 2 files changed, 109 insertions(+), 9 deletions(-) diff --git a/examples/decode.rs b/examples/decode.rs index e40476c..023ce0d 100644 --- a/examples/decode.rs +++ b/examples/decode.rs @@ -1,4 +1,4 @@ -use tiff::decoder::Decoder; +use tiff::decoder::{BufferLayoutPreference, Decoder, DecodingBuffer, DecodingResult}; fn main() -> Result<(), Box> { let Some(image) = std::env::args_os().nth(1) else { @@ -10,10 +10,93 @@ fn main() -> Result<(), Box> { let io = std::io::BufReader::new(file); let mut reader = Decoder::new(io)?; - while { - reader.read_image()?; - reader.more_images() - } {} + for i in 0u32.. { + let layout = reader.image_buffer_layout()?; + let colortype = reader.colortype()?; + let dimensions = reader.dimensions()?; + + let mut data = reader.read_image()?; + debug_planes(i, &mut data, &layout, &colortype, dimensions); + + if !reader.more_images() { + break; + } + + reader.next_image()? + } Ok(()) } + +fn debug_planes( + index: u32, + data: &mut DecodingResult, + layout: &BufferLayoutPreference, + colortype: &tiff::ColorType, + (width, height): (u32, u32), +) { + let (depth, mut tupltype) = match colortype { + tiff::ColorType::Gray(_) => (1, "GRAYSCALE"), + tiff::ColorType::RGB(_) => (3, "RGB"), + tiff::ColorType::RGBA(_) => (4, "RGB_ALPHA"), + tiff::ColorType::Palette(_) => (1, "PALETTE"), + tiff::ColorType::CMYK(_) => (4, "CMYK"), + tiff::ColorType::Multiband { num_samples: 2, .. } => (2, "GRAYSCALE_ALPHA"), + _ => { + eprintln!("Unsupported color type for PAM output: {:?}", colortype); + return; + } + }; + + let maxval = match data.as_buffer(0) { + DecodingBuffer::U8(_) => u64::from(u8::MAX), + DecodingBuffer::U16(_) => u64::from(u16::MAX), + DecodingBuffer::U32(_) => u64::from(u32::MAX), + DecodingBuffer::U64(_) => u64::MAX, + // Eh, this is a good effort I guess. + DecodingBuffer::I8(items) => { + items.iter_mut().for_each(|v| *v = 0.max(*v)); + u64::from(i8::MAX as u8) + } + DecodingBuffer::I16(items) => { + items.iter_mut().for_each(|v| *v = 0.max(*v)); + u64::from(i16::MAX as u16) + } + DecodingBuffer::I32(items) => { + items.iter_mut().for_each(|v| *v = 0.max(*v)); + u64::from(i32::MAX as u32) + } + DecodingBuffer::I64(items) => { + items.iter_mut().for_each(|v| *v = 0.max(*v)); + i64::MAX as u64 + } + // Not supported by PAM. + DecodingBuffer::F16(_) | DecodingBuffer::F32(_) | DecodingBuffer::F64(_) => return, + }; + + let byte_len = if layout.planes > 1 { + tupltype = "GRAYSCALE"; + layout.plane_stride.unwrap().get() + } else { + data.as_buffer(0).byte_len() + }; + + // Write out this image's planes to PAM files. + let header = format!("P7\nWIDTH {width}\nHEIGHT {height}\nDEPTH {depth}\nMAXVAL {maxval}\nTUPLTYPE {tupltype}\nENDHDR\n"); + + for (plane, plane_data) in data + .as_buffer(0) + .as_bytes_mut() + .chunks_exact(byte_len) + .enumerate() + { + let filename = format!("image_{index:03}_plane_{plane:03}.pam"); + let mut file = std::fs::File::create(filename).expect("Failed to create output file"); + + use std::io::Write; + file.write_all(header.as_bytes()) + .expect("Failed to write PAM header"); + file.write_all(plane_data) + .expect("Failed to write PAM data"); + } +} diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs index 2c4a8ac..6c97390 100644 --- a/src/decoder/mod.rs +++ b/src/decoder/mod.rs @@ -110,6 +110,7 @@ impl DecodingResult { Self::new(size, limits, DecodingResult::I64) } + /// Get a view of this buffer starting from the nth _sample_ of the current type. pub fn as_buffer(&mut self, start: usize) -> DecodingBuffer<'_> { match *self { DecodingResult::U8(ref mut buf) => DecodingBuffer::U8(&mut buf[start..]), @@ -154,9 +155,25 @@ pub enum DecodingBuffer<'a> { } impl<'a> DecodingBuffer<'a> { - fn as_bytes_mut(&mut self) -> &mut [u8] { + pub fn as_bytes(&self) -> &[u8] { match self { - DecodingBuffer::U8(ref mut buf) => buf, + DecodingBuffer::U8(buf) => buf, + DecodingBuffer::I8(buf) => bytecast::i8_as_ne_bytes(buf), + DecodingBuffer::U16(buf) => bytecast::u16_as_ne_bytes(buf), + DecodingBuffer::I16(buf) => bytecast::i16_as_ne_bytes(buf), + DecodingBuffer::U32(buf) => bytecast::u32_as_ne_bytes(buf), + DecodingBuffer::I32(buf) => bytecast::i32_as_ne_bytes(buf), + DecodingBuffer::U64(buf) => bytecast::u64_as_ne_bytes(buf), + DecodingBuffer::I64(buf) => bytecast::i64_as_ne_bytes(buf), + DecodingBuffer::F16(buf) => bytecast::f16_as_ne_bytes(buf), + DecodingBuffer::F32(buf) => bytecast::f32_as_ne_bytes(buf), + DecodingBuffer::F64(buf) => bytecast::f64_as_ne_bytes(buf), + } + } + + pub fn as_bytes_mut(&mut self) -> &mut [u8] { + match self { + DecodingBuffer::U8(buf) => buf, DecodingBuffer::I8(buf) => bytecast::i8_as_ne_mut_bytes(buf), DecodingBuffer::U16(buf) => bytecast::u16_as_ne_mut_bytes(buf), DecodingBuffer::I16(buf) => bytecast::i16_as_ne_mut_bytes(buf), @@ -170,8 +187,8 @@ impl<'a> DecodingBuffer<'a> { } } - pub fn byte_len(&mut self) -> usize { - self.as_bytes_mut().len() + pub fn byte_len(&self) -> usize { + self.as_bytes().len() } } From 6d56d014a11b1b7531010863a7d22358e8efa48a Mon Sep 17 00:00:00 2001 From: "A. Molzer" <5550310+197g@users.noreply.github.com> Date: Sun, 28 Dec 2025 21:06:59 +0100 Subject: [PATCH 16/19] Sample types in layouts, resize DecoderResult By adding the type interpretation to `BufferLayoutPreference` (simply computed from the same fields that are trivial to fetch in `ReadoutLayout` with the rest) we can allocate an appropriate `DecoderResult` without any `Decoder` instance. We still want a Limits instance though. This lets a caller read _typed_ data from the `_bytes` methods which perform less allocations. It also lets them read multiple planes in a more convenient interface. See, `read_image` will keep its planar treatment bug for a little bit so you don't accidentally break during an upgrade. This also implies less code is being monomorphized, just a bonus. --- examples/decode.rs | 7 +- src/decoder/image.rs | 3 + src/decoder/mod.rs | 226 +++++++++++++++++++++++++++++-------------- 3 files changed, 163 insertions(+), 73 deletions(-) diff --git a/examples/decode.rs b/examples/decode.rs index 023ce0d..5def514 100644 --- a/examples/decode.rs +++ b/examples/decode.rs @@ -10,12 +10,17 @@ fn main() -> Result<(), Box> { let io = std::io::BufReader::new(file); let mut reader = Decoder::new(io)?; + let mut data = DecodingResult::I8(vec![]); + let limits = tiff::decoder::Limits::unlimited(); + for i in 0u32.. { let layout = reader.image_buffer_layout()?; let colortype = reader.colortype()?; let dimensions = reader.dimensions()?; - let mut data = reader.read_image()?; + data.resize_to(&layout, &limits)?; + reader.read_image_bytes(data.as_buffer(0).as_bytes_mut())?; + debug_planes(i, &mut data, &layout, &colortype, dimensions); if !reader.more_images() { diff --git a/src/decoder/image.rs b/src/decoder/image.rs index 4a7ec76..4ff351b 100644 --- a/src/decoder/image.rs +++ b/src/decoder/image.rs @@ -98,6 +98,8 @@ pub(crate) struct ReadoutLayout { pub color: ColorType, /// The number of bytes from one row to another. pub minimum_row_stride: usize, + /// The format of samples (assumed uniform for now, same with depth of `ColorType`). + pub sample_format: SampleFormat, /// Number of bytes to advance in output per row. pub row_stride: usize, @@ -659,6 +661,7 @@ impl Image { planar_config: self.planar_config, color, minimum_row_stride, + sample_format: self.sample_format, row_stride: data_row_bytes, chunk_row_stride: chunk_row_bytes, chunk_col_stride, diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs index 6c97390..5d33aab 100644 --- a/src/decoder/mod.rs +++ b/src/decoder/mod.rs @@ -54,6 +54,17 @@ pub enum DecodingResult { } impl DecodingResult { + /// Reallocate the buffer to decode all planes of the indicated layout. + pub fn resize_to( + &mut self, + buffer: &BufferLayoutPreference, + limits: &Limits, + ) -> Result<(), TiffError> { + let extent = DecodingExtent::U8(buffer.complete_len); + *self = extent.to_result_buffer(limits)?; + Ok(()) + } + fn new( size: usize, limits: &Limits, @@ -192,6 +203,39 @@ impl<'a> DecodingBuffer<'a> { } } +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum DecodingSampleType { + U8, + U16, + U32, + U64, + F16, + F32, + F64, + I8, + I16, + I32, + I64, +} + +impl DecodingSampleType { + fn extent_for_bytes(self, bytes: usize) -> DecodingExtent { + match self { + DecodingSampleType::U8 => DecodingExtent::U8(bytes), + DecodingSampleType::U16 => DecodingExtent::U16(bytes.div_ceil(2)), + DecodingSampleType::U32 => DecodingExtent::U32(bytes.div_ceil(4)), + DecodingSampleType::U64 => DecodingExtent::U64(bytes.div_ceil(8)), + DecodingSampleType::I8 => DecodingExtent::I8(bytes), + DecodingSampleType::I16 => DecodingExtent::I16(bytes.div_ceil(2)), + DecodingSampleType::I32 => DecodingExtent::I32(bytes.div_ceil(4)), + DecodingSampleType::I64 => DecodingExtent::I64(bytes.div_ceil(8)), + DecodingSampleType::F16 => DecodingExtent::F16(bytes.div_ceil(2)), + DecodingSampleType::F32 => DecodingExtent::F32(bytes.div_ceil(4)), + DecodingSampleType::F64 => DecodingExtent::F64(bytes.div_ceil(8)), + } + } +} + /// Information on the byte buffer that should be supplied to the decoder. /// /// This is relevant for [`Decoder::read_image_bytes`] and [`Decoder::read_chunk_bytes`] where the @@ -202,6 +246,15 @@ impl<'a> DecodingBuffer<'a> { pub struct BufferLayoutPreference { /// Minimum byte size of the buffer to read image data. pub len: usize, + /// The interpretation of each sample in the image. + /// + /// We only support a uniform sample layout. Detailed information for mixed colors may be added + /// in the future and will become available by explicit query. The same goes for the bit-depth + /// of samples that must also be uniform. + pub sample_format: SampleFormat, + /// The type representation for each sample. Only available for depths and formats which the + /// library can describe. + pub sample_type: Option, /// Minimum number of bytes to represent a row of image data of the requested content. pub row_stride: Option, /// Number of planes in the image. @@ -220,11 +273,87 @@ impl BufferLayoutPreference { planes: layout.plane_offsets.len(), plane_stride: core::num::NonZeroUsize::new(layout.readout.plane_stride), complete_len: layout.total_bytes, + sample_format: layout.readout.sample_format, + sample_type: Self::sample_type(layout.readout.sample_format, layout.readout.color), } } + + fn sample_type(sample_format: SampleFormat, color: ColorType) -> Option { + Some(match sample_format { + SampleFormat::Uint => match color.bit_depth() { + n if n <= 8 => DecodingSampleType::U8, + n if n <= 16 => DecodingSampleType::U16, + n if n <= 32 => DecodingSampleType::U32, + n if n <= 64 => DecodingSampleType::U64, + _ => return None, + }, + SampleFormat::IEEEFP => match color.bit_depth() { + 16 => DecodingSampleType::F16, + 32 => DecodingSampleType::F32, + 64 => DecodingSampleType::F64, + _ => return None, + }, + SampleFormat::Int => match color.bit_depth() { + n if n <= 8 => DecodingSampleType::I8, + n if n <= 16 => DecodingSampleType::I16, + n if n <= 32 => DecodingSampleType::I32, + n if n <= 64 => DecodingSampleType::I64, + _ => return None, + }, + _other => { + return None; + } + }) + } } impl image::ReadoutLayout { + // FIXME: when planes are not homogenous (i.e. subsampled or differing depths) then the + // `readout_for_size` or `to_plane_layout` needs a parameter to determine the planes being + // read instead of assuming a constant repeated size for them. + fn result_extent_for_planes( + self: &image::ReadoutLayout, + planes: core::ops::Range, + ) -> TiffResult { + let buffer = self.to_plane_layout()?; + + // The layout is for all planes. So restrict ourselves to the planes that were requested. + let offset = match buffer.plane_offsets.get(usize::from(planes.start)) { + Some(n) => *n, + None => { + return Err(TiffError::UsageError(UsageError::InvalidPlaneIndex( + planes.start, + ))) + } + }; + + let end = match buffer.plane_offsets.get(usize::from(planes.end)) { + Some(n) => *n, + None => buffer.total_bytes, + }; + + let buffer_bytes = end - offset; + let bits_per_sample = self.color.bit_depth(); + + let Some(sample_type) = BufferLayoutPreference::sample_type(self.sample_format, self.color) + else { + if matches!( + self.sample_format, + SampleFormat::Uint | SampleFormat::Int | SampleFormat::IEEEFP + ) { + return Err(TiffError::UnsupportedError( + TiffUnsupportedError::UnsupportedSampleDepth(bits_per_sample), + )); + } else { + return Err(TiffError::UnsupportedError( + TiffUnsupportedError::UnsupportedSampleFormat(vec![self.sample_format]), + )); + } + }; + + Ok(sample_type.extent_for_bytes(buffer_bytes)) + } + #[inline(always)] fn assert_min_layout(&self, buffer: &[T]) -> TiffResult<()> { if core::mem::size_of_val(buffer) < self.plane_stride { @@ -293,6 +422,22 @@ impl DecodingExtent { } .map_err(overflow) } + + fn sample_type(&self) -> DecodingSampleType { + match *self { + DecodingExtent::U8(_) => DecodingSampleType::U8, + DecodingExtent::U16(_) => DecodingSampleType::U16, + DecodingExtent::U32(_) => DecodingSampleType::U32, + DecodingExtent::U64(_) => DecodingSampleType::U64, + DecodingExtent::F16(_) => DecodingSampleType::F16, + DecodingExtent::F32(_) => DecodingSampleType::F32, + DecodingExtent::F64(_) => DecodingSampleType::F64, + DecodingExtent::I8(_) => DecodingSampleType::I8, + DecodingExtent::I16(_) => DecodingSampleType::I16, + DecodingExtent::I32(_) => DecodingSampleType::I32, + DecodingExtent::I64(_) => DecodingSampleType::I64, + } + } } #[derive(Debug, Copy, Clone, PartialEq)] @@ -1030,73 +1175,6 @@ impl Decoder { Ok(()) } - // FIXME: when planes are not homogenous (i.e. subsampled or differing depths) then the - // `readout_for_size` or `to_plane_layout` needs a parameter to determine the planes being - // read instead of assuming a constant repeated size for them. - fn result_extent( - &self, - read: &image::ReadoutLayout, - planes: core::ops::Range, - ) -> TiffResult { - let buffer = read.to_plane_layout()?; - - // The layout is for all planes. So restrict ourselves to the planes that were requested. - let offset = match buffer.plane_offsets.get(usize::from(planes.start)) { - Some(n) => *n, - None => { - return Err(TiffError::UsageError(UsageError::InvalidPlaneIndex( - planes.start, - ))) - } - }; - - let end = match buffer.plane_offsets.get(usize::from(planes.end)) { - Some(n) => *n, - None => buffer.total_bytes, - }; - - let buffer_bytes = end - offset; - let bits_per_sample = read.color.bit_depth(); - - Ok(match self.image().sample_format { - SampleFormat::Uint => match bits_per_sample { - n if n <= 8 => DecodingExtent::U8(buffer_bytes), - n if n <= 16 => DecodingExtent::U16(buffer_bytes / 2), - n if n <= 32 => DecodingExtent::U32(buffer_bytes / 4), - n if n <= 64 => DecodingExtent::U64(buffer_bytes / 8), - n => { - return Err(TiffError::UnsupportedError( - TiffUnsupportedError::UnsupportedBitsPerChannel(n), - )) - } - }, - SampleFormat::IEEEFP => match bits_per_sample { - 16 => DecodingExtent::F16(buffer_bytes / 2), - 32 => DecodingExtent::F32(buffer_bytes / 4), - 64 => DecodingExtent::F64(buffer_bytes / 8), - n => { - return Err(TiffError::UnsupportedError( - TiffUnsupportedError::UnsupportedBitsPerChannel(n), - )) - } - }, - SampleFormat::Int => match bits_per_sample { - n if n <= 8 => DecodingExtent::I8(buffer_bytes), - n if n <= 16 => DecodingExtent::I16(buffer_bytes / 2), - n if n <= 32 => DecodingExtent::I32(buffer_bytes / 4), - n if n <= 64 => DecodingExtent::I64(buffer_bytes / 8), - n => { - return Err(TiffError::UnsupportedError( - TiffUnsupportedError::UnsupportedBitsPerChannel(n), - )) - } - }, - format => { - return Err(TiffUnsupportedError::UnsupportedSampleFormat(vec![format]).into()) - } - }) - } - /// Returns the layout preferred to read the specified chunk with [`Self::read_chunk_bytes`]. /// /// Returns the layout without being specific as to the underlying type for forward @@ -1117,7 +1195,9 @@ impl Decoder { let data_dims = self.image().chunk_data_dimensions(chunk_index)?; let readout = self.image().readout_for_size(data_dims.0, data_dims.1)?; - let layout = self.result_extent(&readout, 0..1)?.preferred_layout()?; + let extent = readout.result_extent_for_planes(0..1)?; + let sample_type = extent.sample_type(); + let layout = extent.preferred_layout()?; let row_stride = core::num::NonZeroUsize::new(readout.minimum_row_stride); let plane_stride = core::num::NonZeroUsize::new(readout.plane_stride); @@ -1128,6 +1208,8 @@ impl Decoder { planes: 1, plane_stride, complete_len: layout.size(), + sample_format: self.image().sample_format, + sample_type: Some(sample_type), }) } @@ -1177,8 +1259,8 @@ impl Decoder { let readout = self.image().readout_for_size(width, height)?; - let mut result = self - .result_extent(&readout, 0..1)? + let mut result = readout + .result_extent_for_planes(0..1)? .to_result_buffer(&self.value_reader.limits)?; self.read_chunk_to_bytes(result.as_buffer(0).as_bytes_mut(), chunk_index, &readout)?; @@ -1303,8 +1385,8 @@ impl Decoder { pub fn read_image(&mut self) -> TiffResult { let readout = self.image().readout_for_image()?; - let mut result = self - .result_extent(&readout, 0..1)? + let mut result = readout + .result_extent_for_planes(0..1)? .to_result_buffer(&self.value_reader.limits)?; self.read_image_bytes(result.as_buffer(0).as_bytes_mut())?; From 5150e91bbb54638aa62aaf32461640bd10fd5436 Mon Sep 17 00:00:00 2001 From: "A. Molzer" <5550310+197g@users.noreply.github.com> Date: Sun, 28 Dec 2025 21:14:41 +0100 Subject: [PATCH 17/19] Document and deprecate `read_image` --- src/decoder/mod.rs | 43 +++++++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs index 5d33aab..0676aa1 100644 --- a/src/decoder/mod.rs +++ b/src/decoder/mod.rs @@ -1182,12 +1182,6 @@ impl Decoder { /// channels of individual *bit* length and format each. /// /// See [`Self::colortype`] to describe the sample types. - /// - /// # Bugs - /// - /// When the image is stored as a planar configuration, this method will currently only - /// indicate the layout needed to read the first data plane. This will be fixed in a future - /// major version of `tiff`. pub fn image_chunk_buffer_layout( &mut self, chunk_index: u32, @@ -1249,11 +1243,7 @@ impl Decoder { /// Read the specified chunk (at index `chunk_index`) and return the binary data as a Vector. /// - /// # Bugs - /// - /// When the image is stored as a planar configuration, this method will currently only - /// correctly read the first sample's plane. This will be fixed in a future major version of - /// `tiff`. + /// Note that for planar images each chunk contains only one sample of the underlying data. pub fn read_chunk(&mut self, chunk_index: u32) -> TiffResult { let (width, height) = self.image().chunk_data_dimensions(chunk_index)?; @@ -1274,10 +1264,7 @@ impl Decoder { /// call to [`Self::image_chunk_buffer_layout`]. Note that the alignment may be arbitrary, but /// an alignment smaller than the preferred alignment may perform worse. /// - /// # Bugs - /// - /// When the image is stored as a planar configuration, this method will currently only read - /// the first sample's plane. This will be fixed in a future major version of `tiff`. + /// Note that for planar images each chunk contains only one sample of the underlying data. pub fn read_chunk_bytes(&mut self, chunk_index: u32, buffer: &mut [u8]) -> TiffResult<()> { let (width, height) = self.image().chunk_data_dimensions(chunk_index)?; @@ -1378,10 +1365,15 @@ impl Decoder { /// Decodes the entire image and return it as a Vector /// + /// # Examples + /// + /// This method is deprecated. For replacement usage see `examples/decode.rs`. + /// /// # Bugs /// /// When the image is stored as a planar configuration, this method will currently only read /// the first sample's plane. This will be fixed in a future major version of `tiff`. + #[deprecated = "Use `DecodingResult::resize_to` and `read_image_bytes` instead. This old method will keep its bugged planar behavior until it is removed."] pub fn read_image(&mut self) -> TiffResult { let readout = self.image().readout_for_image()?; @@ -1400,10 +1392,25 @@ impl Decoder { /// call to [`Self::image_buffer_layout`]. Note that the alignment may be arbitrary, but an /// alignment smaller than the preferred alignment may perform worse. /// - /// # Bugs + /// # Examples /// - /// When the image is stored as a planar configuration, this method will currently only read - /// the first sample's plane. This will be fixed in a future major version of `tiff`. + /// ``` + /// use tiff::decoder::{Decoder, DecodingResult, Limits}; + /// + /// let mut result = DecodingResult::I8(vec![]); + /// let limits = Limits::unlimited(); + /// + /// let mut reader = /* */ + /// # Decoder::new(std::io::Cursor::new(include_bytes!(concat!( + /// # env!("CARGO_MANIFEST_DIR"), "/tests/images/tiled-gray-i1.tif" + /// # )))).unwrap(); + /// + /// let layout = reader.image_buffer_layout()?; + /// result.resize_to(&layout, &limits)?; + /// reader.read_image_bytes(result.as_buffer(0).as_bytes_mut())?; + /// + /// # Ok::<_, tiff::TiffError>(()) + /// ``` /// /// # Error /// From 5a3b11314e0085920f7bd3929efcc9088ac48320 Mon Sep 17 00:00:00 2001 From: "A. Molzer" <5550310+197g@users.noreply.github.com> Date: Sun, 28 Dec 2025 22:17:48 +0100 Subject: [PATCH 18/19] Rewrite `_to_buffer` variants We previously handed it a wrapper that was supposedly type-safe but really internally cast the buffer to bytes. That was rather pointless as the `_bytes` method could always be used instead. We replace the semantics by a utility which allocates an appropriate buffer with the internal limits, wrapping the exact sequence identified previously in the example as a pattern. --- examples/decode.rs | 6 +--- src/decoder/mod.rs | 89 +++++++++++++++++++++++++++++++++------------- 2 files changed, 66 insertions(+), 29 deletions(-) diff --git a/examples/decode.rs b/examples/decode.rs index 5def514..18eaa79 100644 --- a/examples/decode.rs +++ b/examples/decode.rs @@ -11,15 +11,11 @@ fn main() -> Result<(), Box> { let mut reader = Decoder::new(io)?; let mut data = DecodingResult::I8(vec![]); - let limits = tiff::decoder::Limits::unlimited(); for i in 0u32.. { - let layout = reader.image_buffer_layout()?; let colortype = reader.colortype()?; let dimensions = reader.dimensions()?; - - data.resize_to(&layout, &limits)?; - reader.read_image_bytes(data.as_buffer(0).as_bytes_mut())?; + let layout = reader.read_image_to_buffer(&mut data)?; debug_planes(i, &mut data, &layout, &colortype, dimensions); diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs index 0676aa1..ef47425 100644 --- a/src/decoder/mod.rs +++ b/src/decoder/mod.rs @@ -60,7 +60,20 @@ impl DecodingResult { buffer: &BufferLayoutPreference, limits: &Limits, ) -> Result<(), TiffError> { - let extent = DecodingExtent::U8(buffer.complete_len); + let sample_type = buffer.sample_type.ok_or(TiffError::UnsupportedError( + TiffUnsupportedError::UnknownInterpretation, + ))?; + + let extent = sample_type.extent_for_bytes(buffer.complete_len); + self.resize_to_extent(extent, limits) + } + + fn resize_to_extent( + &mut self, + extent: DecodingExtent, + limits: &Limits, + ) -> Result<(), TiffError> { + // FIXME: we *can* reuse the allocation sometimes. *self = extent.to_result_buffer(limits)?; Ok(()) } @@ -1144,22 +1157,6 @@ impl Decoder { Ok(u32::try_from(self.image().chunk_offsets.len())?) } - pub fn read_chunk_to_buffer( - &mut self, - mut buffer: DecodingBuffer, - chunk_index: u32, - output_width: usize, - ) -> TiffResult<()> { - let (width, height) = self.image().chunk_data_dimensions(chunk_index)?; - - let mut layout = self.image().readout_for_size(width, height)?; - layout.set_row_stride(output_width)?; - - self.read_chunk_to_bytes(buffer.as_bytes_mut(), chunk_index, &layout)?; - - Ok(()) - } - fn read_chunk_to_bytes( &mut self, buffer: &mut [u8], @@ -1276,6 +1273,32 @@ impl Decoder { Ok(()) } + /// Read the specified chunk (at index `chunk_index`) into a provide buffer. + /// + /// It will re-allocate the buffer into the correct type and size, within the decoder's + /// configured limits, and then pass it to the underlying method. This is essentially a + /// type-safe wrapper around the raw [`Self::read_chunk_to_bytes`] method. + /// + /// Note that for planar images each chunk contains only one sample of the underlying data. + pub fn read_chunk_to_buffer( + &mut self, + buffer: &mut DecodingResult, + chunk_index: u32, + output_width: usize, + ) -> TiffResult<()> { + let (width, height) = self.image().chunk_data_dimensions(chunk_index)?; + + let mut layout = self.image().readout_for_size(width, height)?; + layout.set_row_stride(output_width)?; + + let extent = layout.result_extent_for_planes(0..1)?; + buffer.resize_to_extent(extent, &self.value_reader.limits)?; + + self.read_chunk_to_bytes(buffer.as_buffer(0).as_bytes_mut(), chunk_index, &layout)?; + + Ok(()) + } + /// Read chunks corresponding to several planes of a region of pixels. /// /// For non planar images this is equivalent to [`Self::read_chunk_bytes`] as there is only one @@ -1388,9 +1411,9 @@ impl Decoder { /// Decodes the entire image into a provided buffer. /// - /// Returns a [`TiffError::UsageError`] if the chunk is smaller than the size indicated with a - /// call to [`Self::image_buffer_layout`]. Note that the alignment may be arbitrary, but an - /// alignment smaller than the preferred alignment may perform worse. + /// It will re-allocate the buffer into the correct type and size, within the decoder's + /// configured limits, and then pass it to the underlying method. This is essentially a + /// type-safe wrapper around the raw [`Self::read_image_bytes`] method. /// /// # Examples /// @@ -1398,19 +1421,37 @@ impl Decoder { /// use tiff::decoder::{Decoder, DecodingResult, Limits}; /// /// let mut result = DecodingResult::I8(vec![]); - /// let limits = Limits::unlimited(); /// /// let mut reader = /* */ /// # Decoder::new(std::io::Cursor::new(include_bytes!(concat!( /// # env!("CARGO_MANIFEST_DIR"), "/tests/images/tiled-gray-i1.tif" /// # )))).unwrap(); /// - /// let layout = reader.image_buffer_layout()?; - /// result.resize_to(&layout, &limits)?; - /// reader.read_image_bytes(result.as_buffer(0).as_bytes_mut())?; + /// reader.read_image_to_buffer(&mut result)?; /// /// # Ok::<_, tiff::TiffError>(()) /// ``` + pub fn read_image_to_buffer( + &mut self, + result: &mut DecodingResult, + ) -> TiffResult { + let readout = self.image().readout_for_image()?; + + let planes = readout.to_plane_layout()?; + let layout = BufferLayoutPreference::from_planes(&planes); + + let extent = readout.result_extent_for_planes(0..1)?; + result.resize_to_extent(extent, &self.value_reader.limits)?; + self.read_image_bytes(result.as_buffer(0).as_bytes_mut())?; + + Ok(layout) + } + + /// Decodes the entire image into a provided buffer. + /// + /// Returns a [`TiffError::UsageError`] if the chunk is smaller than the size indicated with a + /// call to [`Self::image_buffer_layout`]. Note that the alignment may be arbitrary, but an + /// alignment smaller than the preferred alignment may perform worse. /// /// # Error /// From 2d901796025929c2e4f82dce696aa2ab53616032 Mon Sep 17 00:00:00 2001 From: "A. Molzer" <5550310+197g@users.noreply.github.com> Date: Mon, 29 Dec 2025 19:49:09 +0100 Subject: [PATCH 19/19] Soften deprecation of `Decoder::read_image` --- src/decoder/mod.rs | 8 +++++++- src/encoder/writer.rs | 4 +++- tests/encode_images.rs | 20 +++++++++++++++----- 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs index ef47425..cc31572 100644 --- a/src/decoder/mod.rs +++ b/src/decoder/mod.rs @@ -1396,7 +1396,13 @@ impl Decoder { /// /// When the image is stored as a planar configuration, this method will currently only read /// the first sample's plane. This will be fixed in a future major version of `tiff`. - #[deprecated = "Use `DecodingResult::resize_to` and `read_image_bytes` instead. This old method will keep its bugged planar behavior until it is removed."] + /// + /// # Intent to deprecate + /// + /// Use `DecodingResult::resize_to` and `read_image_bytes` instead where possible, preserving + /// the buffer across multiple calls. This old method will likely keep its bugged planar + /// behavior until it is fully replaced, to ensure that existing code will not run into + /// unexpectedly large allocations that will error on limits instead. pub fn read_image(&mut self) -> TiffResult { let readout = self.image().readout_for_image()?; diff --git a/src/encoder/writer.rs b/src/encoder/writer.rs index bd9c603..6b7eb16 100644 --- a/src/encoder/writer.rs +++ b/src/encoder/writer.rs @@ -221,6 +221,8 @@ fn encoder_on_short_writes() { }; let imgdata = vec![42u8; 100 * 100]; + let mut data = crate::decoder::DecodingResult::U8(vec![]); + for comp in [ super::Compression::Uncompressed, super::Compression::Packbits, @@ -240,7 +242,7 @@ fn encoder_on_short_writes() { write.inner.set_position(0); let mut tiff_reader = crate::decoder::Decoder::new(&mut write.inner).unwrap(); - let mut data = tiff_reader.read_image().unwrap(); + let _layout = tiff_reader.read_image_to_buffer(&mut data).unwrap(); assert!( matches!(&data, crate::decoder::DecodingResult::U8(v) if *v == imgdata), diff --git a/tests/encode_images.rs b/tests/encode_images.rs index b16e87d..df0dac9 100644 --- a/tests/encode_images.rs +++ b/tests/encode_images.rs @@ -81,7 +81,10 @@ fn encode_decode() { decoder.get_tag(Tag::Artist).unwrap(), ifd::Value::Ascii("Image-tiff".into()) ); - if let DecodingResult::U8(img_res) = decoder.read_image().unwrap() { + + let mut buffer = tiff::decoder::DecodingResult::U8(vec![]); + let _layout = decoder.read_image_to_buffer(&mut buffer).unwrap(); + if let tiff::decoder::DecodingResult::U8(img_res) = buffer { assert_eq!(image_data, img_res); } else { panic!("Wrong data type"); @@ -161,7 +164,9 @@ fn encode_decode_big() { decoder.get_tag(Tag::Artist).unwrap(), ifd::Value::Ascii("Image-tiff".into()) ); - if let DecodingResult::U8(img_res) = decoder.read_image().unwrap() { + let mut buffer = tiff::decoder::DecodingResult::U8(vec![]); + let _layout = decoder.read_image_to_buffer(&mut buffer).unwrap(); + if let DecodingResult::U8(img_res) = buffer { assert_eq!(image_data, img_res); } else { panic!("Wrong data type"); @@ -245,7 +250,10 @@ macro_rules! test_roundtrip { let mut decoder = Decoder::new(img_file).expect("Cannot create decoder"); assert_eq!(decoder.colortype().unwrap(), expected_type); - let image_data = match decoder.read_image().unwrap() { + let mut buffer = tiff::decoder::DecodingResult::U8(vec![]); + decoder.read_image_to_buffer(&mut buffer).unwrap(); + + let image_data = match buffer { DecodingResult::$buffer(res) => res, _ => panic!("Wrong data type"), }; @@ -259,9 +267,11 @@ macro_rules! test_roundtrip { } file.seek(SeekFrom::Start(0)).unwrap(); { + let mut buffer = tiff::decoder::DecodingResult::U8(vec![]); let mut decoder = Decoder::new(&mut file).unwrap(); - if let DecodingResult::$buffer(img_res) = decoder.read_image().unwrap() { - assert_eq!(image_data, img_res); + decoder.read_image_to_buffer(&mut buffer).unwrap(); + if let DecodingResult::$buffer(img_res) = buffer { + assert_eq!(*image_data, img_res); } else { panic!("Wrong data type"); }