Conversation
| FILE WickedEngineTargets.cmake | ||
| NAMESPACE WickedEngine:: | ||
| DESTINATION ${INSTALL_CONFIGDIR}) | ||
| # I don't quite get why, but this conflicts with specifying the include dirs in Editor |
There was a problem hiding this comment.
IIRC install is currently broken anyway, we can just comment it out for now
There was a problem hiding this comment.
I think the generated files should all be put into build (aswell as the compiled shaders) so you don't have to run Editor from inside the WickedEngine dir, but the build dir. (if that makes sense)
The installation can be a pain with CMake. Maybe I can look at it sometime, since I have some experience with that, but mostly for linux specifically and statically linked libraries.
|
@SiebrenW BTW, check out the build failures, there currently are some problems on Linux GCC (invalid flag gets added now) and on Windows. (And since my communication style often catches people off guard: your work on this PR is appreciated) |
SiebrenW
force-pushed
the
fix/macos_build_cmake
branch
from
d0586a7 to
c81b5bb
Compare
SiebrenW
force-pushed
the
fix/macos_build_cmake
branch
2 times, most recently
from
ab04522 to
1250d15
Compare
Editor/CMakeLists.txt
Outdated
|
|
||
| if (APPLE) | ||
| list(APPEND SOURCE_FILES main_MacOS.mm) | ||
| # PCH has disabled objective C, so don't use PCH |
There was a problem hiding this comment.
btw, what do you mean by "has disabled objective C", do you mean they are broken in obj C?
There was a problem hiding this comment.
There was some warning about saying "disabled ojective C" related to the precompiled headers. I didn't really find it necessary to find out how to re-enable it, so I just disabled PCH for the objective C files.
There was a problem hiding this comment.
My point is that the comment is very confusing, and I have no idea what it's supposed to tell me. "We don't use PCH on Mac because PCH disables objective C". Could you maybe recheck what the warning says specifically? According to my (admittedly very short) research objective C does support PCH, so I'm a bit confused why it needs to be disabled.
I'm fine with disabling it if it doesn't work in our case for some reason, but we should document the exact reason (just so we/somebody else can re-enable it in the future when that reason doesn't exist anymore)
For context: I've worked with quite a few projects that had workarounds like this that stopped being needed for 5 years, but everybody had assumed they still are. That's why I'm a bit anal about carefully documenting special cases.
There was a problem hiding this comment.
Yes, sorry. I'm at work and will be busy/not have access to my Mac until the afternoon (CET) tomorrow, but I was hoping this would be at least a bit informative.
If I had my mac on me I would have elaborated better with snippets and all. Probably just fixed it 😐 .
Thank you for taking the time to review by the way. My intent was to have it as a throwaway hack to get it working, but I'll take it a bit more seriously (hopefully) tomorrow during my break.
There was a problem hiding this comment.
The error in particular is error:
Objective-C was disabled in precompiled file '.../WickedEngine/build/Editor/CMakeFiles/Editor.dir/cmake_pch.hxx.pch' but is currently enabled
1 error generated.
I think the real issue is down to the fact that the precompiled header is a pure C++ header, not objective C. I couldn't find a way to enable it for both, so I think disabling it has the least impact. Then we have 1 file built without PCH, which isn't such a big deal I think.
There was a problem hiding this comment.
According to this thread it could be all that's needed is adding LANGUAGES C CXX OBJC OBJCXX to the target line(s)
There was a problem hiding this comment.
We can probably omit C but better first test it with to make sure it's not required when using OBJC
There was a problem hiding this comment.
Interesting. That did work.
Now the question is: do we really want this? I don' know what the impact is on other targets. I can simply throw it over the fence and see what the pipeline does I suppose.
There was a problem hiding this comment.
I think we would need LANGUAGES CXX on Windows/Linux (maybe LANGUAGES C CXX, not sure) and add objective C for mac, something like
if(APPLE)
set(OBJ_C_LANGS OBJC OBJCXX)
endif()
project(... LANGUAGES C CXX ${OBJ_C_LANGS})(I don't quite remember from the top of my head if CMake treats undefined vars as empty string, if not, we'd just use a set(LANGUAGES C CXX) for linux and the 4 for mac)
There was a problem hiding this comment.
It's not working for all other platforms, I will stick with the exclusion.
| target_compile_definitions(WickedEngine_common PUBLIC _XM_F16C_INTRINSICS_) | ||
| endif() | ||
| if (USE_FMADD) | ||
| target_compile_options(WickedEngine_common PUBLIC -mfma) |
SiebrenW
force-pushed
the
fix/macos_build_cmake
branch
from
1250d15 to
05c6b5b
Compare
SiebrenW
force-pushed
the
fix/macos_build_cmake
branch
from
05c6b5b to
f050c4a
Compare
2 participants
So I tried building the MacOS version of the engine, since I just got a MacBook, but I did notice that XCode is a massive POS and I couldn't even figure out what setting to change to make it work. (because its error messages are 20 clicks away and cryptic as they can be)
After messing quite a bit with it I gave up and I spent significantly less time to fix/hack it in CMake, which is my preference anyway (which is a low bar).
Since it's not production ready or anything I figured it may be helpful to have it. I still notice some wonkiness, but at least it builds.
Apple clang was crashing when building, so I switched to llvm clang. I also suppressed/solved some warnings.
Cheers.