-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Insert vkDeviceWaitIdle to prevent VK_DEVICE_LOST. #3363
Conversation
I haven't actually looked into it yet, but I suspect frames-in-flight to be dependent on that swapchain that gets destroyed. |
@mcourteaux
Looks like a missing VK capabilities bit? |
I have added this missing bit: m_sci.surface = m_surface;
m_sci.minImageCount = swapBufferCount;
m_sci.imageFormat = surfaceFormat;
m_sci.imageColorSpace = surfaceColorSpace;
m_sci.imageExtent.width = width;
m_sci.imageExtent.height = height;
m_sci.imageUsage = imageUsage;
m_sci.preTransform = VK_SURFACE_TRANSFORM_IDENTITY_BIT_KHR; // <-- Here
m_sci.compositeAlpha = compositeAlpha;
m_sci.presentMode = s_presentMode[presentModeIdx].mode;
m_sci.clipped = VK_FALSE;
result = vkCreateSwapchainKHR(device, &m_sci, allocatorCb, &m_swapChain); But it's still crashing. Something is different though:
I'm on Windows BTW. |
This validation error:
Sounds like you didn't properly adjust |
@mcourteaux |
@bkaradzic This really sounds like the issue I mentioned: there are still frames in flight which were using frames from the swapchain that got destroyed. How would we need to properly fix this?
I'm gathering clues:
@pezcode What are your thoughts on the matter? |
Yes, wait for everything to finish, then do resizing/switching. |
What I mean is not just "wait", but also do an extra flip() to allow the last frame-in-flight corresponding to the old swapchain to be presented before we recreate the swapchain. Or am I talking nonsense? I sort of think this is wrong, because the window already IS resized. But I'm trying to figure out what's the reason for the |
I thought the code already force-waits on a fence here: Line 6824 in dd4199b
So I'm not 100% sure why your fix works compared to the existing code. But I don't have a way to reproduce this, so I can't really help here. Bikeshedding opinion ahead: |
Feel free to try, but back when I wrote this it would happen that the swapchain was invalidated, but the new window size from reset() only made it to the renderer 2 frames later. So if you immediately recreated the swapchain, it would be with the old window size.
In general this should be done for smooth resizing. Basically set the old swapchain handle here: Line 6739 in dd4199b
Then create the new one, and only then destroy the old one. I tried this but kind of gave up because it made the code flow terrible to follow, and it was a minor optimization for visuals while resizing. It could be that some drivers have a fit when creating too many swapchains without reusing the old one, but it's not required by the spec. May or may not solve this issue, not sure. |
Yeah I'm also unable to reproduce locally... Anyone who has this issue do you see it in examples, or only in your app? |
It does crash with examples when modifying them to allow smooth resizing (rendering thread is different from the event thread). |
I was able to reproduce, but since the added |
In unmodified examples from repo? |
Lets continue here: https://discord.com/channels/712512073522872352/1294033853102424125 |
Confirmed that adding m_sci.preTransform = VK_SURFACE_TRANSFORM_IDENTITY_BIT_KHR; // <-- Here |
@pgruenbacher You're stomping over data initialized here: Lines 6736 to 6739 in cc789e8
|
hm ok i'll look into what my root cause was then. |
yea nvm I was just mistaken, testing with latest bgfx and this merged PR resolved it for me. |
Potentially fixes #3227. At least on my machine, aggressively resizing the window no longer causes a device lost. For my setup, this change is an improvement. The idea is taken from here: mpv-player/mpv#8360 (comment)