glTF support is missing features #528
Comments
|
Rotation issue: https://sketchfab.com/models/a4ca0e3864e143af9bf1dffc7fc8029b (you have to zoom into the black sphere to seen the scene). This model also seems to use vertex colors? Our render is definitely off. Non-default samplers: https://sketchfab.com/models/94b24a60dc1b48248de50bf087c0f042 The tramway and some of the walls are not textured properly, most likely because we don't use the right sampler None of the textures are loaded for this model: https://sketchfab.com/models/282f497334f64a589edee4e63ad7e428 |
|
Another bug: the water in https://sketchfab.com/models/e051a32d21d6453c8fad408082492267 is not transparent. The alpha mask is also not working on the bushes in that scene. |
|
I'm working on the first bullet in the original problem description (Tangents/bitangents are not computed correctly), which is exemplified by |
We started to write our own implementation of mikktspace, but it required an unindexed mesh for input. This was awkward because assimp is already applying a pipeline of operations that includes tangent generation and indexing. It felt reduandant to add our own pipeline on top of that (unindexing => tangentgen => reindexing). Since assimp's CalcTangentsProcess method is already producing tangents that follow the orientation implied by texture coordinates, this simply needs to be tweaked to match glTF. (flip the bitangent) Also fixed a bug for the case where models have neither UV's nor tangents, where we were calling norm instead of normalize. :)
We started to write our own implementation of mikktspace, but it required an unindexed mesh for input. This was awkward because assimp is already applying a pipeline of operations that includes tangent generation and indexing. It felt reduandant to add our own pipeline on top of that (unindexing => tangentgen => reindexing). Since assimp's CalcTangentsProcess method is already producing tangents that follow the orientation implied by texture coordinates, this simply needs to be tweaked to match glTF. (flip the bitangent) Also fixed a bug for the case where models have neither UV's nor tangents, where we were calling norm instead of normalize. :)
We started to write our own implementation of mikktspace, but it required an unindexed mesh for input. This was awkward because assimp is already applying a pipeline of operations that includes tangent generation and indexing. It felt reduandant to add our own pipeline on top of that (unindexing => tangentgen => reindexing). Since assimp's CalcTangentsProcess method is already producing tangents that follow the orientation implied by texture coordinates, this simply needs to be tweaked to match glTF. (flip the bitangent) Also fixed a bug for the case where models have neither UV's nor tangents, where we were calling norm instead of normalize. :)
The first model seems to just have some rotation issues with the asset itself, and some of the colors aren't showing up because vertex colors aren't implemented yet.
I realized that I didn't implement alpha masking and blending for unlit materials, which this scene uses. I'm working on fixing the issues now. |
|
Crash opening this file: https://sketchfab.com/models/f8d3c7506f3545108dcf7f6c170e19e1 |
|
It seems like that file and the files mentioned in #503 have some kind of error within the gltf file, and this is causing the viewer to crash within Assimp when it is loading the gltf. I saw that Don McCurdy's gltf viewer is still able to show the model even though it says there are errors within the file, so we could find a way to load the files by ignoring the malformed lines. |
If the errors seem unreasonable, I'm glad to send feedback to Sketchfab. Some viewers (like Facebook 3D Posts) enforce a "no validation errors" policy, and if these are issues you'd rather not work around, requiring stricter conformance from exporters is acceptable. The tolerance in my viewer is more accidental than planned. :) |
|
Well crashing is never a good option :) But if the crash is inside Assimp our best bet would be to fix it there and upstream the change. We could also investigate TinyGLTF |
|
Multi UV sets support is now in. @magicwhale were you able to figure out where the other issues come from? |
We started to write our own implementation of mikktspace, but it required an unindexed mesh for input. This was awkward because assimp is already applying a pipeline of operations that includes tangent generation and indexing. It felt reduandant to add our own pipeline on top of that (unindexing => tangentgen => reindexing). Since assimp's CalcTangentsProcess method is already producing tangents that follow the orientation implied by texture coordinates, this simply needs to be tweaked to match glTF. (flip the bitangent) Also fixed a bug for the case where models have neither UV's nor tangents, where we were calling norm instead of normalize. :)
|
@romainguy we recently proposed testing rendering parity between
Related sample model: Normal Tangent Mirror Test
Related sample model: Vertex Color Test
Related sample model: Texture Transform Test When I saw the results of this test it rang a bell. I'm not sure if you meant texture rotations, but perhaps this is relevant.
|
|
Thank you. Texture transforms are just not supported at all at the moment. We're moving to a different gltf loader to fix all these issues. |
|
I'm surprised by the tangents though, @prideout wasn't this fixed? |
|
Just now reproduced the issue with Normal Tangent Mirror, I'm surprised too. Will investigate. |
|
tl;dr: For now I think I should simply modify our copy of MeshAssimp to negate the bitangent, thus moving our current workaround downward so that it doesn't affect the tangents-are-available case. This will be fixed in a more proper way in the new glTF loader that I'm working on. details: We render When MeshAssimp generates tangents and bitangents, it basically makes tangents follow texcoord U, and bitangents follow texcoord V. The resulting tangent vectors are fairly consistent with a couple popular viewers that I tried, but the bitangent vectors are not. Other viewers use different approaches: |
Note that this change is very recent, and won't be visible in my viewer until threejs r102 is released. But I think BabylonJS (and this viewer) do the same. |
Assimp's CalcTangentSpace deviates from de facto glTF 2.0 so we were compensating for this with an unconditional fixup in MeshAssimp. However the fixup should apply only when CalcTangentSpace is active, i.e. when the model is missing tangents. This makes it so that NormalTangentMirrorTest (has tangents) and NormalTangentTest (needs tangents) both look reasonable. I also noticed that MeshAssimp was inexplicably applying aiProcess_CalcTangentSpace twice: once as a flag, and once as a post-process. I removed the latter. This will be fixed in the upcoming cgltf-based loader, which will use our officially-sanctioned utility method in VertexBuffer. See #528
Assimp's CalcTangentSpace deviates from de facto glTF 2.0 so we were compensating for this with an unconditional fixup in MeshAssimp. However the fixup should apply only when CalcTangentSpace is active, i.e. when the model is missing tangents. This makes it so that NormalTangentMirrorTest (has tangents) and NormalTangentTest (needs tangents) both look reasonable. I also noticed that MeshAssimp was inexplicably applying aiProcess_CalcTangentSpace twice: once as a flag, and once as a post-process. I removed the latter. This will be fixed in the upcoming cgltf-based loader, which will use our officially-sanctioned utility method in VertexBuffer. See #528
This moves our existing VertexBuffer utility into a new "geometry" library and adds new functionality for computing tangents based on UV's. The UV-based method comes from Eric Lengyel. We considered mikktspace (which thankfully has a zlib-style license) but it would require re-indexing via meshoptimizer and is therefore a bit heavyweight. The new library has no dependencies and will add only 7 KB to Filament's Android aar file. Fixes #858 and preps for #528.
This moves our existing VertexBuffer utility into a new "geometry" library and adds new functionality for computing tangents based on UV's. The UV-based method comes from Eric Lengyel. We considered mikktspace (which thankfully has a zlib-style license) but it would require re-indexing via meshoptimizer and is therefore a bit heavyweight. The new library has no dependencies and will add only 7 KB to Filament's Android aar file. Fixes #858 and preps for #528.
This moves our existing VertexBuffer utility into a new "geometry" library and adds new functionality for computing tangents based on UV's. The UV-based method comes from Eric Lengyel. We considered mikktspace (which thankfully has a zlib-style license) but it would require re-indexing via meshoptimizer and is therefore a bit heavyweight. The new library has no dependencies and will add only 7 KB to Filament's Android aar file. Fixes #858 and preps for #528.
This moves our existing VertexBuffer utility into a new "geometry" library and adds new functionality for computing tangents based on UV's. The UV-based method comes from Eric Lengyel. We considered mikktspace (which thankfully has a zlib-style license) but it would require re-indexing via meshoptimizer and is therefore a bit heavyweight. The new library has no dependencies and will add only 7 KB to Filament's Android aar file. Fixes #858 and preps for #528.
This moves our existing VertexBuffer utility into a new "geometry" library and adds new functionality for computing tangents based on UV's. The UV-based method comes from Eric Lengyel. We considered mikktspace (which thankfully has a zlib-style license) but it would require re-indexing via meshoptimizer and is therefore a bit heavyweight. The new library has no dependencies and will add only 7 KB to Filament's Android aar file. Fixes #858 and preps for #528.
|
Closing this for now. There are still a couple of unsupported features like blend shapes. |
Our glTF viewer doesn't work entirely just yet. Here's what I found so far:
I found these issues while trying to load various Sketchfab assets.
The text was updated successfully, but these errors were encountered: