WebKit could decode images directly to QPixmap instead of converting from QImage to QPixmap. This has the advantage of reducing memory consumption, because the images do not have to be twice in memory during the conversion. Concretely, if a decoded image takes 10 mbytes of memory, converting it will currently use 20 mbytes of memory. Using pixmap directy would leave us a 10 mbytes of memory consumption.
That would be a really nice change.
Created attachment 59029 [details] Patch This patch require changes in Qt to reduce the memory usage. The change also has a side effect if the graphic system is OpenGL. With the patch, a compressed texture can be loaded as an image in WebKit.
Attachment 59029 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 WebCore/platform/graphics/qt/ImageDecoderQt.cpp:192: One line control clauses should not use braces. [whitespace/braces] [4] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 59029 [details] did not build on qt: Build output: http://webkit-commit-queue.appspot.com/results/3321328
Created attachment 59030 [details] Patch Fix the style.
Attachment 59030 [details] did not build on qt: Build output: http://webkit-commit-queue.appspot.com/results/3302306
(In reply to comment #6) > Attachment 59030 [details] did not build on qt: > Build output: http://webkit-commit-queue.appspot.com/results/3302306 To the reviewer, it does not build because 40620 has not landed yet. If it is still not landed tomorrow, I'll do it manually.
Comment on attachment 59030 [details] Patch LGTM, cq- as the patch in Qt is not available on gitorious yet and waiting for final review. A few questions: 192 else { 193 QImage img; 194 const bool imageLoaded = m_reader->read(&img); 195 if (imageLoaded) { 196 pixmap = QPixmap::fromImage(img); 197 pixmapLoaded = true; 198 } 199 } So we do not support animations? gif etc? Can imageCount() be 0?
Created attachment 59033 [details] Patch Add the case "imageCount == 0" thanks to Kenneth's comments on IRC.
Attachment 59033 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 WebCore/platform/graphics/qt/ImageDecoderQt.cpp:191: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 59033 [details] did not build on qt: Build output: http://webkit-commit-queue.appspot.com/results/3318304
The Qt patch will be applied to Qt 4.6 as well? if not we need an ifdef for checking the Qt version.
Comment on attachment 59033 [details] Patch You can re-set the cq when the patches are in Qt.
Comment on attachment 59030 [details] Patch Cleared Kenneth Rohde Christiansen's review+ from obsolete attachment 59030 [details] so that this bug does not appear in http://webkit.org/pending-commit.
(In reply to comment #12) > The Qt patch will be applied to Qt 4.6 as well? if not we need an ifdef for checking the Qt version. It will not be applied to 4.6, but that should not be a problem. The previous implementation of QPixmap::loadFromData() uses ImageReader, exactly what we do in ImageReaderQt.
Comment on attachment 59033 [details] Patch Clearing flags on attachment: 59033 Committed r61534: <http://trac.webkit.org/changeset/61534>
All reviewed patches have been landed. Closing bug.
There is a useless conversion between QImage and QPixmap in RGBA32Buffer::zeroFill(). See bug 59242.
Oh, no! It is the attachment, the bug is 40910 :S (In reply to comment #18) > There is a useless conversion between QImage and QPixmap in RGBA32Buffer::zeroFill(). See bug 59242.
Revision r61534 cherry-picked into qtwebkit-2.0 with commit 7a3bddf10bee16ee9ff176d0e99d100afe8a0782
Created attachment 60555 [details] Update to take advantage of new APIs in Qt We added new API to Qt 4.7 in order to do in-place color conversion in the all cases. This patch update WebKit to take advantages of this new API.
Comment on attachment 60555 [details] Update to take advantage of new APIs in Qt r=me, you can set qc+ if this is already in public Qt repo.
Comment on attachment 60555 [details] Update to take advantage of new APIs in Qt Yup, it landed in 4.7 recently: c16162214eb8757a62e221c34d38cefc402e5b05
Comment on attachment 60555 [details] Update to take advantage of new APIs in Qt WebCore/platform/graphics/qt/ImageDecoderQt.cpp:200 + pixmap = QPixmap::fromImage(img); I believe this function can return a null pixmap if we run out of memory (I've seen this actually on Symbian).
Created attachment 60606 [details] Update to take advantage of new APIs in Qt + null pixmap > (From update of attachment 60555 [details]) > WebCore/platform/graphics/qt/ImageDecoderQt.cpp:200 > + pixmap = QPixmap::fromImage(img); > I believe this function can return a null pixmap if we run out of memory (I've seen this actually on Symbian). Yep you are right, the pixmap could be null after this call. I updated the patch to simply abort as soon as we get a null pixmap.
Comment on attachment 60606 [details] Update to take advantage of new APIs in Qt + null pixmap r=me, this version is much nicer :)
Committed r65227: <http://trac.webkit.org/changeset/65227>
Revision r65227 cherry-picked into qtwebkit-2.1 with commit f1c7842276d4ca19ad19083b8182b2d1351ebc4f