RESOLVED FIXED 42945
[Qt] getImageData(): Single-pass RGB->BGR and un-premultiplication
https://bugs.webkit.org/show_bug.cgi?id=42945
Summary [Qt] getImageData(): Single-pass RGB->BGR and un-premultiplication
Andreas Kling
Reported 2010-07-25 04:12:27 PDT
When retrieving the unmultiplied pixel data via CanvasRenderingContext2D's getImageData(), QtWebKit currently does the following: 1. QPixmap::toImage() (our canvas is backed by a QPixmap) 2. QImage::convertToFormat(ARGB32) 3. Copy image byte-by-byte to destination buffer swapping Red and Blue channels. Benchmarking with http://www.semantix.gr/statue.html I get these numbers before any changes: 23.43 FPS { 23.52, 23.49, 23.41, 23.46, 23.48, 23.35, 23.33 } We can optimize this as such: 1. QPixmap::toImage() 2. QImage::convertToFormat(ARGB32_PM) (no-op on raster paintengine) 3. Copy image pixel by pixel to destination buffer doing combined RGB->BGR and un-premultiplication. Same benchmark with the above algorithm: 25.19 FPS { 25.15, 25.22, 25.32, 25.31, 25.03, 25.07, 25.23 }
Attachments
Proposed patch (3.68 KB, patch)
2010-07-25 04:21 PDT, Andreas Kling
kling: review-
Proposed patch v2 (4.07 KB, patch)
2010-07-26 04:30 PDT, Andreas Kling
no flags
Andreas Kling
Comment 1 2010-07-25 04:21:46 PDT
Created attachment 62523 [details] Proposed patch
Kenneth Rohde Christiansen
Comment 2 2010-07-25 07:17:43 PDT
Comment on attachment 62523 [details] Proposed patch WebCore/platform/graphics/qt/ImageBufferQt.cpp:178 + QImage image = imageData.m_pixmap.toImage().convertToFormat(QImage::Format_ARGB32_Premultiplied); I think it would be nice with a comment here why you disconsider the bool multiplied, people might think that is an error in the future. WebCore/platform/graphics/qt/ImageBufferQt.cpp:212 + pixel = ((pixel << 16) & 0xff0000) | ((pixel >> 16) & 0xff) | (pixel & 0xff00ff00); Maybe you should explain the RGB->BGR swapping here, just like a little comment
Andreas Kling
Comment 3 2010-07-25 16:56:34 PDT
Comment on attachment 62523 [details] Proposed patch r- due to comments from John Brooks on IRC; this code should have a special path for opaque colors.
Andreas Kling
Comment 4 2010-07-26 04:30:27 PDT
Created attachment 62557 [details] Proposed patch v2 Added special path for un-premultiplication when alpha == 255. Also added comments like Kenneth requested.
Andreas Kling
Comment 5 2010-07-26 04:51:51 PDT
Comment on attachment 62557 [details] Proposed patch v2 Clearing flags on attachment: 62557 Committed r64041: <http://trac.webkit.org/changeset/64041>
Andreas Kling
Comment 6 2010-07-26 04:52:02 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.