ColorMatrix recalculates a lot of values in every pixels.
Created attachment 176934 [details] patch
The strange thing is that the result is not claped between [0..1]. And I couldn't find anything in the specification what happens if the result is > 1 or < 0. Anyway, this is a really powerful filter.
(In reply to comment #2) > The strange thing is that the result is not claped between [0..1]. And I couldn't find anything in the specification what happens if the result is > 1 or < 0. Anyway, this is a really powerful filter. We clapped in the past, but decided to remove it from the spec and follow Opera (which did not clamp). Does the your result clamp?
Comment on attachment 176934 [details] patch 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.
Thanks for the review! > 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. Yes, I did it. Patch landed as: http://trac.webkit.org/changeset/136661