search

CesiumGS / cesium-unity

Bringing the 3D geospatial ecosystem to Unity
https://cesium.com/platform/cesium-for-unity/
Apache License 2.0
347 stars 83 forks source link

Statically link MSVC runtime library #464

Closed jqntn closed 3 months ago

jqntn commented 3 months ago

Hi,

Fixes #419

The built DLLs don't get much larger, maybe around 1-3MB depending on configuration.

⚠️IMPORTANT: This PR depends on https://github.com/CesiumGS/cesium-native/pull/900 being merged first.

kring commented 3 months ago

Thanks for the PR @jqntn! I've pushed your changes to a branch of the same name in the main repo so that CI runs on it. Your changes look good. I'm just going to let CI build it, give the built package a quick test, and then I'll merge this.

csciguy8 commented 3 months ago

With the latest cesium-native, the official way to enable static linking is this...

set(CESIUM_MSVC_STATIC_RUNTIME_ENABLED ON)

... although it might work to just set CMAKE_MSVC_RUNTIME_LIBRARY directly too. That mentioned option was added to just make things a little easier.

jqntn commented 3 months ago

@csciguy8 You're right. @kring It should work fine now.

kring commented 3 months ago

This isn't currently working: https://github.com/CesiumGS/cesium-unity/actions/runs/9669815502/job/26677180619

I think the problem is that doing this alone is not sufficient:

set(CESIUM_MSVC_STATIC_RUNTIME_ENABLED ON)

That will make the cesium-native code use the static runtime library, but it won't affect the native code in cesium-unity itself.

csciguy8 commented 3 months ago

This isn't currently working:

Indeed. Anything using MD_XXX (Multithreaded DLL) doesn't seem like it's getting configured like we expect.

Cesium3DTilesSelection.lib(ViewState.obj) : error LNK2038: mismatch detected for 'RuntimeLibrary': value 'MT_StaticRelease' doesn't match value 'MD_DynamicRelease' in Cesium.obj [D:\cesium\CesiumForUnityBuildProject\Packages\com.cesium.unity\native~\build\Runtime\CesiumForUnityNative-Runtime.vcxproj]
Cesium3DTilesSelection.lib(BoundingVolume.obj) : error LNK2038: mismatch detected for 'RuntimeLibrary': value 'MT_StaticRelease' doesn't match value 'MD_DynamicRelease' in Cesium.obj [D:\cesium\CesiumForUnityBuildProject\Packages\com.cesium.unity\native~\build\Runtime\CesiumForUnityNative-Runtime.vcxproj]
Cesium3DTilesSelection.lib(TileContent.obj) : error LNK2038: mismatch detected for 'RuntimeLibrary': value 'MT_StaticRelease' doesn't match value 'MD_DynamicRelease' in Cesium.obj [D:\cesium\CesiumForUnityBuildProject\Packages\com.cesium.unity\native~\build\Runtime\CesiumForUnityNative-Runtime.vcxproj]
kring commented 3 months ago

Ok, I pushed a change to set CMAKE_MSVC_RUNTIME_LIBRARY as well.

csciguy8 commented 3 months ago

To be help out this PR a bit, I tested the CI artifact produced from the latest build on this branch.

In the Unity Editor, everything seemed to work fine. I added it via the "Package Manager" window using "Add package from disk".

Unfortunately, I saw errors when running "File -> Build and Run" though. But to be fair, I saw the exact same errors in main as well. I don't run this very often so not sure if I'm doing wrong or it's a new problem that was introduced for this release.

Shader error in 'Cesium/CesiumDefaultTilesetShader': maximum ps_5_0 sampler register index (16) exceeded at line 428 (on d3d11) Shader error in 'Cesium/CesiumDefaultTilesetShader': maximum ps_4_0 sampler register index (16) exceeded at line 6450 (on d3d11)

kring commented 3 months ago

Build and Run is working fine in this branch on my system @csciguy8. Were you building a clean checkout of Cesium for Unity Samples? I'm going to merge this because I think it's extremely important for this week's release, but we should dig into what's going wrong on your system, because others might have similar problems.

kring commented 3 months ago

Thanks again @jqntn!