VertexArray: Cache element limit for buffer checks.

src/libANGLE/VertexArray.cpp

@@ -51,8 +51,10 @@ void VertexArrayState::setAttribBinding(size_t attribIndex, GLuint newBindingInd
 {
     ASSERT(attribIndex < MAX_VERTEX_ATTRIBS && newBindingIndex < MAX_VERTEX_ATTRIB_BINDINGS);
 
+    VertexAttribute &attrib = mVertexAttributes[attribIndex];
+
     // Update the binding-attribute map.
-    const GLuint oldBindingIndex = mVertexAttributes[attribIndex].bindingIndex;
+    const GLuint oldBindingIndex = attrib.bindingIndex;
     ASSERT(oldBindingIndex != newBindingIndex);
 
     ASSERT(mVertexBindings[oldBindingIndex].getBoundAttributesMask().test(attribIndex) &&
@@ -62,7 +64,8 @@ void VertexArrayState::setAttribBinding(size_t attribIndex, GLuint newBindingInd
     mVertexBindings[newBindingIndex].setBoundAttribute(attribIndex);
 
     // Set the attribute using the new binding.
-    mVertexAttributes[attribIndex].bindingIndex = newBindingIndex;
+    attrib.bindingIndex = newBindingIndex;
+    attrib.updateCachedElementLimit(mVertexBindings[newBindingIndex]);
 }
 
 // VertexArray implementation.
@@ -185,7 +188,7 @@ void VertexArray::bindVertexBufferImpl(const Context *context,
     binding->setStride(stride);
 
     updateObserverBinding(bindingIndex);
-    updateCachedBufferBindingSize(bindingIndex);
+    updateCachedBufferBindingSize(binding);
     updateCachedTransformFeedbackBindingValidation(bindingIndex, boundBuffer);
 
     // Update client memory attribute pointers. Affects all bound attributes.
@@ -234,8 +237,16 @@ void VertexArray::setVertexBindingDivisor(size_t bindingIndex, GLuint divisor)
 {
     ASSERT(bindingIndex < getMaxBindings());
 
-    mState.mVertexBindings[bindingIndex].setDivisor(divisor);
+    VertexBinding &binding = mState.mVertexBindings[bindingIndex];
+
+    binding.setDivisor(divisor);
     setDirtyBindingBit(bindingIndex, DIRTY_BINDING_DIVISOR);
+
+    // Trigger updates in all bound attributes.
+    for (size_t attribIndex : binding.getBoundAttributesMask())
+    {
+        mState.mVertexAttributes[attribIndex].updateCachedElementLimit(binding);
+    }
 }
 
 void VertexArray::setVertexAttribFormatImpl(size_t attribIndex,
@@ -255,7 +266,6 @@ 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,
@@ -267,6 +277,9 @@ void VertexArray::setVertexAttribFormat(size_t attribIndex,
 {
     setVertexAttribFormatImpl(attribIndex, size, type, normalized, pureInteger, relativeOffset);
     setDirtyAttribBit(attribIndex, DIRTY_ATTRIB_FORMAT);
+
+    VertexAttribute &attrib = mState.mVertexAttributes[attribIndex];
+    attrib.updateCachedElementLimit(mState.mVertexBindings[attrib.bindingIndex]);
 }
 
 void VertexArray::setVertexAttribDivisor(const Context *context, size_t attribIndex, GLuint divisor)
@@ -281,9 +294,10 @@ void VertexArray::enableAttribute(size_t attribIndex, bool enabledState)
 {
     ASSERT(attribIndex < getMaxAttribs());
 
-    mState.mVertexAttributes[attribIndex].enabled = enabledState;
-    mState.mVertexAttributesTypeMask.setIndex(
-        GetVertexAttributeBaseType(mState.mVertexAttributes[attribIndex]), attribIndex);
+    VertexAttribute &attrib = mState.mVertexAttributes[attribIndex];
+
+    attrib.enabled = enabledState;
+    mState.mVertexAttributesTypeMask.setIndex(GetVertexAttributeBaseType(attrib), attribIndex);
 
     setDirtyAttribBit(attribIndex, DIRTY_ATTRIB_ENABLED);
 
@@ -399,7 +413,7 @@ void VertexArray::onSubjectStateChange(const gl::Context *context,
             setDependentDirtyBit(context, false, index);
             if (index < mArrayBufferObserverBindings.size())
             {
-                updateCachedBufferBindingSize(index);
+                updateCachedBufferBindingSize(&mState.mVertexBindings[index]);
             }
             break;
 
@@ -434,14 +448,12 @@ void VertexArray::updateObserverBinding(size_t bindingIndex)
                                                                 : nullptr);
 }
 
-void VertexArray::updateCachedVertexAttributeSize(size_t attribIndex)
+void VertexArray::updateCachedBufferBindingSize(VertexBinding *binding)
 {
-    mState.mVertexAttributes[attribIndex].updateCachedSizePlusRelativeOffset();
-}
-
-void VertexArray::updateCachedBufferBindingSize(size_t bindingIndex)
-{
-    mState.mVertexBindings[bindingIndex].updateCachedBufferSizeMinusOffset();
+    for (size_t boundAttribute : binding->getBoundAttributesMask())
+    {
+        mState.mVertexAttributes[boundAttribute].updateCachedElementLimit(*binding);
+    }
 }
 
 void VertexArray::updateCachedTransformFeedbackBindingValidation(size_t bindingIndex,

src/libANGLE/VertexArray.h

@@ -259,8 +259,7 @@ class VertexArray final : public angle::ObserverInterface, public LabeledObject
                               angle::SubjectIndex index);
 
     // These are used to optimize draw call validation.
-    void updateCachedVertexAttributeSize(size_t attribIndex);
-    void updateCachedBufferBindingSize(size_t bindingIndex);
+    void updateCachedBufferBindingSize(VertexBinding *binding);
     void updateCachedTransformFeedbackBindingValidation(size_t bindingIndex, const Buffer *buffer);
 
     GLuint mId;

src/libANGLE/VertexAttribute.cpp

@@ -17,8 +17,7 @@ VertexBinding::VertexBinding() : VertexBinding(0)
 {
 }
 
-VertexBinding::VertexBinding(GLuint boundAttribute)
-    : mStride(16u), mDivisor(0), mOffset(0), mCachedBufferSizeMinusOffset(0)
+VertexBinding::VertexBinding(GLuint boundAttribute) : mStride(16u), mDivisor(0), mOffset(0)
 {
     mBoundAttributesMask.set(boundAttribute);
 }
@@ -41,7 +40,6 @@ VertexBinding &VertexBinding::operator=(VertexBinding &&binding)
         mOffset  = binding.mOffset;
         mBoundAttributesMask = binding.mBoundAttributesMask;
         std::swap(binding.mBuffer, mBuffer);
-        mCachedBufferSizeMinusOffset = binding.mCachedBufferSizeMinusOffset;
     }
     return *this;
 }
@@ -61,22 +59,6 @@ void VertexBinding::onContainerBindingChanged(const Context *context, bool bound
         mBuffer->onBindingChanged(context, bound, BufferBinding::Array, true);
 }
 
-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;
-    }
-}
-
 VertexAttribute::VertexAttribute(GLuint bindingIndex)
     : enabled(false),
       type(GL_FLOAT),
@@ -87,7 +69,7 @@ VertexAttribute::VertexAttribute(GLuint bindingIndex)
       relativeOffset(0),
       vertexAttribArrayStride(0),
       bindingIndex(bindingIndex),
-      cachedSizePlusRelativeOffset(16)
+      mCachedElementLimit(0)
 {
 }
 
@@ -101,7 +83,7 @@ VertexAttribute::VertexAttribute(VertexAttribute &&attrib)
       relativeOffset(attrib.relativeOffset),
       vertexAttribArrayStride(attrib.vertexAttribArrayStride),
       bindingIndex(attrib.bindingIndex),
-      cachedSizePlusRelativeOffset(attrib.cachedSizePlusRelativeOffset)
+      mCachedElementLimit(attrib.mCachedElementLimit)
 {
 }
 
@@ -118,17 +100,56 @@ VertexAttribute &VertexAttribute::operator=(VertexAttribute &&attrib)
         relativeOffset          = attrib.relativeOffset;
         vertexAttribArrayStride = attrib.vertexAttribArrayStride;
         bindingIndex            = attrib.bindingIndex;
-        cachedSizePlusRelativeOffset = attrib.cachedSizePlusRelativeOffset;
+        mCachedElementLimit     = attrib.mCachedElementLimit;
     }
     return *this;
 }
 
-void VertexAttribute::updateCachedSizePlusRelativeOffset()
+void VertexAttribute::updateCachedElementLimit(const VertexBinding &binding)
 {
-    ASSERT(relativeOffset <=
-           std::numeric_limits<GLuint64>::max() - ComputeVertexAttributeTypeSize(*this));
-    cachedSizePlusRelativeOffset =
-        relativeOffset + static_cast<GLuint64>(ComputeVertexAttributeTypeSize(*this));
+    Buffer *buffer = binding.getBuffer().get();
+    if (!buffer)
+    {
+        mCachedElementLimit = 0;
+        return;
+    }
+
+    angle::CheckedNumeric<GLint64> bufferSize(buffer->getSize());
+    angle::CheckedNumeric<GLint64> bufferOffset(binding.getOffset());
+    angle::CheckedNumeric<GLint64> attribOffset(relativeOffset);
+    angle::CheckedNumeric<GLint64> attribSize(ComputeVertexAttributeTypeSize(*this));
+
+    // (buffer.size - buffer.offset - attrib.relativeOffset - attrib.size) / binding.stride
+    angle::CheckedNumeric<GLint64> elementLimit =
+        (bufferSize - bufferOffset - attribOffset - attribSize);
+
+    if (binding.getStride() > 0)
+    {
+        angle::CheckedNumeric<GLint64> bindingStride(binding.getStride());
+        elementLimit /= bindingStride;
+    }
+    else
+    {
+        // Special case for a zero stride. If we can fit one vertex we can fit infinite vertices.
+        mCachedElementLimit = elementLimit.ValueOrDefault(-1);
+        if (mCachedElementLimit >= 0)
+        {
+            mCachedElementLimit = std::numeric_limits<GLint64>::max();
+        }
+        return;
+    }
+
+    if (binding.getDivisor() > 0)
+    {
+        // For instanced draws, the element count is floor(instanceCount - 1) / binding.divisor.
+        angle::CheckedNumeric<GLint64> bindingDivisor(binding.getDivisor());
+        elementLimit *= bindingDivisor;
+
+        // We account for the floor() part rounding by adding a rounding constant.
+        elementLimit += bindingDivisor - 1;
+    }
+
+    mCachedElementLimit = elementLimit.ValueOrDefault(-1);
 }
 
 size_t ComputeVertexAttributeTypeSize(const VertexAttribute& attrib)

src/libANGLE/VertexAttribute.h

@@ -43,16 +43,8 @@ class VertexBinding final : angle::NonCopyable
 
     void onContainerBindingChanged(const Context *context, bool bound) const;
 
-    GLuint64 getCachedBufferSizeMinusOffset() const
-    {
-        return mCachedBufferSizeMinusOffset;
-    }
-
     const AttributesMask &getBoundAttributesMask() const { return mBoundAttributesMask; }
 
-    // Called from VertexArray.
-    void updateCachedBufferSizeMinusOffset();
-
     void setBoundAttribute(size_t index) { mBoundAttributesMask.set(index); }
 
     void resetBoundAttribute(size_t index) { mBoundAttributesMask.reset(index); }
@@ -66,9 +58,6 @@ class VertexBinding final : angle::NonCopyable
 
     // Mapping from this binding to all of the attributes that are using this binding.
     AttributesMask mBoundAttributesMask;
-
-    // This is kept in sync by the VertexArray. It is used to optimize draw call validation.
-    GLuint64 mCachedBufferSizeMinusOffset;
 };
 
 //
@@ -81,7 +70,8 @@ struct VertexAttribute final : private angle::NonCopyable
     VertexAttribute &operator=(VertexAttribute &&attrib);
 
     // Called from VertexArray.
-    void updateCachedSizePlusRelativeOffset();
+    void updateCachedElementLimit(const VertexBinding &binding);
+    GLint64 getCachedElementLimit() const { return mCachedElementLimit; }
 
     bool enabled;  // For glEnable/DisableVertexAttribArray
     GLenum type;
@@ -95,8 +85,9 @@ struct VertexAttribute final : private angle::NonCopyable
     GLuint vertexAttribArrayStride;  // ONLY for queries of VERTEX_ATTRIB_ARRAY_STRIDE
     GLuint bindingIndex;
 
+  private:
     // This is kept in sync by the VertexArray. It is used to optimize draw call validation.
-    GLuint64 cachedSizePlusRelativeOffset;
+    GLint64 mCachedElementLimit;
 };
 
 size_t ComputeVertexAttributeTypeSize(const VertexAttribute &attrib);

src/libANGLE/validationES.cpp

@@ -137,45 +137,19 @@ bool ValidateDrawAttribs(Context *context, GLint primcount, GLint maxVertex, GLi
         const VertexBinding &binding = vertexBindings[attrib.bindingIndex];
         ASSERT(context->isGLES1() || program->isAttribLocationActive(attributeIndex));
 
-        GLint maxVertexElement = maxVertex;
-        GLuint divisor         = binding.getDivisor();
-        if (divisor != 0)
-        {
-            maxVertexElement = (primcount - 1) / divisor;
-        }
-
-        // We do manual overflow checks here instead of using safe_math.h because it was
-        // a bottleneck. Thanks to some properties of GL we know inequalities that can
-        // help us make the overflow checks faster.
-
-        // The max possible attribSize is 16 for a vector of 4 32 bit values.
-        constexpr uint64_t kMaxAttribSize = 16;
-        constexpr uint64_t kIntMax        = std::numeric_limits<int>::max();
-        constexpr uint64_t kUint64Max     = std::numeric_limits<uint64_t>::max();
+        GLint maxVertexElement = binding.getDivisor() != 0 ? (primcount - 1) : maxVertex;
 
-        // 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, "Unexpected type");
-        ASSERT(ComputeVertexAttributeStride(attrib, binding) == binding.getStride());
-        uint64_t attribStride = binding.getStride();
-        ASSERT(attribStride <= kIntMax && ComputeVertexAttributeTypeSize(attrib) <= kMaxAttribSize);
-
-        // 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.
-        if (attribDataSizeMinusAttribSize > kUint64Max - attrib.cachedSizePlusRelativeOffset)
+        if (maxVertexElement > attrib.getCachedElementLimit())
         {
-            ANGLE_VALIDATION_ERR(context, InvalidOperation(), IntegerOverflow);
-            return false;
-        }
+            // An overflow can happen when adding the offset. Negative indicates overflow.
+            if (attrib.getCachedElementLimit() < 0)
+            {
+                ANGLE_VALIDATION_ERR(context, InvalidOperation(), IntegerOverflow);
+                return false;
+            }
 
-        // [OpenGL ES 3.0.2] section 2.9.4 page 40:
-        // We can return INVALID_OPERATION if our array buffer does not have enough backing data.
-        if (attribDataSizeMinusAttribSize + attrib.cachedSizePlusRelativeOffset >
-            binding.getCachedBufferSizeMinusOffset())
-        {
+            // [OpenGL ES 3.0.2] section 2.9.4 page 40:
+            // We can return INVALID_OPERATION if our buffer does not have enough backing data.
             ANGLE_VALIDATION_ERR(context, InvalidOperation(), InsufficientVertexBufferSize);
             return false;
         }