Fix model shader generation when normals are missing

[lilleyse]

processPbrMetallicRoughness always expects normals to exist, but that is not always the case. We should have a separate shader path for those models that would look similar to the constant lighting in processModelMaterialsCommon.

This will fix #5835.

[emackey]

See implementation note: AnalyticalGraphicsInc/gltf-vscode#28 (comment)

When normals are not specified, client implementations should calculate flat normals.

[lilleyse]

Hm.. that might not look good for photogrammetry.

[lilleyse]

Came up on the forum: https://groups.google.com/forum/#!topic/cesium-dev/eXxk3Oi0hEM

[KermMartian]

We ran into this as well. With GLTF 1.0, Cesium seemed happy to compute normals from our models, which isn't happening with GLTF 2.0. I tried the --materialCommon hint from the linked forum topic, but that appears to be the worst of both worlds (missing textures in Cesium; incompatibility in other GLB viewers). Is there a current suggested fix for this, other than expanding the size of our models with explicit normals?

[KermMartian]

@lilleyse Apologies for the highlight and double comment on this; I wanted to check if there was anything else I could do to help to investigate or work around this issue, especially since GLTF 1.0 models appear to work (ie, generate normals) as expected. Thanks in advance for your help.

[lilleyse]

@KermMartian I'm not sure if we ever generated normals for 1.0 models on load. I tried a simple box model and it shows the image below. If you are able to send me one of your models I could confirm.

But I do think we should generate normals for 2.0 models eventually, including options for smooth normals vs. hard normals like gltf-pipeline has. Cesium even has a built-in methods for doing this but no one has gotten around to integrating it yet. Until then I don't have a suggested fix.

[KermMartian]

@lilleyse Do you know if perhaps the 1.0 branch of KhronosGroup/COLLADA2GLTF was generating the normals unbidden, or perhaps creating a shader to generate the normals on the fly (and 2.0 doesn't)? I'll see if I can answer that question for myself if I can reproduce the GLTF 1 behavior, and I'm happy to send you one of those models, if I reproduce it. Making our DAE-to-CMPT pipeline generate normals along the way isn't too bad, but I'd like to avoid it there's an easy fix in COLLADA2GLTF, since it sounds like an easy fix in Cesium is out for the time being.

[lilleyse]

I'm not sure. @tfili do you know if COLLADA2GLTF master generates normals by default?

[tfili]

I don't think it does. That's why we use gltfPipeline to do it.

[KermMartian]

Duly noted; thanks for the help, everyone! In the end, we just generate normals ourselves before they get to COLLAGA2GLTF (and thence Cesium), so everything is resolved for us as below. I'm afraid that doesn't help those using models without normals, so definitely a strong vote for me in favor of Cesium eventually being able to compute those normals on the fly.

[lilleyse]

Came up on the forum again: https://groups.google.com/forum/#!topic/cesium-dev/se_9ov5HwA8

[lilleyse]

And again: https://groups.google.com/forum/#!topic/cesium-dev/YSNaoirGXMw

[tkazik]

BabylonJS has an option to createNormals based on the mesh:

https://doc.babylonjs.com/api/classes/babylon.abstractmesh#createnormals

Might be worth looking at it and get inspiration from here:

https://github.com/BabylonJS/Babylon.js/blob/c54863cf32d8045db2fc0b56949974563db75fd7/src/Mesh/babylon.abstractMesh.ts#L1801

[emackey]

Lately I've been pondering whether Cesium could just show "unlit" materials (baseColor * occlusion + emissive) in the absence of normals. It's technically not what the spec advises, but it's quick to implement and would get us visibility of models that are currently pitch black.

[lilleyse]

@emackey I think that's a great idea

[lilleyse]

Workaround added in #6501, closing this in favor of #6507 and #6506

[cesium-concierge]

Congratulations on closing the issue! I found these Cesium forum links in the comments above:

https://groups.google.com/forum/#!topic/cesium-dev/eXxk3Oi0hEM
https://groups.google.com/forum/#!topic/cesium-dev/se_9ov5HwA8
https://groups.google.com/forum/#!topic/cesium-dev/YSNaoirGXMw

If this issue affects any of these threads, please post a comment like the following:

The issue at #5838 has just been closed and may resolve your issue. Look for the change in the next stable release of Cesium or get it now in the master branch on GitHub https://github.com/AnalyticalGraphicsInc/cesium.

---

I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome.

🌍 🌎 🌏