Revert "Remove IndexRange retrieving in validation"
This reverts commit 59d9da089580afac175ff5f1a932b987c9d194d6.
Reason for revert: <INSERT REASONING HERE>
Original change's description:
> Remove IndexRange retrieving in validation
>
> This change can improve the performance of drawElements which uses
> the path without translation.
> Paste a set of mean data (repeated 30) for reference on Intel skylake
> Win10 desktop.
> DrawElementsPerfBenchmark.Run/d3d11:
> before after
> mean: 13644.4666667 -> mean: 13887.8333333
> DrawElementsPerfBenchmark.Run/d3d11_index_buffer_changed:
> before after
> mean: 45.8 -> mean: 46.3666666667
>
> BUG=755897, angleproject:1393
>
> Change-Id: I11f5db25445346958dfef52b1d23df5483cda32f
> Reviewed-on: https://chromium-review.googlesource.com/607413
> Reviewed-by: Geoff Lang <geofflang@chromium.org>
> Reviewed-by: Jamie Madill <jmadill@chromium.org>
> Commit-Queue: Jamie Madill <jmadill@chromium.org>
TBR=geofflang@chromium.org,jmadill@chromium.org,jiajia.qin@intel.com
Change-Id: I4b00af2c32af36aa978ac2fddcf7514134497cf3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 755897, angleproject:1393
Reviewed-on: https://chromium-review.googlesource.com/633296
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
diff --git a/src/libANGLE/renderer/d3d/VertexDataManager.cpp b/src/libANGLE/renderer/d3d/VertexDataManager.cpp
index 578283b..f5c6cf0 100644
--- a/src/libANGLE/renderer/d3d/VertexDataManager.cpp
+++ b/src/libANGLE/renderer/d3d/VertexDataManager.cpp
@@ -410,7 +410,7 @@
for (auto attribIndex : dynamicAttribsMask)
{
const auto &dynamicAttrib = (*translatedAttribs)[attribIndex];
- ANGLE_TRY(reserveSpaceForAttrib(dynamicAttrib, start, count, instances));
+ ANGLE_TRY(reserveSpaceForAttrib(dynamicAttrib, count, instances));
}
// Store dynamic attributes
@@ -445,7 +445,6 @@
}
gl::Error VertexDataManager::reserveSpaceForAttrib(const TranslatedAttribute &translatedAttrib,
- GLint start,
GLsizei count,
GLsizei instances) const
{
@@ -461,21 +460,9 @@
size_t totalCount = gl::ComputeVertexBindingElementCount(
binding.getDivisor(), static_cast<size_t>(count), static_cast<size_t>(instances));
- if (bufferD3D)
- {
- // Vertices do not apply the 'start' offset when the divisor is non-zero even when doing
- // a non-instanced draw call
- GLint firstVertexIndex = binding.getDivisor() > 0 ? 0 : start;
- int64_t maxVertexCount =
- static_cast<int64_t>(firstVertexIndex) + static_cast<int64_t>(totalCount);
- int elementsInBuffer =
- ElementsInBuffer(attrib, binding, static_cast<unsigned int>(bufferD3D->getSize()));
-
- if (maxVertexCount > elementsInBuffer)
- {
- return gl::InvalidOperation() << "Vertex buffer is not big enough for the draw call.";
- }
- }
+ ASSERT(!bufferD3D ||
+ ElementsInBuffer(attrib, binding, static_cast<unsigned int>(bufferD3D->getSize())) >=
+ static_cast<int>(totalCount));
return mStreamingBuffer->reserveVertexSpace(attrib, binding, static_cast<GLsizei>(totalCount),
instances);
diff --git a/src/libANGLE/renderer/d3d/VertexDataManager.h b/src/libANGLE/renderer/d3d/VertexDataManager.h
index 08c2397..cbdb283 100644
--- a/src/libANGLE/renderer/d3d/VertexDataManager.h
+++ b/src/libANGLE/renderer/d3d/VertexDataManager.h
@@ -127,7 +127,6 @@
};
gl::Error reserveSpaceForAttrib(const TranslatedAttribute &translatedAttrib,
- GLint start,
GLsizei count,
GLsizei instances) const;
diff --git a/src/libANGLE/validationES.cpp b/src/libANGLE/validationES.cpp
index a878aed..6b9b390 100644
--- a/src/libANGLE/validationES.cpp
+++ b/src/libANGLE/validationES.cpp
@@ -3171,13 +3171,34 @@
}
}
- // Here we use maxVertex = 0 and vertexCount = 1 to avoid retrieving IndexRange.
- if (!ValidateDrawAttribs(context, primcount, 0, 1))
+ // Use the parameter buffer to retrieve and cache the index range.
+ // TODO: offer fast path, with disabled index validation.
+ // TODO: also disable index checking on back-ends that are robust to out-of-range accesses.
+ const auto ¶ms = context->getParams<HasIndexRange>();
+ const auto &indexRangeOpt = params.getIndexRange();
+ if (!indexRangeOpt.valid())
+ {
+ // Unexpected error.
+ return false;
+ }
+
+ // If we use an index greater than our maximum supported index range, return an error.
+ // The ES3 spec does not specify behaviour here, it is undefined, but ANGLE should always
+ // return an error if possible here.
+ if (static_cast<GLuint64>(indexRangeOpt.value().end) >= context->getCaps().maxElementIndex)
+ {
+ ANGLE_VALIDATION_ERR(context, InvalidOperation(), ExceedsMaxElement);
+ return false;
+ }
+
+ if (!ValidateDrawAttribs(context, primcount, static_cast<GLint>(indexRangeOpt.value().end),
+ static_cast<GLint>(indexRangeOpt.value().vertexCount())))
{
return false;
}
- return true;
+ // No op if there are no real indices in the index data (all are primitive restart).
+ return (indexRangeOpt.value().vertexIndexCount > 0);
}
bool ValidateDrawElementsInstancedCommon(ValidationContext *context,
diff --git a/src/tests/gl_tests/VertexAttributeTest.cpp b/src/tests/gl_tests/VertexAttributeTest.cpp
index 243b92d..9d07579 100644
--- a/src/tests/gl_tests/VertexAttributeTest.cpp
+++ b/src/tests/gl_tests/VertexAttributeTest.cpp
@@ -782,6 +782,21 @@
EXPECT_GL_ERROR(GL_INVALID_OPERATION);
}
+// Verify that index draw with an out-of-range offset generates INVALID_OPERATION.
+TEST_P(VertexAttributeTest, DrawElementsBufferTooSmall)
+{
+ std::array<GLfloat, kVertexCount> inputData;
+ std::array<GLfloat, kVertexCount> expectedData;
+ InitTestData(inputData, expectedData);
+
+ TestData data(GL_FLOAT, GL_FALSE, Source::BUFFER, inputData.data(), expectedData.data());
+ data.bufferOffset = (kVertexCount - 3) * TypeStride(GL_FLOAT);
+
+ setupTest(data, 1);
+ drawIndexedQuad(mProgram, "position", 0.5f);
+ EXPECT_GL_ERROR(GL_INVALID_OPERATION);
+}
+
// Verify that using a different start vertex doesn't mess up the draw.
TEST_P(VertexAttributeTest, DrawArraysWithBufferOffset)
{