Bug 35957 – [Qt] Do not use QImageReader::imageCount in QtWebKit

• Status: RESOLVED
• Resolution: INVALID
• Component: Images

[Holger Freyther]

In theory decoding animations should work like the following. We determine the number of frames and then WebCore starts animating and asks us for frame 1, 2, 3, 4, ..., 20 and the throw away some images to save memory 1-8, and when we are at the end n, we have to go back to image 1.

In Qt terms this would work by calling QImageReader::imageCount and then using QImageReader::jumpToImage. Qt 4.7 has gained the abilitiy to index the gif and determine the number of images but jumpToImage is not implemented. This would mean we need to add error handling to the ImageDecoderQt.cpp in case we can not jumpToImage. On top of that the jumpToImage will not be implemented for gif.

Besides the error handling we would need to add code to detect if we access the image in order, and if we don't access it in order recreate the QImageReader. E.g. this would happen if we are at image n of a large animation and need to get back to 'n'.

All this will make the ImageDecoderQt.cpp a lot more complicated without any real gain.

[Holger Freyther]

Created attachment 50377 [details]
First attempt at the removal

With the latest changes to QGifHandler::imageCount this will likely introduce a slowdown but doing the thing is the best we can do in WebCore to assure things will work properly.

[Tor Arne Vestbø]

Please follow the QtWebKit bug reporting guidelines when reporting bugs.

See http://trac.webkit.org/wiki/QtWebKitBugs

Specifically:

  - The 'QtWebKit' component should only be used for bugs/features in the
    public QtWebKit API layer, not to signify that the bug is specific to
    the Qt port of WebKit

      http://trac.webkit.org/wiki/QtWebKitBugs#Component

  - Add the keyword 'Qt' to signal that it's a Qt-related bug

      http://trac.webkit.org/wiki/QtWebKitBugs#Keywords

[Simon Hausmann]

Comment on attachment 50377 [details]
First attempt at the removal

Sad, but the right thing to do. I thought that the changes actually went into Qt 4.6?

Is it worth considering to use the WebCore GIF image decoder instead?

[Eric Seidel (no email)]

Attachment 50377 [details] was posted by a committer and has review+, assigning to Holger Freyther for commit.

[Holger Freyther]

Comment on attachment 50377 [details]
First attempt at the removal

I have stopped working on QtWebKit for now, so someone else needs to take over here.

[Zoltan Horvath]

(In reply to comment #3)

> Is it worth considering to use the WebCore GIF image decoder instead?

I think it's not, because WebCore's GIFImageDecoder also reads the whole data to determine the frame count of the gif. (see WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp:76)

[Zoltan Horvath]

This looks reasonable, I'm going to land it soon.

[Holger Freyther]

(In reply to comment #7)
> This looks reasonable, I'm going to land it soon.

Please make a performance test. :)

[Zoltan Horvath]

(In reply to comment #8)
>
> Please make a performance test. :)

I did the measurement.

r79444, Qt 4.7.1

clean:
76.5 ms +/- 1.07%

patched:
74.3 ms +/- 0.65%

I started the timer before the load method of QtTestBrowser's LauncherWindow and stopped at loadFinished slot. The QtTestBrowser was running in an xvfb. It's even faster with this change. 

Should I land it now?

[Zoltan Horvath]

I've uploaded the test case to this location (too big to upload here):
http://zoltan.sed.hu/webkit/gifloadtest.tar

[Holger Freyther]

(In reply to comment #9)
> (In reply to comment #8)
> >
> > Please make a performance test. :)
> 
> I did the measurement.

The other part of the test is memory usage. E.g. have one of this videos as a gif and let it play twice and see how memory usage is changing... sorry to be the pain here.

[Jocelyn Turcotte]

=== Bulk closing of Qt bugs ===

If you believe that this bug report is still relevant for a non-Qt port of webkit.org, please re-open it and remove [Qt] from the summary.

If you believe that this is still an important QtWebKit bug, please fill a new report at https://bugreports.qt-project.org and add a link to this issue. See http://qt-project.org/wiki/ReportingBugsInQt for additional guidelines.