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 OBJCXX debug flags #128

Closed st0rmbtw closed 3 months ago

st0rmbtw commented 3 months ago

Metal renderer compiles without the LLGL_DEBUG define, because it's written in Objective-C++, but the ADD_DEBUG_DEFINE cmake function sets a define only for C++ via CMAKE_CXX_FLAGS_DEBUG

Solution:

Closes https://github.com/LukasBanana/LLGL/issues/127

LukasBanana commented 3 months ago

I keep reading online that modifying CMAKE_CXX_FLAGS_DEBUG (and most other CMake specific variables) inside CMakeLists.txt is not intended and can lead to undefined behavior. I can confirm that LLGL_DEBUG is not set in Xcode, neither for LLGL_Metal (ObjC++) nor LLGL_OpenGL (C++); it works with VisualStudio but not with Xcode.

It looks as if add_compile_definitions("$<$<CONFIG:Debug>:${IDENT}>") is a viable replacement for the macro ADD_DEBUG_DEFINE, I would have to increase the minimum CMake version to 3.12, though.

I also wanted to refactor a lot of macro use cases from #ifdef to #if, which might allow to define the macro for older CMake versions as add_definitions("-DLLGL_DEBUG=!NDEBUG") or something similar.

As for your PR, I don't see CMAKE_OBJCXX_FLAGS_DEBUG showing up in my Xcode settings, which might be because I have an older version (11.0), but I also never ran into this issue. If your Metal backend compiles without LLGL_DEBUG defined, so should the main module LLGL. Unless this is one of those undefined behaviors. Since I can't reproduce this issue, can you please try to replace the macro like this:

macro(ADD_DEBUG_DEFINE IDENT)
    add_compile_definitions("$<$<CONFIG:Debug>:${IDENT}>")
endmacro()
st0rmbtw commented 3 months ago

Yes, add_compile_definitions also works as expected.

LukasBanana commented 3 months ago

Before I merge this, I would like to understand how this issue came to be on your Mac, since I can't reproduce it. As mentioned in my previous post, if LLGL_DEBUG is not defined in your _LLGLMetalD.dylib, it also shouldn't be defined in LLGLD.dylib, so this shouldn't have a mismatch. Aren't you building both shared libraries in the same Xcode solution?

Can you please confirm that MTModuleInterface.mm does not have LLGL_DEBUG defined while RenderSystem.cpp does have it defined when you build both libraries with Debug configuration? Maybe with something like this:

// MTModuleInterface.mm
#ifndef LLGL_DEBUG
#error LLGL_DEBUG missing
#endif

// RenderSystem.cpp
#ifndef LLGL_DEBUG
#error LLGL_DEBUG missing
#endif
st0rmbtw commented 3 months ago

Just checked that, yep, without the changes made in this PR it fails in MTModuleInterface.mm with #error LLGL_DEBUG missing.

For context, I don't build the project via Xcode, just cmake with Ninja build system.