[x64] [SDL] build failure during make - ID3D12Device

[patrickenfuego]

64-bit build.

This merge was made on 6/5/22 with mentions of ID3D12Device, and the author says they modified CMakeLists but only tested compilation through Visual Studio.

        Starting 64bit compilation of video tools
  Running git update for vpx...
vpx git  .................................................... [Up-to-date]
  Running git update for vmaf...
vmaf git  ................................................... [Up-to-date]
  Running git update for aom...
aom git  .................................................... [Up-to-date]
  Running git update for dav1d...
dav1d git  .................................................. [Up-to-date]
  Running git update for SDL...
┌ SDL git  ............................................ [Recently updated]
├ Running autogen...
├ Running configure...
├ Running make...
Likely error (tail of the failed operation logfile):
 2739 |     result = ID3D12Device_CreateCommittedResource(data->d3dDevice,
      |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/render/direct3d12/SDL_render_d3d12.c:2813:9: warning: passing argument 4 of 'readbackBuffer->lpVtbl->Map' from incompatible pointer type [-Wincompatible-pointer-types]
 2813 |         &textureMemory
      |         ^~~~~~~~~~~~~~
      |         |
      |         BYTE ** {aka unsigned char **}
../src/render/direct3d12/SDL_render_d3d12.c:2813:9: note: expected 'void **' but argument is of type 'BYTE **' {aka 'unsigned char **'}
cc1.exe: some warnings being treated as errors
make: *** [Makefile:461: build/SDL_render_d3d12.lo] Error 1

logs.zip

[Biswa96]

I have compiled latest SDL repository in msys2/mingw environment with cmake and it compiled successfully. The autotools and cmake files in SDL only add -Werror=declaration-after-statement which is different than -Wincompatible-pointer-types.

[chalonverse]

It looks like actually your error is with the autogen/configure workflow, not with the cmake workflow. Of note, it does successfully compile with the autogen/configure workflow on msys2/mingw in the automated builds.

Looking at the full log for the SDL build, it looks like at least one issue is that the version of the Windows SDK you have is not new enough -- so even though there is a d3d12.h it does not have all the expected declarations. The issue with the (void **) conversion is also there. I can fix those, but the larger issue here is just we need to add an SDK version check for the configure/cmake workflows, I think?

[patrickenfuego]

@chalonverse thank you for the insight! I went back and reviewed the logs again myself, and wondered if my Win SDK was out of date, although not my area of expertise at all.

In the meantime, I'm guessing an update should fix? I uninstalled Visual Studio recently to save disk space, so I wonder if that removed some of the header files I had the last time I ran the build 🤔

[chalonverse]

I've figured out how to correctly check for the required Windows SDK version in the build scripts, so I'll be making a PR for that which hopefully will get merged soon.

The PR also fixes those warnings (although it should be noted, the warnings weren't what caused your failure).

[chalonverse]

The PR is here: libsdl-org/SDL#5771

And yes if you install the current version of the Windows 10 SDK from here: https://developer.microsoft.com/en-us/windows/downloads/windows-sdk/, that should also fix the issue

[patrickenfuego]

Updated my Win SDK per the provided link you gave me @chalonverse, but my build is still failing at SDL.

logs.zip

Seems to be some new errors though, so perhaps progress?

[chalonverse]

Ok so it turns out, the reason this happens is actually due to MinGW having their own d3d12 headers that are super old. Frustratingly, it reports a new SDK version even though the headers are not correct.

There's another PR (libsdl-org/SDL#5772) that effectively will disable the d3d12 renderer when running in MinGW until some point in the future when the MinGW developers fix the headers.

[Biswa96]

There is an hidden issue with d3d12 idl/header files explained here https://news.russellsaw.io/article?id=art_4qfDEhAdkMA. The required interfaces are not in mingw-w64 repository yet. I think that SDL code may need some change.

SDL/src/render/direct3d12/SDL_render_d3d12.c: In function 'D3D12_CPUtoGPUHandle':
SDL/src/render/direct3d12/SDL_render_d3d12.c:365:80: error: macro "ID3D12DescriptorHeap_GetCPUDescriptorHandleForHeapStart" passed 2 arguments, but takes just 1
  365 |     ID3D12DescriptorHeap_GetCPUDescriptorHandleForHeapStart(heap, &CPUHeapStart);
      |                                                                                ^

[jenatali]

This issue can be worked around by removing the dependency on the Windows SDK for the D3D12 headers and instead grabbing them from https://github.com/microsoft/DirectX-Headers. We've had a user working there to add MinGW support to those headers. The C bindings should work for MinGW, but the C++ bindings are actually broken, since MinGW doesn't have the same ABI quirk as MSVC.

[chalonverse]

The most up-to-date SDL will detect that the ID3D12Device5 interface is not available in MinGW and disable the d3d12 renderer code. So, out of the box you shouldn’t get any compile error as it’s not trying to compile that code. I confirmed that was the case locally in a fresh MinGW install, and the CI build on the SDL side also disables the d3d12 renderer when compiling for MinGW.

However, if you mess with the idl/headers included in MinGW you could get it to try to compile which likely will cause the issue with the mismatched signatures.

Unfortunately, I don’t think it’s a viable solution to ignore the officially distributed d3d12 headers in the SDK. That configuration must work for the eventual goal of Xbox GDK support.

On the other hand, it’s not that big of a deal if the d3d12 renderer is excluded from MinGW as there are plenty of other renderers that are available in SDL.

Just to clarify, @Biswa96 are you getting that issue without any changes to the MinGW headers? Because I checked the bleeding edge repo for MinGW and it does not include a declaration for ID3D12Device5 and so the source check the build runs should fail. If you had a previous build without a clean, can you make sure you clean first to force the full configuration checks to run again?

[Biswa96]

OK, let me clarify the situation. I have been experimenting with mingw-w64 headers by adding ID3D12Device5 and more d3d declarations for mesa. But those are not in upstream yet, need some time. So, the ID3D12Device5 check is passed in my case.

In my view, this is not an SDL issue but an issue of the differences between MSVC and mingw-w64 headers generated from IDLs. The same thing also happened here libsdl-org/SDL#5589. Some #ifdef may fix those.

[chalonverse]

Ok, thanks for the clarification.

I think we could add some special case code to account for this difference, but I would need some guidance on a potential fix as I have no idea how the WGI fix was arrived at.

[Biswa96]

I can not provide the actual fix right now. I need to learn what happens behind the MSVC "ABI quirk". But as you have said, there would not be an compiler error out-of-box mingw-w64 toolchain.

[jenatali]

I can not provide the actual fix right now. I need to learn what happens behind the MSVC "ABI quirk". But as you have said, there would not be an compiler error out-of-box mingw-w64 toolchain.

https://devblogs.microsoft.com/oldnewthing/20220113-00/?p=106152

[chalonverse]

So is the fix then that the WIDL compiler needs to add a flag similar to cstruct_out to generate the compatible bindings?

[patrickenfuego]

I just reran the build with the latest commits and SDL was a success. Thank you to everyone for fixing this so quickly!