Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flexible buffer bindings #67

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
5 changes: 5 additions & 0 deletions system_modules/naprender/src/bufferbinding.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,11 @@ namespace nap
*/
virtual size_t getSize() const { assert(mBuffer != nullptr); return mBuffer->getSize(); }

/**
* @return The element size in bytes
*/
virtual size_t getElementSize() const { assert(mBuffer != nullptr); return mBuffer->getElementSize(); }

/**
* @return the base GPU buffer, nullptr if not set
*/
Expand Down
60 changes: 33 additions & 27 deletions system_modules/naprender/src/bufferbindinginstance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,33 @@ namespace nap
static const ShaderVariableDeclaration& getBufferDeclaration(const BufferObjectDeclaration& declaration)
{
// If a buffer object declaration is passed, we can safely acquire the actual buffer declaration from it
if (declaration.get_type() == RTTI_OF(BufferObjectDeclaration))
const auto* buffer_object_declaration = rtti_cast<const BufferObjectDeclaration>(&declaration);
lshoek marked this conversation as resolved.
Show resolved Hide resolved
return buffer_object_declaration->getBufferDeclaration();
}


/**
* Checks whether a buffer delcaration is compatible with a buffer binding.
* It does so by ensuring the shader variable stride is equal to the buffer binding element size.
*/
static bool isCompatible(const ShaderVariableDeclaration& declaration, const BufferBinding& binding)
{
assert(binding.getBuffer() != nullptr);

// If the declaration is a struct buffer or value array, check its stride
auto declaration_type = declaration.get_type();
if (declaration_type == RTTI_OF(ShaderVariableStructBufferDeclaration))
{
const BufferObjectDeclaration* buffer_object_declaration = rtti_cast<const BufferObjectDeclaration>(&declaration);
return buffer_object_declaration->getBufferDeclaration();
const auto& struct_buffer_declaration = static_cast<const ShaderVariableStructBufferDeclaration&>(declaration);
return struct_buffer_declaration.mStride == binding.getBuffer()->getElementSize();
}
return declaration;
else if (declaration_type == RTTI_OF(ShaderVariableValueArrayDeclaration))
{
const auto &struct_buffer_declaration = static_cast<const ShaderVariableValueArrayDeclaration &>(declaration);
return struct_buffer_declaration.mStride == binding.getBuffer()->getElementSize();
}
// The declaration is for a primitive value, so we check its size directly
return declaration.mSize == binding.getBuffer()->getElementSize();
}


Expand All @@ -92,51 +113,36 @@ namespace nap

// Ensure we retrieve the actual buffer declaration
const auto& buffer_declaration = getBufferDeclaration(declaration);
rtti::TypeInfo declaration_type = buffer_declaration.get_type();

// If the buffer binding was created from a resource, ensure its element count matches
// the one in the shader declaration and ensure the descriptortype is storage
if (binding != nullptr)
{
// Verify descriptortype
// Verify descriptor type
if (!errorState.check((binding->getBuffer()->getBufferUsageFlags() & VK_BUFFER_USAGE_STORAGE_BUFFER_BIT) > 0,
"DescriptorType mismatch. StructGPUBuffer 'DescriptorType' property must be 'Storage' to be used as a buffer binding"))
return nullptr;

// Verify bounds
if (!errorState.check(binding->getSize() == buffer_declaration.mSize,
utility::stringFormat("Mismatch between total buffer size in shader and json for buffer binding '%s'. Please refer to the alignment requirements for shader resources in Section 15.6.4 of the Vulkan specification", buffer_declaration.mName.c_str())))
if (!errorState.check(isCompatible(buffer_declaration, *binding),
utility::stringFormat("Mismatch between element stride and buffer element size for buffer binding '%s'. Please refer to the alignment requirements for shader resources in Section 15.6.4 of the Vulkan specification", buffer_declaration.mName.c_str())))
return nullptr;
}

auto declaration_type = buffer_declaration.get_type();

// Creates a BufferBindingStructInstance
if (declaration_type == RTTI_OF(ShaderVariableStructBufferDeclaration))
{
const auto& struct_buffer_declaration = static_cast<const ShaderVariableStructBufferDeclaration&>(buffer_declaration);
if (binding != nullptr)
{
// Verify count
if (!errorState.check(binding->getCount() == struct_buffer_declaration.mNumElements,
"Encountered mismatch in array elements between array in material and array in shader"))
return nullptr;
}

return createBufferBindingInstance<BufferBindingStructInstance, BufferBindingStruct, ShaderVariableStructBufferDeclaration>(
binding_name, struct_buffer_declaration, binding, bufferChangedCallback, errorState);
}

// Creates a BufferBindingNumericInstance
if (declaration_type == RTTI_OF(ShaderVariableValueArrayDeclaration))
{
const ShaderVariableValueArrayDeclaration* value_array_declaration = rtti_cast<const ShaderVariableValueArrayDeclaration>(&buffer_declaration);
if (binding != nullptr)
{
// Verify count
if (!errorState.check(binding->getCount() == value_array_declaration->mNumElements,
"Encountered mismatch in array elements between array in material and array in shader"))
return nullptr;
}

const auto* value_array_declaration = rtti_cast<const ShaderVariableValueArrayDeclaration>(&buffer_declaration);
if (value_array_declaration->mElementType == EShaderVariableValueType::UInt)
{
return createBufferBindingInstance<BufferBindingUIntInstance, BufferBindingUInt, ShaderVariableValueArrayDeclaration>(
Expand Down Expand Up @@ -203,7 +209,7 @@ namespace nap

void BufferBindingStructInstance::setBuffer(StructBuffer& buffer)
{
assert(buffer.getSize() == mDeclaration->mSize);
NAP_ASSERT_MSG(mDeclaration->mStride == buffer.getElementSize(), "Buffer declaration stride is not equal to buffer element size");
BufferBindingInstance::mBuffer = &buffer;
raiseChanged();
}
Expand All @@ -212,7 +218,7 @@ namespace nap
void BufferBindingStructInstance::setBuffer(const BufferBindingStruct& resource)
{
assert(resource.mBuffer != nullptr);
assert(resource.mBuffer->getSize() == mDeclaration->mSize);
NAP_ASSERT_MSG(mDeclaration->mStride == resource.mBuffer->getElementSize(), "Buffer declaration stride is not equal to buffer element size");
BufferBindingInstance::mBuffer = resource.mBuffer.get();
raiseChanged();
}
Expand Down
4 changes: 2 additions & 2 deletions system_modules/naprender/src/bufferbindinginstance.h
Original file line number Diff line number Diff line change
Expand Up @@ -340,15 +340,15 @@ namespace nap
template<class T>
void TypedBufferBindingNumericInstance<T>::setBuffer(TypedGPUBufferNumeric<T>& buffer)
{
assert(buffer.getSize() == mDeclaration->mSize);
NAP_ASSERT_MSG(mDeclaration->mStride == buffer.getElementSize(), "Buffer declaration stride is not equal to buffer element size");
BufferBindingInstance::mBuffer = &buffer;
raiseChanged();
}

template<class T>
void TypedBufferBindingNumericInstance<T>::setBuffer(const TypedBufferBindingNumeric<T>& resource)
{
assert(resource.mBuffer.getSize() == mDeclaration->mSize);
NAP_ASSERT_MSG(mDeclaration->mStride == resource.mBuffer.getElementSize(), "Buffer declaration stride is not equal to buffer element size");
BufferBindingInstance::mBuffer = resource.mBuffer.get();
raiseChanged();
}
Expand Down
17 changes: 9 additions & 8 deletions system_modules/naprender/src/rendertarget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ namespace nap
mHasDepthTexture = mDepthTexture != nullptr;

// Set framebuffer size
glm::ivec2 size = mColorTexture->getSize();
const auto size = mColorTexture->getSize();
VkExtent2D framebuffer_size = { (uint32)size.x, (uint32)size.y };

// Store as attachments
Expand Down Expand Up @@ -168,20 +168,21 @@ namespace nap

void RenderTarget::beginRendering()
{
glm::ivec2 size = mColorTexture->getSize();
std::array<VkClearValue, 2> clearValues = {};
clearValues[0].color = { mClearColor[0], mClearColor[1], mClearColor[2], mClearColor[3] };
clearValues[1].depthStencil = { 1.0f, 0 };
const auto size = mColorTexture->getSize();
std::array<VkClearValue, 3> clear_values = {};
clear_values[0].color = { mClearColor[0], mClearColor[1], mClearColor[2], mClearColor[3] };
clear_values[1].depthStencil = { 1.0f, 0 };
clear_values[2].color = { mClearColor[0], mClearColor[1], mClearColor[2], mClearColor[3] };
lshoek marked this conversation as resolved.
Show resolved Hide resolved

// Setup render pass
VkRenderPassBeginInfo renderPassInfo = {};
renderPassInfo.sType = VK_STRUCTURE_TYPE_RENDER_PASS_BEGIN_INFO;
renderPassInfo.renderPass = mRenderPass;
renderPassInfo.framebuffer = mFramebuffer;
renderPassInfo.renderArea.offset = { 0, 0 };
renderPassInfo.renderArea.extent = { static_cast<uint>(size.x), static_cast<uint>(size.y) };;
renderPassInfo.clearValueCount = static_cast<uint32_t>(clearValues.size());
renderPassInfo.pClearValues = clearValues.data();
renderPassInfo.renderArea.extent = { static_cast<uint>(size.x), static_cast<uint>(size.y) };
renderPassInfo.clearValueCount = static_cast<uint>(clear_values.size());
renderPassInfo.pClearValues = clear_values.data();

// Begin render pass
vkCmdBeginRenderPass(mRenderService->getCurrentCommandBuffer(), &renderPassInfo, VK_SUBPASS_CONTENTS_INLINE);
Expand Down
3 changes: 2 additions & 1 deletion system_modules/naprender/src/renderwindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -967,9 +967,10 @@ namespace nap
render_pass_info.renderArea.extent = mSwapchainExtent;

// Clear color
std::array<VkClearValue, 2> clear_values = {};
std::array<VkClearValue, 3> clear_values = {};
clear_values[0].color = { mClearColor[0], mClearColor[1], mClearColor[2], mClearColor[3] };
clear_values[1].depthStencil = { 1.0f, 0 };
clear_values[2].color = { mClearColor[0], mClearColor[1], mClearColor[2], mClearColor[3] };
render_pass_info.clearValueCount = static_cast<uint32_t>(clear_values.size());
render_pass_info.pClearValues = clear_values.data();

Expand Down
44 changes: 18 additions & 26 deletions system_modules/naprender/src/shader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -574,11 +574,11 @@ static bool addShaderVariablesRecursive(nap::ShaderVariableStructDeclaration& pa

for (int index = 0; index < type.member_types.size(); ++index)
{
spirv_cross::SPIRType member_type = compiler.get_type(type.member_types[index]);
auto member_type = compiler.get_type(type.member_types[index]);
lshoek marked this conversation as resolved.
Show resolved Hide resolved
auto member_size = compiler.get_declared_struct_member_size(type, index);

std::string name = compiler.get_member_name(type.self, index);
auto name = compiler.get_member_name(type.self, index);
int absoluteOffset = parentOffset + compiler.type_struct_member_offset(type, index);
size_t member_size = compiler.get_declared_struct_member_size(type, index);

std::string full_path = path + "." + name;

Expand All @@ -588,12 +588,11 @@ static bool addShaderVariablesRecursive(nap::ShaderVariableStructDeclaration& pa
bool is_array = !member_type.array.empty();
if (is_array)
{
int num_elements = member_type.array[0];

auto num_elements = member_type.array[0];
if (member_type.basetype == spirv_cross::SPIRType::Struct)
{
size_t stride = compiler.type_struct_member_array_stride(type, index);
size_t struct_size = compiler.get_declared_struct_size(member_type);
auto stride = compiler.type_struct_member_array_stride(type, index);
auto struct_size = compiler.get_declared_struct_size(member_type);

if (descriptorType == nap::EDescriptorType::Storage)
{
Expand All @@ -603,21 +602,18 @@ static bool addShaderVariablesRecursive(nap::ShaderVariableStructDeclaration& pa
// of the ShaderVariableStructBufferDeclaration along with the element stride and count. All that matters is the size of the struct element,
// which is already resolved by SPIR-V. Therefore, we do not have to traverse the struct recursively here.

std::unique_ptr<nap::ShaderVariableStructBufferDeclaration> buffer_declaration = std::make_unique<nap::ShaderVariableStructBufferDeclaration>(name, absoluteOffset, member_size, stride, num_elements);
std::unique_ptr<nap::ShaderVariableStructDeclaration> struct_declaration = std::make_unique<nap::ShaderVariableStructDeclaration>(name, parentStruct.mDescriptorType, absoluteOffset, struct_size);
buffer_declaration->mElement = std::move(struct_declaration);
auto buffer_declaration = std::make_unique<nap::ShaderVariableStructBufferDeclaration>(name, absoluteOffset, member_size, stride, num_elements);
buffer_declaration->mElement = std::make_unique<nap::ShaderVariableStructDeclaration>(name, parentStruct.mDescriptorType, absoluteOffset, struct_size);

parentStruct.mMembers.emplace_back(std::move(buffer_declaration));
}
else if (descriptorType == nap::EDescriptorType::Uniform)
{
std::unique_ptr<nap::ShaderVariableStructArrayDeclaration> array_declaration = std::make_unique<nap::ShaderVariableStructArrayDeclaration>(name, absoluteOffset, member_size);

auto array_declaration = std::make_unique<nap::ShaderVariableStructArrayDeclaration>(name, absoluteOffset, member_size);
for (int array_index = 0; array_index < num_elements; ++array_index)
{
std::string array_path = nap::utility::stringFormat("%s[%d]", full_path.c_str(), array_index);

std::unique_ptr<nap::ShaderVariableStructDeclaration> struct_declaration = std::make_unique<nap::ShaderVariableStructDeclaration>(name, parentStruct.mDescriptorType, absoluteOffset, struct_size);
auto array_path = nap::utility::stringFormat("%s[%d]", full_path.c_str(), array_index);
auto struct_declaration = std::make_unique<nap::ShaderVariableStructDeclaration>(name, parentStruct.mDescriptorType, absoluteOffset, struct_size);
if (!addShaderVariablesRecursive(*struct_declaration, compiler, member_type, absoluteOffset, array_path, descriptorType, errorState))
return false;

Expand All @@ -629,36 +625,32 @@ static bool addShaderVariablesRecursive(nap::ShaderVariableStructDeclaration& pa
}
else
{
size_t stride = compiler.type_struct_member_array_stride(type, index);

nap::EShaderVariableValueType element_type = getShaderVariableValueType(member_type);
auto stride = compiler.type_struct_member_array_stride(type, index);
auto element_type = getShaderVariableValueType(member_type);
if (!errorState.check(element_type != nap::EShaderVariableValueType::Unknown, "Encountered unknown uniform type"))
return false;

std::unique_ptr<nap::ShaderVariableValueArrayDeclaration> array_declaration = std::make_unique<nap::ShaderVariableValueArrayDeclaration>(name, absoluteOffset, member_size, stride, element_type, num_elements);
parentStruct.mMembers.emplace_back(std::move(array_declaration));
parentStruct.mMembers.emplace_back(std::make_unique<nap::ShaderVariableValueArrayDeclaration>(name, absoluteOffset, member_size, stride, element_type, num_elements));
}
}
else
{
if (member_type.basetype == spirv_cross::SPIRType::Struct)
{
size_t struct_size = compiler.get_declared_struct_size(member_type);

std::unique_ptr<nap::ShaderVariableStructDeclaration> struct_declaration = std::make_unique<nap::ShaderVariableStructDeclaration>(name, parentStruct.mDescriptorType, absoluteOffset, struct_size);
auto struct_size = compiler.get_declared_struct_size(member_type);
auto struct_declaration = std::make_unique<nap::ShaderVariableStructDeclaration>(name, parentStruct.mDescriptorType, absoluteOffset, struct_size);
if (!addShaderVariablesRecursive(*struct_declaration, compiler, member_type, absoluteOffset, name, descriptorType, errorState))
return false;

parentStruct.mMembers.emplace_back(std::move(struct_declaration));
}
else
{
nap::EShaderVariableValueType value_type = getShaderVariableValueType(member_type);
auto value_type = getShaderVariableValueType(member_type);
lshoek marked this conversation as resolved.
Show resolved Hide resolved
if (!errorState.check(value_type != nap::EShaderVariableValueType::Unknown, "Encountered unknown uniform type"))
return false;

std::unique_ptr<nap::ShaderVariableValueDeclaration> value_declaration = std::make_unique<nap::ShaderVariableValueDeclaration>(name, absoluteOffset, member_size, value_type);
parentStruct.mMembers.emplace_back(std::move(value_declaration));
parentStruct.mMembers.emplace_back(std::make_unique<nap::ShaderVariableValueDeclaration>(name, absoluteOffset, member_size, value_type));
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion system_modules/naprender/src/snapshotrendertarget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -255,9 +255,10 @@ namespace nap
glm::ivec2 size = getBufferSize();
const RGBAColorFloat& clear_color = mSnapshot->mClearColor;

std::array<VkClearValue, 2> clearValues = {};
std::array<VkClearValue, 3> clearValues = {};
clearValues[0].color = { clear_color[0], clear_color[1], clear_color[2], clear_color[3] };
clearValues[1].depthStencil = { 1.0f, 0 };
clearValues[2].color = { clear_color[0], clear_color[1], clear_color[2], clear_color[3] };

// Setup render pass
VkRenderPassBeginInfo renderPassInfo = {};
Expand Down
Loading