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