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

Fix lerpVectors for when v1 === this #19435

Merged
merged 1 commit into from
May 25, 2020
Merged

Conversation

sirxemic
Copy link
Contributor

When calling v.lerpVectors(v1, v2, alpha) with v1 being equal to v, the function gives incorrect output. I don't think we should have to do a check like

if (v1 === v) v.lerp(v2, alpha)
else v.lerpVectors(v1, v2, alpha)

so I decided to fix the function.

@WestLangley
Copy link
Collaborator

How does the value of v, prior to calling the method, affect the result?

@sirxemic
Copy link
Contributor Author

Not sure if I understand your question (or if you understand what I'm trying to point out).

But yeah, when v1 === v, then v.lerpVectors(v1, v2, alpha) basically returns 2 * (v2 - v1) * alpha, because the current implementation then basically boils down to this.subVectors( v2, this ).multiplyScalar( alpha ).add( this ), which hopefully you can see is clearly not correct.

@WestLangley
Copy link
Collaborator

WestLangley commented May 23, 2020

So, instead of calling

v.lerp( v2, alpha );

you are calling:

v.lerpVectors( v, v2, alpha ); // v repeated

I see.

@sirxemic
Copy link
Contributor Author

sirxemic commented May 24, 2020

Well, I'm calling v.lerpVectors( v1, v2, alpha ). It just so happens that v and v1 are pointing to the same object, which is why I said I shouldn't have to be doing the following.

if (v1 === v) v.lerp(v2, alpha)
else v.lerpVectors(v1, v2, alpha)

@mrdoob
Copy link
Owner

mrdoob commented May 25, 2020

Well, I'm calling v.lerpVectors( v1, v2, alpha ). It just so happens that v and v1 are pointing to the same object

Hmmm, okay...

@mrdoob mrdoob added this to the r117 milestone May 25, 2020
@mrdoob mrdoob merged commit 2638d65 into mrdoob:dev May 25, 2020
@mrdoob
Copy link
Owner

mrdoob commented May 25, 2020

Thanks!

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

Successfully merging this pull request may close these issues.

4 participants