Optimize ValidateDrawAttribs: Part 3.

src/libANGLE/Buffer.cpp

@@ -243,7 +243,7 @@ bool Buffer::isBoundForTransformFeedbackAndOtherUse() const
            mState.mTransformFeedbackBindingCount != mState.mBindingCount;
 }
 
-void Buffer::onBindingChanged(bool bound, BufferBinding target)
+void Buffer::onBindingChanged(const Context *context, bool bound, BufferBinding target)
 {
     ASSERT(bound || mState.mBindingCount > 0);
     mState.mBindingCount += bound ? 1 : -1;
@@ -251,6 +251,8 @@ void Buffer::onBindingChanged(bool bound, BufferBinding target)
     {
         ASSERT(bound || mState.mTransformFeedbackBindingCount > 0);
         mState.mTransformFeedbackBindingCount += bound ? 1 : -1;
+
+        mImpl->onStateChange(context, angle::SubjectMessage::BINDING_CHANGED);
     }
 }
 

src/libANGLE/Buffer.h

@@ -116,7 +116,7 @@ class Buffer final : public RefCountObject, public LabeledObject
 
     bool isBound() const;
     bool isBoundForTransformFeedbackAndOtherUse() const;
-    void onBindingChanged(bool bound, BufferBinding target);
+    void onBindingChanged(const Context *context, bool bound, BufferBinding target);
 
   private:
     BufferState mState;

src/libANGLE/Context.cpp

@@ -1012,7 +1012,7 @@ void Context::bindDrawFramebuffer(GLuint framebufferHandle)
 void Context::bindVertexArray(GLuint vertexArrayHandle)
 {
     VertexArray *vertexArray = checkVertexArrayAllocation(vertexArrayHandle);
-    mGLState.setVertexArrayBinding(vertexArray);
+    mGLState.setVertexArrayBinding(this, vertexArray);
 }
 
 void Context::bindVertexBuffer(GLuint bindingIndex,
@@ -2669,7 +2669,7 @@ void Context::detachVertexArray(GLuint vertexArray)
     // [OpenGL ES 3.0.2] section 2.10 page 43:
     // If a vertex array object that is currently bound is deleted, the binding
     // for that object reverts to zero and the default vertex array becomes current.
-    if (mGLState.removeVertexArrayBinding(vertexArray))
+    if (mGLState.removeVertexArrayBinding(this, vertexArray))
     {
         bindVertexArray(0);
     }

src/libANGLE/Observer.h

@@ -30,6 +30,7 @@ enum class SubjectMessage
 {
     CONTENTS_CHANGED,
     STORAGE_CHANGED,
+    BINDING_CHANGED,
     DEPENDENT_DIRTY_BITS,
 };
 

src/libANGLE/State.cpp

@@ -45,10 +45,10 @@ void UpdateBufferBinding(const Context *context,
                          BufferBinding target)
 {
     if (binding->get())
-        (*binding)->onBindingChanged(false, target);
+        (*binding)->onBindingChanged(context, false, target);
     binding->set(context, buffer);
     if (binding->get())
-        (*binding)->onBindingChanged(true, target);
+        (*binding)->onBindingChanged(context, true, target);
 }
 
 void UpdateBufferBinding(const Context *context,
@@ -59,10 +59,10 @@ void UpdateBufferBinding(const Context *context,
                          GLsizeiptr size)
 {
     if (binding->get())
-        (*binding)->onBindingChanged(false, target);
+        (*binding)->onBindingChanged(context, false, target);
     binding->set(context, buffer, offset, size);
     if (binding->get())
-        (*binding)->onBindingChanged(true, target);
+        (*binding)->onBindingChanged(context, true, target);
 }
 
 State::State()
@@ -291,7 +291,7 @@ void State::reset(const Context *context)
     mProgramPipeline.set(context, nullptr);
 
     if (mTransformFeedback.get())
-        mTransformFeedback->onBindingChanged(false);
+        mTransformFeedback->onBindingChanged(context, false);
     mTransformFeedback.set(context, nullptr);
 
     for (State::ActiveQueryMap::iterator i = mActiveQueries.begin(); i != mActiveQueries.end(); i++)
@@ -1165,15 +1165,15 @@ bool State::removeDrawFramebufferBinding(GLuint framebuffer)
     return false;
 }
 
-void State::setVertexArrayBinding(VertexArray *vertexArray)
+void State::setVertexArrayBinding(const Context *context, VertexArray *vertexArray)
 {
     if (mVertexArray == vertexArray)
         return;
     if (mVertexArray)
-        mVertexArray->onBindingChanged(false);
+        mVertexArray->onBindingChanged(context, false);
     mVertexArray = vertexArray;
     if (vertexArray)
-        vertexArray->onBindingChanged(true);
+        vertexArray->onBindingChanged(context, true);
     mDirtyBits.set(DIRTY_BIT_VERTEX_ARRAY_BINDING);
 
     if (mVertexArray && mVertexArray->hasAnyDirtyBit())
@@ -1194,11 +1194,11 @@ VertexArray *State::getVertexArray() const
     return mVertexArray;
 }
 
-bool State::removeVertexArrayBinding(GLuint vertexArray)
+bool State::removeVertexArrayBinding(const Context *context, GLuint vertexArray)
 {
     if (mVertexArray && mVertexArray->id() == vertexArray)
     {
-        mVertexArray->onBindingChanged(false);
+        mVertexArray->onBindingChanged(context, false);
         mVertexArray = nullptr;
         mDirtyBits.set(DIRTY_BIT_VERTEX_ARRAY_BINDING);
         mDirtyObjects.set(DIRTY_OBJECT_VERTEX_ARRAY);
@@ -1274,10 +1274,10 @@ void State::setTransformFeedbackBinding(const Context *context,
     if (transformFeedback == mTransformFeedback.get())
         return;
     if (mTransformFeedback.get())
-        mTransformFeedback->onBindingChanged(false);
+        mTransformFeedback->onBindingChanged(context, false);
     mTransformFeedback.set(context, transformFeedback);
     if (mTransformFeedback.get())
-        mTransformFeedback->onBindingChanged(true);
+        mTransformFeedback->onBindingChanged(context, true);
     mDirtyBits.set(DIRTY_BIT_TRANSFORM_FEEDBACK_BINDING);
 }
 
@@ -1298,7 +1298,7 @@ bool State::removeTransformFeedbackBinding(const Context *context, GLuint transf
     if (mTransformFeedback.id() == transformFeedback)
     {
         if (mTransformFeedback.get())
-            mTransformFeedback->onBindingChanged(false);
+            mTransformFeedback->onBindingChanged(context, false);
         mTransformFeedback.set(context, nullptr);
         return true;
     }

src/libANGLE/State.h

@@ -204,10 +204,10 @@ class State : public angle::ObserverInterface, angle::NonCopyable
     bool removeDrawFramebufferBinding(GLuint framebuffer);
 
     // Vertex array object binding manipulation
-    void setVertexArrayBinding(VertexArray *vertexArray);
+    void setVertexArrayBinding(const Context *context, VertexArray *vertexArray);
     GLuint getVertexArrayId() const;
     VertexArray *getVertexArray() const;
-    bool removeVertexArrayBinding(GLuint vertexArray);
+    bool removeVertexArrayBinding(const Context *context, GLuint vertexArray);
 
     // Program binding manipulation
     void setProgram(const Context *context, Program *newProgram);

src/libANGLE/TransformFeedback.cpp

@@ -238,7 +238,7 @@ void TransformFeedback::detachBuffer(const Context *context, GLuint bufferName)
         {
             if (isBound)
             {
-                mState.mIndexedBuffers[index]->onBindingChanged(false,
+                mState.mIndexedBuffers[index]->onBindingChanged(context, false,
                                                                 BufferBinding::TransformFeedback);
             }
             mState.mIndexedBuffers[index].set(context, nullptr);
@@ -257,12 +257,13 @@ void TransformFeedback::bindIndexedBuffer(const Context *context,
     bool isBound = context && context->isCurrentTransformFeedback(this);
     if (isBound && mState.mIndexedBuffers[index].get())
     {
-        mState.mIndexedBuffers[index]->onBindingChanged(false, BufferBinding::TransformFeedback);
+        mState.mIndexedBuffers[index]->onBindingChanged(context, false,
+                                                        BufferBinding::TransformFeedback);
     }
     mState.mIndexedBuffers[index].set(context, buffer, offset, size);
     if (isBound && buffer)
     {
-        buffer->onBindingChanged(true, BufferBinding::TransformFeedback);
+        buffer->onBindingChanged(context, true, BufferBinding::TransformFeedback);
     }
 
     mImplementation->bindIndexedBuffer(index, mState.mIndexedBuffers[index]);
@@ -301,14 +302,14 @@ const rx::TransformFeedbackImpl *TransformFeedback::getImplementation() const
     return mImplementation;
 }
 
-void TransformFeedback::onBindingChanged(bool bound)
+void TransformFeedback::onBindingChanged(const Context *context, bool bound)
 {
     for (auto &buffer : mState.mIndexedBuffers)
     {
         if (buffer.get())
         {
-            buffer->onBindingChanged(bound, BufferBinding::TransformFeedback);
+            buffer->onBindingChanged(context, bound, BufferBinding::TransformFeedback);
         }
     }
 }
-}
+}  // namespace gl

src/libANGLE/TransformFeedback.h

@@ -97,15 +97,15 @@ class TransformFeedback final : public RefCountObject, public LabeledObject
     rx::TransformFeedbackImpl *getImplementation();
     const rx::TransformFeedbackImpl *getImplementation() const;
 
-    void onBindingChanged(bool bound);
+    void onBindingChanged(const Context *context, bool bound);
 
   private:
     void bindProgram(const Context *context, Program *program);
 
     TransformFeedbackState mState;
-    rx::TransformFeedbackImpl* mImplementation;
+    rx::TransformFeedbackImpl *mImplementation;
 };
 
-}
+}  // namespace gl
 
 #endif // LIBANGLE_TRANSFORM_FEEDBACK_H_

src/libANGLE/VertexArray.cpp

@@ -65,7 +65,7 @@ void VertexArray::onDestroy(const Context *context)
         binding.setBuffer(context, nullptr, isBound);
     }
     if (isBound && mState.mElementArrayBuffer.get())
-        mState.mElementArrayBuffer->onBindingChanged(false, BufferBinding::ElementArray);
+        mState.mElementArrayBuffer->onBindingChanged(context, false, BufferBinding::ElementArray);
     mState.mElementArrayBuffer.set(context, nullptr);
     mVertexArray->destroy(context);
     SafeDelete(mVertexArray);
@@ -106,7 +106,7 @@ void VertexArray::detachBuffer(const Context *context, GLuint bufferName)
     if (mState.mElementArrayBuffer.id() == bufferName)
     {
         if (isBound && mState.mElementArrayBuffer.get())
-            mState.mElementArrayBuffer->onBindingChanged(false, BufferBinding::Array);
+            mState.mElementArrayBuffer->onBindingChanged(context, false, BufferBinding::Array);
         mState.mElementArrayBuffer.set(context, nullptr);
     }
 }
@@ -160,6 +160,7 @@ void VertexArray::bindVertexBufferImpl(const Context *context,
 
     updateObserverBinding(bindingIndex);
     updateCachedBufferBindingSize(bindingIndex);
+    updateCachedTransformFeedbackBindingValidation(bindingIndex, boundBuffer);
 }
 
 void VertexArray::bindVertexBuffer(const Context *context,
@@ -287,10 +288,10 @@ void VertexArray::setElementArrayBuffer(const Context *context, Buffer *buffer)
 {
     bool isBound = context->isCurrentVertexArray(this);
     if (isBound && mState.mElementArrayBuffer.get())
-        mState.mElementArrayBuffer->onBindingChanged(false, BufferBinding::ElementArray);
+        mState.mElementArrayBuffer->onBindingChanged(context, false, BufferBinding::ElementArray);
     mState.mElementArrayBuffer.set(context, buffer);
     if (isBound && mState.mElementArrayBuffer.get())
-        mState.mElementArrayBuffer->onBindingChanged(true, BufferBinding::ElementArray);
+        mState.mElementArrayBuffer->onBindingChanged(context, true, BufferBinding::ElementArray);
     mElementArrayBufferObserverBinding.bind(buffer ? buffer->getImplementation() : nullptr);
     mDirtyBits.set(DIRTY_BIT_ELEMENT_ARRAY_BUFFER);
 }
@@ -315,13 +316,13 @@ gl::Error VertexArray::syncState(const Context *context)
     return gl::NoError();
 }
 
-void VertexArray::onBindingChanged(bool bound)
+void VertexArray::onBindingChanged(const Context *context, bool bound)
 {
     if (mState.mElementArrayBuffer.get())
-        mState.mElementArrayBuffer->onBindingChanged(bound, BufferBinding::ElementArray);
+        mState.mElementArrayBuffer->onBindingChanged(context, bound, BufferBinding::ElementArray);
     for (auto &binding : mState.mVertexBindings)
     {
-        binding.onContainerBindingChanged(bound);
+        binding.onContainerBindingChanged(context, bound);
     }
 }
 
@@ -346,17 +347,42 @@ void VertexArray::onSubjectStateChange(const gl::Context *context,
                                        angle::SubjectIndex index,
                                        angle::SubjectMessage message)
 {
-    bool contentsChanged  = (message == angle::SubjectMessage::CONTENTS_CHANGED);
+    switch (message)
+    {
+        case angle::SubjectMessage::CONTENTS_CHANGED:
+            setDependentDirtyBit(context, true, index);
+            break;
+
+        case angle::SubjectMessage::STORAGE_CHANGED:
+            setDependentDirtyBit(context, false, index);
+            if (index < mArrayBufferObserverBindings.size())
+            {
+                updateCachedBufferBindingSize(index);
+            }
+            break;
+
+        case angle::SubjectMessage::BINDING_CHANGED:
+            if (index < mArrayBufferObserverBindings.size())
+            {
+                const Buffer *buffer = mState.mVertexBindings[index].getBuffer().get();
+                updateCachedTransformFeedbackBindingValidation(index, buffer);
+            }
+            break;
+
+        default:
+            UNREACHABLE();
+            break;
+    }
+}
+
+void VertexArray::setDependentDirtyBit(const gl::Context *context,
+                                       bool contentsChanged,
+                                       angle::SubjectIndex index)
+{
     DirtyBitType dirtyBit = getDirtyBitFromIndex(contentsChanged, index);
     ASSERT(!mDirtyBitsGuard.valid() || mDirtyBitsGuard.value().test(dirtyBit));
     mDirtyBits.set(dirtyBit);
     context->getGLState().setVertexArrayDirty(this);
-
-    if (message == angle::SubjectMessage::STORAGE_CHANGED &&
-        index < mArrayBufferObserverBindings.size())
-    {
-        updateCachedBufferBindingSize(index);
-    }
 }
 
 void VertexArray::updateObserverBinding(size_t bindingIndex)
@@ -376,4 +402,32 @@ void VertexArray::updateCachedBufferBindingSize(size_t bindingIndex)
     mState.mVertexBindings[bindingIndex].updateCachedBufferSizeMinusOffset();
 }
 
+void VertexArray::updateCachedTransformFeedbackBindingValidation(size_t bindingIndex,
+                                                                 const Buffer *buffer)
+{
+    const bool hasConflict = buffer && buffer->isBoundForTransformFeedbackAndOtherUse();
+    mCachedTransformFeedbackConflictedBindingsMask.set(bindingIndex, hasConflict);
+}
+
+bool VertexArray::hasTransformFeedbackBindingConflict(const AttributesMask &activeAttribues) const
+{
+    // Fast check first.
+    if (!mCachedTransformFeedbackConflictedBindingsMask.any())
+    {
+        return false;
+    }
+
+    // Slow check. We must ensure that the conflicting attributes are enabled/active.
+    for (size_t attribIndex : activeAttribues)
+    {
+        const VertexAttribute &attrib = mState.mVertexAttributes[attribIndex];
+        if (mCachedTransformFeedbackConflictedBindingsMask[attrib.bindingIndex])
+        {
+            return true;
+        }
+    }
+
+    return false;
+}
+
 }  // namespace gl

src/libANGLE/VertexArray.h

@@ -244,7 +244,8 @@ class VertexArray final : public angle::ObserverInterface, public LabeledObject
     ComponentTypeMask getAttributesTypeMask() const { return mState.mVertexAttributesTypeMask; }
     AttributesMask getAttributesMask() const { return mState.mEnabledAttributesMask; }
 
-    void onBindingChanged(bool bound);
+    void onBindingChanged(const Context *context, bool bound);
+    bool hasTransformFeedbackBindingConflict(const AttributesMask &activeAttribues) const;
 
   private:
     ~VertexArray() override;
@@ -254,10 +255,14 @@ class VertexArray final : public angle::ObserverInterface, public LabeledObject
 
     void updateObserverBinding(size_t bindingIndex);
     DirtyBitType getDirtyBitFromIndex(bool contentsChanged, angle::SubjectIndex index) const;
+    void setDependentDirtyBit(const gl::Context *context,
+                              bool contentsChanged,
+                              angle::SubjectIndex index);
 
     // These are used to optimize draw call validation.
     void updateCachedVertexAttributeSize(size_t attribIndex);
     void updateCachedBufferBindingSize(size_t bindingIndex);
+    void updateCachedTransformFeedbackBindingValidation(size_t bindingIndex, const Buffer *buffer);
 
     GLuint mId;
 
@@ -271,6 +276,8 @@ class VertexArray final : public angle::ObserverInterface, public LabeledObject
 
     std::vector<angle::ObserverBinding> mArrayBufferObserverBindings;
     angle::ObserverBinding mElementArrayBufferObserverBinding;
+
+    AttributesMask mCachedTransformFeedbackConflictedBindingsMask;
 };
 
 }  // namespace gl

src/libANGLE/VertexAttribute.cpp

@@ -43,16 +43,16 @@ VertexBinding &VertexBinding::operator=(VertexBinding &&binding)
 void VertexBinding::setBuffer(const gl::Context *context, Buffer *bufferIn, bool containerIsBound)
 {
     if (mBuffer.get() && containerIsBound)
-        mBuffer->onBindingChanged(false, BufferBinding::Array);
+        mBuffer->onBindingChanged(context, false, BufferBinding::Array);
     mBuffer.set(context, bufferIn);
     if (mBuffer.get() && containerIsBound)
-        mBuffer->onBindingChanged(true, BufferBinding::Array);
+        mBuffer->onBindingChanged(context, true, BufferBinding::Array);
 }
 
-void VertexBinding::onContainerBindingChanged(bool bound)
+void VertexBinding::onContainerBindingChanged(const Context *context, bool bound)
 {
     if (mBuffer.get())
-        mBuffer->onBindingChanged(bound, BufferBinding::Array);
+        mBuffer->onBindingChanged(context, bound, BufferBinding::Array);
 }
 
 void VertexBinding::updateCachedBufferSizeMinusOffset()

src/libANGLE/VertexAttribute.h

@@ -38,7 +38,8 @@ class VertexBinding final : angle::NonCopyable
 
     const BindingPointer<Buffer> &getBuffer() const { return mBuffer; }
     void setBuffer(const gl::Context *context, Buffer *bufferIn, bool containerIsBound);
-    void onContainerBindingChanged(bool bound);
+
+    void onContainerBindingChanged(const Context *context, bool bound);
 
     // Called from VertexArray.
     void updateCachedBufferSizeMinusOffset();

src/libANGLE/validationES.cpp

@@ -172,24 +172,10 @@ bool ValidateDrawAttribs(Context *context, GLint primcount, GLint maxVertex, GLi
         }
     }
 
-    // TODO(jmadill): Cache this. http://anglebug.com/1391
-    if (webglCompatibility)
+    if (webglCompatibility && vao->hasTransformFeedbackBindingConflict(activeAttribs))
     {
-        for (size_t attributeIndex : activeAttribs)
-        {
-            const VertexAttribute &attrib = vertexAttribs[attributeIndex];
-            ASSERT(attrib.enabled);
-
-            const VertexBinding &binding = vertexBindings[attrib.bindingIndex];
-
-            gl::Buffer *buffer = binding.getBuffer().get();
-            if (buffer && buffer->isBoundForTransformFeedbackAndOtherUse())
-            {
-                ANGLE_VALIDATION_ERR(context, InvalidOperation(),
-                                     VertexBufferBoundForTransformFeedback);
-                return false;
-            }
-        }
+        ANGLE_VALIDATION_ERR(context, InvalidOperation(), VertexBufferBoundForTransformFeedback);
+        return false;
     }
 
     return true;