From 2f23c527a497e19f598bcf934767a7b5f5f43f27 Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Thu, 12 Oct 2023 09:56:40 -0400 Subject: [PATCH] Improve comments from #3822 (#3836) ### 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/) --- crates/re_space_view_spatial/src/parts/boxes2d.rs | 5 ++++- crates/re_space_view_spatial/src/parts/images.rs | 5 ++++- crates/re_space_view_spatial/src/parts/lines2d.rs | 5 ++++- crates/re_space_view_spatial/src/parts/points2d.rs | 5 ++++- crates/re_space_view_spatial/src/space_view_2d.rs | 13 ++++++++++--- 5 files changed, 26 insertions(+), 7 deletions(-) diff --git a/crates/re_space_view_spatial/src/parts/boxes2d.rs b/crates/re_space_view_spatial/src/parts/boxes2d.rs index 2161fde29d4a..557fdc9990c5 100644 --- a/crates/re_space_view_spatial/src/parts/boxes2d.rs +++ b/crates/re_space_view_spatial/src/parts/boxes2d.rs @@ -190,7 +190,10 @@ impl ViewPartSystem for Boxes2DPart { return false; } - // If this is a 3D view and there's no parent pinhole, do not include this part. + // 2D parts can only ever be rendered properly as part of a 3D scene if the + // transform graph includes a pinhole projection. Pinholes only map from 2D children + // to 3D parents, so if the required pinhole exists, it must be an ancestor. + // Filter them otherwise to avoid ending up with 2D content mixed into 3D scenes. if ctx.class == "3D" && !ctx.has_ancestor_pinhole { return false; } diff --git a/crates/re_space_view_spatial/src/parts/images.rs b/crates/re_space_view_spatial/src/parts/images.rs index c064402fdb8a..2150a5b075de 100644 --- a/crates/re_space_view_spatial/src/parts/images.rs +++ b/crates/re_space_view_spatial/src/parts/images.rs @@ -689,7 +689,10 @@ impl ViewPartSystem for ImagesPart { return false; } - // If this is a 3D view and there's no parent pinhole, do not include this part. + // 2D parts can only ever be rendered properly as part of a 3D scene if the + // transform graph includes a pinhole projection. Pinholes only map from 2D children + // to 3D parents, so if the required pinhole exists, it must be an ancestor. + // Filter them otherwise to avoid ending up with 2D content mixed into 3D scenes. if ctx.class == "3D" && !ctx.has_ancestor_pinhole { return false; } diff --git a/crates/re_space_view_spatial/src/parts/lines2d.rs b/crates/re_space_view_spatial/src/parts/lines2d.rs index 6fefbd9daab0..62904633c900 100644 --- a/crates/re_space_view_spatial/src/parts/lines2d.rs +++ b/crates/re_space_view_spatial/src/parts/lines2d.rs @@ -185,7 +185,10 @@ impl ViewPartSystem for Lines2DPart { return false; } - // If this is a 3D view and there's no parent pinhole, do not include this part. + // 2D parts can only ever be rendered properly as part of a 3D scene if the + // transform graph includes a pinhole projection. Pinholes only map from 2D children + // to 3D parents, so if the required pinhole exists, it must be an ancestor. + // Filter them otherwise to avoid ending up with 2D content mixed into 3D scenes. if ctx.class == "3D" && !ctx.has_ancestor_pinhole { return false; } diff --git a/crates/re_space_view_spatial/src/parts/points2d.rs b/crates/re_space_view_spatial/src/parts/points2d.rs index 5af11472299a..a53a81432cb4 100644 --- a/crates/re_space_view_spatial/src/parts/points2d.rs +++ b/crates/re_space_view_spatial/src/parts/points2d.rs @@ -212,7 +212,10 @@ impl ViewPartSystem for Points2DPart { return false; } - // If this is a 3D view and there's no parent pinhole, do not include this part. + // 2D parts can only ever be rendered properly as part of a 3D scene if the + // transform graph includes a pinhole projection. Pinholes only map from 2D children + // to 3D parents, so if the required pinhole exists, it must be an ancestor. + // Filter them otherwise to avoid ending up with 2D content mixed into 3D scenes. if ctx.class == "3D" && !ctx.has_ancestor_pinhole { return false; } diff --git a/crates/re_space_view_spatial/src/space_view_2d.rs b/crates/re_space_view_spatial/src/space_view_2d.rs index a3ad347accee..43e4d4f2145a 100644 --- a/crates/re_space_view_spatial/src/space_view_2d.rs +++ b/crates/re_space_view_spatial/src/space_view_2d.rs @@ -80,9 +80,16 @@ impl SpaceViewClass for SpatialSpaceView2D { ); // If this is the root space view, and it contains a part that would - // prefer to be 3D, don't spawn the 2D view. This is because it's never - // possible to correctly project 3d objects to a root 2d view since the - // the pinhole would go past the root. + // prefer to be 3D, don't spawn the 2D view. + // + // Since pinhole projections provide a mapping between a 2D child space and a 3D + // parent space, it means that for any 3D content to to be projected into a 2D space, + // there must exist a common 3D ancestor-space which has a *child* which is a 2D space + // (and contains a pinhole.) But if this space origin is at the root itself, that common + // ancestor would need to be a parent of the root, which doesn't exist. As such, it's + // impossible that this space would be able to correctly account for the required + // content. By not spawning a 2D space in this case we ensure the 3D version gets chosen + // by the heuristics instead. if space_origin.is_root() { let parts = ctx .space_view_class_registry