search

CesiumGS / cesium-native

Apache License 2.0
391 stars 200 forks source link

Move Abseil code in s2geometry to an isolated namespace. #892

Closed kring closed 1 month ago

kring commented 1 month ago

This builds on #891 so merge that first.

This PR moves the Abseil functionality embedded in the s2geometry library into an isolated namespace (cesium_s2geometry_absl). This avoids linker errors when packaging an Unreal Engine 5.4 game that includes both the Cesium for Unreal and Pixel Streaming plugins, because the Pixel Streaming plugin also embeds some Abseil code.

Fixes CesiumGS/cesium-unreal#1420

Immersiv-1 commented 1 month ago

Hi @kring, with the update. Is there a way we can integrate or access? Thanks

kring commented 1 month ago

Yep, just grab a build from this PR: https://github.com/CesiumGS/cesium-unreal/pull/1431

Instructions here: https://github.com/CesiumGS/cesium-unreal/blob/main/Documentation/using-prerelease-packages.md

kring commented 1 month ago

I agree it could be worth upgrading @csciguy8, but I don't think either of those s2geometry PRs will help us. They would allow us to use an external Abseil, but I don't think that helps.

We can't supply our own Abseil, because Unreal will still have its own, and we'd still have the conflict.

We can't use Unreal's Abseil (even if using libraries from Unreal in cesium-native were easy, which it's not) because Unreal doesn't appear to include headers and an independent library for it. It's just baked into the WebRTC library. In theory we could figure out what version of Abseil Unreal uses, and use the headers for the same version. But we'll still have challenges at link time. In a packaged build where WebRTC and Cesium for Unreal are baked into a single executable, webrtc.lib includes Abseil, and so as long as we're compiled against the right version, everything is fine. However, in a regular Editor build, WebRTC and Cesium for Unreal are in separate DLLs, and the Abseil symbols are almost certainly not exported from the WebRTC DLL. So even if we made Cesium for Unreal depend on WebRTC (which we don't want to do), we'd still need to somehow link against Abseil in a non-packaged build in order to avoid undefined symbol linker errors.

So I think the approach in this PR is the best available. Using the preprocessor to rename a namespace is a little hacky, sure, but it's simple and effective, and tightly scoped to just s2geometry (the #define absl doesn't exist except while we're building s2geometry). Theoretical problems we might have with it in the future (such as an unrelated symbol named absl getting inadvertently renamed) are likely to manifest as compiler or linker errors, so it should be relatively safe as well.