Skip to content

Commit

Permalink
Improve comments from #3822 (#3836)
Browse files Browse the repository at this point in the history
### 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)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/9926b13d4f8bc925e37b506f46540058df211249/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://ref.rerun.io/dev/bench/)
- [Wasm size tracking](https://ref.rerun.io/dev/sizes/)
  • Loading branch information
jleibs authored Oct 12, 2023
1 parent 6819943 commit 2f23c52
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 7 deletions.
5 changes: 4 additions & 1 deletion crates/re_space_view_spatial/src/parts/boxes2d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
5 changes: 4 additions & 1 deletion crates/re_space_view_spatial/src/parts/images.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
5 changes: 4 additions & 1 deletion crates/re_space_view_spatial/src/parts/lines2d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
5 changes: 4 additions & 1 deletion crates/re_space_view_spatial/src/parts/points2d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
13 changes: 10 additions & 3 deletions crates/re_space_view_spatial/src/space_view_2d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 2f23c52

Please sign in to comment.