Title: [125128] trunk
Revision
125128
Author
ach...@adobe.com
Date
2012-08-08 17:56:13 -0700 (Wed, 08 Aug 2012)

Log Message

[CSS Shaders] Invalid shaders should act as pass-through filters
https://bugs.webkit.org/show_bug.cgi?id=93405

Reviewed by Dean Jackson.

Source/WebCore:

If the shader fails to apply then clearShaderResult will just copy the result of the previous filter
to the output of the current filter.

Test: css3/filters/custom/invalid-custom-filter-shader.html

* platform/graphics/filters/FECustomFilter.cpp:
(WebCore::FECustomFilter::platformApplySoftware):
(WebCore):
(WebCore::FECustomFilter::clearShaderResult):
(WebCore::FECustomFilter::applyShader):
* platform/graphics/filters/FECustomFilter.h:
(FECustomFilter):
* rendering/style/StyleCustomFilterProgram.h: The test was exposing the fact that if the shaders were referencing the same
file then StyleCustomFilterProgram will never complete the load. Having the same file for both the vertex and the fragment shader
cannot really work, because the shaders would not compile anyway, thus triggering an invalid shader.
I'm fixing it part of this change because the current test actually exposes that on Safari Mac builds. See the note in the LayoutTest/ChangeLog.
(WebCore::StyleCustomFilterProgram::notifyFinished):

LayoutTests:

Note that invalid-custom-filter-shader-expected.html is actually using another builtin filter,
just to make sure we trigger the whole FilterEffectRenderer on the images, otherwise the result will be
slightly different because of the conversion errors. Having another filter like grayscale(0) will trigger
the same precission errors on the result of the expected html.

* css3/filters/custom/invalid-custom-filter-shader-expected.html: Added.
* css3/filters/custom/invalid-custom-filter-shader.html: Added.
* css3/filters/resources/invalid-shader.vs: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (125127 => 125128)


--- trunk/LayoutTests/ChangeLog	2012-08-09 00:52:29 UTC (rev 125127)
+++ trunk/LayoutTests/ChangeLog	2012-08-09 00:56:13 UTC (rev 125128)
@@ -1,3 +1,19 @@
+2012-08-08  Alexandru Chiculita  <ach...@adobe.com>
+
+        [CSS Shaders] Invalid shaders should act as pass-through filters
+        https://bugs.webkit.org/show_bug.cgi?id=93405
+
+        Reviewed by Dean Jackson.
+
+        Note that invalid-custom-filter-shader-expected.html is actually using another builtin filter,
+        just to make sure we trigger the whole FilterEffectRenderer on the images, otherwise the result will be
+        slightly different because of the conversion errors. Having another filter like grayscale(0) will trigger
+        the same precission errors on the result of the expected html.
+
+        * css3/filters/custom/invalid-custom-filter-shader-expected.html: Added.
+        * css3/filters/custom/invalid-custom-filter-shader.html: Added.
+        * css3/filters/resources/invalid-shader.vs: Added.
+
 2012-08-08  Brady Eidson  <beid...@apple.com>
 
         Google search query text reverts to original search query after multiple searches

Added: trunk/LayoutTests/css3/filters/custom/invalid-custom-filter-shader-expected.html (0 => 125128)


--- trunk/LayoutTests/css3/filters/custom/invalid-custom-filter-shader-expected.html	                        (rev 0)
+++ trunk/LayoutTests/css3/filters/custom/invalid-custom-filter-shader-expected.html	2012-08-09 00:56:13 UTC (rev 125128)
@@ -0,0 +1,34 @@
+<!doctype html>
+<html>
+    <head>
+        <script>
+            if (window.testRunner) {
+                // Wait for the images to be loaded.
+                window.testRunner.waitUntilDone();
+            }
+            
+            function notifyDone()
+            {
+                // We need to run the tests after the downloading succeeded or fails.
+                if (window.testRunner)
+                    window.testRunner.notifyDone();
+            }
+        </script>
+        <style>
+            img
+            {
+                /* Note that we use grayscale(0) just to make sure that it triggers the whole FilterEffectRenderer on the images, 
+                otherwise the result will be slightly different because of the precision errors. Having another filter like grayscale(0) 
+                will trigger the same precission errors and will act as a similar pass-through filter. */
+                -webkit-filter: grayscale(0);
+            }
+        </style>
+    </head>
+    <body _onload_="notifyDone()">
+        <img src="" />
+        <img src="" />
+        <img src="" />
+        <img src="" />
+        <img src="" />
+    </body>
+</html>

Added: trunk/LayoutTests/css3/filters/custom/invalid-custom-filter-shader.html (0 => 125128)


--- trunk/LayoutTests/css3/filters/custom/invalid-custom-filter-shader.html	                        (rev 0)
+++ trunk/LayoutTests/css3/filters/custom/invalid-custom-filter-shader.html	2012-08-09 00:56:13 UTC (rev 125128)
@@ -0,0 +1,53 @@
+<!-- This file should test that custom-filters do not render when shaders are invalid. -->
+<!doctype html>
+<html>
+    <head>
+        <title>Testing that custom-filters do not render when shaders are invalid</title>
+        <script>
+            if (window.testRunner) {
+                window.testRunner.overridePreference("WebKitCSSCustomFilterEnabled", "1");
+                window.testRunner.overridePreference("WebKitWebGLEnabled", "1");
+                window.testRunner.dumpAsText(true);
+                window.testRunner.waitUntilDone();
+            }
+            
+            function runTest()
+            {
+                // We need to run the tests after the downloading succeeded or fails.
+                if (window.testRunner)
+                    window.testRunner.notifyDone();
+            }
+        </script>
+        <style>
+            .invalid_vertex_shader
+            {
+                -webkit-filter: custom(url('../resources/invalid-shader.vs'));
+            }
+            .invalid_fragment_shader
+            {
+                -webkit-filter: custom(none url('../resources/invalid-shader.vs'));
+            }
+            .both_shaders_but_invalid_vertex_shader
+            {
+                -webkit-filter: custom(url('../resources/invalid-shader.vs') url('../resources/color-offset.fs'));
+            }
+
+            .both_shaders_but_invalid_fragment_shader
+            {
+                -webkit-filter: custom(url('../resources/vertex-offset.vs') url('../resources/invalid-shader.vs'));
+            }
+            .both_shaders_invalid
+            {
+                -webkit-filter: custom(url('../resources/invalid-shader.vs') url('../resources/invalid-shader.vs'));
+            }
+        </style>
+    </head>
+    <body _onload_="runTest()">
+        <!-- Test of invalid custom filter shaders. You should see 5 blocks of color bars, all the same -->
+        <img class="invalid_vertex_shader" src="" />
+        <img class="invalid_fragment_shader" src="" />
+        <img class="both_shaders_but_invalid_vertex_shader" src="" />
+        <img class="both_shaders_but_invalid_fragment_shader" src="" />
+        <img class="both_shaders_invalid" src="" />
+    </body>
+</html>

Added: trunk/LayoutTests/css3/filters/resources/invalid-shader.vs (0 => 125128)


--- trunk/LayoutTests/css3/filters/resources/invalid-shader.vs	                        (rev 0)
+++ trunk/LayoutTests/css3/filters/resources/invalid-shader.vs	2012-08-09 00:56:13 UTC (rev 125128)
@@ -0,0 +1,2 @@
+This is not a real vertex shader. It is just invalid shader code.
+It is used to test that the filter is not drawn when the shader is invalid.
\ No newline at end of file

Modified: trunk/Source/WebCore/ChangeLog (125127 => 125128)


--- trunk/Source/WebCore/ChangeLog	2012-08-09 00:52:29 UTC (rev 125127)
+++ trunk/Source/WebCore/ChangeLog	2012-08-09 00:56:13 UTC (rev 125128)
@@ -1,3 +1,28 @@
+2012-08-08  Alexandru Chiculita  <ach...@adobe.com>
+
+        [CSS Shaders] Invalid shaders should act as pass-through filters
+        https://bugs.webkit.org/show_bug.cgi?id=93405
+
+        Reviewed by Dean Jackson.
+
+        If the shader fails to apply then clearShaderResult will just copy the result of the previous filter
+        to the output of the current filter.
+
+        Test: css3/filters/custom/invalid-custom-filter-shader.html
+
+        * platform/graphics/filters/FECustomFilter.cpp:
+        (WebCore::FECustomFilter::platformApplySoftware):
+        (WebCore):
+        (WebCore::FECustomFilter::clearShaderResult):
+        (WebCore::FECustomFilter::applyShader):
+        * platform/graphics/filters/FECustomFilter.h:
+        (FECustomFilter):
+        * rendering/style/StyleCustomFilterProgram.h: The test was exposing the fact that if the shaders were referencing the same
+        file then StyleCustomFilterProgram will never complete the load. Having the same file for both the vertex and the fragment shader
+        cannot really work, because the shaders would not compile anyway, thus triggering an invalid shader. 
+        I'm fixing it part of this change because the current test actually exposes that on Safari Mac builds. See the note in the LayoutTest/ChangeLog.
+        (WebCore::StyleCustomFilterProgram::notifyFinished):
+
 2012-08-08  Adam Barth  <aba...@webkit.org>
 
         Rewire the same-origin checks for the _javascript_Core bindings through BindingSecurity

Modified: trunk/Source/WebCore/platform/graphics/filters/FECustomFilter.cpp (125127 => 125128)


--- trunk/Source/WebCore/platform/graphics/filters/FECustomFilter.cpp	2012-08-09 00:52:29 UTC (rev 125127)
+++ trunk/Source/WebCore/platform/graphics/filters/FECustomFilter.cpp	2012-08-09 00:56:13 UTC (rev 125128)
@@ -123,18 +123,37 @@
 
 void FECustomFilter::platformApplySoftware()
 {
+    if (!applyShader())
+        clearShaderResult();
+}
+
+void FECustomFilter::clearShaderResult()
+{
+    clearResult();
     Uint8ClampedArray* dstPixelArray = createUnmultipliedImageResult();
     if (!dstPixelArray)
         return;
 
     FilterEffect* in = inputEffect(0);
+    setIsAlphaImage(in->isAlphaImage());
     IntRect effectDrawingRect = requestedRegionOfInputImageData(in->absolutePaintRect());
+    in->copyUnmultipliedImage(dstPixelArray, effectDrawingRect);
+}
+
+bool FECustomFilter::applyShader()
+{
+    Uint8ClampedArray* dstPixelArray = createUnmultipliedImageResult();
+    if (!dstPixelArray)
+        return false;
+
+    FilterEffect* in = inputEffect(0);
+    IntRect effectDrawingRect = requestedRegionOfInputImageData(in->absolutePaintRect());
     RefPtr<Uint8ClampedArray> srcPixelArray = in->asUnmultipliedImage(effectDrawingRect);
     
     IntSize newContextSize(effectDrawingRect.size());
     bool hadContext = m_context;
     if (!m_context && !initializeContext())
-        return;
+        return false;
     m_context->makeContextCurrent();
     
     if (!hadContext || m_contextSize != newContextSize)
@@ -143,12 +162,12 @@
 #if !PLATFORM(BLACKBERRY) // BlackBerry defines its own Texture class.
     // Do not draw the filter if the input image cannot fit inside a single GPU texture.
     if (m_inputTexture->tiles().numTilesX() != 1 || m_inputTexture->tiles().numTilesY() != 1)
-        return;
+        return false;
 #endif
     
     // The shader had compiler errors. We cannot draw anything.
     if (!m_compiledProgram->isInitialized())
-        return;
+        return false;
 
     m_context->bindFramebuffer(GraphicsContext3D::FRAMEBUFFER, m_frameBuffer);
     m_context->viewport(0, 0, newContextSize.width(), newContextSize.height());
@@ -162,6 +181,8 @@
     
     ASSERT(static_cast<size_t>(newContextSize.width() * newContextSize.height() * 4) == dstPixelArray->length());
     m_context->readPixels(0, 0, newContextSize.width(), newContextSize.height(), GraphicsContext3D::RGBA, GraphicsContext3D::UNSIGNED_BYTE, dstPixelArray->data());
+
+    return true;
 }
 
 bool FECustomFilter::initializeContext()

Modified: trunk/Source/WebCore/platform/graphics/filters/FECustomFilter.h (125127 => 125128)


--- trunk/Source/WebCore/platform/graphics/filters/FECustomFilter.h	2012-08-09 00:52:29 UTC (rev 125127)
+++ trunk/Source/WebCore/platform/graphics/filters/FECustomFilter.h	2012-08-09 00:56:13 UTC (rev 125128)
@@ -72,6 +72,8 @@
                    CustomFilterOperation::MeshType);
     ~FECustomFilter();
     
+    bool applyShader();
+    void clearShaderResult();
     bool initializeContext();
     void deleteRenderBuffers();
     void resizeContext(const IntSize& newContextSize);

Modified: trunk/Source/WebCore/rendering/style/StyleCustomFilterProgram.h (125127 => 125128)


--- trunk/Source/WebCore/rendering/style/StyleCustomFilterProgram.h	2012-08-09 00:52:29 UTC (rev 125127)
+++ trunk/Source/WebCore/rendering/style/StyleCustomFilterProgram.h	2012-08-09 00:56:13 UTC (rev 125128)
@@ -106,9 +106,10 @@
     {
         if (resource->errorOccurred())
             return;
+        // Note that m_cachedVertexShader might be equal to m_cachedFragmentShader and it would only get one event in that case.
         if (resource == m_cachedVertexShader.get())
             m_isVertexShaderLoaded = true;
-        else if (resource == m_cachedFragmentShader.get())
+        if (resource == m_cachedFragmentShader.get())
             m_isFragmentShaderLoaded = true;
         if (isLoaded())
             notifyClients();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to