From eee5171c393222dd2736ad2aed4a2186c3d214b9 Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Wed, 22 Nov 2017 17:15:50 -0500 Subject: [PATCH] Vulkan: Add uploader for BufferSetSubData --- src/backend/CMakeLists.txt | 2 + src/backend/vulkan/BufferUploader.cpp | 104 ++++++++++++++++++++++++++ src/backend/vulkan/BufferUploader.h | 43 +++++++++++ src/backend/vulkan/BufferVk.cpp | 14 +--- src/backend/vulkan/VulkanBackend.cpp | 12 +++ src/backend/vulkan/VulkanBackend.h | 3 + 6 files changed, 168 insertions(+), 10 deletions(-) create mode 100644 src/backend/vulkan/BufferUploader.cpp create mode 100644 src/backend/vulkan/BufferUploader.h diff --git a/src/backend/CMakeLists.txt b/src/backend/CMakeLists.txt index ce0a03f94a..e693585007 100644 --- a/src/backend/CMakeLists.txt +++ b/src/backend/CMakeLists.txt @@ -289,6 +289,8 @@ if (NXT_ENABLE_VULKAN) list(APPEND BACKEND_SOURCES ${VULKAN_DIR}/vulkan_platform.h + ${VULKAN_DIR}/BufferUploader.cpp + ${VULKAN_DIR}/BufferUploader.h ${VULKAN_DIR}/BufferVk.cpp ${VULKAN_DIR}/BufferVk.h ${VULKAN_DIR}/MemoryAllocator.cpp diff --git a/src/backend/vulkan/BufferUploader.cpp b/src/backend/vulkan/BufferUploader.cpp new file mode 100644 index 0000000000..f91bb38655 --- /dev/null +++ b/src/backend/vulkan/BufferUploader.cpp @@ -0,0 +1,104 @@ +// Copyright 2017 The NXT Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "backend/vulkan/BufferUploader.h" + +#include "backend/vulkan/MemoryAllocator.h" +#include "backend/vulkan/VulkanBackend.h" + +#include + +namespace backend { +namespace vulkan { + + BufferUploader::BufferUploader(Device* device) + : device(device) { + } + + BufferUploader::~BufferUploader() { + ASSERT(buffersToDelete.Empty()); + } + + void BufferUploader::BufferSubData(VkBuffer buffer, VkDeviceSize offset, VkDeviceSize size, const void* data) { + // TODO(cwallez@chromium.org): this is soooooo bad. We should use some sort of ring buffer for this. + + // Create a staging buffer + VkBufferCreateInfo createInfo; + createInfo.sType = VK_STRUCTURE_TYPE_BUFFER_CREATE_INFO; + createInfo.pNext = nullptr; + createInfo.flags = 0; + createInfo.size = size; + createInfo.usage = VK_BUFFER_USAGE_TRANSFER_SRC_BIT; + createInfo.sharingMode = VK_SHARING_MODE_EXCLUSIVE; + createInfo.queueFamilyIndexCount = 0; + createInfo.pQueueFamilyIndices = 0; + + VkBuffer stagingBuffer = VK_NULL_HANDLE; + if (device->fn.CreateBuffer(device->GetVkDevice(), &createInfo, nullptr, &stagingBuffer) != VK_SUCCESS) { + ASSERT(false); + } + + VkMemoryRequirements requirements; + device->fn.GetBufferMemoryRequirements(device->GetVkDevice(), stagingBuffer, &requirements); + + DeviceMemoryAllocation allocation; + if (!device->GetMemoryAllocator()->Allocate(requirements, true, &allocation)) { + ASSERT(false); + } + + if (device->fn.BindBufferMemory(device->GetVkDevice(), stagingBuffer, allocation.GetMemory(), + allocation.GetMemoryOffset()) != VK_SUCCESS) { + ASSERT(false); + } + + // Write to the staging buffer + ASSERT(allocation.GetMappedPointer() != nullptr); + memcpy(allocation.GetMappedPointer(), data, size); + + // Enqueue host write -> transfer src barrier and copy command + VkCommandBuffer commands = device->GetPendingCommandBuffer(); + + VkMemoryBarrier barrier; + barrier.sType = VK_STRUCTURE_TYPE_MEMORY_BARRIER; + barrier.pNext = nullptr; + barrier.srcAccessMask = VK_ACCESS_HOST_WRITE_BIT; + barrier.dstAccessMask = VK_ACCESS_TRANSFER_READ_BIT; + + device->fn.CmdPipelineBarrier(commands, + VK_PIPELINE_STAGE_HOST_BIT, VK_PIPELINE_STAGE_TRANSFER_BIT, 0, + 1, &barrier, + 0, nullptr, + 0, nullptr); + + VkBufferCopy copy; + copy.srcOffset = 0; + copy.dstOffset = offset; + copy.size = size; + device->fn.CmdCopyBuffer(commands, stagingBuffer, buffer, 1, ©); + + // TODO(cwallez@chromium.org): Buffers must be deleted before the memory. + // This happens to work for now, but is fragile. + device->GetMemoryAllocator()->Free(&allocation); + buffersToDelete.Enqueue(stagingBuffer, device->GetSerial()); + } + + void BufferUploader::Tick(Serial completedSerial) { + for (VkBuffer buffer : buffersToDelete.IterateUpTo(completedSerial)) { + device->fn.DestroyBuffer(device->GetVkDevice(), buffer, nullptr); + } + buffersToDelete.ClearUpTo(completedSerial); + } + +} +} diff --git a/src/backend/vulkan/BufferUploader.h b/src/backend/vulkan/BufferUploader.h new file mode 100644 index 0000000000..ba3c2ef986 --- /dev/null +++ b/src/backend/vulkan/BufferUploader.h @@ -0,0 +1,43 @@ +// Copyright 2017 The NXT Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef BACKEND_VULKAN_BUFFERUPLOADER_H_ +#define BACKEND_VULKAN_BUFFERUPLOADER_H_ + +#include "backend/vulkan/vulkan_platform.h" +#include "common/SerialQueue.h" + +namespace backend { +namespace vulkan { + + class Device; + + class BufferUploader { + public: + BufferUploader(Device* device); + ~BufferUploader(); + + void BufferSubData(VkBuffer buffer, VkDeviceSize offset, VkDeviceSize size, const void* data); + + void Tick(Serial completedSerial); + + private: + Device* device = nullptr; + SerialQueue buffersToDelete; + }; + +} +} + +#endif // BACKEND_VULKAN_BUFFERUPLOADER_H_ diff --git a/src/backend/vulkan/BufferVk.cpp b/src/backend/vulkan/BufferVk.cpp index 0af6ebd8bc..0c2c91ff2f 100644 --- a/src/backend/vulkan/BufferVk.cpp +++ b/src/backend/vulkan/BufferVk.cpp @@ -13,6 +13,8 @@ // limitations under the License. #include "backend/vulkan/BufferVk.h" + +#include "backend/vulkan/BufferUploader.h" #include "backend/vulkan/VulkanBackend.h" #include @@ -97,14 +99,8 @@ namespace vulkan { } void Buffer::SetSubDataImpl(uint32_t start, uint32_t count, const uint32_t* data) { - // TODO(cwallez@chromium.org): Make a proper resource uploader. Not all resources - // can be directly mapped. - uint8_t* memory = memoryAllocation.GetMappedPointer(); - ASSERT(memory != nullptr); - - memcpy(memory + start * sizeof(uint32_t), data, count * sizeof(uint32_t)); - - ToBackend(GetDevice())->GetPendingCommandBuffer(); + BufferUploader* uploader = ToBackend(GetDevice())->GetBufferUploader(); + uploader->BufferSubData(handle, start * sizeof(uint32_t), count * sizeof(uint32_t), data); } void Buffer::MapReadAsyncImpl(uint32_t serial, uint32_t start, uint32_t /*count*/) { @@ -113,8 +109,6 @@ namespace vulkan { MapReadRequestTracker* tracker = ToBackend(GetDevice())->GetMapReadRequestTracker(); tracker->Track(this, serial, memory + start); - - ToBackend(GetDevice())->GetPendingCommandBuffer(); } void Buffer::UnmapImpl() { diff --git a/src/backend/vulkan/VulkanBackend.cpp b/src/backend/vulkan/VulkanBackend.cpp index 055849a0f1..93b9aa6a09 100644 --- a/src/backend/vulkan/VulkanBackend.cpp +++ b/src/backend/vulkan/VulkanBackend.cpp @@ -16,6 +16,7 @@ #include "backend/Commands.h" #include "backend/vulkan/BufferVk.h" +#include "backend/vulkan/BufferUploader.h" #include "common/Platform.h" #include @@ -109,6 +110,7 @@ namespace vulkan { mapReadRequestTracker = new MapReadRequestTracker(this); memoryAllocator = new MemoryAllocator(this); + bufferUploader = new BufferUploader(this); } Device::~Device() { @@ -138,6 +140,11 @@ namespace vulkan { } unusedFences.clear(); + if (bufferUploader) { + delete bufferUploader; + bufferUploader = nullptr; + } + if (memoryAllocator) { delete memoryAllocator; memoryAllocator = nullptr; @@ -234,6 +241,7 @@ namespace vulkan { RecycleCompletedCommands(); mapReadRequestTracker->Tick(completedSerial); + bufferUploader->Tick(completedSerial); memoryAllocator->Tick(completedSerial); if (pendingCommands.pool != VK_NULL_HANDLE) { @@ -253,6 +261,10 @@ namespace vulkan { return memoryAllocator; } + BufferUploader* Device::GetBufferUploader() const { + return bufferUploader; + } + Serial Device::GetSerial() const { return nextSerial; } diff --git a/src/backend/vulkan/VulkanBackend.h b/src/backend/vulkan/VulkanBackend.h index c0e41ccc0c..117182a505 100644 --- a/src/backend/vulkan/VulkanBackend.h +++ b/src/backend/vulkan/VulkanBackend.h @@ -69,6 +69,7 @@ namespace vulkan { class MapReadRequestTracker; class MemoryAllocator; + class BufferUploader; struct VulkanBackendTraits { using BindGroupType = BindGroup; @@ -128,6 +129,7 @@ namespace vulkan { const VulkanDeviceInfo& GetDeviceInfo() const; MapReadRequestTracker* GetMapReadRequestTracker() const; MemoryAllocator* GetMemoryAllocator() const; + BufferUploader* GetBufferUploader() const; Serial GetSerial() const; @@ -175,6 +177,7 @@ namespace vulkan { MapReadRequestTracker* mapReadRequestTracker = nullptr; MemoryAllocator* memoryAllocator = nullptr; + BufferUploader* bufferUploader = nullptr; VkFence GetUnusedFence(); void CheckPassedFences();