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 missing textures caused by sharing a single texture between multiple primitives #1436

Closed kring closed 4 months ago

kring commented 4 months ago

This builds on #1431 so merge that first.

This PR fixes a bug reported on the forum: https://community.cesium.com/t/textures-appears-white-when-used-multiple-times-on-same-gltf/32278

The model and instructions there make it easy to reproduce.

In this model, two primitives both reference the same material, and that material has a baseColorTexture. So when preparing the glTF for use with Unreal, and processing both of the primitives, we indirectly end up processing the same texture twice.

The first time we process the texture, the associated image's ImageCesium has pixel data. So in a worker thread, we create a new FCesiumTextureResourceBase (RHI texture resource) from it, and then clear the pixel data. So far so good.

The next time we process the texture, we see that an FCesiumTextureResourceBase already exists for the associated Image, so we don't need to upload the pixel data again (which is good because we can't; the pixel data has been cleared). Instead, we create a FCesiumUseExistingTextureResource referencing the existing RHI texture resources. Still all good so far.

Next, in the game thread, we create a UTexture2D for each of the RHI textures previously created. They both refer to the same underlying data, which is no problem.

Now here comes the problem. The way we manage the lifetime of UTexture2D instances is that we store them on the glTF Texture object in a private extension. But in this scenario, we have two UTexture2D instances associated with a single glTF Texture. The second one ends up replacing the first, and as a result the first is destroyed. We go through all the motions of creating the textures we need, but then we accidentally destroy one of them before we can use it.

One way to fix this would be to make a single glTF able to reference multiple UTexture2D instances. But wait, why do we need two UTexture2D instances anyway? They were both created from the same glTF Texture, so they should be identical. They already share the same pixel data, but it would be better if they shared the UTexture2D instances, too.

So that's what this PR does. It adds the private extension to the glTF Texture earlier in the process, and uses it to store both the FCesiumTextureResourceBase texture (during worker thread texture prep) and the UTexture2D (during game thread texture prep). If the extension already exists, we don't need to process the texture again, we just need to use the existing reference counted texture resources.

csciguy8 commented 4 months ago

This PR fixes a bug reported on the forum: https://community.cesium.com/t/textures-appears-white-when-used-multiple-times-on-same-gltf/32278

Hey @kring , this link takes me to a page that says "Oops! That page doesn’t exist or is private.". Could you make it public? Or copy / paste the relevant instructions into this PR description?

kring commented 4 months ago

this link takes me to a page that says "Oops! That page doesn’t exist or is private."

Oops, I didn't realize that was a direct message. You should have access to it now.

kring commented 4 months ago

Merging this because it's approved and needed for today's release.