imgui icon indicating copy to clipboard operation
imgui copied to clipboard

Fix build of imgui_impl_dx12.cpp with gcc 11

Open yh-sb opened this issue 2 years ago • 8 comments

Fixed https://github.com/ocornut/imgui/issues/4594 (imgui_impl_dx12.cpp compilation issue):

D:\dev\software\x86\c++\projects\opengl\opengl-example\third_party\imgui\backends\imgui_impl_dx12.cpp: In function 'bool ImGui_ImplDX12_CreateDeviceObjects()':
D:\dev\software\x86\c++\projects\opengl\opengl-example\third_party\imgui\backends\imgui_impl_dx12.cpp:517:9: error: 'PFN_D3D12_SERIALIZE_ROOT_SIGNATURE' was not declared in this scope;
did you mean 'PFN_D3D12_SERIALIZE_VERSIONED_ROOT_SIGNATURE'?
  517 |         PFN_D3D12_SERIALIZE_ROOT_SIGNATURE D3D12SerializeRootSignatureFn = (PFN_D3D12_SERIALIZE_ROOT_SIGNATURE)::GetProcAddress(d3d12_dll, "D3D12SerializeRootSignature");
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |         PFN_D3D12_SERIALIZE_VERSIONED_ROOT_SIGNATURE
D:\dev\software\x86\c++\projects\opengl\opengl-example\third_party\imgui\backends\imgui_impl_dx12.cpp:518:13: error: 'D3D12SerializeRootSignatureFn' was not declared in this scope; did
you mean 'D3D12SerializeRootSignature'?
  518 |         if (D3D12SerializeRootSignatureFn == NULL)
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |             D3D12SerializeRootSignature
D:\dev\software\x86\c++\projects\opengl\opengl-example\third_party\imgui\backends\imgui_impl_dx12.cpp:522:13: error: 'D3D12SerializeRootSignatureFn' was not declared in this scope; did
you mean 'D3D12SerializeRootSignature'?
  522 |         if (D3D12SerializeRootSignatureFn(&desc, D3D_ROOT_SIGNATURE_VERSION_1, &blob, NULL) != S_OK)
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The root cause of imgui_impl_dx12.cpp compilation issue: https://docs.microsoft.com/en-us/windows/win32/api/d3d12/nf-d3d12-d3d12serializerootsignature#remarks

This function has been superceded by D3D12SerializeVersionedRootSignature as of the Windows 10 Anniversary Update (14393).

yh-sb avatar Sep 30 '21 22:09 yh-sb

Unversioned root signatures were superseded with versioned ones, but they were not removed and should still work just fine. Is GCC not using the official D3D12 headers?

I don't think it's really appropriate for Dear ImGui to switch to versioned root signatures because it isn't even using the features added by root signature 1.1. Windows 10240 is still supported by Microsoft until 2025, this is an unnecessary breakage for people who still support it.

PathogenDavid avatar Oct 02 '21 17:10 PathogenDavid

We could potentially #ifdef our way around but I agree it would be good to clarify how/why your setup doesn't have D3D12_ROOT_SIGNATURE_DESC in headers?

ocornut avatar Oct 06 '21 10:10 ocornut

I have reworked fix to use #ifdef for MinGW. Also tested with all GCC versions which supports dx12. None of them have PFN_D3D12_SERIALIZE_ROOT_SIGNATURE.

yh-sb avatar Oct 10 '21 14:10 yh-sb

Thank you for checking this! I wonder if this should be testing for a DX12 SDK or Windows SDK version number instead?

ocornut avatar Oct 11 '21 08:10 ocornut

Is GCC not using the official D3D12 headers?

I dug into this a little bit more: MinGW uses its own unofficial headers (likely for legal reasons), and their version is missing PFN_D3D12_SERIALIZE_ROOT_SIGNATURE. These headers are derived from Wine which are in turn derived from vkd3d.

(Compare with the official DirectX 12 headers.)

This is definitely an upstream bug that should be reported and fixed in MinGW, Wine, and vkd3d. (In the long term they should probably just use the real Direct3D 12 headers now that they're MIT-licensed.)


As for Dear ImGui, I think a better fix would be to manually define the function pointer signature in the backend. The current implementation of this PR just causes the same issues I outlined earlier for developers using MinGW.

PathogenDavid avatar Oct 11 '21 09:10 PathogenDavid

I still have this problem when building the DX12 example. Is there some workaround for MinGW?

Matej-Chmel avatar Jan 17 '23 23:01 Matej-Chmel

Short update to @PathogenDavid's last comment:

I dug into this a little bit more: MinGW uses its own unofficial headers (likely for legal reasons), and their version is missing PFN_D3D12_SERIALIZE_ROOT_SIGNATURE. These headers are derived from Wine which are in turn derived from vkd3d.

(Compare with the official DirectX 12 headers.)

This is definitely an upstream bug that should be reported and fixed in MinGW, Wine, and vkd3d. (In the long term they should probably just use the real Direct3D 12 headers now that they're MIT-licensed.)

  • vkd3d already fixed missing D3D12_SERIALIZE_ROOT_SIGNATURE: https://source.winehq.org/git/vkd3d.git/commitdiff/6f1f14d97a9f7688d1c838060180035c15f30613
  • I've created a ticket to wine with request to integrate the fix above: https://bugs.winehq.org/show_bug.cgi?id=54971
  • I've sent a patch with fix to MinGW-w64 project public email ([email protected]):
From d81ef1b970a136fa528d796999ee1675eb6a2897 Tue May 23 00:14:13 2023
From: yhsb2k <[email protected]>
Date: Tue, 23 May 2023 00:14:13 +0300
Subject: [PATCH 1/1] headers: Integrate d3d12 header fix from vkd3d

vkd3d fix: https://source.winehq.org/git/vkd3d.git/commitdiff/6f1f14d97a9f7688d1c838060180035c15f30613

Currently, mingw-w64 uses a slightly outdated version of
mingw-w64-headers/include/d3d12.idl header. MinGW uses
its own unofficial headers and its version is missing PFN_D3D12_SERIALIZE_ROOT_SIGNATURE. These headers are
derived from Wine which are in turn derived from vkd3d.

* vkd3d already fixed missing D3D12_SERIALIZE_ROOT_SIGNATURE: https://source.winehq.org/git/vkd3d.git/commitdiff/6f1f14d97a9f7688d1c838060180035c15f30613
* I've created a ticket to wine with request to integrate the fix above: https://bugs.winehq.org/show_bug.cgi?id=54971
---
 mingw-w64-headers/include/d3d12.idl | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/mingw-w64-headers/include/d3d12.idl b/mingw-w64-headers/include/d3d12.idl
index 4fec32d26..392b64069 100644
--- a/mingw-w64-headers/include/d3d12.idl
+++ b/mingw-w64-headers/include/d3d12.idl
@@ -2568,6 +2568,10 @@ typedef HRESULT (__stdcall *PFN_D3D12_CREATE_VERSIONED_ROOT_SIGNATURE_DESERIALIZ
 [local] HRESULT __stdcall D3D12CreateVersionedRootSignatureDeserializer(
         const void *data, SIZE_T data_size, REFIID iid, void **deserializer);
 
+typedef HRESULT (__stdcall *PFN_D3D12_SERIALIZE_ROOT_SIGNATURE)(
+        const D3D12_ROOT_SIGNATURE_DESC *root_signature_desc,
+        D3D_ROOT_SIGNATURE_VERSION version, ID3DBlob **blob, ID3DBlob **error_blob);
+
 [local] HRESULT __stdcall D3D12SerializeRootSignature(
         const D3D12_ROOT_SIGNATURE_DESC *root_signature_desc,
         D3D_ROOT_SIGNATURE_VERSION version, ID3DBlob **blob, ID3DBlob **error_blob);

Hope these will be integrated soon.

yh-sb avatar May 22 '23 21:05 yh-sb

Thank you @yhsb2k for pushing this topic in the other repos, much appreciated!

ocornut avatar May 24 '23 13:05 ocornut