D3D11 resources refactor and DX11.1 feature detection fixes

[CookiePLMonster]

This PR reworks feature set detection in Direct3D 11 backend. Currently false assumptions seem to be present - namely logic op support is assumed to be there when DX11.1 is present. This is not true, and it leads to graphical artifacts on Windows 7 in eg. in Super Mario Wii.

Effectively implements:
https://bugs.dolphin-emu.org/issues/10482

The plan for DX11 on Windows 7 is to allow the user to continue using it (with forementioned glitch fixed), but let the user know that artifacts may occur. Warning currently looks like this:

With this implemented, Mario Kart Wii looks significantly more playable on Windows 7. It probably has glitches elsewhere, but since logic op is not hacked/emulated it's bound to happen.

Before:
https://imgur.com/VBEcorP

After:
https://imgur.com/jvzuCdn

[stenzek]

I don't think using D3D11 (minus .1) will change the black box behavior - that is a result of missing logic op support. The only reason it "worked" prior to the 11.1 change was because of the "emulation" of logic op using blend modes, which was only correct for a few of the operators. I don't see any point in re-introducing such a hack for a 10-year old OS which even MS will no longer be supporting as of next year.

However, if we're crashing there, then fixing that would be nice. I don't have any Win7 boxes anymore, so I've never tested it ;) If assuming D3D11.1 implies logic op support is incorrect, you'd probably want to use ID3D11Device::CheckFeatureSupport to determine whether it is supported. See https://docs.microsoft.com/en-gb/windows/win32/api/d3d11/ns-d3d11-d3d11_feature_data_d3d11_options.

Although, that page says Note This structure is supported by the Direct3D 11.1 runtime, which is available on Windows 8 and later operating systems., which would imply you wouldn't get an 11.1 interface on Win7.. but the documentation isn't always correct..

[Helios747]

I would drop "logic operations in blend state", nobody is going to know what that means, or care. I would either drop that part or simplify it to "The Direct3D 11 renderer requires features that your system configuration blah blah blah"

[CookiePLMonster]

I don't see any point in re-introducing such a hack for a 10-year old OS which even MS will no longer be supporting as of next year.

I don't intend to re-introduce a hack. Code relying on DX11.1 is currently executed conditionally, but it in fact requires a stronger guarantee, that is it requires DX11.1 with logic op. I only intend to fix those checks, not introduce hacky code paths. I don't care what happens with this skipped, as long as it does not crash.

However, incidentally, skipping DX11.1 code chunk "fixes" this issue - that's how the game looks when I don't instantiate a DX11.1 device (which is effectively a single line change):

If assuming D3D11.1 implies logic op support is incorrect, you'd probably want to use ID3D11Device::CheckFeatureSupport to determine whether it is supported.

That's exactly how I'll do it - I already ran tests and it gives expected results.

Although, that page says Note This structure is supported by the Direct3D 11.1 runtime, which is available on Windows 8 and later operating systems., which would imply you wouldn't get an 11.1 interface on Win7.. but the documentation isn't always correct

True, I guess the only page which got updated is this, which clearly states Win7 with a platform update receives partial support (no logic ops for Win7 guys):
https://docs.microsoft.com/en-us/windows/win32/direct3d11/direct3d-11-1-features

Later I'll also uninstall a platform update on Win7 to see if Dolphin still crashes without it (as mentioned in the issue I linked initially). I don't think anyone should support this per se, but we can all agree that it should not crash, but instead try to show the user a descriptive message, just like when d3dcompiler_47.dll is not present.

[CookiePLMonster]

Ownership fixes I pushed now streamline ComPtr usages in D3DState and DXTexture. This ensures managed pointers are not constructed from raw pointers obtained from another managed pointer etc. This is mostly cosmetic (only reduces the amount of refcount changes), but it also actually fixes at least one resource leak - ID3D11BlendState1 created in StateCache::Get had an outstanding reference left after emplacing to the cache.

[stenzek]

What do you think about dropping the "temporary device" completely, and making GetAAModes() just use the global device? This would also disable things like bounding box for older hardware. I'm thinking something like:

void VideoBackend::FillBackendInfo()
{
  // Are we booting a game or just filling the info for the options dialog?
  const bool booting_game = static_cast<bool>(D3D::device);
  if (!booting_game)
  {
    // Create a temporary device used to populate optional backend features.
    if (!D3D::Create(g_Config.iAdapter, false))
      WARN_LOG(VIDEO, "Failed to create temporary D3D device");
  }

  // [...]

  if (D3D::device)
  {
    g_Config.backend_info.AAModes = D3D::GetAAModes();
    g_Config.backend_info.bSupportsLogicOp = D3D::LogicOpSupported();
    g_Config.backend_info.bSupportsST3CTextures = ...
    // [...]
  }

  if (!booting_game)
  {
    // Clean up the temporary device used for populating features.
    D3D::Close();
  }
}

[CookiePLMonster]

I would love to agree with that, but there is a problem with this when polling for logic op support (so I know whether or not to show the warning message). FillBackendInfo for this backend is only created after backends change, and that is before the warning message shows. I don't want to call FillBackendInfo before the warning, because it operates on global state and the user could still change their mind and switch back to the previous backend - potentially leaving config in some weird state.

What I would do instead is ensure FillBackendInfo does not operate on global state - instead, make it fill a local temporary variable and "commit" it when switching backends. This would allow me to fill those as soon as backend change is requested (without polluting the global state with maybe-to-be-rolled-back changes), then actually use those to determine whether to show the warning or not, then commit them if user says yes to switching backends.

This will also potentially rquire changes to dxgi.dll loading as it'll create the case where D3D11 backend attempts to load it (and unload it after) after D3D12 has loaded it. A simple reference counter to DLL loads (instead of the boolean present now) will solve this cleanly.

[stenzek]

I don't see why dxgi.dll would be an issue - it should be unloaded after populating the structure if D3D12 was selected.

Returning the features from FillBackendInfo() would probably be best, avoiding the global state entirely, but that's a separate issue.

It's kinda highlighting another flaw though - if D3D12 was selected and a user switched to D3D11, the adapter index may not line up, and it'd end up querying the incorrect device. For this, I think displaying the warning message as a text box in the configuration dialog would be better than a message box.

A text box would be invariant to system/hardware changes too. e.g. if you switched GPUs, and the new GPU didn't support Vulkan, or was missing logic op support, the warning message could indicate that without having to switch to and from another backend.

Edit: For this, I'm thinking something like returning the struct from FillBackendInfo(), and constructing the warning message entirely in the frontend. After all, the logic op issue applies to OpenGL ES as well ;)

[CookiePLMonster]

Those start to sound pretty invasive, do you think it should be part of this PR? I think at this point it should be split, to ease review and potentially get it merged quicker (as @JMC47 wants it in ASAP). Then those could be addressed as another, lower priority PR in the future.

EDIT:
That said, I totally agree FillBackendInfo should just return a structure, sounds like that would be the most robust.