Commit c1346fba by Corentin Wallez Committed by Commit Bot

Revert "Remove IndexRange retrieving in validation"

This reverts commit 59d9da08. 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/633296Reviewed-by: 's avatarCorentin Wallez <cwallez@chromium.org> Commit-Queue: Corentin Wallez <cwallez@chromium.org>
parent e080387e
...@@ -410,7 +410,7 @@ gl::Error VertexDataManager::storeDynamicAttribs( ...@@ -410,7 +410,7 @@ gl::Error VertexDataManager::storeDynamicAttribs(
for (auto attribIndex : dynamicAttribsMask) for (auto attribIndex : dynamicAttribsMask)
{ {
const auto &dynamicAttrib = (*translatedAttribs)[attribIndex]; const auto &dynamicAttrib = (*translatedAttribs)[attribIndex];
ANGLE_TRY(reserveSpaceForAttrib(dynamicAttrib, start, count, instances)); ANGLE_TRY(reserveSpaceForAttrib(dynamicAttrib, count, instances));
} }
// Store dynamic attributes // Store dynamic attributes
...@@ -445,7 +445,6 @@ void VertexDataManager::PromoteDynamicAttribs( ...@@ -445,7 +445,6 @@ void VertexDataManager::PromoteDynamicAttribs(
} }
gl::Error VertexDataManager::reserveSpaceForAttrib(const TranslatedAttribute &translatedAttrib, gl::Error VertexDataManager::reserveSpaceForAttrib(const TranslatedAttribute &translatedAttrib,
GLint start,
GLsizei count, GLsizei count,
GLsizei instances) const GLsizei instances) const
{ {
...@@ -461,21 +460,9 @@ gl::Error VertexDataManager::reserveSpaceForAttrib(const TranslatedAttribute &tr ...@@ -461,21 +460,9 @@ gl::Error VertexDataManager::reserveSpaceForAttrib(const TranslatedAttribute &tr
size_t totalCount = gl::ComputeVertexBindingElementCount( size_t totalCount = gl::ComputeVertexBindingElementCount(
binding.getDivisor(), static_cast<size_t>(count), static_cast<size_t>(instances)); binding.getDivisor(), static_cast<size_t>(count), static_cast<size_t>(instances));
if (bufferD3D) ASSERT(!bufferD3D ||
{ ElementsInBuffer(attrib, binding, static_cast<unsigned int>(bufferD3D->getSize())) >=
// Vertices do not apply the 'start' offset when the divisor is non-zero even when doing static_cast<int>(totalCount));
// 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.";
}
}
return mStreamingBuffer->reserveVertexSpace(attrib, binding, static_cast<GLsizei>(totalCount), return mStreamingBuffer->reserveVertexSpace(attrib, binding, static_cast<GLsizei>(totalCount),
instances); instances);
......
...@@ -127,7 +127,6 @@ class VertexDataManager : angle::NonCopyable ...@@ -127,7 +127,6 @@ class VertexDataManager : angle::NonCopyable
}; };
gl::Error reserveSpaceForAttrib(const TranslatedAttribute &translatedAttrib, gl::Error reserveSpaceForAttrib(const TranslatedAttribute &translatedAttrib,
GLint start,
GLsizei count, GLsizei count,
GLsizei instances) const; GLsizei instances) const;
......
...@@ -3171,13 +3171,34 @@ bool ValidateDrawElementsCommon(ValidationContext *context, ...@@ -3171,13 +3171,34 @@ bool ValidateDrawElementsCommon(ValidationContext *context,
} }
} }
// Here we use maxVertex = 0 and vertexCount = 1 to avoid retrieving IndexRange. // Use the parameter buffer to retrieve and cache the index range.
if (!ValidateDrawAttribs(context, primcount, 0, 1)) // 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 &params = context->getParams<HasIndexRange>();
const auto &indexRangeOpt = params.getIndexRange();
if (!indexRangeOpt.valid())
{ {
// Unexpected error.
return false; return false;
} }
return true; // 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;
}
// 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, bool ValidateDrawElementsInstancedCommon(ValidationContext *context,
......
...@@ -782,6 +782,21 @@ TEST_P(VertexAttributeTest, DrawArraysBufferTooSmall) ...@@ -782,6 +782,21 @@ TEST_P(VertexAttributeTest, DrawArraysBufferTooSmall)
EXPECT_GL_ERROR(GL_INVALID_OPERATION); 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. // Verify that using a different start vertex doesn't mess up the draw.
TEST_P(VertexAttributeTest, DrawArraysWithBufferOffset) TEST_P(VertexAttributeTest, DrawArraysWithBufferOffset)
{ {
......
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