Merge "Fix deletion of VkSemaphores in VulkanManager." into qt-dev
diff --git a/libs/hwui/renderthread/VulkanManager.cpp b/libs/hwui/renderthread/VulkanManager.cpp
index 5edf330..62fd489 100644
--- a/libs/hwui/renderthread/VulkanManager.cpp
+++ b/libs/hwui/renderthread/VulkanManager.cpp
@@ -58,10 +58,6 @@
 #define GET_DEV_PROC(F) m##F = (PFN_vk##F)vkGetDeviceProcAddr(mDevice, "vk" #F)
 
 void VulkanManager::destroy() {
-    // We don't need to explicitly free the command buffer since it automatically gets freed when we
-    // delete the VkCommandPool below.
-    mDummyCB = VK_NULL_HANDLE;
-
     if (VK_NULL_HANDLE != mCommandPool) {
         mDestroyCommandPool(mDevice, mCommandPool, nullptr);
         mCommandPool = VK_NULL_HANDLE;
@@ -376,12 +372,6 @@
     }
     LOG_ALWAYS_FATAL_IF(mCommandPool == VK_NULL_HANDLE);
 
-    if (!setupDummyCommandBuffer()) {
-        this->destroy();
-        // Pass through will crash on next line.
-    }
-    LOG_ALWAYS_FATAL_IF(mDummyCB == VK_NULL_HANDLE);
-
     mGetDeviceQueue(mDevice, mPresentQueueIndex, 0, &mPresentQueue);
 
     if (Properties::enablePartialUpdates && Properties::useBufferAge) {
@@ -488,6 +478,22 @@
     return Frame(surface->logicalWidth(), surface->logicalHeight(), bufferAge);
 }
 
+struct DestroySemaphoreInfo {
+    PFN_vkDestroySemaphore mDestroyFunction;
+    VkDevice mDevice;
+    VkSemaphore mSemaphore;
+
+    DestroySemaphoreInfo(PFN_vkDestroySemaphore destroyFunction, VkDevice device,
+            VkSemaphore semaphore)
+            : mDestroyFunction(destroyFunction), mDevice(device), mSemaphore(semaphore) {}
+};
+
+static void destroy_semaphore(void* context) {
+    DestroySemaphoreInfo* info = reinterpret_cast<DestroySemaphoreInfo*>(context);
+    info->mDestroyFunction(info->mDevice, info->mSemaphore, nullptr);
+    delete info;
+}
+
 void VulkanManager::swapBuffers(VulkanSurface* surface, const SkRect& dirtyRect) {
     if (CC_UNLIKELY(Properties::waitForGpuCompletion)) {
         ATRACE_NAME("Finishing GPU work");
@@ -517,9 +523,12 @@
     backendSemaphore.initVulkan(semaphore);
 
     int fenceFd = -1;
+    DestroySemaphoreInfo* destroyInfo = new DestroySemaphoreInfo(mDestroySemaphore, mDevice,
+                                                                 semaphore);
     GrSemaphoresSubmitted submitted =
             bufferInfo->skSurface->flush(SkSurface::BackendSurfaceAccess::kPresent,
-                                         SkSurface::kNone_FlushFlags, 1, &backendSemaphore);
+                                         kNone_GrFlushFlags, 1, &backendSemaphore,
+                                         destroy_semaphore, destroyInfo);
     if (submitted == GrSemaphoresSubmitted::kYes) {
         VkSemaphoreGetFdInfoKHR getFdInfo;
         getFdInfo.sType = VK_STRUCTURE_TYPE_SEMAPHORE_GET_FD_INFO_KHR;
@@ -535,12 +544,6 @@
     }
 
     surface->presentCurrentBuffer(dirtyRect, fenceFd);
-
-    // Exporting a semaphore with copy transference via vkGetSemaphoreFdKHR, has the same effect of
-    // destroying the semaphore and creating a new one with the same handle, and the payloads
-    // ownership is move to the Fd we created. Thus the semaphore is in a state that we can delete
-    // it and we don't need to wait on the command buffer we submitted to finish.
-    mDestroySemaphore(mDevice, semaphore, nullptr);
 }
 
 void VulkanManager::destroySurface(VulkanSurface* surface) {
@@ -566,38 +569,7 @@
                                  *this, extraBuffers);
 }
 
-bool VulkanManager::setupDummyCommandBuffer() {
-    if (mDummyCB != VK_NULL_HANDLE) {
-        return true;
-    }
-
-    VkCommandBufferAllocateInfo commandBuffersInfo;
-    memset(&commandBuffersInfo, 0, sizeof(VkCommandBufferAllocateInfo));
-    commandBuffersInfo.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_ALLOCATE_INFO;
-    commandBuffersInfo.pNext = nullptr;
-    commandBuffersInfo.commandPool = mCommandPool;
-    commandBuffersInfo.level = VK_COMMAND_BUFFER_LEVEL_PRIMARY;
-    commandBuffersInfo.commandBufferCount = 1;
-
-    VkResult err = mAllocateCommandBuffers(mDevice, &commandBuffersInfo, &mDummyCB);
-    if (err != VK_SUCCESS) {
-        // It is probably unnecessary to set this back to VK_NULL_HANDLE, but we set it anyways to
-        // make sure the driver didn't set a value and then return a failure.
-        mDummyCB = VK_NULL_HANDLE;
-        return false;
-    }
-
-    VkCommandBufferBeginInfo beginInfo;
-    memset(&beginInfo, 0, sizeof(VkCommandBufferBeginInfo));
-    beginInfo.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO;
-    beginInfo.flags = VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT;
-
-    mBeginCommandBuffer(mDummyCB, &beginInfo);
-    mEndCommandBuffer(mDummyCB);
-    return true;
-}
-
-status_t VulkanManager::fenceWait(sp<Fence>& fence) {
+status_t VulkanManager::fenceWait(sp<Fence>& fence, GrContext* grContext) {
     if (!hasVkContext()) {
         ALOGE("VulkanManager::fenceWait: VkDevice not initialized");
         return INVALID_OPERATION;
@@ -630,36 +602,22 @@
 
     err = mImportSemaphoreFdKHR(mDevice, &importInfo);
     if (VK_SUCCESS != err) {
+        mDestroySemaphore(mDevice, semaphore, nullptr);
         ALOGE("Failed to import semaphore, err: %d", err);
         return UNKNOWN_ERROR;
     }
 
-    LOG_ALWAYS_FATAL_IF(mDummyCB == VK_NULL_HANDLE);
+    GrBackendSemaphore beSemaphore;
+    beSemaphore.initVulkan(semaphore);
 
-    VkPipelineStageFlags waitDstStageFlags = VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT;
+    // Skia takes ownership of the semaphore and will delete it once the wait has finished.
+    grContext->wait(1, &beSemaphore);
+    grContext->flush();
 
-    VkSubmitInfo submitInfo;
-    memset(&submitInfo, 0, sizeof(VkSubmitInfo));
-    submitInfo.sType = VK_STRUCTURE_TYPE_SUBMIT_INFO;
-    submitInfo.waitSemaphoreCount = 1;
-    // Wait to make sure aquire semaphore set above has signaled.
-    submitInfo.pWaitSemaphores = &semaphore;
-    submitInfo.pWaitDstStageMask = &waitDstStageFlags;
-    submitInfo.commandBufferCount = 1;
-    submitInfo.pCommandBuffers = &mDummyCB;
-    submitInfo.signalSemaphoreCount = 0;
-
-    mQueueSubmit(mGraphicsQueue, 1, &submitInfo, VK_NULL_HANDLE);
-
-    // On Android when we import a semaphore, it is imported using temporary permanence. That
-    // means as soon as we queue the semaphore for a wait it reverts to its previous permanent
-    // state before importing. This means it will now be in an idle state with no pending
-    // signal or wait operations, so it is safe to immediately delete it.
-    mDestroySemaphore(mDevice, semaphore, nullptr);
     return OK;
 }
 
-status_t VulkanManager::createReleaseFence(sp<Fence>& nativeFence) {
+status_t VulkanManager::createReleaseFence(sp<Fence>& nativeFence, GrContext* grContext) {
     if (!hasVkContext()) {
         ALOGE("VulkanManager::createReleaseFence: VkDevice not initialized");
         return INVALID_OPERATION;
@@ -681,20 +639,20 @@
         return INVALID_OPERATION;
     }
 
-    LOG_ALWAYS_FATAL_IF(mDummyCB == VK_NULL_HANDLE);
+    GrBackendSemaphore backendSemaphore;
+    backendSemaphore.initVulkan(semaphore);
 
-    VkSubmitInfo submitInfo;
-    memset(&submitInfo, 0, sizeof(VkSubmitInfo));
-    submitInfo.sType = VK_STRUCTURE_TYPE_SUBMIT_INFO;
-    submitInfo.waitSemaphoreCount = 0;
-    submitInfo.pWaitSemaphores = nullptr;
-    submitInfo.pWaitDstStageMask = nullptr;
-    submitInfo.commandBufferCount = 1;
-    submitInfo.pCommandBuffers = &mDummyCB;
-    submitInfo.signalSemaphoreCount = 1;
-    submitInfo.pSignalSemaphores = &semaphore;
+    DestroySemaphoreInfo* destroyInfo = new DestroySemaphoreInfo(mDestroySemaphore, mDevice,
+                                                                 semaphore);
+    GrSemaphoresSubmitted submitted =
+            grContext->flush(kNone_GrFlushFlags, 1, &backendSemaphore,
+                             destroy_semaphore, destroyInfo);
 
-    mQueueSubmit(mGraphicsQueue, 1, &submitInfo, VK_NULL_HANDLE);
+    if (submitted == GrSemaphoresSubmitted::kNo) {
+        ALOGE("VulkanManager::createReleaseFence: Failed to submit semaphore");
+        mDestroySemaphore(mDevice, semaphore, nullptr);
+        return INVALID_OPERATION;
+    }
 
     VkSemaphoreGetFdInfoKHR getFdInfo;
     getFdInfo.sType = VK_STRUCTURE_TYPE_SEMAPHORE_GET_FD_INFO_KHR;
@@ -711,12 +669,6 @@
     }
     nativeFence = new Fence(fenceFd);
 
-    // Exporting a semaphore with copy transference via vkGetSemahporeFdKHR, has the same effect of
-    // destroying the semaphore and creating a new one with the same handle, and the payloads
-    // ownership is move to the Fd we created. Thus the semahpore is in a state that we can delete
-    // it and we don't need to wait on the command buffer we submitted to finish.
-    mDestroySemaphore(mDevice, semaphore, nullptr);
-
     return OK;
 }
 
diff --git a/libs/hwui/renderthread/VulkanManager.h b/libs/hwui/renderthread/VulkanManager.h
index 1a3a0e4..31de803 100644
--- a/libs/hwui/renderthread/VulkanManager.h
+++ b/libs/hwui/renderthread/VulkanManager.h
@@ -70,10 +70,11 @@
     void destroy();
 
     // Inserts a wait on fence command into the Vulkan command buffer.
-    status_t fenceWait(sp<Fence>& fence);
+    status_t fenceWait(sp<Fence>& fence, GrContext* grContext);
 
-    // Creates a fence that is signaled, when all the pending Vulkan commands are flushed.
-    status_t createReleaseFence(sp<Fence>& nativeFence);
+    // Creates a fence that is signaled when all the pending Vulkan commands are finished on the
+    // GPU.
+    status_t createReleaseFence(sp<Fence>& nativeFence, GrContext* grContext);
 
     // Returned pointers are owned by VulkanManager.
     // An instance of VkFunctorInitParams returned from getVkFunctorInitParams refers to
@@ -89,7 +90,6 @@
     // Sets up the VkInstance and VkDevice objects. Also fills out the passed in
     // VkPhysicalDeviceFeatures struct.
     void setupDevice(GrVkExtensions&, VkPhysicalDeviceFeatures2&);
-    bool setupDummyCommandBuffer();
 
     // simple wrapper class that exists only to initialize a pointer to NULL
     template <typename FNPTR_TYPE>
@@ -164,8 +164,6 @@
     VkQueue mPresentQueue = VK_NULL_HANDLE;
     VkCommandPool mCommandPool = VK_NULL_HANDLE;
 
-    VkCommandBuffer mDummyCB = VK_NULL_HANDLE;
-
     // Variables saved to populate VkFunctorInitParams.
     static const uint32_t mAPIVersion = VK_MAKE_VERSION(1, 1, 0);
     std::vector<VkExtensionProperties> mInstanceExtensionsOwner;
diff --git a/libs/hwui/surfacetexture/ImageConsumer.cpp b/libs/hwui/surfacetexture/ImageConsumer.cpp
index 65d95ad..bae616b 100644
--- a/libs/hwui/surfacetexture/ImageConsumer.cpp
+++ b/libs/hwui/surfacetexture/ImageConsumer.cpp
@@ -212,7 +212,8 @@
             uirenderer::RenderPipelineType::SkiaGL) {
             err = renderState.getRenderThread().eglManager().fenceWait(item.mFence);
         } else {
-            err = renderState.getRenderThread().vulkanManager().fenceWait(item.mFence);
+            err = renderState.getRenderThread().vulkanManager().fenceWait(
+                    item.mFence, renderState.getRenderThread().getGrContext());
         }
         if (err != OK) {
             st.releaseBufferLocked(slot, st.mSlots[slot].mGraphicBuffer, EGL_NO_DISPLAY,
@@ -234,7 +235,8 @@
             err = eglManager.createReleaseFence(st.mUseFenceSync, &mImageSlots[slot].eglFence(),
                                                 releaseFence);
         } else {
-            err = renderState.getRenderThread().vulkanManager().createReleaseFence(releaseFence);
+            err = renderState.getRenderThread().vulkanManager().createReleaseFence(
+                    releaseFence, renderState.getRenderThread().getGrContext());
         }
         if (OK != err) {
             st.releaseBufferLocked(slot, st.mSlots[slot].mGraphicBuffer, EGL_NO_DISPLAY,