Hmm... Would you be willing to investigate the history of this, working backwards from #14379?

Perhaps this can be refactored, and the determinant can be computed only once as object.matrixWorld.determinant(), which is independent of the shadow camera.

The determinant is already computed for each object for both normal and shadow rendering before this fix. All this fix adds it matrix inversion computation for the shadow object rendering (the same computation already is present in normal object rendering). Unless there is some experiment which shows this fix introduces a measurable performance impact, I suggest leaving possible optimizations for separate issues / PRs.

Does your edge case go away if we did this, instead?

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

Yes, that fixes it fine, as matrixWorld is already computed both for shadows and normal objects.

I think this is a good idea for a proper fix, as the computations done are:

    `object`.modelViewMatrix.multiplyMatrices(camera.matrixWorldInverse, `object`.matrixWorld)
    `object`.normalMatrix.getNormalMatrix(`object`.modelViewMatrix)

I think we can safely assume camera matrix is a non-mirroring one. Other operations do not change matrix determinant sign.

I think we can safely assume camera matrix is a non-mirroring one.

Actually, not, apparently. (#14372)

This is why I asked you to study the history of #14379. If you look at #12787, the original PR used matrixWorld.

Personally, I do not think reflections in camera.matrixWorld should be supported. For one thing, we would have to fix OrbitControls to support it, and it is not even clear to me what the behavior should be in that case.

Sigh. A proper fix could then be to multiply signs of camera.matrixWorld and object.matrixWorld determinants (3x3 determinants should do). The camera.matrixWorld one can be computed once for the whole scene.

multiply signs of camera.matrixWorld and object.matrixWorld determinants (3x3 determinants should do).

Pushed a commit implementing this.

The camera.matrixWorld one can be computed once for the whole scene.

While this is possible to add, the current implementation computing two 3x3 determinants should be faster then the previous one which computed a 4x4 one, therefore I am not convinced adding the logic to handle the global value computation is worth the effort.

@WestLangley Does this look good to you?

Matrix4.determinant3x3() should be documented here: https://threejs.org/docs/#api/en/math/Matrix4

@WestLangley Does this look good to you?

I am not in favor of this latest change to the PR. This is overkill, IMO.

I would implement what I suggested above:

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

which is the approach originally used in #12787.

The approach in #12787 was modified in a later PR to accommodate a user who wanted to negatively-scale the camera matrix. But the modification did not test shadows or the effects on other features like the controls. The library does not fully-support a negative scale on the camera.

I would restore the approach used in #12787. Shadows will then work correctly.

I would also not worry about determinant3x3(). The need for that optimization has not been demonstrated.

Instead, if there is a performance issue, we can optimize the code in determinant().

Ok, I have no preference on this. Back to the previous commit.

Thanks!

@OndrejSpanel Thank you for working on this.