-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Fix displayed 3d coordinates showing gibberish #23485
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
I don't understand how such a number can ever have meaning unless you arbitrarily assign a surface to project onto. That seems of super limited value to me and I think we should just not show a cursor coordinates in 3D. |
The surfaces we are projecting onto here are not so arbitrary - they’re the “corner of a box” formed by the three axis panes which are visible in every 3D plot that is created. We are already drawing gridlines on them, which on its own shows the value of knowing the values of points on those panes and means they should be easy to interpret. At the very least, they’re unambiguous. I see two use cases which these enable and which I personally would find extremely useful:
|
I'm still not sure how you will communicate to the user that the cursor is running along the backing planes? I guess I agree that doing that is better than having a random output, but it's still going to be a bit mysterious. |
I think it should be pretty discoverable by waving the mouse around? One coordinate is always fixed on each plane, and the transitions when crossing the primary axes are pretty noticeable. Lemme fix the Z coordinate issue and y'all can play around with it and see if it makes sense (I'll take a video too). Open to suggestions on documentation that would make this explicit. |
This is going to take some more effort than I thought, getting the right z coordinate is not a simple fix and will require writing up proper inverse projections. I've got to do some learning on that, so leaving open for now and will try to get back to it when I have time. |
Before further work goes into this, I would like to state that I'm also not convinced of this. The planes themselves are not very much of interest for the user. Data locations would be. While one can rotate and pan so that coordinates of a desired data point can be inferred from the displayed pane coordinates, this is an ugly hack at best:
In my opinion, providing 3D coordinates for a 2D cursor position without additional context* is fundamentally flawed, and we should not do this (In the face of ambiguity, refuse the temptation to guess). *The only reasonable way for returning 3D coordinates is some additional visual feedback and guidance. Something like a 3D snapping crosshair (i.e. the 3D version of https://matplotlib.org/stable/gallery/event_handling/cursor_demo.html#snapping-to-data-points) would work. Unfortunately, I'm afraid, Matplotlib is not well designed for this as you would have to redraw the image when moving the crosshair, which is quite costly and thus would not be very responsive. |
I'd support removing the coordinates entirely right now since they're worse than useless at the moment, but I still see a lot of utility in having us show the backing plane coordinates. Let me see if I can get it working and then I can demo it which I think will show that it's not so unintuitive. |
Fair. To set expectations right: As I see currently see it, this should not be a default behavior in Matplotlib. It might be an example, or a third-party package. |
5401d09
to
ed62274
Compare
ed62274
to
a0b665d
Compare
2d6640d
to
86e5f04
Compare
86e5f04
to
2e81b3a
Compare
This is working now for all projection types and is ready for review / discussion. I think the below video demonstrates the behavior well, as well as the utility of this. When looking at data with an ortho view along one of the primary view planes, the behavior is exactly the same as 2D plots. This is super common in my use cases at least, it was the motivation behind adding documentation for the primary view planes here, here, and here, and would become more useful with the addition of this feature. Otherwise the main utility I see is if a user projects an image onto one of the backing panes and wants to read off coordinates from that, as is shown in these gallery examples: 1, 2, 3. It's also a nice quick way to grab the axis limits visually. It would be nice to have lines along the backing planes to show the cursor location, somewhat analogous to what plotly does, but as @timhoffm points out above the redraw would be prohibitively costly. But I don't think you need this for the behavior to be discoverable - IMO the "fixed" coordinate as you move around on one of the backing panes makes what is happening obvious fairly quickly. As a side note on the issue of documentation / discoverability, I don't think there is any info in the docs showing the figure GUI and how to use it, unless I'm missing it. I think it would make most sense to put an annotated image of the GUI in here. Descriptions of what the coordinates mean could go there. I'll open a docs issue for that separately (edit: #25266). 2023-02-20.21-18-04-1.mp4The alternative is removing these hover coordinates altogether. If that end up as the consensus then I'll open another MR with that change (I think there's some good refactoring that can carry over from here). Will bring up in the dev call this week. |
f2cd740
to
4bd8626
Compare
4bd8626
to
00a4683
Compare
I update the video above to better show some use cases. |
In the dev call today the consensus was that this is useful and we should do it, but we should explicitly tell the user in the UI that these are coordinates from the axis panes to avoid confusion. I changed it to the below as a maximally compact way of conveying this information, but am open to revising this if it seems unclear. if pane_idx == 0:
coords = f'x pane={xs}, y={ys}, z={zs}'
elif pane_idx == 1:
coords = f'x={xs}, y pane={ys}, z={zs}'
elif pane_idx == 2:
coords = f'x={xs}, y={ys}, z pane={zs}' 2023-02-23.18-56-02-1.mp4 |
6745c1b
to
650440d
Compare
650440d
to
11517b5
Compare
This is good to go btw. |
11517b5
to
5ec04ae
Compare
d766069
to
f928ba9
Compare
…ing view pane Get 3d ortho coordinates working Get non-1 focal lengths working Docs
f928ba9
to
9701a39
Compare
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 agree lines on the pane would be nice, but then they should also be optional.
I'm squashing your commits... Thanks for your patience! |
PR Summary
Closes #22775 and closes #4334. Also gets rid of the annoying helper function at cause in #23430. Some previous discussion in #22624.
As the first issue shows, the displayed coordinates under the mouse cursor are currently meaningless. The calculation in this PR shows the x, y, z point on the displayed plane at the mouse location. For example, ignoring the broken Z coordinate for now, the mouse here is at the circled location and the coordinates displayed are on the x-z plane (the y coordinate is fixed at any point on this plane).

TODO:
self.vvec
withself._view_w
, need to switch that over depending on which is merged first. The signs are flipped between those two variables but we multiply twice so it cancels out and can be directly replaced.PR Checklist
Tests and Styling
pytest
passes).flake8-docstrings
and runflake8 --docstring-convention=all
).Documentation
doc/users/next_whats_new/
(follow instructions in README.rst there).doc/api/next_api_changes/
(follow instructions in README.rst there).