From 2f4ab61ff19a1b023c1067afbe886c589eb5f86a Mon Sep 17 00:00:00 2001 From: Johannes Vollmer <32042925+johannesvollmer@users.noreply.github.com> Date: Sat, 2 Apr 2022 12:18:37 +0200 Subject: [PATCH 1/7] make progress callback mutable (otherwise it could not modify state such as progress bars in a safe way) --- src/codecs/openexr.rs | 4 ++-- src/image.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/codecs/openexr.rs b/src/codecs/openexr.rs index 3f20fcfdd0..ace4ecdcfe 100644 --- a/src/codecs/openexr.rs +++ b/src/codecs/openexr.rs @@ -152,10 +152,10 @@ impl<'a, R: 'a + Read + Seek> ImageDecoder<'a> for OpenExrDecoder { } // reads with or without alpha, depending on `self.alpha_preference` and `self.alpha_present_in_file` - fn read_image_with_progress( + fn read_image_with_progress( self, unaligned_bytes: &mut [u8], - progress_callback: F, + mut progress_callback: F, ) -> ImageResult<()> { let blocks_in_header = self.selected_exr_header().chunk_count as u64; let channel_count = self.color_type().channel_count() as usize; diff --git a/src/image.rs b/src/image.rs index c578bad970..aefef293f2 100644 --- a/src/image.rs +++ b/src/image.rs @@ -688,10 +688,10 @@ pub trait ImageDecoder<'a>: Sized { /// Same as `read_image` but periodically calls the provided callback to give updates on loading /// progress. - fn read_image_with_progress( + fn read_image_with_progress( self, buf: &mut [u8], - progress_callback: F, + mut progress_callback: F, ) -> ImageResult<()> { assert_eq!(u64::try_from(buf.len()), Ok(self.total_bytes())); From 2e68dbcdd8d01239fd87319ac7451413bd99fa43 Mon Sep 17 00:00:00 2001 From: Johannes Vollmer <32042925+johannesvollmer@users.noreply.github.com> Date: Sat, 2 Apr 2022 12:29:31 +0200 Subject: [PATCH 2/7] add another write function with callback parameter --- src/image.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/image.rs b/src/image.rs index aefef293f2..c7d8253ba9 100644 --- a/src/image.rs +++ b/src/image.rs @@ -802,6 +802,27 @@ pub trait ImageEncoder { width: u32, height: u32, color_type: ColorType, + ) -> ImageResult<()> { + self.write_image_with_progress(buf, width, height, color_type, |_|{}) + } + + /// Writes all the bytes in an image to the encoder. + /// + /// This function takes a slice of bytes of the pixel data of the image + /// and encodes them. Unlike particular format encoders inherent impl encode + /// methods where endianness is not specified, here image data bytes should + /// always be in native endian. The implementor will reorder the endianess + /// as necessary for the target encoding format. + /// + /// See also `ImageDecoder::read_image` which reads byte buffers into + /// native endian. + fn write_image_with_progress( + self, + buf: &[u8], + width: u32, + height: u32, + color_type: ColorType, + progress: F, ) -> ImageResult<()>; } From cb41079c115dc9912c09f6ab906e34b8c73fe24c Mon Sep 17 00:00:00 2001 From: Johannes Vollmer <32042925+johannesvollmer@users.noreply.github.com> Date: Sat, 2 Apr 2022 13:17:48 +0200 Subject: [PATCH 3/7] prototype progress for farbfeld and openexr --- src/codecs/farbfeld.rs | 29 ++++++++++++++++++++++------ src/codecs/openexr.rs | 44 +++++++++++++++++++++++++++++++++--------- src/image.rs | 22 ++++++++++++++++----- 3 files changed, 75 insertions(+), 20 deletions(-) diff --git a/src/codecs/farbfeld.rs b/src/codecs/farbfeld.rs index 524eb19af6..10721f5228 100644 --- a/src/codecs/farbfeld.rs +++ b/src/codecs/farbfeld.rs @@ -259,12 +259,12 @@ impl FarbfeldEncoder { /// Encodes the image ```data``` (native endian) /// that has dimensions ```width``` and ```height``` - pub fn encode(self, data: &[u8], width: u32, height: u32) -> ImageResult<()> { - self.encode_impl(data, width, height)?; + pub fn encode(self, data: &[u8], width: u32, height: u32, progress: F) -> ImageResult<()> { + self.encode_impl(data, width, height, progress)?; Ok(()) } - fn encode_impl(mut self, data: &[u8], width: u32, height: u32) -> io::Result<()> { + fn encode_impl(mut self, data: &[u8], width: u32, height: u32, mut progress: F) -> io::Result<()> { self.w.write_all(b"farbfeld")?; let mut buf = [0u8; 4]; @@ -274,22 +274,29 @@ impl FarbfeldEncoder { BigEndian::write_u32(&mut buf, height); self.w.write_all(&buf)?; - for channel in data.chunks_exact(2) { + let u16_chunks = data.chunks_exact(2); + let chunk_count = u16_chunks.len() as u64; + + for (chunk_index, channel) in u16_chunks.enumerate() { + progress(Progress::new(chunk_index as u64, chunk_count)); + BigEndian::write_u16(&mut buf, NativeEndian::read_u16(channel)); self.w.write_all(&buf[..2])?; } + progress(Progress::new(chunk_count, chunk_count)); Ok(()) } } impl ImageEncoder for FarbfeldEncoder { - fn write_image( + fn write_image_with_progress( self, buf: &[u8], width: u32, height: u32, color_type: ColorType, + progress: F, ) -> ImageResult<()> { if color_type != ColorType::Rgba16 { return Err(ImageError::Unsupported( @@ -300,7 +307,17 @@ impl ImageEncoder for FarbfeldEncoder { )); } - self.encode(buf, width, height) + self.encode(buf, width, height, progress) + } + + fn write_image( + self, + buf: &[u8], + width: u32, + height: u32, + color_type: ColorType, + ) -> ImageResult<()> { + self.write_image_with_progress(buf, width, height, color_type, |_|{}) } } diff --git a/src/codecs/openexr.rs b/src/codecs/openexr.rs index ace4ecdcfe..7d2e5737a4 100644 --- a/src/codecs/openexr.rs +++ b/src/codecs/openexr.rs @@ -157,7 +157,7 @@ impl<'a, R: 'a + Read + Seek> ImageDecoder<'a> for OpenExrDecoder { unaligned_bytes: &mut [u8], mut progress_callback: F, ) -> ImageResult<()> { - let blocks_in_header = self.selected_exr_header().chunk_count as u64; + let blocks_in_header = self.selected_exr_header().chunk_count; let channel_count = self.color_type().channel_count() as usize; let display_window = self.selected_exr_header().shared_attributes.display_window; @@ -217,10 +217,7 @@ impl<'a, R: 'a + Read + Seek> ImageDecoder<'a> for OpenExrDecoder { .all_attributes() .on_progress(|progress| { progress_callback( - Progress::new( - (progress * blocks_in_header as f64) as u64, - blocks_in_header, - ), // TODO precision errors? + progress_from_f32(blocks_in_header, progress), ); }) .from_chunks(self.exr_reader) @@ -237,18 +234,20 @@ impl<'a, R: 'a + Read + Seek> ImageDecoder<'a> for OpenExrDecoder { } } + /// Write a raw byte buffer of pixels, /// returning an Error if it has an invalid length. /// /// Assumes the writer is buffered. In most cases, /// you should wrap your writer in a `BufWriter` for best performance. // private. access via `OpenExrEncoder` -fn write_buffer( +fn write_buffer( mut buffered_write: impl Write + Seek, unaligned_bytes: &[u8], width: u32, height: u32, color_type: ColorType, + mut progress: F, ) -> ImageResult<()> { let width = width as usize; let height = height as usize; @@ -298,7 +297,7 @@ fn write_buffer( }), ) .write() - // .on_progress(|progress| todo!()) + .on_progress(|progress_f32| progress(progress_from_f32(unaligned_bytes.len(), progress_f32))) .to_buffered(&mut buffered_write) .map_err(to_image_err)?; } @@ -318,7 +317,7 @@ fn write_buffer( }), ) .write() - // .on_progress(|progress| todo!()) + .on_progress(|progress_f32| progress(progress_from_f32(unaligned_bytes.len(), progress_f32))) .to_buffered(&mut buffered_write) .map_err(to_image_err)?; } @@ -355,6 +354,22 @@ impl ImageEncoder for OpenExrEncoder where W: Write + Seek, { + /// Writes the complete image. + /// + /// Returns an Error if it has an invalid length. + /// Assumes the writer is buffered. In most cases, + /// you should wrap your writer in a `BufWriter` for best performance. + fn write_image_with_progress( + self, + buf: &[u8], + width: u32, + height: u32, + color_type: ColorType, + progress: F, + ) -> ImageResult<()> { + write_buffer(self.0, buf, width, height, color_type, progress) + } + /// Writes the complete image. /// /// Returns an Error if it has an invalid length. @@ -367,7 +382,7 @@ where height: u32, color_type: ColorType, ) -> ImageResult<()> { - write_buffer(self.0, buf, width, height, color_type) + write_buffer(self.0, buf, width, height, color_type, |_|{}) } } @@ -593,3 +608,14 @@ mod test { assert!(original.pixels().zip(cropped.pixels()).all(|(a, b)| a == b)); } } + +fn progress_from_f32(granularity: usize, progress: f64) -> Progress { + // avoid float precision in this special case: + if progress == 1.0 { return Progress::new(granularity as u64, granularity as u64) } + + Progress::new( + (progress * granularity as f64) as u64, + granularity as u64, + ) +} + diff --git a/src/image.rs b/src/image.rs index c7d8253ba9..6c385d7d73 100644 --- a/src/image.rs +++ b/src/image.rs @@ -802,9 +802,7 @@ pub trait ImageEncoder { width: u32, height: u32, color_type: ColorType, - ) -> ImageResult<()> { - self.write_image_with_progress(buf, width, height, color_type, |_|{}) - } + ) -> ImageResult<()>; /// Writes all the bytes in an image to the encoder. /// @@ -816,14 +814,28 @@ pub trait ImageEncoder { /// /// See also `ImageDecoder::read_image` which reads byte buffers into /// native endian. + /// + /// The progress callback is a function that is called occasionally, + /// and can be used to update, for example, a progress bar. + /// The granularity of those updates is implementation-defined + /// and might not be very accurate in some implementations. fn write_image_with_progress( self, buf: &[u8], width: u32, height: u32, color_type: ColorType, - progress: F, - ) -> ImageResult<()>; + mut progress: F, + ) -> ImageResult<()> where Self:Sized { + let max_progress = buf.len() as u64; + // TODO force implementations to implement this function instead of write_image()? + progress(Progress::new(0, max_progress)); + + self.write_image(buf, width, height, color_type); + + progress(Progress::new(max_progress, max_progress)); + Ok(()) + } } /// Immutable pixel iterator From e33047527d2099b5ccfb5b3e2fefd205e13b4c08 Mon Sep 17 00:00:00 2001 From: Johannes Vollmer <32042925+johannesvollmer@users.noreply.github.com> Date: Sat, 2 Apr 2022 13:33:32 +0200 Subject: [PATCH 4/7] undo changes from another branch --- src/image.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/image.rs b/src/image.rs index 6c385d7d73..9c4b3e85e5 100644 --- a/src/image.rs +++ b/src/image.rs @@ -688,10 +688,10 @@ pub trait ImageDecoder<'a>: Sized { /// Same as `read_image` but periodically calls the provided callback to give updates on loading /// progress. - fn read_image_with_progress( + fn read_image_with_progress( self, buf: &mut [u8], - mut progress_callback: F, + progress_callback: F, ) -> ImageResult<()> { assert_eq!(u64::try_from(buf.len()), Ok(self.total_bytes())); From 66401fc0e5c07dd539b86199e1495e5da99445b8 Mon Sep 17 00:00:00 2001 From: Johannes Vollmer <32042925+johannesvollmer@users.noreply.github.com> Date: Sat, 2 Apr 2022 13:34:47 +0200 Subject: [PATCH 5/7] undo changes from another branch --- examples/encode_f32.rs | 22 ++++++++++++++++++++++ src/codecs/openexr.rs | 4 ++-- 2 files changed, 24 insertions(+), 2 deletions(-) create mode 100644 examples/encode_f32.rs diff --git a/examples/encode_f32.rs b/examples/encode_f32.rs new file mode 100644 index 0000000000..f0c3c0402b --- /dev/null +++ b/examples/encode_f32.rs @@ -0,0 +1,22 @@ +//! An example of handling a 32-bit float image. +use std::env; +use std::path::Path; +use image::{Rgba32FImage, Rgba}; + +fn main() { + let (width, height) = (200, 200); + + // construct an image with 32-bit float pixels + let mut f32_buffer = Rgba32FImage::from_fn( + width as u32, height as u32, + |x, y| image::Rgba([ + 2.0 * (x as f32 / width as f32), + 2.0 * (y as f32 / height as f32), + (y+x) as f32 / (width+height) as f32, + 1.0, + ]) + ); + + // save the float image as exr + f32_buffer.save("generated_image.exr").unwrap(); +} diff --git a/src/codecs/openexr.rs b/src/codecs/openexr.rs index 7d2e5737a4..6ef9923f86 100644 --- a/src/codecs/openexr.rs +++ b/src/codecs/openexr.rs @@ -152,10 +152,10 @@ impl<'a, R: 'a + Read + Seek> ImageDecoder<'a> for OpenExrDecoder { } // reads with or without alpha, depending on `self.alpha_preference` and `self.alpha_present_in_file` - fn read_image_with_progress( + fn read_image_with_progress( self, unaligned_bytes: &mut [u8], - mut progress_callback: F, + progress_callback: F, ) -> ImageResult<()> { let blocks_in_header = self.selected_exr_header().chunk_count; let channel_count = self.color_type().channel_count() as usize; From ca02fe8334390bae19c7da15b12ee81d142a7941 Mon Sep 17 00:00:00 2001 From: Johannes Vollmer <32042925+johannesvollmer@users.noreply.github.com> Date: Sat, 2 Apr 2022 13:35:20 +0200 Subject: [PATCH 6/7] undo accidentially including an unrelated file --- examples/encode_f32.rs | 22 ---------------------- 1 file changed, 22 deletions(-) delete mode 100644 examples/encode_f32.rs diff --git a/examples/encode_f32.rs b/examples/encode_f32.rs deleted file mode 100644 index f0c3c0402b..0000000000 --- a/examples/encode_f32.rs +++ /dev/null @@ -1,22 +0,0 @@ -//! An example of handling a 32-bit float image. -use std::env; -use std::path::Path; -use image::{Rgba32FImage, Rgba}; - -fn main() { - let (width, height) = (200, 200); - - // construct an image with 32-bit float pixels - let mut f32_buffer = Rgba32FImage::from_fn( - width as u32, height as u32, - |x, y| image::Rgba([ - 2.0 * (x as f32 / width as f32), - 2.0 * (y as f32 / height as f32), - (y+x) as f32 / (width+height) as f32, - 1.0, - ]) - ); - - // save the float image as exr - f32_buffer.save("generated_image.exr").unwrap(); -} From 3bdbe0834c948ac03767a953ee936e06a95a6154 Mon Sep 17 00:00:00 2001 From: Johannes Vollmer <32042925+johannesvollmer@users.noreply.github.com> Date: Thu, 7 Apr 2022 19:19:20 +0200 Subject: [PATCH 7/7] add backwards-compatible mutable progress callbacks --- src/image.rs | 43 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 39 insertions(+), 4 deletions(-) diff --git a/src/image.rs b/src/image.rs index c578bad970..2f34de16fc 100644 --- a/src/image.rs +++ b/src/image.rs @@ -686,8 +686,8 @@ pub trait ImageDecoder<'a>: Sized { self.read_image_with_progress(buf, |_| {}) } - /// Same as `read_image` but periodically calls the provided callback to give updates on loading - /// progress. + /// Same as `read_image_with_progress_mut`, but the callback cannot mutate state. + /// Use `read_image_with_progress_mut` if you want to mutate your state from within the callback. fn read_image_with_progress( self, buf: &mut [u8], @@ -720,6 +720,22 @@ pub trait ImageDecoder<'a>: Sized { Ok(()) } + /// Same as `read_image` but periodically calls the provided callback to give updates on loading + /// progress. + // TODO find a way to offer only _mut versions, eliminating refcells + fn read_image_with_progress_mut( + self, + buf: &mut [u8], + progress_callback: F, + ) -> ImageResult<()> { + let mutable_callback_cell = std::cell::RefCell::new(progress_callback); + self.read_image_with_progress(buf, |progress| { + if let Ok(mut progress_callback) = mutable_callback_cell.try_borrow_mut() { + progress_callback(progress) + } + }) + } + /// Set decoding limits for this decoder. See [`Limits`] for the different kinds of /// limits that is possible to set. /// @@ -755,6 +771,17 @@ pub trait ImageDecoderRect<'a>: ImageDecoder<'a> + Sized { self.read_rect_with_progress(x, y, width, height, buf, |_| {}) } + /// Same as `read_rect_with_progress_mut`, but the callback is not mutable. + fn read_rect_with_progress( + &mut self, + x: u32, + y: u32, + width: u32, + height: u32, + buf: &mut [u8], + progress_callback: F, + ) -> ImageResult<()>; + /// Decode a rectangular section of the image, periodically reporting progress. /// /// The output buffer will be filled with fields specified by @@ -767,7 +794,8 @@ pub trait ImageDecoderRect<'a>: ImageDecoder<'a> + Sized { /// /// This function will panic if the output buffer isn't at least /// `color_type().bytes_per_pixel() * color_type().channel_count() * width * height` bytes long. - fn read_rect_with_progress( + // TODO find a way to offer only _mut versions, eliminating refcells + fn read_rect_with_progress_mut( &mut self, x: u32, y: u32, @@ -775,7 +803,14 @@ pub trait ImageDecoderRect<'a>: ImageDecoder<'a> + Sized { height: u32, buf: &mut [u8], progress_callback: F, - ) -> ImageResult<()>; + ) -> ImageResult<()> { + let mutable_callback_cell = std::cell::RefCell::new(progress_callback); + self.read_rect_with_progress(x, y, width, height, buf,|progress| { + if let Ok(mut progress_callback) = mutable_callback_cell.try_borrow_mut() { + progress_callback(progress) + } + }) + } } /// AnimationDecoder trait