Optimize ValidateDrawAttribs: Part 2.

src/libANGLE/Buffer.cpp

@@ -88,8 +88,8 @@ Error Buffer::bufferData(const Context *context,
     mState.mUsage = usage;
     mState.mSize  = size;
 
-    // Notify when data changes.
-    mImpl->onStateChange(context, angle::SubjectMessage::CONTENTS_CHANGED);
+    // Notify when storage changes.
+    mImpl->onStateChange(context, angle::SubjectMessage::STORAGE_CHANGED);
 
     return NoError();
 }

src/libANGLE/VertexArray.cpp

@@ -159,6 +159,7 @@ void VertexArray::bindVertexBufferImpl(const Context *context,
     binding->setStride(stride);
 
     updateObserverBinding(bindingIndex);
+    updateCachedBufferBindingSize(bindingIndex);
 }
 
 void VertexArray::bindVertexBuffer(const Context *context,
@@ -213,6 +214,7 @@ void VertexArray::setVertexAttribFormatImpl(size_t attribIndex,
     attrib->pureInteger    = pureInteger;
     attrib->relativeOffset = relativeOffset;
     mState.mVertexAttributesTypeMask.setIndex(GetVertexAttributeBaseType(*attrib), attribIndex);
+    attrib->updateCachedSizePlusRelativeOffset();
 }
 
 void VertexArray::setVertexAttribFormat(size_t attribIndex,
@@ -349,6 +351,12 @@ void VertexArray::onSubjectStateChange(const gl::Context *context,
     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)
@@ -358,4 +366,14 @@ void VertexArray::updateObserverBinding(size_t bindingIndex)
                                                                 : nullptr);
 }
 
+void VertexArray::updateCachedVertexAttributeSize(size_t attribIndex)
+{
+    mState.mVertexAttributes[attribIndex].updateCachedSizePlusRelativeOffset();
+}
+
+void VertexArray::updateCachedBufferBindingSize(size_t bindingIndex)
+{
+    mState.mVertexBindings[bindingIndex].updateCachedBufferSizeMinusOffset();
+}
+
 }  // namespace gl

src/libANGLE/VertexArray.h

@@ -255,6 +255,10 @@ class VertexArray final : public angle::ObserverInterface, public LabeledObject
     void updateObserverBinding(size_t bindingIndex);
     DirtyBitType getDirtyBitFromIndex(bool contentsChanged, angle::SubjectIndex index) const;
 
+    // These are used to optimize draw call validation.
+    void updateCachedVertexAttributeSize(size_t attribIndex);
+    void updateCachedBufferBindingSize(size_t bindingIndex);
+
     GLuint mId;
 
     VertexArrayState mState;

src/libANGLE/VertexAttribute.cpp

@@ -13,7 +13,8 @@ namespace gl
 
 // [OpenGL ES 3.1] (November 3, 2016) Section 20 Page 361
 // Table 20.2: Vertex Array Object State
-VertexBinding::VertexBinding() : mStride(16u), mDivisor(0), mOffset(0)
+VertexBinding::VertexBinding()
+    : mStride(16u), mDivisor(0), mOffset(0), mCachedBufferSizeMinusOffset(0)
 {
 }
 
@@ -34,6 +35,7 @@ VertexBinding &VertexBinding::operator=(VertexBinding &&binding)
         mDivisor = binding.mDivisor;
         mOffset  = binding.mOffset;
         std::swap(binding.mBuffer, mBuffer);
+        mCachedBufferSizeMinusOffset = binding.mCachedBufferSizeMinusOffset;
     }
     return *this;
 }
@@ -53,6 +55,27 @@ void VertexBinding::onContainerBindingChanged(bool bound)
         mBuffer->onBindingChanged(bound, BufferBinding::Array);
 }
 
+void VertexBinding::updateCachedBufferSizeMinusOffset()
+{
+    if (mBuffer.get())
+    {
+        angle::CheckedNumeric<GLuint64> checkedSize(mBuffer->getSize());
+        angle::CheckedNumeric<GLuint64> checkedOffset(mOffset);
+
+        // Use a default value of zero so checks will fail on overflow.
+        mCachedBufferSizeMinusOffset = (checkedSize - checkedOffset).ValueOrDefault(0);
+    }
+    else
+    {
+        mCachedBufferSizeMinusOffset = 0;
+    }
+}
+
+GLuint64 VertexBinding::getCachedBufferSizeMinusOffset() const
+{
+    return mCachedBufferSizeMinusOffset;
+}
+
 VertexAttribute::VertexAttribute(GLuint bindingIndex)
     : enabled(false),
       type(GL_FLOAT),
@@ -62,7 +85,8 @@ VertexAttribute::VertexAttribute(GLuint bindingIndex)
       pointer(nullptr),
       relativeOffset(0),
       vertexAttribArrayStride(0),
-      bindingIndex(bindingIndex)
+      bindingIndex(bindingIndex),
+      cachedSizePlusRelativeOffset(0)
 {
 }
 
@@ -75,7 +99,8 @@ VertexAttribute::VertexAttribute(VertexAttribute &&attrib)
       pointer(attrib.pointer),
       relativeOffset(attrib.relativeOffset),
       vertexAttribArrayStride(attrib.vertexAttribArrayStride),
-      bindingIndex(attrib.bindingIndex)
+      bindingIndex(attrib.bindingIndex),
+      cachedSizePlusRelativeOffset(attrib.cachedSizePlusRelativeOffset)
 {
 }
 
@@ -92,10 +117,19 @@ VertexAttribute &VertexAttribute::operator=(VertexAttribute &&attrib)
         relativeOffset          = attrib.relativeOffset;
         vertexAttribArrayStride = attrib.vertexAttribArrayStride;
         bindingIndex            = attrib.bindingIndex;
+        cachedSizePlusRelativeOffset = attrib.cachedSizePlusRelativeOffset;
     }
     return *this;
 }
 
+void VertexAttribute::updateCachedSizePlusRelativeOffset()
+{
+    ASSERT(relativeOffset <=
+           std::numeric_limits<GLuint64>::max() - ComputeVertexAttributeTypeSize(*this));
+    cachedSizePlusRelativeOffset =
+        relativeOffset + static_cast<GLuint64>(ComputeVertexAttributeTypeSize(*this));
+}
+
 size_t ComputeVertexAttributeTypeSize(const VertexAttribute& attrib)
 {
     GLuint size = attrib.size;

src/libANGLE/VertexAttribute.h

@@ -40,12 +40,19 @@ class VertexBinding final : angle::NonCopyable
     void setBuffer(const gl::Context *context, Buffer *bufferIn, bool containerIsBound);
     void onContainerBindingChanged(bool bound);
 
+    // Called from VertexArray.
+    void updateCachedBufferSizeMinusOffset();
+    GLuint64 getCachedBufferSizeMinusOffset() const;
+
   private:
     GLuint mStride;
     GLuint mDivisor;
     GLintptr mOffset;
 
     BindingPointer<Buffer> mBuffer;
+
+    // This is kept in sync by the VertexArray. It is used to optimize draw call validation.
+    GLuint64 mCachedBufferSizeMinusOffset;
 };
 
 //
@@ -57,6 +64,9 @@ struct VertexAttribute final : private angle::NonCopyable
     VertexAttribute(VertexAttribute &&attrib);
     VertexAttribute &operator=(VertexAttribute &&attrib);
 
+    // Called from VertexArray.
+    void updateCachedSizePlusRelativeOffset();
+
     bool enabled;  // For glEnable/DisableVertexAttribArray
     GLenum type;
     GLuint size;
@@ -68,6 +78,9 @@ struct VertexAttribute final : private angle::NonCopyable
 
     GLuint vertexAttribArrayStride;  // ONLY for queries of VERTEX_ATTRIB_ARRAY_STRIDE
     GLuint bindingIndex;
+
+    // This is kept in sync by the VertexArray. It is used to optimize draw call validation.
+    GLuint64 cachedSizePlusRelativeOffset;
 };
 
 size_t ComputeVertexAttributeTypeSize(const VertexAttribute &attrib);

src/libANGLE/renderer/gl/VertexArrayGL.cpp

@@ -650,7 +650,7 @@ void VertexArrayGL::syncDirtyBinding(const gl::Context *context,
                                      size_t bindingIndex,
                                      const gl::VertexArray::DirtyBindingBits &dirtyBindingBits)
 {
-    ASSERT(dirtyBindingBits.any());
+    // Dependent state changes in buffers can trigger updates with no dirty bits set.
 
     for (size_t dirtyBit : dirtyBindingBits)
     {

src/libANGLE/validationES.cpp

@@ -107,8 +107,8 @@ bool ValidateDrawAttribs(Context *context, GLint primcount, GLint maxVertex, GLi
     const auto &vertexAttribs  = vao->getVertexAttributes();
     const auto &vertexBindings = vao->getVertexBindings();
 
-    const AttributesMask &activeAttribs =
-        (program->getActiveAttribLocationsMask() & vao->getEnabledAttributesMask());
+    const AttributesMask &activeAttribs = (program->getActiveAttribLocationsMask() &
+                                           vao->getEnabledAttributesMask() & ~clientAttribs);
 
     for (size_t attributeIndex : activeAttribs)
     {
@@ -116,21 +116,11 @@ bool ValidateDrawAttribs(Context *context, GLint primcount, GLint maxVertex, GLi
         ASSERT(attrib.enabled);
 
         const VertexBinding &binding = vertexBindings[attrib.bindingIndex];
-        gl::Buffer *buffer           = binding.getBuffer().get();
-        if (!buffer)
-        {
-            continue;
-        }
-
         ASSERT(program->isAttribLocationActive(attributeIndex));
 
-        GLint maxVertexElement = 0;
+        GLint maxVertexElement = maxVertex;
         GLuint divisor         = binding.getDivisor();
-        if (divisor == 0)
-        {
-            maxVertexElement = maxVertex;
-        }
-        else
+        if (divisor != 0)
         {
             maxVertexElement = (primcount - 1) / divisor;
         }
@@ -146,39 +136,49 @@ bool ValidateDrawAttribs(Context *context, GLint primcount, GLint maxVertex, GLi
 
         // We know attribStride is given as a GLsizei which is typedefed to int.
         // We also know an upper bound for attribSize.
-        static_assert(std::is_same<int, GLsizei>::value, "");
-        uint64_t attribStride = ComputeVertexAttributeStride(attrib, binding);
-        uint64_t attribSize   = ComputeVertexAttributeTypeSize(attrib);
-        ASSERT(attribStride <= kIntMax && attribSize <= kMaxAttribSize);
+        static_assert(std::is_same<int, GLsizei>::value, "Unexpected type");
+        ASSERT(ComputeVertexAttributeStride(attrib, binding) == binding.getStride());
+        uint64_t attribStride = binding.getStride();
+        ASSERT(attribStride <= kIntMax && ComputeVertexAttributeTypeSize(attrib) <= kMaxAttribSize);
 
-        // Computing the max offset using uint64_t without attrib.offset is overflow
-        // safe. Note: Last vertex element does not take the full stride!
-        static_assert(kIntMax * kIntMax < kUint64Max - kMaxAttribSize, "");
-        uint64_t attribDataSizeNoOffset = maxVertexElement * attribStride + attribSize;
+        // Computing the product of two 32-bit ints will fit in 64 bits without overflow.
+        static_assert(kIntMax * kIntMax < kUint64Max, "Unexpected overflow");
+        uint64_t attribDataSizeMinusAttribSize = maxVertexElement * attribStride;
 
         // An overflow can happen when adding the offset, check for it.
-        uint64_t attribOffset = ComputeVertexAttributeOffset(attrib, binding);
-        if (attribDataSizeNoOffset > kUint64Max - attribOffset)
+        if (attribDataSizeMinusAttribSize > kUint64Max - attrib.cachedSizePlusRelativeOffset)
         {
             ANGLE_VALIDATION_ERR(context, InvalidOperation(), IntegerOverflow);
             return false;
         }
-        uint64_t attribDataSizeWithOffset = attribDataSizeNoOffset + attribOffset;
 
         // [OpenGL ES 3.0.2] section 2.9.4 page 40:
-        // We can return INVALID_OPERATION if our vertex attribute does not have
-        // enough backing data.
-        if (attribDataSizeWithOffset > static_cast<uint64_t>(buffer->getSize()))
+        // We can return INVALID_OPERATION if our array buffer does not have enough backing data.
+        if (attribDataSizeMinusAttribSize + attrib.cachedSizePlusRelativeOffset >
+            binding.getCachedBufferSizeMinusOffset())
         {
             ANGLE_VALIDATION_ERR(context, InvalidOperation(), InsufficientVertexBufferSize);
             return false;
         }
+    }
 
-        if (webglCompatibility && buffer->isBoundForTransformFeedbackAndOtherUse())
+    // TODO(jmadill): Cache this. http://anglebug.com/1391
+    if (webglCompatibility)
+    {
+        for (size_t attributeIndex : activeAttribs)
         {
-            ANGLE_VALIDATION_ERR(context, InvalidOperation(),
-                                 VertexBufferBoundForTransformFeedback);
-            return false;
+            const VertexAttribute &attrib = vertexAttribs[attributeIndex];
+            ASSERT(attrib.enabled);
+
+            const VertexBinding &binding = vertexBindings[attrib.bindingIndex];
+
+            gl::Buffer *buffer = binding.getBuffer().get();
+            if (buffer->isBoundForTransformFeedbackAndOtherUse())
+            {
+                ANGLE_VALIDATION_ERR(context, InvalidOperation(),
+                                     VertexBufferBoundForTransformFeedback);
+                return false;
+            }
         }
     }