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

panic in display_plane_supported_displays #2393

Open
timglauertadder opened this issue Nov 1, 2023 · 10 comments
Open

panic in display_plane_supported_displays #2393

timglauertadder opened this issue Nov 1, 2023 · 10 comments

Comments

@timglauertadder
Copy link

timglauertadder commented Nov 1, 2023

vulkano: 0.34.1

Calling display_plane_supported_displays can cause a panic by calling unwrap on None:

thread 'main' panicked at 'called Option::unwrap() on a None value', /home/tim/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vulkano-0.34.1/src/device/physical.rs:925:66

This is physical.rs lines 921 onwards:

if let Some(display) = self.display_properties.get(&display_vk) {
      display
  } else {
      self.display_properties_unchecked()?;
      self.display_properties.get(&display_vk).unwrap()
  },

The else branch is executed when self.display_properties.get(&display_vk) returns None but the branch calls it again and unwraps it, causing a panic. Presumably self.display_properties_unchecked() is supposed to fix this, but it doesn't on my system.

I found this as part of a wider problem where none of the values returned by display_plane_properties had a current_display that was not None. I may not be initialising something properly, but the code should handle this cleanly rather than calling panic.

@Rua
Copy link
Contributor

Rua commented Nov 1, 2023

What that code is attempting to do, is figure out which display object belongs with the display handle that was returned from the vkGetDisplayPlaneSupportedDisplaysKHR API call. First it looks in its cache of objects it knows about, but this fails, so then it calls display_properties_unchecked, which is then supposed to put all the objects returned from that call into the cache. The final line, then, makes the assumption that the previously missing display handle was returned by vkGetPhysicalDeviceDisplayPropertiesKHR, had an object constructed, and that object must now be in the cache.

The fact that it isn't in the cache is quite strange. That means that vkGetDisplayPlaneSupportedDisplaysKHR returned a display handle that didn't appear in the result of a subsequent vkGetPhysicalDeviceDisplayPropertiesKHR call. I can't really fathom why this would happen.

@Rua
Copy link
Contributor

Rua commented Nov 1, 2023

As a way to confirm this, can you run your code with the Vulkan API dump layer enabled (via vkconfig or directly in your program)? Then specifically find the calls to these two functions, and see what they are returning.

@timglauertadder
Copy link
Author

Is there a vulkano call for this? Not able to get the tools onto the platform I am using.

@Rua
Copy link
Contributor

Rua commented Nov 1, 2023

No, it's part of the Vulkan SDK.

@timglauertadder
Copy link
Author

timglauertadder commented Nov 6, 2023

The BestPractices validation layer is giving this warning:

UNASSIGNED-BestPractices-vkGetDisplayPlaneSupportedDisplaysKHR-properties-not-retrieved` validation warning: Validation Warning: [ UNASSIGNED-BestPractices-vkGetDisplayPlaneSupportedDisplaysKHR-properties-not-retrieved ] Object 0: handle = 0x55ee0f169b30, type = VK_OBJECT_TYPE_PHYSICAL_DEVICE; | MessageID = 0xe3d810d8 | Potential problem with calling vkGetDisplayPlaneCapabilities2KHR() without first retrieving properties from vkGetPhysicalDeviceDisplayPlanePropertiesKHR or vkGetPhysicalDeviceDisplayPlaneProperties2KHR.

However I am calling device.display_properties() and device.display_plane_properties() before this message appears, and before calling display_plane_capabilities(0) on the first mode of the display. Really not sure what is going on.

@timglauertadder
Copy link
Author

It looks like this is a lifetime issue in my code.

This code fails:

let displays = device.display_properties().expect("display properties");
for display in displays {
   ...
   device.display_plane_supported_displays(p_num).unwrap();

Using an explicit .iter() fixes the problem:

for display in displays.iter() {

I don't know enough about rust or vulkano to understand exactly why this makes a difference, but it does at least prevent the panic. (Unfortunately the current_display for each surface is still None and I still don't get any pixels)

@Rua
Copy link
Contributor

Rua commented Nov 10, 2023

An API dump of what your program is calling would be very helpful.

@timglauertadder
Copy link
Author

Finally got dump working and have attached the file of a failed run (before the fix I mentioned above).
I have a solution to the immediate problem (the panic) but I was hoping that it might shed light on why there are no displays associated with any of the surfaces.

api_dump.txt

@Rua
Copy link
Contributor

Rua commented Nov 12, 2023

Ok, distilling out the important bits, the dump contains this sequence of calls related to displays:

  • vkGetPhysicalDeviceDisplayProperties2KHR. Returns 2 display handles 0xfab64d0000000002 and 0xfa21a40000000003.
  • vkGetPhysicalDeviceDisplayPlaneProperties2KHR. Returns 6 display planes, but all of them with display handle 0 as the current display.
  • vkGetDisplayModeProperties2KHR with display handle 0xfab64d0000000002. Returns 38 display mode handles.
  • vkGetDisplayPlaneSupportedDisplaysKHR with plane index 0. Returns 1 display handle 0xfab64d0000000002.
  • vkGetDisplayPlaneCapabilities2KHR with display mode handle 0xf56c9b0000000004 and plane index 0. Call succeeds.
  • vkCreateDisplayPlaneSurfaceKHR with display mode handle 0xf56c9b0000000004 and plane index 0. Call succeeds.
  • vkGetDisplayModeProperties2KHR with display handle 0xfa21a40000000003. Returns 39 display mode handles (one more than before), but none of them are the handle 0xf56c9b0000000004 that was used before.
  • vkGetDisplayPlaneSupportedDisplaysKHR with plane index 0. Returns 1 display handle 0xfab64d0000000002, same as before.
  • vkGetPhysicalDeviceDisplayProperties2KHR. Returns 2 display handles 0xfab64d0000000002 and 0xfa21a40000000003, same as before.

After this point, objects are destroyed and the dump ends, so I assume the crash happens here.

It looks like getting None as the current_display is something the Vulkan driver itself is doing, so at least that part is not a Vulkano bug. Other than that, at first glance, nothing seems out of the ordinary as far as the function calls go. It is a bit strange that, after the display surface is created, there's three more calls to display-related functions. I'm not really sure what those are there for.

@timglauertadder
Copy link
Author

The code is trying to create a surface on each display, so it looks like the code after vkCreateDisplayPlaneSurfaceKHR is reading data for the second display. The final call to vkGetPhysicalDeviceDisplayProperties2KHR is the one in display_plane_supported_displays but for some reason it doesn't populate the display_properties field which is why there is the panic.

I really think this is a resource lifetime issue in my code when I wasn't using .iter() in the outer loop, so not really worth persuing further. As you say, the problem with a Null value for current_display is a driver problem, not a vulkano problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants