-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Gui: Added FreeTurntable orbit style #8048
Conversation
I'm pretty sure the |
664aca7
to
40f3727
Compare
40f3727
to
d7b8402
Compare
Many thanks for your PR. I tested it but need a brief description what the difference of this style to the Trackball style is. |
Yeah sure.
This is useful to create rotating animations of your object as you are sure to make a perfect circle around it. The FreeTurntable that I added does not select an axis before applying transformations, both mouse's X and Y movements are taken into account. The goal is to replicate Blender's orbit style. So the difference to the Trackball is the need for the world's Z axis to stay aligned with the screen's Y axis. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, I only encounter an issue when setting the orbit style via the footer in the FreeCAD main Window
src/Gui/DlgSettingsNavigation.ui
Outdated
@@ -7,7 +7,7 @@ | |||
<x>0</x> | |||
<y>0</y> | |||
<width>500</width> | |||
<height>391</height> | |||
<height>472</height> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
80 pixels more for one additional setting seems strange
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Qt Designer modified this.
That's the first time I use it, so maybe I touched something I shouldn't have.
Should I revert this directly by editing the .ui file ?
src/Gui/NavigationStyle.cpp
Outdated
if (zaxis[1] < 0) { | ||
if (dif[0] < 0) | ||
else if (orbit == Turntable) { | ||
// 0000333: Turntable camera rotation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does the 0000333 mean? I know this is not your code but maybe you have an idea?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know, it was there before so I didn't want to touch it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's the old.ticket numbering format for the mantis bug tracker. It's linking to ticket: https://tracker.freecadweb.org/view.php?id=333
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh great, so now that we have both possibilities, should we remove this comment ?
aTurntable.setChecked(True) | ||
else: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see an issue here because when using the menu, I don't get the orbit style I want (I cannot set Turntable). Why not set an if, elif, elif to catch all possible 3 orbit styles?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like this ?
Thanks. I understand now. In general, we need to improve the Wiki to explain the differences better. Let's do this after this PR is merged. |
Many thanks. I merged and fixed the remaining issue that you changed the default orbit style: 83a7c57 |
- the submenu ordering must be coherent to the one in the preferences combobox otherwise one gets the wrong orbit style displayed in the preferences dialog
There was another issue, I fixed now: 8b6797e |
Finally, the docs must be updated. Can you please add your feature addition to our release notes, see the info text in your initial PR comment? |
OK, I added it now to the release notes: |
There was a further bug that the currently active orbit style was not correctly displayed in the status bar. I could fix this: #8072 |
Sorry for commenting on an already merged PR, but I wanted to thank you for implementing this. |
Adding a second Turntable orbit style but without primary rotation selection.
This aims to offer an environment similar to Blender.
Thank you for creating a pull request to contribute to FreeCAD! Place an "X" in between the brackets below to "check off" to confirm that you have satisfied the requirement, or ask for help in the FreeCAD forum if there is something you don't understand.
Please remember to update the Wiki with the features added or changed once this PR is merged.
Note: If you don't have wiki access, then please mention your contribution on the 1.0 Changelog Forum Thread.