Fix Windows cmake debug build #976
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Force Shiboken to use detected python libraries instead of using its own library detection logic. - Specify -O compiler option for debug builds like the qmake build. - Remove unconditional NDEBUG define added by the Shiboken cmake helper so that asserts work correctly on debug builds.
This fixes asserts in debug builds where Qt's GL context is set when a ScopedGLContext is created. This can happen when we have a project open and the user tries to open another project. When the event callback is dispatched, the Qt GL context is still set. The fix justs removes the assert and adds logic to save and restore any existing GL context that is set.
acolwell
commented
Jul 2, 2024
|
|
||
| if(WIN32) | ||
| # Debug builds need minimal optimizations to avoid excessive link times and link errors related to Eigen. | ||
| add_compile_options(-O) |
There was a problem hiding this comment.
This option is set in the qmake builds used to build the installer. That is where I got the idea to do this.
There was a problem hiding this comment.
It doesn't appear to be required on Linux. The Eigen linking errors I'm seeing on Windows do not appear to occur on Linux. I'm not entirely sure why. My guess is that Windows is not inlining the Eigen code for some reason but Linux is.
There was a problem hiding this comment.
There are no linking issues on Linux with CMake.
|
|
||
| if(IS_DEBUG_BUILD AND WIN32) | ||
| # Remove NDEBUG from Shiboken2 INTERFACE_COMPILE_DEFINITIONS so it is not inherited in debug builds. | ||
| get_property(ShibokenInterfaceDefs TARGET Shiboken2::libshiboken PROPERTY INTERFACE_COMPILE_DEFINITIONS) |
There was a problem hiding this comment.
This is needed because /mingw64/lib/cmake/Shiboken2-5.15.13/Shiboken2Targets.cmake has code that unconditionally sets NDEBUG. :(
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Thanks for submitting a pull request! Please provide enough information so that others can review your pull request. Additionally, make sure you've done all of these things:
PR Description
What type of PR is this? (Check one of the boxes below)
What does this pull request do?
Fixing Windows debug builds built with cmake.
Have you tested your changes (if applicable)? If so, how?
Yes. I've verified that everything still builds and runs. I've tested a debug build with a few projects and have not observed any issues or asserts. I also ran Natron-Tests with the debug build and all the tests that passed with the 2.5.0 build also pass with this build.