From 524e0111332497c40714a64ce3af018d8b9eabc4 Mon Sep 17 00:00:00 2001 From: niki Date: Fri, 29 Jul 2022 22:35:35 +0200 Subject: [PATCH] vulkan: make Texture::uploadByteData async Before this commit we would greedily wait for the upload to be complete before exiting the function. This is of course not correct, since we might be changing data that is still being used in a render in flight. Of course we also need to cleanup the resources asynchronously after the frame has been rendered fully. --- src/modules/graphics/vulkan/Graphics.cpp | 21 ++++++++ src/modules/graphics/vulkan/Graphics.h | 6 +++ src/modules/graphics/vulkan/Texture.cpp | 66 ++++++++++++------------ src/modules/graphics/vulkan/Texture.h | 1 - 4 files changed, 59 insertions(+), 35 deletions(-) diff --git a/src/modules/graphics/vulkan/Graphics.cpp b/src/modules/graphics/vulkan/Graphics.cpp index d88245e5f..ea9e7a736 100644 --- a/src/modules/graphics/vulkan/Graphics.cpp +++ b/src/modules/graphics/vulkan/Graphics.cpp @@ -202,6 +202,9 @@ namespace love { } bool Graphics::setMode(void* context, int width, int height, int pixelwidth, int pixelheight, bool windowhasstencil, int msaa) { + cleanUpFunctions.clear(); + cleanUpFunctions.resize(MAX_FRAMES_IN_FLIGHT); + createVulkanInstance(); createSurface(); pickPhysicalDevice(); @@ -494,6 +497,11 @@ namespace love { break; } + for (auto cleanUpFn : cleanUpFunctions.at(currentFrame)) { + cleanUpFn(); + } + cleanUpFunctions.at(currentFrame).clear(); + VkCommandBufferBeginInfo beginInfo{}; beginInfo.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO; beginInfo.flags = 0; @@ -543,6 +551,11 @@ namespace love { return vkCmdPushDescriptorSet; } + void Graphics::executeCommand(std::function command, std::function cleanUp) { + command(commandBuffers.at(imageIndex)); + cleanUpFunctions.at(currentFrame).push_back(std::move(cleanUp)); + } + VkCommandBuffer Graphics::beginSingleTimeCommands() { VkCommandBufferAllocateInfo allocInfo{}; allocInfo.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_ALLOCATE_INFO; @@ -875,6 +888,7 @@ namespace love { vkCmdPushDescriptorSet = (PFN_vkCmdPushDescriptorSetKHR) vkGetDeviceProcAddr(device, "vkCmdPushDescriptorSetKHR"); if (!vkCmdPushDescriptorSet) { + // fixme: how widely adopted is this extension? throw love::Exception("could not get a valid function pointer for vkCmdPushDescriptorSetKHR"); } } @@ -1523,6 +1537,13 @@ namespace love { void Graphics::cleanup() { cleanupSwapChain(); + for (auto &cleanUpFns : cleanUpFunctions) { + for (auto cleanUpFn : cleanUpFns) { + cleanUpFn(); + } + } + cleanUpFunctions.clear(); + vmaDestroyAllocator(vmaAllocator); batchedDrawBuffers.clear(); for (size_t i = 0; i < MAX_FRAMES_IN_FLIGHT; i++) { diff --git a/src/modules/graphics/vulkan/Graphics.h b/src/modules/graphics/vulkan/Graphics.h index 5854ed59e..b0d51f938 100644 --- a/src/modules/graphics/vulkan/Graphics.h +++ b/src/modules/graphics/vulkan/Graphics.h @@ -14,6 +14,7 @@ #include #include #include +#include namespace love { @@ -113,6 +114,8 @@ namespace love { GraphicsReadback* newReadbackInternal(ReadbackMethod method, love::graphics::Buffer* buffer, size_t offset, size_t size, data::ByteData* dest, size_t destoffset) override { return nullptr; }; GraphicsReadback* newReadbackInternal(ReadbackMethod method, love::graphics::Texture* texture, int slice, int mipmap, const Rect& rect, image::ImageData* dest, int destx, int desty) { return nullptr; } + void executeCommand(std::function command, std::function cleanUp); + VkCommandBuffer beginSingleTimeCommands(); void endSingleTimeCommands(VkCommandBuffer); @@ -201,6 +204,9 @@ namespace love { // we need an array of draw buffers, since the frames are being rendered asynchronously // and we can't (or shouldn't) update the contents of the buffers while they're still in flight / being rendered. std::vector batchedDrawBuffers; + // functions that need to be called to cleanup objects that were needed for rendering a frame. + // just like batchedDrawBuffers we need a vector for each frame in flight. + std::vector>> cleanUpFunctions; graphics::Texture* currentTexture = nullptr; VkPolygonMode currentPolygonMode = VK_POLYGON_MODE_FILL; diff --git a/src/modules/graphics/vulkan/Texture.cpp b/src/modules/graphics/vulkan/Texture.cpp index e0654b454..2783e64b9 100644 --- a/src/modules/graphics/vulkan/Texture.cpp +++ b/src/modules/graphics/vulkan/Texture.cpp @@ -213,38 +213,6 @@ namespace love { vgfx->endSingleTimeCommands(commandBuffer); } - void Texture::copyBufferToImage(VkBuffer buffer, VkImage image, const Rect& r) { - auto commandBuffer = vgfx->beginSingleTimeCommands(); - - VkBufferImageCopy region{}; - region.bufferOffset = 0; - region.bufferRowLength = 0; - region.bufferImageHeight = 0; - - region.imageSubresource.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT; - region.imageSubresource.mipLevel = 0; - region.imageSubresource.baseArrayLayer = 0; - region.imageSubresource.layerCount = 1; - - region.imageOffset = { r.x, r.y, 0 }; - region.imageExtent = { - static_cast(r.w), - static_cast(r.h), 1 - }; - - // fixme: we should use VK_IMAGE_LAYOUT_DST_OPTIMAL for transfer - vkCmdCopyBufferToImage( - commandBuffer, - buffer, - image, - VK_IMAGE_LAYOUT_GENERAL, - 1, - ®ion - ); - - vgfx->endSingleTimeCommands(commandBuffer); - } - void Texture::uploadByteData(PixelFormat pixelformat, const void* data, size_t size, int level, int slice, const Rect& r) { VkBuffer stagingBuffer; VmaAllocation vmaAllocation; @@ -263,9 +231,39 @@ namespace love { memcpy(allocInfo.pMappedData, data, size); - copyBufferToImage(stagingBuffer, textureImage, r); + auto command = [buffer = stagingBuffer, image = textureImage, r = r](VkCommandBuffer commandBuffer) { + VkBufferImageCopy region{}; + region.bufferOffset = 0; + region.bufferRowLength = 0; + region.bufferImageHeight = 0; - vmaDestroyBuffer(allocator, stagingBuffer, vmaAllocation); + region.imageSubresource.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT; + region.imageSubresource.mipLevel = 0; + region.imageSubresource.baseArrayLayer = 0; + region.imageSubresource.layerCount = 1; + + region.imageOffset = { r.x, r.y, 0 }; + region.imageExtent = { + static_cast(r.w), + static_cast(r.h), 1 + }; + + // fixme: use VK_IMAGE_LAYOUT_DST_OPTIMAL + vkCmdCopyBufferToImage( + commandBuffer, + buffer, + image, + VK_IMAGE_LAYOUT_GENERAL, + 1, + ®ion + ); + }; + + auto cleanUp = [allocator = allocator, stagingBuffer, vmaAllocation]() { + vmaDestroyBuffer(allocator, stagingBuffer, vmaAllocation); + }; + + vgfx->executeCommand(command, cleanUp); } } } diff --git a/src/modules/graphics/vulkan/Texture.h b/src/modules/graphics/vulkan/Texture.h index df0b1900d..9162dbd1b 100644 --- a/src/modules/graphics/vulkan/Texture.h +++ b/src/modules/graphics/vulkan/Texture.h @@ -37,7 +37,6 @@ namespace love { private: void transitionImageLayout(VkImage, VkImageLayout oldLayout, VkImageLayout newLayout); - void copyBufferToImage(VkBuffer, VkImage, const Rect&); void createTextureImageView(); void createTextureSampler(); void clear(bool white);