Commit a2d1d2db by Jamie Madill Committed by Commit Bot

Context: Cache VAO element limits.

Cache the minimum value for non instanced and instanced active attributes. The cache is updated in the following places: 1. Context: bindVertexArray. 2. Context: any executable change (linkProgram/useProgram/programBinary). 3. Vertex Array: any state change call. 4. Buffer: a dependent buffer resize. This greatly reduces the time we're spending in ValidateDrawAttribs. Bug: angleproject:1391 Change-Id: I84bb222a1b9736e6165fe40e972cd4299ca1178d Reviewed-on: https://chromium-review.googlesource.com/1150516 Commit-Queue: Jamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarGeoff Lang <geofflang@chromium.org>
parent cf0d8fef
...@@ -1108,6 +1108,7 @@ void Context::bindVertexArray(GLuint vertexArrayHandle) ...@@ -1108,6 +1108,7 @@ void Context::bindVertexArray(GLuint vertexArrayHandle)
mGLState.setVertexArrayBinding(this, vertexArray); mGLState.setVertexArrayBinding(this, vertexArray);
mVertexArrayObserverBinding.bind(vertexArray); mVertexArrayObserverBinding.bind(vertexArray);
mStateCache.updateActiveAttribsMask(this); mStateCache.updateActiveAttribsMask(this);
mStateCache.updateVertexElementLimits(this);
} }
void Context::bindVertexBuffer(GLuint bindingIndex, void Context::bindVertexBuffer(GLuint bindingIndex,
...@@ -1144,6 +1145,7 @@ void Context::useProgram(GLuint program) ...@@ -1144,6 +1145,7 @@ void Context::useProgram(GLuint program)
{ {
mGLState.setProgram(this, getProgram(program)); mGLState.setProgram(this, getProgram(program));
mStateCache.updateActiveAttribsMask(this); mStateCache.updateActiveAttribsMask(this);
mStateCache.updateVertexElementLimits(this);
} }
void Context::useProgramStages(GLuint pipeline, GLbitfield stages, GLuint program) void Context::useProgramStages(GLuint pipeline, GLbitfield stages, GLuint program)
...@@ -2798,6 +2800,7 @@ void Context::vertexAttribDivisor(GLuint index, GLuint divisor) ...@@ -2798,6 +2800,7 @@ void Context::vertexAttribDivisor(GLuint index, GLuint divisor)
{ {
mGLState.setVertexAttribDivisor(this, index, divisor); mGLState.setVertexAttribDivisor(this, index, divisor);
mStateCache.updateActiveAttribsMask(this); mStateCache.updateActiveAttribsMask(this);
mStateCache.updateVertexElementLimits(this);
} }
void Context::samplerParameteri(GLuint sampler, GLenum pname, GLint param) void Context::samplerParameteri(GLuint sampler, GLenum pname, GLint param)
...@@ -4405,6 +4408,7 @@ void Context::disableVertexAttribArray(GLuint index) ...@@ -4405,6 +4408,7 @@ void Context::disableVertexAttribArray(GLuint index)
{ {
mGLState.setEnableVertexAttribArray(index, false); mGLState.setEnableVertexAttribArray(index, false);
mStateCache.updateActiveAttribsMask(this); mStateCache.updateActiveAttribsMask(this);
mStateCache.updateVertexElementLimits(this);
} }
void Context::enable(GLenum cap) void Context::enable(GLenum cap)
...@@ -4416,6 +4420,7 @@ void Context::enableVertexAttribArray(GLuint index) ...@@ -4416,6 +4420,7 @@ void Context::enableVertexAttribArray(GLuint index)
{ {
mGLState.setEnableVertexAttribArray(index, true); mGLState.setEnableVertexAttribArray(index, true);
mStateCache.updateActiveAttribsMask(this); mStateCache.updateActiveAttribsMask(this);
mStateCache.updateVertexElementLimits(this);
} }
void Context::frontFace(GLenum mode) void Context::frontFace(GLenum mode)
...@@ -4624,6 +4629,7 @@ void Context::vertexAttribPointer(GLuint index, ...@@ -4624,6 +4629,7 @@ void Context::vertexAttribPointer(GLuint index,
mGLState.setVertexAttribPointer(this, index, mGLState.getTargetBuffer(BufferBinding::Array), mGLState.setVertexAttribPointer(this, index, mGLState.getTargetBuffer(BufferBinding::Array),
size, type, ConvertToBool(normalized), false, stride, ptr); size, type, ConvertToBool(normalized), false, stride, ptr);
mStateCache.updateActiveAttribsMask(this); mStateCache.updateActiveAttribsMask(this);
mStateCache.updateVertexElementLimits(this);
} }
void Context::vertexAttribFormat(GLuint attribIndex, void Context::vertexAttribFormat(GLuint attribIndex,
...@@ -4634,6 +4640,7 @@ void Context::vertexAttribFormat(GLuint attribIndex, ...@@ -4634,6 +4640,7 @@ void Context::vertexAttribFormat(GLuint attribIndex,
{ {
mGLState.setVertexAttribFormat(attribIndex, size, type, ConvertToBool(normalized), false, mGLState.setVertexAttribFormat(attribIndex, size, type, ConvertToBool(normalized), false,
relativeOffset); relativeOffset);
mStateCache.updateVertexElementLimits(this);
} }
void Context::vertexAttribIFormat(GLuint attribIndex, void Context::vertexAttribIFormat(GLuint attribIndex,
...@@ -4642,17 +4649,20 @@ void Context::vertexAttribIFormat(GLuint attribIndex, ...@@ -4642,17 +4649,20 @@ void Context::vertexAttribIFormat(GLuint attribIndex,
GLuint relativeOffset) GLuint relativeOffset)
{ {
mGLState.setVertexAttribFormat(attribIndex, size, type, false, true, relativeOffset); mGLState.setVertexAttribFormat(attribIndex, size, type, false, true, relativeOffset);
mStateCache.updateVertexElementLimits(this);
} }
void Context::vertexAttribBinding(GLuint attribIndex, GLuint bindingIndex) void Context::vertexAttribBinding(GLuint attribIndex, GLuint bindingIndex)
{ {
mGLState.setVertexAttribBinding(this, attribIndex, bindingIndex); mGLState.setVertexAttribBinding(this, attribIndex, bindingIndex);
mStateCache.updateActiveAttribsMask(this); mStateCache.updateActiveAttribsMask(this);
mStateCache.updateVertexElementLimits(this);
} }
void Context::vertexBindingDivisor(GLuint bindingIndex, GLuint divisor) void Context::vertexBindingDivisor(GLuint bindingIndex, GLuint divisor)
{ {
mGLState.setVertexBindingDivisor(bindingIndex, divisor); mGLState.setVertexBindingDivisor(bindingIndex, divisor);
mStateCache.updateVertexElementLimits(this);
} }
void Context::viewport(GLint x, GLint y, GLsizei width, GLsizei height) void Context::viewport(GLint x, GLint y, GLsizei width, GLsizei height)
...@@ -4669,6 +4679,7 @@ void Context::vertexAttribIPointer(GLuint index, ...@@ -4669,6 +4679,7 @@ void Context::vertexAttribIPointer(GLuint index,
mGLState.setVertexAttribPointer(this, index, mGLState.getTargetBuffer(BufferBinding::Array), mGLState.setVertexAttribPointer(this, index, mGLState.getTargetBuffer(BufferBinding::Array),
size, type, false, true, stride, pointer); size, type, false, true, stride, pointer);
mStateCache.updateActiveAttribsMask(this); mStateCache.updateActiveAttribsMask(this);
mStateCache.updateVertexElementLimits(this);
} }
void Context::vertexAttribI4i(GLuint index, GLint x, GLint y, GLint z, GLint w) void Context::vertexAttribI4i(GLuint index, GLint x, GLint y, GLint z, GLint w)
...@@ -5555,6 +5566,7 @@ void Context::linkProgram(GLuint program) ...@@ -5555,6 +5566,7 @@ void Context::linkProgram(GLuint program)
handleError(programObject->link(this)); handleError(programObject->link(this));
mGLState.onProgramExecutableChange(programObject); mGLState.onProgramExecutableChange(programObject);
mStateCache.updateActiveAttribsMask(this); mStateCache.updateActiveAttribsMask(this);
mStateCache.updateVertexElementLimits(this);
} }
void Context::releaseShaderCompiler() void Context::releaseShaderCompiler()
...@@ -5763,6 +5775,7 @@ void Context::programBinary(GLuint program, GLenum binaryFormat, const void *bin ...@@ -5763,6 +5775,7 @@ void Context::programBinary(GLuint program, GLenum binaryFormat, const void *bin
handleError(programObject->loadBinary(this, binaryFormat, binary, length)); handleError(programObject->loadBinary(this, binaryFormat, binary, length));
mStateCache.updateActiveAttribsMask(this); mStateCache.updateActiveAttribsMask(this);
mStateCache.updateVertexElementLimits(this);
} }
void Context::uniform1ui(GLint location, GLuint v0) void Context::uniform1ui(GLint location, GLuint v0)
...@@ -7567,6 +7580,7 @@ void Context::onSubjectStateChange(const Context *context, ...@@ -7567,6 +7580,7 @@ void Context::onSubjectStateChange(const Context *context,
{ {
case kVertexArraySubjectIndex: case kVertexArraySubjectIndex:
mGLState.setObjectDirty(GL_VERTEX_ARRAY); mGLState.setObjectDirty(GL_VERTEX_ARRAY);
mStateCache.updateVertexElementLimits(this);
break; break;
case kReadFramebufferSubjectIndex: case kReadFramebufferSubjectIndex:
...@@ -7629,7 +7643,10 @@ GLenum ErrorSet::popError() ...@@ -7629,7 +7643,10 @@ GLenum ErrorSet::popError()
} }
// StateCache implementation. // StateCache implementation.
StateCache::StateCache() : mCachedHasAnyEnabledClientAttrib(false) StateCache::StateCache()
: mCachedHasAnyEnabledClientAttrib(false),
mCachedNonInstancedVertexElementLimit(0),
mCachedInstancedVertexElementLimit(0)
{ {
} }
...@@ -7662,4 +7679,44 @@ void StateCache::updateActiveAttribsMask(Context *context) ...@@ -7662,4 +7679,44 @@ void StateCache::updateActiveAttribsMask(Context *context)
mCachedActiveBufferedAttribsMask = activeAttribs & ~clientAttribs; mCachedActiveBufferedAttribsMask = activeAttribs & ~clientAttribs;
mCachedHasAnyEnabledClientAttrib = (clientAttribs & enabledAttribs).any(); mCachedHasAnyEnabledClientAttrib = (clientAttribs & enabledAttribs).any();
} }
void StateCache::updateVertexElementLimits(Context *context)
{
const VertexArray *vao = context->getGLState().getVertexArray();
mCachedNonInstancedVertexElementLimit = std::numeric_limits<GLint64>::max();
mCachedInstancedVertexElementLimit = std::numeric_limits<GLint64>::max();
// VAO can be null on Context startup. If we make this computation lazier we could ASSERT.
// If there are no buffered attributes then we should not limit the draw call count.
if (!vao || !mCachedActiveBufferedAttribsMask.any())
{
return;
}
const auto &vertexAttribs = vao->getVertexAttributes();
const auto &vertexBindings = vao->getVertexBindings();
for (size_t attributeIndex : mCachedActiveBufferedAttribsMask)
{
const VertexAttribute &attrib = vertexAttribs[attributeIndex];
ASSERT(attrib.enabled);
const VertexBinding &binding = vertexBindings[attrib.bindingIndex];
ASSERT(context->isGLES1() ||
context->getGLState().getProgram()->isAttribLocationActive(attributeIndex));
GLint64 limit = attrib.getCachedElementLimit();
if (binding.getDivisor() > 0)
{
mCachedInstancedVertexElementLimit =
std::min(mCachedInstancedVertexElementLimit, limit);
}
else
{
mCachedNonInstancedVertexElementLimit =
std::min(mCachedNonInstancedVertexElementLimit, limit);
}
}
}
} // namespace gl } // namespace gl
...@@ -98,13 +98,27 @@ class StateCache final : angle::NonCopyable ...@@ -98,13 +98,27 @@ class StateCache final : angle::NonCopyable
AttributesMask getActiveClientAttribsMask() const { return mCachedActiveClientAttribsMask; } AttributesMask getActiveClientAttribsMask() const { return mCachedActiveClientAttribsMask; }
bool hasAnyEnabledClientAttrib() const { return mCachedHasAnyEnabledClientAttrib; } bool hasAnyEnabledClientAttrib() const { return mCachedHasAnyEnabledClientAttrib; }
// Places that can trigger updateVertexElementLimits:
// 1. Context: bindVertexArray.
// 2. Context: any executable change (linkProgram/useProgram/programBinary).
// 3. Vertex Array: any state change call.
// 4. Buffer: a dependent buffer resize.
GLint64 getNonInstancedVertexElementLimit() const
{
return mCachedNonInstancedVertexElementLimit;
}
GLint64 getInstancedVertexElementLimit() const { return mCachedInstancedVertexElementLimit; }
// Cache update functions. // Cache update functions.
void updateActiveAttribsMask(Context *context); void updateActiveAttribsMask(Context *context);
void updateVertexElementLimits(Context *context);
private: private:
AttributesMask mCachedActiveBufferedAttribsMask; AttributesMask mCachedActiveBufferedAttribsMask;
AttributesMask mCachedActiveClientAttribsMask; AttributesMask mCachedActiveClientAttribsMask;
bool mCachedHasAnyEnabledClientAttrib; bool mCachedHasAnyEnabledClientAttrib;
GLint64 mCachedNonInstancedVertexElementLimit;
GLint64 mCachedInstancedVertexElementLimit;
}; };
class Context final : public egl::LabeledObject, angle::NonCopyable, public angle::ObserverInterface class Context final : public egl::LabeledObject, angle::NonCopyable, public angle::ObserverInterface
......
...@@ -410,11 +410,11 @@ void VertexArray::onSubjectStateChange(const gl::Context *context, ...@@ -410,11 +410,11 @@ void VertexArray::onSubjectStateChange(const gl::Context *context,
break; break;
case angle::SubjectMessage::STORAGE_CHANGED: case angle::SubjectMessage::STORAGE_CHANGED:
setDependentDirtyBit(context, false, index);
if (index < mArrayBufferObserverBindings.size()) if (index < mArrayBufferObserverBindings.size())
{ {
updateCachedBufferBindingSize(&mState.mVertexBindings[index]); updateCachedBufferBindingSize(&mState.mVertexBindings[index]);
} }
setDependentDirtyBit(context, false, index);
break; break;
case angle::SubjectMessage::BINDING_CHANGED: case angle::SubjectMessage::BINDING_CHANGED:
......
...@@ -109,9 +109,6 @@ bool ValidateDrawClientAttribs(Context *context) ...@@ -109,9 +109,6 @@ bool ValidateDrawClientAttribs(Context *context)
bool ValidateDrawAttribs(Context *context, GLint primcount, GLint maxVertex, GLint vertexCount) bool ValidateDrawAttribs(Context *context, GLint primcount, GLint maxVertex, GLint vertexCount)
{ {
const gl::State &state = context->getGLState();
const gl::Program *program = state.getProgram();
if (!ValidateDrawClientAttribs(context)) if (!ValidateDrawClientAttribs(context))
{ {
return false; return false;
...@@ -123,39 +120,24 @@ bool ValidateDrawAttribs(Context *context, GLint primcount, GLint maxVertex, GLi ...@@ -123,39 +120,24 @@ bool ValidateDrawAttribs(Context *context, GLint primcount, GLint maxVertex, GLi
return true; return true;
} }
const VertexArray *vao = state.getVertexArray(); if (maxVertex <= context->getStateCache().getNonInstancedVertexElementLimit() &&
const auto &vertexAttribs = vao->getVertexAttributes(); (primcount - 1) <= context->getStateCache().getInstancedVertexElementLimit())
const auto &vertexBindings = vao->getVertexBindings();
const AttributesMask &activeAttribs = context->getStateCache().getActiveBufferedAttribsMask();
for (size_t attributeIndex : activeAttribs)
{ {
const VertexAttribute &attrib = vertexAttribs[attributeIndex]; return true;
ASSERT(attrib.enabled); }
const VertexBinding &binding = vertexBindings[attrib.bindingIndex];
ASSERT(context->isGLES1() || program->isAttribLocationActive(attributeIndex));
GLint maxVertexElement = binding.getDivisor() != 0 ? (primcount - 1) : maxVertex;
if (maxVertexElement > attrib.getCachedElementLimit())
{
// 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: // An overflow can happen when adding the offset. Negative indicates overflow.
// We can return INVALID_OPERATION if our buffer does not have enough backing data. if (context->getStateCache().getNonInstancedVertexElementLimit() < 0 ||
ANGLE_VALIDATION_ERR(context, InvalidOperation(), InsufficientVertexBufferSize); context->getStateCache().getInstancedVertexElementLimit() < 0)
return false; {
} ANGLE_VALIDATION_ERR(context, InvalidOperation(), IntegerOverflow);
return false;
} }
return true; // [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;
} }
bool ValidReadPixelsTypeEnum(Context *context, GLenum type) bool ValidReadPixelsTypeEnum(Context *context, GLenum type)
......
...@@ -1590,32 +1590,32 @@ TEST_P(WebGLCompatibilityTest, DrawArraysBufferOutOfBoundsInstancedANGLE) ...@@ -1590,32 +1590,32 @@ TEST_P(WebGLCompatibilityTest, DrawArraysBufferOutOfBoundsInstancedANGLE)
// Test touching the last element is valid. // Test touching the last element is valid.
glVertexAttribPointer(wLocation, 1, GL_UNSIGNED_BYTE, GL_FALSE, 0, zeroOffset + 12); glVertexAttribPointer(wLocation, 1, GL_UNSIGNED_BYTE, GL_FALSE, 0, zeroOffset + 12);
glDrawArraysInstancedANGLE(GL_POINTS, 0, 1, 4); glDrawArraysInstancedANGLE(GL_POINTS, 0, 1, 4);
ASSERT_GL_NO_ERROR(); ASSERT_GL_NO_ERROR() << "touching the last element.";
// Test touching the last element + 1 is invalid. // Test touching the last element + 1 is invalid.
glVertexAttribPointer(wLocation, 1, GL_UNSIGNED_BYTE, GL_FALSE, 0, zeroOffset + 13); glVertexAttribPointer(wLocation, 1, GL_UNSIGNED_BYTE, GL_FALSE, 0, zeroOffset + 13);
glDrawArraysInstancedANGLE(GL_POINTS, 0, 1, 4); glDrawArraysInstancedANGLE(GL_POINTS, 0, 1, 4);
EXPECT_GL_ERROR(GL_INVALID_OPERATION); EXPECT_GL_ERROR(GL_INVALID_OPERATION) << "touching the last element + 1.";
// Test touching the last element is valid, using a stride. // Test touching the last element is valid, using a stride.
glVertexAttribPointer(wLocation, 1, GL_UNSIGNED_BYTE, GL_FALSE, 2, zeroOffset + 9); glVertexAttribPointer(wLocation, 1, GL_UNSIGNED_BYTE, GL_FALSE, 2, zeroOffset + 9);
glDrawArraysInstancedANGLE(GL_POINTS, 0, 1, 4); glDrawArraysInstancedANGLE(GL_POINTS, 0, 1, 4);
ASSERT_GL_NO_ERROR(); ASSERT_GL_NO_ERROR() << "touching the last element using a stride.";
// Test touching the last element + 1 is invalid, using a stride. // Test touching the last element + 1 is invalid, using a stride.
glVertexAttribPointer(wLocation, 1, GL_UNSIGNED_BYTE, GL_FALSE, 2, zeroOffset + 10); glVertexAttribPointer(wLocation, 1, GL_UNSIGNED_BYTE, GL_FALSE, 2, zeroOffset + 10);
glDrawArraysInstancedANGLE(GL_POINTS, 0, 1, 4); glDrawArraysInstancedANGLE(GL_POINTS, 0, 1, 4);
EXPECT_GL_ERROR(GL_INVALID_OPERATION); EXPECT_GL_ERROR(GL_INVALID_OPERATION) << "touching the last element + 1 using a stride.";
// Test any offset is valid if no vertices are drawn. // Test any offset is valid if no vertices are drawn.
glVertexAttribPointer(wLocation, 1, GL_UNSIGNED_BYTE, GL_FALSE, 0, zeroOffset + 32); glVertexAttribPointer(wLocation, 1, GL_UNSIGNED_BYTE, GL_FALSE, 0, zeroOffset + 32);
glDrawArraysInstancedANGLE(GL_POINTS, 0, 0, 1); glDrawArraysInstancedANGLE(GL_POINTS, 0, 0, 1);
ASSERT_GL_NO_ERROR(); ASSERT_GL_NO_ERROR() << "any offset with no vertices.";
// Test any offset is valid if no primitives are drawn. // Test any offset is valid if no primitives are drawn.
glVertexAttribPointer(wLocation, 1, GL_UNSIGNED_BYTE, GL_FALSE, 0, zeroOffset + 32); glVertexAttribPointer(wLocation, 1, GL_UNSIGNED_BYTE, GL_FALSE, 0, zeroOffset + 32);
glDrawArraysInstancedANGLE(GL_POINTS, 0, 1, 0); glDrawArraysInstancedANGLE(GL_POINTS, 0, 1, 0);
ASSERT_GL_NO_ERROR(); ASSERT_GL_NO_ERROR() << "any offset with primitives.";
} }
// Test the checks for OOB reads in the index buffer // Test the checks for OOB reads in the index buffer
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment