vulkano icon indicating copy to clipboard operation
vulkano copied to clipboard

Windows: Fullscreen makes acquire_next_image timeout

Open nicbn opened this issue 6 years ago • 12 comments

  • Version of vulkano: 0.11
  • OS: Windows 10
  • GPU (the selected PhysicalDevice): AMD Radeon HD 7800 Series
  • GPU Driver: 17.50.17.03-180131a-323831C-RadeonSoftwareAdrenalin
  • Vulkan Driver Version: 2.0.1
  • Vulkan API Version: 1.0.65
  • Upload of a reasonably minimal complete main.rs file that demonstrates the issue: https://pastebin.com/3WqWux5P (same as triangle example, 4 lines modified)

This can be triggered by changing

let surface = WindowBuilder::new().build_vk_surface(&events_loop, instance.clone()).unwrap();

to

let surface = WindowBuilder::new().with_fullscreen(Some(events_loop.get_primary_monitor())).build_vk_surface(&events_loop, instance.clone()).unwrap();

in the triangle example.

By debugging I found out that after changing to fullscreen 1 or 2 frames may be rendered, but after that the function vulkano::swapchain::acquire_next_image will block the thread forever, freezing the rendering.

After that, if a timeout is not set, the window won't be responsible (the event loop is blocked as it's in the same thread) and you won't be able to close it or switch to another window unless you use CTRL+ALT+DEL and open the task manager - this will remove the focus from the window. Using CTRL+SHIFT+ESC to open the task manager won't work.

Not sure if I'm doing anything wrong or if this is a bug with Vulkano or Winit.

nicbn avatar Jan 23 '19 19:01 nicbn

I just tested on Arch with an Intel HD620 Integrated GPU and the test case seems to work fine. Perhaps this is a windows specific issue. Are you able to run the winit fullscreen example without issues? Does the loop run fine in that example? It would be interesting to see where exactly in vulkano::swapchain::acquire_next_image is getting stuck, perhaps with a debugger or some printlns. I don't have a windows 10 machine handy to test unfortunately.

mitchmindtree avatar Jan 24 '19 03:01 mitchmindtree

Yes, winit fullscreen example runs fine. The error doesn't seem to trigger if the window is not focused.

By debugging, I found out that the program stopped at https://github.com/vulkano-rs/vulkano/blob/a3d503a5e5d256cf10be8c1701b8bfa3bed3efc4/vk-sys/src/lib.rs#L2699

Call stack:

ntdll.dll!00007ffcef20aa24() (Unknown Source:0)
KernelBase.dll!00007ffceb769252() (Unknown Source:0)
amdvlk64.dll!00007ffcaa7c97eb() (Unknown Source:0)
amdvlk64.dll!00007ffcaa5ed0df() (Unknown Source:0)
amdvlk64.dll!00007ffcaa5ed8a1() (Unknown Source:0)
app.exe!vk_sys::DevicePointers::AcquireNextImageKHR(unsigned __int64 self, unsigned __int64 device, unsigned __int64 swapchain, unsigned __int64 timeout, unsigned __int64 semaphore, unsigned int * fence) Line 2713 (c:\Users\MY_USER\.cargo\registry\src\github.com-1ecc6299db9ec823\vk-sys-0.4.0\src\lib.rs:2713)
app.exe!vulkano::swapchain::swapchain::acquire_next_image_raw<winit::Window>(vulkano::swapchain::swapchain::Swapchain<winit::Window> * swapchain, core::option::Option<core::time::Duration> timeout, core::option::Option<vulkano::sync::semaphore::Semaphore<alloc::sync::Arc<vulkano::device::Device>>*> semaphore, core::option::Option<vulkano::sync::fence::Fence<alloc::sync::Arc<vulkano::device::Device>>*> fence) Line 1175 (c:\Users\MY_USER\.cargo\registry\src\github.com-1ecc6299db9ec823\vulkano-0.11.1\src\swapchain\swapchain.rs:1175)
app.exe!vulkano::swapchain::swapchain::acquire_next_image<winit::Window>(alloc::sync::Arc<vulkano::swapchain::swapchain::Swapchain<winit::Window>> swapchain, core::option::Option<core::time::Duration> timeout) Line 84 (c:\Users\MY_USER\.cargo\registry\src\github.com-1ecc6299db9ec823\vulkano-0.11.1\src\swapchain\swapchain.rs:84)
app.exe!app::main() Line 426 (c:\dev\app\src\main.rs:426)
app.exe!std::rt::lang_start::{{closure}}<()>(closure *) Line 74 (c:\rustc\adbfec229ce07ff4b2a7bf2d6dec2d13cb224980\src\libstd\rt.rs:74)
[Inline Frame] app.exe!std::rt::lang_start_internal::{{closure}}() Line 59 (c:\rustc\adbfec229ce07ff4b2a7bf2d6dec2d13cb224980\src\libstd\rt.rs:59)
app.exe!std::panicking::try::do_call<closure,i32>() Line 307 (c:\rustc\adbfec229ce07ff4b2a7bf2d6dec2d13cb224980\src\libstd\panicking.rs:307)
app.exe!panic_unwind::__rust_maybe_catch_panic() Line 102 (c:\rustc\adbfec229ce07ff4b2a7bf2d6dec2d13cb224980\src\libpanic_unwind\lib.rs:102)
[Inline Frame] app.exe!std::panicking::try() Line 286 (c:\rustc\adbfec229ce07ff4b2a7bf2d6dec2d13cb224980\src\libstd\panicking.rs:286)
[Inline Frame] app.exe!std::panic::catch_unwind() Line 398 (c:\rustc\adbfec229ce07ff4b2a7bf2d6dec2d13cb224980\src\libstd\panic.rs:398)
app.exe!std::rt::lang_start_internal() Line 58 (c:\rustc\adbfec229ce07ff4b2a7bf2d6dec2d13cb224980\src\libstd\rt.rs:58)
app.exe!std::rt::lang_start<()>(void(*)() main, __int64 argc, unsigned char * * argv) Line 74 (c:\rustc\adbfec229ce07ff4b2a7bf2d6dec2d13cb224980\src\libstd\rt.rs:74)
app.exe!main() (Unknown Source:0)
[Inline Frame] app.exe!invoke_main() Line 64 (f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:64)
app.exe!__scrt_common_main_seh() Line 259 (f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:259)

The thread std::sys::windows::thread::{{impl}}::new::thread_start stopped at https://github.com/tomaka/winit/blob/c91dfdd6fe8763811f72d6a8a268d10d29b3dc77/src/platform/windows/events_loop.rs#L202

Call stack:

win32u.dll!00007ffcebb51144() (Unknown Source:0)
user32.dll!00007ffcee381b8b() (Unknown Source:0)
app.exe!winit::platform::platform::events_loop::{{impl}}::with_dpi_awareness::{{closure}}(closure) Line 182 (c:\Users\MY_USER\.cargo\registry\src\github.com-1ecc6299db9ec823\winit-0.18.0\src\platform\windows\events_loop.rs:182)
app.exe!std::sys_common::backtrace::__rust_begin_short_backtrace<closure,()>(closure f) Line 138 (c:\rustc\adbfec229ce07ff4b2a7bf2d6dec2d13cb224980\src\libstd\sys_common\backtrace.rs:138)
app.exe!std::thread::{{impl}}::spawn_unchecked::{{closure}}::{{closure}}<closure,()>(closure) Line 480 (c:\rustc\adbfec229ce07ff4b2a7bf2d6dec2d13cb224980\src\libstd\thread\mod.rs:480)
app.exe!std::panic::{{impl}}::call_once<(),closure>(std::panic::AssertUnwindSafe<closure> self) Line 320 (c:\rustc\adbfec229ce07ff4b2a7bf2d6dec2d13cb224980\src\libstd\panic.rs:320)
app.exe!std::panicking::try::do_call<std::panic::AssertUnwindSafe<closure>,()>(unsigned char * data) Line 307 (c:\rustc\adbfec229ce07ff4b2a7bf2d6dec2d13cb224980\src\libstd\panicking.rs:307)
app.exe!panic_unwind::__rust_maybe_catch_panic() Line 102 (c:\rustc\adbfec229ce07ff4b2a7bf2d6dec2d13cb224980\src\libpanic_unwind\lib.rs:102)
app.exe!std::panicking::try<(),std::panic::AssertUnwindSafe<closure>>(std::panic::AssertUnwindSafe<closure> f) Line 286 (c:\rustc\adbfec229ce07ff4b2a7bf2d6dec2d13cb224980\src\libstd\panicking.rs:286)
app.exe!std::panic::catch_unwind<std::panic::AssertUnwindSafe<closure>,()>(std::panic::AssertUnwindSafe<closure> f) Line 398 (c:\rustc\adbfec229ce07ff4b2a7bf2d6dec2d13cb224980\src\libstd\panic.rs:398)
app.exe!std::thread::{{impl}}::spawn_unchecked::{{closure}}<closure,()>(closure) Line 478 (c:\rustc\adbfec229ce07ff4b2a7bf2d6dec2d13cb224980\src\libstd\thread\mod.rs:478)
app.exe!alloc::boxed::{{impl}}::call_box<(),closure>(closure * self, ...) Line 673 (c:\rustc\adbfec229ce07ff4b2a7bf2d6dec2d13cb224980\src\liballoc\boxed.rs:673)
[Inline Frame] app.exe!alloc::boxed::{{impl}}::call_once() Line 683 (c:\rustc\adbfec229ce07ff4b2a7bf2d6dec2d13cb224980\src\liballoc\boxed.rs:683)
[Inline Frame] app.exe!std::sys_common::thread::start_thread() Line 24 (c:\rustc\adbfec229ce07ff4b2a7bf2d6dec2d13cb224980\src\libstd\sys_common\thread.rs:24)
app.exe!std::sys::windows::thread::{{impl}}::new::thread_start() Line 57 (c:\rustc\adbfec229ce07ff4b2a7bf2d6dec2d13cb224980\src\libstd\sys\windows\thread.rs:57)
kernel32.dll!00007ffcee163034() (Unknown Source:0)
ntdll.dll!00007ffcef1e3691() (Unknown Source:0)

Adding a timeout to the acquire_next_image returns an error instead of freezing everything, I added that information to PR body and title. Ignoring the error just makes the error be returned again in the next function call, freezing the loop.

nicbn avatar Jan 30 '19 02:01 nicbn

I got this issue as well, but on arch with integrated intel graphics and version 0.13.

In my case it was unreliable and sometimes would work but sometimes wouldn't. I'm using bspwm so that might make it easier to debug.

I can give some more info if anyone wants to tackle this. I'm surprised more people haven't had this problem.

shtanton avatar Aug 11 '19 17:08 shtanton

Adding a timeout to the acquire_next_image will probably return an error. The swapchain probably needs to be recreated afterwards.

AustinJ235 avatar Oct 24 '19 09:10 AustinJ235

Catching the timeout and recreating a swapchain "works" for one frame, after which the acquire_next_image proceeds to timeout again.

The solution I've found is to create my swapchain with more than one image:

Swapchain::start(device.clone(), surface.clone())
    .num_images(caps.min_image_count.max(2))
    // [...]

This appears to fix the problem on my side, at least in borderless fullscreen on windows.

It might have been more elegant to check the Capabilities of the Surface<Window> again when recreating the swapchain after switching to fullscreen borderless, but on my machine caps.min_image_count remains 1 in that case, so this doesn't work.

clementbellot avatar Jan 30 '22 11:01 clementbellot

What gpu and os is this? I find it weird that 1 minimum image is even a thing. Generally speaking it is "good practice" to use one image over the minimum, but this may be a bug nonetheless.

AustinJ235 avatar Jan 30 '22 11:01 AustinJ235

Could it be a driver bug?

Rua avatar Jan 30 '22 11:01 Rua

Fullscreen borderless should just be a normal window as far as we are concerned I believe, so it is weird. I wonder is the extent that is being used during recreation is not optimal or incorrect?

AustinJ235 avatar Jan 30 '22 11:01 AustinJ235

GPU is an AMD RX 5700 XT, drivers 21.10.4 (October 2021), on windows 10 version 21H2.

1 minimum images doesn't sound too odd when in windowed mode, as the compositor can synchronize itself with the application and copy it to textures it owns, so there would be little use in having more images in the swap chain.

This section of the doc mentions that:

The buffer count must be at least 2.

I'm still not quite sure how vulkano should prevent this, though. Maybe raise an error on Windows in the re-creation of a swapchain with fewer than two images when the window is fullscreen?

clementbellot avatar Jan 30 '22 12:01 clementbellot

There are advantages to using multiple images. Using one image you are going to waiting for the os to release the image before being the next frame. Using multiple images, however; you can begin work on the next frame right away. This can easily double your frame rate. Drivers tend to experience issues when using the bare minimum amount of images. This is why generally searching about vulkan guides it is recommended to use 1 over the minimum.

So things to be investigated:

  • Is acquire_next_image() returning sub_optimal? If this is the case perhaps something is setup incorrectly when creating the swapchain or recreating it. The driver may expect you to only use this swapchain for one more frame.
  • The driver is misreporting the minimum amount of images? In this case it'd be a driver bug.
  • The driver isn't updating the minimum images to reflect the new configuration? Or perhaps for some reason vulkano isn't returning the latest capabilities?
  • There is a synchronization error on your part or inside of vulkano that is causing the previous frame not to be released? Is cleanup_finished being called before the next acquire?

Things to be considered:

  • Should vulkano error when the image count is less than 2, ignoring the capabilities?

AustinJ235 avatar Jan 30 '22 12:01 AustinJ235

  • acquire_next_image() isn't returning sub_optimal.
  • I've had a look on https://vulkan.gpuinfo.org/listdevices.php, and it appears to be just the AMD Windows drivers that have minImageCount=1, the others have minImageCount=2.
  • In the recreate swapchain when the window goes fullscreen, vulkano is calling vkGetPhysicalDeviceSurfaceCapabilitiesKHR with a surface that appears to be correct (the rust-side data knows it's borderless fullscreen, the native handle is opaque, so I couldn't check whether it did know it was borderless fullscreen). And the native struct returned still has minImageCount at 1 (which is incorrect on windows for a borderless fullscreen window that uses the flip model).
  • That doesn't seem relevant given the above point. Also, window resizing without going to fullscreen work just fine, so the swapchain recreation logic code (derived from vulkano's examples) is probably working.

To me, it feels like the minImageCount at 1 when passed a surface that isn't borderless fullscreen in flip mode can be defended (after all, more images means more gpu memory, and not all applications care about the highest performace). However, the device capabilities issue when running borderless fullscreen in flip mode should have minImageCount at at least 2. So it's a driver bug.

Though it is a driver bug, I still feel vulkano might make it a bit easier to do the right thing, maybe by having num_images panic/error when passed anything less than 2, and a separate function like single_image that would be called when someone positively wants to do a single image swapchain. And updating the examples with caps.min_image_count.max(2).

clementbellot avatar Jan 30 '22 17:01 clementbellot

By having num_images panic when 1, people with NVIDIA cards will rely on min_image_count and create perfectly good programs (for them) which don't work for people with AMD GPUs.

IMO just documenting SwapchainBuilder num_images would be fine. Write that you want at least max(min_image_count, 2) or min_image_count + 1, because some drivers are buggy and just min_image_count will cause problems.

Another option would be to override min_image_count and make it 2 if the system says it is 1. ~However if the user is going to use min_image_count + 1 this will be unnecessary and wasteful, so I wouldn't go with this.~ Actually if this is already supposed to be at least 2, I think this would be ok, because min_image_count + 1 is supposed to be 3. This may be the best solution.

nicbn avatar Jan 30 '22 17:01 nicbn

As this problem is still occurring even with current (23.4.1) AMD drivers, and a solution has been identified, shouldn't it documented, if not fixed, in the next version of vulkano? Or identified in the Example files?

I'm not sure what possible issues could come from this fix, if any, and even if it's a driver bug it's not like vulkano shouldn't make it easier to avoid this.

Bob620 avatar Apr 18 '23 05:04 Bob620

My attempt to fix this: https://github.com/vulkano-rs/vulkano/pull/2216

It introduces SwapchainCreateInfo::allow_single_image. If false (as default), the minimum number of swapchain images will be at least 2.

aedm avatar May 31 '23 10:05 aedm