Vulkan: Disallow loadOp=LOAD for MSRTT depth/stencil textures

src/libANGLE/renderer/vulkan/FramebufferVk.cpp

@@ -2242,6 +2242,10 @@ angle::Result FramebufferVk::startNewRenderPass(ContextVk *contextVk,
         RenderTargetVk *colorRenderTarget = colorRenderTargets[colorIndexGL];
         ASSERT(colorRenderTarget);
 
+        // Color render targets are never entirely transient.  Only depth/stencil
+        // multisampled-render-to-texture textures can be so.
+        ASSERT(!colorRenderTarget->isEntirelyTransient());
+
         renderPassAttachmentOps.setLayouts(colorIndexVk, vk::ImageLayout::ColorAttachment,
                                            vk::ImageLayout::ColorAttachment);
 
@@ -2258,11 +2262,11 @@ angle::Result FramebufferVk::startNewRenderPass(ContextVk *contextVk,
         }
         else
         {
-            renderPassAttachmentOps.setOps(colorIndexVk,
-                                           colorRenderTarget->hasDefinedContent()
-                                               ? VK_ATTACHMENT_LOAD_OP_LOAD
-                                               : VK_ATTACHMENT_LOAD_OP_DONT_CARE,
-                                           storeOp);
+            const VkAttachmentLoadOp loadOp = colorRenderTarget->hasDefinedContent()
+                                                  ? VK_ATTACHMENT_LOAD_OP_LOAD
+                                                  : VK_ATTACHMENT_LOAD_OP_DONT_CARE;
+
+            renderPassAttachmentOps.setOps(colorIndexVk, loadOp, storeOp);
             packedClearValues.store(colorIndexVk, VK_IMAGE_ASPECT_COLOR_BIT,
                                     kUninitializedClearValue);
         }
@@ -2309,8 +2313,7 @@ angle::Result FramebufferVk::startNewRenderPass(ContextVk *contextVk,
         // no resolve attachment, there's no data to load.  The latter is the case with
         // depth/stencil texture attachments per GL_EXT_multisampled_render_to_texture2.
         if (!depthStencilRenderTarget->hasDefinedContent() ||
-            (depthStencilRenderTarget->isImageTransient() &&
-             !depthStencilRenderTarget->hasResolveAttachment()))
+            depthStencilRenderTarget->isEntirelyTransient())
         {
             depthLoadOp   = VK_ATTACHMENT_LOAD_OP_DONT_CARE;
             stencilLoadOp = VK_ATTACHMENT_LOAD_OP_DONT_CARE;

src/libANGLE/renderer/vulkan/RenderTargetVk.cpp

@@ -43,7 +43,7 @@ void RenderTargetVk::init(vk::ImageHelper *image,
                           vk::ImageViewHelper *resolveImageViews,
                           gl::LevelIndex levelIndexGL,
                           uint32_t layerIndex,
-                          bool isImageTransient)
+                          RenderTargetTransience transience)
 {
     mImage             = image;
     mImageViews        = imageViews;
@@ -55,7 +55,7 @@ void RenderTargetVk::init(vk::ImageHelper *image,
     // Conservatively assume the content is defined.
     mContentDefined = true;
 
-    mIsImageTransient = isImageTransient;
+    mTransience = transience;
 }
 
 void RenderTargetVk::reset()
@@ -189,7 +189,7 @@ bool RenderTargetVk::isResolveImageOwnerOfData() const
     // If there's a resolve attachment and the image itself is transient, it's the resolve
     // attachment that owns the data, so all non-render-pass accesses to the render target data
     // should go through the resolve attachment.
-    return hasResolveAttachment() && isImageTransient();
+    return isImageTransient();
 }
 
 angle::Result RenderTargetVk::getAndRetainCopyImageView(ContextVk *contextVk,

src/libANGLE/renderer/vulkan/RenderTargetVk.h

@@ -30,6 +30,17 @@ class RenderPassDesc;
 class ContextVk;
 class TextureVk;
 
+enum class RenderTargetTransience
+{
+    // Regular render targets that load and store from the image.
+    Default,
+    // Multisampled-render-to-texture textures, where the implicit multisampled image is transient,
+    // but the resolved image is persistent.
+    MultisampledTransient,
+    // Multisampled-render-to-texture depth/stencil textures.
+    EntirelyTransient,
+};
+
 // This is a very light-weight class that does not own to the resources it points to.
 // It's meant only to copy across some information from a FramebufferAttachment to the
 // business rendering logic. It stores Images and ImageViews by pointer for performance.
@@ -48,7 +59,7 @@ class RenderTargetVk final : public FramebufferAttachmentRenderTarget
               vk::ImageViewHelper *resolveImageViews,
               gl::LevelIndex levelIndexGL,
               uint32_t layerIndex,
-              bool isImageTransient);
+              RenderTargetTransience transience);
     void reset();
 
     vk::ImageViewSubresourceSerial getDrawSubresourceSerial() const;
@@ -103,10 +114,13 @@ class RenderTargetVk final : public FramebufferAttachmentRenderTarget
     void invalidateEntireContent() { mContentDefined = false; }
     void restoreEntireContent() { mContentDefined = true; }
 
-    // See the description of mIsImageTransient for details of how the following two can
-    // interact.
+    // See the description of mTransience for details of how the following two can interact.
     bool hasResolveAttachment() const { return mResolveImage != nullptr; }
-    bool isImageTransient() const { return mIsImageTransient; }
+    bool isImageTransient() const { return mTransience != RenderTargetTransience::Default; }
+    bool isEntirelyTransient() const
+    {
+        return mTransience == RenderTargetTransience::EntirelyTransient;
+    }
 
   private:
     angle::Result getImageViewImpl(ContextVk *contextVk,
@@ -140,36 +154,44 @@ class RenderTargetVk final : public FramebufferAttachmentRenderTarget
     // loadOp of this attachment.
     bool mContentDefined;
 
-    // If resolve attachment exists, |mIsImageTransient| is true if the multisampled results need to
-    // be discarded.
+    // If resolve attachment exists, |mTransience| could be *Transient if the multisampled results
+    // need to be discarded.
     //
-    // - GL_EXT_multisampled_render_to_texture: this is true for render targets created for this
-    //   extension's usage.  Only color attachments use this optimization at the moment.
-    // - GL_EXT_multisampled_render_to_texture2: this is true for depth/stencil textures per this
-    //   extension, even though a resolve attachment is not even provided.
-    // - Multisampled swapchain: TODO(syoussefi) this is true for the multisampled color attachment.
-    //   http://anglebug.com/4836
+    // - GL_EXT_multisampled_render_to_texture[2]: this is |MultisampledTransient| for render
+    //   targets created from color textures, as well as color or depth/stencil renderbuffers.
+    // - GL_EXT_multisampled_render_to_texture2: this is |EntirelyTransient| for depth/stencil
+    //   textures per this extension, even though a resolve attachment is not even provided.
     //
     // Based on the above, we have:
     //
-    //                   mResolveImage == nullptr        |       mResolveImage != nullptr
-    //                                                   |
-    //                      Normal rendering             |               Invalid
-    // !IsTransient            No resolve                |
-    //                       storeOp = STORE             |
-    //                    Owner of data: mImage          |
-    //                                                   |
-    //      ---------------------------------------------+---------------------------------------
-    //                                                   |
-    //               EXT_multisampled_render_to_texture2 | GL_EXT_multisampled_render_to_texture
-    //                                                   | or multisampled Swapchain optimization
-    // IsTransient             No resolve                |               Resolve
-    //                      storeOp = DONT_CARE          |         storeOp = DONT_CARE
-    //                Owner of data: None (not stored)   |     Owner of data: mResolveImage
+    //                     mResolveImage == nullptr
+    //                        Normal rendering
+    // Default                   No resolve
+    //                         storeOp = STORE
+    //                      Owner of data: mImage
+    //
+    //      ---------------------------------------------
+    //
+    //                     mResolveImage != nullptr
+    //               GL_EXT_multisampled_render_to_texture
+    // Multisampled               Resolve
+    // Transient             storeOp = DONT_CARE
+    //                     resolve storeOp = STORE
+    //                   Owner of data: mResolveImage
+    //
+    //      ---------------------------------------------
+    //
+    //                     mResolveImage != nullptr
+    //               GL_EXT_multisampled_render_to_texture2
+    // Entirely                  No Resolve
+    // Transient             storeOp = DONT_CARE
+    //                   Owner of data: mResolveImage
     //
-    // In the above, storeOp of the resolve attachment is always STORE.  if !IsTransient, storeOp is
-    // affected by a framebuffer invalidate call.
-    bool mIsImageTransient;
+    // In the above, storeOp of the resolve attachment is always STORE.  If |Default|, storeOp is
+    // affected by a framebuffer invalidate call.  Note that even though |EntirelyTransient| has a
+    // resolve attachment, it is not used.  The only purpose of |mResolveImage| is to store deferred
+    // clears.
+    RenderTargetTransience mTransience;
 };
 
 // A vector of rendertargets

src/libANGLE/renderer/vulkan/RenderbufferVk.cpp

@@ -112,11 +112,12 @@ angle::Result RenderbufferVk::setStorageImpl(const gl::Context *context,
             contextVk, renderer->getMemoryProperties(), gl::TextureType::_2D, samples, *mImage));
 
         mRenderTarget.init(&mMultisampledImage, &mMultisampledImageViews, mImage, &mImageViews,
-                           gl::LevelIndex(0), 0, true);
+                           gl::LevelIndex(0), 0, RenderTargetTransience::MultisampledTransient);
     }
     else
     {
-        mRenderTarget.init(mImage, &mImageViews, nullptr, nullptr, gl::LevelIndex(0), 0, false);
+        mRenderTarget.init(mImage, &mImageViews, nullptr, nullptr, gl::LevelIndex(0), 0,
+                           RenderTargetTransience::Default);
     }
 
     return angle::Result::Continue;
@@ -178,7 +179,7 @@ angle::Result RenderbufferVk::setStorageEGLImageTarget(const gl::Context *contex
     }
 
     mRenderTarget.init(mImage, &mImageViews, nullptr, nullptr, imageVk->getImageLevel(),
-                       imageVk->getImageLayer(), false);
+                       imageVk->getImageLayer(), RenderTargetTransience::Default);
 
     return angle::Result::Continue;
 }

src/libANGLE/renderer/vulkan/SurfaceVk.cpp

@@ -240,10 +240,10 @@ OffscreenSurfaceVk::OffscreenSurfaceVk(const egl::SurfaceState &surfaceState, Re
       mDepthStencilAttachment(this)
 {
     mColorRenderTarget.init(&mColorAttachment.image, &mColorAttachment.imageViews, nullptr, nullptr,
-                            gl::LevelIndex(0), 0, false);
+                            gl::LevelIndex(0), 0, RenderTargetTransience::Default);
     mDepthStencilRenderTarget.init(&mDepthStencilAttachment.image,
                                    &mDepthStencilAttachment.imageViews, nullptr, nullptr,
-                                   gl::LevelIndex(0), 0, false);
+                                   gl::LevelIndex(0), 0, RenderTargetTransience::Default);
 }
 
 OffscreenSurfaceVk::~OffscreenSurfaceVk() {}
@@ -270,7 +270,7 @@ angle::Result OffscreenSurfaceVk::initializeImpl(DisplayVk *displayVk)
         ANGLE_TRY(mColorAttachment.initialize(
             displayVk, mWidth, mHeight, renderer->getFormat(config->renderTargetFormat), samples));
         mColorRenderTarget.init(&mColorAttachment.image, &mColorAttachment.imageViews, nullptr,
-                                nullptr, gl::LevelIndex(0), 0, false);
+                                nullptr, gl::LevelIndex(0), 0, RenderTargetTransience::Default);
     }
 
     if (config->depthStencilFormat != GL_NONE)
@@ -279,7 +279,7 @@ angle::Result OffscreenSurfaceVk::initializeImpl(DisplayVk *displayVk)
             displayVk, mWidth, mHeight, renderer->getFormat(config->depthStencilFormat), samples));
         mDepthStencilRenderTarget.init(&mDepthStencilAttachment.image,
                                        &mDepthStencilAttachment.imageViews, nullptr, nullptr,
-                                       gl::LevelIndex(0), 0, false);
+                                       gl::LevelIndex(0), 0, RenderTargetTransience::Default);
     }
 
     return angle::Result::Continue;
@@ -483,9 +483,9 @@ WindowSurfaceVk::WindowSurfaceVk(const egl::SurfaceState &surfaceState, EGLNativ
     // Initialize the color render target with the multisampled targets.  If not multisampled, the
     // render target will be updated to refer to a swapchain image on every acquire.
     mColorRenderTarget.init(&mColorImageMS, &mColorImageMSViews, nullptr, nullptr,
-                            gl::LevelIndex(0), 0, false);
+                            gl::LevelIndex(0), 0, RenderTargetTransience::Default);
     mDepthStencilRenderTarget.init(&mDepthStencilImage, &mDepthStencilImageViews, nullptr, nullptr,
-                                   gl::LevelIndex(0), 0, false);
+                                   gl::LevelIndex(0), 0, RenderTargetTransience::Default);
     mDepthStencilImageBinding.bind(&mDepthStencilImage);
     mColorImageMSBinding.bind(&mColorImageMS);
 }
@@ -969,7 +969,7 @@ angle::Result WindowSurfaceVk::createSwapChain(vk::Context *context,
         // Initialize the color render target with the multisampled targets.  If not multisampled,
         // the render target will be updated to refer to a swapchain image on every acquire.
         mColorRenderTarget.init(&mColorImageMS, &mColorImageMSViews, nullptr, nullptr,
-                                gl::LevelIndex(0), 0, false);
+                                gl::LevelIndex(0), 0, RenderTargetTransience::Default);
     }
 
     ANGLE_TRY(resizeSwapchainImages(context, imageCount));
@@ -995,7 +995,8 @@ angle::Result WindowSurfaceVk::createSwapChain(vk::Context *context,
                                                 VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT));
 
         mDepthStencilRenderTarget.init(&mDepthStencilImage, &mDepthStencilImageViews, nullptr,
-                                       nullptr, gl::LevelIndex(0), 0, false);
+                                       nullptr, gl::LevelIndex(0), 0,
+                                       RenderTargetTransience::Default);
 
         // We will need to pass depth/stencil image views to the RenderTargetVk in the future.
     }

src/libANGLE/renderer/vulkan/TextureVk.cpp

@@ -2001,6 +2001,9 @@ angle::Result TextureVk::initRenderTargets(ContextVk *contextVk,
 
         const bool isMultisampledRenderToTexture =
             renderToTextureIndex != gl::RenderToTextureImageIndex::Default;
+        RenderTargetTransience transience = isMultisampledRenderToTexture
+                                                ? RenderTargetTransience::MultisampledTransient
+                                                : RenderTargetTransience::Default;
 
         // If multisampled render to texture, use the multisampled image as draw image instead, and
         // resolve into the texture's image automatically.
@@ -2014,20 +2017,17 @@ angle::Result TextureVk::initRenderTargets(ContextVk *contextVk,
             drawImageViews    = &mMultisampledImageViews[renderToTextureIndex];
 
             // If the texture is depth/stencil, GL_EXT_multisampled_render_to_texture2 explicitly
-            // indicates that there is no need for the image to be resolved.  In that case, don't
-            // specify the resolve image.  Note that the multisampled image's data is discarded
-            // nevertheless per this spec.
+            // indicates that there is no need for the image to be resolved.  In that case, mark the
+            // render target as entirely transient.
             if (mImage->getAspectFlags() != VK_IMAGE_ASPECT_COLOR_BIT)
             {
-                resolveImage      = nullptr;
-                resolveImageViews = nullptr;
+                transience = RenderTargetTransience::EntirelyTransient;
             }
         }
 
         renderTargets[layerIndex].init(drawImage, drawImageViews, resolveImage, resolveImageViews,
                                        getNativeImageLevel(levelIndex),
-                                       getNativeImageLayer(layerIndex),
-                                       isMultisampledRenderToTexture);
+                                       getNativeImageLayer(layerIndex), transience);
     }
     return angle::Result::Continue;
 }

src/libANGLE/renderer/vulkan/mac/IOSurfaceSurfaceVkMac.mm

@@ -119,7 +119,7 @@ angle::Result IOSurfaceSurfaceVkMac::initializeImpl(DisplayVk *displayVk)
         renderer->getFormat(kIOSurfaceFormats[mFormatIndex].nativeSizedInternalFormat), samples,
         IOSurfaceGetBaseAddressOfPlane(mIOSurface, mPlane)));
     mColorRenderTarget.init(&mColorAttachment.image, &mColorAttachment.imageViews, nullptr, nullptr,
-                            gl::LevelIndex(0), 0, false);
+                            gl::LevelIndex(0), 0, RenderTargetTransience::Default);
 
     return angle::Result::Continue;
 }

src/tests/gl_tests/DepthStencilTest.cpp

@@ -105,6 +105,9 @@ class DepthStencilTest : public ANGLETest
     bool mHasStencil = true;
 };
 
+class DepthStencilTestES3 : public DepthStencilTest
+{};
+
 void DepthStencilTest::ensureColor(GLColor color)
 {
     const int width  = getWindowWidth();
@@ -251,6 +254,62 @@ TEST_P(DepthStencilTest, StencilOnlyEmulatedWithPacked)
     ensureDepthUnaffected();
 }
 
+// Tests that clearing depth/stencil followed by draw works when the depth/stencil attachment is a
+// texture.
+TEST_P(DepthStencilTestES3, ClearThenDraw)
+{
+    GLFramebuffer FBO;
+    glBindFramebuffer(GL_FRAMEBUFFER, FBO);
+
+    constexpr GLsizei kSize = 6;
+
+    // Create framebuffer to draw into, with both color and depth attachments.
+    GLTexture color;
+    glBindTexture(GL_TEXTURE_2D, color);
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, kSize, kSize, 0, GL_RGBA, GL_UNSIGNED_BYTE, nullptr);
+
+    GLTexture depth;
+    glBindTexture(GL_TEXTURE_2D, depth);
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_DEPTH24_STENCIL8, kSize, kSize, 0, GL_DEPTH_STENCIL,
+                 GL_UNSIGNED_INT_24_8_OES, nullptr);
+
+    GLFramebuffer fbo;
+    glBindFramebuffer(GL_FRAMEBUFFER, fbo);
+    glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, color, 0);
+    glFramebufferTexture2D(GL_FRAMEBUFFER, GL_DEPTH_STENCIL_ATTACHMENT, GL_TEXTURE_2D, depth, 0);
+    ASSERT_GL_NO_ERROR();
+
+    // Set viewport and clear depth/stencil
+    glViewport(0, 0, kSize, kSize);
+    glClearDepthf(1);
+    glClearStencil(0x55);
+    glClear(GL_DEPTH_BUFFER_BIT | GL_STENCIL_BUFFER_BIT);
+
+    // If depth is not cleared to 1, rendering would fail.
+    glEnable(GL_DEPTH_TEST);
+    glDepthFunc(GL_LESS);
+
+    // If stencil is not clear to 0x55, rendering would fail.
+    glEnable(GL_STENCIL_TEST);
+    glStencilFunc(GL_EQUAL, 0x55, 0xFF);
+    glStencilOp(GL_KEEP, GL_KEEP, GL_KEEP);
+    glStencilMask(0xFF);
+
+    // Set up program
+    ANGLE_GL_PROGRAM(drawRed, essl1_shaders::vs::Simple(), essl1_shaders::fs::Red());
+
+    // Draw red
+    drawQuad(drawRed, essl1_shaders::PositionAttrib(), 0.0f);
+    ASSERT_GL_NO_ERROR();
+
+    // Verify.
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::red);
+    EXPECT_PIXEL_COLOR_EQ(kSize - 1, 0, GLColor::red);
+    EXPECT_PIXEL_COLOR_EQ(0, kSize - 1, GLColor::red);
+    EXPECT_PIXEL_COLOR_EQ(kSize - 1, kSize - 1, GLColor::red);
+}
+
 ANGLE_INSTANTIATE_TEST_ES2_AND_ES3(DepthStencilTest);
+ANGLE_INSTANTIATE_TEST_ES3(DepthStencilTestES3);
 
 }  // anonymous namespace