Use vcpkg version of cesium-native #479
Conversation
So far it is just a copy of the official vcpkg 2024.02.14 version.
Specifically this fix: Amanieu/asyncplusplus#47 Unfortunately, 2.5 years later, a new official version including that fix still hasn't been released. This is a copy of cesium-unreal commit: CesiumGS/cesium-unreal@d5bb575
|
I'm struggling with the Android build, specifically with installing OpenSSL from vcpkg. Unity includes the Android NDK, which is installed to And then fails later with: Usually a good strategy when dealing with broken software that can't handle spaces in paths is to use the old school 8.3 version of the path. So I tried setting the The original author closed it without a fix in CMake after they worked around it with some complicated code to create NTFS junction points. I'll probably end up doing the same. By the way, this wasn't an issue before vcpkg because we avoided the need for an actual OpenSSL dependency by carving out the bits of s2geometry we actually need and leaving out the bits that depend on OpenSSL. Doing the same with vcpkg would be...tricky. |
|
Today's problem is that the Android build is failing only on CI. The reason appears to be that the GitHub runner has a recent version (v27) of the Android NDK installed, and the This alone wouldn't be a problem, except for this issue: The linked issue says this will be addressed in NDK v28, but it's not completely clear this will fix the problem for us. Recent versions of the Android NDK's CMake toolchain require CMake 3.6, where's asyncplusplus only requires CMake 3.1. We're of course using a plenty-new CMake, but CMake has a whole policy system whereby newer versions behave like older ones based on the declared required version of the project. In any case, I'm going to try to force our builds to use Unity's copy of the NDK, which should fix the problem. |
This is normal for UWP builds.
10.13 doesn't support std::filesystem::path.
These were copied from cesium-unreal.
At commit 00a26bd8823f3e9b69d86ec584eb4580e601ac1b.
This will hopefully fix bitcode conflicts because newer versions of xcode don't do bitcode at all.
Sometimes, during the tests, the managed CesiumIonSession could get garbage collected before the requests made as part of the "resume" completed. Because we were using the corresponding Impl class in the continuations, and the Impl class's lifetime is controlled by the lifetime of the managed object, this would sometimes cause a crash. The solution implemented here is to make sure the continuations also keep the managed instance alive. We also check to see if the held managed instance becomes nullptr, which handles causes like AppDomain unload.
So hopefully we'll get ARM on ARM.
If we don't specify, it sits there waiting for us to choose.
This will hopefully drastically speed up the macOS build because it won't have to wait ages for the audio system to time out.
Build and CI improvements, building on vcpkg
There was a problem hiding this comment.
@kring looks great! I won't pretend like I completely understand CMake stuff, but everything else looks good to me.
I left some comments, and could you also update CHANGES.md + resolve the conflicts with main? (I'm worried if I do the latter I'll mess something up 😅 )
I'd also like to merge the cesium-native PR before we merge this one, but I didn't see that one marked as ready.
|
Thanks for reviewing @j9liu, this is ready for another look. |
|
Final testing across all platforms: ❌ means I cannot test because I don't have access to the necessary hardware.
|
|
I've done all the platform testing I can here, too, so this is ready. |
|
Thanks @kring ! Merging now 😎 |
There's a lot going on this PR. Here's a summary.
vcpkg
We're now using the vcpkg-ified version of cesium-native from CesiumGS/cesium-native#820.
Unity 2022.3.41f.
Previously we used 2021.3.13f1. This means we can actually compile and test all of our code, including
CesiumCartographicPolygonwhich requires 2022.3. The plugin will still work with 2021.3 after building with 2022.3, but there is a chance we might miss compatibility problems with the older release. 2021.3 will be out of support as soon as Unity 6 is released, though, so it should be a very short-term problem.This is probably a good change in general, but the immediate motivation for making the switch here was that 2021.3 uses an older version of the Android NDK that uses Clang 9, which doesn't have very good C++20 support. 2022.3 uses Clang 12 on Android, which is much better.
To use the new Unity version to do builds, I had to fix a longstanding bug where the build process was baking-in all code that came out of a Roslyn code generator. This was fine in 2021.3 because Reinterop was the only Roslyn code generator. In 2022.3 this is no longer true, and it was breaking the build.
C++ 20
The C++ code in Cesium for Unity (but not cesium-native) now target C++20. This is necessary to use swl-variant.
swl-variant
This is an alternate
variantimplementation that requires C++20 and has much better compile-time performance in terms of both CPU time and memory usage. Like I did with Unreal, I've only switched the variants in cesium-unity itself to use this implementation. cesium-native still usesstd::variant, though it may be worthwhile to change that too at some point.macOS builds now run on macOS 14 / ARM64
Previously we were stuck on macOS 12 and x64. With the reduced compile-time memory usage from switching to swl-variant, we're no longer prevented from using GitHub's Apple Silicon runners (which are weirdly memory constrained). This gets us onto the more modern platform (OS and processor architecture) and speeds up our builds.
Builds are faster
In main, builds took over 2 hours on Windows and almost an hour and a half on macOS. In this branch, total CI times on both platforms are cut approximately in half. This is a result of swl-variant, faster runners, and caching builds of third-party dependencies via vcpkg. I also modified the build process to immediately exit Unity after the native code finished building, saving a bunch of pointless work building a Player application that we don't actually need.
The improved build times are also a result of disabling Unity's audio system during our build. Incredibly, on macOS, Unity was doing...something...with audio for 20 minutes every time it started up, presumably because the GitHub runners can't actually do audio or somesuch. Disabling it massively reduced our macOS build times.
Native code build logs are uploaded as build artifacts
Previously, if the native code failed to compile on one of our many platforms, it was hard to figure out what went wrong. We had to add a bit of script to dump the log to the build output, and run the build again. Now, a build artifact is created with all of the native build logs, so they can easily be inspected if needed.
Consistent macOS and iOS version targetting
Previously, we were targeting macOS 12.7 in our Editor build of the plugin. And 10.13 for Player builds. Now, we're targetting 10.15 for both. We moved from 10.13 to 10.15 because 10.13 doesn't support
std::filesystem::path.On iOS, we previously weren't attempting to specify a target version, so we were getting xcode's default (whatever that is). Now we're explicitly targetting iOS 12, which is the minimum version supported by Unity 2022.3.Update: While we weren't explicitly specifying a target before, we were (perhaps coincidentally) getting iOS 12 anyway.Floating point comparisons
When we started running the tests on an Apple Silicon Mac, we initially had some test failures. Apple's ARM processors seem to be a bit less tolerant of testing floating point equality compare to x64 processors. So I added some epsilons to some comparisons in tests. I also changed
CesiumFlyToController.DetectMovementInputto only allow movement of more than 1/1000 of a meter to cancel an in-progress flight.CesiumIonSession crash
We have sometimes seen crashes related to CesiumIonSession. It became quite consistent when running the tests in Unity 2022.3. The crash was caused by a race condition where the managed
CesiumIonSessionwas garbage collected (perhaps due to AppDomain unload) while it was still attempting to resume a previous session, resulting in a use-after-free. I fixed this by holding a reference to the managed object while requests are in flight, and checking when they complete that the managed object is still valid.Fixes #432
(I think!)