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

vkGetMemoryWin32HandleKHR behavior should be more explicit #2420

Open
Agrael1 opened this issue Aug 23, 2024 · 5 comments
Open

vkGetMemoryWin32HandleKHR behavior should be more explicit #2420

Agrael1 opened this issue Aug 23, 2024 · 5 comments

Comments

@Agrael1
Copy link

Agrael1 commented Aug 23, 2024

Hello, I'm investigating the behavior of interop functions for device memory allocations.

I have stumbled upon this spec for win32 handles. After analyzing the behavior on multiple graphics cards the behavior is different. One returns the same handle, the second returns different handle.

I'd like to propose a fix, to make the API more stable, and add opportunity to create handles for the same memory the same way that Fd does: Creating a duplicate handle on subsequent calls. Making them open a duplicate handle will just make internal shared counter increment, so it wouldn't be much of an overhead.

Every such handle will need to be closed.

Proposed change:

VUID-VkMemoryGetWin32HandleInfoKHR-handleType-00663
If handleType is defined as an NT handle, vkGetMemoryWin32HandleKHR must be called no more than once for each valid unique combination of memory and handleType

shall be removed from the spec.

@cubanismo
Copy link

After analyzing the behavior on multiple graphics cards the behavior is different. One returns the same handle, the second returns different handle.

I assume you're saying when you call vkGetMemoryWin32HandleKHR() on a (memory object, handle type) pair multiple times, the behavior of graphics drivers differs? That's expected given the VU makes that undefined behavior.

I'd like to propose a fix,

We don't generally change the spec in backwards-incompatible ways and/or ways that make it inconsistent with existing driver behavior unless we're relatively certain it won't break any applications in the field relying on the old behavior, nor break any future application running on existing drivers but expecting the new behavior, with the rare exception being when there is no other option to resolve a fundamental flaw in the spec. This might meet the first qualification, but not the second, and I don't think it's dire enough to make it worth the risk absent further justification (I.e., 'this could be cleaner' isn't enough to warrant breaking with existing driver behavior). Are you aware of applications in which the currently specified behavior is causing issues?

@Agrael1
Copy link
Author

Agrael1 commented Aug 24, 2024

I see, despite it not being ABI break, it is backward incompatible.

May then the fix be the introduction of vkGetMemoryWin32HandleKHR2 or new extension structure to retrieve same or duplicated handle?

I am thinking of Vulkan Memory Allocator, for which this consistency would be beneficial, since resource allocation is detached from memory allocation and it is encouraged to do so.It makes it hard to predict the behavior of interop.

This may cause issues e.g. for Prime. It currently runs on NVidia, but if we switch to AMD it will leak memory.

Fix on higher level is possible, I am working on custom function for vma, but otherwise it is non-trivial. It requires some kind of map {VkDeviceMemory -> HANDLE}, which adds a lot of unnecessary bloat to the otherwise linear application.

@HansKristian-Work
Copy link
Contributor

This may cause issues e.g. for Prime.

What does this mean? Prime is a Linux thing, but you're talking about Win32 here. Can you clarify?

@Agrael1
Copy link
Author

Agrael1 commented Aug 28, 2024

Chyron Prime to be exact.
We have fixed the issue locally with Vulkan Memory Allocator, but library implementation would still be nice.

@r-potter
Copy link
Contributor

r-potter commented Oct 4, 2024

The Working Group discussed this, and reached the same conclusion as @cubanismo expressed above:

  • We can't remove the VU without breaking backwards compatibility
  • We technically could specify a new extension, but the use case is narrow and we don't think drivers would end up doing anything with the handles that application's can't already do themselves with the Windows APIs.

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

No branches or pull requests

4 participants