Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
deng0 opened this issue Feb 9, 2024 · 3 comments

Comments

@deng0
Copy link

deng0 commented Feb 9, 2024

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:
deng0/gltf-pipeline@58b138d

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

@lilleyse
Copy link
Contributor

lilleyse commented Feb 9, 2024

@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?

@deng0
Copy link
Author

deng0 commented Feb 9, 2024

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

@javagl
Copy link
Contributor

javagl commented Feb 26, 2024

This should be fixed as of #11825 (and I checked the linked sandcastle with the (local) updated state). If there's anything left, it can be reopened.

@javagl javagl closed this as completed Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants