Bug 90443

Summary: Prepare for parallel image decoder.
Product: WebKit Reporter: Dongseong Hwang <dongseong.hwang>
Component: ImagesAssignee: Nobody <webkit-unassigned>
Status: RESOLVED INVALID    
Severity: Normal CC: ap, dglazkov, gram, skyul, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 90375, 90935, 90758    
Attachments:
Description Flags
patch
none
patch v.2
none
patch v.3
none
patch v.4
webkit.review.bot: commit-queue-
patch v.5 none

Description Dongseong Hwang 2012-07-03 03:40:27 PDT
The key point of this patch is to change how BitmapImage uses ImageSource.

Originally, BitmapImage retrieves the frame data from ImageSource by calling the following methods:
- NativeImagePtr createFrameAtIndex(size_t);
- float frameDurationAtIndex(size_t);
- bool frameHasAlphaAtIndex(size_t);
- bool frameIsCompleteAtIndex(size_t);
- ImageOrientation orientationAtIndex(size_t) const;

Upon each method call, ImageSource synchronously decodes the image and returns the requested information.
Because all the methods above are inherently synchronous, it is not easy to make them parallel.

To implement parallel image decoders, these methods must be asynchronous. So I merged all these methods into a single method, requestFrameAtIndex(size_t), which requests all the frame data at once.
Note that this method has no return value.

Instead, BitmapImage now implements ImageSourceObserver and passes itself to ImageSource constructor.
Once image decoding is finished or cached image is available, ImageSource notifies BitmapImage via
ImageSourceObserver::didReceiveImageFrameData(size_t, const ImageFrameData&). ImageFrameData contains all the frame data including frame, duration, isAlpha, isComplete and orientation.
Comment 1 Dongseong Hwang 2012-07-03 03:45:18 PDT
Created attachment 150573 [details]
patch
Comment 2 Dongseong Hwang 2012-07-03 03:53:16 PDT
Created attachment 150574 [details]
patch v.2

missing changelog
Comment 3 Dongseong Hwang 2012-07-03 03:55:25 PDT
Created attachment 150575 [details]
patch v.3
Comment 4 Dongseong Hwang 2012-07-03 03:56:14 PDT
Created attachment 150576 [details]
patch v.4
Comment 5 WebKit Review Bot 2012-07-03 04:29:12 PDT
Comment on attachment 150576 [details]
patch v.4

Attachment 150576 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13136375
Comment 6 Dongseong Hwang 2012-07-03 04:50:13 PDT
Created attachment 150583 [details]
patch v.5
Comment 7 Dongseong Hwang 2012-07-03 05:04:13 PDT
patch v.5 for build fix of chromium.
Comment 8 Alexey Proskuryakov 2012-07-04 12:57:06 PDT
What kind of performance data says that this will be a performance improvement? I think that this idea has been considered and rejected in the past.
Comment 9 Alexey Proskuryakov 2012-07-04 13:03:17 PDT
Sorry, I see some explanation in bug 90375 now. This needs to be announced on webkit-dev in addition to normal review though.
Comment 10 Kwang Yul Seo 2012-07-04 14:21:58 PDT
(In reply to comment #9)
> Sorry, I see some explanation in bug 90375 now. This needs to be announced on webkit-dev in addition to normal review though.

Sure. I will announce this on webkit-dev.

Dungsung and I are writing up the design document so that reviewers can understand the overall architecture before they review the patches line by line.
Comment 11 Dongseong Hwang 2012-08-08 04:56:38 PDT
We are rewriting the preparation patches for async image decoding, so this bug is of no use.
Comment 12 Alexey Proskuryakov 2012-08-08 09:46:49 PDT
WONTFIX is not the correct resolution unless we agree that something is a bug, but refuse to ever fix it, see <https://bugs.webkit.org/page.cgi?id=fields.html#status>. Changing to INVALID.
Comment 13 Dongseong Hwang 2012-08-08 19:38:52 PDT
(In reply to comment #12)
> WONTFIX is not the correct resolution unless we agree that something is a bug, but refuse to ever fix it, see <https://bugs.webkit.org/page.cgi?id=fields.html#status>. Changing to INVALID.

Thanks for your instruction.
Comment 14 Eric Seidel (no email) 2012-08-12 03:00:33 PDT
Comment on attachment 150583 [details]
patch v.5

Cleared review? from attachment 150583 [details] so that this bug does not appear in http://webkit.org/pending-review.  If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).