Implement ESSL 3.00 shader input/output variable type rules

src/compiler/translator/ParseContext.cpp

@@ -880,6 +880,15 @@ bool TParseContext::arrayTypeErrorCheck(const TSourceLoc &line, const TPublicTyp
         error(line, "cannot declare arrays of arrays", TType(type).getCompleteString().c_str());
         return true;
     }
+    // In ESSL1.00 shaders, structs cannot be varying (section 4.3.5). This is checked elsewhere.
+    // In ESSL3.00 shaders, struct inputs/outputs are allowed but not arrays of structs (section
+    // 4.3.4).
+    if (mShaderVersion >= 300 && type.type == EbtStruct && sh::IsVarying(type.qualifier))
+    {
+        error(line, "cannot declare arrays of structs of this qualifier",
+              TType(type).getCompleteString().c_str());
+        return true;
+    }
 
     return false;
 }
@@ -1447,46 +1456,95 @@ TPublicType TParseContext::addFullySpecifiedType(TQualifier qualifier,
                 recover();
             }
         }
-        switch (qualifier)
+        if (sh::IsVarying(qualifier) || qualifier == EvqVertexIn || qualifier == EvqFragmentOut)
         {
-            case EvqSmoothIn:
-            case EvqSmoothOut:
-            case EvqVertexOut:
-            case EvqFragmentIn:
-            case EvqCentroidOut:
-            case EvqCentroidIn:
-                if (typeSpecifier.type == EbtBool)
-                {
-                    error(typeSpecifier.line, "cannot be bool", getQualifierString(qualifier));
-                    recover();
-                }
-                if (typeSpecifier.type == EbtInt || typeSpecifier.type == EbtUInt)
-                {
-                    error(typeSpecifier.line, "must use 'flat' interpolation here",
-                          getQualifierString(qualifier));
-                    recover();
-                }
-                break;
-
-            case EvqVertexIn:
-            case EvqFragmentOut:
-            case EvqFlatIn:
-            case EvqFlatOut:
-                if (typeSpecifier.type == EbtBool)
-                {
-                    error(typeSpecifier.line, "cannot be bool", getQualifierString(qualifier));
-                    recover();
-                }
-                break;
-
-            default:
-                break;
+            es3InputOutputTypeCheck(qualifier, typeSpecifier, typeSpecifier.line);
         }
     }
 
     return returnType;
 }
 
+void TParseContext::es3InputOutputTypeCheck(const TQualifier qualifier,
+                                            const TPublicType &type,
+                                            const TSourceLoc &qualifierLocation)
+{
+    // An input/output variable can never be bool or a sampler. Samplers are checked elsewhere.
+    if (type.type == EbtBool)
+    {
+        error(qualifierLocation, "cannot be bool", getQualifierString(qualifier));
+        recover();
+    }
+
+    // Specific restrictions apply for vertex shader inputs and fragment shader outputs.
+    switch (qualifier)
+    {
+        case EvqVertexIn:
+            // ESSL 3.00 section 4.3.4
+            if (type.array)
+            {
+                error(qualifierLocation, "cannot be array", getQualifierString(qualifier));
+                recover();
+            }
+            // Vertex inputs with a struct type are disallowed in singleDeclarationErrorCheck
+            return;
+        case EvqFragmentOut:
+            // ESSL 3.00 section 4.3.6
+            if (type.isMatrix())
+            {
+                error(qualifierLocation, "cannot be matrix", getQualifierString(qualifier));
+                recover();
+            }
+            // Fragment outputs with a struct type are disallowed in singleDeclarationErrorCheck
+            return;
+        default:
+            break;
+    }
+
+    // Vertex shader outputs / fragment shader inputs have a different, slightly more lenient set of
+    // restrictions.
+    bool typeContainsIntegers =
+        (type.type == EbtInt || type.type == EbtUInt || type.isStructureContainingType(EbtInt) ||
+         type.isStructureContainingType(EbtUInt));
+    if (typeContainsIntegers && qualifier != EvqFlatIn && qualifier != EvqFlatOut)
+    {
+        error(qualifierLocation, "must use 'flat' interpolation here",
+              getQualifierString(qualifier));
+        recover();
+    }
+
+    if (type.type == EbtStruct)
+    {
+        // ESSL 3.00 sections 4.3.4 and 4.3.6.
+        // These restrictions are only implied by the ESSL 3.00 spec, but
+        // the ESSL 3.10 spec lists these restrictions explicitly.
+        if (type.array)
+        {
+            error(qualifierLocation, "cannot be an array of structures",
+                  getQualifierString(qualifier));
+            recover();
+        }
+        if (type.isStructureContainingArrays())
+        {
+            error(qualifierLocation, "cannot be a structure containing an array",
+                  getQualifierString(qualifier));
+            recover();
+        }
+        if (type.isStructureContainingType(EbtStruct))
+        {
+            error(qualifierLocation, "cannot be a structure containing a structure",
+                  getQualifierString(qualifier));
+            recover();
+        }
+        if (type.isStructureContainingType(EbtBool))
+        {
+            error(qualifierLocation, "cannot be a structure containing a bool",
+                  getQualifierString(qualifier));
+            recover();
+        }
+    }
+}
+
 TIntermAggregate *TParseContext::parseSingleDeclaration(TPublicType &publicType,
                                                         const TSourceLoc &identifierOrTypeLocation,
                                                         const TString &identifier)

src/compiler/translator/ParseContext.h

@@ -140,6 +140,9 @@ class TParseContext : angle::NonCopyable
     bool layoutLocationErrorCheck(const TSourceLoc &location, const TLayoutQualifier &layoutQualifier);
     bool functionCallLValueErrorCheck(const TFunction *fnCandidate, TIntermAggregate *);
     void es3InvariantErrorCheck(const TQualifier qualifier, const TSourceLoc &invariantLocation);
+    void es3InputOutputTypeCheck(const TQualifier qualifier,
+                                 const TPublicType &type,
+                                 const TSourceLoc &qualifierLocation);
 
     const TPragma &pragma() const { return mDirectiveHandler.pragma(); }
     const TExtensionBehavior &extensionBehavior() const { return mDirectiveHandler.extensionBehavior(); }

src/compiler/translator/Types.cpp

@@ -201,6 +201,17 @@ bool TStructure::containsArrays() const
     return false;
 }
 
+bool TStructure::containsType(TBasicType type) const
+{
+    for (size_t i = 0; i < mFields->size(); ++i)
+    {
+        const TType *fieldType = (*mFields)[i]->type();
+        if (fieldType->getBasicType() == type || fieldType->isStructureContainingType(type))
+            return true;
+    }
+    return false;
+}
+
 bool TStructure::containsSamplers() const
 {
     for (size_t i = 0; i < mFields->size(); ++i)

src/compiler/translator/Types.h

@@ -124,6 +124,7 @@ class TStructure : public TFieldListCollection
         return mDeepestNesting;
     }
     bool containsArrays() const;
+    bool containsType(TBasicType type) const;
     bool containsSamplers() const;
 
     bool equals(const TStructure &other) const;
@@ -490,6 +491,11 @@ class TType
         return structure ? structure->containsArrays() : false;
     }
 
+    bool isStructureContainingType(TBasicType type) const
+    {
+        return structure ? structure->containsType(type) : false;
+    }
+
     bool isStructureContainingSamplers() const
     {
         return structure ? structure->containsSamplers() : false;
@@ -599,6 +605,16 @@ struct TPublicType
         return userDef->isStructureContainingArrays();
     }
 
+    bool isStructureContainingType(TBasicType type) const
+    {
+        if (!userDef)
+        {
+            return false;
+        }
+
+        return userDef->isStructureContainingType(type);
+    }
+
     bool isMatrix() const
     {
         return primarySize > 1 && secondarySize > 1;

src/tests/compiler_tests/ConstantFolding_test.cpp

@@ -271,11 +271,13 @@ TEST_F(ConstantFoldingTest, Fold2x2MatrixInverse)
     const std::string &shaderString =
         "#version 300 es\n"
         "precision mediump float;\n"
-        "out mat2 my_Matrix;"
+        "in float i;\n"
+        "out vec2 my_Vec;\n"
         "void main() {\n"
         "   const mat2 m2 = inverse(mat2(2.0f, 3.0f,\n"
         "                                5.0f, 7.0f));\n"
-        "   my_Matrix = m2;\n"
+        "   mat2 m = m2 * mat2(i);\n"
+        "   my_Vec = m[0];\n"
         "}\n";
     compile(shaderString);
     float inputElements[] =
@@ -300,12 +302,14 @@ TEST_F(ConstantFoldingTest, Fold3x3MatrixInverse)
     const std::string &shaderString =
         "#version 300 es\n"
         "precision mediump float;\n"
-        "out mat3 my_Matrix;"
+        "in float i;\n"
+        "out vec3 my_Vec;\n"
         "void main() {\n"
         "   const mat3 m3 = inverse(mat3(11.0f, 13.0f, 19.0f,\n"
         "                                23.0f, 29.0f, 31.0f,\n"
         "                                37.0f, 41.0f, 43.0f));\n"
-        "   my_Matrix = m3;\n"
+        "   mat3 m = m3 * mat3(i);\n"
+        "   my_Vec = m3[0];\n"
         "}\n";
     compile(shaderString);
     float inputElements[] =
@@ -333,13 +337,15 @@ TEST_F(ConstantFoldingTest, Fold4x4MatrixInverse)
     const std::string &shaderString =
         "#version 300 es\n"
         "precision mediump float;\n"
-        "out mat4 my_Matrix;"
+        "in float i;\n"
+        "out vec4 my_Vec;\n"
         "void main() {\n"
         "   const mat4 m4 = inverse(mat4(29.0f, 31.0f, 37.0f, 41.0f,\n"
         "                                43.0f, 47.0f, 53.0f, 59.0f,\n"
         "                                61.0f, 67.0f, 71.0f, 73.0f,\n"
         "                                79.0f, 83.0f, 89.0f, 97.0f));\n"
-        "   my_Matrix = m4;\n"
+        "   mat4 m = m4 * mat4(i);\n"
+        "   my_Vec = m[0];\n"
         "}\n";
     compile(shaderString);
     float inputElements[] =
@@ -448,12 +454,14 @@ TEST_F(ConstantFoldingTest, Fold3x3MatrixTranspose)
     const std::string &shaderString =
         "#version 300 es\n"
         "precision mediump float;\n"
-        "out mat3 my_Matrix;"
+        "in float i;\n"
+        "out vec3 my_Vec;\n"
         "void main() {\n"
         "   const mat3 m3 = transpose(mat3(11.0f, 13.0f, 19.0f,\n"
         "                                  23.0f, 29.0f, 31.0f,\n"
         "                                  37.0f, 41.0f, 43.0f));\n"
-        "   my_Matrix = m3;\n"
+        "   mat3 m = m3 * mat3(i);\n"
+        "   my_Vec = m[0];\n"
         "}\n";
     compile(shaderString);
     float inputElements[] =

src/tests/compiler_tests/MalformedShader_test.cpp

@@ -49,11 +49,25 @@ class MalformedShaderTest : public testing::Test
 
   protected:
     std::string mInfoLog;
-
-  private:
     TranslatorESSL *mTranslator;
 };
 
+class MalformedVertexShaderTest : public MalformedShaderTest
+{
+  public:
+    MalformedVertexShaderTest() {}
+
+  protected:
+    void SetUp() override
+    {
+        ShBuiltInResources resources;
+        ShInitBuiltInResources(&resources);
+
+        mTranslator = new TranslatorESSL(GL_VERTEX_SHADER, SH_GLES3_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)
@@ -763,3 +777,98 @@ TEST_F(MalformedShaderTest, UniformArray)
         FAIL() << "Shader compilation failed, expecting success " << mInfoLog;
     }
 }
+
+// Fragment shader input variables cannot be arrays of structs (ESSL 3.00 section 4.3.4)
+TEST_F(MalformedShaderTest, FragmentInputArrayOfStructs)
+{
+    const std::string &shaderString =
+        "#version 300 es\n"
+        "precision mediump float;\n"
+        "struct S {\n"
+        "    vec4 foo;\n"
+        "};\n"
+        "in S i[2];\n"
+        "out vec4 my_FragColor;\n"
+        "void main() {\n"
+        "    my_FragColor = i[0].foo;\n"
+        "}\n";
+    if (compile(shaderString))
+    {
+        FAIL() << "Shader compilation succeeded, expecting failure " << mInfoLog;
+    }
+}
+
+// Vertex shader inputs can't be arrays (ESSL 3.00 section 4.3.4)
+// This test is testing the case where the array brackets are after the variable name, so
+// the arrayness isn't known when the type and qualifiers are initially parsed.
+TEST_F(MalformedVertexShaderTest, VertexShaderInputArray)
+{
+    const std::string &shaderString =
+        "#version 300 es\n"
+        "precision mediump float;\n"
+        "in vec4 i[2];\n"
+        "void main() {\n"
+        "    gl_Position = i[0];\n"
+        "}\n";
+    if (compile(shaderString))
+    {
+        FAIL() << "Shader compilation succeeded, expecting failure " << mInfoLog;
+    }
+}
+
+// Vertex shader inputs can't be arrays (ESSL 3.00 section 4.3.4)
+// This test is testing the case where the array brackets are after the type.
+TEST_F(MalformedVertexShaderTest, VertexShaderInputArrayType)
+{
+    const std::string &shaderString =
+        "#version 300 es\n"
+        "precision mediump float;\n"
+        "in vec4[2] i;\n"
+        "void main() {\n"
+        "    gl_Position = i[0];\n"
+        "}\n";
+    if (compile(shaderString))
+    {
+        FAIL() << "Shader compilation succeeded, expecting failure " << mInfoLog;
+    }
+}
+
+// Fragment shader inputs can't contain booleans (ESSL 3.00 section 4.3.4)
+TEST_F(MalformedShaderTest, FragmentShaderInputStructWithBool)
+{
+    const std::string &shaderString =
+        "#version 300 es\n"
+        "precision mediump float;\n"
+        "struct S {\n"
+        "    bool foo;\n"
+        "};\n"
+        "in S s;\n"
+        "out vec4 my_FragColor;\n"
+        "void main() {\n"
+        "    my_FragColor = vec4(0.0);\n"
+        "}\n";
+    if (compile(shaderString))
+    {
+        FAIL() << "Shader compilation succeeded, expecting failure " << mInfoLog;
+    }
+}
+
+// Fragment shader inputs without a flat qualifier can't contain integers (ESSL 3.00 section 4.3.4)
+TEST_F(MalformedShaderTest, FragmentShaderInputStructWithInt)
+{
+    const std::string &shaderString =
+        "#version 300 es\n"
+        "precision mediump float;\n"
+        "struct S {\n"
+        "    int foo;\n"
+        "};\n"
+        "in S s;\n"
+        "out vec4 my_FragColor;\n"
+        "void main() {\n"
+        "    my_FragColor = vec4(0.0);\n"
+        "}\n";
+    if (compile(shaderString))
+    {
+        FAIL() << "Shader compilation succeeded, expecting failure " << mInfoLog;
+    }
+}