Conversation
…ions for round shapes.
BD103
left a comment
There was a problem hiding this comment.
Good docs and unit tests check out. I have one non-blocking nit, but beyond that it looks good!
Co-authored-by: BD103 <59022059+BD103@users.noreply.github.com>
|
Oops, I accidentally though that @Jondolf's review was an approval. Marking this as |
janhohenheim
left a comment
There was a problem hiding this comment.
Left some documentation improvement suggestions, but nothing blocking :)
Courtesy of @janhohenheim Co-authored-by: Jan Hohenheim <jan@hohenheim.ch>
|
@janhohenheim Great suggestions, thanks a bunch. |
|
Looks like they had some syntax errors, sorry about that 😅 |
|
We gotta keep the CI on its toes somehow, right? |
|
ah... are these the infamous doctests I've heard of? How do I fix this CI failure? |
|
🎉 |
Jondolf
left a comment
There was a problem hiding this comment.
LGTM! Adding a similar method for quaternions would be nice, but we can do that later, either by adding it to Glam directly or by using an extension trait.
|
Added an issue for this to |
Co-authored-by: Joona Aalto <jondolf.dev@gmail.com>
|
I ran benchmarks for this and wrote about them in bitshifter/glam-rs#540 (comment). The speed change, while an improvement, is only in a few picoseconds. This is desirable for hot code, but may not be worth upstreaming. |
|
I wonder if the performance improvement would be larger when the |
I'm not sure how to do that myself, but feel free to try it! This is the code that I used to benchmark. |
Aceeri
left a comment
There was a problem hiding this comment.
Neat! Hadn't thought about it like this before, but makes sense.
|
Thank you to everyone involved with the authoring or reviewing of this PR! This work is relatively important and needs release notes! Head over to bevyengine/bevy-website#1664 if you'd like to help out. |
Objective
Solution
Testing