Review Request 109998: kjs: Fix CanvasImageData eating the color value by doing wrong alpha premultiply

[Bernd Buschinski]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109998/
-----------------------------------------------------------

Review request for kdelibs.


Description
-------

previous knowledge:
- the CanvasImageData allows setting the single color red/green/blue/alpha-values for each pixel
- the default for all pixel is Rgba 0,0,0,0 and must not be changed
- you can set the color-rgba values in any order, there is no fixed order

the bug:
We use QImage::Format_ARGB32_Premultiplied beacuse it faster for many operations.
As we directly set the red/green/blue/alpha color value via QImage::scanLine,
we must premultiply it, to keep it in the QImage::Format_ARGB32_Premultiplied format.

But as CanvasImageData does not specify any order of setting the values or does not know what values are set,
we might end up doing a wrong premultiply.
For premultiply we MUST have the correct alpha value, if it is not set it is assumed 0 ( the default),
but if we premultiply a given (for example) red value with 0, its get zero, the red color value is gone.
Gone and no way to recover it.
So for example if we set for each pixel (in this order) red=255,green=0,blue=0,alpha=255,
we get a black image instead of a red one.

my solution:
As I don't want to introduce a "is set" check for the alpha value, I suggest we use ARG32 instead of ARGB32_Premultiplied.
This allows us to set the color values in any order we want.

possible problems:
ARG32 is slower than ARGB32_Premultiplied.
Yes, but as far as I can see we do not display the image directly, it is "only" used as in
  QPainter::setCompositionMode(QPainter::CompositionMode_Source);
  QPainter::drawImage(x,y,ourImage);
to get painted on another image.
I hope QPainter uses some sse magic, so the paint is faster than our custom premultiply.

btw:
While (afaik) it is possible to re-use that not-displayed ImageData, I only found javascript code 
creating new ImageData for each animation-frame. In this case ARGB32_Premultiplied would be more performant.


Diffs
-----

  khtml/html/html_canvasimpl.cpp c7fce27 

Diff: http://git.reviewboard.kde.org/r/109998/diff/


Testing
-------

http://www.w3schools.com/tags/canvas_createimagedata.asp
shows the problem, as it shows a black image but it should be red.


Thanks,

Bernd Buschinski

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20130413/db1391f6/attachment.htm>