[webkit-reviews] review granted: [Bug 103728] Optimize ColorMatrix filter : [Attachment 176934] patch

[bugzilla-daemon at webkit.org]

Dirk Schulze <krit at webkit.org> has granted Zoltan Herczeg
<zherczeg at webkit.org>'s request for review:
Bug 103728: Optimize ColorMatrix filter
https://bugs.webkit.org/show_bug.cgi?id=103728

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

------- Additional Comments from Dirk Schulze <krit at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=176934&action=review


Looks reasonable to me. r=me. I am just not sure about the following:

> Source/WebCore/platform/graphics/filters/FEColorMatrix.cpp:136
> +    if (filterType == FECOLORMATRIX_TYPE_SATURATE) {
> +	   components[0] = (0.213 + 0.787 * values[0]);
> +	   components[1] = (0.715 - 0.715 * values[0]);
> +	   components[2] = (0.072 - 0.072 * values[0]);
> +	   components[3] = (0.213 - 0.213 * values[0]);
> +	   components[4] = (0.715 + 0.285 * values[0]);
> +	   components[5] = (0.072 - 0.072 * values[0]);
> +	   components[6] = (0.213 - 0.213 * values[0]);
> +	   components[7] = (0.715 - 0.715 * values[0]);
> +	   components[8] = (0.072 + 0.928 * values[0]);
> +    } else if (filterType == FECOLORMATRIX_TYPE_HUEROTATE) {
> +	   float cosHue = cos(values[0] * piFloat / 180);
> +	   float sinHue = sin(values[0] * piFloat / 180);
> +	   components[0] = 0.213 + cosHue * 0.787 - sinHue * 0.213;
> +	   components[1] = 0.715 - cosHue * 0.715 - sinHue * 0.715;
> +	   components[2] = 0.072 - cosHue * 0.072 + sinHue * 0.928;
> +	   components[3] = 0.213 - cosHue * 0.213 + sinHue * 0.143;
> +	   components[4] = 0.715 + cosHue * 0.285 + sinHue * 0.140;
> +	   components[5] = 0.072 - cosHue * 0.072 - sinHue * 0.283;
> +	   components[6] = 0.213 - cosHue * 0.213 - sinHue * 0.787;
> +	   components[7] = 0.715 - cosHue * 0.715 + sinHue * 0.715;
> +	   components[8] = 0.072 + cosHue * 0.928 + sinHue * 0.072;
> +    }

Does it make sense to move this in inline functions? It looks so strange here
and makes it less readable. Please consider it before landing.