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

add Slerp for 3D rotations #252

Merged
merged 5 commits into from
Mar 2, 2023
Merged

Conversation

bicycle1885
Copy link
Contributor

@bicycle1885 bicycle1885 commented Feb 28, 2023

This PR adds slerp for 3D rotations. The method for QuatRotations is the canonical implementation and methods for other rotation types are implemented via this canonical one.

I don't know which representation it should return for arguments with mixed types. The proposed one returns typeof(r1) for slerp(r1, r2, t).

Closes #214.

@codecov
Copy link

codecov bot commented Feb 28, 2023

Codecov Report

Merging #252 (6b49efb) into master (2f9be71) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #252      +/-   ##
==========================================
+ Coverage   88.82%   88.84%   +0.02%     
==========================================
  Files          16       16              
  Lines        1602     1605       +3     
==========================================
+ Hits         1423     1426       +3     
  Misses        179      179              
Impacted Files Coverage Δ
src/euler_types.jl 92.27% <100.00%> (+0.01%) ⬆️
src/unitquaternion.jl 97.56% <100.00%> (+0.02%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Collaborator

@hyrodium hyrodium left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution! I added comments about the return types and one-axis rotations.

src/unitquaternion.jl Show resolved Hide resolved
src/unitquaternion.jl Outdated Show resolved Hide resolved
@bicycle1885
Copy link
Contributor Author

Thank you for reviewing. I've update the PR following your advice. Can you look at the changes and give me feedback again?

Copy link
Collaborator

@hyrodium hyrodium left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@hyrodium hyrodium merged commit 6ec40c2 into JuliaGeometry:master Mar 2, 2023
@bicycle1885 bicycle1885 deleted the slerp branch March 2, 2023 12:01
@bicycle1885
Copy link
Contributor Author

Thank you!

@c42f
Copy link
Member

c42f commented Mar 3, 2023

I haven't been active here lately, but I thought I'd pop in and say thanks @hyrodium for doing a great job maintaining this package. It's amazing to have people like you step in and care for code I once worked on, but don't have time for right now. Thanks ❤️

(Also thanks @bicycle1885 for the specific contribution here, this looks nice :-) )

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.

Add slerp function
3 participants