Skip to content

Commit

Permalink
[Vulkan] Fixed the last major validation error, it would manifest as …
Browse files Browse the repository at this point in the history 
…flickering because sometimes an image would be acquired while it was still being presented
  • Loading branch information
@PanosK92
PanosK92 committed Feb 5, 2024
1 parent 1ff9624 commit 301c1de
Show file tree
Hide file tree
Showing 4 changed files with 120 additions and 56 deletions.
21 changes: 13 additions & 8 deletions runtime/RHI/RHI_SwapChain.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,29 +49,29 @@ namespace Spartan
);
~RHI_SwapChain();

// Size
// size
void Resize(uint32_t width, uint32_t height, const bool force = false);
void ResizeToWindowSize();

// HDR
// hdr
void SetHdr(const bool enabled);
bool IsHdr() const { return m_format == format_hdr; }

// VSync
// vsync
void SetVsync(const bool enabled);
bool GetVsync();

void Present();

// Properties
// properties
uint32_t GetWidth() const { return m_width; }
uint32_t GetHeight() const { return m_height; }
uint32_t GetBufferCount() const { return m_buffer_count; }
RHI_Format GetFormat() const { return m_format; }
void* GetRhiRt() const { return m_rhi_rt[m_image_index]; }
void* GetRhiRtv() const { return m_rhi_rtv[m_image_index]; }

// Layout
// layout
RHI_Image_Layout GetLayout() const;
void SetLayout(const RHI_Image_Layout& layout, RHI_CommandList* cmd_list);

Expand All @@ -80,15 +80,15 @@ namespace Spartan
void Destroy();
void AcquireNextImage();

// Main
// main
bool m_windowed = false;
uint32_t m_buffer_count = 0;
uint32_t m_width = 0;
uint32_t m_height = 0;
RHI_Format m_format = RHI_Format::Max;
RHI_Present_Mode m_present_mode = RHI_Present_Mode::Immediate;

// Misc
// misc
uint32_t m_sync_index = std::numeric_limits<uint32_t>::max();
uint32_t m_image_index = std::numeric_limits<uint32_t>::max();
uint32_t m_image_index_previous = m_image_index;
Expand All @@ -97,7 +97,12 @@ namespace Spartan
std::array<std::shared_ptr<RHI_Semaphore>, max_buffer_count> m_acquire_semaphore;
std::vector<RHI_Semaphore*> m_wait_semaphores;

// RHI
// wait for present capability
std::shared_ptr<RHI_Fence> m_present_fence;
void* m_present_cmd_list = nullptr;
void* m_present_cmd_pool = nullptr;

// rhi
void* m_rhi_swapchain = nullptr;
void* m_rhi_surface = nullptr;
std::array<void*, max_buffer_count> m_rhi_rt = { nullptr, nullptr, nullptr};
Expand Down
1 change: 0 additions & 1 deletion runtime/RHI/Vulkan/Vulkan_CommandList.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,6 @@ namespace Spartan
// begin command buffer
VkCommandBufferBeginInfo begin_info = {};
begin_info.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO;
begin_info.flags = VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT;
SP_ASSERT_MSG(vkBeginCommandBuffer(static_cast<VkCommandBuffer>(m_rhi_resource), &begin_info) == VK_SUCCESS, "Failed to begin command buffer");

// update states
Expand Down
152 changes: 106 additions & 46 deletions runtime/RHI/Vulkan/Vulkan_SwapChain.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
#include "../RHI_Device.h"
#include "../RHI_SwapChain.h"
#include "../RHI_Implementation.h"
#include "../RHI_Fence.h"
#include "../RHI_Semaphore.h"
#include "../RHI_CommandPool.h"
#include "../Display/Display.h"
Expand All @@ -41,7 +42,7 @@ using namespace Spartan::Math;
namespace Spartan
{
namespace
{
{
VkColorSpaceKHR get_color_space(const RHI_Format format)
{
// VK_COLOR_SPACE_HDR10_ST2084_EXT represents the HDR10 color space with the ST.2084 (PQ)electro - optical transfer function.
Expand All @@ -53,8 +54,8 @@ namespace Spartan
// When displaying an image in sRGB, the values must be converted to linear space before they are displayed.

VkColorSpaceKHR color_space = VK_COLOR_SPACE_SRGB_NONLINEAR_KHR; // SDR
color_space = format == RHI_Format::R16G16B16A16_Float ? VK_COLOR_SPACE_EXTENDED_SRGB_LINEAR_EXT : color_space; // HDR
color_space = format == RHI_Format::R10G10B10A2_Unorm ? VK_COLOR_SPACE_HDR10_ST2084_EXT : color_space; // HDR
color_space = format == RHI_Format::R16G16B16A16_Float ? VK_COLOR_SPACE_EXTENDED_SRGB_LINEAR_EXT : color_space; // HDR
color_space = format == RHI_Format::R10G10B10A2_Unorm ? VK_COLOR_SPACE_HDR10_ST2084_EXT : color_space; // HDR

return color_space;
}
Expand Down Expand Up @@ -130,7 +131,7 @@ namespace Spartan

for (const VkSurfaceFormatKHR& supported_format : supported_formats)
{
bool support_format = supported_format.format == vulkan_format[rhi_format_to_index(*format)];
bool support_format = supported_format.format == vulkan_format[rhi_format_to_index(*format)];
bool support_color_space = supported_format.colorSpace == color_space;

if (support_format && support_color_space)
Expand Down Expand Up @@ -179,14 +180,14 @@ namespace Spartan
)
{
SP_ASSERT_MSG(RHI_Device::IsValidResolution(width, height), "Invalid resolution");
SP_ASSERT_MSG(buffer_count >= 2, "Buffer can't be less than 2");
SP_ASSERT_MSG(buffer_count >= 2, "Buffer can't be less than 2");

m_format = format_sdr; // for now, we use SDR by default as HDR doesn't look rigth - Display::GetHdr() ? format_hdr : format_sdr;
m_format = format_sdr; // for now, we use SDR by default as HDR doesn't look rigth - Display::GetHdr() ? format_hdr : format_sdr;
m_buffer_count = buffer_count;
m_width = width;
m_height = height;
m_sdl_window = sdl_window;
m_object_name = name;
m_width = width;
m_height = height;
m_sdl_window = sdl_window;
m_object_name = name;
m_present_mode = present_mode;

Create();
Expand Down Expand Up @@ -230,42 +231,42 @@ namespace Spartan
SP_ASSERT_MSG(is_format_and_color_space_supported(surface, &m_format, color_space), "The surface doesn't support the requested format");

// clamp size between the supported min and max
m_width = Math::Helper::Clamp(m_width, capabilities.minImageExtent.width, capabilities.maxImageExtent.width);
m_width = Math::Helper::Clamp(m_width, capabilities.minImageExtent.width, capabilities.maxImageExtent.width);
m_height = Math::Helper::Clamp(m_height, capabilities.minImageExtent.height, capabilities.maxImageExtent.height);

// swap chain
VkSwapchainKHR swap_chain;
{
VkSwapchainCreateInfoKHR create_info = {};
create_info.sType = VK_STRUCTURE_TYPE_SWAPCHAIN_CREATE_INFO_KHR;
create_info.surface = surface;
create_info.minImageCount = m_buffer_count;
create_info.imageFormat = vulkan_format[rhi_format_to_index(m_format)];
create_info.imageColorSpace = color_space;
create_info.imageExtent = { m_width, m_height };
create_info.imageArrayLayers = 1;
create_info.imageUsage = VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT; // fer rendering on it
create_info.imageUsage |= VK_IMAGE_USAGE_TRANSFER_DST_BIT; // for blitting to it
VkSwapchainCreateInfoKHR create_info = {};
create_info.sType = VK_STRUCTURE_TYPE_SWAPCHAIN_CREATE_INFO_KHR;
create_info.surface = surface;
create_info.minImageCount = m_buffer_count;
create_info.imageFormat = vulkan_format[rhi_format_to_index(m_format)];
create_info.imageColorSpace = color_space;
create_info.imageExtent = { m_width, m_height };
create_info.imageArrayLayers = 1;
create_info.imageUsage = VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT; // fer rendering on it
create_info.imageUsage |= VK_IMAGE_USAGE_TRANSFER_DST_BIT; // for blitting to it

uint32_t queueFamilyIndices[] = { RHI_Device::QueueGetIndex(RHI_Queue_Type::Compute), RHI_Device::QueueGetIndex(RHI_Queue_Type::Graphics) };
if (queueFamilyIndices[0] != queueFamilyIndices[1])
{
create_info.imageSharingMode = VK_SHARING_MODE_CONCURRENT;
create_info.imageSharingMode = VK_SHARING_MODE_CONCURRENT;
create_info.queueFamilyIndexCount = 2;
create_info.pQueueFamilyIndices = queueFamilyIndices;
create_info.pQueueFamilyIndices = queueFamilyIndices;
}
else
{
create_info.imageSharingMode = VK_SHARING_MODE_EXCLUSIVE;
create_info.imageSharingMode = VK_SHARING_MODE_EXCLUSIVE;
create_info.queueFamilyIndexCount = 0;
create_info.pQueueFamilyIndices = nullptr;
create_info.pQueueFamilyIndices = nullptr;
}

create_info.preTransform = capabilities.currentTransform;
create_info.preTransform = capabilities.currentTransform;
create_info.compositeAlpha = get_supported_composite_alpha_format(surface);
create_info.presentMode = get_present_mode(surface, m_present_mode);
create_info.clipped = VK_TRUE;
create_info.oldSwapchain = nullptr;
create_info.presentMode = get_present_mode(surface, m_present_mode);
create_info.clipped = VK_TRUE;
create_info.oldSwapchain = nullptr;

SP_VK_ASSERT_MSG(vkCreateSwapchainKHR(RHI_Context::device, &create_info, nullptr, &swap_chain),
"Failed to create swapchain");
Expand Down Expand Up @@ -306,26 +307,26 @@ namespace Spartan
{
RHI_Device::SetResourceName(m_rhi_rt[i], RHI_Resource_Type::Texture, string(string("swapchain_image_") + to_string(i)));

VkImageViewCreateInfo create_info = {};
create_info.sType = VK_STRUCTURE_TYPE_IMAGE_VIEW_CREATE_INFO;
create_info.image = static_cast<VkImage>(m_rhi_rt[i]);
create_info.viewType = VK_IMAGE_VIEW_TYPE_2D;
create_info.format = vulkan_format[rhi_format_to_index(m_format)];
create_info.subresourceRange.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT;
create_info.subresourceRange.baseMipLevel = 0;
create_info.subresourceRange.levelCount = 1;
VkImageViewCreateInfo create_info = {};
create_info.sType = VK_STRUCTURE_TYPE_IMAGE_VIEW_CREATE_INFO;
create_info.image = static_cast<VkImage>(m_rhi_rt[i]);
create_info.viewType = VK_IMAGE_VIEW_TYPE_2D;
create_info.format = vulkan_format[rhi_format_to_index(m_format)];
create_info.subresourceRange.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT;
create_info.subresourceRange.baseMipLevel = 0;
create_info.subresourceRange.levelCount = 1;
create_info.subresourceRange.baseArrayLayer = 0;
create_info.subresourceRange.layerCount = 1;
create_info.components.r = VK_COMPONENT_SWIZZLE_IDENTITY;
create_info.components.g = VK_COMPONENT_SWIZZLE_IDENTITY;
create_info.components.b = VK_COMPONENT_SWIZZLE_IDENTITY;
create_info.components.a = VK_COMPONENT_SWIZZLE_IDENTITY;
create_info.subresourceRange.layerCount = 1;
create_info.components.r = VK_COMPONENT_SWIZZLE_IDENTITY;
create_info.components.g = VK_COMPONENT_SWIZZLE_IDENTITY;
create_info.components.b = VK_COMPONENT_SWIZZLE_IDENTITY;
create_info.components.a = VK_COMPONENT_SWIZZLE_IDENTITY;

SP_ASSERT_MSG(vkCreateImageView(RHI_Context::device, &create_info, nullptr, reinterpret_cast<VkImageView*>(&m_rhi_rtv[i])) == VK_SUCCESS, "Failed to create swapchain RTV");
}
}

m_rhi_surface = static_cast<void*>(surface);
m_rhi_surface = static_cast<void*>(surface);
m_rhi_swapchain = static_cast<void*>(swap_chain);

// semaphores
Expand All @@ -334,6 +335,35 @@ namespace Spartan
string name = (string("swapchain_image_acquired_") + to_string(i));
m_acquire_semaphore[i] = make_shared<RHI_Semaphore>(false, name.c_str());
}

// present wait capability
{
// fence
m_present_fence = make_shared<RHI_Fence>("swapchain_present");

// cmd pool
VkCommandPoolCreateInfo cmd_pool_info = {};
cmd_pool_info.sType = VK_STRUCTURE_TYPE_COMMAND_POOL_CREATE_INFO;
cmd_pool_info.queueFamilyIndex = RHI_Device::QueueGetIndex(RHI_Queue_Type::Graphics);
cmd_pool_info.flags = VK_COMMAND_POOL_CREATE_TRANSIENT_BIT | VK_COMMAND_POOL_CREATE_RESET_COMMAND_BUFFER_BIT;

VkCommandPool cmd_pool = nullptr;
SP_VK_ASSERT_MSG(vkCreateCommandPool(RHI_Context::device, &cmd_pool_info, nullptr, &cmd_pool), "Failed to create command pool");
RHI_Device::SetResourceName(cmd_pool, RHI_Resource_Type::CommandPool, "swapchain_present");
m_present_cmd_pool = cmd_pool;

// cmd list
VkCommandBufferAllocateInfo cmd_list_info = {};
cmd_list_info.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_ALLOCATE_INFO;
cmd_list_info.commandPool = cmd_pool;
cmd_list_info.level = VK_COMMAND_BUFFER_LEVEL_PRIMARY;
cmd_list_info.commandBufferCount = 1;

VkCommandBuffer present_cmd_buffer;
SP_VK_ASSERT_MSG(vkAllocateCommandBuffers(RHI_Context::device, &cmd_list_info, &present_cmd_buffer), "Failed to allocate command buffer");
RHI_Device::SetResourceName(present_cmd_buffer, RHI_Resource_Type::CommandList, "swapchain_present");
m_present_cmd_list = present_cmd_buffer;
}
}

void RHI_SwapChain::Destroy()
Expand All @@ -351,6 +381,13 @@ namespace Spartan

RHI_Device::QueueWaitAll();

// present wait capability
{
m_present_fence = nullptr;
vkFreeCommandBuffers(RHI_Context::device, static_cast<VkCommandPool>(m_present_cmd_pool), 1, reinterpret_cast<VkCommandBuffer*>(&m_present_cmd_list));
vkDestroyCommandPool(RHI_Context::device, static_cast<VkCommandPool>(m_present_cmd_pool), nullptr);
}

vkDestroySwapchainKHR(RHI_Context::device, static_cast<VkSwapchainKHR>(m_rhi_swapchain), nullptr);
m_rhi_swapchain = nullptr;

Expand Down Expand Up @@ -391,14 +428,37 @@ namespace Spartan

void RHI_SwapChain::AcquireNextImage()
{
// get signal semaphore
m_sync_index = (m_sync_index + 1) % m_buffer_count;

// note:
// cpu tracking of semaphore states helps identify logic errors, but doesn't reflect real-time gpu execution
// a semaphore might be marked as 'submitted' cpu-side, yet pending gpu processing
// to ensure synchronization, we submit an empty command buffer with a fence, creating a reliable gpu wait mechanism
{
m_present_fence->Wait();
m_present_fence->Reset();

// create no actual work
VkCommandBufferBeginInfo begin_info = {};
begin_info.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO;
begin_info.flags = VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT;
vkBeginCommandBuffer(static_cast<VkCommandBuffer>(m_present_cmd_list), &begin_info);
vkEndCommandBuffer(static_cast<VkCommandBuffer>(m_present_cmd_list));

// submit the fence
VkSubmitInfo submitInfo = {};
submitInfo.sType = VK_STRUCTURE_TYPE_SUBMIT_INFO;
submitInfo.commandBufferCount = 1;
submitInfo.pCommandBuffers = reinterpret_cast<VkCommandBuffer*>(&m_present_cmd_list);
RHI_Device::QueueSubmit(RHI_Queue_Type::Graphics, 0, m_present_cmd_list, nullptr, nullptr, m_present_fence.get());
}

// get signal semaphore
RHI_Semaphore* signal_semaphore = m_acquire_semaphore[m_sync_index].get();
SP_ASSERT_MSG(signal_semaphore->GetStateCpu() != RHI_Sync_State::Submitted, "The semaphore is already signaled");

m_image_index_previous = m_image_index;

// acquire next image
m_image_index_previous = m_image_index;
SP_VK_ASSERT_MSG(vkAcquireNextImageKHR(
RHI_Context::device, // device
static_cast<VkSwapchainKHR>(m_rhi_swapchain), // swapchain
Expand Down
2 changes: 1 addition & 1 deletion runtime/Rendering/Renderer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ namespace Spartan
SetOption(Renderer_Option::Bloom, 0.03f); // non-zero values activate it and define the blend factor
SetOption(Renderer_Option::MotionBlur, 1.0f);
SetOption(Renderer_Option::ScreenSpaceGlobalIllumination, 1.0f);
SetOption(Renderer_Option::ScreenSpaceShadows, static_cast<float>(Renderer_ScreenspaceShadow::Bend));
SetOption(Renderer_Option::ScreenSpaceShadows, static_cast<float>(Renderer_ScreenspaceShadow::Bend));
SetOption(Renderer_Option::ScreenSpaceReflections, 1.0f);
SetOption(Renderer_Option::Anisotropy, 16.0f);
SetOption(Renderer_Option::ShadowResolution, 2048.0f);
Expand Down

0 comments on commit 301c1de

Please sign in to comment.