Bug 90443 – Prepare for parallel image decoder.

• Component: Images
• Status: RESOLVED INVALID

[Dongseong Hwang]

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.

[Dongseong Hwang]

Created attachment 150573 [details]
patch

[Dongseong Hwang]

Created attachment 150574 [details]
patch v.2

missing changelog

[Dongseong Hwang]

Created attachment 150575 [details]
patch v.3

[Dongseong Hwang]

Created attachment 150576 [details]
patch v.4

[WebKit Review Bot]

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]

Created attachment 150583 [details]
patch v.5

[Dongseong Hwang]

patch v.5 for build fix of chromium.

[Alexey Proskuryakov]

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]

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]

(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]

We are rewriting the preparation patches for async image decoding, so this bug is of no use.

[Alexey Proskuryakov]

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]

(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 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).