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

Quaternion: Return early from slerpFlat if t is 0 or 1 #21183

Merged
merged 1 commit into from
Feb 3, 2021

Conversation

jure
Copy link
Contributor

@jure jure commented Jan 31, 2021

Description

In Quaternion.slerp we return early if t is 0 or 1, returning qa if t 0 and qb if t 1. We don't do the same in slerpFlat and it impacts performance in certain cases.

1.000.000 slerpFlats without early return in msec (using the quats in in Quaternion.tests.js):

t time in msec
0 56.661
1 60.9848
0.5 50.9151
0.25 47.1928
0.75 47.2224
0.5 36.2757
0.5 39.1083

1.000.000 slerpFlats with early return:

t time in msec
0 12.8091
1 14.754
0.5 52.2216
0.25 48.1502
0.75 47.7769
0.5 34.5647
0.5 37.7248

There is some natural variation, but for t 0 and t 1 the early return variant is on average 4-5x faster.

In a real world example where slerpFlat is used to interpolate between player rotations in a multiplayer scenario, I'm definitely seeing an improvement.

Happy to code golf it a bit more, but I'm looking for feedback on whether the change is appropriate first.

@jure
Copy link
Contributor Author

jure commented Feb 2, 2021

Anything else you'd like me to do here? Wondering so I can put a mental ✔️ here and let the winds of PR time take it away. It's not likely this will go out of sync with dev since it's a pretty stable piece of the codebase, so no hurry as far as I'm concerned.

@mrdoob mrdoob added this to the r126 milestone Feb 3, 2021
@mrdoob mrdoob merged commit d6d265f into mrdoob:dev Feb 3, 2021
@mrdoob mrdoob changed the title Return early from slerpFlat if t is 0 or 1 Quaternion: Return early from slerpFlat if t is 0 or 1 Feb 3, 2021
@mrdoob
Copy link
Owner

mrdoob commented Feb 3, 2021

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.

3 participants