[webkit-reviews] review denied: [Bug 103398] OpenCL version of FEColorMatrix : [Attachment 178807] proposed patch

[bugzilla-daemon at webkit.org]

Zoltan Herczeg <zherczeg at webkit.org> has denied Tamas Czene
<tczene at inf.u-szeged.hu>'s request for review:
Bug 103398: OpenCL version of FEColorMatrix
https://bugs.webkit.org/show_bug.cgi?id=103398

Attachment 178807: proposed patch
https://bugs.webkit.org/attachment.cgi?id=178807&action=review

------- Additional Comments from Zoltan Herczeg <zherczeg at webkit.org>
The patch looks good, only a few changes:

View in context: https://bugs.webkit.org/attachment.cgi?id=178807&action=review


> Source/WebCore/ChangeLog:4
> +	   OpenCL version of FEColorMatrix.
> +	   https://bugs.webkit.org/show_bug.cgi?id=103398

Where is the "revewed by" line?

> Source/WebCore/platform/graphics/gpu/opencl/OpenCLFEColorMatrix.cpp:52
> +    values[5]  * sourcePixel.x + values[6]  * sourcePixel.y + values[7]  *
sourcePixel.z + values[8]  * sourcePixel.w + values[9],
> +    values[10] * sourcePixel.x + values[11] * sourcePixel.y + values[12] *
sourcePixel.z + values[13] * sourcePixel.w + values[14],
> +    values[15] * sourcePixel.x + values[16] * sourcePixel.y + values[17] *
sourcePixel.z + values[18] * sourcePixel.w + values[19])

+4 space indentation

> Source/WebCore/platform/graphics/gpu/opencl/OpenCLFEColorMatrix.cpp:60
> +    sourcePixel.x * components[3] + sourcePixel.y * components[4] +
sourcePixel.z * components[5],	
> +    sourcePixel.x * components[6] + sourcePixel.y * components[7] +
sourcePixel.z * components[8],	
> +    sourcePixel.w)

ditto.

> Source/WebCore/platform/graphics/gpu/opencl/OpenCLFEColorMatrix.cpp:112
> +	   colorMatrix = 0;

return. 0 is an invalid value. And an ASSERT_NOT_REACHED()

> Source/WebCore/platform/graphics/gpu/opencl/OpenCLFEColorMatrix.cpp:141
> +    absolutePaintRect().x() - in->absolutePaintRect().location().x(),
> +    absolutePaintRect().y() - in->absolutePaintRect().location().y());

Indentation again.