make progress callback mutable #1694
Conversation
…uch as progress bars in a safe way)
|
The only case where this is less general is when |
|
Fixing the formatting... |
|
Well, not sure if that was the right button. My brain assumed it was the |
| @@ -152,10 +152,10 @@ impl<'a, R: 'a + Read + Seek> ImageDecoder<'a> for OpenExrDecoder<R> { | |||
| } | |||
|
|
|||
| // reads with or without alpha, depending on `self.alpha_preference` and `self.alpha_present_in_file` | |||
| fn read_image_with_progress<F: Fn(Progress)>( | |||
| fn read_image_with_progress<F: FnMut(Progress)>( | |||
There was a problem hiding this comment.
So, @HeroicKatora will there be any problem with recursion of the progress callback? If yes, is it even relevant, would it happen in the real world?
There was a problem hiding this comment.
No, I don't believe so. Only a really small fraction will be recursive, if any, and our default implementation handles this gracefully.
|
Is there a particular rush to expose this feature? It feels like a lot of churn to add read_image_with_progress_mut in |
|
No, we could also note this down for the next major release |
|
I've create a new branch |
|
todo:
|
|
@fintelia is the current stance to remove progress completely? |
|
That's the plan at the moment, yes. It turns out that having a method taking a callback function prevents the |
|
Maybe removing it now allows someone in the future to add it back in without breaking object safety. Let's remove it for now. Or perhaps the byte-level adapter for the input stream would be a good way to do it in the future, maybe completely separate from this crate. |
see #1488
note: this will break any downstream implementations of
ImageDecoder::read_image_with_progressandImageDecoderRect::read_rect_with_progress. However, it most cases, it should be easy to fix, just by making the callback parameter mutable. Only where the progress callback is captured or sent in multiple places, more changes might be required.which branch should it go?