Parse binding layout qualifier for opaque types

This patch adds binding layout qualifier support for opaque types. Binding layout qualifier on blocks is not yet supported. This includes support for GLSL output and some minor simplification of related functionality in ParseContext. TEST=angle_unittests, dEQP-GLES31.functional.layout_binding.* BUG=angleproject:1442 Change-Id: I53fb505b5a539bccee70613f3969fba81965ae84 Reviewed-on: https://chromium-review.googlesource.com/441586 Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Corentin Wallez <cwallez@chromium.org> Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
2026-01-07 06:09:57 +03:00 · 2017-02-13 16:00:12 +00:00
parent 92db39e89d
commit 4336489ffc
9 changed files with 330 additions and 51 deletions
--- a/samples/shader_translator/shader_translator.cpp
+++ b/samples/shader_translator/shader_translator.cpp
@@ -226,6 +226,8 @@ int main(int argc, char *argv[])
            if (spec != SH_GLES2_SPEC && spec != SH_WEBGL_SPEC)
            {
                resources.MaxDrawBuffers = 8;
+                resources.MaxVertexTextureImageUnits = 16;
+                resources.MaxTextureImageUnits       = 16;
            }
            ShHandle compiler = 0;
            switch (FindShaderType(argv[0]))
--- a/src/compiler/translator/BaseTypes.h
+++ b/src/compiler/translator/BaseTypes.h
@@ -609,6 +609,8 @@ struct TLayoutQualifier
    // Compute shader layout qualifiers.
    sh::WorkGroupSize localSize;

+    int binding;
+
    // Image format layout qualifier
    TLayoutImageInternalFormat imageInternalFormat;

@@ -625,6 +627,7 @@ struct TLayoutQualifier
        layoutQualifier.blockStorage       = EbsUnspecified;

        layoutQualifier.localSize.fill(-1);
+        layoutQualifier.binding  = -1;
        layoutQualifier.numViews = -1;

        layoutQualifier.imageInternalFormat = EiifUnspecified;
@@ -633,9 +636,9 @@ struct TLayoutQualifier

    bool isEmpty() const
    {
-        return location == -1 && numViews == -1 && matrixPacking == EmpUnspecified &&
-               blockStorage == EbsUnspecified && !localSize.isAnyValueSet() &&
-               imageInternalFormat == EiifUnspecified;
+        return location == -1 && binding == -1 && numViews == -1 &&
+               matrixPacking == EmpUnspecified && blockStorage == EbsUnspecified &&
+               !localSize.isAnyValueSet() && imageInternalFormat == EiifUnspecified;
    }

    bool isCombinationValid() const
@@ -643,8 +646,8 @@ struct TLayoutQualifier
        bool workSizeSpecified = localSize.isAnyValueSet();
        bool numViewsSet       = (numViews != -1);
        bool otherLayoutQualifiersSpecified =
-            (location != -1 || matrixPacking != EmpUnspecified || blockStorage != EbsUnspecified ||
-             imageInternalFormat != EiifUnspecified);
+            (location != -1 || binding != -1 || matrixPacking != EmpUnspecified ||
+             blockStorage != EbsUnspecified || imageInternalFormat != EiifUnspecified);

        // we can have either the work group size specified, or number of views, or the other layout
        // qualifiers.
--- a/src/compiler/translator/OutputGLSLBase.cpp
+++ b/src/compiler/translator/OutputGLSLBase.cpp
@@ -72,6 +72,12 @@ bool NeedsToWriteLayoutQualifier(const TType &type)
    {
        return true;
    }
+
+    if (IsOpaqueType(type.getBasicType()) && layoutQualifier.binding != -1)
+    {
+        return true;
+    }
+
    if (IsImage(type.getBasicType()) && layoutQualifier.imageInternalFormat != EiifUnspecified)
    {
        return true;
@@ -79,6 +85,30 @@ bool NeedsToWriteLayoutQualifier(const TType &type)
    return false;
 }

+class CommaSeparatedListItemPrefixGenerator
+{
+  public:
+    CommaSeparatedListItemPrefixGenerator() : mFirst(true) {}
+  private:
+    bool mFirst;
+
+    friend TInfoSinkBase &operator<<(TInfoSinkBase &out,
+                                     CommaSeparatedListItemPrefixGenerator &gen);
+};
+
+TInfoSinkBase &operator<<(TInfoSinkBase &out, CommaSeparatedListItemPrefixGenerator &gen)
+{
+    if (gen.mFirst)
+    {
+        gen.mFirst = false;
+    }
+    else
+    {
+        out << ", ";
+    }
+    return out;
+}
+
 }  // namespace

 TOutputGLSLBase::TOutputGLSLBase(TInfoSinkBase &objSink,
@@ -174,18 +204,32 @@ void TOutputGLSLBase::writeLayoutQualifier(const TType &type)
    const TLayoutQualifier &layoutQualifier = type.getLayoutQualifier();
    out << "layout(";

+    CommaSeparatedListItemPrefixGenerator listItemPrefix;
+
    if (type.getQualifier() == EvqFragmentOut || type.getQualifier() == EvqVertexIn)
    {
        if (layoutQualifier.location >= 0)
        {
-            out << "location = " << layoutQualifier.location;
+            out << listItemPrefix << "location = " << layoutQualifier.location;
        }
    }

-    if (IsImage(type.getBasicType()) && layoutQualifier.imageInternalFormat != EiifUnspecified)
+    if (IsOpaqueType(type.getBasicType()))
    {
-        ASSERT(type.getQualifier() == EvqTemporary || type.getQualifier() == EvqUniform);
-        out << getImageInternalFormatString(layoutQualifier.imageInternalFormat);
+        if (layoutQualifier.binding >= 0)
+        {
+            out << listItemPrefix << "binding = " << layoutQualifier.binding;
+        }
+    }
+
+    if (IsImage(type.getBasicType()))
+    {
+        if (layoutQualifier.imageInternalFormat != EiifUnspecified)
+        {
+            ASSERT(type.getQualifier() == EvqTemporary || type.getQualifier() == EvqUniform);
+            out << listItemPrefix
+                << getImageInternalFormatString(layoutQualifier.imageInternalFormat);
+        }
    }

    out << ") ";
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -128,6 +128,8 @@ TParseContext::TParseContext(TSymbolTable &symt,
      mComputeShaderLocalSizeDeclared(false),
      mNumViews(-1),
      mMaxNumViews(resources.MaxViewsOVR),
+      mMaxImageUnits(resources.MaxImageUnits),
+      mMaxCombinedTextureImageUnits(resources.MaxCombinedTextureImageUnits),
      mDeclaringFunction(false)
 {
    mComputeShaderLocalSize.fill(-1);
@@ -1028,6 +1030,8 @@ bool TParseContext::declareVariable(const TSourceLoc &line,
 {
    ASSERT((*variable) == nullptr);

+    checkBindingIsValid(line, type);
+
    bool needsReservedCheck = true;

    // gl_LastFragData may be redeclared with a new precision qualifier
@@ -1081,7 +1085,7 @@ void TParseContext::checkIsParameterQualifierValid(

    if (!IsImage(type->getBasicType()))
    {
-        checkIsMemoryQualifierNotSpecified(typeQualifier.memoryQualifier, line);
+        checkMemoryQualifierIsNotSpecified(typeQualifier.memoryQualifier, line);
    }
    else
    {
@@ -1270,17 +1274,28 @@ void TParseContext::singleDeclarationErrorCheck(const TPublicType &publicType,
    }
    else
    {
+        checkInternalFormatIsNotSpecified(identifierLocation, layoutQualifier.imageInternalFormat);

-        if (!checkInternalFormatIsNotSpecified(identifierLocation,
-                                               layoutQualifier.imageInternalFormat))
-        {
-            return;
-        }
+        checkMemoryQualifierIsNotSpecified(publicType.memoryQualifier, identifierLocation);
+    }
+}

-        if (!checkIsMemoryQualifierNotSpecified(publicType.memoryQualifier, identifierLocation))
-        {
-            return;
-        }
+void TParseContext::checkBindingIsValid(const TSourceLoc &identifierLocation, const TType &type)
+{
+    TLayoutQualifier layoutQualifier = type.getLayoutQualifier();
+    int arraySize                    = type.isArray() ? type.getArraySize() : 1;
+    if (IsImage(type.getBasicType()))
+    {
+        checkImageBindingIsValid(identifierLocation, layoutQualifier.binding, arraySize);
+    }
+    else if (IsSampler(type.getBasicType()))
+    {
+        checkSamplerBindingIsValid(identifierLocation, layoutQualifier.binding, arraySize);
+    }
+    else
+    {
+        ASSERT(!IsOpaqueType(type.getBasicType()));
+        checkBindingIsNotSpecified(identifierLocation, layoutQualifier.binding);
    }
 }

@@ -1314,16 +1329,44 @@ bool TParseContext::checkWorkGroupSizeIsNotSpecified(const TSourceLoc &location,
    return true;
 }

-bool TParseContext::checkInternalFormatIsNotSpecified(const TSourceLoc &location,
+void TParseContext::checkInternalFormatIsNotSpecified(const TSourceLoc &location,
                                                      TLayoutImageInternalFormat internalFormat)
 {
    if (internalFormat != EiifUnspecified)
    {
        error(location, "invalid layout qualifier: only valid when used with images",
              getImageInternalFormatString(internalFormat));
-        return false;
    }
-    return true;
+}
+
+void TParseContext::checkBindingIsNotSpecified(const TSourceLoc &location, int binding)
+{
+    if (binding != -1)
+    {
+        error(location,
+              "invalid layout qualifier: only valid when used with opaque types or blocks",
+              "binding");
+    }
+}
+
+void TParseContext::checkImageBindingIsValid(const TSourceLoc &location, int binding, int arraySize)
+{
+    // Expects arraySize to be 1 when setting binding for only a single variable.
+    if (binding >= 0 && binding + arraySize > mMaxImageUnits)
+    {
+        error(location, "image binding greater than gl_MaxImageUnits", "binding");
+    }
+}
+
+void TParseContext::checkSamplerBindingIsValid(const TSourceLoc &location,
+                                               int binding,
+                                               int arraySize)
+{
+    // Expects arraySize to be 1 when setting binding for only a single variable.
+    if (binding >= 0 && binding + arraySize > mMaxCombinedTextureImageUnits)
+    {
+        error(location, "sampler binding greater than maximum texture units", "binding");
+    }
 }

 void TParseContext::functionCallLValueErrorCheck(const TFunction *fnCandidate,
@@ -1845,35 +1888,29 @@ void TParseContext::checkLocalVariableConstStorageQualifier(const TQualifierWrap
    }
 }

-bool TParseContext::checkIsMemoryQualifierNotSpecified(const TMemoryQualifier &memoryQualifier,
+void TParseContext::checkMemoryQualifierIsNotSpecified(const TMemoryQualifier &memoryQualifier,
                                                       const TSourceLoc &location)
 {
    if (memoryQualifier.readonly)
    {
        error(location, "Only allowed with images.", "readonly");
-        return false;
    }
    if (memoryQualifier.writeonly)
    {
        error(location, "Only allowed with images.", "writeonly");
-        return false;
    }
    if (memoryQualifier.coherent)
    {
        error(location, "Only allowed with images.", "coherent");
-        return false;
    }
    if (memoryQualifier.restrictQualifier)
    {
        error(location, "Only allowed with images.", "restrict");
-        return false;
    }
    if (memoryQualifier.volatileQualifier)
    {
        error(location, "Only allowed with images.", "volatile");
-        return false;
    }
-    return true;
 }

 TIntermDeclaration *TParseContext::parseSingleDeclaration(
@@ -2092,7 +2129,7 @@ TIntermInvariantDeclaration *TParseContext::parseInvariantDeclaration(

    checkInvariantVariableQualifier(typeQualifier.invariant, type.getQualifier(),
                                    typeQualifier.line);
-    checkIsMemoryQualifierNotSpecified(typeQualifier.memoryQualifier, typeQualifier.line);
+    checkMemoryQualifierIsNotSpecified(typeQualifier.memoryQualifier, typeQualifier.line);

    symbolTable.addInvariantVarying(std::string(identifier->c_str()));

@@ -2120,10 +2157,10 @@ void TParseContext::parseDeclarator(TPublicType &publicType,
    checkCanBeDeclaredWithoutInitializer(identifierLocation, identifier, &publicType);

    TVariable *variable = nullptr;
-    declareVariable(identifierLocation, identifier, TType(publicType), &variable);
+    TType type(publicType);
+    declareVariable(identifierLocation, identifier, type, &variable);

-    TIntermSymbol *symbol =
-        intermediate.addSymbol(0, identifier, TType(publicType), identifierLocation);
+    TIntermSymbol *symbol = intermediate.addSymbol(0, identifier, type, identifierLocation);
    if (variable && symbol)
    {
        symbol->setId(variable->getUniqueId());
@@ -2260,11 +2297,13 @@ void TParseContext::parseGlobalLayoutQualifier(const TTypeQualifierBuilder &type

    if (!layoutQualifier.isCombinationValid())
    {
-        error(typeQualifier.line, "invalid combination:", "layout");
+        error(typeQualifier.line, "invalid layout qualifier combination", "layout");
        return;
    }

-    checkIsMemoryQualifierNotSpecified(typeQualifier.memoryQualifier, typeQualifier.line);
+    checkBindingIsNotSpecified(typeQualifier.line, layoutQualifier.binding);
+
+    checkMemoryQualifierIsNotSpecified(typeQualifier.memoryQualifier, typeQualifier.line);

    checkInternalFormatIsNotSpecified(typeQualifier.line, layoutQualifier.imageInternalFormat);

@@ -2741,7 +2780,10 @@ TIntermDeclaration *TParseContext::addInterfaceBlock(
        error(typeQualifier.line, "invalid qualifier on interface block member", "invariant");
    }

-    checkIsMemoryQualifierNotSpecified(typeQualifier.memoryQualifier, typeQualifier.line);
+    checkMemoryQualifierIsNotSpecified(typeQualifier.memoryQualifier, typeQualifier.line);
+
+    // TODO(oetuaho): Remove this and support binding for blocks.
+    checkBindingIsNotSpecified(typeQualifier.line, typeQualifier.layoutQualifier.binding);

    TLayoutQualifier blockLayoutQualifier = typeQualifier.layoutQualifier;
    checkLocationIsNotSpecified(typeQualifier.line, blockLayoutQualifier);
@@ -3359,6 +3401,19 @@ TLayoutQualifier TParseContext::parseLayoutQualifier(const TString &qualifierTyp
            qualifier.locationsSpecified = 1;
        }
    }
+    else if (qualifierType == "binding")
+    {
+        checkLayoutQualifierSupported(qualifierTypeLine, qualifierType, 310);
+        if (intValue < 0)
+        {
+            error(intValueLine, "out of range: binding must be non-negative",
+                  intValueString.c_str());
+        }
+        else
+        {
+            qualifier.binding = intValue;
+        }
+    }
    else if (qualifierType == "local_size_x")
    {
        parseLocalSize(qualifierType, qualifierTypeLine, intValue, intValueLine, intValueString, 0u,
@@ -3528,7 +3583,9 @@ TTypeSpecifierNonArray TParseContext::addStructure(const TSourceLoc &structLine,
            error(field.line(), "disallowed type in struct", field.type()->getBasicString());
        }

-        checkIsMemoryQualifierNotSpecified(field.type()->getMemoryQualifier(), field.line());
+        checkMemoryQualifierIsNotSpecified(field.type()->getMemoryQualifier(), field.line());
+
+        checkBindingIsNotSpecified(field.line(), field.type()->getLayoutQualifier().binding);

        checkLocationIsNotSpecified(field.line(), field.type()->getLayoutQualifier());
    }
--- a/src/compiler/translator/ParseContext.h
+++ b/src/compiler/translator/ParseContext.h
@@ -144,8 +144,6 @@ class TParseContext : angle::NonCopyable
                                       int versionRequired);
    bool checkWorkGroupSizeIsNotSpecified(const TSourceLoc &location,
                                          const TLayoutQualifier &layoutQualifier);
-    bool checkInternalFormatIsNotSpecified(const TSourceLoc &location,
-                                           TLayoutImageInternalFormat internalFormat);
    void functionCallLValueErrorCheck(const TFunction *fnCandidate, TIntermAggregate *fnCall);
    void checkInvariantVariableQualifier(bool invariant,
                                         const TQualifier qualifier,
@@ -378,8 +376,6 @@ class TParseContext : angle::NonCopyable
    // Assumes that multiplication op has already been set based on the types.
    bool isMultiplicationTypeCombinationValid(TOperator op, const TType &left, const TType &right);

-    bool checkIsMemoryQualifierNotSpecified(const TMemoryQualifier &memoryQualifier,
-                                            const TSourceLoc &location);
    void checkOutParameterIsNotImage(const TSourceLoc &line,
                                     TQualifier qualifier,
                                     const TType &type);
@@ -390,6 +386,15 @@ class TParseContext : angle::NonCopyable
                                       TQualifier qualifier,
                                       const TType &type);

+    void checkInternalFormatIsNotSpecified(const TSourceLoc &location,
+                                           TLayoutImageInternalFormat internalFormat);
+    void checkMemoryQualifierIsNotSpecified(const TMemoryQualifier &memoryQualifier,
+                                            const TSourceLoc &location);
+    void checkBindingIsValid(const TSourceLoc &identifierLocation, const TType &type);
+    void checkBindingIsNotSpecified(const TSourceLoc &location, int binding);
+    void checkImageBindingIsValid(const TSourceLoc &location, int binding, int arraySize);
+    void checkSamplerBindingIsValid(const TSourceLoc &location, int binding, int arraySize);
+
    TIntermTyped *addBinaryMathInternal(TOperator op,
                                        TIntermTyped *left,
                                        TIntermTyped *right,
@@ -462,6 +467,8 @@ class TParseContext : angle::NonCopyable
    // keep track of number of views declared in layout.
    int mNumViews;
    int mMaxNumViews;
+    int mMaxImageUnits;
+    int mMaxCombinedTextureImageUnits;
    // keeps track whether we are declaring / defining a function
    bool mDeclaringFunction;
 };
--- a/src/compiler/translator/QualifierTypes.cpp
+++ b/src/compiler/translator/QualifierTypes.cpp
@@ -560,6 +560,10 @@ TLayoutQualifier JoinLayoutQualifiers(TLayoutQualifier leftQualifier,
        joinedQualifier.location = rightQualifier.location;
        ++joinedQualifier.locationsSpecified;
    }
+    if (rightQualifier.binding != -1)
+    {
+        joinedQualifier.binding = rightQualifier.binding;
+    }
    if (rightQualifier.matrixPacking != EmpUnspecified)
    {
        joinedQualifier.matrixPacking = rightQualifier.matrixPacking;
--- a/src/tests/compiler_tests/ShaderImage_test.cpp
+++ b/src/tests/compiler_tests/ShaderImage_test.cpp
@@ -93,7 +93,8 @@ void CheckImageDeclaration(TIntermNode *astRoot,
                           bool writeonly,
                           bool coherent,
                           bool restrictQualifier,
-                           bool volatileQualifier)
+                           bool volatileQualifier,
+                           int binding)
 {
    const TIntermSymbol *myImageNode = FindSymbolNode(astRoot, imageName, imageType);
    ASSERT_NE(nullptr, myImageNode);
@@ -107,6 +108,7 @@ void CheckImageDeclaration(TIntermNode *astRoot,
    ASSERT_EQ(coherent, myImageMemoryQualifier.coherent);
    ASSERT_EQ(restrictQualifier, myImageMemoryQualifier.restrictQualifier);
    ASSERT_EQ(volatileQualifier, myImageMemoryQualifier.volatileQualifier);
+    ASSERT_EQ(binding, myImageType.getLayoutQualifier().binding);
 }

 }  // namespace
@@ -133,7 +135,7 @@ TEST_F(ShaderImageTest, Image2DDeclaration)
    const std::string &shaderString =
        "#version 310 es\n"
        "layout(local_size_x = 4) in;\n"
-        "layout(rgba32f) uniform highp readonly image2D myImage;\n"
+        "layout(rgba32f, binding = 1) uniform highp readonly image2D myImage;\n"
        "void main() {\n"
        "   ivec2 sz = imageSize(myImage);\n"
        "}";
@@ -144,7 +146,7 @@ TEST_F(ShaderImageTest, Image2DDeclaration)

    CheckExportedImageUniform(getUniforms(), 0, GL_IMAGE_2D, "myImage");
    CheckImageDeclaration(mASTRoot, "myImage", EbtImage2D, EiifRGBA32F, true, false, false, false,
-                          false);
+                          false, 1);
 }

 // Test that an image3D is properly parsed and exported as a uniform.
@@ -153,7 +155,7 @@ TEST_F(ShaderImageTest, Image3DDeclaration)
    const std::string &shaderString =
        "#version 310 es\n"
        "layout(local_size_x = 4) in;\n"
-        "layout(rgba32ui) uniform highp writeonly readonly uimage3D myImage;\n"
+        "layout(rgba32ui, binding = 3) uniform highp writeonly readonly uimage3D myImage;\n"
        "void main() {\n"
        "   ivec3 sz = imageSize(myImage);\n"
        "}";
@@ -164,7 +166,7 @@ TEST_F(ShaderImageTest, Image3DDeclaration)

    CheckExportedImageUniform(getUniforms(), 0, GL_UNSIGNED_INT_IMAGE_3D, "myImage");
    CheckImageDeclaration(mASTRoot, "myImage", EbtUImage3D, EiifRGBA32UI, true, true, false, false,
-                          false);
+                          false, 3);
 }

 // Check that imageLoad calls get correctly parsed.
@@ -232,9 +234,9 @@ TEST_F(ShaderImageTest, ImageMemoryQualifiers)
    }

    CheckImageDeclaration(mASTRoot, "image1", EbtImage2D, EiifRGBA32F, true, false, true, false,
-                          false);
+                          false, -1);
    CheckImageDeclaration(mASTRoot, "image2", EbtImage2D, EiifRGBA32F, false, true, true, false,
-                          true);
-    CheckImageDeclaration(mASTRoot, "image3", EbtImage2D, EiifRGBA32F, true, true, true, true,
-                          true);
+                          true, -1);
+    CheckImageDeclaration(mASTRoot, "image3", EbtImage2D, EiifRGBA32F, true, true, true, true, true,
+                          -1);
 }
--- a/src/tests/compiler_tests/ShaderValidation_test.cpp
+++ b/src/tests/compiler_tests/ShaderValidation_test.cpp
@@ -2519,7 +2519,7 @@ TEST_F(FragmentShaderValidationTest, ImageR32FNoMemoryQualifier)
 // Images which do not have r32f, r32i or r32ui as internal format, must have readonly or writeonly
 // specified.
 // GLSL ES 3.10, Revision 4, 4.9 Memory Access Qualifiers
-TEST_F(FragmentShaderValidationTest, ImageR32FWithCorrectMemoryQualifier)
+TEST_F(FragmentShaderValidationTest, ImageRGBA32FWithIncorrectMemoryQualifier)
 {
    const std::string &shaderString =
        "#version 310 es\n"
@@ -3556,3 +3556,154 @@ TEST_F(VertexShaderValidationTest, InvalidArrayConstruction)
        FAIL() << "Shader compilation succeeded, expecting failure " << mInfoLog;
    }
 }
+
+// Correct usage of image binding layout qualifier.
+TEST_F(ComputeShaderValidationTest, CorrectImageBindingLayoutQualifier)
+{
+    const std::string &shaderString =
+        "#version 310 es\n"
+        "precision mediump float;\n"
+        "precision mediump image2D;\n"
+        "layout(local_size_x = 5) in;\n"
+        "layout(binding = 1, rgba32f) writeonly uniform image2D myImage;\n"
+        "void main()\n"
+        "{\n"
+        "   imageStore(myImage, ivec2(gl_LocalInvocationID.xy), vec4(1.0));\n"
+        "}\n";
+
+    if (!compile(shaderString))
+    {
+        FAIL() << "Shader compilation failed, expecting success " << mInfoLog;
+    }
+}
+
+// Incorrect use of "binding" on a global layout qualifier.
+TEST_F(ComputeShaderValidationTest, IncorrectGlobalBindingLayoutQualifier)
+{
+    const std::string &shaderString =
+        "#version 310 es\n"
+        "layout(local_size_x = 5, binding = 0) in;\n"
+        "void main() {}\n";
+
+    if (compile(shaderString))
+    {
+        FAIL() << "Shader compilation succeeded, expecting failure " << mInfoLog;
+    }
+}
+
+// Incorrect use of "binding" on a struct field layout qualifier.
+TEST_F(ComputeShaderValidationTest, IncorrectStructFieldBindingLayoutQualifier)
+{
+    const std::string &shaderString =
+        "#version 310 es\n"
+        "precision mediump float;\n"
+        "layout(local_size_x = 1) in;\n"
+        "struct S\n"
+        "{\n"
+        "  layout(binding = 0) float f;\n"
+        "};\n"
+        "void main() {}\n";
+
+    if (compile(shaderString))
+    {
+        FAIL() << "Shader compilation succeeded, expecting failure " << mInfoLog;
+    }
+}
+
+// Variable binding layout qualifier is set to a negative value. 0xffffffff wraps around to -1
+// according to the integer parsing rules.
+TEST_F(FragmentShaderValidationTest, ImageBindingUnitNegative)
+{
+    const std::string &shaderString =
+        "#version 310 es\n"
+        "precision mediump float;\n"
+        "layout(rgba32f, binding = 0xffffffff) writeonly uniform mediump image2D myImage;\n"
+        "out vec4 outFrag;\n"
+        "void main()\n"
+        "{\n"
+        "   outFrag = vec4(0.0);\n"
+        "}\n";
+
+    if (compile(shaderString))
+    {
+        FAIL() << "Shader compilation succeeded, expecting failure " << mInfoLog;
+    }
+}
+
+// Image binding layout qualifier value is greater than the maximum image binding.
+TEST_F(FragmentShaderValidationTest, ImageBindingUnitTooBig)
+{
+    const std::string &shaderString =
+        "#version 310 es\n"
+        "precision mediump float;\n"
+        "layout(rgba32f, binding = 9999) writeonly uniform mediump image2D myImage;\n"
+        "out vec4 outFrag;\n"
+        "void main()\n"
+        "{\n"
+        "   outFrag = vec4(0.0);\n"
+        "}\n";
+
+    if (compile(shaderString))
+    {
+        FAIL() << "Shader compilation succeeded, expecting failure " << mInfoLog;
+    }
+}
+
+// Uniform variable binding is set on a non-opaque type.
+TEST_F(FragmentShaderValidationTest, NonOpaqueUniformBinding)
+{
+    const std::string &shaderString =
+        "#version 310 es\n"
+        "precision mediump float;\n"
+        "layout(binding = 0) uniform float myFloat;\n"
+        "out vec4 outFrag;\n"
+        "void main()\n"
+        "{\n"
+        "   outFrag = vec4(myFloat);\n"
+        "}\n";
+
+    if (compile(shaderString))
+    {
+        FAIL() << "Shader compilation succeeded, expecting failure " << mInfoLog;
+    }
+}
+
+// Uniform variable binding is set on a sampler type.
+// ESSL 3.10 section 4.4.5 Opaque Uniform Layout Qualifiers.
+TEST_F(FragmentShaderValidationTest, SamplerUniformBinding)
+{
+    const std::string &shaderString =
+        "#version 310 es\n"
+        "precision mediump float;\n"
+        "layout(binding = 0) uniform mediump sampler2D mySampler;\n"
+        "out vec4 outFrag;\n"
+        "void main()\n"
+        "{\n"
+        "   outFrag = vec4(0.0);\n"
+        "}\n";
+
+    if (!compile(shaderString))
+    {
+        FAIL() << "Shader compilation failed, expecting success " << mInfoLog;
+    }
+}
+
+// Uniform variable binding is set on a sampler type in an ESSL 3.00 shader.
+// The binding layout qualifier was added in ESSL 3.10, so this is incorrect.
+TEST_F(FragmentShaderValidationTest, SamplerUniformBindingESSL300)
+{
+    const std::string &shaderString =
+        "#version 300 es\n"
+        "precision mediump float;\n"
+        "layout(binding = 0) uniform mediump sampler2D mySampler;\n"
+        "out vec4 outFrag;\n"
+        "void main()\n"
+        "{\n"
+        "   outFrag = vec4(0.0);\n"
+        "}\n";
+
+    if (compile(shaderString))
+    {
+        FAIL() << "Shader compilation succeeded, expecting failure " << mInfoLog;
+    }
+}
--- a/src/tests/deqp_support/deqp_gles31_test_expectations.txt
+++ b/src/tests/deqp_support/deqp_gles31_test_expectations.txt
@@ -1435,3 +1435,12 @@
 1442 OPENGL D3D11 : dEQP-GLES31.functional.layout_binding.ssbo.* = FAIL
 1442 OPENGL D3D11 : dEQP-GLES31.functional.layout_binding.image.image2d.* = FAIL
 1442 OPENGL D3D11 : dEQP-GLES31.functional.layout_binding.image.image3d.* = FAIL
+
+1893 OPENGL D3D11 : dEQP-GLES31.functional.layout_binding.negative.sampler.sampler2d.binding_contradictory = FAIL
+1893 OPENGL D3D11 : dEQP-GLES31.functional.layout_binding.negative.sampler.sampler2d.binding_contradictory_array = FAIL
+1893 OPENGL D3D11 : dEQP-GLES31.functional.layout_binding.negative.sampler.sampler3d.binding_contradictory = FAIL
+1893 OPENGL D3D11 : dEQP-GLES31.functional.layout_binding.negative.sampler.sampler3d.binding_contradictory_array = FAIL
+1893 OPENGL D3D11 : dEQP-GLES31.functional.layout_binding.negative.image.image2d.binding_contradictory = FAIL
+1893 OPENGL D3D11 : dEQP-GLES31.functional.layout_binding.negative.image.image2d.binding_contradictory_array = FAIL
+1893 OPENGL D3D11 : dEQP-GLES31.functional.layout_binding.negative.image.image3d.binding_contradictory = FAIL
+1893 OPENGL D3D11 : dEQP-GLES31.functional.layout_binding.negative.image.image3d.binding_contradictory_array = FAIL