Skip to content

Commit

Permalink
Fix Context Validation Layer Errors
Browse files Browse the repository at this point in the history
* Multisampling was wrongly selected, possibly fixes godotengine#49937
* Image semaphore acquisition is now per window, possibly fixes godotengine#41614

Please make sure to test the above two issues again, since I can't reproduce either anyway.
  • Loading branch information
reduz committed Jun 28, 2021
1 parent 89f270f commit 4d19c05
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 16 deletions.
5 changes: 4 additions & 1 deletion drivers/vulkan/rendering_device_vulkan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3524,7 +3524,10 @@ VkRenderPass RenderingDeviceVulkan::_render_pass_create(const Vector<AttachmentF
depth_stencil_reference.layout = VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL;
attachment_last_pass[attachment] = i;

if (!is_multisample_first) {
if (is_multisample_first) {
texture_samples = p_attachments[attachment].samples;
is_multisample_first = false;
} else {
ERR_FAIL_COND_V_MSG(texture_samples != p_attachments[attachment].samples, VK_NULL_HANDLE, "Invalid framebuffer depth format attachment(" + itos(attachment) + "), in pass (" + itos(i) + "), if an attachment is marked as multisample, all of them should be multisample and use the same number of samples including the depth.");
}

Expand Down
63 changes: 51 additions & 12 deletions drivers/vulkan/vulkan_context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1013,11 +1013,11 @@ Error VulkanContext::_create_device() {
return OK;
}

Error VulkanContext::_initialize_queues(VkSurfaceKHR surface) {
Error VulkanContext::_initialize_queues(VkSurfaceKHR p_surface) {
// Iterate over each queue to learn whether it supports presenting:
VkBool32 *supportsPresent = (VkBool32 *)malloc(queue_family_count * sizeof(VkBool32));
for (uint32_t i = 0; i < queue_family_count; i++) {
fpGetPhysicalDeviceSurfaceSupportKHR(gpu, i, surface, &supportsPresent[i]);
fpGetPhysicalDeviceSurfaceSupportKHR(gpu, i, p_surface, &supportsPresent[i]);
}

// Search for a graphics and a present queue in the array of queue
Expand Down Expand Up @@ -1091,10 +1091,10 @@ Error VulkanContext::_initialize_queues(VkSurfaceKHR surface) {

// Get the list of VkFormat's that are supported:
uint32_t formatCount;
VkResult err = fpGetPhysicalDeviceSurfaceFormatsKHR(gpu, surface, &formatCount, nullptr);
VkResult err = fpGetPhysicalDeviceSurfaceFormatsKHR(gpu, p_surface, &formatCount, nullptr);
ERR_FAIL_COND_V(err, ERR_CANT_CREATE);
VkSurfaceFormatKHR *surfFormats = (VkSurfaceFormatKHR *)malloc(formatCount * sizeof(VkSurfaceFormatKHR));
err = fpGetPhysicalDeviceSurfaceFormatsKHR(gpu, surface, &formatCount, surfFormats);
err = fpGetPhysicalDeviceSurfaceFormatsKHR(gpu, p_surface, &formatCount, surfFormats);
if (err) {
free(surfFormats);
ERR_FAIL_V(ERR_CANT_CREATE);
Expand Down Expand Up @@ -1169,9 +1169,6 @@ Error VulkanContext::_create_semaphores() {
err = vkCreateFence(device, &fence_ci, nullptr, &fences[i]);
ERR_FAIL_COND_V(err, ERR_CANT_CREATE);

err = vkCreateSemaphore(device, &semaphoreCreateInfo, nullptr, &image_acquired_semaphores[i]);
ERR_FAIL_COND_V(err, ERR_CANT_CREATE);

err = vkCreateSemaphore(device, &semaphoreCreateInfo, nullptr, &draw_complete_semaphores[i]);
ERR_FAIL_COND_V(err, ERR_CANT_CREATE);

Expand Down Expand Up @@ -1201,6 +1198,19 @@ Error VulkanContext::_window_create(DisplayServer::WindowID p_window_id, VkSurfa
// is created.
Error err = _initialize_queues(p_surface);
ERR_FAIL_COND_V(err != OK, ERR_CANT_CREATE);
} else {
// make sure any of the surfaces supports present (validation layer complains if this is not done).
bool any_supports_present = false;
for (uint32_t i = 0; i < queue_family_count; i++) {
VkBool32 supports;
fpGetPhysicalDeviceSurfaceSupportKHR(gpu, i, p_surface, &supports);
if (supports) {
any_supports_present = true;
break;
}
}

ERR_FAIL_COND_V_MSG(!any_supports_present, ERR_CANT_CREATE, "Surface passed for sub-window creation does not support presenting");
}

Window window;
Expand All @@ -1210,6 +1220,17 @@ Error VulkanContext::_window_create(DisplayServer::WindowID p_window_id, VkSurfa
Error err = _update_swap_chain(&window);
ERR_FAIL_COND_V(err != OK, ERR_CANT_CREATE);

VkSemaphoreCreateInfo semaphoreCreateInfo = {
/*sType*/ VK_STRUCTURE_TYPE_SEMAPHORE_CREATE_INFO,
/*pNext*/ nullptr,
/*flags*/ 0,
};

for (uint32_t i = 0; i < FRAME_LAG; i++) {
VkResult vkerr = vkCreateSemaphore(device, &semaphoreCreateInfo, nullptr, &window.image_acquired_semaphores[i]);
ERR_FAIL_COND_V(vkerr, ERR_CANT_CREATE);
}

windows[p_window_id] = window;
return OK;
}
Expand Down Expand Up @@ -1249,6 +1270,10 @@ VkFramebuffer VulkanContext::window_get_framebuffer(DisplayServer::WindowID p_wi
void VulkanContext::window_destroy(DisplayServer::WindowID p_window_id) {
ERR_FAIL_COND(!windows.has(p_window_id));
_clean_up_swap_chain(&windows[p_window_id]);
for (uint32_t i = 0; i < FRAME_LAG; i++) {
vkDestroySemaphore(device, windows[p_window_id].image_acquired_semaphores[i], nullptr);
}

vkDestroySurfaceKHR(inst, windows[p_window_id].surface, nullptr);
windows.erase(p_window_id);
}
Expand Down Expand Up @@ -1703,6 +1728,8 @@ Error VulkanContext::prepare_buffers() {
for (Map<int, Window>::Element *E = windows.front(); E; E = E->next()) {
Window *w = &E->get();

w->semaphore_acquired = false;

if (w->swapchain == VK_NULL_HANDLE) {
continue;
}
Expand All @@ -1711,7 +1738,7 @@ Error VulkanContext::prepare_buffers() {
// Get the index of the next available swapchain image:
err =
fpAcquireNextImageKHR(device, w->swapchain, UINT64_MAX,
image_acquired_semaphores[frame_index], VK_NULL_HANDLE, &w->current_buffer);
w->image_acquired_semaphores[frame_index], VK_NULL_HANDLE, &w->current_buffer);

if (err == VK_ERROR_OUT_OF_DATE_KHR) {
// swapchain is out of date (e.g. the window was resized) and
Expand All @@ -1724,8 +1751,10 @@ Error VulkanContext::prepare_buffers() {
// presentation engine will still present the image correctly.
print_verbose("Vulkan: Early suboptimal swapchain.");
break;
} else if (err != VK_SUCCESS) {
ERR_BREAK(ERR_CANT_CREATE);
} else {
ERR_FAIL_COND_V(err, ERR_CANT_CREATE);
w->semaphore_acquired = true;
}
} while (err != VK_SUCCESS);
}
Expand Down Expand Up @@ -1775,14 +1804,25 @@ Error VulkanContext::swap_buffers() {
commands_to_submit = command_buffer_count;
}

VkSemaphore *semaphores_to_acquire = (VkSemaphore *)alloca(windows.size() * sizeof(VkSemaphore));
uint32_t semaphores_to_acquire_count = 0;

for (Map<int, Window>::Element *E = windows.front(); E; E = E->next()) {
Window *w = &E->get();

if (w->semaphore_acquired) {
semaphores_to_acquire[semaphores_to_acquire_count++] = w->image_acquired_semaphores[frame_index];
}
}

VkPipelineStageFlags pipe_stage_flags;
VkSubmitInfo submit_info;
submit_info.sType = VK_STRUCTURE_TYPE_SUBMIT_INFO;
submit_info.pNext = nullptr;
submit_info.pWaitDstStageMask = &pipe_stage_flags;
pipe_stage_flags = VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT;
submit_info.waitSemaphoreCount = 1;
submit_info.pWaitSemaphores = &image_acquired_semaphores[frame_index];
submit_info.waitSemaphoreCount = semaphores_to_acquire_count;
submit_info.pWaitSemaphores = semaphores_to_acquire;
submit_info.commandBufferCount = commands_to_submit;
submit_info.pCommandBuffers = commands_ptr;
submit_info.signalSemaphoreCount = 1;
Expand Down Expand Up @@ -2134,7 +2174,6 @@ VulkanContext::~VulkanContext() {
if (device_initialized) {
for (uint32_t i = 0; i < FRAME_LAG; i++) {
vkDestroyFence(device, fences[i], nullptr);
vkDestroySemaphore(device, image_acquired_semaphores[i], nullptr);
vkDestroySemaphore(device, draw_complete_semaphores[i], nullptr);
if (separate_present_queue) {
vkDestroySemaphore(device, image_ownership_semaphores[i], nullptr);
Expand Down
6 changes: 3 additions & 3 deletions drivers/vulkan/vulkan_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ class VulkanContext {
};

VkInstance inst = VK_NULL_HANDLE;
VkSurfaceKHR surface = VK_NULL_HANDLE;
VkPhysicalDevice gpu = VK_NULL_HANDLE;
VkPhysicalDeviceProperties gpu_props;
uint32_t queue_family_count = 0;
Expand Down Expand Up @@ -101,7 +100,6 @@ class VulkanContext {
VkQueue present_queue = VK_NULL_HANDLE;
VkColorSpaceKHR color_space;
VkFormat format;
VkSemaphore image_acquired_semaphores[FRAME_LAG];
VkSemaphore draw_complete_semaphores[FRAME_LAG];
VkSemaphore image_ownership_semaphores[FRAME_LAG];
int frame_index = 0;
Expand All @@ -121,6 +119,8 @@ class VulkanContext {
VkSwapchainKHR swapchain = VK_NULL_HANDLE;
SwapchainImageResources *swapchain_image_resources = VK_NULL_HANDLE;
VkPresentModeKHR presentMode = VK_PRESENT_MODE_FIFO_KHR;
VkSemaphore image_acquired_semaphores[FRAME_LAG];
bool semaphore_acquired = false;
uint32_t current_buffer = 0;
int width = 0;
int height = 0;
Expand Down Expand Up @@ -208,7 +208,7 @@ class VulkanContext {

Error _create_physical_device();

Error _initialize_queues(VkSurfaceKHR surface);
Error _initialize_queues(VkSurfaceKHR p_surface);

Error _create_device();

Expand Down

0 comments on commit 4d19c05

Please sign in to comment.