Remove flushAndSubit from SkiaPipeline renderFrame.

Previously we would submit a command buffer for the
frames draws and then immediately submit a second
command buffer that had no commands and just an attached
semaphore. This merges those two submit calls into Skia
into one call.

Test: manual running of system of benches
Bug: 175913056

Change-Id: If0d054960de8b459814cbfa9289756f8ee9d4a93
(cherry picked from commit f16ba60194)

libs/hwui/pipeline/skia/SkiaOpenGLPipeline.cpp

@@ -105,7 +105,6 @@ bool SkiaOpenGLPipeline::draw(const Frame& frame, const SkRect& screenDirty, con
     LightingInfo::updateLighting(lightGeometry, lightInfo);
     renderFrame(*layerUpdateQueue, dirty, renderNodes, opaque, contentDrawBounds, surface,
                 SkMatrix::I());
-    layerUpdateQueue->clear();
 
     // Draw visual debugging features
     if (CC_UNLIKELY(Properties::showDirtyRegions ||
@@ -113,9 +112,14 @@ bool SkiaOpenGLPipeline::draw(const Frame& frame, const SkRect& screenDirty, con
         SkCanvas* profileCanvas = surface->getCanvas();
         SkiaProfileRenderer profileRenderer(profileCanvas);
         profiler->draw(profileRenderer);
-        profileCanvas->flush();
     }
 
+    {
+        ATRACE_NAME("flush commands");
+        surface->flushAndSubmit();
+    }
+    layerUpdateQueue->clear();
+
     // Log memory statistics
     if (CC_UNLIKELY(Properties::debugLevel != kDebugDisabled)) {
         dumpResourceCacheUsage();

libs/hwui/pipeline/skia/SkiaPipeline.cpp

@@ -454,9 +454,6 @@ void SkiaPipeline::renderFrame(const LayerUpdateQueue& layers, const SkRect& cli
         renderOverdraw(clip, nodes, contentDrawBounds, surface, preTransform);
     }
 
-    ATRACE_NAME("flush commands");
-    surface->flushAndSubmit();
-
     Properties::skpCaptureEnabled = previousSkpEnabled;
 }
 

libs/hwui/pipeline/skia/SkiaVulkanPipeline.cpp

@@ -17,14 +17,15 @@
 #include "SkiaVulkanPipeline.h"
 
 #include "DeferredLayerUpdater.h"
+#include "LightingInfo.h"
 #include "Readback.h"
 #include "ShaderCache.h"
-#include "LightingInfo.h"
 #include "SkiaPipeline.h"
 #include "SkiaProfileRenderer.h"
 #include "VkInteropFunctorDrawable.h"
 #include "renderstate/RenderState.h"
 #include "renderthread/Frame.h"
+#include "utils/TraceUtils.h"
 
 #include <SkSurface.h>
 #include <SkTypes.h>
@@ -73,8 +74,6 @@ bool SkiaVulkanPipeline::draw(const Frame& frame, const SkRect& screenDirty, con
     LightingInfo::updateLighting(lightGeometry, lightInfo);
     renderFrame(*layerUpdateQueue, dirty, renderNodes, opaque, contentDrawBounds, backBuffer,
                 mVkSurface->getCurrentPreTransform());
-    ShaderCache::get().onVkFrameFlushed(mRenderThread.getGrContext());
-    layerUpdateQueue->clear();
 
     // Draw visual debugging features
     if (CC_UNLIKELY(Properties::showDirtyRegions ||
@@ -82,9 +81,14 @@ bool SkiaVulkanPipeline::draw(const Frame& frame, const SkRect& screenDirty, con
         SkCanvas* profileCanvas = backBuffer->getCanvas();
         SkiaProfileRenderer profileRenderer(profileCanvas);
         profiler->draw(profileRenderer);
-        profileCanvas->flush();
     }
 
+    {
+        ATRACE_NAME("flush commands");
+        mVkManager.finishFrame(backBuffer.get());
+    }
+    layerUpdateQueue->clear();
+
     // Log memory statistics
     if (CC_UNLIKELY(Properties::debugLevel != kDebugDisabled)) {
         dumpResourceCacheUsage();

libs/hwui/renderthread/VulkanManager.cpp

@@ -31,6 +31,7 @@
 
 #include "Properties.h"
 #include "RenderThread.h"
+#include "pipeline/skia/ShaderCache.h"
 #include "renderstate/RenderState.h"
 #include "utils/TraceUtils.h"
 
@@ -476,17 +477,10 @@ static void destroy_semaphore(void* context) {
     }
 }
 
-void VulkanManager::swapBuffers(VulkanSurface* surface, const SkRect& dirtyRect) {
-    if (CC_UNLIKELY(Properties::waitForGpuCompletion)) {
-        ATRACE_NAME("Finishing GPU work");
-        mDeviceWaitIdle(mDevice);
-    }
-
-    VulkanSurface::NativeBufferInfo* bufferInfo = surface->getCurrentBufferInfo();
-    if (!bufferInfo) {
-        // If VulkanSurface::dequeueNativeBuffer failed earlier, then swapBuffers is a no-op.
-        return;
-    }
+void VulkanManager::finishFrame(SkSurface* surface) {
+    ATRACE_NAME("Vulkan finish frame");
+    ALOGE_IF(mSwapSemaphore != VK_NULL_HANDLE || mDestroySemaphoreContext != nullptr,
+             "finishFrame already has an outstanding semaphore");
 
     VkExportSemaphoreCreateInfo exportInfo;
     exportInfo.sType = VK_STRUCTURE_TYPE_EXPORT_SEMAPHORE_CREATE_INFO;
@@ -499,32 +493,52 @@ void VulkanManager::swapBuffers(VulkanSurface* surface, const SkRect& dirtyRect)
     semaphoreInfo.flags = 0;
     VkSemaphore semaphore;
     VkResult err = mCreateSemaphore(mDevice, &semaphoreInfo, nullptr, &semaphore);
-    ALOGE_IF(VK_SUCCESS != err, "VulkanManager::swapBuffers(): Failed to create semaphore");
+    ALOGE_IF(VK_SUCCESS != err, "VulkanManager::makeSwapSemaphore(): Failed to create semaphore");
 
     GrBackendSemaphore backendSemaphore;
     backendSemaphore.initVulkan(semaphore);
 
-    int fenceFd = -1;
-    DestroySemaphoreInfo* destroyInfo =
-            new DestroySemaphoreInfo(mDestroySemaphore, mDevice, semaphore);
     GrFlushInfo flushInfo;
-    flushInfo.fNumSemaphores = 1;
-    flushInfo.fSignalSemaphores = &backendSemaphore;
-    flushInfo.fFinishedProc = destroy_semaphore;
-    flushInfo.fFinishedContext = destroyInfo;
-    GrSemaphoresSubmitted submitted = bufferInfo->skSurface->flush(
-            SkSurface::BackendSurfaceAccess::kPresent, flushInfo);
-    GrDirectContext* context = GrAsDirectContext(bufferInfo->skSurface->recordingContext());
+    if (err == VK_SUCCESS) {
+        mDestroySemaphoreContext = new DestroySemaphoreInfo(mDestroySemaphore, mDevice, semaphore);
+        flushInfo.fNumSemaphores = 1;
+        flushInfo.fSignalSemaphores = &backendSemaphore;
+        flushInfo.fFinishedProc = destroy_semaphore;
+        flushInfo.fFinishedContext = mDestroySemaphoreContext;
+    } else {
+        semaphore = VK_NULL_HANDLE;
+    }
+    GrSemaphoresSubmitted submitted =
+            surface->flush(SkSurface::BackendSurfaceAccess::kPresent, flushInfo);
+    GrDirectContext* context = GrAsDirectContext(surface->recordingContext());
     ALOGE_IF(!context, "Surface is not backed by gpu");
     context->submit();
-    if (submitted == GrSemaphoresSubmitted::kYes) {
+    if (semaphore != VK_NULL_HANDLE) {
+        if (submitted == GrSemaphoresSubmitted::kYes) {
+            mSwapSemaphore = semaphore;
+        } else {
+            destroy_semaphore(mDestroySemaphoreContext);
+            mDestroySemaphoreContext = nullptr;
+        }
+    }
+    skiapipeline::ShaderCache::get().onVkFrameFlushed(context);
+}
+
+void VulkanManager::swapBuffers(VulkanSurface* surface, const SkRect& dirtyRect) {
+    if (CC_UNLIKELY(Properties::waitForGpuCompletion)) {
+        ATRACE_NAME("Finishing GPU work");
+        mDeviceWaitIdle(mDevice);
+    }
+
+    int fenceFd = -1;
+    if (mSwapSemaphore != VK_NULL_HANDLE) {
         VkSemaphoreGetFdInfoKHR getFdInfo;
         getFdInfo.sType = VK_STRUCTURE_TYPE_SEMAPHORE_GET_FD_INFO_KHR;
         getFdInfo.pNext = nullptr;
-        getFdInfo.semaphore = semaphore;
+        getFdInfo.semaphore = mSwapSemaphore;
         getFdInfo.handleType = VK_EXTERNAL_SEMAPHORE_HANDLE_TYPE_SYNC_FD_BIT;
 
-        err = mGetSemaphoreFdKHR(mDevice, &getFdInfo, &fenceFd);
+        VkResult err = mGetSemaphoreFdKHR(mDevice, &getFdInfo, &fenceFd);
         ALOGE_IF(VK_SUCCESS != err, "VulkanManager::swapBuffers(): Failed to get semaphore Fd");
     } else {
         ALOGE("VulkanManager::swapBuffers(): Semaphore submission failed");
@@ -532,9 +546,11 @@ void VulkanManager::swapBuffers(VulkanSurface* surface, const SkRect& dirtyRect)
         std::lock_guard<std::mutex> lock(mGraphicsQueueMutex);
         mQueueWaitIdle(mGraphicsQueue);
     }
-    destroy_semaphore(destroyInfo);
+    destroy_semaphore(mDestroySemaphoreContext);
 
     surface->presentCurrentBuffer(dirtyRect, fenceFd);
+    mSwapSemaphore = VK_NULL_HANDLE;
+    mDestroySemaphoreContext = nullptr;
 }
 
 void VulkanManager::destroySurface(VulkanSurface* surface) {

libs/hwui/renderthread/VulkanManager.h

@@ -69,6 +69,7 @@ public:
     void destroySurface(VulkanSurface* surface);
 
     Frame dequeueNextBuffer(VulkanSurface* surface);
+    void finishFrame(SkSurface* surface);
     void swapBuffers(VulkanSurface* surface, const SkRect& dirtyRect);
 
     // Inserts a wait on fence command into the Vulkan command buffer.
@@ -200,6 +201,9 @@ private:
     SwapBehavior mSwapBehavior = SwapBehavior::Discard;
     GrVkExtensions mExtensions;
     uint32_t mDriverVersion = 0;
+
+    VkSemaphore mSwapSemaphore = VK_NULL_HANDLE;
+    void* mDestroySemaphoreContext = nullptr;
 };
 
 } /* namespace renderthread */