Commit bb062070 by Shahbaz Youssefi Committed by Commit Bot

Vulkan: Fix updates to element buffer

If glBufferSubData results in a new vk::BufferHelper allocation, VertexArrayVk::mCurrentElementArrayBuffer needs to be updated. VertexArrayVk::syncState was working under the assumption that DIRTY_BIT_ELEMENT_ARRAY_BUFFER_DATA cannot result in a vk::BufferHelper pointer change. This assumption was broken in https://chromium-review.googlesource.com/c/angle/angle/+/2204655. Bug: b/178231226 Change-Id: I969549c5ffec3456bdc08ac3e03a0fa0e7b4593f Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2685439Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
parent c3887991
...@@ -445,10 +445,13 @@ angle::Result VertexArrayVk::syncState(const gl::Context *context, ...@@ -445,10 +445,13 @@ angle::Result VertexArrayVk::syncState(const gl::Context *context,
switch (dirtyBit) switch (dirtyBit)
{ {
case gl::VertexArray::DIRTY_BIT_ELEMENT_ARRAY_BUFFER: case gl::VertexArray::DIRTY_BIT_ELEMENT_ARRAY_BUFFER:
case gl::VertexArray::DIRTY_BIT_ELEMENT_ARRAY_BUFFER_DATA:
{ {
gl::Buffer *bufferGL = mState.getElementArrayBuffer(); gl::Buffer *bufferGL = mState.getElementArrayBuffer();
if (bufferGL) if (bufferGL)
{ {
// Note that just updating buffer data may still result in a new
// vk::BufferHelper allocation.
BufferVk *bufferVk = vk::GetImpl(bufferGL); BufferVk *bufferVk = vk::GetImpl(bufferGL);
mCurrentElementArrayBuffer = &bufferVk->getBuffer(); mCurrentElementArrayBuffer = &bufferVk->getBuffer();
} }
...@@ -464,13 +467,6 @@ angle::Result VertexArrayVk::syncState(const gl::Context *context, ...@@ -464,13 +467,6 @@ angle::Result VertexArrayVk::syncState(const gl::Context *context,
break; break;
} }
case gl::VertexArray::DIRTY_BIT_ELEMENT_ARRAY_BUFFER_DATA:
mLineLoopBufferFirstIndex.reset();
mLineLoopBufferLastIndex.reset();
ANGLE_TRY(contextVk->onIndexBufferChange(mCurrentElementArrayBuffer));
mDirtyLineLoopTranslation = true;
break;
#define ANGLE_VERTEX_DIRTY_ATTRIB_FUNC(INDEX) \ #define ANGLE_VERTEX_DIRTY_ATTRIB_FUNC(INDEX) \
case gl::VertexArray::DIRTY_BIT_ATTRIB_0 + INDEX: \ case gl::VertexArray::DIRTY_BIT_ATTRIB_0 + INDEX: \
{ \ { \
......
...@@ -1799,10 +1799,80 @@ TEST_P(SimpleStateChangeTest, DrawElementsThenDrawElementsNewIndices) ...@@ -1799,10 +1799,80 @@ TEST_P(SimpleStateChangeTest, DrawElementsThenDrawElementsNewIndices)
// We expect to draw the triangle with the last three points on the bottom right, and // We expect to draw the triangle with the last three points on the bottom right, and
// rebind the same element buffer and draw with the same indices. // rebind the same element buffer and draw with the same indices.
auto vertices = std::vector<Vector3>{ std::vector<Vector3> vertices = {
{-1.0f, 1.0f, 0.0f}, {1.0f, 1.0f, 0.0f}, {1.0f, -1.0f, 0.0f}, {-1.0f, -1.0f, 0.0f}};
std::vector<GLubyte> indices8 = {0, 1, 2, 2, 3, 0};
GLint positionLocation = glGetAttribLocation(program, essl1_shaders::PositionAttrib());
ASSERT_NE(-1, positionLocation);
GLint colorUniformLocation =
glGetUniformLocation(program, angle::essl1_shaders::ColorUniform());
ASSERT_NE(colorUniformLocation, -1);
glUniform4f(colorUniformLocation, 1.0f, 1.0f, 1.0f, 1.0f);
GLBuffer indexBuffer8;
glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, indexBuffer8);
glBufferData(GL_ELEMENT_ARRAY_BUFFER, indices8.size() * sizeof(GLubyte), &indices8[0],
GL_DYNAMIC_DRAW);
GLBuffer vertexBuffer;
glBindBuffer(GL_ARRAY_BUFFER, vertexBuffer);
glBufferData(GL_ARRAY_BUFFER, sizeof(vertices[0]) * vertices.size(), vertices.data(),
GL_STATIC_DRAW);
glVertexAttribPointer(positionLocation, 3, GL_FLOAT, GL_FALSE, 0, nullptr);
glEnableVertexAttribArray(positionLocation);
glDrawElements(GL_TRIANGLES, 3, GL_UNSIGNED_BYTE, (void *)(0 * sizeof(GLubyte)));
std::vector<GLubyte> newIndices8 = {2, 3, 0};
glBufferSubData(GL_ELEMENT_ARRAY_BUFFER, 0, newIndices8.size() * sizeof(GLubyte),
&newIndices8[0]);
glUniform4f(colorUniformLocation, 0.0f, 0.0f, 1.0f, 1.0f);
// Draw the triangle again with the same offset.
glDrawElements(GL_TRIANGLES, 3, GL_UNSIGNED_BYTE, (void *)(0 * sizeof(GLubyte)));
glDisableVertexAttribArray(positionLocation);
ASSERT_GL_NO_ERROR();
int quarterWidth = getWindowWidth() / 4;
int quarterHeight = getWindowHeight() / 4;
// Validate the triangle is drawn on the bottom left.
EXPECT_PIXEL_COLOR_EQ(quarterWidth * 2, quarterHeight, GLColor::blue);
EXPECT_PIXEL_COLOR_EQ(quarterWidth, quarterHeight * 2, GLColor::blue);
// Validate the triangle is NOT on the top right part.
EXPECT_PIXEL_COLOR_EQ(quarterWidth * 2, quarterHeight * 3, GLColor::white);
}
// Draw a triangle with drawElements then change the indices and draw again. Similar to
// DrawElementsThenDrawElementsNewIndices, but changes the whole index buffer (not just half). This
// triggers a different path in the Vulkan backend based on the fact that the majority of the buffer
// is being updated.
TEST_P(SimpleStateChangeTest, DrawElementsThenDrawElementsWholeNewIndices)
{
ANGLE_GL_PROGRAM(program, essl1_shaders::vs::Simple(), essl1_shaders::fs::UniformColor());
glUseProgram(program);
// Background Red color
glClearColor(1.0f, 0.0f, 0.0f, 1.0f);
glClear(GL_COLOR_BUFFER_BIT);
// We expect to draw the triangle with the last three points on the bottom right, and
// rebind the same element buffer and draw with the same indices.
std::vector<Vector3> vertices = {
{-1.0f, 1.0f, 0.0f}, {1.0f, 1.0f, 0.0f}, {1.0f, -1.0f, 0.0f}, {-1.0f, -1.0f, 0.0f}}; {-1.0f, 1.0f, 0.0f}, {1.0f, 1.0f, 0.0f}, {1.0f, -1.0f, 0.0f}, {-1.0f, -1.0f, 0.0f}};
auto indices8 = std::vector<GLubyte>{0, 1, 2, 2, 3, 0}; std::vector<GLubyte> indices8 = {0, 1, 2, 2, 3, 0};
GLint positionLocation = glGetAttribLocation(program, essl1_shaders::PositionAttrib()); GLint positionLocation = glGetAttribLocation(program, essl1_shaders::PositionAttrib());
ASSERT_NE(-1, positionLocation); ASSERT_NE(-1, positionLocation);
...@@ -1828,7 +1898,7 @@ TEST_P(SimpleStateChangeTest, DrawElementsThenDrawElementsNewIndices) ...@@ -1828,7 +1898,7 @@ TEST_P(SimpleStateChangeTest, DrawElementsThenDrawElementsNewIndices)
glDrawElements(GL_TRIANGLES, 3, GL_UNSIGNED_BYTE, (void *)(0 * sizeof(GLubyte))); glDrawElements(GL_TRIANGLES, 3, GL_UNSIGNED_BYTE, (void *)(0 * sizeof(GLubyte)));
auto newIndices8 = std::vector<GLubyte>{2, 3, 0}; std::vector<GLubyte> newIndices8 = {2, 3, 0, 0, 0, 0};
glBufferSubData(GL_ELEMENT_ARRAY_BUFFER, 0, newIndices8.size() * sizeof(GLubyte), glBufferSubData(GL_ELEMENT_ARRAY_BUFFER, 0, newIndices8.size() * sizeof(GLubyte),
&newIndices8[0]); &newIndices8[0]);
......
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