Context: Cache attributes masks.

src/libANGLE/Context.cpp

@@ -1096,6 +1096,7 @@ void Context::bindVertexArray(GLuint vertexArrayHandle)
 {
     VertexArray *vertexArray = checkVertexArrayAllocation(vertexArrayHandle);
     mGLState.setVertexArrayBinding(this, vertexArray);
+    mStateCache.updateActiveAttribsMask(this);
 }
 
 void Context::bindVertexBuffer(GLuint bindingIndex,
@@ -1105,6 +1106,7 @@ void Context::bindVertexBuffer(GLuint bindingIndex,
 {
     Buffer *buffer = mState.mBuffers->checkBufferAllocation(mImplementation.get(), bufferHandle);
     mGLState.bindVertexBuffer(this, bindingIndex, buffer, offset, stride);
+    mStateCache.updateActiveAttribsMask(this);
 }
 
 void Context::bindSampler(GLuint textureUnit, GLuint samplerHandle)
@@ -1130,6 +1132,7 @@ void Context::bindImageTexture(GLuint unit,
 void Context::useProgram(GLuint program)
 {
     mGLState.setProgram(this, getProgram(program));
+    mStateCache.updateActiveAttribsMask(this);
 }
 
 void Context::useProgramStages(GLuint pipeline, GLbitfield stages, GLuint program)
@@ -2783,6 +2786,7 @@ void Context::detachProgramPipeline(GLuint pipeline)
 void Context::vertexAttribDivisor(GLuint index, GLuint divisor)
 {
     mGLState.setVertexAttribDivisor(this, index, divisor);
+    mStateCache.updateActiveAttribsMask(this);
 }
 
 void Context::samplerParameteri(GLuint sampler, GLenum pname, GLint param)
@@ -4389,6 +4393,7 @@ void Context::disable(GLenum cap)
 void Context::disableVertexAttribArray(GLuint index)
 {
     mGLState.setEnableVertexAttribArray(index, false);
+    mStateCache.updateActiveAttribsMask(this);
 }
 
 void Context::enable(GLenum cap)
@@ -4399,6 +4404,7 @@ void Context::enable(GLenum cap)
 void Context::enableVertexAttribArray(GLuint index)
 {
     mGLState.setEnableVertexAttribArray(index, true);
+    mStateCache.updateActiveAttribsMask(this);
 }
 
 void Context::frontFace(GLenum mode)
@@ -4606,6 +4612,7 @@ void Context::vertexAttribPointer(GLuint index,
 {
     mGLState.setVertexAttribPointer(this, index, mGLState.getTargetBuffer(BufferBinding::Array),
                                     size, type, ConvertToBool(normalized), false, stride, ptr);
+    mStateCache.updateActiveAttribsMask(this);
 }
 
 void Context::vertexAttribFormat(GLuint attribIndex,
@@ -4629,6 +4636,7 @@ void Context::vertexAttribIFormat(GLuint attribIndex,
 void Context::vertexAttribBinding(GLuint attribIndex, GLuint bindingIndex)
 {
     mGLState.setVertexAttribBinding(this, attribIndex, bindingIndex);
+    mStateCache.updateActiveAttribsMask(this);
 }
 
 void Context::vertexBindingDivisor(GLuint bindingIndex, GLuint divisor)
@@ -4649,6 +4657,7 @@ void Context::vertexAttribIPointer(GLuint index,
 {
     mGLState.setVertexAttribPointer(this, index, mGLState.getTargetBuffer(BufferBinding::Array),
                                     size, type, false, true, stride, pointer);
+    mStateCache.updateActiveAttribsMask(this);
 }
 
 void Context::vertexAttribI4i(GLuint index, GLint x, GLint y, GLint z, GLint w)
@@ -5534,6 +5543,7 @@ void Context::linkProgram(GLuint program)
     ASSERT(programObject);
     handleError(programObject->link(this));
     mGLState.onProgramExecutableChange(programObject);
+    mStateCache.updateActiveAttribsMask(this);
 }
 
 void Context::releaseShaderCompiler()
@@ -5741,6 +5751,7 @@ void Context::programBinary(GLuint program, GLenum binaryFormat, const void *bin
     ASSERT(programObject != nullptr);
 
     handleError(programObject->loadBinary(this, binaryFormat, binary, length));
+    mStateCache.updateActiveAttribsMask(this);
 }
 
 void Context::uniform1ui(GLint location, GLuint v0)
@@ -7536,23 +7547,6 @@ bool Context::isGLES1() const
     return mState.getClientVersion() < Version(2, 0);
 }
 
-AttributesMask Context::getActiveBufferedAttribsMask() const
-{
-    // TODO(jmadill): Cache this. http://anglebug.com/1391
-    ASSERT(mGLState.getProgram() || isGLES1());
-
-    const AttributesMask &activeAttribs =
-        isGLES1() ? mGLState.gles1().getVertexArraysAttributeMask()
-                  : mGLState.getProgram()->getActiveAttribLocationsMask();
-
-    const VertexArray *vao = mGLState.getVertexArray();
-    ASSERT(vao);
-
-    const AttributesMask &clientAttribs = vao->getEnabledClientMemoryAttribsMask();
-
-    return (activeAttribs & vao->getEnabledAttributesMask() & ~clientAttribs);
-}
-
 // ErrorSet implementation.
 ErrorSet::ErrorSet(Context *context) : mContext(context)
 {
@@ -7597,4 +7591,39 @@ GLenum ErrorSet::popError()
     mErrors.erase(mErrors.begin());
     return error;
 }
+
+// StateCache implementation.
+StateCache::StateCache() : mCachedHasAnyEnabledClientAttrib(false)
+{
+}
+
+StateCache::~StateCache() = default;
+
+void StateCache::updateActiveAttribsMask(Context *context)
+{
+    bool isGLES1         = context->isGLES1();
+    const State &glState = context->getGLState();
+
+    if (!isGLES1 && !glState.getProgram())
+    {
+        mCachedActiveBufferedAttribsMask = AttributesMask();
+        mCachedActiveClientAttribsMask   = AttributesMask();
+        return;
+    }
+
+    AttributesMask activeAttribs = isGLES1 ? glState.gles1().getVertexArraysAttributeMask()
+                                           : glState.getProgram()->getActiveAttribLocationsMask();
+
+    const VertexArray *vao = glState.getVertexArray();
+    ASSERT(vao);
+
+    const AttributesMask &clientAttribs  = vao->getClientAttribsMask();
+    const AttributesMask &enabledAttribs = vao->getEnabledAttributesMask();
+
+    activeAttribs &= enabledAttribs;
+
+    mCachedActiveClientAttribsMask   = activeAttribs & clientAttribs;
+    mCachedActiveBufferedAttribsMask = activeAttribs & ~clientAttribs;
+    mCachedHasAnyEnabledClientAttrib = (clientAttribs & enabledAttribs).any();
+}
 }  // namespace gl

src/libANGLE/Context.h

@@ -81,6 +81,32 @@ class ErrorSet : angle::NonCopyable
     mutable std::set<GLenum> mErrors;
 };
 
+// Helper class for managing cache variables and state changes.
+class StateCache final : angle::NonCopyable
+{
+  public:
+    StateCache();
+    ~StateCache();
+
+    // Places that can trigger updateActiveAttribsMask:
+    // 1. GLES1: clientActiveTexture.
+    // 2. GLES1: disableClientState/enableClientState.
+    // 3. Context: linkProgram/useProgram/programBinary. Note: should check programBinary bits.
+    // 4. Context: bindVertexArray.
+    // 5. Vertex Array: most any state change.
+    AttributesMask getActiveBufferedAttribsMask() const { return mCachedActiveBufferedAttribsMask; }
+    AttributesMask getActiveClientAttribsMask() const { return mCachedActiveClientAttribsMask; }
+    bool hasAnyEnabledClientAttrib() const { return mCachedHasAnyEnabledClientAttrib; }
+
+    // Cache update functions.
+    void updateActiveAttribsMask(Context *context);
+
+  private:
+    AttributesMask mCachedActiveBufferedAttribsMask;
+    AttributesMask mCachedActiveClientAttribsMask;
+    bool mCachedHasAnyEnabledClientAttrib;
+};
+
 class Context final : public egl::LabeledObject, angle::NonCopyable
 {
   public:
@@ -1493,7 +1519,7 @@ class Context final : public egl::LabeledObject, angle::NonCopyable
     // GL_KHR_parallel_shader_compile
     void maxShaderCompilerThreads(GLuint count);
 
-    AttributesMask getActiveBufferedAttribsMask() const;
+    const StateCache &getStateCache() const { return mStateCache; }
 
   private:
     void initialize();
@@ -1616,6 +1642,9 @@ class Context final : public egl::LabeledObject, angle::NonCopyable
 
     State::DirtyObjects mDrawDirtyObjects;
     State::DirtyObjects mPathOperationDirtyObjects;
+
+    StateCache mStateCache;
+
     State::DirtyBits mTexImageDirtyBits;
     State::DirtyObjects mTexImageDirtyObjects;
     State::DirtyBits mReadPixelsDirtyBits;

src/libANGLE/Context_gles_1_0.cpp

@@ -59,6 +59,7 @@ void Context::clearDepthx(GLfixed depth)
 void Context::clientActiveTexture(GLenum texture)
 {
     mGLState.gles1().setClientTextureUnit(texture - GL_TEXTURE0);
+    mStateCache.updateActiveAttribsMask(this);
 }
 
 void Context::clipPlanef(GLenum p, const GLfloat *eqn)
@@ -109,12 +110,14 @@ void Context::disableClientState(ClientVertexArrayType clientState)
 {
     mGLState.gles1().setClientStateEnabled(clientState, false);
     disableVertexAttribArray(vertexArrayIndex(clientState));
+    mStateCache.updateActiveAttribsMask(this);
 }
 
 void Context::enableClientState(ClientVertexArrayType clientState)
 {
     mGLState.gles1().setClientStateEnabled(clientState, true);
     enableVertexAttribArray(vertexArrayIndex(clientState));
+    mStateCache.updateActiveAttribsMask(this);
 }
 
 void Context::fogf(GLenum pname, GLfloat param)

src/libANGLE/VertexArray.cpp

@@ -26,15 +26,13 @@ VertexArrayState::VertexArrayState(size_t maxAttribs, size_t maxAttribBindings)
         mVertexAttributes.emplace_back(static_cast<GLuint>(i));
         mBindingToAttributeMasks[i].set(i);
     }
-}
 
-VertexArrayState::~VertexArrayState()
-{
+    // Initially all attributes start as "client" with no buffer bound.
+    mClientMemoryAttribsMask.set();
 }
 
-gl::AttributesMask VertexArrayState::getEnabledClientMemoryAttribsMask() const
+VertexArrayState::~VertexArrayState()
 {
-    return (mClientMemoryAttribsMask & mEnabledAttributesMask);
 }
 
 bool VertexArrayState::hasEnabledNullPointerClientArray() const
@@ -189,6 +187,16 @@ void VertexArray::bindVertexBufferImpl(const Context *context,
     updateObserverBinding(bindingIndex);
     updateCachedBufferBindingSize(bindingIndex);
     updateCachedTransformFeedbackBindingValidation(bindingIndex, boundBuffer);
+
+    // Update client memory attribute pointers. Affects all bound attributes.
+    if (boundBuffer)
+    {
+        mState.mClientMemoryAttribsMask &= ~mState.mBindingToAttributeMasks[bindingIndex];
+    }
+    else
+    {
+        mState.mClientMemoryAttribsMask |= mState.mBindingToAttributeMasks[bindingIndex];
+    }
 }
 
 void VertexArray::bindVertexBuffer(const Context *context,
@@ -215,6 +223,10 @@ void VertexArray::setVertexAttribBinding(const Context *context,
         mState.setAttribBinding(attribIndex, bindingIndex);
 
         setDirtyAttribBit(attribIndex, DIRTY_ATTRIB_BINDING);
+
+        // Update client attribs mask.
+        bool hasBuffer = mState.mVertexBindings[bindingIndex].getBuffer().get() != nullptr;
+        mState.mClientMemoryAttribsMask.set(attribIndex, !hasBuffer);
     }
 }
 
@@ -307,7 +319,6 @@ void VertexArray::setVertexAttribPointer(const Context *context,
 
     setDirtyAttribBit(attribIndex, DIRTY_ATTRIB_POINTER);
 
-    mState.mClientMemoryAttribsMask.set(attribIndex, boundBuffer == nullptr);
     mState.mNullPointerClientMemoryAttribsMask.set(attribIndex,
                                                    boundBuffer == nullptr && pointer == nullptr);
 }

src/libANGLE/VertexArray.h

@@ -65,9 +65,6 @@ class VertexArrayState final : angle::NonCopyable
 
     void setAttribBinding(size_t attribIndex, GLuint newBindingIndex);
 
-    // Combines mClientMemoryAttribsMask with mEnabledAttributesMask.
-    gl::AttributesMask getEnabledClientMemoryAttribsMask() const;
-
     // Extra validation performed on the Vertex Array.
     bool hasEnabledNullPointerClientArray() const;
 
@@ -177,10 +174,7 @@ class VertexArray final : public angle::ObserverInterface, public LabeledObject
         return mState.getEnabledAttributesMask();
     }
 
-    gl::AttributesMask getEnabledClientMemoryAttribsMask() const
-    {
-        return mState.getEnabledClientMemoryAttribsMask();
-    }
+    gl::AttributesMask getClientAttribsMask() const { return mState.mClientMemoryAttribsMask; }
 
     bool hasEnabledNullPointerClientArray() const
     {

src/libANGLE/renderer/gl/VertexArrayGL.cpp

@@ -13,6 +13,7 @@
 #include "common/mathutil.h"
 #include "common/utilities.h"
 #include "libANGLE/Buffer.h"
+#include "libANGLE/Context.h"
 #include "libANGLE/angletypes.h"
 #include "libANGLE/formatutils.h"
 #include "libANGLE/renderer/gl/BufferGL.h"
@@ -149,7 +150,7 @@ gl::Error VertexArrayGL::syncDrawState(const gl::Context *context,
     // Check if any attributes need to be streamed, determines if the index range needs to be
     // computed
     const gl::AttributesMask &needsStreamingAttribs =
-        (mState.getEnabledClientMemoryAttribsMask() & activeAttributesMask);
+        context->getStateCache().getActiveClientAttribsMask();
 
     // Determine if an index buffer needs to be streamed and the range of vertices that need to be
     // copied

src/libANGLE/renderer/vulkan/VertexArrayVk.cpp

@@ -566,20 +566,15 @@ gl::Error VertexArrayVk::onDraw(const gl::Context *context,
     ContextVk *contextVk                    = vk::GetImpl(context);
     const gl::State &state                  = context->getGLState();
     const gl::Program *programGL            = state.getProgram();
-    const gl::AttributesMask &clientAttribs = mState.getEnabledClientMemoryAttribsMask();
-    const gl::AttributesMask &activeAttribs = programGL->getActiveAttribLocationsMask();
+    const gl::AttributesMask &clientAttribs = context->getStateCache().getActiveClientAttribsMask();
     uint32_t maxAttrib                      = programGL->getState().getMaxActiveAttribLocation();
 
     if (clientAttribs.any())
     {
-        const gl::AttributesMask &attribsToStream = (clientAttribs & activeAttribs);
-        if (attribsToStream.any())
-        {
-            ANGLE_TRY(drawCallParams.ensureIndexRangeResolved(context));
-            ANGLE_TRY(streamVertexData(contextVk, attribsToStream, drawCallParams));
-            commandBuffer->bindVertexBuffers(0, maxAttrib, mCurrentArrayBufferHandles.data(),
-                                             mCurrentArrayBufferOffsets.data());
-        }
+        ANGLE_TRY(drawCallParams.ensureIndexRangeResolved(context));
+        ANGLE_TRY(streamVertexData(contextVk, clientAttribs, drawCallParams));
+        commandBuffer->bindVertexBuffers(0, maxAttrib, mCurrentArrayBufferHandles.data(),
+                                         mCurrentArrayBufferOffsets.data());
     }
     else if (mVertexBuffersDirty || newCommandBuffer)
     {
@@ -588,8 +583,11 @@ gl::Error VertexArrayVk::onDraw(const gl::Context *context,
             commandBuffer->bindVertexBuffers(0, maxAttrib, mCurrentArrayBufferHandles.data(),
                                              mCurrentArrayBufferOffsets.data());
 
+            const gl::AttributesMask &bufferedAttribs =
+                context->getStateCache().getActiveBufferedAttribsMask();
+
             vk::CommandGraphResource *drawFramebuffer = vk::GetImpl(state.getDrawFramebuffer());
-            updateArrayBufferReadDependencies(drawFramebuffer, activeAttribs,
+            updateArrayBufferReadDependencies(drawFramebuffer, bufferedAttribs,
                                               contextVk->getRenderer()->getCurrentQueueSerial());
         }
 

src/libANGLE/validationES.cpp

@@ -83,13 +83,11 @@ bool ValidateDrawAttribs(Context *context, GLint primcount, GLint maxVertex, GLi
 {
     const gl::State &state     = context->getGLState();
     const gl::Program *program = state.getProgram();
+    const VertexArray *vao     = state.getVertexArray();
 
     bool webglCompatibility = context->getExtensions().webglCompatibility;
 
-    const VertexArray *vao              = state.getVertexArray();
-    const AttributesMask &clientAttribs = vao->getEnabledClientMemoryAttribsMask();
-
-    if (clientAttribs.any())
+    if (context->getStateCache().hasAnyEnabledClientAttrib())
     {
         if (webglCompatibility || !state.areClientArraysEnabled())
         {
@@ -118,7 +116,7 @@ bool ValidateDrawAttribs(Context *context, GLint primcount, GLint maxVertex, GLi
     const auto &vertexAttribs  = vao->getVertexAttributes();
     const auto &vertexBindings = vao->getVertexBindings();
 
-    const AttributesMask &activeAttribs = context->getActiveBufferedAttribsMask();
+    const AttributesMask &activeAttribs = context->getStateCache().getActiveBufferedAttribsMask();
 
     for (size_t attributeIndex : activeAttribs)
     {

src/tests/gl_tests/WebGLCompatibilityTest.cpp

@@ -4163,12 +4163,18 @@ TEST_P(WebGL2CompatibilityTest, TransformFeedbackCheckNullDeref)
     ANGLE_GL_PROGRAM(program, kVS, kFS);
     glUseProgram(program);
 
-    GLBuffer buffer;
-    glBindBuffer(GL_ARRAY_BUFFER, buffer);
     glEnableVertexAttribArray(0);
     glDrawArrays(GL_POINTS, 0, 1);
 
-    // This should fail because it is trying to pull one vertex from an empty buffer.
+    // This should fail because it is trying to pull a vertex with no buffer.
+    EXPECT_GL_ERROR(GL_INVALID_OPERATION);
+
+    GLBuffer buffer;
+    glBindBuffer(GL_ARRAY_BUFFER, buffer);
+    glVertexAttribPointer(0, 4, GL_FLOAT, GL_FALSE, 0, nullptr);
+
+    // This should fail because it is trying to pull a vertex from an empty buffer.
+    glDrawArrays(GL_POINTS, 0, 1);
     EXPECT_GL_ERROR(GL_INVALID_OPERATION);
 }