Re-re-land "Move sampler validation to the GL layer."

src/libANGLE/Program.cpp

@@ -174,6 +174,17 @@ void UniformStateQueryCastLoop(DestT *dataOut, const uint8_t *srcPointer, int co
     }
 }
 
+bool UniformInList(const std::vector<LinkedUniform> &list, const std::string &name)
+{
+    for (const LinkedUniform &uniform : list)
+    {
+        if (uniform.name == name)
+            return true;
+    }
+
+    return false;
+}
+
 }  // anonymous namespace
 
 AttributeBindings::AttributeBindings()
@@ -359,7 +370,8 @@ Program::Program(rx::ImplFactory *factory, ResourceManager *manager, GLuint hand
       mDeleteStatus(false),
       mRefCount(0),
       mResourceManager(manager),
-      mHandle(handle)
+      mHandle(handle),
+      mSamplerUniformRange(0, 0)
 {
     ASSERT(mProgram);
 
@@ -674,12 +686,15 @@ Error Program::loadBinary(GLenum binaryFormat, const void *binary, GLsizei lengt
     {
         int locationIndex = stream.readInt<int>();
         VariableLocation locationData;
-        locationData.element                  = stream.readInt<unsigned int>();
-        locationData.index                    = stream.readInt<unsigned int>();
-        locationData.name                     = stream.readString();
+        stream.readInt(&locationData.element);
+        stream.readInt(&locationData.index);
+        stream.readString(&locationData.name);
         mData.mOutputVariables[locationIndex] = locationData;
     }
 
+    stream.readInt(&mSamplerUniformRange.start);
+    stream.readInt(&mSamplerUniformRange.end);
+
     rx::LinkResult result = mProgram->load(mInfoLog, &stream);
     if (result.error.isError() || !result.linkSuccess)
     {
@@ -769,6 +784,9 @@ Error Program::saveBinary(GLenum *binaryFormat, void *binary, GLsizei bufSize, G
         stream.writeString(outputPair.second.name);
     }
 
+    stream.writeInt(mSamplerUniformRange.start);
+    stream.writeInt(mSamplerUniformRange.end);
+
     gl::Error error = mProgram->save(&stream);
     if (error.isError())
     {
@@ -1297,7 +1315,78 @@ void Program::validate(const Caps &caps)
 
 bool Program::validateSamplers(InfoLog *infoLog, const Caps &caps)
 {
-    return mProgram->validateSamplers(infoLog, caps);
+    // Skip cache if we're using an infolog, so we get the full error.
+    // Also skip the cache if the sample mapping has changed, or if we haven't ever validated.
+    if (infoLog == nullptr && mCachedValidateSamplersResult.valid())
+    {
+        return mCachedValidateSamplersResult.value();
+    }
+
+    if (mTextureUnitTypesCache.empty())
+    {
+        mTextureUnitTypesCache.resize(caps.maxCombinedTextureImageUnits, GL_NONE);
+    }
+    else
+    {
+        std::fill(mTextureUnitTypesCache.begin(), mTextureUnitTypesCache.end(), GL_NONE);
+    }
+
+    // if any two active samplers in a program are of different types, but refer to the same
+    // texture image unit, and this is the current program, then ValidateProgram will fail, and
+    // DrawArrays and DrawElements will issue the INVALID_OPERATION error.
+    for (unsigned int samplerIndex = mSamplerUniformRange.start;
+         samplerIndex < mSamplerUniformRange.end; ++samplerIndex)
+    {
+        const LinkedUniform &uniform = mData.mUniforms[samplerIndex];
+        ASSERT(uniform.isSampler());
+
+        if (!uniform.staticUse)
+            continue;
+
+        const GLuint *dataPtr = reinterpret_cast<const GLuint *>(uniform.getDataPtrToElement(0));
+        GLenum textureType    = SamplerTypeToTextureType(uniform.type);
+
+        for (unsigned int arrayElement = 0; arrayElement < uniform.elementCount(); ++arrayElement)
+        {
+            GLuint textureUnit = dataPtr[arrayElement];
+
+            if (textureUnit >= caps.maxCombinedTextureImageUnits)
+            {
+                if (infoLog)
+                {
+                    (*infoLog) << "Sampler uniform (" << textureUnit
+                               << ") exceeds GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS ("
+                               << caps.maxCombinedTextureImageUnits << ")";
+                }
+
+                mCachedValidateSamplersResult = false;
+                return false;
+            }
+
+            if (mTextureUnitTypesCache[textureUnit] != GL_NONE)
+            {
+                if (textureType != mTextureUnitTypesCache[textureUnit])
+                {
+                    if (infoLog)
+                    {
+                        (*infoLog) << "Samplers of conflicting types refer to the same texture "
+                                      "image unit ("
+                                   << textureUnit << ").";
+                    }
+
+                    mCachedValidateSamplersResult = false;
+                    return false;
+                }
+            }
+            else
+            {
+                mTextureUnitTypesCache[textureUnit] = textureType;
+            }
+        }
+    }
+
+    mCachedValidateSamplersResult = true;
+    return true;
 }
 
 bool Program::isValidated() const
@@ -2086,11 +2175,13 @@ bool Program::flattenUniformsAndCheckCaps(const Caps &caps, InfoLog &infoLog)
     const gl::Shader *vertexShader = mData.getAttachedVertexShader();
     VectorAndSamplerCount vsCounts;
 
+    std::vector<LinkedUniform> samplerUniforms;
+
     for (const sh::Uniform &uniform : vertexShader->getUniforms())
     {
         if (uniform.staticUse)
         {
-            vsCounts += flattenUniform(uniform, uniform.name);
+            vsCounts += flattenUniform(uniform, uniform.name, &samplerUniforms);
         }
     }
 
@@ -2115,7 +2206,7 @@ bool Program::flattenUniformsAndCheckCaps(const Caps &caps, InfoLog &infoLog)
     {
         if (uniform.staticUse)
         {
-            fsCounts += flattenUniform(uniform, uniform.name);
+            fsCounts += flattenUniform(uniform, uniform.name, &samplerUniforms);
         }
     }
 
@@ -2133,11 +2224,18 @@ bool Program::flattenUniformsAndCheckCaps(const Caps &caps, InfoLog &infoLog)
         return false;
     }
 
+    mSamplerUniformRange.start = static_cast<unsigned int>(mData.mUniforms.size());
+    mSamplerUniformRange.end =
+        mSamplerUniformRange.start + static_cast<unsigned int>(samplerUniforms.size());
+
+    mData.mUniforms.insert(mData.mUniforms.end(), samplerUniforms.begin(), samplerUniforms.end());
+
     return true;
 }
 
 Program::VectorAndSamplerCount Program::flattenUniform(const sh::ShaderVariable &uniform,
-                                                       const std::string &fullName)
+                                                       const std::string &fullName,
+                                                       std::vector<LinkedUniform> *samplerUniforms)
 {
     VectorAndSamplerCount vectorAndSamplerCount;
 
@@ -2152,7 +2250,7 @@ Program::VectorAndSamplerCount Program::flattenUniform(const sh::ShaderVariable
                 const sh::ShaderVariable &field  = uniform.fields[fieldIndex];
                 const std::string &fieldFullName = (fullName + elementString + "." + field.name);
 
-                vectorAndSamplerCount += flattenUniform(field, fieldFullName);
+                vectorAndSamplerCount += flattenUniform(field, fieldFullName, samplerUniforms);
             }
         }
 
@@ -2160,18 +2258,28 @@ Program::VectorAndSamplerCount Program::flattenUniform(const sh::ShaderVariable
     }
 
     // Not a struct
-    if (mData.getUniformByName(fullName) == nullptr)
+    bool isSampler = IsSamplerType(uniform.type);
+    if (!UniformInList(mData.getUniforms(), fullName) && !UniformInList(*samplerUniforms, fullName))
     {
         gl::LinkedUniform linkedUniform(uniform.type, uniform.precision, fullName,
                                         uniform.arraySize, -1,
                                         sh::BlockMemberInfo::getDefaultBlockInfo());
         linkedUniform.staticUse = true;
-        mData.mUniforms.push_back(linkedUniform);
+
+        // Store sampler uniforms separately, so we'll append them to the end of the list.
+        if (isSampler)
+        {
+            samplerUniforms->push_back(linkedUniform);
+        }
+        else
+        {
+            mData.mUniforms.push_back(linkedUniform);
+        }
     }
 
-    vectorAndSamplerCount.vectorCount =
-        (VariableRegisterCount(uniform.type) * uniform.elementCount());
-    vectorAndSamplerCount.samplerCount = (IsSamplerType(uniform.type) ? uniform.elementCount() : 0);
+    unsigned int elementCount          = uniform.elementCount();
+    vectorAndSamplerCount.vectorCount  = (VariableRegisterCount(uniform.type) * elementCount);
+    vectorAndSamplerCount.samplerCount = (isSampler ? elementCount : 0);
 
     return vectorAndSamplerCount;
 }
@@ -2294,6 +2402,12 @@ void Program::setUniformInternal(GLint location, GLsizei count, const T *v)
     }
     else
     {
+        // Invalide the validation cache if we modify the sampler data.
+        if (linkedUniform->isSampler() && memcmp(destPointer, v, sizeof(T) * count) != 0)
+        {
+            mCachedValidateSamplersResult.reset();
+        }
+
         memcpy(destPointer, v, sizeof(T) * count);
     }
 }

src/libANGLE/Program.h

@@ -10,13 +10,6 @@
 #ifndef LIBANGLE_PROGRAM_H_
 #define LIBANGLE_PROGRAM_H_
 
-#include "libANGLE/angletypes.h"
-#include "libANGLE/Constants.h"
-#include "libANGLE/Error.h"
-#include "libANGLE/RefCountObject.h"
-
-#include "common/angleutils.h"
-
 #include <GLES2/gl2.h>
 #include <GLSLANG/ShaderLang.h>
 
@@ -25,6 +18,15 @@
 #include <string>
 #include <vector>
 
+#include "common/angleutils.h"
+#include "common/mathutil.h"
+#include "common/Optional.h"
+
+#include "libANGLE/angletypes.h"
+#include "libANGLE/Constants.h"
+#include "libANGLE/Error.h"
+#include "libANGLE/RefCountObject.h"
+
 namespace rx
 {
 class ImplFactory;
@@ -215,6 +217,11 @@ class Program : angle::NonCopyable
         std::vector<sh::Attribute> mAttributes;
         std::bitset<MAX_VERTEX_ATTRIBS> mActiveAttribLocationsMask;
 
+        // Uniforms are sorted in order:
+        //  1. Non-sampler uniforms
+        //  2. Sampler uniforms
+        //  3. Uniform block uniforms
+        // This makes sampler validation easier, since we don't need a separate list.
         std::vector<LinkedUniform> mUniforms;
         std::vector<VariableLocation> mUniformLocations;
         std::vector<UniformBlock> mUniformBlocks;
@@ -384,7 +391,8 @@ class Program : angle::NonCopyable
     };
 
     VectorAndSamplerCount flattenUniform(const sh::ShaderVariable &uniform,
-                                         const std::string &fullName);
+                                         const std::string &fullName,
+                                         std::vector<LinkedUniform> *samplerUniforms);
 
     void gatherInterfaceBlockInfo();
     void defineUniformBlock(const sh::InterfaceBlock &interfaceBlock, GLenum shaderType);
@@ -414,6 +422,11 @@ class Program : angle::NonCopyable
     const GLuint mHandle;
 
     InfoLog mInfoLog;
+
+    // Cache for sampler validation
+    Optional<bool> mCachedValidateSamplersResult;
+    std::vector<GLenum> mTextureUnitTypesCache;
+    RangeUI mSamplerUniformRange;
 };
 }
 

src/libANGLE/State.cpp

@@ -633,6 +633,7 @@ Texture *State::getSamplerTexture(unsigned int sampler, GLenum type) const
 {
     const auto it = mSamplerTextures.find(type);
     ASSERT(it != mSamplerTextures.end());
+    ASSERT(sampler < it->second.size());
     return it->second[sampler].get();
 }
 
@@ -640,6 +641,7 @@ GLuint State::getSamplerTextureId(unsigned int sampler, GLenum type) const
 {
     const auto it = mSamplerTextures.find(type);
     ASSERT(it != mSamplerTextures.end());
+    ASSERT(sampler < it->second.size());
     return it->second[sampler].id();
 }
 

src/libANGLE/Uniform.h

@@ -64,6 +64,7 @@ struct UniformBlock
 
     std::vector<unsigned int> memberUniformIndexes;
 
+    // TODO(jmadill): Make D3D-only.
     unsigned int psRegisterIndex;
     unsigned int vsRegisterIndex;
 };

src/libANGLE/renderer/ProgramImpl.h

@@ -66,10 +66,6 @@ class ProgramImpl : angle::NonCopyable
     virtual void setUniformMatrix3x4fv(GLint location, GLsizei count, GLboolean transpose, const GLfloat *value) = 0;
     virtual void setUniformMatrix4x3fv(GLint location, GLsizei count, GLboolean transpose, const GLfloat *value) = 0;
 
-    // TODO: The following functions are possibly only applicable to D3D backends. The should be carefully evaluated to
-    // determine if they can be removed from this interface.
-    virtual bool validateSamplers(gl::InfoLog *infoLog, const gl::Caps &caps) = 0;
-
     // Gather uniform block active uniform indices, and uniform block offset info.
     virtual void gatherUniformBlockInfo(std::vector<gl::UniformBlock> *uniformBlocks,
                                         std::vector<gl::LinkedUniform> *uniforms) = 0;

src/libANGLE/renderer/d3d/ProgramD3D.h

@@ -12,7 +12,6 @@
 #include <string>
 #include <vector>
 
-#include "common/Optional.h"
 #include "compiler/translator/blocklayoutHLSL.h"
 #include "libANGLE/Constants.h"
 #include "libANGLE/formatutils.h"
@@ -33,7 +32,7 @@ class ShaderExecutableD3D;
 #endif
 
 // Helper struct representing a single shader uniform
-struct D3DUniform
+struct D3DUniform : angle::NonCopyable
 {
     D3DUniform(GLenum typeIn,
                const std::string &nameIn,
@@ -84,7 +83,6 @@ class ProgramD3D : public ProgramImpl
     GLenum getSamplerTextureType(gl::SamplerType type, unsigned int samplerIndex) const;
     GLint getUsedSamplerRange(gl::SamplerType type) const;
     void updateSamplerMapping();
-    bool validateSamplers(gl::InfoLog *infoLog, const gl::Caps &caps);
 
     bool usesPointSize() const { return mUsesPointSize; }
     bool usesPointSpriteEmulation() const;
@@ -198,14 +196,18 @@ class ProgramD3D : public ProgramImpl
         GLenum textureType;
     };
 
+    typedef std::map<std::string, D3DUniform *> D3DUniformMap;
     typedef std::map<std::string, sh::BlockMemberInfo> BlockInfoMap;
 
-    void assignUniformRegisters();
-    void assignUniformRegistersBase(const ShaderD3D *shader, const sh::Uniform &uniform);
-    void assignUniformRegisters(const ShaderD3D *shader,
-                                const sh::ShaderVariable &uniform,
-                                const std::string &fullName,
-                                sh::HLSLBlockEncoder *encoder);
+    void defineUniformsAndAssignRegisters();
+    void defineUniformBase(const ShaderD3D *shader,
+                           const sh::Uniform &uniform,
+                           D3DUniformMap *uniformMap);
+    void defineUniform(const ShaderD3D *shader,
+                       const sh::ShaderVariable &uniform,
+                       const std::string &fullName,
+                       sh::HLSLBlockEncoder *encoder,
+                       D3DUniformMap *uniformMap);
     void assignAllSamplerRegisters();
     void assignSamplerRegisters(const D3DUniform *d3dUniform);
 
@@ -262,9 +264,6 @@ class ProgramD3D : public ProgramImpl
     GLuint mUsedPixelSamplerRange;
     bool mDirtySamplerMapping;
 
-    // Cache for validateSamplers
-    std::vector<GLenum> mTextureUnitTypesCache;
-
     // Cache for getPixelExecutableForFramebuffer
     std::vector<GLenum> mPixelShaderOutputFormatCache;
 
@@ -275,8 +274,6 @@ class ProgramD3D : public ProgramImpl
 
     unsigned int mSerial;
 
-    Optional<bool> mCachedValidateSamplersResult;
-
     std::vector<GLint> mVertexUBOCache;
     std::vector<GLint> mFragmentUBOCache;
     VertexExecutable::Signature mCachedVertexSignature;

src/libANGLE/renderer/gl/ProgramGL.cpp

@@ -299,12 +299,6 @@ void ProgramGL::setUniformMatrix4x3fv(GLint location, GLsizei count, GLboolean t
     mFunctions->uniformMatrix4x3fv(uniLoc(location), count, transpose, value);
 }
 
-bool ProgramGL::validateSamplers(gl::InfoLog *infoLog, const gl::Caps &caps)
-{
-    //UNIMPLEMENTED();
-    return true;
-}
-
 void ProgramGL::reset()
 {
     mUniformRealLocationMap.clear();

src/libANGLE/renderer/gl/ProgramGL.h

@@ -62,8 +62,6 @@ class ProgramGL : public ProgramImpl
     void setUniformMatrix3x4fv(GLint location, GLsizei count, GLboolean transpose, const GLfloat *value) override;
     void setUniformMatrix4x3fv(GLint location, GLsizei count, GLboolean transpose, const GLfloat *value) override;
 
-    bool validateSamplers(gl::InfoLog *infoLog, const gl::Caps &caps) override;
-
     void gatherUniformBlockInfo(std::vector<gl::UniformBlock> *uniformBlocks,
                                 std::vector<gl::LinkedUniform> *uniforms) override;
 

src/tests/gl_tests/UniformTest.cpp

@@ -452,6 +452,46 @@ TEST_P(UniformTestES3, TranposedMatrixArrayUniformStateQuery)
     }
 }
 
+// Check that sampler uniforms only show up one time in the list
+TEST_P(UniformTest, SamplerUniformsAppearOnce)
+{
+    const std::string &vertShader =
+        "attribute vec2 position;\n"
+        "uniform sampler2D tex2D;\n"
+        "varying vec4 color;\n"
+        "void main() {\n"
+        "  gl_Position = vec4(position, 0, 1);\n"
+        "  color = texture2D(tex2D, vec2(0));\n"
+        "}";
+
+    const std::string &fragShader =
+        "precision mediump float;\n"
+        "varying vec4 color;\n"
+        "uniform sampler2D tex2D;\n"
+        "void main() {\n"
+        "  gl_FragColor = texture2D(tex2D, vec2(0)) + color;\n"
+        "}";
+
+    GLuint program = CompileProgram(vertShader, fragShader);
+    ASSERT_NE(0u, program);
+
+    GLint activeUniformsCount = 0;
+    glGetProgramiv(program, GL_ACTIVE_UNIFORMS, &activeUniformsCount);
+    ASSERT_EQ(1, activeUniformsCount);
+
+    GLint size       = 0;
+    GLenum type      = GL_NONE;
+    GLchar name[120] = {0};
+    glGetActiveUniform(program, 0, 100, nullptr, &size, &type, name);
+    EXPECT_EQ(1, size);
+    EXPECT_GLENUM_EQ(GL_SAMPLER_2D, type);
+    EXPECT_STREQ("tex2D", name);
+
+    EXPECT_GL_NO_ERROR();
+
+    glDeleteProgram(program);
+}
+
 // Use this to select which configurations (e.g. which renderer, which GLES major version) these tests should be run against.
 ANGLE_INSTANTIATE_TEST(UniformTest, ES2_D3D9(), ES2_D3D11(), ES2_OPENGL());
 ANGLE_INSTANTIATE_TEST(UniformTestES3, ES3_D3D11(), ES3_OPENGL());