----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109998/#review31244 -----------------------------------------------------------
This review has been submitted with commit d01c7f821a9b1f0415fbce668a24341aadd61eff by Bernd Buschinski to branch master. - Commit Hook On April 13, 2013, 3:34 p.m., Bernd Buschinski wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/109998/ > ----------------------------------------------------------- > > (Updated April 13, 2013, 3:34 p.m.) > > > 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 > >
