commit 3e960463bbac6a69369946d1081d19b1b9eeea45
author Olli Etuaho <oetuaho@nvidia.com> Thu Nov 12 15:58:39 2015 +0200
committer Olli Etuaho <oetuaho@nvidia.com> Thu Nov 19 16:14:13 2015 +0000
tree 4b47841157fdf0256fb4fd1ec30db79c93980e0f
parent bd163f6a497dcb1393b2b5f14ff89da7b5cec373

Detect when gl_FragData is indexed with != 0 in WebGL 2.0

WebGL 2.0 explicitly specifies it to be an error when gl_FragData is
indexed with anything else than constant zero in spec section
'GLSL ES 1.00 Fragment Shader Output'.

This doesn't apply to WebGL 1.0 or GLES.
dEQP-GLES2.functional.shaders.fragdata* test that dynamic indexing of
gl_FragData is allowed.

TEST=angle_unittests
BUG=angleproject:1210

Change-Id: Ib401242e7867f5e7943456b059dd8e24dc404098
Reviewed-on: https://chromium-review.googlesource.com/312045
Tested-by: Olli Etuaho <oetuaho@nvidia.com>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>

src/compiler/translator/ParseContext.cpp

diff --git a/src/compiler/translator/ParseContext.cpp b/src/compiler/translator/ParseContext.cpp
index 9676016..6b9ba3f 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -2724,6 +2724,11 @@
                   "array indexes for fragment outputs must be constant integral expressions");
             recover();
         }
+        else if (mShaderSpec == SH_WEBGL2_SPEC && baseExpression->getQualifier() == EvqFragData)
+        {
+            error(location, "", "[", "array index for gl_FragData must be constant zero");
+            recover();
+        }
     }
 
     if (indexConstantUnion)
@@ -2775,7 +2780,29 @@
 
             if (baseExpression->isArray())
             {
-                if (index >= baseExpression->getType().getArraySize())
+                if (baseExpression->getQualifier() == EvqFragData && index > 0)
+                {
+                    if (mShaderSpec == SH_WEBGL2_SPEC)
+                    {
+                        // Error has been already generated if index is not const.
+                        if (indexExpression->getQualifier() == EvqConst)
+                        {
+                            error(location, "", "[",
+                                  "array index for gl_FragData must be constant zero");
+                            recover();
+                        }
+                        safeIndex = 0;
+                    }
+                    else if (!isExtensionEnabled("GL_EXT_draw_buffers"))
+                    {
+                        outOfRangeError(outOfRangeIndexIsError, location, "", "[",
+                                        "array index for gl_FragData must be zero when "
+                                        "GL_EXT_draw_buffers is disabled");
+                        safeIndex = 0;
+                    }
+                }
+                // Only do generic out-of-range check if similar error hasn't already been reported.
+                if (safeIndex < 0 && index >= baseExpression->getType().getArraySize())
                 {
                     std::stringstream extraInfoStream;
                     extraInfoStream << "array index out of range '" << index << "'";
@@ -2783,15 +2810,6 @@
                     outOfRangeError(outOfRangeIndexIsError, location, "", "[", extraInfo.c_str());
                     safeIndex = baseExpression->getType().getArraySize() - 1;
                 }
-                else if (baseExpression->getQualifier() == EvqFragData && index > 0 &&
-                         !isExtensionEnabled("GL_EXT_draw_buffers"))
-                {
-                    outOfRangeError(
-                        outOfRangeIndexIsError, location, "", "[",
-                        "array indexes for gl_FragData must be zero when GL_EXT_draw_buffers is "
-                        "disabled");
-                    safeIndex = 0;
-                }
             }
             else if ((baseExpression->isVector() || baseExpression->isMatrix()) &&
                      baseExpression->getType().getNominalSize() <= index)

src/tests/compiler_tests/MalformedShader_test.cpp

diff --git a/src/tests/compiler_tests/MalformedShader_test.cpp b/src/tests/compiler_tests/MalformedShader_test.cpp
index 01f3edd..c75261c 100644
--- a/src/tests/compiler_tests/MalformedShader_test.cpp
+++ b/src/tests/compiler_tests/MalformedShader_test.cpp
@@ -68,6 +68,22 @@
     }
 };
 
+class MalformedWebGL2ShaderTest : public MalformedShaderTest
+{
+  public:
+    MalformedWebGL2ShaderTest() {}
+
+  protected:
+    void SetUp() override
+    {
+        ShBuiltInResources resources;
+        ShInitBuiltInResources(&resources);
+
+        mTranslator = new TranslatorESSL(GL_FRAGMENT_SHADER, SH_WEBGL2_SPEC);
+        ASSERT_TRUE(mTranslator->Init(resources));
+    }
+};
+
 // This is a test for a bug that used to exist in ANGLE:
 // Calling a function with all parameters missing should not succeed.
 TEST_F(MalformedShaderTest, FunctionParameterMismatch)
@@ -1218,3 +1234,22 @@
         FAIL() << "Shader compilation succeeded, expecting failure " << mInfoLog;
     }
 }
+
+// Test that indexing gl_FragData with a non-constant expression is forbidden in WebGL 2.0, even
+// when ANGLE is able to constant fold the index.
+// WebGL 2.0 spec section 'GLSL ES 1.00 Fragment Shader Output'
+TEST_F(MalformedWebGL2ShaderTest, IndexFragDataWithNonConstant)
+{
+    const std::string &shaderString =
+        "precision mediump float;\n"
+        "void main()\n"
+        "{\n"
+        "    for (int i = 0; i < 2; ++i) {\n"
+        "        gl_FragData[true ? 0 : i] = vec4(0.0);\n"
+        "    }\n"
+        "}\n";
+    if (compile(shaderString))
+    {
+        FAIL() << "Shader compilation succeeded, expecting failure " << mInfoLog;
+    }
+}