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

src/libANGLE/Program.cpp

@@ -174,17 +174,6 @@ 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()
@@ -370,8 +359,7 @@ Program::Program(rx::ImplFactory *factory, ResourceManager *manager, GLuint hand
       mDeleteStatus(false),
       mRefCount(0),
       mResourceManager(manager),
-      mHandle(handle),
-      mSamplerUniformRange(0, 0)
+      mHandle(handle)
 {
     ASSERT(mProgram);
 
@@ -686,15 +674,12 @@ Error Program::loadBinary(GLenum binaryFormat, const void *binary, GLsizei lengt
     {
         int locationIndex = stream.readInt<int>();
         VariableLocation locationData;
-        stream.readInt(&locationData.element);
-        stream.readInt(&locationData.index);
-        stream.readString(&locationData.name);
+        locationData.element                  = stream.readInt<unsigned int>();
+        locationData.index                    = stream.readInt<unsigned int>();
+        locationData.name                     = stream.readString();
         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)
     {
@@ -784,9 +769,6 @@ 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())
     {
@@ -1315,78 +1297,7 @@ void Program::validate(const Caps &caps)
 
 bool Program::validateSamplers(InfoLog *infoLog, const Caps &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;
+    return mProgram->validateSamplers(infoLog, caps);
 }
 
 bool Program::isValidated() const
@@ -2175,13 +2086,11 @@ 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, &samplerUniforms);
+            vsCounts += flattenUniform(uniform, uniform.name);
         }
     }
 
@@ -2206,7 +2115,7 @@ bool Program::flattenUniformsAndCheckCaps(const Caps &caps, InfoLog &infoLog)
     {
         if (uniform.staticUse)
         {
-            fsCounts += flattenUniform(uniform, uniform.name, &samplerUniforms);
+            fsCounts += flattenUniform(uniform, uniform.name);
         }
     }
 
@@ -2224,18 +2133,11 @@ 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,
-                                                       std::vector<LinkedUniform> *samplerUniforms)
+                                                       const std::string &fullName)
 {
     VectorAndSamplerCount vectorAndSamplerCount;
 
@@ -2250,7 +2152,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, samplerUniforms);
+                vectorAndSamplerCount += flattenUniform(field, fieldFullName);
             }
         }
 
@@ -2258,28 +2160,18 @@ Program::VectorAndSamplerCount Program::flattenUniform(const sh::ShaderVariable
     }
 
     // Not a struct
-    bool isSampler = IsSamplerType(uniform.type);
-    if (!UniformInList(mData.getUniforms(), fullName) && !UniformInList(*samplerUniforms, fullName))
+    if (mData.getUniformByName(fullName) == nullptr)
     {
         gl::LinkedUniform linkedUniform(uniform.type, uniform.precision, fullName,
                                         uniform.arraySize, -1,
                                         sh::BlockMemberInfo::getDefaultBlockInfo());
         linkedUniform.staticUse = true;
-
-        // 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);
-        }
+        mData.mUniforms.push_back(linkedUniform);
     }
 
-    unsigned int elementCount          = uniform.elementCount();
-    vectorAndSamplerCount.vectorCount  = (VariableRegisterCount(uniform.type) * elementCount);
-    vectorAndSamplerCount.samplerCount = (isSampler ? elementCount : 0);
+    vectorAndSamplerCount.vectorCount =
+        (VariableRegisterCount(uniform.type) * uniform.elementCount());
+    vectorAndSamplerCount.samplerCount = (IsSamplerType(uniform.type) ? uniform.elementCount() : 0);
 
     return vectorAndSamplerCount;
 }
@@ -2402,12 +2294,6 @@ 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,6 +10,13 @@
 #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>
 
@@ -18,15 +25,6 @@
 #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;
@@ -217,11 +215,6 @@ 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;
@@ -391,8 +384,7 @@ class Program : angle::NonCopyable
     };
 
     VectorAndSamplerCount flattenUniform(const sh::ShaderVariable &uniform,
-                                         const std::string &fullName,
-                                         std::vector<LinkedUniform> *samplerUniforms);
+                                         const std::string &fullName);
 
     void gatherInterfaceBlockInfo();
     void defineUniformBlock(const sh::InterfaceBlock &interfaceBlock, GLenum shaderType);
@@ -422,11 +414,6 @@ 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,7 +633,6 @@ 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();
 }
 
@@ -641,7 +640,6 @@ 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,7 +64,6 @@ struct UniformBlock
 
     std::vector<unsigned int> memberUniformIndexes;
 
-    // TODO(jmadill): Make D3D-only.
     unsigned int psRegisterIndex;
     unsigned int vsRegisterIndex;
 };

src/libANGLE/renderer/ProgramImpl.h

@@ -66,6 +66,10 @@ 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,6 +12,7 @@
 #include <string>
 #include <vector>
 
+#include "common/Optional.h"
 #include "compiler/translator/blocklayoutHLSL.h"
 #include "libANGLE/Constants.h"
 #include "libANGLE/formatutils.h"
@@ -32,7 +33,7 @@ class ShaderExecutableD3D;
 #endif
 
 // Helper struct representing a single shader uniform
-struct D3DUniform : angle::NonCopyable
+struct D3DUniform
 {
     D3DUniform(GLenum typeIn,
                const std::string &nameIn,
@@ -83,6 +84,7 @@ 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;
@@ -196,18 +198,14 @@ class ProgramD3D : public ProgramImpl
         GLenum textureType;
     };
 
-    typedef std::map<std::string, D3DUniform *> D3DUniformMap;
     typedef std::map<std::string, sh::BlockMemberInfo> BlockInfoMap;
 
-    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 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 assignAllSamplerRegisters();
     void assignSamplerRegisters(const D3DUniform *d3dUniform);
 
@@ -264,6 +262,9 @@ class ProgramD3D : public ProgramImpl
     GLuint mUsedPixelSamplerRange;
     bool mDirtySamplerMapping;
 
+    // Cache for validateSamplers
+    std::vector<GLenum> mTextureUnitTypesCache;
+
     // Cache for getPixelExecutableForFramebuffer
     std::vector<GLenum> mPixelShaderOutputFormatCache;
 
@@ -274,6 +275,8 @@ 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,6 +299,12 @@ 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,6 +62,8 @@ 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,46 +452,6 @@ 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_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());