From 95bfd948ca5b40f9e999faab7077ddc1a86d4fcb Mon Sep 17 00:00:00 2001 From: slime Date: Sun, 25 Sep 2022 17:40:53 -0300 Subject: [PATCH] vulkan: move StreamBuffer frame management to StreamBuffer implementation Fixes crashes with lots of auto-batched draws in a frame. --- src/modules/graphics/vulkan/Graphics.cpp | 66 +++++++++----------- src/modules/graphics/vulkan/Graphics.h | 23 +------ src/modules/graphics/vulkan/Shader.cpp | 28 ++++----- src/modules/graphics/vulkan/Shader.h | 5 +- src/modules/graphics/vulkan/StreamBuffer.cpp | 27 ++++---- src/modules/graphics/vulkan/StreamBuffer.h | 2 +- src/modules/graphics/vulkan/Vulkan.h | 2 + 7 files changed, 64 insertions(+), 89 deletions(-) diff --git a/src/modules/graphics/vulkan/Graphics.cpp b/src/modules/graphics/vulkan/Graphics.cpp index 84d204fcb..94d5eedba 100644 --- a/src/modules/graphics/vulkan/Graphics.cpp +++ b/src/modules/graphics/vulkan/Graphics.cpp @@ -59,8 +59,6 @@ static const std::vector deviceExtensions = { VK_KHR_SWAPCHAIN_EXTENSION_NAME, }; -constexpr uint32_t MAX_FRAMES_IN_FLIGHT = 2; - constexpr uint32_t USAGES_POLL_INTERVAL = 5000; const char *Graphics::getName() const @@ -90,6 +88,9 @@ Graphics::Graphics() Graphics::~Graphics() { + defaultConstantColor.set(nullptr); + defaultTexture.set(nullptr); + SDL_Vulkan_UnloadLibrary(); // We already cleaned those up by clearing out batchedDrawBuffers. @@ -454,6 +455,10 @@ void Graphics::present(void *screenshotCallbackdata) else if (result != VK_SUCCESS) throw love::Exception("failed to present swap chain image"); + for (love::graphics::StreamBuffer *buffer : batchedDrawState.vb) + buffer->nextFrame(); + batchedDrawState.indexBuffer->nextFrame(); + drawCalls = 0; renderTargetSwitchCount = 0; drawCallsBatched = 0; @@ -465,8 +470,6 @@ void Graphics::present(void *screenshotCallbackdata) currentFrame = (currentFrame + 1) % MAX_FRAMES_IN_FLIGHT; beginFrame(); - - updatedBatchedDrawBuffers(); } void Graphics::setViewportSize(int width, int height, int pixelwidth, int pixelheight) @@ -508,29 +511,26 @@ bool Graphics::setMode(void *context, int width, int height, int pixelwidth, int createCommandPool(); createCommandBuffers(); - float whiteColor[] = { 1.0f, 1.0f, 1.0f, 1.0f }; + beginFrame(); - batchedDrawBuffers.clear(); - batchedDrawBuffers.reserve(MAX_FRAMES_IN_FLIGHT); - for (int i = 0; i < MAX_FRAMES_IN_FLIGHT; i++) + uint8 whiteColor[] = { 255, 255, 255, 255 }; + + if (batchedDrawState.vb[0] == nullptr) { - batchedDrawBuffers.emplace_back(); // Initial sizes that should be good enough for most cases. It will // resize to fit if needed, later. - batchedDrawBuffers[i].vertexBuffer1 = new StreamBuffer(this, BUFFERUSAGE_VERTEX, 1024 * 1024 * 1); - batchedDrawBuffers[i].vertexBuffer2 = new StreamBuffer(this, BUFFERUSAGE_VERTEX, 256 * 1024 * 1); - batchedDrawBuffers[i].indexBuffer = new StreamBuffer(this, BUFFERUSAGE_INDEX, sizeof(uint16) * LOVE_UINT16_MAX); - - // sometimes the VertexColor is not set, so we manually adjust it to white color - batchedDrawBuffers[i].constantColorBuffer = new StreamBuffer(this, BUFFERUSAGE_VERTEX, sizeof(whiteColor)); - auto mapInfo = batchedDrawBuffers[i].constantColorBuffer->map(sizeof(whiteColor)); - memcpy(mapInfo.data, whiteColor, sizeof(whiteColor)); - batchedDrawBuffers[i].constantColorBuffer->unmap(sizeof(whiteColor)); - batchedDrawBuffers[i].constantColorBuffer->markUsed(sizeof(whiteColor)); + batchedDrawState.vb[0] = new StreamBuffer(this, BUFFERUSAGE_VERTEX, 1024 * 1024 * 1); + batchedDrawState.vb[1] = new StreamBuffer(this, BUFFERUSAGE_VERTEX, 256 * 1024 * 1); + batchedDrawState.indexBuffer = new StreamBuffer(this, BUFFERUSAGE_INDEX, sizeof(uint16) * LOVE_UINT16_MAX); } - updatedBatchedDrawBuffers(); - beginFrame(); + // sometimes the VertexColor is not set, so we manually adjust it to white color + if (defaultConstantColor == nullptr) + { + Buffer::DataDeclaration format("ConstantColor", DATAFORMAT_UNORM8_VEC4); + Buffer::Settings settings(BUFFERUSAGEFLAG_VERTEX, BUFFERDATAUSAGE_STATIC); + defaultConstantColor = newBuffer(settings, { format }, whiteColor, sizeof(whiteColor), 1); + } createDefaultTexture(); createDefaultShaders(); @@ -1205,16 +1205,6 @@ void Graphics::endRecordingGraphicsCommands(bool present) { throw love::Exception("failed to record command buffer"); } -void Graphics::updatedBatchedDrawBuffers() -{ - batchedDrawState.vb[0] = batchedDrawBuffers[currentFrame].vertexBuffer1; - batchedDrawState.vb[0]->nextFrame(); - batchedDrawState.vb[1] = batchedDrawBuffers[currentFrame].vertexBuffer2; - batchedDrawState.vb[1]->nextFrame(); - batchedDrawState.indexBuffer = batchedDrawBuffers[currentFrame].indexBuffer; - batchedDrawState.indexBuffer->nextFrame(); -} - uint32_t Graphics::getNumImagesInFlight() const { return MAX_FRAMES_IN_FLIGHT; @@ -1232,7 +1222,7 @@ const VkDeviceSize Graphics::getMinUniformBufferOffsetAlignment() const graphics::Texture *Graphics::getDefaultTexture() const { - return dynamic_cast(standardTexture.get()); + return defaultTexture; } VkCommandBuffer Graphics::getCommandBufferForDataTransfer() @@ -2364,20 +2354,22 @@ void Graphics::prepareDraw(const VertexAttributes &attributes, const BufferBindi std::vector offsets; for (uint32_t i = 0; i < VertexAttributes::MAX; i++) + { if (buffers.useBits & (1u << i)) { bufferVector.push_back((VkBuffer)buffers.info[i].buffer->getHandle()); offsets.push_back((VkDeviceSize)buffers.info[i].offset); } + } if (usesConstantVertexColor(attributes)) { - bufferVector.push_back((VkBuffer)batchedDrawBuffers[currentFrame].constantColorBuffer->getHandle()); + bufferVector.push_back((VkBuffer)defaultConstantColor->getHandle()); offsets.push_back((VkDeviceSize)0); } if (texture == nullptr) - configuration.shader->setMainTex(standardTexture.get()); + configuration.shader->setMainTex(defaultTexture); else configuration.shader->setMainTex(texture); @@ -2986,9 +2978,10 @@ void Graphics::createSyncObjects() void Graphics::createDefaultTexture() { Texture::Settings settings; - standardTexture.reset((Texture*)newTexture(settings, nullptr)); + defaultTexture.set(newTexture(settings, nullptr), Acquire::NORETAIN); + uint8_t whitePixels[] = {255, 255, 255, 255}; - standardTexture->replacePixels(whitePixels, sizeof(whitePixels), 0, 0, { 0, 0, 1, 1 }, false); + defaultTexture->replacePixels(whitePixels, sizeof(whitePixels), 0, 0, { 0, 0, 1, 1 }, false); } void Graphics::cleanup() @@ -3001,7 +2994,6 @@ void Graphics::cleanup() cleanUpFunctions.clear(); vmaDestroyAllocator(vmaAllocator); - batchedDrawBuffers.clear(); for (size_t i = 0; i < MAX_FRAMES_IN_FLIGHT; i++) { vkDestroySemaphore(device, renderFinishedSemaphores[i], nullptr); diff --git a/src/modules/graphics/vulkan/Graphics.h b/src/modules/graphics/vulkan/Graphics.h index e9fc5c492..9339cb7c4 100644 --- a/src/modules/graphics/vulkan/Graphics.h +++ b/src/modules/graphics/vulkan/Graphics.h @@ -193,22 +193,6 @@ struct GraphicsPipelineConfigurationHasher } }; -struct BatchedDrawBuffers -{ - StreamBuffer *vertexBuffer1; - StreamBuffer *vertexBuffer2; - StreamBuffer *indexBuffer; - StreamBuffer *constantColorBuffer; - - ~BatchedDrawBuffers() - { - delete vertexBuffer1; - delete vertexBuffer2; - delete indexBuffer; - delete constantColorBuffer; - } -}; - struct QueueFamilyIndices { Optional graphicsFamily; @@ -365,7 +349,6 @@ private: void startRecordingGraphicsCommands(bool newFrame); void endRecordingGraphicsCommands(bool present); void ensureGraphicsPipelineConfiguration(GraphicsPipelineConfiguration &configuration); - void updatedBatchedDrawBuffers(); bool usesConstantVertexColor(const VertexAttributes &attribs); void createVulkanVertexFormat( VertexAttributes vertexAttributes, @@ -431,10 +414,8 @@ private: bool framebufferResized = false; bool transitionColorDepthLayouts = false; VmaAllocator vmaAllocator = VK_NULL_HANDLE; - std::unique_ptr standardTexture = nullptr; - // 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; + StrongRef defaultTexture; + StrongRef defaultConstantColor; // 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; diff --git a/src/modules/graphics/vulkan/Shader.cpp b/src/modules/graphics/vulkan/Shader.cpp index 49972a32c..dcbc7be4e 100644 --- a/src/modules/graphics/vulkan/Shader.cpp +++ b/src/modules/graphics/vulkan/Shader.cpp @@ -249,9 +249,8 @@ void Shader::unloadVolatile() while (!freeDescriptorSets.empty()) freeDescriptorSets.pop(); - for (const auto &streamBufferVector : streamBuffers) - for (const auto streamBuffer : streamBufferVector) - delete streamBuffer; + for (const auto streamBuffer : streamBuffers) + streamBuffer->release(); shaderModules.clear(); shaderStages.clear(); @@ -282,19 +281,19 @@ void Shader::newFrame(uint32_t frameIndex) currentUsedUniformStreamBuffersCount = 0; currentUsedDescriptorSetsCount = 0; - if (streamBuffers.at(currentFrame).size() > 1) + if (streamBuffers.size() > 1) { size_t newSize = 0; - for (auto streamBuffer : streamBuffers.at(currentFrame)) + for (auto streamBuffer : streamBuffers) { newSize += streamBuffer->getSize(); - delete streamBuffer; + streamBuffer->release(); } - streamBuffers.at(currentFrame).clear(); - streamBuffers.at(currentFrame).push_back(new StreamBuffer(vgfx, BUFFERUSAGE_UNIFORM, newSize)); + streamBuffers.clear(); + streamBuffers.push_back(new StreamBuffer(vgfx, BUFFERUSAGE_UNIFORM, newSize)); } else - streamBuffers.at(currentFrame).at(0)->nextFrame(); + streamBuffers.at(0)->nextFrame(); if (currentUsedDescriptorSetsCount >= static_cast(descriptorSetsVector.at(currentFrame).size())) descriptorSetsVector.at(currentFrame).push_back(allocateDescriptorSet()); @@ -309,9 +308,9 @@ void Shader::cmdPushDescriptorSets(VkCommandBuffer commandBuffer, VkPipelineBind if (!localUniformData.empty()) { auto usedStreamBufferMemory = currentUsedUniformStreamBuffersCount * uniformBufferSizeAligned; - if (usedStreamBufferMemory >= streamBuffers.at(currentFrame).back()->getSize()) + if (usedStreamBufferMemory >= streamBuffers.back()->getSize()) { - streamBuffers.at(currentFrame).push_back(new StreamBuffer(vgfx, BUFFERUSAGE_UNIFORM, STREAMBUFFER_DEFAULT_SIZE * uniformBufferSizeAligned)); + streamBuffers.push_back(new StreamBuffer(vgfx, BUFFERUSAGE_UNIFORM, STREAMBUFFER_DEFAULT_SIZE * uniformBufferSizeAligned)); currentUsedUniformStreamBuffersCount = 0; } @@ -322,7 +321,7 @@ void Shader::cmdPushDescriptorSets(VkCommandBuffer commandBuffer, VkPipelineBind memcpy(dst, &builtinData, sizeof(builtinData)); } - auto currentStreamBuffer = streamBuffers.at(currentFrame).back(); + auto currentStreamBuffer = streamBuffers.back(); auto mapInfo = currentStreamBuffer->map(uniformBufferSizeAligned); memcpy(mapInfo.data, localUniformData.data(), localUniformData.size()); @@ -1023,10 +1022,7 @@ void Shader::createDescriptorPoolSizes() void Shader::createStreamBuffers() { - const auto numImagesInFlight = vgfx->getNumImagesInFlight(); - streamBuffers.resize(numImagesInFlight); - for (uint32_t i = 0; i < numImagesInFlight; i++) - streamBuffers[i].push_back(new StreamBuffer(vgfx, BUFFERUSAGE_UNIFORM, STREAMBUFFER_DEFAULT_SIZE * uniformBufferSizeAligned)); + streamBuffers.push_back(new StreamBuffer(vgfx, BUFFERUSAGE_UNIFORM, STREAMBUFFER_DEFAULT_SIZE * uniformBufferSizeAligned)); } void Shader::setVideoTextures(graphics::Texture *ytexture, graphics::Texture *cbtexture, graphics::Texture *crtexture) diff --git a/src/modules/graphics/vulkan/Shader.h b/src/modules/graphics/vulkan/Shader.h index 1519565dd..66abcd9ab 100644 --- a/src/modules/graphics/vulkan/Shader.h +++ b/src/modules/graphics/vulkan/Shader.h @@ -115,9 +115,8 @@ private: std::vector descriptorPoolSizes; // we don't know how much memory we need per frame for the uniform buffer descriptors - // we keep a vector of stream buffers per frame in flight - // that gets dynamically increased if more memory is needed - std::vector> streamBuffers; + // we keep a vector of stream buffers that gets dynamically increased if more memory is needed + std::vector streamBuffers; std::vector descriptorPools; std::queue freeDescriptorSets; std::vector> descriptorSetsVector; diff --git a/src/modules/graphics/vulkan/StreamBuffer.cpp b/src/modules/graphics/vulkan/StreamBuffer.cpp index dd9249d73..3e87ecdea 100644 --- a/src/modules/graphics/vulkan/StreamBuffer.cpp +++ b/src/modules/graphics/vulkan/StreamBuffer.cpp @@ -54,7 +54,7 @@ bool StreamBuffer::loadVolatile() VkBufferCreateInfo bufferInfo{}; bufferInfo.sType = VK_STRUCTURE_TYPE_BUFFER_CREATE_INFO; - bufferInfo.size = getSize(); + bufferInfo.size = getSize() * MAX_FRAMES_IN_FLIGHT; // TODO: Is this sufficient or should it be +1? bufferInfo.usage = getUsageFlags(mode); bufferInfo.sharingMode = VK_SHARING_MODE_EXCLUSIVE; @@ -62,9 +62,8 @@ bool StreamBuffer::loadVolatile() allocCreateInfo.usage = VMA_MEMORY_USAGE_AUTO; allocCreateInfo.flags = VMA_ALLOCATION_CREATE_HOST_ACCESS_RANDOM_BIT | VMA_ALLOCATION_CREATE_MAPPED_BIT; - vmaCreateBuffer(allocator, &bufferInfo, &allocCreateInfo, &buffer, &allocation, &allocInfo); - - usedGPUMemory = 0; + if (vmaCreateBuffer(allocator, &bufferInfo, &allocCreateInfo, &buffer, &allocation, &allocInfo) != VK_SUCCESS) + throw love::Exception("Cannot create stream buffer: out of graphics memory."); return true; } @@ -90,25 +89,31 @@ ptrdiff_t StreamBuffer::getHandle() const return (ptrdiff_t) buffer; } -love::graphics::StreamBuffer::MapInfo StreamBuffer::map(size_t minsize) +love::graphics::StreamBuffer::MapInfo StreamBuffer::map(size_t /*minsize*/) { - (void)minsize; - return love::graphics::StreamBuffer::MapInfo((uint8*) allocInfo.pMappedData + usedGPUMemory, getSize()); + // TODO: do we also need to wait until a fence is complete, here? + + MapInfo info; + info.size = bufferSize - frameGPUReadOffset; + info.data = (uint8*)allocInfo.pMappedData + (frameIndex * bufferSize) + frameGPUReadOffset; + return info; } -size_t StreamBuffer::unmap(size_t usedSize) +size_t StreamBuffer::unmap(size_t /*usedSize*/) { - return usedGPUMemory; + size_t offset = (frameIndex * bufferSize) + frameGPUReadOffset; + return offset; } void StreamBuffer::markUsed(size_t usedSize) { - usedGPUMemory += usedSize; + frameGPUReadOffset += usedSize; } void StreamBuffer::nextFrame() { - usedGPUMemory = 0; + frameIndex = (frameIndex + 1) % MAX_FRAMES_IN_FLIGHT; + frameGPUReadOffset = 0; } } // vulkan diff --git a/src/modules/graphics/vulkan/StreamBuffer.h b/src/modules/graphics/vulkan/StreamBuffer.h index 821dce938..b86548066 100644 --- a/src/modules/graphics/vulkan/StreamBuffer.h +++ b/src/modules/graphics/vulkan/StreamBuffer.h @@ -61,7 +61,7 @@ private: VmaAllocation allocation; VmaAllocationInfo allocInfo; VkBuffer buffer = VK_NULL_HANDLE; - size_t usedGPUMemory; + int frameIndex = 0; }; diff --git a/src/modules/graphics/vulkan/Vulkan.h b/src/modules/graphics/vulkan/Vulkan.h index b57cd39d6..59917070e 100644 --- a/src/modules/graphics/vulkan/Vulkan.h +++ b/src/modules/graphics/vulkan/Vulkan.h @@ -49,6 +49,8 @@ struct TextureFormat VkComponentSwizzle swizzleA = VK_COMPONENT_SWIZZLE_IDENTITY; }; +constexpr uint32_t MAX_FRAMES_IN_FLIGHT = 2; + class Vulkan { public: