Skip to content

Commit

Permalink
Fix Vulkan swapchain invalidation issue. (#3379)
Browse files Browse the repository at this point in the history 
* Fix Vulkan swapchain invalidation issue.

* Always clamp render pass to frame buffer size.

* Fix formatting.
  • Loading branch information
@mcourteaux
mcourteaux authored Nov 21, 2024
1 parent 4372a1f commit 01af383
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 40 deletions.
3 changes: 3 additions & 0 deletions examples/common/entry/entry.cpp
View file
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
View file
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
View file
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
View file
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

0 comments on commit 01af383

Please sign in to comment.