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

@@ -59,8 +59,6 @@ static const std::vector<const char*> 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();
+
+	uint8 whiteColor[] = { 255, 255, 255, 255 };
 
-	batchedDrawBuffers.clear();
-	batchedDrawBuffers.reserve(MAX_FRAMES_IN_FLIGHT);
-	for (int i = 0; i < MAX_FRAMES_IN_FLIGHT; i++)
+	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<graphics::Texture*>(standardTexture.get());
+	return defaultTexture;
 }
 
 VkCommandBuffer Graphics::getCommandBufferForDataTransfer()
@@ -2364,20 +2354,22 @@ void Graphics::prepareDraw(const VertexAttributes &attributes, const BufferBindi
 	std::vector<VkDeviceSize> 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);

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<uint32_t> 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<Texture> 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> batchedDrawBuffers;
+	StrongRef<love::graphics::Texture> defaultTexture;
+	StrongRef<love::graphics::Buffer> 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<std::vector<std::function<void()>>> cleanUpFunctions;

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<uint32_t>(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)

src/modules/graphics/vulkan/Shader.h

@@ -115,9 +115,8 @@ private:
 	std::vector<VkDescriptorPoolSize> 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<std::vector<StreamBuffer*>> streamBuffers;
+	// we keep a vector of stream buffers that gets dynamically increased if more memory is needed
+	std::vector<StreamBuffer*> streamBuffers;
 	std::vector<VkDescriptorPool> descriptorPools;
 	std::queue<VkDescriptorSet> freeDescriptorSets;
 	std::vector<std::vector<VkDescriptorSet>> descriptorSetsVector;

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

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;
 
 };
 

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: