-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Add ability to roll the camera in 3D plots #21426
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
Conversation
Here's my test script for whoever reviews this. You can copy in the text annotations from this example if you want to try that out - they work fine. from mpl_toolkits.mplot3d import axes3d
import matplotlib.pyplot as plt
import matplotlib.animation as animation
fig = plt.figure()
ax = fig.add_subplot(projection='3d')
# Grab some test data.
X, Y, Z = axes3d.get_test_data(0.05)
# Plot a basic wireframe.
ax.plot_wireframe(X, Y, Z, rstride=10, cstride=10)
ax.set_xlabel('x')
ax.set_ylabel('y')
ax.set_zlabel('z')
vertical_axis = 'z'
elev, azim, roll = 30, 0, 0
ax.view_init(elev, azim, roll, vertical_axis=vertical_axis)
'''
def update_view(num):
elev = num
azim = num
roll = num
ax.view_init(elev, azim, roll, vertical_axis=vertical_axis)
anim = animation.FuncAnimation(fig, update_view, interval=25, save_count=360)
anim.save("roll.mp4")
#'''
plt.show(block=True) |
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.
Thank you for opening your first PR into Matplotlib!
If you have not heard from us in a while, please feel free to ping @matplotlib/developers
or anyone who has commented on the PR. Most of our reviewers are volunteers and sometimes things fall through the cracks.
You can also join us on gitter for real-time discussion.
For details on testing, writing docs, and our review process, please see the developer guide
We strive to be a welcoming and open project. Please follow our Code of Conduct.
a6e55e3
to
e1770bc
Compare
Note that I believe the inversion at about 6.5 seconds into the second video is due to the issue addressed by PR #10762 |
I updated this to fix issue #10241 as well, since PR #10762 looks abandoned and this fix couples into the roll angle as well. Per the discussion in that PR, I only normalized the angles when generating the projection matrix, not when storing them. The second animation looks a lot better now. roll2.mp4 |
e100d14
to
4ce3252
Compare
Thanks for the contribution! The animations look really great! can you please comment how this PR behaves wrt. to my two remarks in #14453 (comment)? |
Sure thing!
|
@jklymak So the problem here is that elevation and azimuth already break with standard axis rotations. As matlab (ref 1, ref 2) and matplotlib currently define them, azimuth corresponds to a standard right-handed "yaw" rotation about z, but elevation corresponds to a left-handed "pitch" rotation about y (i.e. elevation = -pitch). This really breaks with all other normally used conventions which only use right-handed rotations. And matlab doesn't use a roll angle with the elev/azim convention. In that second reference, mathworks does talk about another camera module they have which uses roll/pitch/yaw as the camera frames. They cite "IEEE. Standard for Distributed Interactive Simulation – Application Protocols. IEEE P1278.1/D16 Rev 18, May 2012." as the source for that. But those are all right-hand rotations, and to change to match with that would break the current implementation. So we have to keep elevation and azimuth as-is for backwards compatibility. But we're a bit in the dark as to prior art for how to define the roll angle. To be clear it should definitely go last in the rotation order here, but whether it's a left-hand or right-hand rotation is up to us. I currently have it defined as a left-hand rotation as shown here (first image below). Searching around I found a slide deck that shows that OpenGL uses a right-hand rotation as shown here (second image below), but note that with how the XYZ axes are defined in the picture with the plane, their rotations are all left-handed, which makes me really question this source. Note also that in both of these sources, 1) their elevation and azimuth rotations are both flipped in handedness from our/matlab convention, and 2) the axes that are being rotated about also conflict. So we can't straight up adopt OpenGL's approach. Both those sources also call this angle something different - "tilt". Using this or "twist" might help disambiguate it if a more complex camera with roll/pitch/yaw ever gets added. The image I've found with the clearest example is this one, though it's not really sourced to anything. The roll/"twist" rotation here is right-handed as viewed from the origin. Is anyone's head hurting yet? :) To make a simple summary, we're pretty locked in but still have to decide which way the purple arrow is pointing in the bottom image. I'm leaning now towards what's in the bottom image but calling it "tilt" instead of "roll". If anyone has a good canonical source or prior art on this I'm all ears. And this should probably get a documentation image that's clear on the sign conventions. Another summary: |
@scottshambaugh yes, Euler angles always hurt the head! a) agree that we can't break our current meaning of Angles are somewhat funny because are they what happens if I turn the camera in the positive direction or are they what happens to the scene when I turn in the positive direction? Matplotlib seems consistent to me in choosing camera movement. i.e. azimuth=10 moves the camera 10 degrees right-handedly around the z axis. elevation=10 moves the camera up 10 degrees from the original x-y plane. This is exactly the same as your last diagram. If you agree that these two angles mean where the camera goes, then I think the "roll" or "twist" should be right handed movement of the camera, again, just as your diagram shows i.e. roll=10 will spin the camera 10 degrees counter-clockwise. However, again, that means the scene will spin 10 degrees clockwise. It would be great to include a diagram like the above in the docs so this is crystal clear for folks. It would probably be better made in something that has light shading. If the one above is open source, it would be fine. None of this is made simpler by the choice of azimuth=0 as looking from the positive x-axis side! If we add this, I would suggest two diagrams: |
Ok, I opened up issue #21508 for creating that diagram for the docs. Could use a hand with that, creating those kinds of images isn't my strong suit. I renamed the angle to "tilt" to disambiguate from roll/pitch/yaw, and added that speedup optimization for the default tilt=0. The rotation directions were kept as originally, meaning that the last image there correctly shows the movement of the camera (ie positive tilt means the camera rotates counterclockwise, world rotates clockwise). That means the coordinate transformation of the world axes are the second option: |
8ad61bd
to
2e54be2
Compare
4278447
to
21f1177
Compare
I think thats fine. PR cleanliness is failing because you have probably added and removed the same images in different commits. Rebase your commits to a single commit (or a couple if they make sense to you) and force-push back to GitHub and the PR should pass.... |
Add roll angle for 3d plot examples Align rotation of 3D plot by mouse with rolled axes 3D plot roll angle flake8, tests, and whats new Fix plot flipping at elev>270 Test for 3d plot with elev>270 Normalize the displayed angles for 3d plots when rotating with mouse Make elev, azim, roll ordering consistent everywhere Rename roll angle to tilt Speed up calculations for default tilt=0 Code review updates Switch tilt naming back to roll, flip rotation handedness Switch tilt naming back to roll, flip rotation handedness
21f1177
to
761f1ed
Compare
@jklymak Does that address everything you were worried about? |
Ooops sorry. I'll try and look tomorrow. Thanks for the ping. |
As a documentation idea: It would be cool to have an explanation of the degrees of freedom as animation:
@scottshambaugh since you make these awesome animations, would you be interested in doing this as a follow-up PR? |
Sure, that’s not hard to do. Make sense to put it in the gallery? |
Yes, please put it in 3d plotting. |
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, but lets take the opportunity to carefully define the angles. You dont' need to take my wording verbatim, but its frustrating to users to have to decode the handedness themselves.
Bump |
Looks great. Did you want to squash the two commits, us to do it, or leave them as-is? |
I don't have a preference, go ahead and do whatever the general convention is. |
OK, Squashed and merged.... Thanks a lot for working on this! |
Thanks for making it an easy process! |
PR Summary
This closes #14453 by adding a 3rd "roll" angle to ax.view_init() for 3D plots. The roll angle rotates the view about the viewing axis, as shown in the first plot. Rotation using the mouse on an interactive plot still only controls elevation and azimuth, meaning that this feature is only really relevant to users who want to create more complex camera angles programmatically. The default roll angle of 0 is backwards-compatible with existing 3D plots.
The rotation math comes from here, and works as expected.
Two caveats:
roll.mp4
roll2.mp4
PR Checklist
pytest
passes).flake8
on changed files to check).flake8-docstrings
and runflake8 --docstring-convention=all
).doc/users/next_whats_new/
(follow instructions in README.rst there).doc/api/next_api_changes/
(follow instructions in README.rst there).