cesium icon indicating copy to clipboard operation
cesium copied to clipboard

b3dm tileset created with gltf1.0 KHR_materials_common CONSTANT technique does not show textures

Open deng0 opened this issue 1 year ago • 2 comments

What happened?

We have a bunch of old tilesets that show up white, when viewed in newer version of cesium: image

In old versions they were displayed correctly.

I've looked into the cesium code and found the problem:

As these b3dm/gltf1 models are not supposed to be realtime lighted, they have no normals baked in. Instead they use the CONSTANT technique of the KHR_materials_common extension: https://github.com/KhronosGroup/glTF/blob/main/extensions/1.0/Khronos/KHR_materials_common/README.md

When the new cesium version converts these materials to gltf2 it uses the KHR_materials_unlit extension: https://github.com/KhronosGroup/glTF/blob/main/extensions/2.0/Khronos/KHR_materials_unlit/README.md

This makes sense, but the problem is, that the KHR_materials_unlit extension only uses the baseColor of the material, which is only set from the old diffuse value. On the other hand the CONSTANT technique only uses emission and ambient values (at least that's how I understand the KHR_materials_common specification).

In any case the models we created, do not specify diffuse but instead use emission for the texture.

I would propose the following fix: if CONSTANT technique was used and diffuse is not defined, use emission (or ambient) value for diffuse, which will then be used for baseColor.

I've forked gltf-pipeline and committed a suggested fix: https://github.com/deng0/gltf-pipeline/commit/58b138d2080a3b1c9c2f99b6b03664038c06fe70

It would be great, if this or a similar fix could be added to cesium, so that our old tilesets still work with new version of cesium.

Reproduction steps

The provided sandcastle example shows the problem with a small test tileset.

This test tileset can also be downloaded here: https://api.cesium.com/v1/assets/2455188/archives/59489/download or here: tileset_gltf1_constant_technique.zip

Sandcastle example

https://sandcastle.cesium.com/#c=jZJvb9owEMa/yikvqiBtziijajVarQWGEhUYg9GmijS5jikmjp3aBppM/e5z/rC10zbtlWXf7567e86eByOFhYE+1WybBnPAhFCtwUjI5VYBkwKw1tToSNQM8qVAMV3hLTeXFbyQCRVwDpFD82B9PyJsygL/a+G3J8zXvvjSJX3/xE+y22U/OEMWeoxHSQklYRHw8GbYDfP2Jtw87K8Xs/3daGbCdJZP8nYapsPuuCCdSZHkk4Kw636Q3Vmx8SJ5Nx2EnfIe3s7YdDPsjAekfDseF8szlC5Wnzb+PBnmfVbsroLP8XLQnm10fvz27PFqejMK0m8bfxDvJ6eR8yESRAptYMfonio7iqD7xhK0rN7cyCHVvS+FwUxQFTktmxcJo3L4HgmAWsIwTq1dVgPvMTsYi+qjM1jUYbRSMrVGXpbW+rF7/L7bbZ+elorQdIE0oYKiTLGUGbajGuE4dhv5GqwrNHghZbqQL4ES8Ty4zDKeg1lTaLYG2uScAluBzaZPTJfLPfRPn4zC2rbfCKFq+6h+rqraPLc84TCblbWGxG7NtODo6B9hZH/UfyCHHzYvW22VcKt2GX42Vk/xalcvXa4y/6pYTfIciWcg2JA1uFQpqVq/Nik5RVw+NO8Wt7DzxulVVS9K6iNLM6kMbBV3EfIMTTOODdXe/ZYktj+idZnX8w4pvZjtgMXnf/hKQLj12UZWW87nrKCRc9HzLP8qjUscM/Ew3VHFcW6Rso3eun1xXQcQQj3PXsuiv+caKfk9Vi90fwA

Environment

No response

deng0 avatar Feb 09 '24 14:02 deng0

@deng0 the fix makes sense. Could you open a PR in gltf-pipeline first? Then we can bring the changes into CesiumJS. @javagl would you be able to help review once the gltf-pipeline PR is open?

lilleyse avatar Feb 09 '24 15:02 lilleyse

Ok, I've recreated the fix in gltf-pipeline and opened a pull request.

deng0 avatar Feb 09 '24 15:02 deng0

This should be fixed as of https://github.com/CesiumGS/cesium/pull/11825 (and I checked the linked sandcastle with the (local) updated state). If there's anything left, it can be reopened.

javagl avatar Feb 26 '24 22:02 javagl