[PATCH v7 4/4] winevulkan: Implement VK_KHR_external_memory_win32 for images.

[Zebediah Figura (she/her)]

On 6/1/21 10:17 AM, Derek Lesho wrote:
> 
> On 5/31/21 10:43 AM, Zebediah Figura (she/her) wrote:
>> On 5/26/21 9:21 AM, Derek Lesho wrote:
>>> On 5/25/21 10:07 PM, Zebediah Figura (she/her) wrote:
>>>
>>>>
>>>>>>
>>>>>> * Shared resources in d3d9-11, when using the OpenGL backend
>>>>>
>>>>> As mentioned, the EXT_external_objects extension exists, (
>>>>> https://www.khronos.org/registry/OpenGL/extensions/EXT/EXT_external_objects.txt
>>>>>
>>>>>
>>>>> ), which accepts and produces file descriptors compatible with the
>>>>> ones
>>>>> Vulkan produces and accepts.  The interactions here seem to be well
>>>>> defined.
>>>>
>>>> According to my reading, neither GL_EXT_external_objects nor
>>>> GL_EXT_external_objects_fd provide for export of an object from an
>>>> OpenGL context; they only allow importing objects created by Vulkan.
>>>
>>> Yeah, from what I can see, openGL has no standard way to share memory
>>> across processes without the use of this extension and Vulkan. I
>>> suspect this would be difficult due to all the user mode driver code
>>> protecting against invalid use of resources.  I think the most
>>> reasonable path forward is to create a Vulkan dummy object in wined3d,
>>> then import it into GL.  Of course, this would make shared resources
>>> unsupported for old GPUs and systems that don't support Vulkan, but the
>>> API is half a decade old at this point, and only GPUs from almost a
>>> decade ago don't support it.
>>
>> There is no free software NVidia Vulkan driver, which is a rather
>> glaring hole. There also exist cards released as recently as 2014 that
>> don't support Vulkan. And in general, although it's sometimes
>> convenient for the implementer to require the latest hardware, it's a
>> rather mean thing to do to users.
>>
>> Keep in mind that we need Direct3D shared resources for bug 44985,
>> which affects a very long list of applications.
>>
>> Now, with that in mind, two options I immediately see are:
>>
>> * Create all contexts as shared. It's not clear to me whether this has
>> negative consequences, however. It's also not clear to me whether this
>> can be done across processes.
>>
>> * Try to introduce an OpenGL extension that allows exporting file
>> descriptors.
> 
> The second option definitely sounds better, as with the first you're
> still limited to in-process sharing, if my understanding is correct.
> Additionally, the case where you're using wined3d's d3d9 for example,
> create a shared resource, then try importing it into DXVK for d3d10/11,
> would only work with the second solution from what I can see.  I imagine
> that we could cover the vast majority of users by implementing the
> "vulkan helper" path that I suggested before, and quite easily
> transition into using an extension if/when needed.
>>
>>>
>>> Alternatively we could get try to use platform-specific buffer sharing
>>> APIs like EGLStreams or GBM, or just support in-process shared resources
>>> through the eglCreateContext API, which allows the creation of "shared
>>> contexts", in which the GPU resources are shared.
>>>
>>>>>>
>>>>>> * Shared resources between d3d9-12 devices, when using wined3d or
>>>>>> libvkd3d on Windows
>>>>> Putting aside how rare and questionable of a use-case this must be, I
>>>>> don't think this concerns any of my patches.  My future patches would
>>>>> provide a way for translation layers on wine to store metadata
>>>>> alongside
>>>>> the host handlers, it doesn't have any correspondence to Windows.  If
>>>>> you really wanted to (why?), you could probably create your own
>>>>> backing
>>>>> storage in a shared memory object and accept the caveats that come
>>>>> along
>>>>> with that.
>>>>
>>>> I suspect those of us who have worked with Direct3D for multiple
>>>> decades would take issue with "rare and questionable", but regardless,
>>>> part of the point here is to ask: *is* it possible to use a native set
>>>> of APIs, like perhaps the D3DKMT*() functions from gdi32, to share
>>>> resources? I.e. should the implementation live in gdi32 instead of
>>>> winevulkan/opengl32/wined3d? Just from a quick scan I see several
>>>> which look like the ones we want, and they're even documented.
>>>
>>> Could you link these APIs so I can look further into them?  I conferred
>>> with Josh and he didn't find anything.
>>
>> Sure, here are some APIs that look promising, and are documented:
>>
>> * D3DKMTCreateAllocation()
>> * D3DKMTOpenResource()
>> * D3DKMTOpenResourceFromNtHandle()
>> * D3DKMTQueryResourceInfo()
>> * D3DKMTQueryResourceInfoFromNtHandle()
> 
> So I discussed this with Josh and Georg, and the problems with these
> APIs for our usecases are
> 
> 1) In order to interface with any of these APIs, you need a KMT device
> handle.  This roughly corresponds to a Vulkan device object or OpenGL
> context, but according to Josh they don't have to map 1:1.

It's a bit unfortunate that this discussion still isn't happening in a 
public place, but anyway, we already use KMT device handles; see e.g. 
wined3d_output_init().

> 
> 2) There's no way to update the private runtime data once the resource
> is created.  In the current and simpler flow, the host graphics API will
> create the object, and the translation layer will then attach the needed
> private data.  For this to work on windows, the translation layers would
> have to agree on some way to get Vulkan to set the desired private data,
> and that's assuming that no Windows drivers already use this for their
> own purposes.

Part of my point here is that, ideally, we shouldn't need to. Presumably 
if a Win32 application creates a Vulkan resource and tries to import it 
into Direct3D, the Microsoft Direct3D runtime will call such functions 
to get information from it. It seems at least plausible that we could do 
the same.

There may be any number of foreseen and unforeseen pitfalls along the 
way, which could prevent this approach from working at all, but my point 
is, it's worth testing—and I mean actually testing, not just theorizing 
about what we can and can't do. And even if it doesn't work on Windows, 
it still bears asking whether we should use these APIs for Wine.

And, if the answer ends up being yes, we should start there when 
implementing them. It's far better to plan out the final architecture 
and implement things accordingly, when possible, rather than to start 
implementing things one way and then have to reorganize them later.

> 
>>
>>>
>>> Also, if there are APIs which allow registering resources retrieved from
>>> Vulkan with certain metadata, winevulkan would still have roughly the
>>> same role.  The only difference would be in the translation layers,
>>> where instead of using IOCTLs on the HANDLEs to shared resources they
>>> have to get/set metadata, they would use some D3DKMT API you've found.
>>
>> Well, broadly, yes, but it changes the way that this patch series
>> should be structured and sent.
> 
> This patch's main focus is just setting up the winevulkan infrastructure
> for shared resources.  If we decide to still use D3DKMT for one reason
> or another, the only change to the current patchset would be to the tiny
> function create_gpu_resource to get the HANDLE through KMT APIs instead
> of wine_server_fd_to_handle.
>>
>>>
>>>>
>>>>>>
>>>>>> I note your tests also don't seem very interesting; they only share
>>>>>> objects within the same device. Sharing objects across multiple
>>>>>> devices, and across multiple processes, strikes me as more
>>>>>> interesting.
>>>>>
>>>>> In my opinion, given that the semantics for VK_KHR_external_memory_fd
>>>>> and VK_KHR_external_memory_win32 are so similar, I wonder how relevant
>>>>> such tests would be, it seems like the kind of thing the Vulkan CTS
>>>>> should add for testing drivers.
>>>>>
>>>>> I think the tests should be more for testing the properties of the
>>>>> HANDLEs we get, as this is what we are implementing ourselves. Maybe I
>>>>> should add more tests looking at the locations of the named objects in
>>>>> the NT directory, or tests relating to types of the objects and their
>>>>> access permissions.
>>>>>
>>>>>
>>>>
>>>> The point is more of a smoke test, because it's easy to get it wrong,
>>>> and not too hard to verify that we got it right.
>>>>
>>>> It'd also be nice to demonstrate what the behaviour of KMT handles is
>>>> across processes.
>>>>
>>> An in-process intra-device test tests the same amount of winevulkan
>>> behavior as a cross-process inter-device test.  All the implementation
>>> details of cross-process and cross-device sharing are handled on the
>>> host and through wineserver's well established HANDLE<->FD mapping.
>>>
>>
>> Is it really that hard just to add a test?
> 
> 
> No, but I just think it would be a waste of time and lines of code,
> especially for testing cross-process when it makes no difference which
> processes the two devices are made on.  Given that you said "a test", do
> you mean you think it would be fine if I were to just add a test testing
> shared resources across devices in the same process?
> 
> 

It's not that hard to write a cross-process test; see kernel32:pipe for 
an example. We have infrastructure for it.

For a rationale: even if the code path isn't significantly different 
now, it might be in the future, especially when this patch series seems 
far from maturity. And, like I said, we have no tests for KMT handles 
across processes.