Conversation
6770052 to
8cb1175
Compare
71e5fdc to
28c47b1
Compare
turanszkij
left a comment
There was a problem hiding this comment.
I have nothing to add, just question is it going to be a breaking change in other people's projects that use Wicked Engine with cmake? Looks like maybe not, but I don't understand changes in the other project cmakes.
|
No, it won’t affect other projects. The target_precompile_headers only uses PRIVATE which means it only applies to when the target itself is built, not when it is used. The changes in the example projects were only needed because they were also changed to use PCH themselves. Since in CMake, PCH are always included when compiling, it caused the embedded imgui to also be compiled with it. Both wicked an imgui use some stb headers and that caused the same function to be defined multiple times resulting in a linker error. I could just have not added the PCH line in the example projects to avoid this, but since mixing third party sources into the “real” sources was kinda iffy anyway, I decided to fix it by actually turning the embedded imgui into a separate target that the example depends on, as an exaple on what should be done in a real project. This way, the PCH only applies to the example sources, not the imgui ones. |
Preliminary results on my laptop without ccache
With the recent changes regarding warnings, PCH on GCC work as well, and compilation is faster for GCC as well with PCH enabled, although only slightly. The difference is a lot larger on Clang.
It also works for cmake on windows, even when using MSVC and CMake, PCH give significant better performance.
See this run for example; ccache was disabled and "ON/OFF" refers to the state of PCH.
Therefore, I think we can always use PCH and don't need a flag for it.