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

Blender-inspired rotation of objects in 3D has an unwanted rotation jump when the tool is initialized #85810

Closed
Lamoot opened this issue Dec 5, 2023 · 3 comments · Fixed by #90098

Comments

@Lamoot
Copy link

Lamoot commented Dec 5, 2023

Tested versions

  • Reproducible in 4.2 stable release
  • Not reproducible in 4.1.3 stable release

System information

Godot v4.2.stable - Ubuntu 23.04 23.04 - X11 - Vulkan (Forward+) - dedicated AMD Radeon RX 570 Series (RADV POLARIS10) () - AMD Ryzen 5 3600 6-Core Processor (12 Threads)

Issue description

When rotating an object in 3D using the blender-inspired tools (as implemented by #56543) there is a rotation jump of the object when the tool is initialized.

godot_4.2_rotation_jump.mov

For comparison, the rotation tool works properly and as expected in 4.1.3 stable release.

godot_4.1_no_rotation_jump.mov

Could this be related to mouse wrap improvement that came in Godot 4.2 #59467 ?
The line from the origin of the transformation to the mouse cursor is another indicator of what could be happening under the hood. In 4.1.3 it connects properly but in 4.2 it doesn't follow the mouse cursor exactly.

Steps to reproduce

1.) In the 3D view, rotate an object using the blender-inspired transformation tool. When the tool starts, the object will jump in its rotation.

Minimal reproduction project (MRP)

Happens in a bare, default Godot project.

@viksl
Copy link
Contributor

viksl commented Apr 1, 2024

It's not when the tool is initialized but when the rotation happens.
If you don't move your mouse at all after invoking the rotaion and wait a bit the line from the object will be drawn properly to the cursor showing something is done properly at the first step but the moment your carry on rotating then either the cursor position or the object transform or both are done wrong and the center for rotation is elsewhere on the screep as can be seen in this report:
#90081

@viksl
Copy link
Contributor

viksl commented Apr 1, 2024

Here's a PR #90098 which enables the wrapping for the rotation which should also fix this issue.
Whether this PR will get merged or not is a different question. Originally the godot's implementation disallowed the wrapping for rotation but blender has it so I think we should just keep it too since that was the original purpose of bringing blender style controls into godot.


Anyway the issue itself, there are two input methods input and sinput involved in this issue.
input - gives position based on Godot Editor (which reports position even when the cursor outside the viewport and thus allows wrapping)
sinput - gives position based on the Viewport and not outside (used for gizmos that's why they work just fine but only if the cursor remains inside the viewport)

The issue comes from the fact that when begin_transform is called it stores the initial position based on the Viewport but then only the input is called which returns position based on the editor/screen itself and thus there's an offset which shows as a jump which you can see in this report at the beginning, it also shows as the line from the object doesn't point at the cursor position, due to the offset the center of rotation is not at the same position as the object itself on the screen which lead to my report here wit ha video: #90081

Personally I think we should allow wrapping for reasons mentioned in the PR, the change is small so I just made it and left it up for testing discussion if someone has an issue with it? I haven't found a reason why it was blocked and during testing haven't found any issues so far - the functionality already works for translation and scale so the wrapping itself has been tested thoroughly already so I think the PR is safe but check it out, please and voice your opinions.

@Lamoot
Copy link
Author

Lamoot commented Apr 1, 2024

Thank you for looking into it and addressing the issue 👍

@akien-mga akien-mga added this to the 4.3 milestone Jun 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants