blender
/
blender
136
404
Fork 586
Code Issues 6.2k Pull Requests 761 Projects 19 Wiki Activity

Vulkan: Push constants #104880

Merged
Jeroen Bakker merged 73 commits from Jeroen-Bakker/blender:vulkan-push-constants into main 2023-03-06 12:29:06 +01:00
Conversation 1 Commits 73 Files Changed 20 +39 -31
7 changed files with 39 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit dee686bd9d - Show all commits

6
source/blender/gpu/vulkan/vk_backend.cc
View File

@ -70,7 +70,7 @@ void VKBackend::compute_dispatch(int groups_x_len, int groups_y_len, int groups_
VKPushConstants &push_constants = pipeline.push_constants_get();
/* Update push constants based on their storage type.*/
switch (push_constants.storage_type_get()) {
switch (push_constants.layout_get().storage_type_get()) {
Jeroen-Bakker marked this conversation as resolved
Bastien Montagne commented 2023-03-02 21:07:43 +01:00
Review
Copy Link

I may be missing something, but this whole switch block feels like it does not belong here. It looks way too specific to me.

Would rather see that logic as part of the VKPushConstants class itself, but no idea if this is doable in practice... At the very least would have it in a dedicated util function of VKBackend otherwise?

I may be missing something, but this whole `switch` block feels like it does not belong here. It looks way too specific to me. Would rather see that logic as part of the `VKPushConstants` class itself, but no idea if this is doable in practice... At the very least would have it in a dedicated util function of `VKBackend` otherwise?
Jeroen Bakker commented 2023-03-06 09:55:03 +01:00
Review
Copy Link

Yes you're right, will move this part into a method of VKPushConstants.

Yes you're right, will move this part into a method of VKPushConstants.
case VKPushConstantsLayout::StorageType::NONE:
break;
@ -82,13 +82,13 @@ void VKBackend::compute_dispatch(int groups_x_len, int groups_y_len, int groups_
case VKPushConstantsLayout::StorageType::STORAGE_BUFFER:
push_constants.update_storage_buffer(context.device_get());
descriptor_set.bind(push_constants.storage_buffer_get(),
push_constants.storage_buffer_binding_get());
push_constants.layout_get().storage_buffer_binding_get());
break;
case VKPushConstantsLayout::StorageType::UNIFORM_BUFFER:
push_constants.update_uniform_buffer(context.device_get());
descriptor_set.bind(push_constants.uniform_buffer_get(),
push_constants.storage_buffer_binding_get());
push_constants.layout_get().storage_buffer_binding_get());
break;
}
descriptor_set.update(context.device_get());

7
source/blender/gpu/vulkan/vk_command_buffer.cc
View File

@ -9,8 +9,8 @@
#include "vk_buffer.hh"
#include "vk_context.hh"
#include "vk_memory.hh"
#include "vk_texture.hh"
#include "vk_pipeline.hh"
#include "vk_texture.hh"
#include "BLI_assert.h"
@ -76,14 +76,15 @@ void VKCommandBuffer::push_constants(const VKPushConstants &push_constants,
const VkPipelineLayout vk_pipeline_layout,
const VkShaderStageFlags vk_shader_stages)
{
if (push_constants.storage_type_get() != VKPushConstantsLayout::StorageType::PUSH_CONSTANTS) {
if (push_constants.layout_get().storage_type_get() !=
VKPushConstantsLayout::StorageType::PUSH_CONSTANTS) {
return;
}
vkCmdPushConstants(vk_command_buffer_,
vk_pipeline_layout,
vk_shader_stages,
push_constants.offset(),
push_constants.size_in_bytes(),
push_constants.layout_get().size_in_bytes(),
push_constants.data());
}

2
source/blender/gpu/vulkan/vk_pipeline.cc
View File

@ -52,7 +52,7 @@ VKPipeline VKPipeline::create_compute_pipeline(VKContext &context,
if (vkCreateComputePipelines(
vk_device, nullptr, 1, &pipeline_info, vk_allocation_callbacks, &vk_pipeline) !=
VK_SUCCESS) {
//return VKPipeline();
return VKPipeline();
Jeroen-Bakker marked this conversation as resolved
Jeroen Bakker commented 2023-02-21 15:26:05 +01:00
Review
Copy Link

This should be fixed.

This should be fixed.
}
VKDescriptorSet descriptor_set = context.descriptor_pools_get().allocate(descriptor_set_layout);

18
source/blender/gpu/vulkan/vk_push_constants.cc
View File

@ -78,8 +78,8 @@ VKPushConstantsLayout::StorageType VKPushConstantsLayout::determine_storage_type
}
uint32_t size = struct_size<Std430>(info.push_constants_);
return size <= vk_physical_device_limits.maxPushConstantsSize ? StorageType::PUSH_CONSTANTS :
StorageType::STORAGE_BUFFER;
return size <= vk_physical_device_limits.maxPushConstantsSize ? STORAGE_TYPE_DEFAULT :
STORAGE_TYPE_FALLBACK;
}
void VKPushConstantsLayout::init(const shader::ShaderCreateInfo &info,
@ -122,13 +122,13 @@ VKPushConstants::VKPushConstants() = default;
VKPushConstants::VKPushConstants(const VKPushConstantsLayout *layout) : layout_(layout)
{
data_ = MEM_mallocN(layout->size_in_bytes(), __func__);
switch (storage_type_get()) {
switch (layout_->storage_type_get()) {
case VKPushConstantsLayout::StorageType::STORAGE_BUFFER:
storage_buffer_ = new VKStorageBuffer(size_in_bytes(), GPU_USAGE_DYNAMIC, __func__);
storage_buffer_ = new VKStorageBuffer(layout_->size_in_bytes(), GPU_USAGE_DYNAMIC, __func__);
break;
case VKPushConstantsLayout::StorageType::UNIFORM_BUFFER:
uniform_buffer_ = new VKUniformBuffer(size_in_bytes(), __func__);
uniform_buffer_ = new VKUniformBuffer(layout_->size_in_bytes(), __func__);
break;
case VKPushConstantsLayout::StorageType::PUSH_CONSTANTS:
@ -178,7 +178,7 @@ VKPushConstants &VKPushConstants::operator=(VKPushConstants &&other)
void VKPushConstants::update_storage_buffer(VkDevice /*vk_device*/)
{
BLI_assert(storage_type_get() == VKPushConstantsLayout::StorageType::STORAGE_BUFFER);
BLI_assert(layout_->storage_type_get() == VKPushConstantsLayout::StorageType::STORAGE_BUFFER);
BLI_assert(storage_buffer_ != nullptr);
BLI_assert(data_ != nullptr);
storage_buffer_->update(data_);
@ -186,14 +186,14 @@ void VKPushConstants::update_storage_buffer(VkDevice /*vk_device*/)
VKStorageBuffer &VKPushConstants::storage_buffer_get()
{
BLI_assert(storage_type_get() == VKPushConstantsLayout::StorageType::STORAGE_BUFFER);
BLI_assert(layout_->storage_type_get() == VKPushConstantsLayout::StorageType::STORAGE_BUFFER);
BLI_assert(storage_buffer_ != nullptr);
return *storage_buffer_;
}
void VKPushConstants::update_uniform_buffer(VkDevice /*vk_device*/)
{
BLI_assert(storage_type_get() == VKPushConstantsLayout::StorageType::UNIFORM_BUFFER);
BLI_assert(layout_->storage_type_get() == VKPushConstantsLayout::StorageType::UNIFORM_BUFFER);
BLI_assert(uniform_buffer_ != nullptr);
BLI_assert(data_ != nullptr);
uniform_buffer_->update(data_);
@ -201,7 +201,7 @@ void VKPushConstants::update_uniform_buffer(VkDevice /*vk_device*/)
VKUniformBuffer &VKPushConstants::uniform_buffer_get()
{
BLI_assert(storage_type_get() == VKPushConstantsLayout::StorageType::UNIFORM_BUFFER);
BLI_assert(layout_->storage_type_get() == VKPushConstantsLayout::StorageType::UNIFORM_BUFFER);
BLI_assert(uniform_buffer_ != nullptr);
return *uniform_buffer_;
}

31
source/blender/gpu/vulkan/vk_push_constants.hh
View File

@ -35,13 +35,28 @@ class VKStorageBuffer;
* Describe the layout of the push constants and the storage type that should be used.
*/
struct VKPushConstantsLayout {
/* Should the push constant use regular push constants or a buffer.*/
/* Different methods to store push constants.*/
enum class StorageType {
/** Push constants aren't in use.*/
NONE,
/** Store push constants as regular vulkan push constants.*/
PUSH_CONSTANTS,
/**
* Fallback when push constants doesn't meet the device requirements. This fallback uses a
* storage buffer.
*/
STORAGE_BUFFER,
/**
* Fallback when push constants doesn't meet the device requirements. This fallback uses an
* uniform buffer.
*/
UNIFORM_BUFFER,
};
static constexpr StorageType STORAGE_TYPE_DEFAULT = StorageType::PUSH_CONSTANTS;
static constexpr StorageType STORAGE_TYPE_FALLBACK = StorageType::UNIFORM_BUFFER;
struct PushConstantLayout {
/* TODO: location requires sequential lookups, we should make the location index based for
@ -114,19 +129,9 @@ class VKPushConstants : NonCopyable {
return 0;
}
size_t size_in_bytes() const
const VKPushConstantsLayout &layout_get() const
{
return layout_->size_in_bytes();
}
VKPushConstantsLayout::StorageType storage_type_get() const
{
return layout_->storage_type_get();
}
VKDescriptorSet::Location storage_buffer_binding_get() const
{
return layout_->storage_buffer_binding_get();
return *layout_;
}
void update_storage_buffer(VkDevice vk_device);

5
source/blender/gpu/vulkan/vk_shader.cc
View File

@ -925,8 +925,9 @@ static void add_descriptor_set_layout_bindings(
/* Add push constants to the descriptor when push constants are stored in a storage buffer.*/
const VKPushConstantsLayout &push_constants_layout = interface.push_constants_layout_get();
if (push_constants_layout.storage_type_get() ==
VKPushConstantsLayout::StorageType::STORAGE_BUFFER) {
if (ELEM(push_constants_layout.storage_type_get(),
VKPushConstantsLayout::StorageType::UNIFORM_BUFFER,
Jeroen-Bakker marked this conversation as resolved
Jeroen Bakker commented 2023-02-21 20:32:46 +01:00
Review
Copy Link

Also check for uniform_buffer

Also check for uniform_buffer
VKPushConstantsLayout::StorageType::STORAGE_BUFFER)) {
r_bindings.append(create_descriptor_set_layout_binding(push_constants_layout));
}
}

1
source/blender/gpu/vulkan/vk_shader_interface.cc
View File

@ -143,6 +143,7 @@ void VKShaderInterface::init(const shader::ShaderCreateInfo &info)
info, *this, push_constants_storage_type, push_constants_fallback_input);
sort_inputs();
debug_print();
}
const ShaderInput *VKShaderInterface::shader_input_get(