CesiumGS / cesium-unreal

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

Fix experimental occlusion culling #1439

Closed kring closed 4 months ago

kring commented 4 months ago

Fixes #1429

This was broken by a change in UE 5.1: the signature of PostRenderViewFamily_RenderThread method of ISceneViewExtension changed. Had that been all, our code would have failed to compile when we upgraded to UE 5.1, and we would have fixed it. But for some reason, Epic kept the old signature around, they just made it do nothing. It's marked with a deprecating macro:

UE_DEPRECATED(5.1, "Use PostRenderViewFamily_RenderThread with an RDG builder instead. RHI commands can be enqueued directly using GraphBuilder.RHICmdList or queued onto the graph.")
virtual void PostRenderViewFamily_RenderThread(FRHICommandListImmediate& RHICmdList, FSceneViewFamily& InViewFamily) {}

But apparently C++ compilers don't flag a warning when you override a deprecated method, which is really unfortunate. So we had no help from the compiler regarding the breaking change, and being an experimental feature we don't use it often enough to have noticed it ourselves.

Once I updated the signature, occlusion culling started working again, except when the tileset was selected in the Outliner. The problem here was that Unreal skips all occlusion checks for primitives selected in the Editor. Because the occlusion check was skipped, and because UCesiumBoundingVolumeComponent doesn't actually render anything (it's only used for occlusion checks), Unreal was deeming the primitive "not visible". This might be a change from previous UE versions? I'm not sure. In any case, our code then assumed this meant the primitive was frustum culled (which is what "not visible" usually means at this stage), and so the associated tile wasn't refined. End result: low detail or entirely missing tiles when the tileset was selected in the Outliner.

This PR fixes this second problem by using the same check that UE itself uses when deciding to skip the occlusion culling step: it checks if the primitive is selected in the Editor. If it is, it's treated as unoccluded.

kring commented 4 months ago

Self merging this in order to include it in today's release. The changes are isolated to a code path that is only activated when an experimental flag is enabled, and that path is completely broken without these changes, so the risk here is minimal.