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

FBXLoader inverts geometry if Lcl Scaling is negative #11911

Closed
looeee opened this issue Aug 9, 2017 · 26 comments
Closed

FBXLoader inverts geometry if Lcl Scaling is negative #11911

looeee opened this issue Aug 9, 2017 · 26 comments

Comments

@looeee
Copy link
Collaborator

looeee commented Aug 9, 2017

I'm attempting to load a complex rigged model, and finding that some parts of the geometry appear to have flipped normals, as you can see here:

post_fix

However, the normals on the flipped parts of the geometry are actually correct, and neither inverting them nor applying double sided material fixes the issue. Aside from this, I checked the model in Windows Paint3D and it loads correctly.

So, I isolated the two feet, and then took a single polygon from each, scaled them and rotated them to match and positioned them both at (0,0,0). At this point the only difference I can see in my modelling program are the pivots being oriented differently. But when I load them with the FBXLoader, I get this:

Polygon from left foot, displays correctly and is affected by lights

correct

Polygon from right foot, has the same material, position, scaling, orientation etc but displays dark grey and is not affected by lights

incorrect

The two FBX files, one polygon from each foot, are here
foot pieces.zip

And running them through a diff checker, the only important difference I can see is the line:

Left foot (correct)

P: "Lcl Scaling", "Lcl Scaling", "", "A",1.00000095367432,0.999999701976776,1.00000035762787

Right foot (incorrect)

P: "Lcl Scaling", "Lcl Scaling", "", "A",-1.00000107288361,-0.999999761581421,-1.00000035762787

Sure enough, removing the minus signs from this line causes the model to load correctly.

@looeee
Copy link
Collaborator Author

looeee commented Aug 9, 2017

Applying the obvious fix to the FBXLoader:

if ( 'Lcl_Scaling' in node.properties ) {

      var scaleFactor = parseFloatArray( node.properties.Lcl_Scaling.value );

      scaleFactor[0] = Math.abs( scaleFactor[0] );
      scaleFactor[1] = Math.abs( scaleFactor[1] );
      scaleFactor[2] = Math.abs( scaleFactor[2] );

      model.scale.fromArray( scaleFactor );

    }

...fixes the inverted geometries. However now the foot the objects that were previously flipped inside out are flipped in other ways:

flipped

@WestLangley
Copy link
Collaborator

three.js does not support reflections in the object matrix; reflections leave the vertex normals and winding order in an inconsistent state. The normals will look correct, but the winding order is reversed.

@mrdoob
Copy link
Owner

mrdoob commented Aug 9, 2017

What did you exported it with? In 3DMax there used to be an option called Reset XForm that fixed this kind of stuff.

@mrdoob
Copy link
Owner

mrdoob commented Aug 9, 2017

Aside from this, I checked the model in Windows Paint3D and it loads correctly.

Hmm...

@looeee
Copy link
Collaborator Author

looeee commented Aug 9, 2017

@WestLangley thanks - that explains the appearance.

So it appears that FBX uses reflections as a way of transforming geometry, and assumes that the rendering software will support it. Since it's not supported in three.js that puts us in a bit of bind here.

My trigonometry is a bit rusty, but I don't think there is any way of representing a reflection as a series of rotations or other operations. Can you see any kind of workaround here?

@looeee
Copy link
Collaborator Author

looeee commented Aug 9, 2017

What did you exported it with? In 3DMax there used to be an option called Reset XForm that fixed this kind of stuff.

Yes, I'm exporting from 3ds max, and I've already tried the reset Xform trick. It doesn't fix this, unfortunately.

Also tried tried every other modifier I can think of as well as converting to editable poly or editable mesh before exporting.

@looeee
Copy link
Collaborator Author

looeee commented Aug 9, 2017

Here are the feet in 3DS Max. It looks like the right foot was created by mirroring the left.

feet

@WestLangley
Copy link
Collaborator

I believe in the official OBJ format -- for example -- the front face is determined by the face normal or vertex normals when specified. three.js/webGL uses the winding order to determine the front face.

After loading, you could try traversing your model and flip the winding order for all faces having matrixWorld with a negative determinant. But that is just a guess.

We should identify all loaders/formats from other vendors that handle this sort of thing (with examples) and figure out what technique they use.

@looeee
Copy link
Collaborator Author

looeee commented Aug 9, 2017

OK, I have successfully loaded the model with the following software:

Paint3D
paint3d

SketchFab (animations also play correctly here)
sketchfab

Clara.io (didn't test animations here)
clara io

I also tried loading it in Blender (latest version), but it's complete mess there no matter what settings I use.

@makc
Copy link
Contributor

makc commented Aug 10, 2017

tried loading it in Blender

off topic, but nao robot has official blender file somewhere on github. here's one copy.

@looeee
Copy link
Collaborator Author

looeee commented Aug 10, 2017

@makc thanks - that a much less detailed model though, and it's not rigged as far as I can see. I'm not very familiar with blender and learning how to set up rigging there in the time I have really won't be possible. I appreciate the suggestion though! 😄

@looeee
Copy link
Collaborator Author

looeee commented Aug 10, 2017

Note: I deleted a previous version of this comment as there was an error in my code.

@WestLangley I implemented your suggestion to invert winding order for the geometry of meshes whose .matrixWorld determinant is negative, and it seems to work.

object.traverse( ( child ) => {

  if ( child instanceof THREE.Mesh ) {

    if ( child.matrixWorld.determinant() < 0 ) {

      const geom = new THREE.Geometry().fromBufferGeometry( child.geometry );

      for ( let i = 0; i < geom.faces.length; i++ ) {

        const face = geom.faces[i];

        const tempA = face.a;

        face.a = face.c;
        face.c = tempA;

        child.geometry = new THREE.BufferGeometry().fromGeometry( geom );


      }

    }

  }

} );

The results look pretty good, but there are a couple of issues - the first is that is's sloowww - around 26 seconds for the robot mesh.

Second is that it breaks smoothing.

foot

I've tried fixing this by running .computeVertexNormals() and / or .computeFaceNormals() on the intermediate geometry. .computeVertexNormals() actually re-inverts the geometry (although unfortunately running it on it's own without reversing the winding order doesn't fix things).

@WestLangley
Copy link
Collaborator

First of all, don't instantiate new geometry; modify the existing geometry.

Second, if there are false positives, then that isn't the correct test.

@makc
Copy link
Contributor

makc commented Aug 10, 2017

  • what's the point of running this on all meshes, when you know exactly which ones have a problem?
  • what's the point of converting to geometry? just swap evry 2nd and 3rd position and normal in BufferGeometry

it breaks smoothing

of course, you did not swap normals

@makc
Copy link
Contributor

makc commented Aug 10, 2017

also, your model is rigged. this opens the room for problems with this method, you need to think of skin indices and weights

@makc
Copy link
Contributor

makc commented Aug 10, 2017

did you try to set side = THREE.BackSide on those meshes materials? if FBXLoader shares them between correct and inverted meshes, you might need to clone them too.

@looeee
Copy link
Collaborator Author

looeee commented Aug 10, 2017

@WestLangley there was an error in my code, I updated the previous comment. It seems that this is the correct test, although on second thoughts it would make more sense to do this in the loader if possible. But I'll switch to modifying the geometry in place now.

@makc your points in order:

what's the point of running this on all meshes?

I guess it would make more sense to do this in the loader whenever the 'Lcl_Scaling' has negative values. But I'd need to do more testing to figure out whether all three values have to be negative or not, and I'm hoping for a quickest solution here. I can come back and fix the loader later when I'm not as pressed for time.

what's the point of converting to geometry?

I just implemented this as a quick and dirty test to see if this method worked. But yes, modifying the buffer geometry directly would make more sense if it's possible

of course, you did not swap normals

Right, so I need to figure out how to swap the normals correctly. Do you have any hints here? Can I just invert the vector?

you need to think of skin indices and weights

Yes, probably. But one step at a time.

side = THREE.BackSide

Setting all the materials to DoubleSide was literally the first thing I tried.

@looeee
Copy link
Collaborator Author

looeee commented Aug 10, 2017

OK, I've switched to flipping the winding order on the buffer geometry directly, but it has some pretty weird effects.

  • it successfully inverts the geometry
  • it doesn't break smoothing
  • it rotates the entire geometry in some way
object.traverse( ( child ) => {

  if ( child instanceof THREE.Mesh ) {

    if ( child.matrixWorld.determinant() < 0 ) {

      const l = child.geometry.attributes.position.array.length;

      for ( let i = 0; i < l; i += 3 ) {

        const temp = child.geometry.attributes.position.array[ i ];

        child.geometry.attributes.position.array[ i ] = child.geometry.attributes.position.array[ i + 2 ];

        child.geometry.attributes.position.array[ i + 2 ] = temp;

      }

    }

  }

} );

Here's it's affect on just the foot - it's rendering correctly, but is rotated 90 degrees around y. Other geometries are also rotated - it's hard to tell, but I think all by 90 degrees although not necessarily around y.

foot-solo

I suspect this may be because of the initial reflections, investigating now.

@makc
Copy link
Contributor

makc commented Aug 10, 2017

that's an array of numbers. you are swapping coordinates, not positions

@looeee
Copy link
Collaborator Author

looeee commented Aug 10, 2017

@makc Doh! Right, my head is clearly too melted to continue with this today, time for sleep and I'll fix that in the morning

@mrdoob
Copy link
Owner

mrdoob commented Aug 10, 2017

@looeee
Copy link
Collaborator Author

looeee commented Aug 11, 2017

@mrdoob yes, it doesn't work. In fact, I was testing applying a negative scale to the object in Max to see if that fixes things. Here's what happens with x form.

Here are the initial feet. The right foot is mirrored and displays inverted when loaded in three.js. If I apply a reset xform modifier to it nothing happens ( the pivot is still inverted).

pre_scale

Next, I negatively scale it in the x-axis. The pivots are now equal.

post_scale

Finally I apply an x-form modifier to the right foot again, and the pivots are back in their initial (mirrored) state

pre_scale

Unfortunately negatively scaling in Max isn't a direct solution as the foot loaded in three.js now displays upside down

three_post_scale

@looeee
Copy link
Collaborator Author

looeee commented Aug 11, 2017

OK, here is the fixed reverse winding order function:

object.traverse( ( child ) => {

  if ( child instanceof THREE.Mesh ) {

    if ( child.matrixWorld.determinant() < 0 ) {

      const l = child.geometry.attributes.position.array.length;

      const positions = child.geometry.attributes.position.array;
      const normals = child.geometry.attributes.normal.array;

      for ( let i = 0; i < l; i += 9 ) {

        // reverse winding order
        const tempX = positions[ i + 0 ];
        const tempY = positions[ i + 1 ];
        const tempZ = positions[ i + 2 ];

        positions[ i + 0 ] = positions[ i + 6 ];
        positions[ i + 1 ] = positions[ i + 7 ];
        positions[ i + 2 ] = positions[ i + 8 ];

        positions[ i + 6 ] = tempX;
        positions[ i + 7 ] = tempY;
        positions[ i + 8 ] = tempZ;

        // switch vertex normals
        const tempNX = normals[ i + 0 ];
        const tempNY = normals[ i + 1 ];
        const tempNZ = normals[ i + 2 ];

        normals[ i + 0 ] = normals[ i + 6 ];
        normals[ i + 1 ] = normals[ i + 7 ];
        normals[ i + 2 ] = normals[ i + 8 ];

        normals[ i + 6 ] = tempNX;
        normals[ i + 7 ] = tempNY;
        normals[ i + 8 ] = tempNZ;

      }
    }
  }
} );

It's very fast (14ms on the robot model), and looks correct. Clearly there are still quite a few other problems, but it's a step forward at least!

fixed

As @makc pointed out this may affect vertex weighting. I'm not sure that this will be an issue as I don't think FBX exports skeletons anyway, and the only way I can get animations to play correctly (with any loader, not just three.js) is to bake and re-sample all animations on export. But nonetheless that will need to be tested before considering adding this to the loader.

@makc
Copy link
Contributor

makc commented Aug 11, 2017

I don't think FBX exports skeletons anyway

skins, and yes they do

@mrdoob
Copy link
Owner

mrdoob commented Aug 11, 2017

@looeee Just updated your post with a simplified code.

@looeee
Copy link
Collaborator Author

looeee commented Dec 8, 2017

Fixed by #12787 👏

@looeee looeee closed this as completed Dec 8, 2017
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

4 participants