Commit 9fdaa497 by Jamie Madill Committed by Commit Bot

Don't no-op draw calls for zero count in validation.

Make the no-op happen in the Context, so we can properly generator more errors for negative WebGL tests. Some validation tests still need to check for no-ops, such as range checks. BUG=angleproject:2050 Change-Id: I48d0b3cf640f7f128679600e5df108146cfd6348 Reviewed-on: https://chromium-review.googlesource.com/522869 Commit-Queue: Jamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarCorentin Wallez <cwallez@chromium.org>
parent 0cbfa586
...@@ -1781,6 +1781,12 @@ void Context::texParameteriv(GLenum target, GLenum pname, const GLint *params) ...@@ -1781,6 +1781,12 @@ void Context::texParameteriv(GLenum target, GLenum pname, const GLint *params)
void Context::drawArrays(GLenum mode, GLint first, GLsizei count) void Context::drawArrays(GLenum mode, GLint first, GLsizei count)
{ {
// No-op if zero count
if (count == 0)
{
return;
}
ANGLE_CONTEXT_TRY(prepareForDraw()); ANGLE_CONTEXT_TRY(prepareForDraw());
ANGLE_CONTEXT_TRY(mImplementation->drawArrays(this, mode, first, count)); ANGLE_CONTEXT_TRY(mImplementation->drawArrays(this, mode, first, count));
MarkTransformFeedbackBufferUsage(mGLState.getCurrentTransformFeedback()); MarkTransformFeedbackBufferUsage(mGLState.getCurrentTransformFeedback());
...@@ -1788,6 +1794,12 @@ void Context::drawArrays(GLenum mode, GLint first, GLsizei count) ...@@ -1788,6 +1794,12 @@ void Context::drawArrays(GLenum mode, GLint first, GLsizei count)
void Context::drawArraysInstanced(GLenum mode, GLint first, GLsizei count, GLsizei instanceCount) void Context::drawArraysInstanced(GLenum mode, GLint first, GLsizei count, GLsizei instanceCount)
{ {
// No-op if zero count
if (count == 0 || instanceCount == 0)
{
return;
}
ANGLE_CONTEXT_TRY(prepareForDraw()); ANGLE_CONTEXT_TRY(prepareForDraw());
ANGLE_CONTEXT_TRY( ANGLE_CONTEXT_TRY(
mImplementation->drawArraysInstanced(this, mode, first, count, instanceCount)); mImplementation->drawArraysInstanced(this, mode, first, count, instanceCount));
...@@ -1796,6 +1808,12 @@ void Context::drawArraysInstanced(GLenum mode, GLint first, GLsizei count, GLsiz ...@@ -1796,6 +1808,12 @@ void Context::drawArraysInstanced(GLenum mode, GLint first, GLsizei count, GLsiz
void Context::drawElements(GLenum mode, GLsizei count, GLenum type, const void *indices) void Context::drawElements(GLenum mode, GLsizei count, GLenum type, const void *indices)
{ {
// No-op if zero count
if (count == 0)
{
return;
}
ANGLE_CONTEXT_TRY(prepareForDraw()); ANGLE_CONTEXT_TRY(prepareForDraw());
ANGLE_CONTEXT_TRY(mImplementation->drawElements(this, mode, count, type, indices)); ANGLE_CONTEXT_TRY(mImplementation->drawElements(this, mode, count, type, indices));
} }
...@@ -1806,6 +1824,12 @@ void Context::drawElementsInstanced(GLenum mode, ...@@ -1806,6 +1824,12 @@ void Context::drawElementsInstanced(GLenum mode,
const void *indices, const void *indices,
GLsizei instances) GLsizei instances)
{ {
// No-op if zero count
if (count == 0 || instances == 0)
{
return;
}
ANGLE_CONTEXT_TRY(prepareForDraw()); ANGLE_CONTEXT_TRY(prepareForDraw());
ANGLE_CONTEXT_TRY( ANGLE_CONTEXT_TRY(
mImplementation->drawElementsInstanced(this, mode, count, type, indices, instances)); mImplementation->drawElementsInstanced(this, mode, count, type, indices, instances));
...@@ -1818,6 +1842,12 @@ void Context::drawRangeElements(GLenum mode, ...@@ -1818,6 +1842,12 @@ void Context::drawRangeElements(GLenum mode,
GLenum type, GLenum type,
const void *indices) const void *indices)
{ {
// No-op if zero count
if (count == 0)
{
return;
}
ANGLE_CONTEXT_TRY(prepareForDraw()); ANGLE_CONTEXT_TRY(prepareForDraw());
ANGLE_CONTEXT_TRY( ANGLE_CONTEXT_TRY(
mImplementation->drawRangeElements(this, mode, start, end, count, type, indices)); mImplementation->drawRangeElements(this, mode, start, end, count, type, indices));
......
...@@ -619,8 +619,7 @@ bool ValidateDrawElementsInstancedBase(ValidationContext *context, ...@@ -619,8 +619,7 @@ bool ValidateDrawElementsInstancedBase(ValidationContext *context,
return false; return false;
} }
// No-op zero primitive count return true;
return (primcount > 0);
} }
bool ValidateDrawArraysInstancedBase(Context *context, bool ValidateDrawArraysInstancedBase(Context *context,
...@@ -640,8 +639,7 @@ bool ValidateDrawArraysInstancedBase(Context *context, ...@@ -640,8 +639,7 @@ bool ValidateDrawArraysInstancedBase(Context *context,
return false; return false;
} }
// No-op if zero primitive count return true;
return (primcount > 0);
} }
bool ValidateDrawInstancedANGLE(ValidationContext *context) bool ValidateDrawInstancedANGLE(ValidationContext *context)
...@@ -2654,8 +2652,7 @@ bool ValidateDrawBase(ValidationContext *context, GLenum mode, GLsizei count) ...@@ -2654,8 +2652,7 @@ bool ValidateDrawBase(ValidationContext *context, GLenum mode, GLsizei count)
} }
} }
// No-op if zero count return true;
return (count > 0);
} }
bool ValidateDrawArraysCommon(ValidationContext *context, bool ValidateDrawArraysCommon(ValidationContext *context,
...@@ -2689,20 +2686,23 @@ bool ValidateDrawArraysCommon(ValidationContext *context, ...@@ -2689,20 +2686,23 @@ bool ValidateDrawArraysCommon(ValidationContext *context,
} }
// Check the computation of maxVertex doesn't overflow. // Check the computation of maxVertex doesn't overflow.
// - first < 0 or count < 0 have been checked as an error condition // - first < 0 has been checked as an error condition.
// - count > 0 has been checked in ValidateDrawBase as it makes the call a noop // - if count < 0, skip validating no-op draw calls.
// From this we know maxVertex will be positive, and only need to check if it overflows GLint. // From this we know maxVertex will be positive, and only need to check if it overflows GLint.
ASSERT(count > 0 && first >= 0); ASSERT(first >= 0);
int64_t maxVertex = static_cast<int64_t>(first) + static_cast<int64_t>(count) - 1; if (count > 0)
if (maxVertex > static_cast<int64_t>(std::numeric_limits<GLint>::max()))
{ {
ANGLE_VALIDATION_ERR(context, InvalidOperation(), IntegerOverflow); int64_t maxVertex = static_cast<int64_t>(first) + static_cast<int64_t>(count) - 1;
return false; if (maxVertex > static_cast<int64_t>(std::numeric_limits<GLint>::max()))
} {
ANGLE_VALIDATION_ERR(context, InvalidOperation(), IntegerOverflow);
return false;
}
if (!ValidateDrawAttribs(context, primcount, static_cast<GLint>(maxVertex), count)) if (!ValidateDrawAttribs(context, primcount, static_cast<GLint>(maxVertex), count))
{ {
return false; return false;
}
} }
return true; return true;
...@@ -2834,52 +2834,50 @@ bool ValidateDrawElementsCommon(ValidationContext *context, ...@@ -2834,52 +2834,50 @@ bool ValidateDrawElementsCommon(ValidationContext *context,
} }
} }
if (count > 0) if (count > 0 && !elementArrayBuffer && !indices)
{ {
if (elementArrayBuffer) // This is an application error that would normally result in a crash, but we catch it and
{ // return an error
// The max possible type size is 8 and count is on 32 bits so doing the multiplication context->handleError(InvalidOperation() << "No element array buffer and no pointer.");
// in a 64 bit integer is safe. Also we are guaranteed that here count > 0. return false;
static_assert(std::is_same<int, GLsizei>::value, "GLsizei isn't the expected type"); }
constexpr uint64_t kMaxTypeSize = 8;
constexpr uint64_t kIntMax = std::numeric_limits<int>::max();
constexpr uint64_t kUint64Max = std::numeric_limits<uint64_t>::max();
static_assert(kIntMax < kUint64Max / kMaxTypeSize, "");
uint64_t typeSize = typeBytes;
uint64_t elementCount = static_cast<uint64_t>(count);
ASSERT(elementCount > 0 && typeSize <= kMaxTypeSize);
// Doing the multiplication here is overflow-safe
uint64_t elementDataSizeNoOffset = typeSize * elementCount;
// The offset can be any value, check for overflows
uint64_t offset = static_cast<uint64_t>(reinterpret_cast<uintptr_t>(indices));
if (elementDataSizeNoOffset > kUint64Max - offset)
{
ANGLE_VALIDATION_ERR(context, InvalidOperation(), IntegerOverflow);
return false;
}
uint64_t elementDataSizeWithOffset = elementDataSizeNoOffset + offset; if (count > 0 && elementArrayBuffer)
if (elementDataSizeWithOffset > static_cast<uint64_t>(elementArrayBuffer->getSize())) {
{ // The max possible type size is 8 and count is on 32 bits so doing the multiplication
ANGLE_VALIDATION_ERR(context, InvalidOperation(), InsufficientBufferSize); // in a 64 bit integer is safe. Also we are guaranteed that here count > 0.
return false; static_assert(std::is_same<int, GLsizei>::value, "GLsizei isn't the expected type");
} constexpr uint64_t kMaxTypeSize = 8;
constexpr uint64_t kIntMax = std::numeric_limits<int>::max();
constexpr uint64_t kUint64Max = std::numeric_limits<uint64_t>::max();
static_assert(kIntMax < kUint64Max / kMaxTypeSize, "");
ASSERT(isPow2(typeSize) && typeSize > 0); uint64_t typeSize = typeBytes;
if ((elementArrayBuffer->getSize() & (typeSize - 1)) != 0) uint64_t elementCount = static_cast<uint64_t>(count);
{ ASSERT(elementCount > 0 && typeSize <= kMaxTypeSize);
ANGLE_VALIDATION_ERR(context, InvalidOperation(), MismatchedByteCountType);
return false; // Doing the multiplication here is overflow-safe
} uint64_t elementDataSizeNoOffset = typeSize * elementCount;
// The offset can be any value, check for overflows
uint64_t offset = static_cast<uint64_t>(reinterpret_cast<uintptr_t>(indices));
if (elementDataSizeNoOffset > kUint64Max - offset)
{
ANGLE_VALIDATION_ERR(context, InvalidOperation(), IntegerOverflow);
return false;
} }
else if (!indices)
uint64_t elementDataSizeWithOffset = elementDataSizeNoOffset + offset;
if (elementDataSizeWithOffset > static_cast<uint64_t>(elementArrayBuffer->getSize()))
{
ANGLE_VALIDATION_ERR(context, InvalidOperation(), InsufficientBufferSize);
return false;
}
ASSERT(isPow2(typeSize) && typeSize > 0);
if ((elementArrayBuffer->getSize() & (typeSize - 1)) != 0)
{ {
// This is an application error that would normally result in a crash, ANGLE_VALIDATION_ERR(context, InvalidOperation(), MismatchedByteCountType);
// but we catch it and return an error
context->handleError(InvalidOperation() << "No element array buffer and no pointer.");
return false; return false;
} }
} }
...@@ -2893,6 +2891,15 @@ bool ValidateDrawElementsCommon(ValidationContext *context, ...@@ -2893,6 +2891,15 @@ bool ValidateDrawElementsCommon(ValidationContext *context,
return false; return false;
} }
} }
else if (count == 0)
{
// ValidateDrawAttribs also does some extra validation that is independent of the vertex
// count.
if (!ValidateDrawAttribs(context, 0, 0, 0))
{
return false;
}
}
else else
{ {
// Use the parameter buffer to retrieve and cache the index range. // Use the parameter buffer to retrieve and cache the index range.
......
...@@ -1276,6 +1276,12 @@ bool ValidateDrawRangeElements(Context *context, ...@@ -1276,6 +1276,12 @@ bool ValidateDrawRangeElements(Context *context,
return false; return false;
} }
// Skip range checks for no-op calls.
if (count <= 0)
{
return true;
}
// Use the parameter buffer to retrieve and cache the index range. // Use the parameter buffer to retrieve and cache the index range.
const auto &params = context->getParams<HasIndexRange>(); const auto &params = context->getParams<HasIndexRange>();
const auto &indexRangeOpt = params.getIndexRange(); const auto &indexRangeOpt = params.getIndexRange();
......
...@@ -98,9 +98,12 @@ TEST_P(DrawElementsTest, ClientSideNullptrArrayZeroCount) ...@@ -98,9 +98,12 @@ TEST_P(DrawElementsTest, ClientSideNullptrArrayZeroCount)
glEnableVertexAttribArray(posLocation); glEnableVertexAttribArray(posLocation);
ASSERT_GL_NO_ERROR(); ASSERT_GL_NO_ERROR();
// "If drawElements is called with a count greater than zero, and no WebGLBuffer is bound to the
// ELEMENT_ARRAY_BUFFER binding point, an INVALID_OPERATION error is generated."
glDrawElements(GL_TRIANGLES, 1, GL_UNSIGNED_BYTE, nullptr); glDrawElements(GL_TRIANGLES, 1, GL_UNSIGNED_BYTE, nullptr);
EXPECT_GL_ERROR(GL_INVALID_OPERATION); EXPECT_GL_ERROR(GL_INVALID_OPERATION);
// count == 0 so it's fine to have no element array buffer bound.
glDrawElements(GL_TRIANGLES, 0, GL_UNSIGNED_BYTE, nullptr); glDrawElements(GL_TRIANGLES, 0, GL_UNSIGNED_BYTE, nullptr);
ASSERT_GL_NO_ERROR(); ASSERT_GL_NO_ERROR();
} }
......
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