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

glTF loader fails to render some scenes loaded using loader.parse(...) #14281

Closed
4 of 13 tasks
ikeough opened this issue Jun 13, 2018 · 16 comments
Closed
4 of 13 tasks

glTF loader fails to render some scenes loaded using loader.parse(...) #14281

ikeough opened this issue Jun 13, 2018 · 16 comments

Comments

@ikeough
Copy link

ikeough commented Jun 13, 2018

I have thumbnail server which uses GLTFLoader to load .glb data from Buffer. The server is a node app using headless gl and three-png-stream.

Previously, I'd written a python glTF exporter which generates glbs which load and render successfully on the server. Lately, I've been experimenting with glTF files that are generated using the glTF C# loader(@bghgary). I've attached one file generated with the C# loader, box.glb.zip, to help diagnose. In each of the following cases, files written using my python exporter load correctly both in the glTF viewer and on the server. The following applies only to models generated using the C# loader.

Files generated using the C# loader visualize perfectly using @donmccurdy's glTF viewer, but when loading on the server, nothing renders. In an effort to debug, I set the material for all meshes contained in the glb scene to draw to wireframe, and I see a wireframe in my scene where I would expect it. Perhaps there is an issue with loader.parse(...) reading materials. I've experimented with including both the KHR_materials_pbrSpecularGlossiness extension and pbrMetallicRoughness, and inlcuding only pbrMetallicRoughness, to no avail.

This is for an architectural application with Z up, so I have tried both setting the Matrix and the Rotation of the root node. Again, everything visualizes fine in the glTF viewer with the scene being properly transformed. And, models which have their Matrix set to make Z up, generated by the python exporter, work fine using loader.parse(...) Perhaps there is a winding issue that is caused by setting a Matrix or Rotation which is causing facets not to render correctly.

I've compared the simplest case, a unit box, exported with the python generator and one generated with the C# generator. The only difference that I can see (when diagnosing by saving to .gltf and inspecting the files manually) is that the C# generator does not serialize default values. Perhaps there is an issue using loader.parse(...) with a .glb which omits default values? For example, the C# loader does not serialize mode:4 (TRIANGLES) for primitives, perhaps because this is a default?

This behavior is exhibited when I run the server remotely and locally, so I don't believe it has to do with the environment.

I did see #14256, which may apply as the server is running node, and I'm doing the rendering in the onLoad callback. But again, this works with models generated from my python exporter.

Three.js version
  • Dev
  • r93
  • ...
Browser
  • All of them
  • Chrome
  • Firefox
  • Internet Explorer
OS
  • All of them
  • Windows
  • macOS
  • Linux
  • Android
  • iOS
Hardware Requirements (graphics card, VR Device, ...)
@ikeough ikeough changed the title glTF loader fails to render some scenes loaded using loader.parse(...) glTF loader fails to render some scenes loaded using loader.parse(...) Jun 13, 2018
@takahirox
Copy link
Collaborator

My quick guess, because of no window in loadTextures on node?

https://github.com/mrdoob/three.js/blob/dev/examples/js/loaders/GLTFLoader.js#L1947

Any error logs?

@donmccurdy
Copy link
Collaborator

The example doesn't appear to use any textures, though. 🤔

@ikeough just to confirm the parse function's onLoad callback is being called, and onError is not? Are you able to share runnable node.js rendering code?

@ikeough
Copy link
Author

ikeough commented Jun 13, 2018 via email

@ikeough
Copy link
Author

ikeough commented Jun 13, 2018

@donmccurdy, here's an example project: example.zip.

After unzipping, install packages:

npm install

Then run the "app":

node thumbnail.js box.glb

You will find a blank thumbnail.png file in the directory.

If instead you run:

node thumbnail.js site.glb

You will find a thumbnail.png with data. The site.glb was created using the python exporter mentioned above. While, box.glb is the non-working example provided early created with the C# exporter. Both glb files preview correctly in the glTF viewer.

@ikeough
Copy link
Author

ikeough commented Jun 13, 2018

Also, @takahirox thanks for the suggestion. But alas, no :( There are no textures. Also, I would expect that loader.parse(...) would not take a path that requires Window, if the assumption is that this function is going to be used to load data from a buffer. My understanding is that texture images can be embedded in the .glb and would be extracted as part of parsing the buffer containing that data. This is why I'm using this method on the server. If in fact loader.parse(...) requires a Window, I might not ever be able to do textures.

@mrdoob mrdoob added this to the r94 milestone Jun 14, 2018
@takahirox
Copy link
Collaborator

Confirmed and reproduced the same issue here. Yeah, realized no textures in box.glb so what I suggested isn't the problem in this case.

I converted geometry.index.array type from Uint16Array to Uint32Array by inserting the following code then somehow a box will be rendered. But not sure why....

return new Promise(function(resolve, reject) {
    loader.parse(buffer, "./", 
        function(gltf) {

            // Converting index.array type from Uint32Array to Uint16Array here.
            // This code is box.glb specific
            let mesh = gltf.scene.children[0].children[0];
            let indices = new Uint16Array(mesh.geometry.index.array.length);
            for (let i = 0, il = mesh.geometry.index.array.length; i < il; i ++) {
                indices[i] = mesh.geometry.index.array[i];
            }
            mesh.geometry.index.array = indices;

            // Add the gltf to the scene
            scene.add(gltf.scene)

            ....

thumbnail

@takahirox
Copy link
Collaborator

geometry.indexes type in site.glb is Uint16Array. So Uint32Array geometry.index seems the root issue but why...?

@ikeough
Copy link
Author

ikeough commented Jun 15, 2018

It occurs to me that I might be writing the index buffers as int32, not uint32. They have the same byte size, but in C#, the last bit is used in int32 to indicate the sign. Perhaps this is screwing something up. The act of converting the int32 bytes to an uint16 could create weirdness. Not sure how rewriting the index array could “fix” this. I’ll test this hypothesis tonight.

@ikeough
Copy link
Author

ikeough commented Jun 15, 2018

@takahirox You rock! That tip lead me in the right direction. There were two things different about the C# code. Index arrays contained int32 (4 bytes), and Accessors used a component type of UNSIGNED_INT. When I switched index arrays to use ushort (2 bytes) and Acessors to use a component type of UNSIGNED_SHORT, it works!

My original setup was just plain wrong. Your fix works because it widens the type to accommodate the original size of the data.

However, load(...) does seem much more permissive than parse(...). I don't think that should be the case. One would expect them to succeed or fail similarly. It seems that the only way to properly vet incoming models would be to compare the Accessor component type with the specified index count and the size of the supplied byte array and determine if you've got the right stuff. In my original setup you would have seen an Accessor type that aligned with the number of indices but an incorrect amount of associated data.

I'm happy to close this if the discrepancy between these two methods is by design.

@donmccurdy
Copy link
Collaborator

However, load(...) does seem much more permissive than parse(...).

It is the same code, excluding the entrypoint, so I'm not sure why we're getting different results here... does using parse(...) in a browser rather than node.js give the same result? it could be some discrepancy in the node.js support for typed arrays, arraybuffers, or (lack of) TextDecoder API....

@vyv03354
Copy link
Contributor

UNSIGNED_INT element array indices require the OES_element_index_uint extension. Since WebGLRenderer enables the extension automatically, @donmccurdy's glTF viewer works. Maybe headless-gl does not. Did you enable the extension in your node app on your own?

If you do not want to depend on the OES_element_index_uint extension, it is a good practice to avoid UNSIGNED_INT element array indices as much as possible.

@takahirox
Copy link
Collaborator

Maybe that's the reason!

Logs.

$ node thumbnail.js box.glb
THREE.WebGLRenderer 93
THREE.WebGLRenderer: WEBGL_depth_texture extension not supported.
THREE.WebGLRenderer: OES_texture_float extension not supported.
THREE.WebGLRenderer: OES_texture_float_linear extension not supported.
THREE.WebGLRenderer: OES_texture_half_float extension not supported.
THREE.WebGLRenderer: OES_texture_half_float_linear extension not supported.
THREE.WebGLRenderer: OES_standard_derivatives extension not supported.
THREE.WebGLRenderer: OES_element_index_uint extension not supported.
THREE.WebGLRenderer: EXT_texture_filter_anisotropic extension not supported.

I'm thinking if adding note about it to glTF spec would be helpful...

@vyv03354
Copy link
Contributor

The spec already has a note about that, although it is a bit vague:
https://github.com/KhronosGroup/glTF/blob/master/specification/2.0/README.md#primitiveindices

componentType must be 5121 (UNSIGNED_BYTE), 5123 (UNSIGNED_SHORT) or 5125 (UNSIGNED_INT), the latter may require enabling additional hardware support

@takahirox
Copy link
Collaborator

Oh, right. Thanks

@vyv03354
Copy link
Contributor

Apparently the latest headless-gl trunk added support for OES_element_index_uint: stackgl/headless-gl#121

But headless-gl 4.0.4, the latest version from npm, does not include this fix yet.

@ikeough
Copy link
Author

ikeough commented Jun 18, 2018

Thanks @vyv03354 and @takahirox. I should have reviewed the warnings on headless gl startup more carefully. As this is not a three issue, I’m closing.

@ikeough ikeough closed this as completed Jun 18, 2018
@mrdoob mrdoob removed this from the r94 milestone Jun 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants