mrdoob / three.js

JavaScript 3D Library.
https://threejs.org/
MIT License
102.69k stars 35.37k forks source link

ColladaLoader fails when mesh exported with tangents/normals #1130

Closed canassa closed 12 years ago

canassa commented 12 years ago

I found this problem on the latest dev build while I was trying to export a mesh from 3D Max using the OpenCollada plugin.

I noticed that when the mesh is exported using the 'include tangents/normals' option the ColladaLoader fails to load the mesh.

The importer fails on the Geometry.js computeCentroids method.

computeCentroids: function () {

    var f, fl, face;

    for ( f = 0, fl = this.faces.length; f < fl; f ++ ) {

        face = this.faces[ f ];
        face.centroid.set( 0, 0, 0 );

        if ( face instanceof THREE.Face3 ) {

            face.centroid.addSelf( this.vertices[ face.a ].position );
            face.centroid.addSelf( this.vertices[ face.b ].position );
            face.centroid.addSelf( this.vertices[ face.c ].position );  // Fails here
            face.centroid.divideScalar( 3 );

I ran these tests on the Chrome console, as you can see the face.c is pointing to a index outside the this.vertices array:

>> this.vertices.length
8
>> face.c
10

If the mesh is exported without the tangents/normals option it loads fine.

The mesh is a simple cube, here is the collada with that failed the import:

http://pastebin.com/q97CYBaL

And for reference, this is same mesh without the extra options (loads ok)

http://pastebin.com/06AJTn7k

AddictArts commented 12 years ago

Thanks mrdoob for pointing out this issue. I ran into this doing the same thing with an export from 3ds max and the opencollada exporter. The ColladaLoader does not properly handle inputs that use a shared offset. In this case the BITANGENT and BINORMAL share the same index into their source data streams.

mrdoob commented 12 years ago

Thank you @AddictArts! @canassa bug has been fixed in the dev branch.

canassa commented 12 years ago

Thank you guys! Amazing job!

poshaughnessy commented 12 years ago

Sorry if I say anything stupid here - I'm quite new to this...

I've updated to the dev branch and generated a new ThreeDebug.js. However, I'm still getting what sounds like the same error as discussed here - i.e. face.c points to an index outside of the vertices array. So at line:

face.centroid.addSelf( this.vertices[ face.c ].position )

I get: Cannot read property 'position' of undefined

In my example, face.c is 58949 which is one more than the highest index available in vertices (vertices length is 58949).

I exported the Collada model using Blender.

Please might you have any advice for what I should try next?

Thank you.

AddictArts commented 12 years ago

Can you share your COLLADA file, or a sample one? Do you know which type of mesh primitives it is using, triangles, polylist, polygons? Feel free to send me a sample jpywtora at gmail dot com.

poshaughnessy commented 12 years ago

Thank you AddictArts. As emailed... I imported the Collada file that wasn't working back into Blender, and re-exported it. The new one works fine!

Now just to work out why / what happened... Doing a diff between the files, it's hard to identify key differences, because the order of the elements has changed around, so the diff output is massive. Unfortunately I don't think I'm going to be allowed to share the model file. I'll try to dig deeper into it though and maybe if I narrow it down a bit I can share part of the file.

A few more details in the meantime: it's using three polylists (the counts are: 53888, 113564, 1941). The file is pretty big, around 21MB. And I'm using Blender 2.61.0 r42614.

In the meantime, thanks - it's working now!