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

Improves handling of window resizes on vulkan (avoids crashes) #2123

Merged
merged 2 commits into from
Jul 18, 2020

Conversation

kingscallop
Copy link
Contributor

No description provided.

Copy link
Owner

@bkaradzic bkaradzic left a comment

Choose a reason for hiding this comment

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

This should be handled inside updateResolution. This change would cause renderer to flicker.

@kingscallop
Copy link
Contributor Author

kingscallop commented Jul 12, 2020

Sorry for taking so long to reply.

The vulkan spec dictates that on windows and linux/x11 the VkSurfaceCapabilitiesKHR::currentExtent (framebuffer), VkSurfaceCapabilitiesKHR::minImageExtent and VkSurfaceCapabilitiesKHR::maxImageExtent are always equal to the current size of the window. (On wayland, for example, minImageExtent and maxImageExtent are the true hardware capabilities and not the current window size)

I'm going to try to give a specific example to explain this change. Whenever a new frame arrives at bgfx::RenderContextVK::submit(), updateResolution() tries to see if the resolution of the previous rendered frame is the same as the new one. If it is, it doesn't recreate the swapchain, and vkAcquireNextImage() is called. If between the call to vkAcquireNextImage() and vkQueuePresentKHR() the window changes size, the vkQueuePresentKHR() will fail with VK_ERROR_OUT_OF_DATE and the acquired swapchain image will not be returned.

Once the next frame arrives, updateResolution() will once more check if the resolution of the previous frame matches the current frame. Now two things can happen:
1) if the resolution is different, the swapchain gets recreated and all is fine.
2) if the resolution is the same (since the WM_SIZE/CONFIGURE_NOTIFY message could still not have reached the API Thread (game thread)), no recreation of the swapchain is made and a new vkAcquireNextImage() will be performed, which will fail with a validation error because the previous image wasn't returned.

The code in this pull request just avoids making an invalid acquire.

Basically, if the m_resolution of the arriving bgfx::Frame doesn't cause the swapchain to be recreated, which will only happen when it matches the window size (currentExtent), no new frame can be rendered.

Would you prefer this piece of code to be inside updateResolution() which would return a bool and then return from submit() like in the renderer_d3d12.cpp?

|| updateResolution(_render->m_resolution) )

@bkaradzic
Copy link
Owner

Yes, change it to match D3D12.

@kingscallop kingscallop force-pushed the vulkan-window-resize-crash branch from b3fd695 to 0a3fbe5 Compare July 18, 2020 20:11
Rebased and added refresh swapchain check inside updateResolution(),
similar to renderer_d3d12.cpp.
@kingscallop kingscallop requested a review from bkaradzic July 18, 2020 20:29
@bkaradzic bkaradzic merged commit ff9f624 into bkaradzic:master Jul 18, 2020
rozgo pushed a commit to VertexStudio/bgfx that referenced this pull request Aug 14, 2020
…dzic#2123)

* Improves handling of window resizes on vulkan (avoids crashes)

* Change to previous commit as requested.
Rebased and added refresh swapchain check inside updateResolution(),
similar to renderer_d3d12.cpp.
pigpigyyy added a commit to pigpigyyy/bgfx that referenced this pull request Mar 11, 2021
* master: (21 commits)
  Improves handling of window resizes on vulkan (avoids crashes) (bkaradzic#2123)
  Fix crash on window minimize on vulkan renderer (bkaradzic#2204)
  Fix X11 flicker when window is being resized (bkaradzic#2203)
  Updated spirv-cross.
  Updated spirv-tools.
  Updated glslang.
  Updated ImGui.
  Cleanup.
  Fixed docs.
  Fixed issue bkaradzic#2201.
  Updated version number.
  Cleanup.
  OpenGL: Disable scissor testing while blitting framebuffers when MSAA is enabled (bkaradzic#2200)
  Updating xcode config. (bkaradzic#2198)
  Updated glslang.
  shaderc: Disabled warnings.
  Updated spirv-cross.
  Updated spirv-tools.
  Updated spirv-headers.
  Updated vulkan headers.
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants