GL: Update BufferGL to use ANGLE_GL_TRY.

include/platform/FeaturesGL.h

@@ -492,6 +492,13 @@ struct FeaturesGL : FeatureSetBase
         "Speculative fix for issues on Linux/Wayland where exposing GLX_OML_sync_control renders "
         "Chrome unusable",
         &members, "https://crbug.com/1137851"};
+
+    // Buffers need to maintain a shadow copy of data when buffer data readback is not possible
+    // through the GL API
+    Feature keepBufferShadowCopy = {
+        "keep_buffer_shadow_copy", FeatureCategory::OpenGLWorkarounds,
+        "Maintain a shadow copy of buffer data when the GL API does not permit reading data back.",
+        &members};
 };
 
 inline FeaturesGL::FeaturesGL()  = default;

src/libANGLE/renderer/gl/BufferGL.cpp

@@ -31,29 +31,25 @@ static constexpr gl::BufferBinding SourceBufferOperationTarget = gl::BufferBindi
 // supported GL versions and doesn't affect any current state when it changes.
 static constexpr gl::BufferBinding DestBufferOperationTarget = gl::BufferBinding::Array;
 
-BufferGL::BufferGL(const gl::BufferState &state,
-                   const FunctionsGL *functions,
-                   StateManagerGL *stateManager)
+BufferGL::BufferGL(const gl::BufferState &state, GLuint buffer)
     : BufferImpl(state),
       mIsMapped(false),
       mMapOffset(0),
       mMapSize(0),
-      mShadowBufferData(!CanMapBufferForRead(functions)),
       mShadowCopy(),
       mBufferSize(0),
-      mFunctions(functions),
-      mStateManager(stateManager),
-      mBufferID(0)
-{
-    ASSERT(mFunctions);
-    ASSERT(mStateManager);
+      mBufferID(buffer)
+{}
 
-    mFunctions->genBuffers(1, &mBufferID);
+BufferGL::~BufferGL()
+{
+    ASSERT(mBufferID == 0);
 }
 
-BufferGL::~BufferGL()
+void BufferGL::destroy(const gl::Context *context)
 {
-    mStateManager->deleteBuffer(mBufferID);
+    StateManagerGL *stateManager = GetStateManagerGL(context);
+    stateManager->deleteBuffer(mBufferID);
     mBufferID = 0;
 }
 
@@ -63,12 +59,18 @@ angle::Result BufferGL::setData(const gl::Context *context,
                                 size_t size,
                                 gl::BufferUsage usage)
 {
-    mStateManager->bindBuffer(DestBufferOperationTarget, mBufferID);
-    mFunctions->bufferData(gl::ToGLenum(DestBufferOperationTarget), size, data, ToGLenum(usage));
+    ContextGL *contextGL              = GetImplAs<ContextGL>(context);
+    const FunctionsGL *functions      = GetFunctionsGL(context);
+    StateManagerGL *stateManager      = GetStateManagerGL(context);
+    const angle::FeaturesGL &features = GetFeaturesGL(context);
+
+    stateManager->bindBuffer(DestBufferOperationTarget, mBufferID);
+    ANGLE_GL_TRY(context, functions->bufferData(gl::ToGLenum(DestBufferOperationTarget), size, data,
+                                                ToGLenum(usage)));
 
-    if (mShadowBufferData)
+    if (features.keepBufferShadowCopy.enabled)
     {
-        ANGLE_CHECK_GL_ALLOC(GetImplAs<ContextGL>(context), mShadowCopy.resize(size));
+        ANGLE_CHECK_GL_ALLOC(contextGL, mShadowCopy.resize(size));
 
         if (size > 0 && data != nullptr)
         {
@@ -87,10 +89,15 @@ angle::Result BufferGL::setSubData(const gl::Context *context,
                                    size_t size,
                                    size_t offset)
 {
-    mStateManager->bindBuffer(DestBufferOperationTarget, mBufferID);
-    mFunctions->bufferSubData(gl::ToGLenum(DestBufferOperationTarget), offset, size, data);
+    const FunctionsGL *functions      = GetFunctionsGL(context);
+    StateManagerGL *stateManager      = GetStateManagerGL(context);
+    const angle::FeaturesGL &features = GetFeaturesGL(context);
+
+    stateManager->bindBuffer(DestBufferOperationTarget, mBufferID);
+    ANGLE_GL_TRY(context, functions->bufferSubData(gl::ToGLenum(DestBufferOperationTarget), offset,
+                                                   size, data));
 
-    if (mShadowBufferData && size > 0)
+    if (features.keepBufferShadowCopy.enabled && size > 0)
     {
         memcpy(mShadowCopy.data() + offset, data, size);
     }
@@ -104,18 +111,22 @@ angle::Result BufferGL::copySubData(const gl::Context *context,
                                     GLintptr destOffset,
                                     GLsizeiptr size)
 {
+    const FunctionsGL *functions      = GetFunctionsGL(context);
+    StateManagerGL *stateManager      = GetStateManagerGL(context);
+    const angle::FeaturesGL &features = GetFeaturesGL(context);
+
     BufferGL *sourceGL = GetAs<BufferGL>(source);
 
-    mStateManager->bindBuffer(DestBufferOperationTarget, mBufferID);
-    mStateManager->bindBuffer(SourceBufferOperationTarget, sourceGL->getBufferID());
+    stateManager->bindBuffer(DestBufferOperationTarget, mBufferID);
+    stateManager->bindBuffer(SourceBufferOperationTarget, sourceGL->getBufferID());
 
-    mFunctions->copyBufferSubData(gl::ToGLenum(SourceBufferOperationTarget),
-                                  gl::ToGLenum(DestBufferOperationTarget), sourceOffset, destOffset,
-                                  size);
+    ANGLE_GL_TRY(context, functions->copyBufferSubData(gl::ToGLenum(SourceBufferOperationTarget),
+                                                       gl::ToGLenum(DestBufferOperationTarget),
+                                                       sourceOffset, destOffset, size));
 
-    if (mShadowBufferData && size > 0)
+    if (features.keepBufferShadowCopy.enabled && size > 0)
     {
-        ASSERT(sourceGL->mShadowBufferData);
+        ASSERT(sourceGL->mShadowCopy.size() >= static_cast<size_t>(sourceOffset + size));
         memcpy(mShadowCopy.data() + destOffset, sourceGL->mShadowCopy.data() + sourceOffset, size);
     }
 
@@ -124,21 +135,27 @@ angle::Result BufferGL::copySubData(const gl::Context *context,
 
 angle::Result BufferGL::map(const gl::Context *context, GLenum access, void **mapPtr)
 {
-    if (mShadowBufferData)
+    const FunctionsGL *functions      = GetFunctionsGL(context);
+    StateManagerGL *stateManager      = GetStateManagerGL(context);
+    const angle::FeaturesGL &features = GetFeaturesGL(context);
+
+    if (features.keepBufferShadowCopy.enabled)
     {
         *mapPtr = mShadowCopy.data();
     }
-    else if (mFunctions->mapBuffer)
+    else if (functions->mapBuffer)
     {
-        mStateManager->bindBuffer(DestBufferOperationTarget, mBufferID);
-        *mapPtr = mFunctions->mapBuffer(gl::ToGLenum(DestBufferOperationTarget), access);
+        stateManager->bindBuffer(DestBufferOperationTarget, mBufferID);
+        *mapPtr = ANGLE_GL_TRY(
+            context, functions->mapBuffer(gl::ToGLenum(DestBufferOperationTarget), access));
     }
     else
     {
-        ASSERT(mFunctions->mapBufferRange && access == GL_WRITE_ONLY_OES);
-        mStateManager->bindBuffer(DestBufferOperationTarget, mBufferID);
-        *mapPtr = mFunctions->mapBufferRange(gl::ToGLenum(DestBufferOperationTarget), 0,
-                                             mBufferSize, GL_MAP_WRITE_BIT);
+        ASSERT(functions->mapBufferRange && access == GL_WRITE_ONLY_OES);
+        stateManager->bindBuffer(DestBufferOperationTarget, mBufferID);
+        *mapPtr =
+            ANGLE_GL_TRY(context, functions->mapBufferRange(gl::ToGLenum(DestBufferOperationTarget),
+                                                            0, mBufferSize, GL_MAP_WRITE_BIT));
     }
 
     mIsMapped  = true;
@@ -154,15 +171,20 @@ angle::Result BufferGL::mapRange(const gl::Context *context,
                                  GLbitfield access,
                                  void **mapPtr)
 {
-    if (mShadowBufferData)
+    const FunctionsGL *functions      = GetFunctionsGL(context);
+    StateManagerGL *stateManager      = GetStateManagerGL(context);
+    const angle::FeaturesGL &features = GetFeaturesGL(context);
+
+    if (features.keepBufferShadowCopy.enabled)
     {
         *mapPtr = mShadowCopy.data() + offset;
     }
     else
     {
-        mStateManager->bindBuffer(DestBufferOperationTarget, mBufferID);
-        *mapPtr = mFunctions->mapBufferRange(gl::ToGLenum(DestBufferOperationTarget), offset,
-                                             length, access);
+        stateManager->bindBuffer(DestBufferOperationTarget, mBufferID);
+        *mapPtr =
+            ANGLE_GL_TRY(context, functions->mapBufferRange(gl::ToGLenum(DestBufferOperationTarget),
+                                                            offset, length, access));
     }
 
     mIsMapped  = true;
@@ -174,20 +196,26 @@ angle::Result BufferGL::mapRange(const gl::Context *context,
 
 angle::Result BufferGL::unmap(const gl::Context *context, GLboolean *result)
 {
+    const FunctionsGL *functions      = GetFunctionsGL(context);
+    StateManagerGL *stateManager      = GetStateManagerGL(context);
+    const angle::FeaturesGL &features = GetFeaturesGL(context);
+
     ASSERT(result);
     ASSERT(mIsMapped);
 
-    if (mShadowBufferData)
+    if (features.keepBufferShadowCopy.enabled)
     {
-        mStateManager->bindBuffer(DestBufferOperationTarget, mBufferID);
-        mFunctions->bufferSubData(gl::ToGLenum(DestBufferOperationTarget), mMapOffset, mMapSize,
-                                  mShadowCopy.data() + mMapOffset);
+        stateManager->bindBuffer(DestBufferOperationTarget, mBufferID);
+        ANGLE_GL_TRY(context,
+                     functions->bufferSubData(gl::ToGLenum(DestBufferOperationTarget), mMapOffset,
+                                              mMapSize, mShadowCopy.data() + mMapOffset));
         *result = GL_TRUE;
     }
     else
     {
-        mStateManager->bindBuffer(DestBufferOperationTarget, mBufferID);
-        *result = mFunctions->unmapBuffer(gl::ToGLenum(DestBufferOperationTarget));
+        stateManager->bindBuffer(DestBufferOperationTarget, mBufferID);
+        *result =
+            ANGLE_GL_TRY(context, functions->unmapBuffer(gl::ToGLenum(DestBufferOperationTarget)));
     }
 
     mIsMapped = false;
@@ -201,25 +229,29 @@ angle::Result BufferGL::getIndexRange(const gl::Context *context,
                                       bool primitiveRestartEnabled,
                                       gl::IndexRange *outRange)
 {
+    const FunctionsGL *functions      = GetFunctionsGL(context);
+    StateManagerGL *stateManager      = GetStateManagerGL(context);
+    const angle::FeaturesGL &features = GetFeaturesGL(context);
+
     ASSERT(!mIsMapped);
 
-    if (mShadowBufferData)
+    if (features.keepBufferShadowCopy.enabled)
     {
         *outRange = gl::ComputeIndexRange(type, mShadowCopy.data() + offset, count,
                                           primitiveRestartEnabled);
     }
     else
     {
-        mStateManager->bindBuffer(DestBufferOperationTarget, mBufferID);
+        stateManager->bindBuffer(DestBufferOperationTarget, mBufferID);
 
         const GLuint typeBytes = gl::GetDrawElementsTypeSize(type);
         const uint8_t *bufferData =
-            MapBufferRangeWithFallback(mFunctions, gl::ToGLenum(DestBufferOperationTarget), offset,
+            MapBufferRangeWithFallback(functions, gl::ToGLenum(DestBufferOperationTarget), offset,
                                        count * typeBytes, GL_MAP_READ_BIT);
         if (bufferData)
         {
             *outRange = gl::ComputeIndexRange(type, bufferData, count, primitiveRestartEnabled);
-            mFunctions->unmapBuffer(gl::ToGLenum(DestBufferOperationTarget));
+            ANGLE_GL_TRY(context, functions->unmapBuffer(gl::ToGLenum(DestBufferOperationTarget)));
         }
         else
         {

src/libANGLE/renderer/gl/BufferGL.h

@@ -21,11 +21,11 @@ class StateManagerGL;
 class BufferGL : public BufferImpl
 {
   public:
-    BufferGL(const gl::BufferState &state,
-             const FunctionsGL *functions,
-             StateManagerGL *stateManager);
+    BufferGL(const gl::BufferState &state, GLuint buffer);
     ~BufferGL() override;
 
+    void destroy(const gl::Context *context) override;
+
     angle::Result setData(const gl::Context *context,
                           gl::BufferBinding target,
                           const void *data,
@@ -63,14 +63,10 @@ class BufferGL : public BufferImpl
     size_t mMapOffset;
     size_t mMapSize;
 
-    bool mShadowBufferData;
     angle::MemoryBuffer mShadowCopy;
 
     size_t mBufferSize;
 
-    const FunctionsGL *mFunctions;
-    StateManagerGL *mStateManager;
-
     GLuint mBufferID;
 };
 

src/libANGLE/renderer/gl/ContextGL.cpp

@@ -105,7 +105,12 @@ RenderbufferImpl *ContextGL::createRenderbuffer(const gl::RenderbufferState &sta
 
 BufferImpl *ContextGL::createBuffer(const gl::BufferState &state)
 {
-    return new BufferGL(state, getFunctions(), getStateManager());
+    const FunctionsGL *functions = getFunctions();
+
+    GLuint buffer = 0;
+    functions->genBuffers(1, &buffer);
+
+    return new BufferGL(state, buffer);
 }
 
 VertexArrayImpl *ContextGL::createVertexArray(const gl::VertexArrayState &data)

src/libANGLE/renderer/gl/renderergl_utils.cpp

@@ -1834,6 +1834,8 @@ void InitializeFeatures(const FunctionsGL *functions, angle::FeaturesGL *feature
     // http://crbug.com/1137851
     // Speculative fix for now, leave disabled so users can enable it via flags.
     ANGLE_FEATURE_CONDITION(features, disableSyncControlSupport, false);
+
+    ANGLE_FEATURE_CONDITION(features, keepBufferShadowCopy, !CanMapBufferForRead(functions));
 }
 
 void InitializeFrontendFeatures(const FunctionsGL *functions, angle::FrontendFeatures *features)