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

Don't use std::variant in FCesiumPropertyTableProperty #1241

Merged
merged 5 commits into from
Oct 5, 2023

Conversation

j9liu
Copy link
Contributor

@j9liu j9liu commented Oct 4, 2023

PropertyTablePropertyViewType was a typedef for a large std::variant defined in Cesium Native. It was so large that all of the normalized property versions had to be put in their own std::variant, and the two were then grouped under their own std::variant in Cesium for Unreal. While Visual Studio's compiler was able to handle this well, Clang would experience incredibly slow compilation times and often would not be able to complete CI.

This replaces the std::variant with a std::any. The original property type is stored in the FCesiumMetadataValueType, and the struct can be used to re-find the C++ type to std::any_cast to. This allows Clang + CI to finally compile successfully, and to somewhat reduce the size of our object files.

@cesium-concierge
Copy link

Thanks for the pull request @j9liu!

  • ✔️ Signed CLA found.

Reviewers, don't forget to make sure that:

@kring kring merged commit f258271 into ue5-main Oct 5, 2023
25 of 28 checks passed
@kring kring deleted the dont-use-variant branch October 5, 2023 04:58
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.

3 participants