Improve heuristics around 2D vs 3D space-view creation #3822

jleibs · 2023-10-11T23:20:11Z

What

Resolves:

Only 2D views show up once rr.Boxes2D is logged #3712 and probably more

Against my better judgement I ended up with one more pile of compelling hacks.
I believe it addresses many of the existing annoyances with awkwardly mixed 2d/3d views without (to my knowledge) causing regressions on the existing examples, though I haven't thoroughly tested everything yet, SFM, Human Motion, Arkit, etc. all look good.

The very high level idea is to prevent including 2D data in 3D views when it can't be projected via a pinhole, and then in a few other common edge-cases, more aggressively force 3D views.

This is done through 3 heuristic changes:

In order for a 2D part to ever be part of a 3D view, there must be a Pinhole above it in the hierarchy. Because this is something that can be inspected within the context of the tree without regard to any given space, we can evaluate this for each entity path as part of identify_entities_per_system_per_class.
- To make the information available to the filter, we introduce a new HeuristicFilterContext that allows the heuristic filter to observe which class it is being evaluated for as well as other information about the tree.
In the event that a 2d-space-view has a score of 0, we no longer spawn it. This should logically use 3d instead.
In the event that a 2d-space-view at the root has any entities that would rather be 3D don't spawn it. Logically this follows from the fact that the only applicable pinhole in such a space would be at the root itself, and 3d data would need to transform past the root to be projected.

Some examples of the improved behavior:

As reported in #3712

rr.init("test", spawn=True)
rr.log("image/box", rr.Boxes2D(array=[10, 10, 50, 50], array_format=rr.Box2DFormat.XYXY))
rr.log("image", rr.Image(np.random.rand(100, 100, 3)))
rr.log("points", rr.Points3D(np.random.rand(100, 3)))

Before:

After:

And another particularly bad case I found while exploring:

rr.init("test", spawn=True)
rr.log("box", rr.Boxes3D(half_sizes=[1, 1, 1]))
rr.log("image", rr.Image(np.random.rand(100, 100, 3)))
rr.log("points", rr.Points3D(np.random.rand(100, 3)))

Before:

After:

Checklist

I have read and agree to Contributor Guide and the Code of Conduct
I've included a screenshot or gif (if applicable)
I have tested demo.rerun.io (if applicable)
The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG

…ut of 3d spaces when they don't have a pinhole

teh-cmc

I'm no expert in this whole space-view-heuristic business (yet!) but this makes perfect sense to me.

crates/re_space_view_spatial/src/space_view_2d.rs

crates/re_viewer_context/src/space_view/view_part_system.rs

crates/re_viewport/src/space_view_heuristics.rs

emilk

I'm missing rationals for why we do things. That will be pretty important when we return to this code.

Let's not let that block the merging of this PR, but @jleibs please go back and add that later.

emilk · 2023-10-12T09:40:12Z

crates/re_space_view_spatial/src/parts/images.rs

+        // If this is a 3D view and there's no parent pinhole, do not include this part.
+        if ctx.class == "3D" && !ctx.has_ancestor_pinhole {
+            return false;
+        }


Why though?

I can already read the Rust code and see what it does, I want to know why it does it!

emilk · 2023-10-12T09:40:41Z

crates/re_space_view_spatial/src/parts/boxes2d.rs

+        // If this is a 3D view and there's no parent pinhole, do not include this part.
+        if ctx.class == "3D" && !ctx.has_ancestor_pinhole {
+            return false;
+        }


Again - why?

emilk · 2023-10-12T09:42:37Z

crates/re_space_view_spatial/src/parts/points2d.rs

+        // If this is a 3D view and there's no parent pinhole, do not include this part.
+        if ctx.class == "3D" && !ctx.has_ancestor_pinhole {
+            return false;
+        }


I've read this many times now

emilk · 2023-10-12T09:44:09Z

crates/re_space_view_spatial/src/space_view_2d.rs

+        // possible to correctly project 3d objects to a root 2d view since the
+        // the pinhole would go past the root.


I'm not sure I follow this. "go past the root"?

teh-cmc · 2023-10-12T09:58:00Z

Couldn't find a single heuristic issue with run_all.py.

### What Address some PR concerns from: - #3822 ### 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/3836) (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/3836) - [Docs preview](https://rerun.io/preview/9926b13d4f8bc925e37b506f46540058df211249/docs)  - [Examples preview](https://rerun.io/preview/9926b13d4f8bc925e37b506f46540058df211249/examples)  - [Recent benchmark results](https://ref.rerun.io/dev/bench/) - [Wasm size tracking](https://ref.rerun.io/dev/sizes/)

jleibs added 5 commits October 12, 2023 00:50

Add extra context to the heuristic_filter so we can filter 2d parts o…

8273ce8

…ut of 3d spaces when they don't have a pinhole

More opportunities not to spawn

396c2ca

Add the 2D filtering for points and lines

14ae2db

lint / comment

1696142

Move the score-modification/never-spawn just to space_view 2d

bae63aa

jleibs added the 😤 annoying Something in the UI / SDK is annoying to use label Oct 11, 2023

jleibs added this to the 0.9.1 milestone Oct 11, 2023

jleibs changed the title ~~More heuristic handling of edge-cases related to pinholes and 2D/3D view preferences~~ Improve heuristics around 2D vs 3D space-view creation Oct 11, 2023

jleibs added include in changelog 📺 re_viewer affects re_viewer itself labels Oct 11, 2023

jleibs marked this pull request as ready for review October 11, 2023 23:40

Merge branch 'main' into jleibs/ignore_2d_in_3d_without_pinhole

62966a3

teh-cmc self-requested a review October 12, 2023 07:14

teh-cmc approved these changes Oct 12, 2023

View reviewed changes

crates/re_space_view_spatial/src/space_view_2d.rs Outdated Show resolved Hide resolved

crates/re_viewer_context/src/space_view/view_part_system.rs Outdated Show resolved Hide resolved

crates/re_viewport/src/space_view_heuristics.rs Outdated Show resolved Hide resolved

address PR comments

680bf58

emilk requested changes Oct 12, 2023

View reviewed changes

emilk reviewed Oct 12, 2023

View reviewed changes

emilk approved these changes Oct 12, 2023

View reviewed changes

teh-cmc merged commit 541f34d into main Oct 12, 2023
30 checks passed

teh-cmc deleted the jleibs/ignore_2d_in_3d_without_pinhole branch October 12, 2023 09:58

teh-cmc mentioned this pull request Oct 12, 2023

Logging tensor first, then pinhole causes incorrect accumulated bounding box and thus broken default camera (eye) position #2869

Closed

jleibs added a commit that referenced this pull request Oct 12, 2023

Improve comments from #3822

5197178

jleibs mentioned this pull request Oct 12, 2023

Improve comments from #3822 #3836

Merged

4 tasks

This was referenced Oct 18, 2023

Only 2D views show up once rr.Boxes2D is logged #3712

Closed

Only show image in 3D space view if there's a pinhole camera #3092

Closed

Allow adding a 2D image to a 3D space view #3932

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve heuristics around 2D vs 3D space-view creation #3822

Improve heuristics around 2D vs 3D space-view creation #3822

jleibs commented Oct 11, 2023 •

edited by github-actions bot

Loading

teh-cmc left a comment

emilk left a comment •

edited

Loading

emilk Oct 12, 2023

emilk Oct 12, 2023

jleibs Oct 12, 2023

emilk Oct 12, 2023

emilk Oct 12, 2023

emilk Oct 12, 2023

teh-cmc commented Oct 12, 2023

		// possible to correctly project 3d objects to a root 2d view since the
		// the pinhole would go past the root.

Improve heuristics around 2D vs 3D space-view creation #3822

Improve heuristics around 2D vs 3D space-view creation #3822

Conversation

jleibs commented Oct 11, 2023 • edited by github-actions bot Loading

What

Checklist

teh-cmc left a comment

Choose a reason for hiding this comment

emilk left a comment • edited Loading

Choose a reason for hiding this comment

emilk Oct 12, 2023

Choose a reason for hiding this comment

emilk Oct 12, 2023

Choose a reason for hiding this comment

jleibs Oct 12, 2023

Choose a reason for hiding this comment

emilk Oct 12, 2023

Choose a reason for hiding this comment

emilk Oct 12, 2023

Choose a reason for hiding this comment

emilk Oct 12, 2023

Choose a reason for hiding this comment

teh-cmc commented Oct 12, 2023

jleibs commented Oct 11, 2023 •

edited by github-actions bot

Loading

emilk left a comment •

edited

Loading