Skip to content

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

Merged
merged 2 commits into from
Dec 3, 2021

Conversation

scottshambaugh
Copy link
Contributor

@scottshambaugh scottshambaugh commented Oct 22, 2021

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:

  1. As the elevation angle increases, the azimuth and roll angles become increasingly coupled. You can see this a bit in the second animation as a slowing/speeding of the simultaneous rotation about all 3 angles. This is unavoidable if we are to keep azimuth and elevation as our backwards-compatible rotation axes.
  2. With elevation angles of +-90, there is some jittering when incrementing the roll angle. This numeric instability at the two poles is unavoidable with a rotation matrix-based implementation. This could both be fixed by using quaternions rather than rotation matrices, but as numpy does not implement quaternions it's not worth introducing a dependency just for this purpose.
roll.mp4
roll2.mp4

PR Checklist

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • Conforms to Matplotlib style conventions (install flake8-docstrings and run flake8 --docstring-convention=all).
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • [n/a] API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

@scottshambaugh scottshambaugh marked this pull request as draft October 22, 2021 06:36
@scottshambaugh
Copy link
Contributor Author

scottshambaugh commented Oct 22, 2021

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)

Copy link

@github-actions github-actions bot left a 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.

@scottshambaugh scottshambaugh marked this pull request as ready for review October 22, 2021 18:02
@story645 story645 added this to the v3.6.0 milestone Oct 22, 2021
@story645 story645 added the status: needs comment/discussion needs consensus on next step label Oct 22, 2021
@scottshambaugh
Copy link
Contributor Author

scottshambaugh commented Oct 25, 2021

Note that I believe the inversion at about 6.5 seconds into the second video is due to the issue addressed by PR #10762

@scottshambaugh
Copy link
Contributor Author

scottshambaugh commented Oct 25, 2021

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

@story645 story645 added the status: needs workflow approval For PRs from new contributors, from which GitHub blocks workflows by default. label Oct 25, 2021
@scottshambaugh scottshambaugh marked this pull request as draft October 25, 2021 17:04
@scottshambaugh scottshambaugh marked this pull request as ready for review October 26, 2021 01:27
@timhoffm
Copy link
Member

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)?

@scottshambaugh
Copy link
Contributor Author

scottshambaugh commented Oct 26, 2021

Sure thing!

  1. In short, azimuth then elevation rotations are first applied, and then roll.
    In long, the elevation and azimuth first define a location for the "eye" looking at the scene. The primary view is then formed by looking from the eye down the vector towards the center of the viewing volume "R". This vector "v" is given an orthogonal vector "u" that aligns with the vertical_axis, and a vector orthogonal to both "w". u and w define the "up" and "right" directions for the viewport. The current code stops there. What I do is then simply rotate u and w in the viewing plane clockwise by the roll angle so that the end result is only a rotation of the viewport. You can dig into this in the lib/mpl_toolkits/mplot3d/proj3d.py:view_transformation() function.
  2. 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. You won't be able to control roll by clicking and dragging.

@scottshambaugh
Copy link
Contributor Author

scottshambaugh commented Oct 28, 2021

@jklymak
That's a good optimization, I can add it in this evening!

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.

image

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.

image
image

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.

image

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:
Standard rotation matrices about each of the XYZ axes are applied like so: R(+X)(roll) * R(+Y)(pitch) * R(+Z)(yaw)
We have to choose between: R(+X)(tilt) * R(-Y)(elevation) * R(+Z)(azimuth) or R(-X)(tilt) * R(-Y)(elevation) * R(+Z)(azimuth). I would recommend the former.

@scottshambaugh scottshambaugh changed the title Add ability to roll the camera in 3D plots Add ability to tilt the camera in 3D plots Oct 28, 2021
@jklymak
Copy link
Member

jklymak commented Oct 29, 2021

@scottshambaugh yes, Euler angles always hurt the head!

a) agree that we can't break our current meaning of elev and azimuth.
b) agree that it looks like "roll" going last is pretty typical

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:

@scottshambaugh
Copy link
Contributor Author

scottshambaugh commented Oct 30, 2021

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: R(-X)(tilt) * R(-Y)(elevation) * R(+Z)(azimuth)

@scottshambaugh scottshambaugh marked this pull request as ready for review October 30, 2021 23:20
@timhoffm timhoffm removed the status: needs workflow approval For PRs from new contributors, from which GitHub blocks workflows by default. label Oct 31, 2021
@scottshambaugh scottshambaugh force-pushed the 3d-plot-roll branch 2 times, most recently from 4278447 to 21f1177 Compare November 16, 2021 00:37
@scottshambaugh
Copy link
Contributor Author

scottshambaugh commented Nov 16, 2021

Ok, switched it back to roll and flipped the handedness to match with pyvista. #21508 is updated as well. Ready to rock and roll.

I'm not sure how to fix the PR Cleanliness failing check.

roll.mp4
roll2.mp4

@jklymak
Copy link
Member

jklymak commented Nov 16, 2021

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
@scottshambaugh
Copy link
Contributor Author

@jklymak Does that address everything you were worried about?

@jklymak
Copy link
Member

jklymak commented Nov 23, 2021

Ooops sorry. I'll try and look tomorrow. Thanks for the ping.

@jklymak jklymak added status: needs review and removed status: needs comment/discussion needs consensus on next step labels Nov 23, 2021
@timhoffm
Copy link
Member

As a documentation idea: It would be cool to have an explanation of the degrees of freedom as animation:

  1. Start with a certain view.
  2. Move +/- azimuth and while doing so, show a title/label "azimuth"
  3. Move +/- elevation and while doing so, show a title/label "elevation"
  4. Move +/- roll and while doing so, show a title/label "elevation"

@scottshambaugh since you make these awesome animations, would you be interested in doing this as a follow-up PR?

@scottshambaugh
Copy link
Contributor Author

Sure, that’s not hard to do. Make sense to put it in the gallery?

@timhoffm
Copy link
Member

Yes, please put it in 3d plotting.

Copy link
Member

@jklymak jklymak left a 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.

@scottshambaugh
Copy link
Contributor Author

Bump

@jklymak
Copy link
Member

jklymak commented Dec 3, 2021

Looks great. Did you want to squash the two commits, us to do it, or leave them as-is?

@scottshambaugh
Copy link
Contributor Author

I don't have a preference, go ahead and do whatever the general convention is.

@jklymak jklymak merged commit 30f1c94 into matplotlib:main Dec 3, 2021
@jklymak
Copy link
Member

jklymak commented Dec 3, 2021

OK, Squashed and merged.... Thanks a lot for working on this!

@scottshambaugh
Copy link
Contributor Author

Thanks for making it an easy process!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add third angle to view_init()
4 participants