nvapi-adapter: Free Vulkan after adapter initialization

[jp7677]

Just free the resources, including the DXGIFactory, once we have all the info we need. Also use the existing DXGIFactory to obtain the Vulkan entry points.

@Saancreed Any thoughts on this? In my opinion it is less clean, but freeing the resources once we don't need them anymore feels good too.

[Saancreed]

No strong opinions here. On one hand, I like how we don't persist anything Vulkan-related after the NvapiAdapter is intialized (at which point we don't need any of that anymore) and how dependencies are injected from the outside (might make testing easier), but I don't like how it's now the caller's responsibility to keep the factory alive for as long as Vulkan might be used. And I don't understand COM stuff well enough to be able to clearly tell which one is better.

[jp7677]

Yeah. these are my hesitations too. From what I understood: if we don't keep the factory alive, its ref counter might go back to zero after going out of scope in our code, which in turn might clean up the factory and thus might lead to unload dxvk (or vulkan in dxvk) which invalidates the pointer we have and thus lets things explode if we use the Vulkan entrypoint at this stage. May be it is worth to still hold another reference in the Vulkan class itself (but not as std::unique_pointer like before), that also increases/decreases the reference count as long as it is alive. That at least would remove that hidden dependency.

[Saancreed]

May be it is worth to still hold another reference in the Vulkan class itself (but not as std::unique_pointer like before), that also increases/decreases the reference count as long as it is alive.

Was is actually unique_ptr before? Looks like the previous revision has Com<…> which is quite reasonable to keep.

I'd probably go with the following:

1. Let ResourceFactory::CreateVulkan accept DXGI factory but then forward it to returned Vulkan instance and keep Com<IDXGIFactory> there like before.
2. Pass Vulkan to NvapiAdapter::Initialize instead of the constructor, and don't keep that reference anywhere in NvapiAdapter after Initialize returns, like in this PR.

This way the one instance we create in NvapiAdapterRegistry::Initialize should be freed nicely when the method returns and there should be no chance of Vulkan outliving DXGI factory because it keeps its own reference to it.

Of course, this only works because we can pull all the data we need from Vulkan at initialization time, which is great right now but let's hope it won't turn out to be too limiting in the future.

[jp7677]

May be it is worth to still hold another reference in the Vulkan class itself (but not as std::unique_pointer like before), that also increases/decreases the reference count as long as it is alive.

Was is actually unique_ptr before? Looks like the previous revision has Com<…> which is quite reasonable to keep.

ah yeah, weak moment of myself when writing this ;).

I'd probably go with the following:

1. Let `ResourceFactory::CreateVulkan` accept DXGI factory but then forward it to returned `Vulkan` instance and keep `Com<IDXGIFactory>` there like before.

2. Pass `Vulkan` to `NvapiAdapter::Initialize` instead of the constructor, and don't keep that reference anywhere in `NvapiAdapter` after `Initialize` returns, like in this PR.

This way the one instance we create in NvapiAdapterRegistry::Initialize should be freed nicely when the method returns and there should be no chance of Vulkan outliving DXGI factory because it keeps its own reference to it.

Of course, this only works because we can pull all the data we need from Vulkan at initialization time, which is great right now but let's hope it won't turn out to be too limiting in the future.

Yeah, this looks indeed like the sanest solution. I've validated the reference balance manually with some sophisticated std::cout debugging techniques and all is neutral from out side after initialization. I've figured that DXVK increases the factory reference counter when calling EnumAdapters, though that is DXVK internal magic.

Anyway, thanks for reviewing!