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] 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)