dxvk icon indicating copy to clipboard operation
dxvk copied to clipboard

[dxgi] Only enum outputs which belong to an adapter.

Open gofman opened this issue 3 years ago • 4 comments

That's what I see dxgi doing on Windows, and the method of selecting outputs seems to work both in Wine and Windows.

Fixes Monster Hunter Rise in case of multiple adapters which gets a bit mad when dxgi outputs don't match what it gets from QueryDisplayConfig(). Here on a dual GPU laptop (AMD) it keeps requerying display configs and resizing swapchain each frame which effectively results in not working alttabbing out of the game, unability to change display settings (like resolution, monitor) which gets reset to the previous value immediately and hang on exit. With the patch all that seems to be fixed, switching monitor through menu also works correctly.

gofman avatar May 31 '22 00:05 gofman

Why does this work? data->adapterName is pretty much guaranteed to mismatch other device names since we get that from the Vulkan driver, and I'm not really sure how the EnumDisplayDevicesA loop relates to Vulkan adapters.

I'm not sure if it is possible to implement correct behaviour with our DXGI in all situations, e.g. when users have multiple AMD drivers installed, which would result in the same device being reported multiple times. We'd need to change the way we enumerate devices to be consistent with the rest of win32, but I don't know how that works.

doitsujin avatar May 31 '22 12:05 doitsujin

EnumDisplayDevices relates to display adapters, not Vulkan, the names come from Vulkan in Wine now and happen to be the same Windows too (although, tbh, I don’t know if that’s a firm rule on Windows). The very idea is that if we have adxgi adapter which doesn’t correspond to display adapter at all (like \.\display1) or that gdi adapter has no displays the same should be in dxgi.

Will it help if I try to use QueryDisplayConfig? I didn’t want to because our implementation of that depends on EnumDisplayDevices anyway in some aspects, using it is convoluted and I still have to see if we have enough queries for source implemented to end uo getting, e. g. device luid. But probably that would be more straightforward as that what we ultimately want to match.

Also I realized I have to check some things around working with outputs from swapchains, I got a suspicion the current code may rely on dxgi adapters always having the outputs they present to (which will not be the case).

gofman avatar May 31 '22 14:05 gofman

I've redid the original patch on top of QueryDisplayConfig() which is a stricter way to link the devices and doesn't depend on device names.

Then, during testing the patch with various games I figured it breaks Elden Ring on a dual GPU configuration in a way it doesn't see display resolutions and ends up with a broken one which can't be changed. The game depends on the first dxgi adapter having some outputs.

After testing on Windows with dual GPU (discrete / integrated) and plugging the additional external monitor to either adapter I figured the following. Windows always seem to have the first dxgi adapter with nonzero outputs. QueryDisplayConfig() reports the true paths corresponding to real monitor connections (that is, if it is plugged to discrete GPU port it will be always shown as such). dxgi interchanges the outputs between integrated and discrete GPU however based on the discrete / integrated GPU preference. Whatever GPU is used it will have both monitors as outputs from that GPU and that GPU will go first.

So currently dxvk's dxgi moves the discrete GPU first essentially suggesting to force the use of it ("Switchable graphics/ maximize performance" in Windows 10 terms). The second patch adds to this hack by also making all the iGPU outputs reported from the dGPU which will go first. This shouldn't change things for multiple discrete GPU configurations, the output should still be reported correctly (once we were able to get that from xrandr of course in winex11.drv).

gofman avatar Jun 01 '22 01:06 gofman

Since these patches are apparently controversial I am posting a teaser what I could achieve with that (apart from MHR, could make RDR2 sensibly work with multiple monitors for the first time):

IMG-0967

gofman avatar Jun 02 '22 00:06 gofman

I've updated the PR, now properly rebasing it and making the following changes:

  • Fall back to returning all the monitors in case of invalid LUIDs or an unexpected error;
  • Move the logic to wsi;
  • Handle mirrored monitor case. Mirrored monitor is reported as the same gdi device name on Windows, only once in dxgi outputs but multiple times in QueryDisplayConfig (with different source ids). Since recently (and after I submitted the first version of the patches) this is also the case with upstream Wine and Proton;
  • Refactor second patch: make it effective only when we have 1 dGPU and 1 iGPU; avoid extra class variables (just query dxvk adapter for linked adapter).

PS when working with this it actually becomes clear why many games are so picky about the consistent adapter & monitor information reporting and break in various ways when this is not the case. Correctly recovering the adapter / display topology is convoluted on Windows, and if the game wants to be able to work on a specified display and provide a reasonable default for adapter and display, it has to handle all those configs...

gofman avatar Nov 15 '22 19:11 gofman

Merged manually with some minor formatting changes.

doitsujin avatar May 05 '23 14:05 doitsujin