Vulkan: clean up framebuffer clear

include/platform/FeaturesVk.h

@@ -72,9 +72,9 @@ struct FeaturesVk
     // VK_PRESENT_MODE_FIFO_KHR causes random timeouts on Linux Intel. http://anglebug.com/3153
     bool disableFifoPresentMode = false;
 
-    // On Qualcomm, a bug is preventing us from using this optimization with inline commands in the
+    // On Qualcomm, a bug is preventing us from using loadOp=Clear with inline commands in the
     // render pass.  http://anglebug.com/2361
-    bool disableClearWithRenderPassLoadOp = false;
+    bool restartRenderPassAfterLoadOpClear = false;
 };
 
 inline FeaturesVk::FeaturesVk() = default;

src/libANGLE/renderer/vulkan/CommandGraph.cpp

@@ -138,11 +138,9 @@ void MakeDebugUtilsLabel(GLenum source, const char *marker, VkDebugUtilsLabelEXT
     kLabelColors[colorIndex].writeData(label->color);
 }
 
-#if ANGLE_USE_CUSTOM_VULKAN_CMD_BUFFERS
-constexpr VkSubpassContents kRenderPassContents = VK_SUBPASS_CONTENTS_INLINE;
-#else
-constexpr VkSubpassContents kRenderPassContents = VK_SUBPASS_CONTENTS_SECONDARY_COMMAND_BUFFERS;
-#endif
+constexpr VkSubpassContents kRenderPassContents =
+    CommandBuffer::ExecutesInline() ? VK_SUBPASS_CONTENTS_INLINE
+                                    : VK_SUBPASS_CONTENTS_SECONDARY_COMMAND_BUFFERS;
 
 // Helpers to unify executeCommands call based on underlying cmd buffer type
 ANGLE_MAYBE_UNUSED

src/libANGLE/renderer/vulkan/FramebufferVk.cpp

@@ -236,9 +236,6 @@ angle::Result FramebufferVk::clearImpl(const gl::Context *context,
     ASSERT((clearColorBuffers & mState.getEnabledDrawBuffers()) == clearColorBuffers);
     bool clearColor = clearColorBuffers.any();
 
-    // This command buffer is only started once.
-    vk::CommandBuffer *commandBuffer = nullptr;
-
     const gl::FramebufferAttachment *depthAttachment = mState.getDepthAttachment();
     clearDepth                                       = clearDepth && depthAttachment;
     ASSERT(!clearDepth || depthAttachment->isAttached());
@@ -304,8 +301,7 @@ angle::Result FramebufferVk::clearImpl(const gl::Context *context,
     bool clearAnyWithRenderPassLoadOp =
         clearColorWithRenderPassLoadOp || clearDepth || clearStencil;
 
-    if (clearAnyWithRenderPassLoadOp && !isScissorTestEffectivelyEnabled &&
-        !contextVk->getRenderer()->getFeatures().disableClearWithRenderPassLoadOp)
+    if (clearAnyWithRenderPassLoadOp && !isScissorTestEffectivelyEnabled)
     {
         // Clearing color is indicated by the set bits in this mask.  If not clearing colors with
         // render pass loadOp, the default value of all-zeros means the clear is not done in
@@ -319,6 +315,13 @@ angle::Result FramebufferVk::clearImpl(const gl::Context *context,
         ANGLE_TRY(clearWithRenderPassOp(contextVk, clearBuffersWithRenderPassLoadOp, clearDepth,
                                         clearStencil, clearColorValue, modifiedDepthStencilValue));
 
+        // On some hardware, having inline commands at this point results in corrupted output.  In
+        // that case, end the render pass immediately.  http://anglebug.com/2361
+        if (contextVk->getRenderer()->getFeatures().restartRenderPassAfterLoadOpClear)
+        {
+            mFramebuffer.finishCurrentCommands(contextVk->getRenderer());
+        }
+
         // Fallback to other methods for whatever isn't cleared here.
         clearDepth   = false;
         clearStencil = false;
@@ -355,77 +358,13 @@ angle::Result FramebufferVk::clearImpl(const gl::Context *context,
         return angle::Result::Continue;
     }
 
-    if (isScissorTestEffectivelyEnabled)
-    {
-        // With scissor test enabled, we clear very differently and we don't need to access
-        // the image inside each attachment we can just use clearCmdAttachments with our
-        // scissor region instead.
-        ANGLE_TRY(clearWithClearAttachments(contextVk, clearColorBuffers, clearDepth, clearStencil,
-                                            clearColorValue, modifiedDepthStencilValue));
-        return angle::Result::Continue;
-    }
-
-    // Unless working around driver bugs, every clear should have been covered at this point.
-    ASSERT(contextVk->getRenderer()->getFeatures().disableClearWithRenderPassLoadOp);
-
-    // Standard Depth/stencil clear without scissor.
-    if (clearDepth || clearStencil)
-    {
-        ANGLE_TRY(mFramebuffer.recordCommands(contextVk, &commandBuffer));
-
-        RenderTargetVk *renderTarget         = mRenderTargetCache.getDepthStencil();
-        const angle::Format &format          = renderTarget->getImageFormat().textureFormat();
-        const VkImageAspectFlags aspectFlags = vk::GetDepthStencilAspectFlags(format);
-
-        VkImageAspectFlags clearAspects = aspectFlags;
-        if (!clearDepth)
-        {
-            clearAspects &= ~VK_IMAGE_ASPECT_DEPTH_BIT;
-        }
-        if (!clearStencil)
-        {
-            clearAspects &= ~VK_IMAGE_ASPECT_STENCIL_BIT;
-        }
-
-        vk::ImageHelper *image = renderTarget->getImageForWrite(&mFramebuffer);
-        image->clearDepthStencil(aspectFlags, clearAspects, modifiedDepthStencilValue,
-                                 commandBuffer);
-    }
-
-    if (!clearColor)
-    {
-        return angle::Result::Continue;
-    }
-
-    if (!commandBuffer)
-    {
-        ANGLE_TRY(mFramebuffer.recordCommands(contextVk, &commandBuffer));
-    }
-
-    // TODO(jmadill): Support gaps in RenderTargets. http://anglebug.com/2394
-    const auto &colorRenderTargets = mRenderTargetCache.getColors();
-    for (size_t colorIndex : clearColorBuffers)
-    {
-        VkClearColorValue modifiedClearColorValue = clearColorValue;
-        RenderTargetVk *colorRenderTarget         = colorRenderTargets[colorIndex];
-
-        // It's possible we're clearing a render target that has no alpha channel but we represent
-        // it with a texture that has one. We must not affect its alpha channel no matter what the
-        // clear value is in that case.
-        if (mEmulatedAlphaAttachmentMask[colorIndex])
-        {
-            SetEmulatedAlphaValue(colorRenderTarget->getImageFormat(), &modifiedClearColorValue);
-        }
-
-        ASSERT(colorRenderTarget);
-        vk::ImageHelper *image = colorRenderTarget->getImageForWrite(&mFramebuffer);
+    ASSERT(isScissorTestEffectivelyEnabled);
 
-        // If we're clearing a cube map face ensure we only clear the selected layer.
-        image->clearColorLayer(modifiedClearColorValue, colorRenderTarget->getLevelIndex(), 1,
-                               colorRenderTarget->getLayerIndex(), 1, commandBuffer);
-    }
-
-    return angle::Result::Continue;
+    // With scissor test enabled, we clear very differently and we don't need to access
+    // the image inside each attachment we can just use clearCmdAttachments with our
+    // scissor region instead.
+    return clearWithClearAttachments(contextVk, clearColorBuffers, clearDepth, clearStencil,
+                                     clearColorValue, modifiedDepthStencilValue);
 }
 
 angle::Result FramebufferVk::clearBufferfv(const gl::Context *context,

src/libANGLE/renderer/vulkan/RendererVk.cpp

@@ -979,9 +979,10 @@ angle::Result RendererVk::initializeDevice(DisplayVk *displayVk, uint32_t queueF
     enabledFeatures.features.independentBlend    = mPhysicalDeviceFeatures.independentBlend;
     enabledFeatures.features.robustBufferAccess  = mPhysicalDeviceFeatures.robustBufferAccess;
     enabledFeatures.features.samplerAnisotropy   = mPhysicalDeviceFeatures.samplerAnisotropy;
-#if !ANGLE_USE_CUSTOM_VULKAN_CMD_BUFFERS
-    enabledFeatures.features.inheritedQueries = mPhysicalDeviceFeatures.inheritedQueries;
-#endif
+    if (!vk::CommandBuffer::ExecutesInline())
+    {
+        enabledFeatures.features.inheritedQueries = mPhysicalDeviceFeatures.inheritedQueries;
+    }
 
     VkPhysicalDeviceVertexAttributeDivisorFeaturesEXT divisorFeatures = {};
     divisorFeatures.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_VERTEX_ATTRIBUTE_DIVISOR_FEATURES_EXT;
@@ -1253,9 +1254,12 @@ void RendererVk::initFeatures(const ExtensionNameList &deviceExtensionNames)
         mFeatures.disableFifoPresentMode = true;
     }
 
-    if (IsAndroid() && IsQualcomm(mPhysicalDeviceProperties.vendorID))
+    if (vk::CommandBuffer::ExecutesInline())
     {
-        mFeatures.disableClearWithRenderPassLoadOp = true;
+        if (IsAndroid() && IsQualcomm(mPhysicalDeviceProperties.vendorID))
+        {
+            mFeatures.restartRenderPassAfterLoadOpClear = true;
+        }
     }
 }
 

src/libANGLE/renderer/vulkan/SecondaryCommandBuffer.h

@@ -330,6 +330,10 @@ class SecondaryCommandBuffer final : angle::NonCopyable
 
     static bool SupportsQueries(const VkPhysicalDeviceFeatures &features) { return true; }
 
+    // SecondaryCommandBuffer replays its commands inline when executed on the primary command
+    // buffer.
+    static constexpr bool ExecutesInline() { return true; }
+
     // Add commands
     void beginQuery(VkQueryPool queryPool, uint32_t query, VkQueryControlFlags flags);
 

src/libANGLE/renderer/vulkan/vk_wrapper.h

@@ -180,6 +180,10 @@ class CommandBuffer : public WrappedObject<CommandBuffer, VkCommandBuffer>
         return features.inheritedQueries;
     }
 
+    // Vulkan command buffers are executed as secondary command buffers within a primary command
+    // buffer.
+    static constexpr bool ExecutesInline() { return false; }
+
     VkResult begin(const VkCommandBufferBeginInfo &info);
 
     void beginQuery(VkQueryPool queryPool, uint32_t query, VkQueryControlFlags flags);