Vulkan: Defer glFlush issued in middle of renderpass to endRenderpass

include/platform/FeaturesVk.h

@@ -361,6 +361,15 @@ struct FeaturesVk : FeatureSetBase
         "preferred_large_heap_block_size_4M", FeatureCategory::VulkanWorkarounds,
         "Use 4 MB preferred large heap block size with AMD allocator", &members,
         "http://anglebug.com/4995"};
+
+    // Manhattan is calling glFlush in the middle of renderpass which breaks renderpass and hurts
+    // performance on tile based GPU. When this is enabled, we will defer the glFlush call made in
+    // the middle of renderpass to the end of renderpass.
+    // https://issuetracker.google.com/issues/166475273
+    Feature deferFlushUntilEndRenderPass = {
+        "defer_flush_until_endrenderpass", FeatureCategory::VulkanWorkarounds,
+        "Allow glFlush to be deferred until renderpass ends", &members,
+        "https://issuetracker.google.com/issues/166475273"};
 };
 
 inline FeaturesVk::FeaturesVk()  = default;

src/libANGLE/renderer/vulkan/ContextVk.cpp

@@ -671,6 +671,8 @@ ContextVk::ContextVk(const gl::State &state, gl::ErrorSet *errorSet, RendererVk
       mRenderPassCommands(nullptr),
       mHasPrimaryCommands(false),
       mGpuEventsEnabled(false),
+      mSyncObjectPendingFlush(false),
+      mDeferredFlushCount(0),
       mGpuClockSync{std::numeric_limits<double>::max(), std::numeric_limits<double>::max()},
       mGpuEventTimestampOrigin(0),
       mPerfCounters{},
@@ -973,6 +975,17 @@ angle::Result ContextVk::startPrimaryCommandBuffer()
 
 angle::Result ContextVk::flush(const gl::Context *context)
 {
+    // If we are in middle of renderpass and it is not a shared context, then we will defer the
+    // glFlush call here until the renderpass ends. If sync object has been used, we must respect
+    // glFlush call, otherwise we a wait for sync object without GL_SYNC_FLUSH_COMMANDS_BIT may
+    // never come back.
+    if (mRenderer->getFeatures().deferFlushUntilEndRenderPass.enabled && !context->isShared() &&
+        !mSyncObjectPendingFlush && hasStartedRenderPass())
+    {
+        mDeferredFlushCount++;
+        return angle::Result::Continue;
+    }
+
     return flushImpl(nullptr);
 }
 
@@ -4078,11 +4091,17 @@ angle::Result ContextVk::flushImpl(const vk::Semaphore *signalSemaphore)
     bool hasPendingSemaphore = signalSemaphore || !mWaitSemaphores.empty();
     if (!hasRecordedCommands() && !hasPendingSemaphore && !mGpuEventsEnabled)
     {
+        ASSERT(!mDeferredFlushCount);
         return angle::Result::Continue;
     }
 
     ANGLE_TRACE_EVENT0("gpu.angle", "ContextVk::flush");
 
+    // We must set this to zero before calling flushCommandsAndEndRenderPass to prevent it from
+    // calling back to flushImpl.
+    mDeferredFlushCount     = 0;
+    mSyncObjectPendingFlush = false;
+
     ANGLE_TRY(flushCommandsAndEndRenderPass());
 
     if (mIsAnyHostVisibleBufferWritten)
@@ -4589,6 +4608,7 @@ angle::Result ContextVk::flushCommandsAndEndRenderPass()
 
     if (!mRenderPassCommands->started())
     {
+        ASSERT(!mDeferredFlushCount);
         onRenderPassFinished();
         return angle::Result::Continue;
     }
@@ -4657,6 +4677,12 @@ angle::Result ContextVk::flushCommandsAndEndRenderPass()
         ANGLE_TRY(flushOutsideRenderPassCommands());
     }
 
+    if (mDeferredFlushCount > 0)
+    {
+        // If we have deferred glFlush call in the middle of renderpass, flush them now.
+        ANGLE_TRY(flushImpl(nullptr));
+    }
+
     return angle::Result::Continue;
 }
 

src/libANGLE/renderer/vulkan/ContextVk.h

@@ -634,6 +634,8 @@ class ContextVk : public ContextImpl, public vk::Context
     const vk::PerfCounters &getPerfCounters() const { return mPerfCounters; }
     vk::PerfCounters &getPerfCounters() { return mPerfCounters; }
 
+    void onSyncHelperInitialize() { mSyncObjectPendingFlush = true; }
+
   private:
     // Dirty bits.
     enum DirtyBitType : size_t
@@ -1077,6 +1079,11 @@ class ContextVk : public ContextImpl, public vk::Context
     // A list of gpu events since the last clock sync.
     std::vector<GpuEvent> mGpuEvents;
 
+    // Track SyncHelper object been added into secondary command buffer that has not been flushed to
+    // vulkan.
+    bool mSyncObjectPendingFlush;
+    uint32_t mDeferredFlushCount;
+
     // Semaphores that must be waited on in the next submission.
     std::vector<VkSemaphore> mWaitSemaphores;
     std::vector<VkPipelineStageFlags> mWaitSemaphoreStageMasks;

src/libANGLE/renderer/vulkan/RendererVk.cpp

@@ -1868,6 +1868,8 @@ void RendererVk::initFeatures(DisplayVk *displayVk, const ExtensionNameList &dev
 
     ANGLE_FEATURE_CONDITION(&mFeatures, preferredLargeHeapBlockSize4MB, !isQualcomm);
 
+    ANGLE_FEATURE_CONDITION(&mFeatures, deferFlushUntilEndRenderPass, true);
+
     angle::PlatformMethods *platform = ANGLEPlatformCurrent();
     platform->overrideFeaturesVk(platform, &mFeatures);
 

src/libANGLE/renderer/vulkan/SyncVk.cpp

@@ -56,6 +56,8 @@ angle::Result SyncHelper::initialize(ContextVk *contextVk)
     commandBuffer.setEvent(mEvent.getHandle(), VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT);
     retain(&contextVk->getResourceUseList());
 
+    contextVk->onSyncHelperInitialize();
+
     return angle::Result::Continue;
 }
 

src/tests/gl_tests/OcclusionQueriesTest.cpp

@@ -225,8 +225,9 @@ TEST_P(OcclusionQueriesTest, MultiQueries)
 
     EXPECT_GL_NO_ERROR();
 
-    // A flush shound't clear the query result
-    glFlush();
+    // Due to implementation might skip in-renderpass flush, we are using glFinish here to force a
+    // flush. A flush shound't clear the query result.
+    glFinish();
 
     glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT | GL_STENCIL_BUFFER_BIT);
     drawQuad(mProgram, essl1_shaders::PositionAttrib(), -2, 0.25f);  // this quad should be occluded
@@ -248,7 +249,7 @@ TEST_P(OcclusionQueriesTest, MultiQueries)
              0.5f);  // this quad should not be occluded
     glEndQueryEXT(GL_ANY_SAMPLES_PASSED_EXT);
     // ------------
-    glFlush();
+    glFinish();
 
     glViewport(0, 0, getWindowWidth() / 2, getWindowHeight());
     glScissor(0, 0, getWindowWidth() / 2, getWindowHeight());

src/tests/gl_tests/VulkanPerformanceCounterTest.cpp

@@ -1242,6 +1242,33 @@ TEST_P(VulkanPerformanceCounterTest, DrawbufferChangeWithAllColorMaskDisabled)
     EXPECT_EQ(expectedRenderPassCount, actualRenderPassCount);
 }
 
+// Tests the optimization that a glFlush call issued inside a renderpass will be skipped.
+TEST_P(VulkanPerformanceCounterTest, InRenderpassFlushShouldBotBreakRenderpass)
+{
+    const rx::vk::PerfCounters &counters = hackANGLE();
+    uint32_t expectedRenderPassCount     = counters.renderPasses + 1;
+
+    GLTexture texture;
+    glBindTexture(GL_TEXTURE_2D, texture);
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 256, 256, 0, GL_RGBA, GL_UNSIGNED_BYTE, nullptr);
+
+    GLFramebuffer framebuffer;
+    glBindFramebuffer(GL_FRAMEBUFFER, framebuffer);
+    glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, texture, 0);
+    ASSERT_GL_FRAMEBUFFER_COMPLETE(GL_FRAMEBUFFER);
+    ASSERT_GL_NO_ERROR();
+
+    ANGLE_GL_PROGRAM(redProgram, essl1_shaders::vs::Simple(), essl1_shaders::fs::Red());
+    drawQuad(redProgram, essl1_shaders::PositionAttrib(), 0.5f);
+    glFlush();
+    ANGLE_GL_PROGRAM(greenProgram, essl1_shaders::vs::Simple(), essl1_shaders::fs::Green());
+    drawQuad(greenProgram, essl1_shaders::PositionAttrib(), 0.5f);
+    ASSERT_GL_NO_ERROR();
+
+    uint32_t actualRenderPassCount = counters.renderPasses;
+    EXPECT_EQ(expectedRenderPassCount, actualRenderPassCount);
+}
+
 ANGLE_INSTANTIATE_TEST(VulkanPerformanceCounterTest, ES3_VULKAN());
 ANGLE_INSTANTIATE_TEST(VulkanPerformanceCounterTest_ES31, ES31_VULKAN());