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

ArrowHelper: Make .setLength() more robust. #17458

Merged
merged 1 commit into from Sep 10, 2019

Conversation

@Mugen87
Copy link
Collaborator

commented Sep 10, 2019

see https://discourse.threejs.org/t/cant-get-the-animation-to-work-three-matrix3-getinverse-cant-invert-matrix-determinant-is-0/5249/3?u=mugen87

This change will avoid the following warning if headLength is bigger than length.

THREE.Matrix3: .getInverse() can't invert matrix, determinant is 0

@WestLangley

This comment has been minimized.

Copy link
Collaborator

commented Sep 10, 2019

this.line.scale.set( 1, Math.max( Number.EPSILON, length - headLength ), 1 );

Setting the scale to be Number.EPSILON has consequences elsewhere. For example, the normal matrix in this case is:

Screen Shot 2019-09-10 at 1 56 56 PM

It is true that the normal matrix is not used in this case because the material is of the basic type.

However, since humans can only see 3-significant digits -- or so -- I would suggest doing something like this, instead:

this.line.scale.set( 1, Math.max( 0.0001, length - headLength ), 1 ); // ref this post

That should not have bad numerical behavior, and the line would not be visible because it is so short.

@Mugen87

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 10, 2019

I'm fine with that 👍

@Mugen87 Mugen87 force-pushed the Mugen87:dev38 branch from b0da110 to eac7b75 Sep 10, 2019

@mrdoob mrdoob added this to the r109 milestone Sep 10, 2019

@mrdoob mrdoob merged commit cd70772 into mrdoob:dev Sep 10, 2019

1 of 2 checks passed

LGTM analysis: JavaScript Running analyses for revisions
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@mrdoob

This comment has been minimized.

Copy link
Owner

commented Sep 10, 2019

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.