CesiumGS / cesium-unity

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

Using copies of the default materials results in error log spam #344

Closed j9liu closed 9 months ago

j9liu commented 1 year ago

Multiple users have tried creating their own copies of our shaders for their projects, only to get this error spammed in the console.

image

"Destroying assets is not permitted to avoid data loss."

Using Resources.Load gives us an object that is linked to the asset itself, so we don't want to destroy a material returned by that function. But we always Instantiate that material, so one would think the material is safe to destroy.

It's also hard to reliably reproduce this bug. I remember getting it to show up at one point on Unity 2021.3.14f, but I don't have concrete steps. I also haven't been able to reproduce it on Unity 2021.3.26f.

A user commented that they were able to get rid of the bug by "deleting the reference to “green1x1.png” in the “metalicRoughnessTexture”". So maybe we're incorrectly trying to destroy the texture reference?

Known forum posts: X, X

j9liu commented 1 year ago

I've noticed this again (while trying to create a clipping shader described in #248), so I'm guessing that whenever we do Material.Instantiate, it's not copying any textures that are created for the material. The materials seem to refer to the texture asset itself, hence the log. Why the message doesn't appear within our own plugin, I'm not sure. But if a user supplied their own texture asset, then this would probably attempt to delete their texture too.

Maybe we can keep track of the variables that Cesium explicitly generates textures for, then only destroy those. It may also help to do texture pooling while we're at it. So instead of outright destroying those textures, we can return them to the pool, then resize / rewrite them for new textures.

j9liu commented 1 year ago

Reported on the forum again, X and X

vrenken commented 1 year ago

Hi,

first off kudo's for the fantastic work - I am sure that many people are loving the Cesium maps, and the ability to bring them into their Unity projects.

Having said that I would like to mention that this issue is indeed also bugging a few of our development teams, and it reduces the fun of troubleshooting issues by means of the log console. If a fix (or a workaround) is available we'll happily embrace it.

Thanks in advance, greetings from Germany & keep up the great work!

Peter Vrenken

j9liu commented 1 year ago

Hi @vrenken,

Another user who was seeing the console spam said they fixed it by deleting the reference to “green1x1.png” in the “metalicRoughnessTexture” of the new material. Could you confirm if that works for you?

R-Stokke commented 1 year ago

I ran into the same error messages when duplicating the default material & shader (CesiumDefaultTilesetMaterial and ..Shader), and selecting 'None' as the 'metallicRoughnessTexture' stopped the error messages. Thanks for the workaround @j9liu ! I am running Unity 2022.1.24f1 with Cesium v1.6.3.

vrenken commented 1 year ago

Hi @j9liu,

Thanks for the information @j9liu and @R-Stokke.

We tested it in one of the projects. The trick that worked for us was:

We did not conduct any further investigation, but expect that something broke when we first changed the shader.

Kind regards,

Peter Vrenken

TobiasWehrum commented 11 months ago

I seem to have the same problem with Cesium 1.6.4 and Unity 2022.3.14f1.

What I did:

I don't see any _metalicRoughnessTexture in the material and I'm already using the newest Cesium version (and copied its shader), so I can't use any workarounds I see earlier in the thread.

edit: Ah, I had to switch the Inspector to "Debug" to find _metalicRoughnessTexture in Saved Properties -> Tex Envs on the material! And indeed, removing the reference to "green1x1.png" seems to have fixed the problem. Nice.