Remove "FIXME : share gaussian formula between shaders" in TextureMapperShaderManager. Moreover, this patch can increase performance, because it calculates gaussian kernel only one time.
Created attachment 147958 [details] patch v.1
Comment on attachment 147958 [details] patch v.1 View in context: https://bugs.webkit.org/attachment.cgi?id=147958&action=review > Source/WebCore/platform/graphics/texmap/TextureMapperShaderManager.cpp:43 > + STRINGIFY_VAL(src) It is needless, but I amended it for symmetric with FRAGMENT_SHADER.
Comment on attachment 147958 [details] patch v.1 View in context: https://bugs.webkit.org/attachment.cgi?id=147958&action=review > Source/WebCore/platform/graphics/texmap/TextureMapperShaderManager.cpp:46 > + STRINGIFY_VAL(src) I add STRINGIFY_VAL in order to use macro in shader code, especially GAUSSIAN_KERNEL_HALF_WIDTH and GAUSSIAN_KERNEL_STEP.
Comment on attachment 147958 [details] patch v.1 View in context: https://bugs.webkit.org/attachment.cgi?id=147958&action=review > Source/WebCore/ChangeLog:8 > + [Texmap] Share gaussian formula between shaders in TextureMapperShaderManager. > + https://bugs.webkit.org/show_bug.cgi?id=89277 > + > + Reviewed by NOBODY (OOPS!). > + > + No new tests. This patch doesn't change behavior. You're doing a lot more than that, actually moving to a more accurate gaussian blur... I like that, but please describe in the Changelog. > Source/WebCore/platform/graphics/texmap/TextureMapperShaderManager.cpp:672 > + } > + // Normalize the kernel Extra line > Source/WebCore/platform/graphics/texmap/TextureMapperShaderManager.cpp:678 > + } > + return kernel; Extra line
Created attachment 148000 [details] patch v.2
I'm glad because you like this. :)
Comment on attachment 148000 [details] patch v.2 View in context: https://bugs.webkit.org/attachment.cgi?id=148000&action=review Awesome! Please fix nit-picks before committing. > Source/WebCore/platform/graphics/texmap/TextureMapperShaderManager.cpp:658 > + return exp(- (x * x) / 2.f); Remove space between - and ( Use 2. instead of 2.f > Source/WebCore/platform/graphics/texmap/TextureMapperShaderManager.cpp:678 > + if (!prepared) { > + kernel[0] = gauss(0); > + float sum = kernel[0]; > + for (unsigned i = 1; i < GAUSSIAN_KERNEL_HALF_WIDTH; ++i) { > + kernel[i] = gauss(i * GAUSSIAN_KERNEL_STEP); > + sum += 2 * kernel[i]; > + } > + > + // Normalize the kernel > + float scale = 1 / sum; > + for (unsigned i = 0; i < GAUSSIAN_KERNEL_HALF_WIDTH; ++i) > + kernel[i] *= scale; > + prepared = true; > + } Better to do if (prepared) return kernel;
Created attachment 148004 [details] patch v.3 Good advice.
Comment on attachment 148004 [details] patch v.3 Clearing flags on attachment: 148004 Committed r120545: <http://trac.webkit.org/changeset/120545>
All reviewed patches have been landed. Closing bug.