From 4982b903033b1d97c8f278b1a81f4471c2e1e25f Mon Sep 17 00:00:00 2001 From: Charlie Lao Date: Tue, 14 Mar 2023 19:56:51 +0000 Subject: [PATCH] Revert "Vulkan: Remove inUseAndRespecifiedWithoutData from BufferVk" This reverts commit 755bfe471d23bc2aac5e78493537801dc5f90792. Reason for revert: Causing flaky on pixel 6 angleproject:8082 Original change's description: > Vulkan: Remove inUseAndRespecifiedWithoutData from BufferVk > > BufferVk::setDataWithMemoryType() has one optimization that it tries to > detect glBufferData(target, size, nullptr, usage) and if existing > storage is busy, it immediately reallocate storage. With the > optimization in previous CL (crrev.com/c/4317488), the storage reuse > logic should detect if we can reuse the storage or not. If the size > matches the existing storage's size, then there is no reason we can not > reuse existing storage. Later on when glBufferSubData or > glMapBufferRange is called, there are optimization in those calls that > will detect if we should reallocate storage or not as the further > optimization. This CL removes this check and replies on the other > optimization to handle the storage reallocate (shadowing) if necessary. > This simplifies code and also potentially avoids storage reallocation in > certain usage cases. > > This CL also fixes a test bug in > BufferDataTestES3.BufferDataWithNullFollowedByMap that was calling > glMapBufferRange with MAP_UNSYNCHRONIZED_BIT but incorrectly expecting > GL to do synchronization. > > Bug: b/271915956 > Change-Id: I7901687b3e3e262e77699f14eb8602d8a57eda3e > Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4322048 > Reviewed-by: Shahbaz Youssefi > Commit-Queue: Charlie Lao > Reviewed-by: Amirali Abdolrashidi Bug: b/271915956 Change-Id: Ie5716b609ab96b96afbe5927f20dfcf2bf5d4db6 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4338263 Auto-Submit: Charlie Lao Commit-Queue: Rubber Stamper Bot-Commit: Rubber Stamper --- src/libANGLE/renderer/vulkan/BufferVk.cpp | 6 +++++- src/tests/gl_tests/BufferDataTest.cpp | 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/libANGLE/renderer/vulkan/BufferVk.cpp b/src/libANGLE/renderer/vulkan/BufferVk.cpp index 0e05523a5..e40b7cceb 100644 --- a/src/libANGLE/renderer/vulkan/BufferVk.cpp +++ b/src/libANGLE/renderer/vulkan/BufferVk.cpp @@ -417,9 +417,13 @@ angle::Result BufferVk::setDataWithMemoryType(const gl::Context *context, } else { + const bool inUseAndRespecifiedWithoutData = (data == nullptr && isCurrentlyInUse(renderer)); // Optimization: Lets figure out if we can reuse the existing storage. bool redefineStorage = shouldRedefineStorage(renderer, usage, memoryPropertyFlags, size); - if (redefineStorage) + + // The entire buffer is being respecified, possibly with null data. + // Release and init a new mBuffer with requested size. + if (redefineStorage || inUseAndRespecifiedWithoutData) { // Release and re-create the memory and buffer. release(contextVk); diff --git a/src/tests/gl_tests/BufferDataTest.cpp b/src/tests/gl_tests/BufferDataTest.cpp index 144792b81..03a869fb8 100644 --- a/src/tests/gl_tests/BufferDataTest.cpp +++ b/src/tests/gl_tests/BufferDataTest.cpp @@ -1219,7 +1219,8 @@ TEST_P(BufferDataTestES3, BufferDataWithNullFollowedByMap) const std::vector kZeros(6, 0.0f); glBufferData(GL_ARRAY_BUFFER, sizeof(GLfloat) * kZeros.size(), nullptr, GL_STATIC_DRAW); uint8_t *mapPtr = reinterpret_cast( - glMapBufferRange(GL_ARRAY_BUFFER, 0, sizeof(GLfloat) * kZeros.size(), GL_MAP_WRITE_BIT)); + 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());