[Webkit-unassigned] [Bug 90375] Parallel image decoders

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=90375





--- Comment #67 from Hin-Chung Lam <hclam at google.com>  2012-08-02 19:49:06 PST ---
> You already made BitmapImage::currentFrameOrientation() and BitmapImage::currentFrameHasAlpha() not decode the current frame in the previous patch.
> However, this patch is little bit different. :)

Yeah sorry about going backwards.

> in BitmapImage::startAnimation about 433 line
> 
>     ensureFrameIsCached(nextFrame);          <-- add this line.
>     if (!m_allDataReceived && !frameIsCompleteAtIndex(nextFrame)) {
>         return;
>     }

I think just unplugging image decoding from currentFrameHasAlpha() can get us going with with everything except multiframe ICO and GIF already. The benefit of supporting ICO and GIF seems small given the complexity to do it correctly.

My thought is frameIsCompleteAtIndex() and frameDurationAtIndex() are only needed for GIF animation, which your parallel image decoding don't support anyway. Thinking about what happens if we make animated GIF decoding asynchronous, I think we'll run into problems if frameIsCompleteAtIndex() doesn't do decoding. If you look into startAnimation() (the for loop), it actually looks to see if the next frame is complete before switching to the new frame, and because decoding of that frame has not started it will never be completed and hence animation will stall. That's why I back off from removing decoding from frameIsCompleteAtIndex() and frameDurationAtIndex().

startAnimation() is meaningful for multiframe images anyway and with your patch this is not parallel. I suggest we exclude this at the moment. Make single frame image decoding work great first, and then multiframe images (which involves fixing frameIsCompleteAtIndex and frameDurationAtIndex).

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.