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

Fix validation error when resizing window #80571

Merged
merged 1 commit into from
Aug 16, 2023

Conversation

darksylinc
Copy link
Contributor

Sometimes when resizing the window we may get the following validation error:

ERROR: VALIDATION - Message Id Number: -370888023 | Message Id Name: VUID-vkAcquireNextImageKHR-semaphore-01286
	Validation Error: [ VUID-vkAcquireNextImageKHR-semaphore-01286 ]
Object 0: handle = 0xdcc8fd0000000012, type = VK_OBJECT_TYPE_SEMAPHORE; | MessageID = 0xe9e4b2a9 | vkAcquireNextImageKHR: Semaphore must not be currently signaled or in a wait state. The Vulkan spec states: If semaphore is not VK_NULL_HANDLE it must be unsignaled (https://vulkan.lunarg.com/doc/view/1.2.198.1/linux/1.2-extensions/vkspec.html#VUID-vkAcquireNextImageKHR-semaphore-01286)

In VulkanContext::prepare_buffers the problem was that vkAcquireNextImageKHR returned VK_SUBOPTIMAL_KHR but it already signaled the semaphore (because it is possible to continue normally with a VK_SUBOPTIMAL_KHR result).

Then we recreate the swapchain and reuse the
w->image_acquired_semaphores[frame_index] which is in an inconsistent state.

Fixed by recreating the semamphores along the swapchain.

Fix #80570

@BastiaanOlij
Copy link
Contributor

I'm not sure how to judge this as I've not dived deep enough into this part of the Vulkan API so I'm not sure if there is a better way to do this, but I'm happy for this to be merged.

I would add a remark in the code why we're recreating the semaphores because someone looking at this code later on isn't going to get the why and might break this again.

@darksylinc
Copy link
Contributor Author

Good point.

Done. I've updated the code with a comment explaining why we destroy the semaphore.

@clayjohn clayjohn added this to the 4.2 milestone Aug 13, 2023
Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change makes sense to me. I've wondered for awhile if we needed more careful handling of the swapchain semaphore upon recreating the swap chain. This just confirms my suspicions

@akien-mga
Copy link
Member

Looks good! Could you squash the commits? We usually keep tightly related changes in the same commit, such as here adding context to the change in the first commit (and we don't use the Squash on Merge strategy for a number of reasons, we prefer the history to be final in the PR itself).

Sometimes when resizing the window we may get the following validation
error:

ERROR: VALIDATION - Message Id Number: -370888023 | Message Id Name:
VUID-vkAcquireNextImageKHR-semaphore-01286
	Validation Error: [ VUID-vkAcquireNextImageKHR-semaphore-01286 ]
Object 0: handle = 0xdcc8fd0000000012, type = VK_OBJECT_TYPE_SEMAPHORE;
| MessageID = 0xe9e4b2a9 | vkAcquireNextImageKHR: Semaphore must not be
currently signaled or in a wait state. The Vulkan spec states: If
semaphore is not VK_NULL_HANDLE it must be unsignaled
(https://vulkan.lunarg.com/doc/view/1.2.198.1/linux/1.2-extensions/vkspec.html#VUID-vkAcquireNextImageKHR-semaphore-01286)

In VulkanContext::prepare_buffers the problem was that
vkAcquireNextImageKHR returned VK_SUBOPTIMAL_KHR but it already signaled
the semaphore (because it is possible to continue normally with a
VK_SUBOPTIMAL_KHR result).

Then we recreate the swapchain and reuse the
w->image_acquired_semaphores[frame_index] which is in an inconsistent
state.

Fixed by recreating the semamphores along the swapchain.

Fix godotengine#80570
@darksylinc
Copy link
Contributor Author

Looks good! Could you squash the commits?

Done.

@akien-mga akien-mga merged commit d1b8e9a into godotengine:master Aug 16, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vulkan Validation Error when resizing
4 participants