Metal:Clear Backbuffer when Robust Resource Init enabled

In trying to optimize readPixels
(see: https://chromium-review.googlesource.com/c/angle/angle/+/3584423)
the test RobustResourceInitTest.SurfaceInitialized was failing.
Digging into it it turns out that backbuffer surfaces were not being
cleared.

WindowSurfaceMtl was losing initialization requests due to back-to-back
calls to ensureCurrentDrawableObtained. Refactor surface preparation and
track initialized state separately.

Bug: angleproject:7117
Change-Id: Ic7eac9e77c4412c55340039a21be63e39b2abc0c
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3590971
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Commit-Queue: Gregg Tavares <gman@chromium.org>

4 files changed, 36 insertions, 47 deletions

diff --git a/src/libANGLE/renderer/metal/FrameBufferMtl.h b/src/libANGLE/renderer/metal/FrameBufferMtl.h
index 00693bfe94..2e033c0da1 100644
--- a/src/libANGLE/renderer/metal/FrameBufferMtl.h
+++ b/src/libANGLE/renderer/metal/FrameBufferMtl.h
@@ -206,6 +206,7 @@ class FramebufferMtl : public FramebufferImpl
                                      const mtl::BufferRef *dstBuffer) const;
 
     RenderTargetMtl *getColorReadRenderTargetNoCache(const gl::Context *context) const;
+    bool prepareForUse(const gl::Context *context) const;
 
     // NOTE: we cannot use RenderTargetCache here because it doesn't support separate
     // depth & stencil attachments as of now. Separate depth & stencil could be useful to
diff --git a/src/libANGLE/renderer/metal/FrameBufferMtl.mm b/src/libANGLE/renderer/metal/FrameBufferMtl.mm
index daf8e3a90b..422b0cac21 100644
--- a/src/libANGLE/renderer/metal/FrameBufferMtl.mm
+++ b/src/libANGLE/renderer/metal/FrameBufferMtl.mm
@@ -687,26 +687,37 @@ angle::Result FramebufferMtl::getSamplePosition(const gl::Context *context,
     return angle::Result::Stop;
 }
 
-RenderTargetMtl *FramebufferMtl::getColorReadRenderTarget(const gl::Context *context) const
+bool FramebufferMtl::prepareForUse(const gl::Context *context) const
 {
-    if (mState.getReadIndex() >= mColorRenderTargets.size())
-    {
-        return nullptr;
-    }
-
     if (mBackbuffer)
     {
-        bool isNewDrawable = false;
-        if (IsError(mBackbuffer->ensureCurrentDrawableObtained(context, &isNewDrawable)))
+        // Backbuffer might obtain new drawable, which means it might change the
+        // the native texture used as the target of the render pass.
+        // We need to call this before creating render encoder.
+        if (IsError(mBackbuffer->ensureCurrentDrawableObtained(context)))
         {
-            return nullptr;
+            return false;
         }
 
-        if (isNewDrawable && mBackbuffer->hasRobustResourceInit())
+        if (mBackbuffer->hasRobustResourceInit())
         {
             (void)mBackbuffer->initializeContents(context, gl::ImageIndex::Make2D(0));
         }
     }
+    return true;
+}
+
+RenderTargetMtl *FramebufferMtl::getColorReadRenderTarget(const gl::Context *context) const
+{
+    if (mState.getReadIndex() >= mColorRenderTargets.size())
+    {
+        return nullptr;
+    }
+
+    if (!prepareForUse(context))
+    {
+        return nullptr;
+    }
 
     return mColorRenderTargets[mState.getReadIndex()];
 }
@@ -761,22 +772,9 @@ mtl::RenderCommandEncoder *FramebufferMtl::ensureRenderPassStarted(const gl::Con
 {
     ContextMtl *contextMtl = mtl::GetImpl(context);
 
-    if (mBackbuffer)
+    if (!prepareForUse(context))
     {
-        // Backbuffer might obtain new drawable, which means it might change the
-        // the native texture used as the target of the render pass.
-        // We need to call this before creating render encoder.
-        bool isNewDrawable;
-        if (IsError(mBackbuffer->ensureCurrentDrawableObtained(context, &isNewDrawable)))
-        {
-            return nullptr;
-        }
-
-        if (isNewDrawable && mBackbuffer->hasRobustResourceInit())
-        {
-            // Apply robust resource initialization on newly obtained drawable.
-            (void)mBackbuffer->initializeContents(context, gl::ImageIndex::Make2D(0));
-        }
+        return nullptr;
     }
 
     // Only support ensureRenderPassStarted() with different load & store options only. The
diff --git a/src/libANGLE/renderer/metal/SurfaceMtl.h b/src/libANGLE/renderer/metal/SurfaceMtl.h
index 10d2e527e9..142ee9e99d 100644
--- a/src/libANGLE/renderer/metal/SurfaceMtl.h
+++ b/src/libANGLE/renderer/metal/SurfaceMtl.h
@@ -107,6 +107,7 @@ class SurfaceMtl : public SurfaceImpl
     bool mAutoResolveMSColorTexture = false;
 
     bool mRobustResourceInit = false;
+    bool mContentInitialized = false;
 
     mtl::Format mColorFormat;
     mtl::Format mDepthFormat;
@@ -154,8 +155,6 @@ class WindowSurfaceMtl : public SurfaceMtl
                                             FramebufferAttachmentRenderTarget **rtOut) override;
 
     angle::Result ensureCurrentDrawableObtained(const gl::Context *context);
-    angle::Result ensureCurrentDrawableObtained(const gl::Context *context,
-                                                bool *newDrawableOut /** nullable */);
 
     // Ensure the the texture returned from getColorTexture() is ready for glReadPixels(). This
     // implicitly calls ensureCurrentDrawableObtained().
diff --git a/src/libANGLE/renderer/metal/SurfaceMtl.mm b/src/libANGLE/renderer/metal/SurfaceMtl.mm
index b1949b58f6..b88ff19476 100644
--- a/src/libANGLE/renderer/metal/SurfaceMtl.mm
+++ b/src/libANGLE/renderer/metal/SurfaceMtl.mm
@@ -255,6 +255,12 @@ angle::Result SurfaceMtl::initializeContents(const gl::Context *context,
                                              const gl::ImageIndex &imageIndex)
 {
     ASSERT(mColorTexture);
+
+    if (mContentInitialized)
+    {
+        return angle::Result::Continue;
+    }
+
     ContextMtl *contextMtl = mtl::GetImpl(context);
 
     // Use loadAction=clear
@@ -279,6 +285,7 @@ angle::Result SurfaceMtl::initializeContents(const gl::Context *context,
     }
     mtl::RenderCommandEncoder *encoder = contextMtl->getRenderPassCommandEncoder(rpDesc);
     encoder->setStoreAction(MTLStoreActionStore);
+    mContentInitialized = true;
 
     return angle::Result::Continue;
 }
@@ -499,14 +506,7 @@ EGLint WindowSurfaceMtl::getSwapBehavior() const
 angle::Result WindowSurfaceMtl::initializeContents(const gl::Context *context,
                                                    const gl::ImageIndex &imageIndex)
 {
-    bool newDrawable;
-    ANGLE_TRY(ensureCurrentDrawableObtained(context, &newDrawable));
-
-    if (!newDrawable)
-    {
-        return angle::Result::Continue;
-    }
-
+    ANGLE_TRY(ensureCurrentDrawableObtained(context));
     return SurfaceMtl::initializeContents(context, imageIndex);
 }
 
@@ -516,7 +516,7 @@ angle::Result WindowSurfaceMtl::getAttachmentRenderTarget(const gl::Context *con
                                                           GLsizei samples,
                                                           FramebufferAttachmentRenderTarget **rtOut)
 {
-    ANGLE_TRY(ensureCurrentDrawableObtained(context, nullptr));
+    ANGLE_TRY(ensureCurrentDrawableObtained(context));
     ANGLE_TRY(ensureCompanionTexturesSizeCorrect(context));
 
     return SurfaceMtl::getAttachmentRenderTarget(context, binding, imageIndex, samples, rtOut);
@@ -524,16 +524,6 @@ angle::Result WindowSurfaceMtl::getAttachmentRenderTarget(const gl::Context *con
 
 angle::Result WindowSurfaceMtl::ensureCurrentDrawableObtained(const gl::Context *context)
 {
-    return ensureCurrentDrawableObtained(context, nullptr);
-}
-angle::Result WindowSurfaceMtl::ensureCurrentDrawableObtained(const gl::Context *context,
-                                                              bool *newDrawableOut)
-{
-    if (newDrawableOut)
-    {
-        *newDrawableOut = !mCurrentDrawable;
-    }
-
     if (!mCurrentDrawable)
     {
         ANGLE_TRY(obtainNextDrawable(context));
@@ -556,7 +546,7 @@ angle::Result WindowSurfaceMtl::ensureCompanionTexturesSizeCorrect(const gl::Con
 
 angle::Result WindowSurfaceMtl::ensureColorTextureReadyForReadPixels(const gl::Context *context)
 {
-    ANGLE_TRY(ensureCurrentDrawableObtained(context, nullptr));
+    ANGLE_TRY(ensureCurrentDrawableObtained(context));
 
     if (mMSColorTexture)
     {
@@ -632,6 +622,7 @@ angle::Result WindowSurfaceMtl::obtainNextDrawable(const gl::Context *context)
             mMetalLayer.get().allowsNextDrawableTimeout = NO;
             mCurrentDrawable.retainAssign([mMetalLayer nextDrawable]);
             mMetalLayer.get().allowsNextDrawableTimeout = YES;
+            mContentInitialized                         = false;
         }
 
         if (!mColorTexture)