From 090fd3fefab9ef251e6e5bf4011280a657340f2a Mon Sep 17 00:00:00 2001 From: ReinUsesLisp Date: Sat, 4 Apr 2020 02:54:55 -0300 Subject: buffer_cache: Return handles instead of pointer to handles The original idea of returning pointers is that handles can be moved. The problem is that the implementation didn't take that in mind and made everything harder to work with. This commit drops pointer to handles and returns the handles themselves. While it is still true that handles can be invalidated, this way we get an old handle instead of a dangling pointer. This problem can be solved in the future with sparse buffers. --- src/video_core/renderer_vulkan/vk_buffer_cache.cpp | 12 ++++----- src/video_core/renderer_vulkan/vk_buffer_cache.h | 10 ++++---- src/video_core/renderer_vulkan/vk_compute_pass.cpp | 16 ++++++------ src/video_core/renderer_vulkan/vk_compute_pass.h | 4 +-- src/video_core/renderer_vulkan/vk_rasterizer.cpp | 30 ++++++++++------------ .../renderer_vulkan/vk_update_descriptor.cpp | 5 ++-- .../renderer_vulkan/vk_update_descriptor.h | 16 ++++-------- 7 files changed, 42 insertions(+), 51 deletions(-) (limited to 'src/video_core/renderer_vulkan') diff --git a/src/video_core/renderer_vulkan/vk_buffer_cache.cpp b/src/video_core/renderer_vulkan/vk_buffer_cache.cpp index 0d167afbd..81e1de2be 100644 --- a/src/video_core/renderer_vulkan/vk_buffer_cache.cpp +++ b/src/video_core/renderer_vulkan/vk_buffer_cache.cpp @@ -74,18 +74,18 @@ Buffer VKBufferCache::CreateBlock(VAddr cpu_addr, std::size_t size) { return std::make_shared(device, memory_manager, cpu_addr, size); } -const VkBuffer* VKBufferCache::ToHandle(const Buffer& buffer) { +VkBuffer VKBufferCache::ToHandle(const Buffer& buffer) { return buffer->GetHandle(); } -const VkBuffer* VKBufferCache::GetEmptyBuffer(std::size_t size) { +VkBuffer VKBufferCache::GetEmptyBuffer(std::size_t size) { size = std::max(size, std::size_t(4)); const auto& empty = staging_pool.GetUnusedBuffer(size, false); scheduler.RequestOutsideRenderPassOperationContext(); scheduler.Record([size, buffer = *empty.handle](vk::CommandBuffer cmdbuf) { cmdbuf.FillBuffer(buffer, 0, size, 0); }); - return empty.handle.address(); + return *empty.handle; } void VKBufferCache::UploadBlockData(const Buffer& buffer, std::size_t offset, std::size_t size, @@ -94,7 +94,7 @@ void VKBufferCache::UploadBlockData(const Buffer& buffer, std::size_t offset, st std::memcpy(staging.commit->Map(size), data, size); scheduler.RequestOutsideRenderPassOperationContext(); - scheduler.Record([staging = *staging.handle, buffer = *buffer->GetHandle(), offset, + scheduler.Record([staging = *staging.handle, buffer = buffer->GetHandle(), offset, size](vk::CommandBuffer cmdbuf) { cmdbuf.CopyBuffer(staging, buffer, VkBufferCopy{0, offset, size}); @@ -117,7 +117,7 @@ void VKBufferCache::DownloadBlockData(const Buffer& buffer, std::size_t offset, u8* data) { const auto& staging = staging_pool.GetUnusedBuffer(size, true); scheduler.RequestOutsideRenderPassOperationContext(); - scheduler.Record([staging = *staging.handle, buffer = *buffer->GetHandle(), offset, + scheduler.Record([staging = *staging.handle, buffer = buffer->GetHandle(), offset, size](vk::CommandBuffer cmdbuf) { VkBufferMemoryBarrier barrier; barrier.sType = VK_STRUCTURE_TYPE_BUFFER_MEMORY_BARRIER; @@ -144,7 +144,7 @@ void VKBufferCache::DownloadBlockData(const Buffer& buffer, std::size_t offset, void VKBufferCache::CopyBlock(const Buffer& src, const Buffer& dst, std::size_t src_offset, std::size_t dst_offset, std::size_t size) { scheduler.RequestOutsideRenderPassOperationContext(); - scheduler.Record([src_buffer = *src->GetHandle(), dst_buffer = *dst->GetHandle(), src_offset, + scheduler.Record([src_buffer = src->GetHandle(), dst_buffer = dst->GetHandle(), src_offset, dst_offset, size](vk::CommandBuffer cmdbuf) { cmdbuf.CopyBuffer(src_buffer, dst_buffer, VkBufferCopy{src_offset, dst_offset, size}); diff --git a/src/video_core/renderer_vulkan/vk_buffer_cache.h b/src/video_core/renderer_vulkan/vk_buffer_cache.h index d3c23da98..3cd2e2774 100644 --- a/src/video_core/renderer_vulkan/vk_buffer_cache.h +++ b/src/video_core/renderer_vulkan/vk_buffer_cache.h @@ -33,8 +33,8 @@ public: VAddr cpu_addr, std::size_t size); ~CachedBufferBlock(); - const VkBuffer* GetHandle() const { - return buffer.handle.address(); + VkBuffer GetHandle() const { + return *buffer.handle; } private: @@ -50,15 +50,15 @@ public: VKScheduler& scheduler, VKStagingBufferPool& staging_pool); ~VKBufferCache(); - const VkBuffer* GetEmptyBuffer(std::size_t size) override; + VkBuffer GetEmptyBuffer(std::size_t size) override; protected: + VkBuffer ToHandle(const Buffer& buffer) override; + void WriteBarrier() override {} Buffer CreateBlock(VAddr cpu_addr, std::size_t size) override; - const VkBuffer* ToHandle(const Buffer& buffer) override; - void UploadBlockData(const Buffer& buffer, std::size_t offset, std::size_t size, const u8* data) override; diff --git a/src/video_core/renderer_vulkan/vk_compute_pass.cpp b/src/video_core/renderer_vulkan/vk_compute_pass.cpp index 9d92305f4..878a78755 100644 --- a/src/video_core/renderer_vulkan/vk_compute_pass.cpp +++ b/src/video_core/renderer_vulkan/vk_compute_pass.cpp @@ -343,13 +343,13 @@ QuadArrayPass::QuadArrayPass(const VKDevice& device, VKScheduler& scheduler, QuadArrayPass::~QuadArrayPass() = default; -std::pair QuadArrayPass::Assemble(u32 num_vertices, u32 first) { +std::pair QuadArrayPass::Assemble(u32 num_vertices, u32 first) { const u32 num_triangle_vertices = num_vertices * 6 / 4; const std::size_t staging_size = num_triangle_vertices * sizeof(u32); auto& buffer = staging_buffer_pool.GetUnusedBuffer(staging_size, false); update_descriptor_queue.Acquire(); - update_descriptor_queue.AddBuffer(buffer.handle.address(), 0, staging_size); + update_descriptor_queue.AddBuffer(*buffer.handle, 0, staging_size); const auto set = CommitDescriptorSet(update_descriptor_queue, scheduler.GetFence()); scheduler.RequestOutsideRenderPassOperationContext(); @@ -377,7 +377,7 @@ std::pair QuadArrayPass::Assemble(u32 num_vertice cmdbuf.PipelineBarrier(VK_PIPELINE_STAGE_COMPUTE_SHADER_BIT, VK_PIPELINE_STAGE_VERTEX_INPUT_BIT, 0, {}, {barrier}, {}); }); - return {buffer.handle.address(), 0}; + return {*buffer.handle, 0}; } Uint8Pass::Uint8Pass(const VKDevice& device, VKScheduler& scheduler, @@ -391,14 +391,14 @@ Uint8Pass::Uint8Pass(const VKDevice& device, VKScheduler& scheduler, Uint8Pass::~Uint8Pass() = default; -std::pair Uint8Pass::Assemble(u32 num_vertices, VkBuffer src_buffer, - u64 src_offset) { +std::pair Uint8Pass::Assemble(u32 num_vertices, VkBuffer src_buffer, + u64 src_offset) { const auto staging_size = static_cast(num_vertices * sizeof(u16)); auto& buffer = staging_buffer_pool.GetUnusedBuffer(staging_size, false); update_descriptor_queue.Acquire(); - update_descriptor_queue.AddBuffer(&src_buffer, src_offset, num_vertices); - update_descriptor_queue.AddBuffer(buffer.handle.address(), 0, staging_size); + update_descriptor_queue.AddBuffer(src_buffer, src_offset, num_vertices); + update_descriptor_queue.AddBuffer(*buffer.handle, 0, staging_size); const auto set = CommitDescriptorSet(update_descriptor_queue, scheduler.GetFence()); scheduler.RequestOutsideRenderPassOperationContext(); @@ -422,7 +422,7 @@ std::pair Uint8Pass::Assemble(u32 num_vertices, VkBuffer s cmdbuf.PipelineBarrier(VK_PIPELINE_STAGE_COMPUTE_SHADER_BIT, VK_PIPELINE_STAGE_VERTEX_INPUT_BIT, 0, {}, barrier, {}); }); - return {buffer.handle.address(), 0}; + return {*buffer.handle, 0}; } } // namespace Vulkan diff --git a/src/video_core/renderer_vulkan/vk_compute_pass.h b/src/video_core/renderer_vulkan/vk_compute_pass.h index c62516bff..ec80c8683 100644 --- a/src/video_core/renderer_vulkan/vk_compute_pass.h +++ b/src/video_core/renderer_vulkan/vk_compute_pass.h @@ -50,7 +50,7 @@ public: VKUpdateDescriptorQueue& update_descriptor_queue); ~QuadArrayPass(); - std::pair Assemble(u32 num_vertices, u32 first); + std::pair Assemble(u32 num_vertices, u32 first); private: VKScheduler& scheduler; @@ -65,7 +65,7 @@ public: VKUpdateDescriptorQueue& update_descriptor_queue); ~Uint8Pass(); - std::pair Assemble(u32 num_vertices, VkBuffer src_buffer, u64 src_offset); + std::pair Assemble(u32 num_vertices, VkBuffer src_buffer, u64 src_offset); private: VKScheduler& scheduler; diff --git a/src/video_core/renderer_vulkan/vk_rasterizer.cpp b/src/video_core/renderer_vulkan/vk_rasterizer.cpp index 33cbc0bb6..ab281c9e2 100644 --- a/src/video_core/renderer_vulkan/vk_rasterizer.cpp +++ b/src/video_core/renderer_vulkan/vk_rasterizer.cpp @@ -134,13 +134,13 @@ Tegra::Texture::FullTextureInfo GetTextureInfo(const Engine& engine, const Entry class BufferBindings final { public: - void AddVertexBinding(const VkBuffer* buffer, VkDeviceSize offset) { - vertex.buffer_ptrs[vertex.num_buffers] = buffer; + void AddVertexBinding(VkBuffer buffer, VkDeviceSize offset) { + vertex.buffers[vertex.num_buffers] = buffer; vertex.offsets[vertex.num_buffers] = offset; ++vertex.num_buffers; } - void SetIndexBinding(const VkBuffer* buffer, VkDeviceSize offset, VkIndexType type) { + void SetIndexBinding(VkBuffer buffer, VkDeviceSize offset, VkIndexType type) { index.buffer = buffer; index.offset = offset; index.type = type; @@ -224,19 +224,19 @@ private: // Some of these fields are intentionally left uninitialized to avoid initializing them twice. struct { std::size_t num_buffers = 0; - std::array buffer_ptrs; + std::array buffers; std::array offsets; } vertex; struct { - const VkBuffer* buffer = nullptr; + VkBuffer buffer = nullptr; VkDeviceSize offset; VkIndexType type; } index; template void BindStatic(VKScheduler& scheduler) const { - if (index.buffer != nullptr) { + if (index.buffer) { BindStatic(scheduler); } else { BindStatic(scheduler); @@ -251,18 +251,14 @@ private: } std::array buffers; - std::transform(vertex.buffer_ptrs.begin(), vertex.buffer_ptrs.begin() + N, buffers.begin(), - [](const auto ptr) { return *ptr; }); - std::array offsets; + std::copy(vertex.buffers.begin(), vertex.buffers.begin() + N, buffers.begin()); std::copy(vertex.offsets.begin(), vertex.offsets.begin() + N, offsets.begin()); if constexpr (is_indexed) { // Indexed draw - scheduler.Record([buffers, offsets, index_buffer = *index.buffer, - index_offset = index.offset, - index_type = index.type](vk::CommandBuffer cmdbuf) { - cmdbuf.BindIndexBuffer(index_buffer, index_offset, index_type); + scheduler.Record([buffers, offsets, index = index](vk::CommandBuffer cmdbuf) { + cmdbuf.BindIndexBuffer(index.buffer, index.offset, index.type); cmdbuf.BindVertexBuffers(0, static_cast(N), buffers.data(), offsets.data()); }); } else { @@ -787,7 +783,7 @@ void RasterizerVulkan::BeginTransformFeedback() { const std::size_t size = binding.buffer_size; const auto [buffer, offset] = buffer_cache.UploadMemory(gpu_addr, size, 4, true); - scheduler.Record([buffer = *buffer, offset = offset, size](vk::CommandBuffer cmdbuf) { + scheduler.Record([buffer = buffer, offset = offset, size](vk::CommandBuffer cmdbuf) { cmdbuf.BindTransformFeedbackBuffersEXT(0, 1, &buffer, &offset, &size); cmdbuf.BeginTransformFeedbackEXT(0, 0, nullptr, nullptr); }); @@ -867,7 +863,7 @@ void RasterizerVulkan::SetupIndexBuffer(BufferBindings& buffer_bindings, DrawPar auto format = regs.index_array.format; const bool is_uint8 = format == Maxwell::IndexFormat::UnsignedByte; if (is_uint8 && !device.IsExtIndexTypeUint8Supported()) { - std::tie(buffer, offset) = uint8_pass.Assemble(params.num_vertices, *buffer, offset); + std::tie(buffer, offset) = uint8_pass.Assemble(params.num_vertices, buffer, offset); format = Maxwell::IndexFormat::UnsignedShort; } @@ -1004,8 +1000,8 @@ void RasterizerVulkan::SetupGlobalBuffer(const GlobalBufferEntry& entry, GPUVAdd const auto size = memory_manager.Read(address + 8); if (size == 0) { - // Sometimes global memory pointers don't have a proper size. Upload a dummy entry because - // Vulkan doesn't like empty buffers. + // Sometimes global memory pointers don't have a proper size. Upload a dummy entry + // because Vulkan doesn't like empty buffers. constexpr std::size_t dummy_size = 4; const auto buffer = buffer_cache.GetEmptyBuffer(dummy_size); update_descriptor_queue.AddBuffer(buffer, 0, dummy_size); diff --git a/src/video_core/renderer_vulkan/vk_update_descriptor.cpp b/src/video_core/renderer_vulkan/vk_update_descriptor.cpp index 4bfec0077..681ecde98 100644 --- a/src/video_core/renderer_vulkan/vk_update_descriptor.cpp +++ b/src/video_core/renderer_vulkan/vk_update_descriptor.cpp @@ -35,12 +35,13 @@ void VKUpdateDescriptorQueue::Send(VkDescriptorUpdateTemplateKHR update_template payload.clear(); } + // TODO(Rodrigo): Rework to write the payload directly const auto payload_start = payload.data() + payload.size(); for (const auto& entry : entries) { if (const auto image = std::get_if(&entry)) { payload.push_back(*image); - } else if (const auto buffer = std::get_if(&entry)) { - payload.emplace_back(*buffer->buffer, buffer->offset, buffer->size); + } else if (const auto buffer = std::get_if(&entry)) { + payload.push_back(*buffer); } else if (const auto texel = std::get_if(&entry)) { payload.push_back(*texel); } else { diff --git a/src/video_core/renderer_vulkan/vk_update_descriptor.h b/src/video_core/renderer_vulkan/vk_update_descriptor.h index a9e3d5dba..6ba2c9997 100644 --- a/src/video_core/renderer_vulkan/vk_update_descriptor.h +++ b/src/video_core/renderer_vulkan/vk_update_descriptor.h @@ -18,12 +18,11 @@ class VKScheduler; class DescriptorUpdateEntry { public: - explicit DescriptorUpdateEntry() : image{} {} + explicit DescriptorUpdateEntry() {} DescriptorUpdateEntry(VkDescriptorImageInfo image) : image{image} {} - DescriptorUpdateEntry(VkBuffer buffer, VkDeviceSize offset, VkDeviceSize size) - : buffer{buffer, offset, size} {} + DescriptorUpdateEntry(VkDescriptorBufferInfo buffer) : buffer{buffer} {} DescriptorUpdateEntry(VkBufferView texel_buffer) : texel_buffer{texel_buffer} {} @@ -54,8 +53,8 @@ public: entries.emplace_back(VkDescriptorImageInfo{{}, image_view, {}}); } - void AddBuffer(const VkBuffer* buffer, u64 offset, std::size_t size) { - entries.push_back(Buffer{buffer, offset, size}); + void AddBuffer(VkBuffer buffer, u64 offset, std::size_t size) { + entries.emplace_back(VkDescriptorBufferInfo{buffer, offset, size}); } void AddTexelBuffer(VkBufferView texel_buffer) { @@ -67,12 +66,7 @@ public: } private: - struct Buffer { - const VkBuffer* buffer = nullptr; - u64 offset = 0; - std::size_t size = 0; - }; - using Variant = std::variant; + using Variant = std::variant; const VKDevice& device; VKScheduler& scheduler; -- cgit v1.2.3