CesiumGS / cesium-unreal

Bringing the 3D geospatial ecosystem to Unreal Engine
https://cesium.com/platform/cesium-for-unreal/
Apache License 2.0
885 stars 286 forks source link

Add CI for Unreal Engine 5.4 #1403

Closed kring closed 2 months ago

kring commented 2 months ago

This builds on #1385 because main is broken.

Closes #1400

csciguy8 commented 2 months ago

2. Artifact size for 5.4 seems to be ~2x larger than 5.3. This is a bit concerning.

Good news! I diff'd the artifacts and discovered that the problem seems to be in the Intermediate folder. Usually we see Module.CesiumRuntime.X.cpp.o files in there. But with the new 5.4 build, we're seeing additional .o files (non-unity build).

image

Bad news, not sure what we did to cause this. From the looks of it, we build 5.4 just like we build 5.3. Worst case we just clean up these extra files before creating the artifact?

csciguy8 commented 2 months ago

Also noticed that 5.4 packaging failed...

[11/13] Compile [x64] Module.TestsProject.cpp
[12/13] Link [x64] TestsProject.exe
CesiumGltfReader.lib(GltfReader.obj) : error LNK2005: stbir_resize already defined in Module.ImageCore.cpp.obj
CesiumGltfReader.lib(GltfReader.obj) : error LNK2005: stbir_resize_uint8_srgb already defined in Module.ImageCore.cpp.obj
   Creating library D:\a\cesium-unreal\cesium-unreal\TestsProject\Binaries\Win64\TestsProject.lib and object D:\a\cesium-unreal\cesium-unreal\TestsProject\Binaries\Win64\TestsProject.exp
D:\a\cesium-unreal\cesium-unreal\TestsProject\Binaries\Win64\TestsProject.exe : fatal error LNK1169: one or more multiply defined symbols found

https://github.com/CesiumGS/cesium-unreal/actions/runs/8893486883/job/24432801834

csciguy8 commented 2 months ago

[11/13] Compile [x64] Module.TestsProject.cpp [12/13] Link [x64] TestsProject.exe CesiumGltfReader.lib(GltfReader.obj) : error LNK2005: stbir_resize already defined in Module.ImageCore.cpp.obj CesiumGltfReader.lib(GltfReader.obj) : error LNK2005: stbir_resize_uint8_srgb already defined in Module.ImageCore.cpp.obj Creating library D:\a\cesium-unreal\cesium-unreal\TestsProject\Binaries\Win64\TestsProject.lib and object D:\a\cesium-unreal\cesium-unreal\TestsProject\Binaries\Win64\TestsProject.exp D:\a\cesium-unreal\cesium-unreal\TestsProject\Binaries\Win64\TestsProject.exe : fatal error LNK1169: one or more multiply defined symbols found

Module.ImageCore.cpp.obj is an obj file in the 5.4 engine source... which hints that stb symbols were newly defined here in 5.4, and we need to work around this somehow UE_5.4\Engine\Intermediate\Build\Win64\x64\UnrealGame\Development\ImageCore

.

kring commented 2 months ago

Module.ImageCore.cpp.obj is an obj file in the 5.4 engine source... which hints that stb symbols were newly defined here in 5.4, and we need to work around this somehow

Yep. We'll probably have to modify the cesium-native build to optionally not include those sbir functions. We may also have to add ImageCore or similar to CesiumRuntime's .build.cs so that non-packaged builds don't break.

csciguy8 commented 2 months ago

I think I have a fix for the packaging test failure (stb linker error). It involved a change to cesium-native so I committed that.

In theory, both cesium-native and cesium-unreal should now be able to build (crossing fingers).

kring commented 2 months ago

If your fix works (it looks promising), then perhaps we can ship UE 5.4 support today after all. I see you cancelled the build, though? Was that because you didn't think it would work, or you were just trying not to use build time during the release?

csciguy8 commented 2 months ago

Was that because you didn't think it would work, or you were just trying not to use build time during the release?

It was because I didn't think it would work. I saw an error, then fixed it, then reupdated cesium-native in this PR.

Should be good to go now. Just waiting for CI results for this PR.

We now depend on cesium-native PR #871