Bug 42945 - [Qt] getImageData(): Single-pass RGB->BGR and un-premultiplication
Summary: [Qt] getImageData(): Single-pass RGB->BGR and un-premultiplication
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: QtWebKit Unassigned
URL:
Keywords: Performance, Qt, QtTriaged
Depends on:
Blocks:
 
Reported: 2010-07-25 04:12 PDT by Andreas Kling
Modified: 2019-05-03 18:55 PDT (History)
7 users (show)

See Also:


Attachments
Proposed patch (3.68 KB, patch)
2010-07-25 04:21 PDT, Andreas Kling
kling: review-
Details | Formatted Diff | Diff
Proposed patch v2 (4.07 KB, patch)
2010-07-26 04:30 PDT, Andreas Kling
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Kling 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 }
Comment 1 Andreas Kling 2010-07-25 04:21:46 PDT
Created attachment 62523 [details]
Proposed patch
Comment 2 Kenneth Rohde Christiansen 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
Comment 3 Andreas Kling 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.
Comment 4 Andreas Kling 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.
Comment 5 Andreas Kling 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>
Comment 6 Andreas Kling 2010-07-26 04:52:02 PDT
All reviewed patches have been landed.  Closing bug.