Vulkan: Acquire a new buffer even when size is unchanged

If a buffer is respecified using glBufferData with no changes to size
but client data pointer is null, we need to acquire a new BufferHelper
to avoid affecting the results of previously submitted draws.

Test: BufferDataTestES3.BufferDataWithNullFollowedByMap*Vulkan
Bug: angleproject:7211
Change-Id: Icc20fe3509f94098c7a15988a9ebc888b06fd3c8
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3588955
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Commit-Queue: mohan maiya <m.maiya@samsung.com>

5 files changed, 66 insertions, 15 deletions

diff --git a/src/libANGLE/Buffer.cpp b/src/libANGLE/Buffer.cpp
index e307d6450d..c756347527 100644
--- a/src/libANGLE/Buffer.cpp
+++ b/src/libANGLE/Buffer.cpp
@@ -391,15 +391,11 @@ angle::Result Buffer::getSubData(const gl::Context *context,
 
 void Buffer::onSubjectStateChange(angle::SubjectIndex index, angle::SubjectMessage message)
 {
-    if (message == angle::SubjectMessage::BufferVkStorageChanged)
-    {
-        return;
-    }
-
     // Pass it along!
     ASSERT(index == kImplementationSubjectIndex);
     ASSERT(message == angle::SubjectMessage::SubjectChanged ||
-           message == angle::SubjectMessage::InternalMemoryAllocationChanged);
+           message == angle::SubjectMessage::InternalMemoryAllocationChanged ||
+           message == angle::SubjectMessage::BufferVkStorageChanged);
     onStateChange(message);
 }
 
diff --git a/src/libANGLE/VertexArray.cpp b/src/libANGLE/VertexArray.cpp
index 1ba4bf310b..2ec19e82c5 100644
--- a/src/libANGLE/VertexArray.cpp
+++ b/src/libANGLE/VertexArray.cpp
@@ -714,10 +714,8 @@ void VertexArray::onSubjectStateChange(angle::SubjectIndex index, angle::Subject
             break;
 
         case angle::SubjectMessage::InternalMemoryAllocationChanged:
-            setDependentDirtyBit(false, index);
-            break;
-
         case angle::SubjectMessage::BufferVkStorageChanged:
+            setDependentDirtyBit(false, index);
             break;
 
         default:
diff --git a/src/libANGLE/renderer/vulkan/BufferVk.cpp b/src/libANGLE/renderer/vulkan/BufferVk.cpp
index 120db49aae..a2abbdcac1 100644
--- a/src/libANGLE/renderer/vulkan/BufferVk.cpp
+++ b/src/libANGLE/renderer/vulkan/BufferVk.cpp
@@ -365,11 +365,12 @@ angle::Result BufferVk::setDataWithMemoryType(const gl::Context *context,
         return angle::Result::Continue;
     }
 
-    const bool wholeSize = size == static_cast<size_t>(mState.getSize());
+    const bool bufferSizeChanged              = size != static_cast<size_t>(mState.getSize());
+    const bool inUseAndRespecifiedWithoutData = (data == nullptr && isCurrentlyInUse(contextVk));
 
-    // BufferData call is re-specifying the entire buffer
-    // Release and init a new mBuffer with this new size
-    if (!wholeSize)
+    // The entire buffer is being respecified, possibly with null data.
+    // Release and init a new mBuffer with requested size.
+    if (bufferSizeChanged || inUseAndRespecifiedWithoutData)
     {
         // Release and re-create the memory and buffer.
         release(contextVk);
@@ -383,8 +384,8 @@ angle::Result BufferVk::setDataWithMemoryType(const gl::Context *context,
     if (data)
     {
         // Treat full-buffer updates as SubData calls.
-        BufferUpdateType updateType =
-            wholeSize ? BufferUpdateType::ContentsUpdate : BufferUpdateType::StorageRedefined;
+        BufferUpdateType updateType = bufferSizeChanged ? BufferUpdateType::StorageRedefined
+                                                        : BufferUpdateType::ContentsUpdate;
 
         ANGLE_TRY(setDataImpl(contextVk, static_cast<const uint8_t *>(data), size, 0, updateType));
     }
diff --git a/src/tests/angle_end2end_tests_expectations.txt b/src/tests/angle_end2end_tests_expectations.txt
index 7d5bdc7169..94794897c9 100644
--- a/src/tests/angle_end2end_tests_expectations.txt
+++ b/src/tests/angle_end2end_tests_expectations.txt
@@ -288,6 +288,7 @@
 7109 WIN D3D11 : SimpleStateChangeTestES3.DrawAndInvalidateRGBThenVerifyAlpha/* = SKIP
 7113 WIN D3D9 : GLSLTest.LoopBodyEndingInBranch*/* = SKIP
 7148 WIN D3D9 : RobustResourceInitTest.OutOfBoundsArrayBuffers/* = SKIP
+7213 WIN D3D11 : BufferDataTestES3.BufferDataWithNullFollowedByMap/* = SKIP
 
 // Android
 6095 ANDROID GLES : GLSLTest_ES3.InitGlobalComplexConstant/* = SKIP
@@ -378,6 +379,7 @@
 5981 PIXEL4ORXL GLES : VulkanExternalImageTest.ShouldClearOpaqueFdRGBA8/* = SKIP
 5981 PIXEL4ORXL GLES : VulkanExternalImageTest.TextureFormatCompatChromiumFd/* = SKIP
 7142 PIXEL4ORXL GLES : GLSLTest.AliasingFunctionOutParamAndGlobal/* = SKIP
+7213 PIXEL4ORXL GLES : BufferDataTestES3.BufferDataWithNullFollowedByMap/* = SKIP
 
 5946 PIXEL4ORXL VULKAN : TransformFeedbackTestES32.PrimitivesWrittenAndGenerated/* = SKIP
 5947 PIXEL4ORXL VULKAN : FramebufferFetchES31.DrawFetchBlitDrawFetch_NonCoherent/* = SKIP
diff --git a/src/tests/gl_tests/BufferDataTest.cpp b/src/tests/gl_tests/BufferDataTest.cpp
index ba49f3cd5f..649bde57cd 100644
--- a/src/tests/gl_tests/BufferDataTest.cpp
+++ b/src/tests/gl_tests/BufferDataTest.cpp
@@ -1143,6 +1143,60 @@ void main()
     EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::cyan);
 }
 
+// Verify that previous draws are not affected when a buffer is respecified with null data
+// and updated by calling map.
+TEST_P(BufferDataTestES3, BufferDataWithNullFollowedByMap)
+{
+    // Draw without using drawQuad.
+    glUseProgram(mProgram);
+
+    // Set up position attribute
+    const auto &quadVertices = GetQuadVertices();
+    GLint positionLocation   = glGetAttribLocation(mProgram, "position");
+    ASSERT_NE(-1, positionLocation);
+    GLBuffer positionBuffer;
+    glBindBuffer(GL_ARRAY_BUFFER, positionBuffer);
+    glBufferData(GL_ARRAY_BUFFER, sizeof(GLfloat) * quadVertices.size() * 3, quadVertices.data(),
+                 GL_DYNAMIC_DRAW);
+    glVertexAttribPointer(positionLocation, 3, GL_FLOAT, GL_FALSE, 0, nullptr);
+    glEnableVertexAttribArray(positionLocation);
+    EXPECT_GL_NO_ERROR();
+
+    // Set up "in_attrib" attribute
+    const std::vector<GLfloat> kData(6, 1.0f);
+    glBindBuffer(GL_ARRAY_BUFFER, mBuffer);
+    glBufferData(GL_ARRAY_BUFFER, sizeof(GLfloat) * kData.size(), kData.data(), GL_STATIC_DRAW);
+    glVertexAttribPointer(mAttribLocation, 1, GL_FLOAT, GL_FALSE, 0, nullptr);
+    glEnableVertexAttribArray(mAttribLocation);
+    EXPECT_GL_NO_ERROR();
+
+    // This draw (draw_0) renders red to the entire window.
+    glDrawArrays(GL_TRIANGLES, 0, 6);
+    EXPECT_GL_NO_ERROR();
+
+    // Respecify buffer bound to "in_attrib" attribute then map it and fill it with zeroes.
+    const std::vector<GLfloat> kZeros(6, 0.0f);
+    glBufferData(GL_ARRAY_BUFFER, sizeof(GLfloat) * kZeros.size(), nullptr, GL_STATIC_DRAW);
+    uint8_t *mapPtr = reinterpret_cast<uint8_t *>(
+        glMapBufferRange(GL_ARRAY_BUFFER, 0, sizeof(GLfloat) * kZeros.size(),
+                         GL_MAP_WRITE_BIT | GL_MAP_UNSYNCHRONIZED_BIT));
+    ASSERT_NE(nullptr, mapPtr);
+    ASSERT_GL_NO_ERROR();
+    memcpy(mapPtr, kZeros.data(), sizeof(GLfloat) * kZeros.size());
+    glUnmapBuffer(GL_ARRAY_BUFFER);
+    ASSERT_GL_NO_ERROR();
+
+    // This draw (draw_1) renders black to the upper right triangle.
+    glDrawArrays(GL_TRIANGLES, 3, 3);
+    EXPECT_GL_NO_ERROR();
+
+    // Respecification and data update of mBuffer should not have affected draw_0.
+    // Expect bottom left to be red and top right to be black.
+    EXPECT_PIXEL_COLOR_EQ(1, 1, GLColor::red);
+    EXPECT_PIXEL_COLOR_EQ(getWindowWidth() - 1, getWindowHeight() - 1, GLColor::black);
+    EXPECT_GL_NO_ERROR();
+}
+
 class BufferStorageTestES3 : public BufferDataTest
 {};