Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated to vcpkg version of cesium-native #729

Merged
merged 2 commits into from
Sep 13, 2024
Merged

Conversation

lilleyse
Copy link
Contributor

Updates to the latest version of cesium-native, which now uses vcpkg.

Currently targeting the cesium-omniverse branch because of #727.

The ktx and asyncplusplus ports are copied directly from cesium-unity. The ktx port was especially important for fixing the release vs. debug VC runtime discrepancies we were seeing earlier.

Eventually would like to

@@ -1,5 +1,231 @@
include(Macros)

if(MSVC)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we set CESIUM_NATIVE_DEPENDENCIES here to include the common dependencies, then append the platform-specific ones? If we also do this in src/core/CMakeLists.txt, that cuts this list down from four occurrences to two.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll see if we can do that, with the caveat that dependency order sometimes matters.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1aaf653 with a slightly different approach

@corybarr
Copy link
Contributor

  • Use add_subdirectory instead of ExternalProject_add, since that added some complexity here

Is it worth creating an issue for this to improve how handoffable the build system is? I got stuck here, but also haven't managed complex CMake build systems professionally.

@lilleyse
Copy link
Contributor Author

Copy link
Contributor

@corybarr corybarr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Builds on my system. Code changes look good. Good to go.

@lilleyse lilleyse merged commit aaf0696 into main Sep 13, 2024
3 checks passed
@lilleyse lilleyse deleted the update-cesium-native branch September 13, 2024 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove _DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR
2 participants