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 Vulkan swapchain invalidation issue. #3379

Merged
merged 3 commits into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions examples/common/entry/entry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -767,6 +767,7 @@ BX_PRAGMA_DIAGNOSTIC_POP();
handle = size->m_handle;
_width = size->m_width;
_height = size->m_height;
BX_TRACE("Window resize event: %d: %dx%d", handle, _width, _height);

needReset = true;
}
Expand Down Expand Up @@ -800,6 +801,7 @@ BX_PRAGMA_DIAGNOSTIC_POP();
&& needReset)
{
_reset = s_reset;
BX_TRACE("bgfx::reset(%d, %d, 0x%x)", _width, _height, _reset)
bgfx::reset(_width, _height, _reset);
inputSetMouseResolution(uint16_t(_width), uint16_t(_height) );
}
Expand Down Expand Up @@ -979,6 +981,7 @@ BX_PRAGMA_DIAGNOSTIC_POP();
if (needReset)
{
_reset = s_reset;
BX_TRACE("bgfx::reset(%d, %d, 0x%x)", s_window[0].m_width, s_window[0].m_height, _reset)
bgfx::reset(s_window[0].m_width, s_window[0].m_height, _reset);
inputSetMouseResolution(uint16_t(s_window[0].m_width), uint16_t(s_window[0].m_height) );
}
Expand Down
29 changes: 12 additions & 17 deletions examples/common/entry/entry_sdl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ namespace entry

// Force window resolution...
WindowHandle defaultWindow = { 0 };
setWindowSize(defaultWindow, m_width, m_height, true);
entry::setWindowSize(defaultWindow, m_width, m_height);

SDL_EventState(SDL_DROPFILE, SDL_ENABLE);

Expand Down Expand Up @@ -624,7 +624,15 @@ namespace entry
case SDL_WINDOWEVENT_SIZE_CHANGED:
{
WindowHandle handle = findHandle(wev.windowID);
setWindowSize(handle, wev.data1, wev.data2);
uint32_t width = wev.data1;
uint32_t height = wev.data2;
if (width != m_width
|| height != m_height)
{
m_width = width;
m_height = height;
m_eventQueue.postSizeEvent(handle, m_width, m_height);
}
}
break;

Expand Down Expand Up @@ -825,7 +833,7 @@ namespace entry
Msg* msg = (Msg*)uev.data2;
if (isValid(handle) )
{
setWindowSize(handle, msg->m_width, msg->m_height);
SDL_SetWindowSize(m_window[handle.idx], msg->m_width, msg->m_height);
}
delete msg;
}
Expand Down Expand Up @@ -897,20 +905,6 @@ namespace entry
return invalid;
}

void setWindowSize(WindowHandle _handle, uint32_t _width, uint32_t _height, bool _force = false)
{
if (_width != m_width
|| _height != m_height
|| _force)
{
m_width = _width;
m_height = _height;

SDL_SetWindowSize(m_window[_handle.idx], m_width, m_height);
m_eventQueue.postSizeEvent(_handle, m_width, m_height);
}
}

GamepadHandle findGamepad(SDL_JoystickID _jid)
{
for (uint32_t ii = 0, num = m_gamepadAlloc.getNumHandles(); ii < num; ++ii)
Expand Down Expand Up @@ -1011,6 +1005,7 @@ namespace entry

void setWindowSize(WindowHandle _handle, uint32_t _width, uint32_t _height)
{
// Function to set the window size programmatically from the examples/tools.
Msg* msg = new Msg;
msg->m_width = _width;
msg->m_height = _height;
Expand Down
93 changes: 71 additions & 22 deletions src/renderer_vk.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2705,6 +2705,7 @@ VK_IMPORT_DEVICE
m_indexBuffers[_blitter.m_ib->handle.idx].update(m_commandBuffer, 0, _numIndices*2, _blitter.m_ib->data);
m_vertexBuffers[_blitter.m_vb->handle.idx].update(m_commandBuffer, 0, numVertices*_blitter.m_layout.m_stride, _blitter.m_vb->data, true);


VkRenderPassBeginInfo rpbi;
rpbi.sType = VK_STRUCTURE_TYPE_RENDER_PASS_BEGIN_INFO;
rpbi.pNext = NULL;
Expand Down Expand Up @@ -2799,27 +2800,26 @@ VK_IMPORT_DEVICE
return suspended;
}

//BX_TRACE("updateResolution(%d, %d) m_resolution=(%d, %d)"
// , _resolution.width
// , _resolution.height
// , m_resolution.width
// , m_resolution.height
// );

uint32_t flags = _resolution.reset & ~(0
| BGFX_RESET_SUSPEND
| BGFX_RESET_MAXANISOTROPY
| BGFX_RESET_DEPTH_CLAMP
);

// Note: m_needToRefreshSwapchain is deliberately ignored when deciding whether to
// recreate the swapchain because it can happen several frames before submit is called
// with the new resolution.
//
// Instead, vkAcquireNextImageKHR and all draws to the backbuffer are skipped until
// the window size is updated. That also fixes a related issue where VK_ERROR_OUT_OF_DATE_KHR
// is returned from vkQueuePresentKHR when the window doesn't exist anymore, and
// vkGetPhysicalDeviceSurfaceCapabilitiesKHR fails with VK_ERROR_SURFACE_LOST_KHR.

if (false
|| m_resolution.format != _resolution.format
|| m_resolution.width != _resolution.width
|| m_resolution.height != _resolution.height
|| m_resolution.reset != flags
|| m_backBuffer.m_swapChain.m_needToRecreateSurface)
|| m_backBuffer.m_swapChain.m_needToRecreateSurface
|| m_backBuffer.m_swapChain.m_needToRecreateSwapchain)
{
flags &= ~BGFX_RESET_INTERNAL_FORCE;

Expand All @@ -2837,6 +2837,10 @@ VK_IMPORT_DEVICE
preReset();

m_backBuffer.update(m_commandBuffer, m_resolution);
// Update the resolution again here, as the actual width and height
// is now final (as it was potentially clamped by the Vulkan driver).
m_resolution.width = m_backBuffer.m_width;
m_resolution.height = m_backBuffer.m_height;

postReset();
}
Expand Down Expand Up @@ -6849,6 +6853,7 @@ VK_DESTROY
;

const bool recreateSwapchain = false
|| m_needToRecreateSwapchain
|| m_resolution.format != _resolution.format
|| m_resolution.width != _resolution.width
|| m_resolution.height != _resolution.height
Expand Down Expand Up @@ -6962,6 +6967,7 @@ VK_DESTROY
&& NULL != vkCreateWaylandSurfaceKHR
)
{
BX_TRACE("Attempting Wayland surface creation.");
VkWaylandSurfaceCreateInfoKHR sci;
sci.sType = VK_STRUCTURE_TYPE_WAYLAND_SURFACE_CREATE_INFO_KHR;
sci.pNext = NULL;
Expand All @@ -6977,6 +6983,7 @@ VK_DESTROY
&& NULL != vkCreateXlibSurfaceKHR
)
{
BX_TRACE("Attempting Xlib surface creation.");
VkXlibSurfaceCreateInfoKHR sci;
sci.sType = VK_STRUCTURE_TYPE_XLIB_SURFACE_CREATE_INFO_KHR;
sci.pNext = NULL;
Expand All @@ -6996,6 +7003,7 @@ VK_DESTROY

if (NULL != xcbdll)
{
BX_TRACE("Attempting XCB surface creation.");
typedef xcb_connection_t* (*PFN_XGETXCBCONNECTION)(Display*);
PFN_XGETXCBCONNECTION XGetXCBConnection = (PFN_XGETXCBCONNECTION)bx::dlsym(xcbdll, "XGetXCBConnection");

Expand Down Expand Up @@ -7108,8 +7116,9 @@ VK_DESTROY
const VkAllocationCallbacks* allocatorCb = s_renderVK->m_allocatorCb;

// Waiting for the device to be idle seems to get rid of VK_DEVICE_LOST
// upon resizing the window quickly. (See https://github.com/mpv-player/mpv/issues/8360
// and https://github.com/bkaradzic/bgfx/issues/3227).
// upon resizing the window quickly. See:
// - https://github.com/mpv-player/mpv/issues/8360
// - https://github.com/bkaradzic/bgfx/issues/3227
result = vkDeviceWaitIdle(device);
BX_WARN(VK_SUCCESS == result, "Create swapchain error: vkDeviceWaitIdle() failed: %d: %s", result, getName(result));

Expand Down Expand Up @@ -7166,6 +7175,15 @@ VK_DESTROY
, surfaceCapabilities.minImageExtent.height
, surfaceCapabilities.maxImageExtent.height
);
if (width != m_resolution.width || height != m_resolution.height)
{
BX_TRACE("Clamped swapchain resolution from %dx%d to %dx%d"
, m_resolution.width
, m_resolution.height
, width
, height
);
}

VkCompositeAlphaFlagBitsKHR compositeAlpha = VK_COMPOSITE_ALPHA_OPAQUE_BIT_KHR;

Expand Down Expand Up @@ -7292,6 +7310,8 @@ VK_DESTROY
m_backBufferColorImageLayout[ii] = VK_IMAGE_LAYOUT_UNDEFINED;
}

BX_TRACE("Succesfully created swapchain (%dx%d) with %d images.", width, height, m_numSwapChainImages);

VkSemaphoreCreateInfo sci;
sci.sType = VK_STRUCTURE_TYPE_SEMAPHORE_CREATE_INFO;
sci.pNext = NULL;
Expand All @@ -7314,7 +7334,7 @@ VK_DESTROY
m_currentSemaphore = 0;

m_needPresent = false;
m_needToRefreshSwapchain = false;
m_needToRecreateSwapchain = false;

return result;
}
Expand Down Expand Up @@ -7626,7 +7646,7 @@ VK_DESTROY
{
BGFX_PROFILER_SCOPE("SwapChainVK::acquire", kColorFrame);
if (VK_NULL_HANDLE == m_swapChain
|| m_needToRefreshSwapchain)
|| m_needToRecreateSwapchain)
{
return false;
}
Expand All @@ -7652,18 +7672,25 @@ VK_DESTROY
);
}


if (result != VK_SUCCESS)
{
BX_TRACE("vkAcquireNextImageKHR(...): result = %s", getName(result));
}

switch (result)
{
case VK_SUCCESS:
break;

case VK_ERROR_SURFACE_LOST_KHR:
m_needToRecreateSurface = true;
[[fallthrough]];
m_needToRecreateSwapchain = true;
return false;

case VK_ERROR_OUT_OF_DATE_KHR:
case VK_SUBOPTIMAL_KHR:
m_needToRefreshSwapchain = true;
m_needToRecreateSwapchain = true;
return false;

default:
Expand Down Expand Up @@ -7712,15 +7739,21 @@ VK_DESTROY
result = vkQueuePresentKHR(m_queue, &pi);
}

if (result != VK_SUCCESS)
{
BX_TRACE("vkQueuePresentKHR(...): result = %s", getName(result));
}

switch (result)
{
case VK_ERROR_SURFACE_LOST_KHR:
m_needToRecreateSurface = true;
[[fallthrough]];
m_needToRecreateSwapchain = true;
break;

case VK_ERROR_OUT_OF_DATE_KHR:
case VK_SUBOPTIMAL_KHR:
m_needToRefreshSwapchain = true;
m_needToRecreateSwapchain = true;
break;

default:
Expand Down Expand Up @@ -7883,8 +7916,10 @@ VK_DESTROY
BGFX_PROFILER_SCOPE("FrameBufferVK::update", kColorResource);
m_swapChain.update(_commandBuffer, m_nwh, _resolution);
VK_CHECK(s_renderVK->getRenderPass(m_swapChain, &m_renderPass) );
m_width = _resolution.width;
m_height = _resolution.height;
// Don't believe the passed Resolution, as the Vulkan driver might have
// specified another resolution, which we had to obey.
m_width = m_swapChain.m_sci.imageExtent.width;
m_height = m_swapChain.m_sci.imageExtent.height;
m_sampler = m_swapChain.m_sampler;
}

Expand Down Expand Up @@ -8563,12 +8598,26 @@ VK_DESTROY
if (isFrameBufferValid)
{
viewState.m_rect = _render->m_view[view].m_rect;
const Rect& rect = _render->m_view[view].m_rect;
const Rect& scissorRect = _render->m_view[view].m_scissor;
Rect rect = _render->m_view[view].m_rect;
Rect scissorRect = _render->m_view[view].m_scissor;
viewHasScissor = !scissorRect.isZero();
viewScissorRect = viewHasScissor ? scissorRect : rect;
restoreScissor = false;

// Clamp the rect to what's valid according to Vulkan.
rect.m_width = bx::min(rect.m_width, fb.m_width - rect.m_x);
rect.m_height = bx::min(rect.m_height, fb.m_height - rect.m_y);
if (_render->m_view[view].m_rect.m_width != rect.m_width
|| _render->m_view[view].m_rect.m_height != rect.m_height)
{
BX_TRACE("Clamp render pass from %dx%d to %dx%d"
, _render->m_view[view].m_rect.m_width
, _render->m_view[view].m_rect.m_height
, rect.m_width
, rect.m_height
);
}

rpbi.framebuffer = fb.m_currentFramebuffer;
rpbi.renderPass = fb.m_renderPass;
rpbi.renderArea.offset.x = rect.m_x;
Expand Down
2 changes: 1 addition & 1 deletion src/renderer_vk.h
Original file line number Diff line number Diff line change
Expand Up @@ -762,7 +762,7 @@ VK_DESTROY_FUNC(DescriptorSet);
VkSemaphore m_lastImageAcquiredSemaphore;

bool m_needPresent;
bool m_needToRefreshSwapchain;
bool m_needToRecreateSwapchain;
bool m_needToRecreateSurface;

TextureVK m_backBufferDepthStencil;
Expand Down
Loading