[webkit-reviews] review granted: [Bug 73011] Change ImageData to reference Uint8ClampedArray rather than CanvasPixelArray : [Attachment 138106] Patch

[bugzilla-daemon at webkit.org]

Oliver Hunt <oliver at apple.com> has granted Kenneth Russell <kbr at google.com>'s
request for review:
Bug 73011: Change ImageData to reference Uint8ClampedArray rather than
CanvasPixelArray
https://bugs.webkit.org/show_bug.cgi?id=73011

Attachment 138106: Patch
https://bugs.webkit.org/attachment.cgi?id=138106&action=review

------- Additional Comments from Oliver Hunt <oliver at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=138106&action=review


> Source/WebCore/platform/graphics/cg/ImageBufferDataCG.cpp:-131
> -    if (rect.x() < 0 || rect.y() < 0 || endx > size.width() || endy >
size.height())
> -	   memset(data, 0, result->length());

why is this removed?  Does uint8array zero initialize?	if so we probably want
a non-zero initializing version so we can avoid the double fill here.

> Source/WebCore/platform/graphics/cg/ImageBufferDataCG.cpp:343
> +void ImageBufferData::putData(Uint8ClampedArray*& source, const IntSize&
sourceSize, const IntRect& sourceRect, const IntPoint& destPoint, const
IntSize& size, bool accelerateRendering, bool unmultiplied, float
resolutionScale)

Errr, not a change in this patch per se, but why Uint8ClampedArray*&?  why does
source	need to be a reference?

> Source/WebCore/platform/graphics/filters/FEBlend.cpp:110
> +	   unsigned char alphaA = srcPixelArrayA->item(pixelOffset + 3);

Just for my own sanity: is item() non-virtual?

> Source/WebCore/platform/graphics/skia/ImageBufferSkia.cpp:-249
> -    if (rect.x() < 0
> -	   || rect.y() < 0
> -	   || rect.maxX() > size.width()
> -	   || rect.maxY() > size.height())
> -	   memset(data, 0, result->length());
> -

and again, are we introducing an unnecessary clear and set?