commit b2cf52e677f96b19e1c83d1531abe5c721fe5073
author Mohan Maiya <m.maiya@samsung.com> Wed Nov 10 14:49:09 2021 -0800
committer Angle LUCI CQ <angle-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri Nov 12 20:06:45 2021 +0000
tree 8177533a63874d706522ae12f219b2df3c6f2490
parent 573f9415843f408bfabb36ad603be7b49f87597a

Bug fix in unpack buffer validation

For an implementation that supports GL_EXT_buffer_storage extension
it is valid to read or write from a buffer that is partially or fully
mapped if it was allocated by a call to glBufferStorageEXT with the
GL_MAP_PERSISTENT_BIT_EXT included in <flags>

Bug: angleproject:5056
Bug: angleproject:6689
Test: BufferStorageTestES3.TexImage2DPixelUnpackBufferMappedPersistently
Change-Id: Ia4b6967aab02bbfb101d5253b9c83d314bc92f5f
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3278482
Commit-Queue: Mohan Maiya <m.maiya@samsung.com>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Charlie Lao <cclao@google.com>

src/libANGLE/Buffer.h

diff --git a/src/libANGLE/Buffer.h b/src/libANGLE/Buffer.h
index 9809a3b..da3c8a3 100644
--- a/src/libANGLE/Buffer.h
+++ b/src/libANGLE/Buffer.h
@@ -140,6 +140,10 @@
     GLbitfield getAccessFlags() const { return mState.mAccessFlags; }
     GLenum getAccess() const { return mState.mAccess; }
     GLboolean isMapped() const { return mState.mMapped; }
+    bool isPersistentlyMapped() const
+    {
+        return (mState.mStorageExtUsageFlags & GL_MAP_PERSISTENT_BIT_EXT) != 0;
+    }
     void *getMapPointer() const { return mState.mMapPointer; }
     GLint64 getMapOffset() const { return mState.mMapOffset; }
     GLint64 getMapLength() const { return mState.mMapLength; }

src/libANGLE/validationES3.cpp

diff --git a/src/libANGLE/validationES3.cpp b/src/libANGLE/validationES3.cpp
index 87b9f94..dfa0dfa 100644
--- a/src/libANGLE/validationES3.cpp
+++ b/src/libANGLE/validationES3.cpp
@@ -803,8 +803,8 @@
             }
         }
 
-        // ...the buffer object's data store is currently mapped.
-        if (pixelUnpackBuffer->isMapped())
+        // ...the buffer object's data store is currently mapped but not persistently.
+        if (pixelUnpackBuffer->isMapped() && !pixelUnpackBuffer->isPersistentlyMapped())
         {
             context->validationError(entryPoint, GL_INVALID_OPERATION, kBufferMapped);
             return false;

src/tests/gl_tests/BufferDataTest.cpp

diff --git a/src/tests/gl_tests/BufferDataTest.cpp b/src/tests/gl_tests/BufferDataTest.cpp
index 74c39e4..325ba67 100644
--- a/src/tests/gl_tests/BufferDataTest.cpp
+++ b/src/tests/gl_tests/BufferDataTest.cpp
@@ -1225,6 +1225,41 @@
     EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::red);
 }
 
+// Test that we are able to perform glTex*D calls while a pixel unpack buffer is bound
+// and persistently mapped.
+TEST_P(BufferStorageTestES3, TexImage2DPixelUnpackBufferMappedPersistently)
+{
+    ANGLE_SKIP_TEST_IF(getClientMajorVersion() < 3 ||
+                       !IsGLExtensionEnabled("GL_EXT_buffer_storage"));
+
+    std::vector<uint8_t> data(64);
+    FillVectorWithRandomUBytes(&data);
+
+    GLBuffer buffer;
+    glBindBuffer(GL_PIXEL_UNPACK_BUFFER, buffer.get());
+    glBufferStorageEXT(GL_PIXEL_UNPACK_BUFFER, data.size(), data.data(),
+                       GL_MAP_WRITE_BIT | GL_MAP_PERSISTENT_BIT_EXT);
+
+    // Map the buffer.
+    void *mapPtr = glMapBufferRange(GL_PIXEL_UNPACK_BUFFER, 0, data.size(),
+                                    GL_MAP_WRITE_BIT | GL_MAP_PERSISTENT_BIT_EXT);
+    ASSERT_NE(nullptr, mapPtr);
+    ASSERT_GL_NO_ERROR();
+
+    // Create a 2D texture and fill it using the persistenly mapped unpack buffer
+    GLTexture tex;
+    glBindTexture(GL_TEXTURE_2D, tex);
+
+    constexpr GLsizei kTextureWidth  = 4;
+    constexpr GLsizei kTextureHeight = 4;
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, kTextureWidth, kTextureHeight, 0, GL_RGBA,
+                 GL_UNSIGNED_BYTE, 0);
+    ASSERT_GL_NO_ERROR();
+
+    glUnmapBuffer(GL_PIXEL_UNPACK_BUFFER);
+    ASSERT_GL_NO_ERROR();
+}
+
 // Verify persistently mapped buffers can use glBufferSubData
 TEST_P(BufferStorageTestES3, StorageBufferSubDataMapped)
 {