LukasBanana / LLGL

Low Level Graphics Library (LLGL) is a thin abstraction layer for the modern graphics APIs OpenGL, Direct3D, Vulkan, and Metal
BSD 3-Clause "New" or "Revised" License
2.1k stars 139 forks source link

Fix DLLGL_DX_ENABLE_D3D12 causing multiple same-signature function destruction. #96

Closed gdianaty closed 1 year ago

gdianaty commented 1 year ago

The Problem

LLGL's macro LLGL_DX_ENABLE_D3D12 is used to provide differentiation between D3D11 and D3D12 for DXCore.cpp and DXTypes.cpp. When compiled statically, multiple versions with identical signatures of the functions in DXCore.cpp are created. This results in the linker's first-come-first-serve approach to multiply-defined, same-signature behavior to kick in.

The practical result of this is that when statically compiled LLGL_Direct3D11 and LLGL_Direct3D12 coexist in the same project, and you query LLGL::RenderSystem::GetRendererInfo() on D3D12, the returned value for LLGL::RendererInfo::rendererName is something like DirectX 11.2, and not DirectX 12.1.

The (Proposed) Solution

Remove LLGL_DX_ENABLE_D3D12. LLGL's shared DX code is very robust and in my testing I have found that Occam's Razor works in this problem's favor. The symbols that are wrapped with LLGL_DX_ENABLE_D3D12 macros exist across versions of DirectX in the dxcommon.h header file, so D3D11 and D3D12 both pass and the symbols are able to kumbaya.

One thing of note is I am unsure if VS2015 (used by @LukasBanana) also keeps the variables that are currently wrapped in LLGL_DX_ENABLE_D3D12 in dxcommon.h, and I don't see a CI job for VS2015 compiles.

LukasBanana commented 1 year ago

I'm generally fine with this change but I'd like to make sure it also works with environments that have older Windows SDKs. This CI job uses the build engine for VS2015 (14.0.25420.1) and passes: https://ci.appveyor.com/project/LukasBanana/llgl/builds/47897846/job/nohwyn4o3go2k6b8

But let me take a look on my local VS2015 installation later today.

gdianaty commented 1 year ago

That was just what I was hoping you'd do! Thanks.

Let me know if you get anything unexpected and I'll revisit this problem with a multiplicity of compilers in mind.

LukasBanana commented 1 year ago

I just confirmed the issue. I thought this was a linker problem, but the problem is that the D3D12 backend will simply lack the respective entries for D3D_FEATURE_LEVEL when the other object code for DXCore.cpp was selected. Thanks for bringing this to my attention and again fixing this nicely!

Currently, the minimum required Windows SDK I can have installed to support all the D3D12 related identifiers is 10.0.10240.0, which is old enough to demand it. I might just add a similar CMake option as with D3D11 minor version to select the minimum required WinSDK.