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

ViewCoordinates are not working properly for camera frustums #2244

Closed
emilk opened this issue May 29, 2023 · 12 comments · Fixed by #2614
Closed

ViewCoordinates are not working properly for camera frustums #2244

emilk opened this issue May 29, 2023 · 12 comments · Fixed by #2614
Assignees
Labels
🪳 bug Something isn't working user-request This is a pressing issue for one of our users

Comments

@emilk
Copy link
Member

emilk commented May 29, 2023

A user reported that FLU doesn't work as expected, resulting in camera frustums pointing the wrong way in 3D space.

This is for setting the view coordinates of e.g. world/camera (NOT world/camera/image, which is covered by #1387)

@emilk emilk added the 🪳 bug Something isn't working label May 29, 2023
@emilk emilk changed the title Our camera frustums only support RUB? ViewCoordinates are not working properly for camera frustums May 29, 2023
abey79 added a commit that referenced this issue Jun 26, 2023
@emilk emilk added the user-request This is a pressing issue for one of our users label Jun 26, 2023
@abey79
Copy link
Member

abey79 commented Jun 27, 2023

Goal is to make #2512 work with nice and intuitive code :)

@jleibs
Copy link
Member

jleibs commented Jun 28, 2023

Can we capture more details here regarding what the user was trying to do, what the expected behavior is, and what the actual behavior is with a minimal repro? Very hard to assess whether #2553 actually fixes the behavior without this context.

@jleibs
Copy link
Member

jleibs commented Jun 28, 2023

Something like this:

import rerun as rr
import numpy as np

rr.init("coord", spawn=True)

img = np.zeros((100, 100, 3))
img[49:50, 10:50] = [255, 0, 0]
img[40:60, 10:12] = [255, 0, 0]
img[10:50, 49:50] = [0, 0, 255]
img[10:12, 40:60] = [0, 0, 255]

rr.log_image("world/image", img)
rr.log_pinhole("world/image", child_from_parent=[[100, 0, 50], [0, 100, 50], [0, 0, 1]], width=100, height=100)
rr.log_view_coordinates("world", xyz="FLU")

Looks totally fine to me. What's the issue?

emilk pushed a commit that referenced this issue Jun 29, 2023
emilk added a commit that referenced this issue Jul 2, 2023
…2512)

### What

Add a simple example to display Open Photogrammetry Format datasets


![](https://static.rerun.io/3bb25c43fa2a4c367d036c27943812ebfe3e4d42_open_photogrammetry_format_1200w.png)

This example is currently minimalist in that there is lots more in the
OPF that could be displayed, such as uncalibrated vs. calibrated
cameras, matches between points and cameras, etc. Also, I opted to
display each calibrated camera as individual frames in the timeline, as
displaying them currently spams the viewer with image views.

Closes #2246

Would greatly benefit from #1136
Blocked by #2244 

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)

<!-- This line will get updated when the PR build summary job finishes.
-->
PR Build Summary: https://build.rerun.io/pr/2512

<!-- pr-link-docs:start -->
Docs preview: https://rerun.io/preview/c1fd3e3/docs
Examples preview: https://rerun.io/preview/c1fd3e3/examples
<!-- pr-link-docs:end -->

---------

Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>
@emilk
Copy link
Member Author

emilk commented Jul 3, 2023

Something like this:

Looks totally fine to me. What's the issue?

The issue is that the forward of the camera is not along X, but along Z. xyz=FLY has no effect on anything:
image

@emilk
Copy link
Member Author

emilk commented Jul 3, 2023

We have a user that is using a quadcopter to capture images, and the quadcopter uses FLU coordinates, i.e. X is the forward axis, Y is left, and Z is up. The camera has a known resolution and focal length, but no explicit pinhole projection matrix.

It makes sense to expect Rerun to easily support this use case in a way that looking at the camera in Rerun, the frustum is along the X+ axis in the cameras local coordinate system.

There is a strong argument that having anything except Z+ or Z- as the forwards axis is mathematically wrong when you have a standard pinhole matrix where Z is always the projected axis. Correct mathematics matters because we want to be able to help users understand when they are doing something wrong. However, our user above doesn't really want to set a specific pinhole matrix, just point a camera and set a focal length (field-of-view). Currently we don't support setting just the focal length of a pinhole camera, but we should (as an alternative to setting the full 3x3 matrix). I think this is actually the problem: that we force users to set a 3x3 pinhole matrix with Z=forward, when some users don't want Z forward.

I suggest therefore that we merge #2553 - I think it is the best solution proposed so far, especially if we then proceed to work on supporting pinhole transforms without an explicit 3x3 matrix.

We can perhaps come up with better solutions in the future, but I think any solution would need to be able to handle this use case of wanting to see a camera frustum pointing along the local X+ axis. If we say that only Z can be forward, then we need to create helpers for transforming between coordinate systems, and come up with a UI for that. We also need to add some of warning or error if the user sets view coordinates that doesn't not conform to that. I think this is a much bigger design task, and not worth it right now.

@jleibs
Copy link
Member

jleibs commented Jul 3, 2023

I still pretty strongly disagree that this is a good idea and anticipate a new number of confused users exceeding the number of users that you think that PR will help. But, if you insist, then at a minimum please update the relevant documentation for view coordinates to indicate that it is no longer just adding semantics but will in fact add hidden transforms into the transform chain.

@jleibs
Copy link
Member

jleibs commented Jul 3, 2023

xyz=FLU has no effect on anything:

Not true, xyz=FLU in that example correctly sets the world coordinates to FLU and does change the default scene layout and eye-camera behavior.

There was no world-to-camera transform logged, so I expect my world-to-camera transform to be Identity. As such, I would expect the camera projection axis (Z) to be pointed up, which is exactly what it's doing. Plenty of roboticists will agree with me on this being totally expected behavior for that example snippet.

This is perhaps more clear using FRD:

rr.log_image("world/image", img)
rr.log_pinhole("world/image", child_from_parent=[[100, 0, 50], [0, 100, 50], [0, 0, 1]], width=100, height=100)
rr.log_view_coordinates("world", xyz="FRD")

In this case I would now expect the optical axis of the camera (Z) to point down under the identity transform, and again, this behaves as expected.

image

@jleibs jleibs closed this as completed Jul 3, 2023
@jleibs
Copy link
Member

jleibs commented Jul 3, 2023

Not sure how I closed this 😕 bad click on comment?

@emilk
Copy link
Member Author

emilk commented Jul 3, 2023

Your example is quite confusing though, since you hang a pinhole transform in world space, i.e. you have no camera extrinsics.

I 100% agree that the world coordinate system should not affect the camera view coordinates.

This is a more realistic test:

rr.log_image("world/camera/image", img)
rr.log_pinhole("world/camera/image", child_from_parent=[[100, 0, 50], [0, 100, 50], [0, 0, 1]], width=100, height=100)
rr.log_view_coordinates("world/camera", xyz="FLU")

I created a new issue:

Once resolved, this could then become:

rr.log_image("world/camera/image", img)
rr.log_pinhole("world/camera/image", focal_length_px=50, width=100, height=100)
rr.log_view_coordinates("world/camera", xyz="FLU")

There would here be no hidden transform or any confusion. The user states that they want X to be Forward, and so when we visualize the camera frustum we point it along X.

@emilk
Copy link
Member Author

emilk commented Jul 3, 2023

Since we allow putting the extrinsics (Transform) and intrinsics (Pinhole) on the same entity, we should probably use the ViewCoordinates of the pinhole entity to decide the direction in which to point the camera frustum (in contradiction with what I said earlier in this thread).

We should also consider (in the future) to split ViewCoordinates into multiple components doing different things:

  • Setting 3D up-axis, and maybe compass directions (X=East)
  • Setting the camera view coordinates (frustum orientation)
  • Setting the 2D view coordinates of an image (XY=RU, XY=UL etc)

This should significantly reduce the risk of misunderstanding and mistakes

@jleibs
Copy link
Member

jleibs commented Jul 3, 2023

There would here be no hidden transform or any confusion

(1) Which axes do image-x and image-y map to? Do we negate X when mapping to L? Do we negate Y when mapping to U? What if we specify UFL? Is U always "camera-y"? Plenty of ambiguity and room for confusion.

(2) First I add this to my code because I don't have a calibration and I want to "just see things work". Now I run almost any camera calibration code and it provides me with camera exstrinsics which almost certainly describe a rigid transform between world and the standard "z-forward" image. I log these extrinsics:

rr.log_transform3d("world/camera", my_exstrinsics)

But now my camera is pointing the wrong direction (because yes, there is a hidden transform). How do I make sense of what's happening? How do I know that I actually have to remove view coordinates to use these exstrinsics? What's the correct way to adjust my exstrinsics so the right thing happens when I specify view coordinates?

@emilk
Copy link
Member Author

emilk commented Jul 3, 2023

I'm starting to come down to this as a (short-term) plan:

log_pinhole(…) is given an optional xyz argument for orienting the camera frustum. This is equivalent to setting the view-coordinates for the same entity. For instance: rr.log_pinhole("world/camera/image", focal_length=50, width=100, height=200, xyz="RUB"). This is now the preferred way of setting the frustum orientation. That way the coordinate system of the parent (which could be world) does not affect it at all.

Yes, there is still confusion to be found if a users sets something other than RUB or RDF (e.g. X in image is Z in view-space), but such confusion would be explicitly self-inflicted, and often not a problem (e.g. the common FLU for drones, where you just want to show an image in front of the drone).

In the long run we try to split the view coordinates into different components as outlined in #2244 (comment)

@emilk emilk assigned emilk and unassigned Wumpf Jul 6, 2023
emilk added a commit that referenced this issue Jul 7, 2023
Closes  #2244, closes #2589
*  #2244
* #2589


### What
`log_pinhole` is now much easier to use. You only need to set focal
length and optional principal point instead of setting the full 3x3
matrix.

There is also a new argument to it `camera_xyz` for setting the
coordinate system. The default is RDF (the old default). This affects
the visible camera frustum, how rays are projected when hovering a 2D
image, and how depth clouds are projected.

There is also a bunch of bug fixes along the way, including an accurate
camera frustum which works for non-centered principal points.

This simplifies some examples, especially
`examples/python/open_photogrammetry_format/main.py` which uses RUB.

### Tests
There is a new test: `tests/python/view_coordinates/main.py` creates a
bunch of pinholes with RGB and Depth.


![image](https://github.com/rerun-io/rerun/assets/1148717/f08eb77b-cab1-4557-9b4c-b4afa45069d2)

I've also run 

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested [demo.rerun.io](https://demo.rerun.io/pr/2614) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/2614)
- [Docs
preview](https://rerun.io/preview/pr%3Aemilk%2Fimprove-pinhole/docs)
- [Examples
preview](https://rerun.io/preview/pr%3Aemilk%2Fimprove-pinhole/examples)

---------

Co-authored-by: Andreas Reich <r_andreas2@web.de>
Co-authored-by: Jeremy Leibs <jeremy@rerun.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🪳 bug Something isn't working user-request This is a pressing issue for one of our users
Projects
None yet
4 participants