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.

Sign up for GitHub

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

Jump to bottom

Point radius not correctly transformed during projection. #1219

Closed
jleibs opened this issue Feb 12, 2023 · 4 comments · Fixed by #4199
Closed

Point radius not correctly transformed during projection. #1219

jleibs opened this issue Feb 12, 2023 · 4 comments · Fixed by #4199
Labels
🪳 bug Something isn't working 🔺 re_renderer affects re_renderer itself

Comments

@jleibs
Copy link
Member

jleibs commented Feb 12, 2023 •
edited
Loading

Currently, when we project points from 3d -> 2d they continue to be rendered with a screenspace radius. Since most geometric spaces are in meters, but image-spaces are in pixels this makes it nearly impossible to have a single radius size that looks good in both.

I had set the point radius in colmap to 1cm and thought it had broken the projection. However when I turned off the image I noticed I did have super tiny projections that just weren't visible in the image.

These scenes mostly demonstrate the problem

  • Upper-left: 3d scene w/ points at 1cm
  • Upper-right: view from camera, which has proper projection since it's a true 3D scene
  • Lower-left: 2D view with projected points. Image hidden so you can see the point sizes.
  • Lower-right: The scene where I was hoping to see projected points

image

Ideally the lower right and upper right would have the same-sized points.
@jleibs jleibs added 🪳 bug Something isn't working 🔺 re_renderer affects re_renderer itself labels Feb 12, 2023
@Wumpf Wumpf mentioned this issue Feb 12, 2023
Closed
@Wumpf
Copy link
Member

Wumpf commented Feb 12, 2023 •
edited
Loading

The underlying issue is that under the hood the 2D view always uses an orthographic projection. To make projection of the points into that space possible, we apply a perspective projection onto the point's world_from_obj matrix. On top of that scaling in matrices doesn't affect point size, see #1223.

The orthographic limitation means that there is no perspective shrinking etc. in the 2D view which makes no intuitive sense when a user previously looks "through" the camera in 3D space which really should look just the same (see #1025)

I believe we need to take a small step back and make transform_cache again only deal with affine matrix (glam::Affine3A). In order to get the projection behavior in 2D views correct, we then know about the camera there being perspective and arrange our primitives accordingly! Meaning camera extrinsics are still dealt as before, but the 2D contents are placed to "infinite" distance and all 3D objects are rendered in front of them, just like they would be in the 3D view with "a very far away image plane".

As we progress in solving #1025 it is conceivable that we'd use different camera matrices during the rendering to the same target.

@teh-cmc teh-cmc mentioned this issue Mar 1, 2023
Open
@Wumpf Wumpf mentioned this issue Mar 27, 2023
Closed
4 tasks
@nikolausWest
Copy link
Member

Is this fixed?

@rangsjo
Copy link

rangsjo commented Sep 14, 2023 •
edited
Loading

I see this problem in version 0.8.2.
image

@nikolausWest
Copy link
Member

I see this problem in version 0.8.2.

Ouch, that looks super bad

This was referenced Nov 10, 2023
Closed
Merged
Wumpf added a commit that referenced this issue Nov 13, 2023
@Wumpf
Fix line & points (& depth clouds points) radii being unaffected by s…
6ee3c6a 
…cale & projection via Pinhole (#4199)

### What
Longstanding issue!

* Fixes #1223
* Fixes #1219
* Fixes #2494
* Replaces #4196

Shader only fix, in the future the scale factor shouldn't be extracted
on the fly for every vertex out of the transform and instead passed in,
but I wanted to keep the change minimal. The added vertex shading cost
is unlikely to matter all that much _short term_.
(also was very nice iterating on this and get before/after screenshots
;))

Throw-away test script for this:
```py
import numpy as np
import rerun as rr

rr.init("scale fix test!!!", spawn=True)

#############################
# #2494 & #1219
#############################
rr.log("world/camera", rr.ViewCoordinates.RDF, timeless=True)
rr.log(
    "world/camera/image",
    rr.Pinhole(
        image_from_camera=np.array([[500, 0, 250], [0, 500, 250], [0, 0, 1]]),
        width=500,
        height=500,
    ),
)
rr.log("world/camera/image/rgb", rr.Image(np.ones((500, 500, 3))))
rr.log(
    "world/camera/image/points",
    rr.Points2D(np.random.uniform(0, 500, (30, 2)), radii=1),
)

#############################
# #1223
#############################
rr.log(
    "scaling_stuff/points_unscaled",
    rr.Points3D(
        np.random.uniform(0, 1, (30, 3)),
        radii=0.1,
    ),
)
rr.log(
    "scaling_stuff/points_scaled",
    rr.Points3D(
        np.random.uniform(0, 1, (30, 3)),
        radii=0.1,
    ),
    rr.Transform3D(scale=2.0, translation=[2, 2, 2]),
)


rr.log(
    "scaling_stuff/lines_unscaled",
    rr.LineStrips3D([[0, 1, 0], [0, 1, 1], [0, 0, 3]], radii=0.1),
)
rr.log(
    "scaling_stuff/lines_scaled",
    rr.LineStrips3D([[0, 1, 0], [0, 1, 1], [0, 0, 3]], radii=0.1),
    rr.Transform3D(scale=2.0, translation=[2, 2, 2]),
)
```

Result:

Before:

![image](https://github.com/rerun-io/rerun/assets/1220815/29d8f98a-c2f9-4503-84c7-83fc7a8814ff)

After:

![image](https://github.com/rerun-io/rerun/assets/1220815/3d555ad9-790f-446f-a3c0-d72904db7a84)


This not only fixes issues with 2D->3D but also with 3D->2D.
Here we add the 3D points to the 2D camera and set a world space size
for the points:

Before:

![image](https://github.com/rerun-io/rerun/assets/1220815/038c6826-d6ee-4e00-93f4-2a964bc56abe)

After:

![image](https://github.com/rerun-io/rerun/assets/1220815/132ba0d6-9906-4f22-b4f6-c6ced82e3748)


### 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/4199) (if
applicable)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/4199)
- [Docs
preview](https://rerun.io/preview/7465b1650dd9be5c712c994827ced405c142edad/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/7465b1650dd9be5c712c994827ced405c142edad/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://ref.rerun.io/dev/bench/)
- [Wasm size tracking](https://ref.rerun.io/dev/sizes/)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
🪳 bug Something isn't working 🔺 re_renderer affects re_renderer itself
Projects
None yet
Milestone
No milestone
4 participants
@Wumpf @nikolausWest @jleibs @rangsjo