Bug 74652 – [CSS Shaders] Move CustomFilterOperation to the platform layer

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Alexandru Chiculita]

Move CustomFilterOperation to the platform layer and use a PlatformShader instead of a CachedShader.

Quoted from previous discussions with Chris Marrin:

1) Make an abstract base class called Shader (or maybe PlatformShader or ShaderClient or something). This has a pure virtual method to get the shader string.

2) Subclass StyleShader from Shader and implement the get shader method.

3) Replace the methods that take/return StyleShader with those that take/return Shader.

4) Move CustomFilterOperation down into platform along with the others.

[Alexandru Chiculita]

Created attachment 121267 [details]
Patch V1

[Chris Marrin]

Comment on attachment 121267 [details]
Patch V1

Great patch. But I'm really concerned that we have only 3 custom filter tests in LayoutTests/css3/filters. And two of those are just testing style, so they don't exercise any of this new code. Please add at least a couple of tests which exercise the tolerance of missing shaders and the use of the cache (write two shaders which use the same vertex or fragment shader, etc.).

[Alexandru Chiculita]

Created attachment 121669 [details]
Patch V2

Added two new tests.

[Alexandru Chiculita]

Created attachment 121671 [details]
Patch V3

This time including binary files.

[Chris Marrin]

Comment on attachment 121671 [details]
Patch V3

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

r=me, but please add text to the test cases so a successful test is obvious without looking at the pixel results

> LayoutTests/css3/filters/custom-filter-shader-cache-expected.txt:1
> +     

You should really have something output here. At least say something like "Test of cached custom filter shaders. You should see 12 blocks of color bars, with increasing lightness from left to right"

> LayoutTests/css3/filters/missing-custom-filter-shader-expected.txt:1
> +    

Same here. At least say something like "Test of missing custom filter shaders. You should see 5 blocks of color bars, all the same"

[Simon Fraser (smfr)]

Comment on attachment 121671 [details]
Patch V3

Custom filter tests always fail (bug 75047) so I don't see how you can be adding more.

[Alexandru Chiculita]

(In reply to comment #6)
> (From update of attachment 121671 [details])
> Custom filter tests always fail (bug 75047) so I don't see how you can be adding more.

New tests have window.layoutTestController.overridePreference("WebKitWebGLEnabled", "1"); which seems to do the trick about enabling WebGL in DRT (at least on SnowLeopard it works, not sure about Lion).

For the old test case I've did the same in https://bugs.webkit.org/show_bug.cgi?id=75842 .

[Alexandru Chiculita]

Landed in http://trac.webkit.org/changeset/104702. Closing bug.