Because image decoding is used in various contexts in WebKit, it is simply impossible to make all decoding asynchronous. Instead, we selectively use async image decoders where possible and use the existing sequential image decoders otherwise. RetainedModeBitmapImage wraps a BitmapImage and helps decide when it is safe and beneficial to use async image decoders.
Created attachment 157418 [details] Patch
While I understand the intended analogy to retained/immediate graphics APIs, the "RetainedMode" designation doesn't seem to describe what actually happens. I'd prefer this name indicate more clearly that the decode may happen in the future, and not complete before the draw operation. I think the dichotomy that exists here is not between "Immediate/Retained", but "Immediate/Eventual". Some ideas: FutureBitmapImage, DelayedBitmapImage, EventualBitmapImage.
Comment on attachment 157418 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=157418&action=review > Source/WebCore/platform/graphics/RetainedModeBitmapImage.cpp:1 > +/* I think a better name would be NonImmediateBitmapImage. It is unclear from the name what RetainedModeBitmapImage does. > Source/WebCore/platform/graphics/RetainedModeBitmapImage.cpp:40 > + , m_retainedModeCount(0) This can be a simple boolean instead of counter. > Source/WebCore/platform/graphics/RetainedModeBitmapImage.cpp:46 > + RetainedModeSaver retainedModeSaver(this); There's no need to have this because this method doesn't trigger image decoding. > Source/WebCore/platform/graphics/RetainedModeBitmapImage.cpp:106 > + RetainedModeSaver retainedModeSaver(this); It will be more clear to only have this line in this method and draw(). > Source/WebCore/platform/graphics/RetainedModeBitmapImage.cpp:148 > +RetainedModeBitmapImage* toRetainedModeBitmapImage(Image* image) Looks like there's no user of this function other than toRetainedModeBitmapImageIfPossible, I'd combine them.
Created attachment 157704 [details] Patch
(In reply to comment #2) > Some ideas: FutureBitmapImage, DelayedBitmapImage, EventualBitmapImage. I agree with you. Thanks for good suggestions.
(In reply to comment #3) > I think a better name would be NonImmediateBitmapImage. It is unclear from the name what RetainedModeBitmapImage does. Ok, I changed. > This can be a simple boolean instead of counter. Absolutely! > There's no need to have this because this method doesn't trigger image decoding. Yes. > It will be more clear to only have this line in this method and draw(). Now only NonImmediateBitmapImage::draw() and NonImmediateBitmapImage::nativeImageForCurrentFrame() have this line. > Looks like there's no user of this function other than toRetainedModeBitmapImageIfPossible, I'd combine them. Done.
Created attachment 158722 [details] Patch
(In reply to comment #7) > Created an attachment (id=158722) [details] > Patch Rebased to the upstream.
Comment on attachment 158722 [details] Patch Clearing review flag on patches from before 2014. If this patch is still relevant, please reset the r? flag.