[Merged by Bors] - Fix incorrect rotation in `Transform::rotate_around`. #5300

tim-blackbird · 2022-07-12T21:42:45Z

Someone noted that the rotate_around method did not give the results they expected: discord thread
I tested rotate_around and their workaround and it seems like it was indeed incorrect.

Here is a scene with some cubes at different angles all being rotated around the center on the Y axis.

2022-07-12.22-52-49.mp4

Interestingly, the middle cube rotates as you might expect. This threw me for a bit of a loop before I added the other cubes to the test haha.

Here is the same scene with the order multiplication of the quaternions flipped in rotate_around.

2022-07-12.22-53-20.mp4

That looks better :)

Changelog

Fixed rotate_around rotating the wrong way around
Added translate_around. - Split out the translation code from rotate_around.
Simplified/optimized rotate_local_* methods. - Yep, That works somehow.

^{Quaternions sure are wacky. Do not ask me how this works exactly, haha.}

tim-blackbird · 2022-07-12T21:45:19Z

I really need to stop forgetting to edit the PR title.
^{But I will never stop with the terrible commit messages hueheu /hj}

IceSentry

The changes make sense to me. Should this be considered a breaking change since it would change the behavior of an api?

Nilirad · 2022-07-12T21:56:49Z

The changes make sense to me. Should this be considered a breaking change since it would change the behavior of an api?

I think not, because from what I understood, the current behavior is incorrect, therefore a bug.

tim-blackbird · 2022-07-12T23:04:12Z

I added rotate_local() and rotate_local_axis() methods to mirror the existing non-local methods.
Also added to and edited the documentation a bit.

crates/bevy_transform/src/components/transform.rs

alice-i-cecile · 2022-07-13T14:52:01Z

bors r+

Someone noted that the `rotate_around` method did not give the results they expected: [discord thread](https://discord.com/channels/691052431525675048/996497295325544479) I tested `rotate_around` and their workaround and it seems like it was indeed incorrect. Here is a scene with some cubes at different angles all being rotated around the center on the Y axis. https://user-images.githubusercontent.com/29694403/178598432-407d7e80-1caf-4b17-b69b-66d9156c81e1.mp4 Interestingly, the middle cube rotates as you might expect. This threw me for a bit of a loop before I added the other cubes to the test haha. Here is the same scene with the order multiplication of the quaternions flipped in `rotate_around`. https://user-images.githubusercontent.com/29694403/178598446-a98026f3-524c-448b-8437-4d0d3175c6ca.mp4 That looks better :) ## Changelog * Fixed `rotate_around` rotating the wrong way around * Added `translate_around`. - Split out the translation code from `rotate_around`. * Simplified/optimized `rotate_local_*` methods. - Yep, That works somehow. Quaternions sure are wacky. Do not ask me how this works exactly, haha. Co-authored-by: devil-ira <[email protected]>

bors · 2022-07-13T15:05:45Z

Build failed (retrying...):

check-compiles

Someone noted that the `rotate_around` method did not give the results they expected: [discord thread](https://discord.com/channels/691052431525675048/996497295325544479) I tested `rotate_around` and their workaround and it seems like it was indeed incorrect. Here is a scene with some cubes at different angles all being rotated around the center on the Y axis. https://user-images.githubusercontent.com/29694403/178598432-407d7e80-1caf-4b17-b69b-66d9156c81e1.mp4 Interestingly, the middle cube rotates as you might expect. This threw me for a bit of a loop before I added the other cubes to the test haha. Here is the same scene with the order multiplication of the quaternions flipped in `rotate_around`. https://user-images.githubusercontent.com/29694403/178598446-a98026f3-524c-448b-8437-4d0d3175c6ca.mp4 That looks better :) ## Changelog * Fixed `rotate_around` rotating the wrong way around * Added `translate_around`. - Split out the translation code from `rotate_around`. * Simplified/optimized `rotate_local_*` methods. - Yep, That works somehow. Quaternions sure are wacky. Do not ask me how this works exactly, haha. Co-authored-by: devil-ira <[email protected]>

bors · 2022-07-13T15:27:41Z

Pull request successfully merged into main.

Build succeeded:

Someone noted that the `rotate_around` method did not give the results they expected: [discord thread](https://discord.com/channels/691052431525675048/996497295325544479) I tested `rotate_around` and their workaround and it seems like it was indeed incorrect. Here is a scene with some cubes at different angles all being rotated around the center on the Y axis. https://user-images.githubusercontent.com/29694403/178598432-407d7e80-1caf-4b17-b69b-66d9156c81e1.mp4 Interestingly, the middle cube rotates as you might expect. This threw me for a bit of a loop before I added the other cubes to the test haha. Here is the same scene with the order multiplication of the quaternions flipped in `rotate_around`. https://user-images.githubusercontent.com/29694403/178598446-a98026f3-524c-448b-8437-4d0d3175c6ca.mp4 That looks better :) ## Changelog * Fixed `rotate_around` rotating the wrong way around * Added `translate_around`. - Split out the translation code from `rotate_around`. * Simplified/optimized `rotate_local_*` methods. - Yep, That works somehow. Quaternions sure are wacky. Do not ask me how this works exactly, haha. Co-authored-by: devil-ira <[email protected]>

Nice quats bro.

d009d8c

tim-blackbird changed the title ~~Nice quats bro.~~ Fix incorrect rotation in Transform::rotate_around. Jul 12, 2022

Nilirad added C-Bug An unexpected or incorrect behavior A-Transform Translations, rotations and scales labels Jul 12, 2022

IceSentry approved these changes Jul 12, 2022

View reviewed changes

alice-i-cecile added this to the Bevy 0.8 milestone Jul 12, 2022

alice-i-cecile approved these changes Jul 12, 2022

View reviewed changes

alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Jul 12, 2022

Added more rotate_local* methods.

3516aba

mockersf approved these changes Jul 12, 2022

View reviewed changes

jpsikstus reviewed Jul 12, 2022

View reviewed changes

crates/bevy_transform/src/components/transform.rs Show resolved Hide resolved

bors bot changed the title ~~Fix incorrect rotation in Transform::rotate_around.~~ [Merged by Bors] - Fix incorrect rotation in Transform::rotate_around. Jul 13, 2022

bors bot closed this Jul 13, 2022

tim-blackbird deleted the transform-fixes branch October 21, 2022 15:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Merged by Bors] - Fix incorrect rotation in `Transform::rotate_around`. #5300

[Merged by Bors] - Fix incorrect rotation in `Transform::rotate_around`. #5300

tim-blackbird commented Jul 12, 2022 •

edited

Loading

tim-blackbird commented Jul 12, 2022 •

edited

Loading

IceSentry left a comment

Nilirad commented Jul 12, 2022

tim-blackbird commented Jul 12, 2022

alice-i-cecile commented Jul 13, 2022

bors bot commented Jul 13, 2022

bors bot commented Jul 13, 2022

[Merged by Bors] - Fix incorrect rotation in Transform::rotate_around. #5300

[Merged by Bors] - Fix incorrect rotation in Transform::rotate_around. #5300

Conversation

tim-blackbird commented Jul 12, 2022 • edited Loading

Changelog

tim-blackbird commented Jul 12, 2022 • edited Loading

IceSentry left a comment

Choose a reason for hiding this comment

Nilirad commented Jul 12, 2022

tim-blackbird commented Jul 12, 2022

alice-i-cecile commented Jul 13, 2022

bors bot commented Jul 13, 2022

bors bot commented Jul 13, 2022

[Merged by Bors] - Fix incorrect rotation in `Transform::rotate_around`. #5300

[Merged by Bors] - Fix incorrect rotation in `Transform::rotate_around`. #5300

tim-blackbird commented Jul 12, 2022 •

edited

Loading

tim-blackbird commented Jul 12, 2022 •

edited

Loading