WIP: Support negative scale factors

[WestLangley]

This PR addresses the issue raised in #4904 -- negative scale in the object matrix.

Users expect

object.scale.x = - 1;

to "just work". The problem is that application of a negative scale (i.e., a reflection in the object matrix) turns a watertight mesh "inside out".

That is, application of a negative scale -- either to a geometry or to a mesh -- flips the normals as it should, but does not alter the winding order, so the front-faces and normals become inconsistent.

Note that the problem is with the winding order, not the normals.

This PR proposes adding a property to a mesh to "invert the winding order".

The advantage of this approach is the original mesh and the mirrored mesh can share the same geometry and material.

This differs from #4904, where the proposal was to add a material property, instead.

Here is a typical use case:

// mesh
var mesh1 = new THREE.Mesh( geometry, material );
mesh1.scale.set( 1, 1, 1 );
scene.add( mesh1 );

// mesh with negative scale
var mesh2 = new THREE.Mesh( geometry, material ); // re-use geometry and material
mesh2.scale.set( - 1, 1, 1 );
mesh2.frontFaceCW = true; // property can be renamed to something else
scene.add( mesh2 );

Implementation is simple, as is evident by the diffs.

Issues:

1. We could automate this by checking the determinant of the object's world matrix every frame, but I don't think that is a good idea.

2. This solutions does not address the situation where a parent Group, for example, has a negative scale.

3. We would have to modify the raycasting logic to accommodate this feature.

4. We need to decide if we want to add a property to the Mesh Class, itself -- or if a flag on the instance is adequate, as is demonstrated here.

5. . . .

This PR is for discussion, and is not intended to be merged.

[looeee]

I would be very happy if a solution to this issue gets merged!

Thoughts:

1. users expect negative scale to "just work": while it's reasonable to expect three.js developers to learn that this doesn't work as expected, the problem is that many 3D artists use negative scaling as a modelling technique, in which case the developer may have no control over this situation (if they are using 3rd party assets for example). That's why this is needed, to my mind at least.

2. We could automate this by checking the determinant of the object's world matrix every frame, but I don't think that is a good idea. This is not that common an issue - it should minimally affect performance and workflow for people that have not encountered the problem, so I agree that's it's best to avoid this if possible.

[mrdoob]

We could automate this by checking the determinant of the object's world matrix every frame, but I don't think that is a good idea.

How cpu intensive would that be...?

[WestLangley]

@mrdoob I have not investigated the performance issue, but it does appear that this pattern works in general:

var onBeforeRender = function() {

    this.frontFaceCW = ( this.isMesh && this.matrixWorld.determinant() < 0 );

};

So we a have a proof-of-concept. (In practice, we would have the renderer handle this.) Whether we automate this is up to you.

I suppose automation would be handy if a user has a hierarchical model and set a negative scale at the root.

Might as well /ping @bhouston for feedback...

[mrdoob]

So we a have a proof-of-concept. (In practice, we would have the renderer handle this.) Whether we automate this is up to you.

I vote for adding this to the renderer 👌

[haroldiedema]

Apologies for the 'intrusion', but is this related to the issue where I have to flip some of my skybox textures every time before using them in my game? For example, download a standard set of 6 skybox textures. Apply them in the correct order as 'multi material' as demonstrated in some of the examples and then you see that the bottom and top images are not matching up. Flipping the top and bottom 180 degrees seems to fix it.

My skybox mesh is of type BoxBufferGeometry with a negative scale set to it.

If this is it, it would surely explain a lot and I feel kind of silly not figuring this out by myself 😅

[WestLangley]

@haroldiedema No. Whatever problems you are facing are not likely related to the issue addressed in this thread.

[WestLangley]

I vote for adding this to the renderer

Done. The renderer now handles reflections in the world transform automatically.

The only example I have found so far that breaks if a negative scale is applied to a root object is webgl_loader_mmd_audio.html. I am not sure what is going on there...

[bhouston]

This is a good solution thank you @WestLangley.

I am not a fan of the variable name "frontFaceCW", instead I would prefer a more precise "frontFaceWindingOrder = THREE.WindingOrderClockWise", or something like that, but that is just me being perfectionist/difficult. Feel free to ignore me.

[bhouston]

BTW I do worry about the performance of this change. Maybe we should confirm it is not a major problem via the examples that have a lot of objects?

Do we calculate a matrix inverse for each object when rendering it? If we do, then we are already calculating the determinant and maybe we could piggyback off that change. If we are not calculating it per object per frame then I worry about the performance impact, especially on slow CPU devices like mobile.

If it is a blocker in terms of performance, we could add a flag, to the renderer e.g. renderer.autoWindingOrder = true/false that does this change on a per object per frame basis?

Most people do not need this functionality. And even us, who do need it, do not need it to be automatic as we know when matrices go negative determinatnt. Thus I believe that there is only a small minority who need this do be done automatically on a per frame, per object basis.

[haroldiedema]

Unrelated to this PR, but this caught my attention.

Why are we calling toUpperCase() 256 times instead of 1 time for the final result when the UUID is generated? This looks like a silly performance issue.

edit: Nevermind, I'm blind apparently... I didn't see the for-loop was only executed once until I opened the code in my IDE.

[WestLangley]

Unrelated to this PR

It is unrelated. Please file a new issue if you feel it is warranted.

[WestLangley]

@bhouston I have the same concerns that you have, which is why this is proposed as a WIP.

But to continue the discussion, one advantage of automating this is a hierarchy of meshes can be mirrored by setting a negative scale on a root object or group. There is no need to set a flag on each mesh in the hierarchy.

BTW, when automating this, the negative determinant that needs to be checked is on matirxWorld, not matrix.

I can change the variable name once there is agreement on an approach.

[mrdoob]

Any chance you can remove build/three.js from the PR (will make things easier in case we have to revert).

Apart from that it looks good to me 👍

[WestLangley]

I'll file a new PR. :)

[WestLangley]

@bhouston As requested by @mrdoob, I filed #12787. If the automated approach turns out to be non-performant, we can revert to an alternate approach and discuss a new API. Thank you for your feedback. :)

[mrdoob]

@looeee

I would be very happy if a solution to this issue gets merged!

It got merged! 😊

[pailhead]

What's the summary here?
Unexpectedly, i found myself not having to do anything when applying a negative scale to a mesh.

[bhouston]

I think it was merged. :)

[bhouston]

#12787