Bug 93590

Summary: Add NonImmediateBitmapImage to mark it is safe to decode asynchronously.
Product: WebKit Reporter: Dongseong Hwang <dongseong.hwang>
Component: ImagesAssignee: Nobody <webkit-unassigned>
Status: UNCONFIRMED    
Severity: Normal CC: hclam, jamesr, nick, simon.fraser, skyul, zoltan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 93467, 94190    
Bug Blocks: 90375, 90935    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Dongseong Hwang
Reported 2012-08-09 01:05:10 PDT
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.
Attachments
Patch (26.30 KB, patch)
2012-08-09 01:24 PDT, Dongseong Hwang 		no flags
DetailsFormatted DiffDiff
Patch (25.12 KB, patch)
2012-08-10 04:47 PDT, Dongseong Hwang 		no flags
DetailsFormatted DiffDiff
Patch (25.10 KB, patch)
2012-08-15 23:47 PDT, Dongseong Hwang 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Dongseong Hwang
Comment 1 2012-08-09 01:24:45 PDT
Created attachment 157418 [details] Patch
Nick Carter
Comment 2 2012-08-09 14:24:23 PDT
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.
Hin-Chung Lam
Comment 3 2012-08-09 15:56:10 PDT
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.
Dongseong Hwang
Comment 4 2012-08-10 04:47:42 PDT
Created attachment 157704 [details] Patch
Dongseong Hwang
Comment 5 2012-08-10 04:49:08 PDT
(In reply to comment #2) > Some ideas: FutureBitmapImage, DelayedBitmapImage, EventualBitmapImage. I agree with you. Thanks for good suggestions.
Dongseong Hwang
Comment 6 2012-08-10 04:51:56 PDT
(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.
Dongseong Hwang
Comment 7 2012-08-15 23:47:49 PDT
Created attachment 158722 [details] Patch
Dongseong Hwang
Comment 8 2012-08-15 23:48:23 PDT
(In reply to comment #7) > Created an attachment (id=158722) [details] > Patch Rebased to the upstream.
Andreas Kling
Comment 9 2014-02-05 10:52:10 PST
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.
Note You need to log in before you can comment on or make changes to this bug.