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: Corentin Wallez <cwallez@chromium.org>
diff --git a/src/libANGLE/validationES.cpp b/src/libANGLE/validationES.cpp
index 1d948f8..45bf909 100644
--- a/src/libANGLE/validationES.cpp
+++ b/src/libANGLE/validationES.cpp
@@ -619,8 +619,7 @@
return false;
}
- // No-op zero primitive count
- return (primcount > 0);
+ return true;
}
bool ValidateDrawArraysInstancedBase(Context *context,
@@ -640,8 +639,7 @@
return false;
}
- // No-op if zero primitive count
- return (primcount > 0);
+ return true;
}
bool ValidateDrawInstancedANGLE(ValidationContext *context)
@@ -2654,8 +2652,7 @@
}
}
- // No-op if zero count
- return (count > 0);
+ return true;
}
bool ValidateDrawArraysCommon(ValidationContext *context,
@@ -2689,20 +2686,23 @@
}
// Check the computation of maxVertex doesn't overflow.
- // - first < 0 or count < 0 have been checked as an error condition
- // - count > 0 has been checked in ValidateDrawBase as it makes the call a noop
+ // - first < 0 has been checked as an error condition.
+ // - 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.
- ASSERT(count > 0 && first >= 0);
- int64_t maxVertex = static_cast<int64_t>(first) + static_cast<int64_t>(count) - 1;
- if (maxVertex > static_cast<int64_t>(std::numeric_limits<GLint>::max()))
+ ASSERT(first >= 0);
+ if (count > 0)
{
- ANGLE_VALIDATION_ERR(context, InvalidOperation(), IntegerOverflow);
- return false;
- }
+ int64_t maxVertex = static_cast<int64_t>(first) + static_cast<int64_t>(count) - 1;
+ 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))
- {
- return false;
+ if (!ValidateDrawAttribs(context, primcount, static_cast<GLint>(maxVertex), count))
+ {
+ return false;
+ }
}
return true;
@@ -2834,52 +2834,50 @@
}
}
- 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
+ context->handleError(InvalidOperation() << "No element array buffer and no pointer.");
+ return false;
+ }
+
+ if (count > 0 && elementArrayBuffer)
+ {
+ // The max possible type size is 8 and count is on 32 bits so doing the multiplication
+ // in a 64 bit integer is safe. Also we are guaranteed that here count > 0.
+ 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)
{
- // The max possible type size is 8 and count is on 32 bits so doing the multiplication
- // in a 64 bit integer is safe. Also we are guaranteed that here count > 0.
- 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 (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)
- {
- ANGLE_VALIDATION_ERR(context, InvalidOperation(), MismatchedByteCountType);
- return false;
- }
+ ANGLE_VALIDATION_ERR(context, InvalidOperation(), IntegerOverflow);
+ return false;
}
- else if (!indices)
+
+ uint64_t elementDataSizeWithOffset = elementDataSizeNoOffset + offset;
+ if (elementDataSizeWithOffset > static_cast<uint64_t>(elementArrayBuffer->getSize()))
{
- // This is an application error that would normally result in a crash,
- // but we catch it and return an error
- context->handleError(InvalidOperation() << "No element array buffer and no pointer.");
+ ANGLE_VALIDATION_ERR(context, InvalidOperation(), InsufficientBufferSize);
+ return false;
+ }
+
+ ASSERT(isPow2(typeSize) && typeSize > 0);
+ if ((elementArrayBuffer->getSize() & (typeSize - 1)) != 0)
+ {
+ ANGLE_VALIDATION_ERR(context, InvalidOperation(), MismatchedByteCountType);
return false;
}
}
@@ -2893,6 +2891,15 @@
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
{
// Use the parameter buffer to retrieve and cache the index range.