RESOLVED INVALID 90443
Prepare for parallel image decoder.
https://bugs.webkit.org/show_bug.cgi?id=90443
Summary Prepare for parallel image decoder.
Dongseong Hwang
Reported 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.
Attachments
patch (32.81 KB, patch)
2012-07-03 03:45 PDT, Dongseong Hwang
no flags
patch v.2 (24.87 KB, patch)
2012-07-03 03:53 PDT, Dongseong Hwang
no flags
patch v.3 (24.87 KB, patch)
2012-07-03 03:55 PDT, Dongseong Hwang
no flags
patch v.4 (37.11 KB, patch)
2012-07-03 03:56 PDT, Dongseong Hwang
webkit.review.bot: commit-queue-
patch v.5 (39.31 KB, patch)
2012-07-03 04:50 PDT, Dongseong Hwang
no flags
Dongseong Hwang
Comment 1 2012-07-03 03:45:18 PDT
Dongseong Hwang
Comment 2 2012-07-03 03:53:16 PDT
Created attachment 150574 [details] patch v.2 missing changelog
Dongseong Hwang
Comment 3 2012-07-03 03:55:25 PDT
Created attachment 150575 [details] patch v.3
Dongseong Hwang
Comment 4 2012-07-03 03:56:14 PDT
Created attachment 150576 [details] patch v.4
WebKit Review Bot
Comment 5 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
Dongseong Hwang
Comment 6 2012-07-03 04:50:13 PDT
Created attachment 150583 [details] patch v.5
Dongseong Hwang
Comment 7 2012-07-03 05:04:13 PDT
patch v.5 for build fix of chromium.
Alexey Proskuryakov
Comment 8 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.
Alexey Proskuryakov
Comment 9 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.
Kwang Yul Seo
Comment 10 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.
Dongseong Hwang
Comment 11 2012-08-08 04:56:38 PDT
We are rewriting the preparation patches for async image decoding, so this bug is of no use.
Alexey Proskuryakov
Comment 12 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.
Dongseong Hwang
Comment 13 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.
Eric Seidel (no email)
Comment 14 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).
Note You need to log in before you can comment on or make changes to this bug.