Skip to content

Commit

Permalink
[vulkan] fixed and issue where the frame's constant buffer the light …
Browse files Browse the repository at this point in the history 
…bindless buffer could be read while it was being written to, this could rarely manifest as flickering
  • Loading branch information
@PanosK92
PanosK92 committed Dec 19, 2024
1 parent 5611b4b commit 90f287d
Show file tree
Hide file tree
Showing 6 changed files with 87 additions and 55 deletions.
4 changes: 3 additions & 1 deletion runtime/RHI/RHI_Buffer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.

//= INCLUDES =====================
#include "../Core/SpartanObject.h"
#include "RHI_Definitions.h"
//================================

namespace Spartan
Expand Down Expand Up @@ -68,7 +69,7 @@ namespace Spartan
~RHI_Buffer() { RHI_DestroyResource(); }

// storage and constant buffer updating
void Update(void* data_cpu, const uint32_t size = 0);
void Update(RHI_CommandList* cmd_list, void* data_cpu, const uint32_t size = 0);
void ResetOffset() { m_offset = 0; first_update = true; }

// propeties
Expand All @@ -78,6 +79,7 @@ namespace Spartan
uint32_t GetOffset() const { return m_offset; }
void* GetMappedData() const { return m_data_gpu; }
void* GetRhiResource() const { return m_rhi_resource; }
RHI_Buffer_Type GetType() const { return m_type; }

private:
RHI_Buffer_Type m_type = RHI_Buffer_Type::Max;
Expand Down
25 changes: 17 additions & 8 deletions runtime/RHI/Vulkan/Vulkan_Buffer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ namespace Spartan
RHI_Device::SetResourceName(m_rhi_resource, RHI_Resource_Type::Buffer, m_object_name);
}

void RHI_Buffer::Update(void* data_cpu, const uint32_t size)
void RHI_Buffer::Update(RHI_CommandList* cmd_list, void* data_cpu, const uint32_t size)
{
SP_ASSERT_MSG(m_mappable, "Can't update unmappable buffer");
SP_ASSERT_MSG(data_cpu != nullptr, "Invalid cpu data");
Expand All @@ -132,11 +132,20 @@ namespace Spartan
m_offset += m_stride;
}

// persistently mapped so a memcpy is enough
memcpy(
reinterpret_cast<std::byte*>(m_data_gpu) + m_offset, // destination
reinterpret_cast<std::byte*>(data_cpu), // source
size != 0 ? size : m_stride // size
);
}
// update
if (cmd_list)
{
// vkCmdUpdateBuffer and vkCmdPipelineBarrier
cmd_list->UpdateBuffer(this, m_offset, size != 0 ? size : m_stride, data_cpu);
}
else
{
// direct and async
memcpy(
reinterpret_cast<std::byte*>(m_data_gpu) + m_offset, // destination
reinterpret_cast<std::byte*>(data_cpu), // source
size != 0 ? size : m_stride // size
);
}
}
}
23 changes: 21 additions & 2 deletions runtime/RHI/Vulkan/Vulkan_CommandList.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1580,16 +1580,35 @@ namespace Spartan
size,
data
);

VkBufferMemoryBarrier barrier = {};
barrier.sType = VK_STRUCTURE_TYPE_BUFFER_MEMORY_BARRIER;
barrier.srcAccessMask = VK_ACCESS_TRANSFER_WRITE_BIT;
barrier.dstAccessMask = VK_ACCESS_SHADER_READ_BIT | VK_ACCESS_TRANSFER_READ_BIT | VK_ACCESS_INDEX_READ_BIT | VK_ACCESS_VERTEX_ATTRIBUTE_READ_BIT;
barrier.srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED;
barrier.dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED;
barrier.buffer = static_cast<VkBuffer>(buffer->GetRhiResource());
barrier.offset = offset;
barrier.size = size;

// set dstAccessMask based on buffer type
switch (buffer->GetType()) {
case RHI_Buffer_Type::Vertex:
case RHI_Buffer_Type::Instance:
barrier.dstAccessMask = VK_ACCESS_VERTEX_ATTRIBUTE_READ_BIT;
break;
case RHI_Buffer_Type::Index:
barrier.dstAccessMask = VK_ACCESS_INDEX_READ_BIT;
break;
case RHI_Buffer_Type::Storage:
barrier.dstAccessMask = VK_ACCESS_SHADER_READ_BIT;
break;
case RHI_Buffer_Type::Constant:
barrier.dstAccessMask = VK_ACCESS_UNIFORM_READ_BIT;
break;
default:
barrier.dstAccessMask = VK_ACCESS_SHADER_READ_BIT;
break;
}

vkCmdPipelineBarrier(
static_cast<VkCommandBuffer>(m_rhi_resource),
Expand Down
82 changes: 42 additions & 40 deletions runtime/Rendering/Renderer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ namespace Spartan
// bindless
static array<RHI_Texture*, rhi_max_array_size> bindless_textures;
bool bindless_materials_dirty = true;
bool bindless_lights_dirty = true;

// misc
unordered_map<Renderer_Option, float> m_options;
Expand Down Expand Up @@ -216,8 +217,8 @@ namespace Spartan
// subscribe
SP_SUBSCRIBE_TO_EVENT(EventType::WorldClear, SP_EVENT_HANDLER_STATIC(OnClear));
SP_SUBSCRIBE_TO_EVENT(EventType::WindowFullScreenToggled, SP_EVENT_HANDLER_STATIC(OnFullScreenToggled));
SP_SUBSCRIBE_TO_EVENT(EventType::MaterialOnChanged, SP_EVENT_HANDLER_STATIC(BindlessUpdateMaterials));
SP_SUBSCRIBE_TO_EVENT(EventType::LightOnChanged, SP_EVENT_HANDLER_STATIC(BindlessUpdateLights));
SP_SUBSCRIBE_TO_EVENT(EventType::MaterialOnChanged, SP_EVENT_HANDLER_EXPRESSION_STATIC( bindless_materials_dirty = true; ));
SP_SUBSCRIBE_TO_EVENT(EventType::LightOnChanged, SP_EVENT_HANDLER_EXPRESSION_STATIC( bindless_lights_dirty = true; ));

// fire
SP_FIRE_EVENT(EventType::RendererOnInitialized);
Expand Down Expand Up @@ -305,7 +306,7 @@ namespace Spartan
cmd_list_graphics->Begin(queue_graphics);
//cmd_list_compute->Begin(queue_compute);

OnSyncPoint(cmd_list_graphics);
OnUpdateBuffers(cmd_list_graphics);
ProduceFrame(cmd_list_graphics, cmd_list_compute);

// blit to back buffer when not in editor mode
Expand Down Expand Up @@ -402,7 +403,7 @@ namespace Spartan
SP_LOG_INFO("Output resolution output has been set to %dx%d", width, height);
}

void Renderer::UpdateConstantBufferFrame(RHI_CommandList* cmd_list)
void Renderer::UpdateFrameConstantBuffer(RHI_CommandList* cmd_list)
{
// matrices
{
Expand Down Expand Up @@ -479,7 +480,7 @@ namespace Spartan
m_cb_frame_cpu.set_bit(GetOption<bool>(Renderer_Option::Fog), 1 << 2);

// set
GetBuffer(Renderer_Buffer::ConstantFrame)->Update(&m_cb_frame_cpu);
GetBuffer(Renderer_Buffer::ConstantFrame)->Update(cmd_list, &m_cb_frame_cpu);
}

void Renderer::SetEntities(unordered_map<uint64_t, shared_ptr<Entity>>& entities)
Expand Down Expand Up @@ -528,12 +529,8 @@ namespace Spartan
}

m_mutex_renderables.unlock();

// update structures that rely on the renderables
{
BindlessUpdateMaterials();
BindlessUpdateLights();
}
bindless_materials_dirty = true;
bindless_lights_dirty = true;
}

bool Renderer::CanUseCmdList()
Expand Down Expand Up @@ -576,37 +573,45 @@ namespace Spartan
Input::SetMouseCursorVisible(!Window::IsFullScreen());
}

void Renderer::OnSyncPoint(RHI_CommandList* cmd_list_graphics)
void Renderer::OnUpdateBuffers(RHI_CommandList* cmd_list)
{
// is_sync_point: the command pool has exhausted its command lists and
// is about to reset them, this is an opportune moment for us to perform
// certain operations, knowing that no rendering commands are currently
// executing and no resources are being used by any command list
m_resource_index++;
bool is_sync_point = m_resource_index == resources_frame_lifetime;
if (is_sync_point)
{
m_resource_index = 0;

// delete any rhi resources that have accumulated
if (RHI_Device::DeletionQueueNeedsToParse())
// reset dynamic buffers and parse deletion queue
{
m_resource_index++;
bool is_sync_point = m_resource_index == resources_frame_lifetime;
if (is_sync_point)
{
RHI_Device::QueueWaitAll();
RHI_Device::DeletionQueueParse();
}
m_resource_index = 0;

// reset dynamic buffer offsets
GetBuffer(Renderer_Buffer::StorageSpd)->ResetOffset();
GetBuffer(Renderer_Buffer::ConstantFrame)->ResetOffset();
// delete any rhi resources that have accumulated
if (RHI_Device::DeletionQueueNeedsToParse())
{
RHI_Device::QueueWaitAll();
RHI_Device::DeletionQueueParse();
}

if (bindless_materials_dirty)
{
RHI_Device::UpdateBindlessResources(nullptr, &bindless_textures);
bindless_materials_dirty = false;
// reset dynamic buffer offsets
GetBuffer(Renderer_Buffer::StorageSpd)->ResetOffset();
GetBuffer(Renderer_Buffer::ConstantFrame)->ResetOffset();
}
}

UpdateConstantBufferFrame(cmd_list_graphics);
if (bindless_materials_dirty)
{
BindlessUpdateMaterials(); // properties
RHI_Device::UpdateBindlessResources(nullptr, &bindless_textures); // textures
bindless_materials_dirty = false;
}

if (bindless_lights_dirty)
{
BindlessUpdateLights(cmd_list);
bindless_lights_dirty = false;
}

UpdateFrameConstantBuffer(cmd_list);

AddLinesToBeRendered();
}

Expand Down Expand Up @@ -961,16 +966,13 @@ namespace Spartan
// material properties
Renderer::GetBuffer(Renderer_Buffer::StorageMaterials)->ResetOffset();
uint32_t update_size = static_cast<uint32_t>(sizeof(Sb_Material)) * index;
Renderer::GetBuffer(Renderer_Buffer::StorageMaterials)->Update(&properties[0], update_size);

// material textures
bindless_materials_dirty = true;
Renderer::GetBuffer(Renderer_Buffer::StorageMaterials)->Update(nullptr, &properties[0], update_size);
}

index = 0;
}

void Renderer::BindlessUpdateLights()
void Renderer::BindlessUpdateLights(RHI_CommandList* cmd_list)
{
static array<Sb_Light, rhi_max_array_size_lights> properties;

Expand Down Expand Up @@ -1034,7 +1036,7 @@ namespace Spartan
uint32_t update_size = static_cast<uint32_t>(sizeof(Sb_Light)) * index;
RHI_Buffer* buffer = GetBuffer(Renderer_Buffer::StorageLights);
buffer->ResetOffset();
buffer->Update(&properties[0], update_size);
buffer->Update(cmd_list, &properties[0], update_size);
}

void Renderer::Screenshot(const string& file_path)
Expand Down
6 changes: 3 additions & 3 deletions runtime/Rendering/Renderer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ namespace Spartan
//=====================================================================================================================

private:
static void UpdateConstantBufferFrame(RHI_CommandList* cmd_list);
static void UpdateFrameConstantBuffer(RHI_CommandList* cmd_list);

// resource creation
static void CreateBuffers();
Expand Down Expand Up @@ -181,7 +181,7 @@ namespace Spartan
// event handlers
static void OnClear();
static void OnFullScreenToggled();
static void OnSyncPoint(RHI_CommandList* cmd_list);
static void OnUpdateBuffers(RHI_CommandList* cmd_list);

// misc
static void AddLinesToBeRendered();
Expand All @@ -190,7 +190,7 @@ namespace Spartan

// bindless
static void BindlessUpdateMaterials();
static void BindlessUpdateLights();
static void BindlessUpdateLights(RHI_CommandList* cmd_lis);

// misc
static std::unordered_map<Renderer_Entity, std::vector<std::shared_ptr<Entity>>> m_renderables;
Expand Down
2 changes: 1 addition & 1 deletion runtime/Rendering/Renderer_Resources.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ namespace Spartan

// only needs to be set once, then after each use SPD resets it itself
uint32_t counter_value = 0;
buffer(Renderer_Buffer::StorageSpd)->Update(&counter_value);
buffer(Renderer_Buffer::StorageSpd)->Update(nullptr, &counter_value);
}

uint32_t stride = static_cast<uint32_t>(sizeof(Sb_Material)) * rhi_max_array_size;
Expand Down

0 comments on commit 90f287d

Please sign in to comment.