-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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 morph crash. #379
Fix morph crash. #379
Conversation
Instead of |
@emackey Good idea. I changed the vector comparison to use an epsilon as well. Anything else? |
I was going to nag you for tests, but it looks like there are 0 Am I correct in that this has nothing to do with #377 |
You are correct. I know what the problem is for #377 and it is separate. |
// If the camera and frustum are already in position for the switch to | ||
// a perspective projection, nothing needs to be animated. | ||
camera.position = endPos2D; | ||
camera.frustum = frustumCV.clone(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not frustumCV.clone(camera.frustum);
? Which will re-use the existing instance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
camera.frustum
is an OrthographicFrustum
. frustumCV
is a PerspectiveFrustum
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense. Merging.
Prevent an animation duration of 0ms. In Tween.js, the elapsed time is computed as
(time - startTime) / duration
and clamped at one. This is fine for the most common case wheretime > startTime
which gets divided by zero to equal infinity and clamped to one. In the case wheretime == startTime
(which explains why it was so hard to reproduce), the elapsed time will beNaN
and cause a crash.This fixes the original problem in #319.
#338 is unrelated.