From d84a64900fa8b8218c687de7043cff5e1ecab95e Mon Sep 17 00:00:00 2001 From: Jeroen Bakker Date: Thu, 15 Jun 2023 08:14:37 +0200 Subject: [PATCH] Vulkan: Device Context Resource Management The current Vulkan resource management has some issues as context that are not active can still use resources that are freed via another context. When this happens incorrect data can be read on the GPU and even crash Blender. When trying to bind something that now contains other memory pointers. This change introduces that contexts are tracked via the device. Context will be registered/unregistered with the device instance. Unbinding of resources must pass the device and the device will check all registered contexts. Binding of resources will happen via the active context only. On user perspective this now allowes: - Opening/switching files - Switching workspaces - Switching render engines Pull Request: https://projects.blender.org/blender/blender/pulls/108968 --- source/blender/gpu/CMakeLists.txt | 2 + source/blender/gpu/tests/texture_test.cc | 2 + .../blender/gpu/tests/vertex_buffer_test.cc | 7 ++ source/blender/gpu/vulkan/vk_backend.cc | 16 ++- .../gpu/vulkan/vk_bindable_resource.cc | 39 +++++++ .../gpu/vulkan/vk_bindable_resource.hh | 92 ++++++++++++++++ source/blender/gpu/vulkan/vk_buffer.hh | 2 +- source/blender/gpu/vulkan/vk_context.cc | 16 +-- source/blender/gpu/vulkan/vk_context.hh | 8 +- .../blender/gpu/vulkan/vk_descriptor_set.cc | 16 ++- .../blender/gpu/vulkan/vk_descriptor_set.hh | 3 +- source/blender/gpu/vulkan/vk_device.cc | 28 +++++ source/blender/gpu/vulkan/vk_device.hh | 31 ++++++ source/blender/gpu/vulkan/vk_index_buffer.cc | 15 ++- source/blender/gpu/vulkan/vk_index_buffer.hh | 4 +- source/blender/gpu/vulkan/vk_sampler.hh | 2 +- source/blender/gpu/vulkan/vk_state_manager.cc | 104 ++++++------------ source/blender/gpu/vulkan/vk_state_manager.hh | 35 +++--- .../blender/gpu/vulkan/vk_storage_buffer.cc | 33 ++++-- .../blender/gpu/vulkan/vk_storage_buffer.hh | 11 +- source/blender/gpu/vulkan/vk_texture.cc | 37 ++----- source/blender/gpu/vulkan/vk_texture.hh | 13 ++- .../blender/gpu/vulkan/vk_uniform_buffer.cc | 25 +++-- .../blender/gpu/vulkan/vk_uniform_buffer.hh | 17 ++- source/blender/gpu/vulkan/vk_vertex_buffer.cc | 41 +++---- source/blender/gpu/vulkan/vk_vertex_buffer.hh | 8 +- 26 files changed, 404 insertions(+), 203 deletions(-) create mode 100644 source/blender/gpu/vulkan/vk_bindable_resource.cc create mode 100644 source/blender/gpu/vulkan/vk_bindable_resource.hh diff --git a/source/blender/gpu/CMakeLists.txt b/source/blender/gpu/CMakeLists.txt index f7e244d8d32..4eaaada535a 100644 --- a/source/blender/gpu/CMakeLists.txt +++ b/source/blender/gpu/CMakeLists.txt @@ -203,6 +203,7 @@ set(OPENGL_SRC set(VULKAN_SRC vulkan/vk_backend.cc vulkan/vk_batch.cc + vulkan/vk_bindable_resource.cc vulkan/vk_buffer.cc vulkan/vk_command_buffer.cc vulkan/vk_common.cc @@ -239,6 +240,7 @@ set(VULKAN_SRC vulkan/vk_backend.hh vulkan/vk_batch.hh + vulkan/vk_bindable_resource.hh vulkan/vk_buffer.hh vulkan/vk_command_buffer.hh vulkan/vk_common.hh diff --git a/source/blender/gpu/tests/texture_test.cc b/source/blender/gpu/tests/texture_test.cc index bed20fe14af..42b91983992 100644 --- a/source/blender/gpu/tests/texture_test.cc +++ b/source/blender/gpu/tests/texture_test.cc @@ -73,6 +73,8 @@ static void test_texture_cube() float4 clear_color(1.0f, 0.5f, 0.2f, 1.0f); GPU_texture_clear(tex, GPU_DATA_FLOAT, clear_color); + GPU_memory_barrier(GPU_BARRIER_TEXTURE_UPDATE); + float4 *data = (float4 *)GPU_texture_read(tex, GPU_DATA_FLOAT, 0); for (int index : IndexRange(SIZE * SIZE * 6)) { EXPECT_EQ(clear_color, data[index]); diff --git a/source/blender/gpu/tests/vertex_buffer_test.cc b/source/blender/gpu/tests/vertex_buffer_test.cc index 839672d8df4..4fdf03043aa 100644 --- a/source/blender/gpu/tests/vertex_buffer_test.cc +++ b/source/blender/gpu/tests/vertex_buffer_test.cc @@ -4,6 +4,7 @@ #include "testing/testing.h" +#include "GPU_context.h" #include "GPU_framebuffer.h" #include "GPU_immediate.h" #include "GPU_shader.h" @@ -101,12 +102,18 @@ GPU_TEST(vertex_buffer_fetch_mode__GPU_COMP_U16__GPU_FETCH_INT_TO_FLOAT); static void test_vertex_buffer_fetch_mode__GPU_COMP_I32__GPU_FETCH_INT_TO_FLOAT() { + if (GPU_backend_type_selection_get() == GPU_BACKEND_VULKAN) { + GTEST_SKIP() << "interleaved format not supported yet"; + } vertex_buffer_fetch_mode(int4(4, 5, 6, 1)); } GPU_TEST(vertex_buffer_fetch_mode__GPU_COMP_I32__GPU_FETCH_INT_TO_FLOAT); static void test_vertex_buffer_fetch_mode__GPU_COMP_U32__GPU_FETCH_INT_TO_FLOAT() { + if (GPU_backend_type_selection_get() == GPU_BACKEND_VULKAN) { + GTEST_SKIP() << "interleaved format not supported yet"; + } vertex_buffer_fetch_mode(uint4(4, 5, 6, 1)); } GPU_TEST(vertex_buffer_fetch_mode__GPU_COMP_U32__GPU_FETCH_INT_TO_FLOAT); diff --git a/source/blender/gpu/vulkan/vk_backend.cc b/source/blender/gpu/vulkan/vk_backend.cc index 64fa7ff9d68..f3db5054218 100644 --- a/source/blender/gpu/vulkan/vk_backend.cc +++ b/source/blender/gpu/vulkan/vk_backend.cc @@ -6,6 +6,8 @@ * \ingroup gpu */ +#include "GHOST_C-api.h" + #include "gpu_capabilities_private.hh" #include "gpu_platform_private.hh" @@ -121,7 +123,19 @@ void VKBackend::compute_dispatch_indirect(StorageBuf *indirect_buf) Context *VKBackend::context_alloc(void *ghost_window, void *ghost_context) { - return new VKContext(ghost_window, ghost_context); + if (ghost_window) { + BLI_assert(ghost_context == nullptr); + ghost_context = GHOST_GetDrawingContext((GHOST_WindowHandle)ghost_window); + } + + BLI_assert(ghost_context != nullptr); + if (!device_.is_initialized()) { + device_.init(ghost_context); + } + + VKContext *context = new VKContext(ghost_window, ghost_context); + device_.context_register(*context); + return context; } Batch *VKBackend::batch_alloc() diff --git a/source/blender/gpu/vulkan/vk_bindable_resource.cc b/source/blender/gpu/vulkan/vk_bindable_resource.cc new file mode 100644 index 00000000000..343ce9eeafc --- /dev/null +++ b/source/blender/gpu/vulkan/vk_bindable_resource.cc @@ -0,0 +1,39 @@ +/* SPDX-FileCopyrightText: 2023 Blender Foundation + * + * SPDX-License-Identifier: GPL-2.0-or-later */ + +/** \file + * \ingroup gpu + */ + +#include "vk_bindable_resource.hh" +#include "vk_backend.hh" +#include "vk_context.hh" +#include "vk_device.hh" +#include "vk_state_manager.hh" + +namespace blender::gpu { + +VKBindableResource::~VKBindableResource() +{ + unbind_from_all_contexts(); +} + +void VKBindableResource::unbind_from_active_context() +{ + const VKContext *context = VKContext::get(); + if (context != nullptr) { + VKStateManager &state_manager = context->state_manager_get(); + state_manager.unbind_from_all_namespaces(*this); + } +} + +void VKBindableResource::unbind_from_all_contexts() +{ + for (const VKContext &context : VKBackend::get().device_get().contexts_get()) { + VKStateManager &state_manager = context.state_manager_get(); + state_manager.unbind_from_all_namespaces(*this); + } +} + +} // namespace blender::gpu \ No newline at end of file diff --git a/source/blender/gpu/vulkan/vk_bindable_resource.hh b/source/blender/gpu/vulkan/vk_bindable_resource.hh new file mode 100644 index 00000000000..5030164e6b4 --- /dev/null +++ b/source/blender/gpu/vulkan/vk_bindable_resource.hh @@ -0,0 +1,92 @@ +/* SPDX-FileCopyrightText: 2023 Blender Foundation + * + * SPDX-License-Identifier: GPL-2.0-or-later */ + +/** \file + * \ingroup gpu + */ + +#pragma once + +#include "BLI_array.hh" + +#include "gpu_shader_create_info.hh" + +namespace blender::gpu { + +/** + * Super class for resources that can be bound to a shader. + */ +class VKBindableResource { + protected: + virtual ~VKBindableResource(); + + public: + /** + * Bind the resource to the shader. + */ + virtual void bind(int binding, shader::ShaderCreateInfo::Resource::BindType bind_type) = 0; + + protected: + void unbind_from_active_context(); + + private: + void unbind_from_all_contexts(); +}; + +/** + * Blender binds resources at context level (VKStateManager). The bindings are organized in + * namespaces. + */ +template +class VKBindSpace { + Array bindings_ = Array(MaxBindings); + + public: + VKBindSpace() + { + bindings_.fill(nullptr); + } + + /** + * Register a binding to this namespace. + */ + void bind(int binding, VKBindableResource &resource) + { + bindings_[binding] = &resource; + } + + /** + * Apply registered bindings to the active shader. + */ + void apply_bindings() + { + for (int binding : IndexRange(MaxBindings)) { + if (bindings_[binding] != nullptr) { + bindings_[binding]->bind(binding, BindType); + } + } + } + + /** + * Unregister the given resource from this namespace. + */ + void unbind(VKBindableResource &resource) + { + for (int binding : IndexRange(MaxBindings)) { + if (bindings_[binding] == &resource) { + bindings_[binding] = nullptr; + } + } + } + + /** + * Remove all bindings from this namespace. + */ + void unbind_all() + { + bindings_.fill(nullptr); + } +}; + +} // namespace blender::gpu diff --git a/source/blender/gpu/vulkan/vk_buffer.hh b/source/blender/gpu/vulkan/vk_buffer.hh index 1919999f3e6..4d7560f2890 100644 --- a/source/blender/gpu/vulkan/vk_buffer.hh +++ b/source/blender/gpu/vulkan/vk_buffer.hh @@ -19,7 +19,7 @@ class VKContext; * Class for handing vulkan buffers (allocation/updating/binding). */ class VKBuffer { - int64_t size_in_bytes_; + int64_t size_in_bytes_ = 0; VkBuffer vk_buffer_ = VK_NULL_HANDLE; VmaAllocation allocation_ = VK_NULL_HANDLE; /* Pointer to the virtually mapped memory. */ diff --git a/source/blender/gpu/vulkan/vk_context.cc b/source/blender/gpu/vulkan/vk_context.cc index 0699cc6576d..8302d38e842 100644 --- a/source/blender/gpu/vulkan/vk_context.cc +++ b/source/blender/gpu/vulkan/vk_context.cc @@ -23,14 +23,7 @@ namespace blender::gpu { VKContext::VKContext(void *ghost_window, void *ghost_context) { ghost_window_ = ghost_window; - if (ghost_window) { - ghost_context = GHOST_GetDrawingContext((GHOST_WindowHandle)ghost_window); - } ghost_context_ = ghost_context; - VKDevice &device = VKBackend::get().device_; - if (!device.is_initialized()) { - device.init(ghost_context); - } state_manager = new VKStateManager(); imm = new VKImmediate(); @@ -43,6 +36,8 @@ VKContext::VKContext(void *ghost_window, void *ghost_context) VKContext::~VKContext() { + VKBackend::get().device_.context_unregister(*this); + delete imm; imm = nullptr; } @@ -130,12 +125,7 @@ void VKContext::memory_statistics_get(int * /*total_mem*/, int * /*free_mem*/) { /** \name State manager * \{ */ -const VKStateManager &VKContext::state_manager_get() const -{ - return *static_cast(state_manager); -} - -VKStateManager &VKContext::state_manager_get() +VKStateManager &VKContext::state_manager_get() const { return *static_cast(state_manager); } diff --git a/source/blender/gpu/vulkan/vk_context.hh b/source/blender/gpu/vulkan/vk_context.hh index 0679f227a03..be6602bd564 100644 --- a/source/blender/gpu/vulkan/vk_context.hh +++ b/source/blender/gpu/vulkan/vk_context.hh @@ -68,8 +68,12 @@ class VKContext : public Context, NonCopyable { return command_buffer_; } - const VKStateManager &state_manager_get() const; - VKStateManager &state_manager_get(); + VKStateManager &state_manager_get() const; }; +BLI_INLINE bool operator==(const VKContext &a, const VKContext &b) +{ + return static_cast(&a) == static_cast(&b); +} + } // namespace blender::gpu diff --git a/source/blender/gpu/vulkan/vk_descriptor_set.cc b/source/blender/gpu/vulkan/vk_descriptor_set.cc index 3698aecd6fa..d11ad91a057 100644 --- a/source/blender/gpu/vulkan/vk_descriptor_set.cc +++ b/source/blender/gpu/vulkan/vk_descriptor_set.cc @@ -77,6 +77,15 @@ void VKDescriptorSetTracker::bind_as_ssbo(VKIndexBuffer &buffer, binding.buffer_size = buffer.size_get(); } +void VKDescriptorSetTracker::bind_as_ssbo(VKUniformBuffer &buffer, + const VKDescriptorSet::Location location) +{ + Binding &binding = ensure_location(location); + binding.type = VK_DESCRIPTOR_TYPE_STORAGE_BUFFER; + binding.vk_buffer = buffer.vk_handle(); + binding.buffer_size = buffer.size_in_bytes(); +} + void VKDescriptorSetTracker::image_bind(VKTexture &texture, const VKDescriptorSet::Location location) { @@ -87,7 +96,7 @@ void VKDescriptorSetTracker::image_bind(VKTexture &texture, void VKDescriptorSetTracker::bind(VKTexture &texture, const VKDescriptorSet::Location location, - VKSampler &sampler) + const VKSampler &sampler) { Binding &binding = ensure_location(location); binding.type = VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER; @@ -170,8 +179,9 @@ void VKDescriptorSetTracker::update(VKContext &context) if (!binding.is_image()) { continue; } - /* When updating the descriptor sets the layout of the texture should already be updated. */ - binding.texture->layout_ensure(context, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL); + /* TODO: Based on the actual usage we should use + * VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL/VK_IMAGE_LAYOUT_GENERAL. */ + binding.texture->layout_ensure(context, VK_IMAGE_LAYOUT_GENERAL); VkDescriptorImageInfo image_info = {}; image_info.sampler = binding.vk_sampler; image_info.imageView = binding.texture->image_view_get().vk_handle(); diff --git a/source/blender/gpu/vulkan/vk_descriptor_set.hh b/source/blender/gpu/vulkan/vk_descriptor_set.hh index 454c03653ac..f559553c0a5 100644 --- a/source/blender/gpu/vulkan/vk_descriptor_set.hh +++ b/source/blender/gpu/vulkan/vk_descriptor_set.hh @@ -162,11 +162,12 @@ class VKDescriptorSetTracker : protected VKResourceTracker { void bind_as_ssbo(VKVertexBuffer &buffer, VKDescriptorSet::Location location); void bind_as_ssbo(VKIndexBuffer &buffer, VKDescriptorSet::Location location); + void bind_as_ssbo(VKUniformBuffer &buffer, VKDescriptorSet::Location location); void bind(VKStorageBuffer &buffer, VKDescriptorSet::Location location); void bind(VKUniformBuffer &buffer, VKDescriptorSet::Location location); /* TODO: bind as image */ void image_bind(VKTexture &texture, VKDescriptorSet::Location location); - void bind(VKTexture &texture, VKDescriptorSet::Location location, VKSampler &sampler); + void bind(VKTexture &texture, VKDescriptorSet::Location location, const VKSampler &sampler); /* Bind as uniform texel buffer. */ void bind(VKVertexBuffer &vertex_buffer, VKDescriptorSet::Location location); /** diff --git a/source/blender/gpu/vulkan/vk_device.cc b/source/blender/gpu/vulkan/vk_device.cc index ee36c5d097a..8550c21ff23 100644 --- a/source/blender/gpu/vulkan/vk_device.cc +++ b/source/blender/gpu/vulkan/vk_device.cc @@ -8,7 +8,12 @@ #include "vk_device.hh" #include "vk_backend.hh" +#include "vk_context.hh" #include "vk_memory.hh" +#include "vk_state_manager.hh" +#include "vk_storage_buffer.hh" +#include "vk_texture.hh" +#include "vk_vertex_buffer.hh" #include "GHOST_C-api.h" @@ -16,6 +21,7 @@ namespace blender::gpu { void VKDevice::deinit() { + sampler_.free(); vmaDestroyAllocator(mem_allocator_); mem_allocator_ = VK_NULL_HANDLE; debugging_tools_.deinit(vk_instance_); @@ -50,6 +56,8 @@ void VKDevice::init(void *ghost_context) init_memory_allocator(); init_descriptor_pools(); + sampler_.create(); + debug::object_label(device_get(), "LogicalDevice"); debug::object_label(queue_get(), "GenericQueue"); } @@ -178,4 +186,24 @@ std::string VKDevice::driver_version() const /** \} */ +/* -------------------------------------------------------------------- */ +/** \name Resource management + * \{ */ + +void VKDevice::context_register(VKContext &context) +{ + contexts_.append(std::reference_wrapper(context)); +} + +void VKDevice::context_unregister(VKContext &context) +{ + contexts_.remove(contexts_.first_index_of(std::reference_wrapper(context))); +} +const Vector> &VKDevice::contexts_get() const +{ + return contexts_; +}; + +/** \} */ + } // namespace blender::gpu diff --git a/source/blender/gpu/vulkan/vk_device.hh b/source/blender/gpu/vulkan/vk_device.hh index dac8a713556..3b2af3a1e7a 100644 --- a/source/blender/gpu/vulkan/vk_device.hh +++ b/source/blender/gpu/vulkan/vk_device.hh @@ -9,10 +9,12 @@ #pragma once #include "BLI_utility_mixins.hh" +#include "BLI_vector.hh" #include "vk_common.hh" #include "vk_debug.hh" #include "vk_descriptor_pools.hh" +#include "vk_sampler.hh" namespace blender::gpu { class VKBackend; @@ -36,6 +38,20 @@ class VKDevice : public NonCopyable { uint32_t vk_queue_family_ = 0; VkQueue vk_queue_ = VK_NULL_HANDLE; + /* Dummy sampler for now. */ + VKSampler sampler_; + + /** + * Available Contexts for this device. + * + * Device keeps track of each contexts. When buffers/images are freed they need to be removed + * from all contexts state managers. + * + * The contexts inside this list aren't owned by the VKDevice. Caller of `GPU_context_create` + * holds the ownership. + */ + Vector> contexts_; + /** Allocator used for texture and buffers and other resources. */ VmaAllocator mem_allocator_ = VK_NULL_HANDLE; VKDescriptorPools descriptor_pools_; @@ -100,6 +116,11 @@ class VKDevice : public NonCopyable { return debugging_tools_; } + const VKSampler &sampler_get() const + { + return sampler_; + } + bool is_initialized() const; void init(void *ghost_context); void deinit(); @@ -114,6 +135,16 @@ class VKDevice : public NonCopyable { return workarounds_; } + /* -------------------------------------------------------------------- */ + /** \name Resource management + * \{ */ + + void context_register(VKContext &context); + void context_unregister(VKContext &context); + const Vector> &contexts_get() const; + + /** \} */ + private: void init_physical_device_properties(); void init_debug_callbacks(); diff --git a/source/blender/gpu/vulkan/vk_index_buffer.cc b/source/blender/gpu/vulkan/vk_index_buffer.cc index 3fa8a2ea30f..ca681b99b99 100644 --- a/source/blender/gpu/vulkan/vk_index_buffer.cc +++ b/source/blender/gpu/vulkan/vk_index_buffer.cc @@ -9,6 +9,7 @@ #include "vk_index_buffer.hh" #include "vk_shader.hh" #include "vk_shader_interface.hh" +#include "vk_state_manager.hh" namespace blender::gpu { @@ -41,16 +42,22 @@ void VKIndexBuffer::bind(VKContext &context) void VKIndexBuffer::bind_as_ssbo(uint binding) { + VKContext::get()->state_manager_get().storage_buffer_bind(*this, binding); +} + +void VKIndexBuffer::bind(int binding, shader::ShaderCreateInfo::Resource::BindType bind_type) +{ + BLI_assert(bind_type == shader::ShaderCreateInfo::Resource::BindType::STORAGE_BUFFER); ensure_updated(); VKContext &context = *VKContext::get(); VKShader *shader = static_cast(context.shader); const VKShaderInterface &shader_interface = shader->interface_get(); const std::optional location = - shader_interface.descriptor_set_location( - shader::ShaderCreateInfo::Resource::BindType::STORAGE_BUFFER, binding); - BLI_assert_msg(location, "Locations to SSBOs should always exist."); - shader->pipeline_get().descriptor_set_get().bind_as_ssbo(*this, *location); + shader_interface.descriptor_set_location(bind_type, binding); + if (location) { + shader->pipeline_get().descriptor_set_get().bind_as_ssbo(*this, *location); + } } void VKIndexBuffer::read(uint32_t *data) const diff --git a/source/blender/gpu/vulkan/vk_index_buffer.hh b/source/blender/gpu/vulkan/vk_index_buffer.hh index 372cc9b9040..d1bdd1e4df1 100644 --- a/source/blender/gpu/vulkan/vk_index_buffer.hh +++ b/source/blender/gpu/vulkan/vk_index_buffer.hh @@ -10,11 +10,12 @@ #include "gpu_index_buffer_private.hh" +#include "vk_bindable_resource.hh" #include "vk_buffer.hh" namespace blender::gpu { -class VKIndexBuffer : public IndexBuf { +class VKIndexBuffer : public IndexBuf, public VKBindableResource { VKBuffer buffer_; public: @@ -22,6 +23,7 @@ class VKIndexBuffer : public IndexBuf { void bind_as_ssbo(uint binding) override; void bind(VKContext &context); + void bind(int binding, shader::ShaderCreateInfo::Resource::BindType bind_type) override; void read(uint32_t *data) const override; diff --git a/source/blender/gpu/vulkan/vk_sampler.hh b/source/blender/gpu/vulkan/vk_sampler.hh index 6914a3c87ca..a238f529dc0 100644 --- a/source/blender/gpu/vulkan/vk_sampler.hh +++ b/source/blender/gpu/vulkan/vk_sampler.hh @@ -25,7 +25,7 @@ class VKSampler : public NonCopyable { void create(); void free(); - VkSampler vk_handle() + VkSampler vk_handle() const { BLI_assert(vk_sampler_ != VK_NULL_HANDLE); return vk_sampler_; diff --git a/source/blender/gpu/vulkan/vk_state_manager.cc b/source/blender/gpu/vulkan/vk_state_manager.cc index 8f20cea834f..525afaa7327 100644 --- a/source/blender/gpu/vulkan/vk_state_manager.cc +++ b/source/blender/gpu/vulkan/vk_state_manager.cc @@ -8,8 +8,10 @@ #include "vk_state_manager.hh" #include "vk_context.hh" +#include "vk_index_buffer.hh" #include "vk_pipeline.hh" #include "vk_shader.hh" +#include "vk_storage_buffer.hh" #include "vk_texture.hh" #include "vk_vertex_buffer.hh" @@ -17,18 +19,6 @@ namespace blender::gpu { -VKStateManager::VKStateManager() -{ - sampler_.create(); - constexpr int max_bindings = 16; - image_bindings_ = Array(max_bindings); - image_bindings_.fill(ImageBinding()); - texture_bindings_ = Array(max_bindings); - texture_bindings_.fill(TextureBinding()); - uniform_buffer_bindings_ = Array(max_bindings); - uniform_buffer_bindings_.fill(UniformBufferBinding()); -} - void VKStateManager::apply_state() { VKContext &context = *VKContext::get(); @@ -43,27 +33,10 @@ void VKStateManager::apply_bindings() { VKContext &context = *VKContext::get(); if (context.shader) { - for (int binding : IndexRange(image_bindings_.size())) { - if (image_bindings_[binding].texture) { - image_bindings_[binding].texture->image_bind(binding); - } - } - - for (int binding : IndexRange(texture_bindings_.size())) { - if (texture_bindings_[binding].texture) { - texture_bindings_[binding].texture->bind(binding, sampler_); - } - else if (texture_bindings_[binding].vertex_buffer) { - texture_bindings_[binding].vertex_buffer->bind(binding); - } - } - - for (int binding : IndexRange(uniform_buffer_bindings_.size())) { - if (uniform_buffer_bindings_[binding].buffer) { - uniform_buffer_bindings_[binding].buffer->bind( - binding, shader::ShaderCreateInfo::Resource::BindType::UNIFORM_BUFFER); - } - } + textures_.apply_bindings(); + images_.apply_bindings(); + uniform_buffers_.apply_bindings(); + storage_buffers_.apply_bindings(); } } @@ -88,78 +61,73 @@ void VKStateManager::issue_barrier(eGPUBarrier /*barrier_bits*/) void VKStateManager::texture_bind(Texture *tex, GPUSamplerState /*sampler*/, int unit) { VKTexture *texture = unwrap(tex); - texture_bindings_[unit].texture = texture; - texture_bindings_[unit].vertex_buffer = nullptr; + textures_.bind(unit, *texture); } void VKStateManager::texture_unbind(Texture *tex) { VKTexture *texture = unwrap(tex); - for (TextureBinding &binding : texture_bindings_) { - if (binding.texture == texture) { - binding.texture = nullptr; - } - } + textures_.unbind(*texture); } void VKStateManager::texture_unbind_all() { - for (TextureBinding &binding : texture_bindings_) { - binding.texture = nullptr; - } + textures_.unbind_all(); } void VKStateManager::image_bind(Texture *tex, int binding) { VKTexture *texture = unwrap(tex); - image_bindings_[binding].texture = texture; + images_.bind(binding, *texture); } void VKStateManager::image_unbind(Texture *tex) { VKTexture *texture = unwrap(tex); - for (ImageBinding &binding : image_bindings_) { - if (binding.texture == texture) { - binding.texture = nullptr; - } - } + images_.unbind(*texture); } void VKStateManager::image_unbind_all() { - for (ImageBinding &binding : image_bindings_) { - binding.texture = nullptr; - } + images_.unbind_all(); } void VKStateManager::uniform_buffer_bind(VKUniformBuffer *uniform_buffer, int slot) { - uniform_buffer_bindings_[slot].buffer = uniform_buffer; + uniform_buffers_.bind(slot, *uniform_buffer); } void VKStateManager::uniform_buffer_unbind(VKUniformBuffer *uniform_buffer) { - for (UniformBufferBinding &binding : uniform_buffer_bindings_) { - if (binding.buffer == uniform_buffer) { - binding.buffer = nullptr; - } - } + uniform_buffers_.unbind(*uniform_buffer); } -void VKStateManager::texel_buffer_bind(VKVertexBuffer *vertex_buffer, int slot) +void VKStateManager::unbind_from_all_namespaces(VKBindableResource &resource) { - texture_bindings_[slot].vertex_buffer = vertex_buffer; - texture_bindings_[slot].texture = nullptr; + uniform_buffers_.unbind(resource); + storage_buffers_.unbind(resource); + images_.unbind(resource); + textures_.unbind(resource); } -void VKStateManager::texel_buffer_unbind(VKVertexBuffer *vertex_buffer) +void VKStateManager::texel_buffer_bind(VKVertexBuffer &vertex_buffer, int slot) { - for (TextureBinding &binding : texture_bindings_) { - if (binding.vertex_buffer == vertex_buffer) { - binding.vertex_buffer = nullptr; - binding.texture = nullptr; - } - } + textures_.bind(slot, vertex_buffer); +} + +void VKStateManager::texel_buffer_unbind(VKVertexBuffer &vertex_buffer) +{ + textures_.unbind(vertex_buffer); +} + +void VKStateManager::storage_buffer_bind(VKBindableResource &resource, int slot) +{ + storage_buffers_.bind(slot, resource); +} + +void VKStateManager::storage_buffer_unbind(VKBindableResource &resource) +{ + storage_buffers_.unbind(resource); } void VKStateManager::texture_unpack_row_length_set(uint len) diff --git a/source/blender/gpu/vulkan/vk_state_manager.hh b/source/blender/gpu/vulkan/vk_state_manager.hh index 5b3caa9f5c5..50fa4dafa9f 100644 --- a/source/blender/gpu/vulkan/vk_state_manager.hh +++ b/source/blender/gpu/vulkan/vk_state_manager.hh @@ -12,37 +12,25 @@ #include "BLI_array.hh" -#include "vk_sampler.hh" +#include "vk_bindable_resource.hh" namespace blender::gpu { class VKTexture; class VKUniformBuffer; class VKVertexBuffer; +class VKStorageBuffer; +class VKIndexBuffer; class VKStateManager : public StateManager { - /* Dummy sampler for now. */ - VKSampler sampler_; uint texture_unpack_row_length_ = 0; - struct TextureBinding { - VKTexture *texture = nullptr; - /* bufferTextures and samplers share the same namespace. */ - VKVertexBuffer *vertex_buffer = nullptr; - }; - struct ImageBinding { - VKTexture *texture = nullptr; - }; - struct UniformBufferBinding { - VKUniformBuffer *buffer = nullptr; - }; - Array image_bindings_; - Array texture_bindings_; - Array uniform_buffer_bindings_; + VKBindSpace textures_; + VKBindSpace images_; + VKBindSpace uniform_buffers_; + VKBindSpace storage_buffers_; public: - VKStateManager(); - void apply_state() override; void force_state() override; @@ -62,8 +50,13 @@ class VKStateManager : public StateManager { void uniform_buffer_bind(VKUniformBuffer *uniform_buffer, int slot); void uniform_buffer_unbind(VKUniformBuffer *uniform_buffer); - void texel_buffer_bind(VKVertexBuffer *vertex_buffer, int slot); - void texel_buffer_unbind(VKVertexBuffer *vertex_buffer); + void texel_buffer_bind(VKVertexBuffer &vertex_buffer, int slot); + void texel_buffer_unbind(VKVertexBuffer &vertex_buffer); + + void storage_buffer_bind(VKBindableResource &resource, int slot); + void storage_buffer_unbind(VKBindableResource &resource); + + void unbind_from_all_namespaces(VKBindableResource &bindable_resource); void texture_unpack_row_length_set(uint len) override; diff --git a/source/blender/gpu/vulkan/vk_storage_buffer.cc b/source/blender/gpu/vulkan/vk_storage_buffer.cc index 3af57afe959..9e3f11716f7 100644 --- a/source/blender/gpu/vulkan/vk_storage_buffer.cc +++ b/source/blender/gpu/vulkan/vk_storage_buffer.cc @@ -7,12 +7,18 @@ */ #include "vk_shader.hh" #include "vk_shader_interface.hh" +#include "vk_state_manager.hh" #include "vk_vertex_buffer.hh" #include "vk_storage_buffer.hh" namespace blender::gpu { +VKStorageBuffer::VKStorageBuffer(int size, GPUUsageType usage, const char *name) + : StorageBuf(size, name), usage_(usage) +{ +} + void VKStorageBuffer::update(const void *data) { ensure_allocated(); @@ -38,18 +44,27 @@ void VKStorageBuffer::allocate() void VKStorageBuffer::bind(int slot) { - ensure_allocated(); VKContext &context = *VKContext::get(); - VKShader *shader = static_cast(context.shader); - const VKShaderInterface &shader_interface = shader->interface_get(); - const std::optional location = - shader_interface.descriptor_set_location( - shader::ShaderCreateInfo::Resource::BindType::STORAGE_BUFFER, slot); - BLI_assert_msg(location, "Locations to SSBOs should always exist."); - shader->pipeline_get().descriptor_set_get().bind(*this, *location); + context.state_manager_get().storage_buffer_bind(*this, slot); } -void VKStorageBuffer::unbind() {} +void VKStorageBuffer::bind(int slot, shader::ShaderCreateInfo::Resource::BindType bind_type) +{ + VKContext &context = *VKContext::get(); + VKShader *shader = static_cast(context.shader); + ensure_allocated(); + const VKShaderInterface &shader_interface = shader->interface_get(); + const std::optional location = + shader_interface.descriptor_set_location(bind_type, slot); + if (location) { + shader->pipeline_get().descriptor_set_get().bind(*this, *location); + } +} + +void VKStorageBuffer::unbind() +{ + unbind_from_active_context(); +} void VKStorageBuffer::clear(uint32_t clear_value) { diff --git a/source/blender/gpu/vulkan/vk_storage_buffer.hh b/source/blender/gpu/vulkan/vk_storage_buffer.hh index a807916c113..71fd4a8dce1 100644 --- a/source/blender/gpu/vulkan/vk_storage_buffer.hh +++ b/source/blender/gpu/vulkan/vk_storage_buffer.hh @@ -13,23 +13,22 @@ #include "gpu_storage_buffer_private.hh" #include "gpu_vertex_buffer_private.hh" +#include "vk_bindable_resource.hh" #include "vk_buffer.hh" namespace blender::gpu { class VertBuf; -class VKStorageBuffer : public StorageBuf { +class VKStorageBuffer : public StorageBuf, public VKBindableResource { GPUUsageType usage_; VKBuffer buffer_; public: - VKStorageBuffer(int size, GPUUsageType usage, const char *name) - : StorageBuf(size, name), usage_(usage) - { - } + VKStorageBuffer(int size, GPUUsageType usage, const char *name); void update(const void *data) override; void bind(int slot) override; + void bind(int slot, shader::ShaderCreateInfo::Resource::BindType bind_type) override; void unbind() override; void clear(uint32_t clear_value) override; void copy_sub(VertBuf *src, uint dst_offset, uint src_offset, uint copy_size) override; @@ -51,7 +50,7 @@ class VKStorageBuffer : public StorageBuf { void allocate(); }; -static inline VKStorageBuffer *unwrap(StorageBuf *storage_buffer) +BLI_INLINE VKStorageBuffer *unwrap(StorageBuf *storage_buffer) { return static_cast(storage_buffer); } diff --git a/source/blender/gpu/vulkan/vk_texture.cc b/source/blender/gpu/vulkan/vk_texture.cc index 4c8f0223e49..c7f05dc2db3 100644 --- a/source/blender/gpu/vulkan/vk_texture.cc +++ b/source/blender/gpu/vulkan/vk_texture.cc @@ -177,8 +177,7 @@ void VKTexture::read_sub(int mip, eGPUDataFormat format, const int area[4], void size_t sample_len = area[2] * area[3] * layer_count(); size_t device_memory_size = sample_len * to_bytesize(format_); - staging_buffer.create( - device_memory_size, GPU_USAGE_DEVICE_ONLY, VK_BUFFER_USAGE_TRANSFER_DST_BIT); + staging_buffer.create(device_memory_size, GPU_USAGE_DYNAMIC, VK_BUFFER_USAGE_TRANSFER_DST_BIT); VkBufferImageCopy region = {}; region.imageOffset.x = area[0]; @@ -224,8 +223,7 @@ void VKTexture::update_sub( size_t sample_len = extent.x * extent.y * extent.z; size_t device_memory_size = sample_len * to_bytesize(format_); - staging_buffer.create( - device_memory_size, GPU_USAGE_DEVICE_ONLY, VK_BUFFER_USAGE_TRANSFER_SRC_BIT); + staging_buffer.create(device_memory_size, GPU_USAGE_DYNAMIC, VK_BUFFER_USAGE_TRANSFER_SRC_BIT); uint buffer_row_length = context.state_manager_get().texture_unpack_row_length_get(); if (buffer_row_length) { @@ -463,8 +461,7 @@ bool VKTexture::allocate() return result == VK_SUCCESS; } -/* TODO: move texture/image bindings to shader. */ -void VKTexture::bind(int unit, VKSampler &sampler) +void VKTexture::bind(int binding, shader::ShaderCreateInfo::Resource::BindType bind_type) { if (!is_allocated()) { allocate(); @@ -473,28 +470,16 @@ void VKTexture::bind(int unit, VKSampler &sampler) VKShader *shader = static_cast(context.shader); const VKShaderInterface &shader_interface = shader->interface_get(); const std::optional location = - shader_interface.descriptor_set_location( - shader::ShaderCreateInfo::Resource::BindType::SAMPLER, unit); + shader_interface.descriptor_set_location(bind_type, binding); if (location) { VKDescriptorSetTracker &descriptor_set = shader->pipeline_get().descriptor_set_get(); - descriptor_set.bind(*this, *location, sampler); - } -} - -void VKTexture::image_bind(int binding) -{ - if (!is_allocated()) { - allocate(); - } - VKContext &context = *VKContext::get(); - VKShader *shader = static_cast(context.shader); - const VKShaderInterface &shader_interface = shader->interface_get(); - const std::optional location = - shader_interface.descriptor_set_location(shader::ShaderCreateInfo::Resource::BindType::IMAGE, - binding); - if (location) { - VKDescriptorSetTracker &descriptor_set = shader->pipeline_get().descriptor_set_get(); - descriptor_set.image_bind(*this, *location); + if (bind_type == shader::ShaderCreateInfo::Resource::BindType::IMAGE) { + descriptor_set.image_bind(*this, *location); + } + else { + const VKDevice &device = VKBackend::get().device_get(); + descriptor_set.bind(*this, *location, device.sampler_get()); + } } } diff --git a/source/blender/gpu/vulkan/vk_texture.hh b/source/blender/gpu/vulkan/vk_texture.hh index 57c1156a87e..ae248da5f9b 100644 --- a/source/blender/gpu/vulkan/vk_texture.hh +++ b/source/blender/gpu/vulkan/vk_texture.hh @@ -10,6 +10,7 @@ #include "gpu_texture_private.hh" +#include "vk_bindable_resource.hh" #include "vk_context.hh" #include "vk_image_view.hh" @@ -17,7 +18,7 @@ namespace blender::gpu { class VKSampler; -class VKTexture : public Texture { +class VKTexture : public Texture, public VKBindableResource { VkImage vk_image_ = VK_NULL_HANDLE; VmaAllocation allocation_ = VK_NULL_HANDLE; @@ -60,8 +61,7 @@ class VKTexture : public Texture { /* TODO(fclem): Legacy. Should be removed at some point. */ uint gl_bindcode_get() const override; - void bind(int unit, VKSampler &sampler); - void image_bind(int location); + void bind(int location, shader::ShaderCreateInfo::Resource::BindType bind_type) override; VkImage vk_image_handle() const { @@ -149,9 +149,14 @@ class VKTexture : public Texture { /** \} */ }; -static inline VKTexture *unwrap(Texture *tex) +BLI_INLINE VKTexture *unwrap(Texture *tex) { return static_cast(tex); } +BLI_INLINE Texture *wrap(VKTexture *texture) +{ + return static_cast(texture); +} + } // namespace blender::gpu diff --git a/source/blender/gpu/vulkan/vk_uniform_buffer.cc b/source/blender/gpu/vulkan/vk_uniform_buffer.cc index 11d881f10b4..da98dbc1a62 100644 --- a/source/blender/gpu/vulkan/vk_uniform_buffer.cc +++ b/source/blender/gpu/vulkan/vk_uniform_buffer.cc @@ -14,11 +14,6 @@ namespace blender::gpu { -VKUniformBuffer::~VKUniformBuffer() -{ - unbind(); -} - void VKUniformBuffer::update(const void *data) { if (!buffer_.is_allocated()) { @@ -55,27 +50,35 @@ void VKUniformBuffer::bind(int slot, shader::ShaderCreateInfo::Resource::BindTyp shader_interface.descriptor_set_location(bind_type, slot); if (location) { VKDescriptorSetTracker &descriptor_set = shader->pipeline_get().descriptor_set_get(); - descriptor_set.bind(*this, *location); + /* TODO: move to descriptor set. */ + if (bind_type == shader::ShaderCreateInfo::Resource::BindType::UNIFORM_BUFFER) { + descriptor_set.bind(*this, *location); + } + else { + descriptor_set.bind_as_ssbo(*this, *location); + } } } void VKUniformBuffer::bind(int slot) { - /* Uniform buffers can be bound without an shader. */ VKContext &context = *VKContext::get(); context.state_manager_get().uniform_buffer_bind(this, slot); } void VKUniformBuffer::bind_as_ssbo(int slot) { - bind(slot, shader::ShaderCreateInfo::Resource::BindType::STORAGE_BUFFER); + VKContext &context = *VKContext::get(); + context.state_manager_get().storage_buffer_bind(*this, slot); } void VKUniformBuffer::unbind() { - VKContext *context = VKContext::get(); - if (context) { - context->state_manager_get().uniform_buffer_unbind(this); + const VKContext *context = VKContext::get(); + if (context != nullptr) { + VKStateManager &state_manager = context->state_manager_get(); + state_manager.uniform_buffer_unbind(this); + state_manager.storage_buffer_unbind(*this); } } diff --git a/source/blender/gpu/vulkan/vk_uniform_buffer.hh b/source/blender/gpu/vulkan/vk_uniform_buffer.hh index 619258fca8f..6b7c440d90c 100644 --- a/source/blender/gpu/vulkan/vk_uniform_buffer.hh +++ b/source/blender/gpu/vulkan/vk_uniform_buffer.hh @@ -12,22 +12,25 @@ #include "gpu_uniform_buffer_private.hh" +#include "vk_bindable_resource.hh" #include "vk_buffer.hh" namespace blender::gpu { -class VKUniformBuffer : public UniformBuf, NonCopyable { +class VKUniformBuffer : public UniformBuf, public VKBindableResource, NonCopyable { VKBuffer buffer_; public: VKUniformBuffer(int size, const char *name) : UniformBuf(size, name) {} - ~VKUniformBuffer(); void update(const void *data) override; void clear_to_zero() override; void bind(int slot) override; void bind_as_ssbo(int slot) override; - void bind(int slot, shader::ShaderCreateInfo::Resource::BindType bind_type); + + /** + * Unbind uniform buffer from active context. + */ void unbind() override; VkBuffer vk_handle() const @@ -40,8 +43,16 @@ class VKUniformBuffer : public UniformBuf, NonCopyable { return size_in_bytes_; } + /* Bindable resource */ + void bind(int binding, shader::ShaderCreateInfo::Resource::BindType bind_type) override; + private: void allocate(); }; +BLI_INLINE UniformBuf *wrap(VKUniformBuffer *uniform_buffer) +{ + return static_cast(uniform_buffer); +} + } // namespace blender::gpu diff --git a/source/blender/gpu/vulkan/vk_vertex_buffer.cc b/source/blender/gpu/vulkan/vk_vertex_buffer.cc index fd824253f35..f25ab6e5001 100644 --- a/source/blender/gpu/vulkan/vk_vertex_buffer.cc +++ b/source/blender/gpu/vulkan/vk_vertex_buffer.cc @@ -24,43 +24,34 @@ VKVertexBuffer::~VKVertexBuffer() void VKVertexBuffer::bind_as_ssbo(uint binding) { - if (!buffer_.is_allocated()) { - allocate(); - } - VKContext &context = *VKContext::get(); - VKShader *shader = static_cast(context.shader); - const VKShaderInterface &shader_interface = shader->interface_get(); - const std::optional location = - shader_interface.descriptor_set_location( - shader::ShaderCreateInfo::Resource::BindType::STORAGE_BUFFER, binding); - BLI_assert_msg(location, "Locations to SSBOs should always exist."); - shader->pipeline_get().descriptor_set_get().bind_as_ssbo(*this, *location); + VKStateManager &state_manager = context.state_manager_get(); + state_manager.storage_buffer_bind(*this, binding); } void VKVertexBuffer::bind_as_texture(uint binding) { VKContext &context = *VKContext::get(); VKStateManager &state_manager = context.state_manager_get(); - state_manager.texel_buffer_bind(this, binding); - should_unbind_ = true; + state_manager.texel_buffer_bind(*this, binding); } -void VKVertexBuffer::bind(uint binding) +void VKVertexBuffer::bind(int binding, shader::ShaderCreateInfo::Resource::BindType bind_type) { VKContext &context = *VKContext::get(); VKShader *shader = static_cast(context.shader); const VKShaderInterface &shader_interface = shader->interface_get(); const std::optional location = - shader_interface.descriptor_set_location( - shader::ShaderCreateInfo::Resource::BindType::SAMPLER, binding); + shader_interface.descriptor_set_location(bind_type, binding); if (!location) { return; } upload_data(); - if (vk_buffer_view_ == VK_NULL_HANDLE) { + if (bind_type == shader::ShaderCreateInfo::Resource::BindType::SAMPLER && + vk_buffer_view_ == VK_NULL_HANDLE) + { VkBufferViewCreateInfo buffer_view_info = {}; eGPUTextureFormat texture_format = to_texture_format(&format); @@ -75,7 +66,14 @@ void VKVertexBuffer::bind(uint binding) device.device_get(), &buffer_view_info, vk_allocation_callbacks, &vk_buffer_view_); } - shader->pipeline_get().descriptor_set_get().bind(*this, *location); + /* TODO: Check if we can move this check inside the descriptor set. */ + VKDescriptorSetTracker &descriptor_set = shader->pipeline_get().descriptor_set_get(); + if (bind_type == shader::ShaderCreateInfo::Resource::BindType::SAMPLER) { + descriptor_set.bind(*this, *location); + } + else { + descriptor_set.bind_as_ssbo(*this, *location); + } } void VKVertexBuffer::wrap_handle(uint64_t /*handle*/) @@ -119,14 +117,9 @@ void VKVertexBuffer::resize_data() void VKVertexBuffer::release_data() { - VKContext *context = VKContext::get(); - if (should_unbind_ && context) { - context->state_manager_get().texel_buffer_unbind(this); - } - if (vk_buffer_view_ != VK_NULL_HANDLE) { - VK_ALLOCATION_CALLBACKS; const VKDevice &device = VKBackend::get().device_get(); + VK_ALLOCATION_CALLBACKS; vkDestroyBufferView(device.device_get(), vk_buffer_view_, vk_allocation_callbacks); vk_buffer_view_ = VK_NULL_HANDLE; } diff --git a/source/blender/gpu/vulkan/vk_vertex_buffer.hh b/source/blender/gpu/vulkan/vk_vertex_buffer.hh index 6660cdedb84..aac986d4ac3 100644 --- a/source/blender/gpu/vulkan/vk_vertex_buffer.hh +++ b/source/blender/gpu/vulkan/vk_vertex_buffer.hh @@ -10,13 +10,13 @@ #include "gpu_vertex_buffer_private.hh" +#include "vk_bindable_resource.hh" #include "vk_buffer.hh" namespace blender::gpu { -class VKVertexBuffer : public VertBuf { +class VKVertexBuffer : public VertBuf, public VKBindableResource { VKBuffer buffer_; - bool should_unbind_ = false; /** When a vertex buffer is used as a UNIFORM_TEXEL_BUFFER the buffer requires a buffer view. */ VkBufferView vk_buffer_view_ = VK_NULL_HANDLE; @@ -25,7 +25,7 @@ class VKVertexBuffer : public VertBuf { void bind_as_ssbo(uint binding) override; void bind_as_texture(uint binding) override; - void bind(uint binding); + void bind(int binding, shader::ShaderCreateInfo::Resource::BindType bind_type) override; void wrap_handle(uint64_t handle) override; void update_sub(uint start, uint len, const void *data) override; @@ -58,7 +58,7 @@ class VKVertexBuffer : public VertBuf { friend class VKTexture; }; -static inline VKVertexBuffer *unwrap(VertBuf *vertex_buffer) +BLI_INLINE VKVertexBuffer *unwrap(VertBuf *vertex_buffer) { return static_cast(vertex_buffer); }