Skip to content

Commit

Permalink
More rigorious evaluation of visualizable entities in spatial views u…
Browse files Browse the repository at this point in the history
…sing new `SpatialTopology` store subscriber (#4836)

### What

* Important part of #4388

Introduces a `SpatialTopology` support structure which is built
incrementally using a store subscription.
It is geared towards replacing the existing `SpaceInfo` (todo!).

The first usecase for this is a more rigorous determination of
visualizable entities for spatial views: this now allows us to much more
accurately reason about what entities are visualizable and how to treat
corner cases (/invalid cases) like logged pinhole under a pinhole.

Since this makes the evaluation of visualizable entities / creation of
their context a bit more complex in some scenes, this comes with a bit
of a perf regression during heuristic eval. However, since the topology
changes rarely and deterministically, this also brings a big opportunity
for only evaluating the more complex context objects when needed and
even sharing them accross different space view (candidates). This is not
included in this pr since this comes with the open question of where to
store such a secondary cache (likely on topology) and how to flush it
(probably best for any new entity or new relevant component).


### 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 the web demo (if applicable):
* Using newly built examples:
[app.rerun.io](https://app.rerun.io/pr/4836/index.html)
* Using examples from latest `main` build:
[app.rerun.io](https://app.rerun.io/pr/4836/index.html?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[app.rerun.io](https://app.rerun.io/pr/4836/index.html?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [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/4836)
- [Docs
preview](https://rerun.io/preview/4e908b26a2323f145b06a7938a44cc178019d3bf/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/4e908b26a2323f145b06a7938a44cc178019d3bf/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

---------

Co-authored-by: Jeremy Leibs <[email protected]>
  • Loading branch information
Wumpf and jleibs authored Jan 18, 2024
1 parent a789eca commit 083a851
Show file tree
Hide file tree
Showing 14 changed files with 865 additions and 113 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions crates/re_space_view_spatial/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,14 @@ re_space_view.workspace = true

ahash.workspace = true
anyhow.workspace = true
bitflags.workspace = true
bytemuck.workspace = true
egui = { workspace = true, features = ["serde"] }
glam.workspace = true
itertools.workspace = true
macaw = { workspace = true, features = ["with_serde"] }
nohash-hasher.workspace = true
once_cell.workspace = true
parking_lot.workspace = true
rayon.workspace = true
serde.workspace = true
Expand Down
1 change: 1 addition & 0 deletions crates/re_space_view_spatial/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ mod scene_bounding_boxes;
mod space_camera_3d;
mod space_view_2d;
mod space_view_3d;
mod spatial_topology;
mod ui;
mod ui_2d;
mod ui_3d;
Expand Down
100 changes: 61 additions & 39 deletions crates/re_space_view_spatial/src/space_view_2d.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use re_entity_db::{EntityProperties, EntityTree};
use nohash_hasher::IntSet;
use re_entity_db::EntityProperties;
use re_log_types::EntityPath;
use re_types::{components::PinholeProjection, Loggable as _};
use re_viewer_context::{
AutoSpawnHeuristic, PerSystemEntities, SpaceViewClass, SpaceViewClassRegistryError,
SpaceViewId, SpaceViewSystemExecutionError, ViewQuery, ViewerContext,
Expand All @@ -10,18 +10,17 @@ use re_viewer_context::{
use crate::{
contexts::{register_spatial_contexts, PrimitiveCounter},
heuristics::{auto_spawn_heuristic, update_object_property_heuristics},
spatial_topology::{SpatialTopology, SubSpaceDimensionality},
ui::SpatialSpaceViewState,
view_kind::SpatialSpaceViewKind,
visualizers::{register_2d_spatial_visualizers, SpatialViewVisualizerData},
};

// TODO(#4388): 2D/3D relationships should be solved via a "SpatialTopology" construct.
#[derive(Default)]
pub struct VisualizableFilterContext2D {
/// True if there's a pinhole camera at the origin, meaning we can display 3d content.
pub has_pinhole_at_origin: bool,

/// All subtrees that are invalid since they're behind a pinhole that's not at the origin.
pub invalid_subtrees: Vec<EntityPath>,
// TODO(andreas): Would be nice to use `EntityPathHash` in order to avoid bumping reference counters.
pub entities_in_main_2d_space: IntSet<EntityPath>,
pub reprojectable_3d_entities: IntSet<EntityPath>,
}

impl VisualizableFilterContext for VisualizableFilterContext2D {
Expand Down Expand Up @@ -51,6 +50,9 @@ impl SpaceViewClass for SpatialSpaceView2D {
&self,
system_registry: &mut re_viewer_context::SpaceViewSystemRegistrator<'_>,
) -> Result<(), SpaceViewClassRegistryError> {
// Ensure spatial topology is registered.
crate::spatial_topology::SpatialTopologyStoreSubscriber::subscription_handle();

register_spatial_contexts(system_registry)?;
register_2d_spatial_visualizers(system_registry)?;

Expand All @@ -73,41 +75,61 @@ impl SpaceViewClass for SpatialSpaceView2D {
) -> Box<dyn VisualizableFilterContext> {
re_tracing::profile_function!();

// See also `SpatialSpaceView3D::visualizable_filter_context`

let origin_tree = entity_db.tree().subtree(space_origin);

let has_pinhole_at_origin = origin_tree.map_or(false, |tree| {
tree.entity
.components
.contains_key(&PinholeProjection::name())
});

fn visit_children_recursively(tree: &EntityTree, invalid_subtrees: &mut Vec<EntityPath>) {
if tree
.entity
.components
.contains_key(&PinholeProjection::name())
{
invalid_subtrees.push(tree.path.clone());
} else {
for child in tree.children.values() {
visit_children_recursively(child, invalid_subtrees);
// TODO(andreas): The `VisualizableFilterContext` depends entirely on the spatial topology.
// If the topology hasn't changed, we don't need to recompute any of this.
// Also, we arrive at the same `VisualizableFilterContext` for lots of different origins!

let context = SpatialTopology::access(entity_db.store_id(), |topo| {
let primary_space = topo.subspace_for_entity(space_origin);
match primary_space.dimensionality {
SubSpaceDimensionality::Unknown => VisualizableFilterContext2D {
entities_in_main_2d_space: primary_space.entities.clone(),
reprojectable_3d_entities: Default::default(),
},

SubSpaceDimensionality::TwoD => {
// All entities in the 2d space are visualizable + the parent space if it is connected via a pinhole.
// For the moment we don't allow going down pinholes again.
let reprojected_3d_entities = primary_space
.parent_space
.and_then(|parent_space_origin| {
let is_connected_pinhole = topo
.subspace_for_subspace_origin(parent_space_origin)
.and_then(|parent_space| {
parent_space
.child_spaces
.get(&primary_space.origin)
.map(|connection| connection.is_connected_pinhole())
})
.unwrap_or(false);

if is_connected_pinhole {
topo.subspace_for_subspace_origin(parent_space_origin)
.map(|parent_space| parent_space.entities.clone())
} else {
None
}
})
.unwrap_or_default();

VisualizableFilterContext2D {
entities_in_main_2d_space: primary_space.entities.clone(),
reprojectable_3d_entities: reprojected_3d_entities,
}
}
}
}

let mut invalid_subtrees = Vec::new();
if let Some(origin_tree) = origin_tree {
for child_tree in origin_tree.children.values() {
visit_children_recursively(child_tree, &mut invalid_subtrees);
SubSpaceDimensionality::ThreeD => {
// If this is 3D space, only display the origin entity itself.
// Everything else we have to assume requires some form of transformation.
VisualizableFilterContext2D {
entities_in_main_2d_space: std::iter::once(space_origin.clone()).collect(),
reprojectable_3d_entities: Default::default(),
}
}
}
};
});

Box::new(VisualizableFilterContext2D {
has_pinhole_at_origin,
invalid_subtrees,
})
Box::new(context.unwrap_or_default())
}

fn on_frame_start(
Expand Down
101 changes: 57 additions & 44 deletions crates/re_space_view_spatial/src/space_view_3d.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use nohash_hasher::IntSet;
use re_entity_db::{EntityProperties, EntityTree};
use re_log_types::{EntityPath, EntityPathHash};
use re_types::{components::PinholeProjection, Loggable as _};
use re_entity_db::EntityProperties;
use re_log_types::EntityPath;
use re_viewer_context::{
AutoSpawnHeuristic, IdentifiedViewSystem as _, PerSystemEntities, SpaceViewClass,
SpaceViewClassRegistryError, SpaceViewId, SpaceViewSystemExecutionError, ViewQuery,
Expand All @@ -11,16 +10,17 @@ use re_viewer_context::{
use crate::{
contexts::{register_spatial_contexts, PrimitiveCounter},
heuristics::{auto_spawn_heuristic, update_object_property_heuristics},
spatial_topology::{SpatialTopology, SubSpaceDimensionality},
ui::SpatialSpaceViewState,
view_kind::SpatialSpaceViewKind,
visualizers::{register_3d_spatial_visualizers, CamerasVisualizer},
};

// TODO(andreas): This context is used to determine whether a 2D entity has a valid transform
// and is thus visualizable. This should be expanded to cover any invalid transform as non-visualizable.
#[derive(Default)]
pub struct VisualizableFilterContext3D {
/// Set of all entities that are under a pinhole camera.
pub entities_under_pinhole: IntSet<EntityPathHash>,
// TODO(andreas): Would be nice to use `EntityPathHash` in order to avoid bumping reference counters.
pub entities_in_main_3d_space: IntSet<EntityPath>,
pub entities_under_pinholes: IntSet<EntityPath>,
}

impl VisualizableFilterContext for VisualizableFilterContext3D {
Expand Down Expand Up @@ -50,6 +50,9 @@ impl SpaceViewClass for SpatialSpaceView3D {
&self,
system_registry: &mut re_viewer_context::SpaceViewSystemRegistrator<'_>,
) -> Result<(), SpaceViewClassRegistryError> {
// Ensure spatial topology is registered.
crate::spatial_topology::SpatialTopologyStoreSubscriber::subscription_handle();

register_spatial_contexts(system_registry)?;
register_3d_spatial_visualizers(system_registry)?;

Expand All @@ -71,46 +74,56 @@ impl SpaceViewClass for SpatialSpaceView3D {
) -> Box<dyn VisualizableFilterContext> {
re_tracing::profile_function!();

// TODO(andreas): Potential optimization:
// We already know all the entities that have a pinhole camera - indirectly today through visualizers, but could also be directly.
// Meaning we don't need to walk until we find a pinhole camera!
// Obviously should skip the whole thing if there are no pinhole cameras under space_origin!
// TODO(jleibs): Component prefix tree on EntityTree would fix this problem nicely.

let mut entities_under_pinhole = IntSet::default();

fn visit_children_recursively(
tree: &EntityTree,
entities_under_pinhole: &mut IntSet<EntityPathHash>,
) {
if tree
.entity
.components
.contains_key(&PinholeProjection::name())
{
// This and all children under it are under a pinhole camera!
tree.visit_children_recursively(&mut |ent_path| {
entities_under_pinhole.insert(ent_path.hash());
});
} else {
for child in tree.children.values() {
visit_children_recursively(child, entities_under_pinhole);
// TODO(andreas): The `VisualizableFilterContext` depends entirely on the spatial topology.
// If the topology hasn't changed, we don't need to recompute any of this.
// Also, we arrive at the same `VisualizableFilterContext` for lots of different origins!

let context = SpatialTopology::access(entity_db.store_id(), |topo| {
let primary_space = topo.subspace_for_entity(space_origin);
match primary_space.dimensionality {
SubSpaceDimensionality::Unknown => VisualizableFilterContext3D {
entities_in_main_3d_space: primary_space.entities.clone(),
entities_under_pinholes: Default::default(),
},

SubSpaceDimensionality::ThreeD => {
// All entities in the 3d space are visualizable + everything under pinholes.
let mut entities_in_main_3d_space = primary_space.entities.clone();
let mut entities_under_pinholes = IntSet::<EntityPath>::default();

for (child_origin, connection) in &primary_space.child_spaces {
if connection.is_connected_pinhole() {
let Some(child_space) =
topo.subspace_for_subspace_origin(child_origin.hash())
else {
// Should never happen, implies that a child space is not in the list of subspaces.
continue;
};

// Entities _at_ pinholes are a special case: we display both 3d and 2d visualizers for them.
entities_in_main_3d_space.insert(child_space.origin.clone());
entities_under_pinholes.extend(child_space.entities.iter().cloned());
}
}

VisualizableFilterContext3D {
entities_in_main_3d_space,
entities_under_pinholes,
}
}
}
}

let entity_tree = &entity_db.tree();

// Walk down the tree from the space_origin.
// Note that if there's no subtree, we still have to return a `VisualizerFilterContext3D` to
// indicate to all visualizable-filters that we're in a 3D space view.
if let Some(current_tree) = &entity_tree.subtree(space_origin) {
visit_children_recursively(current_tree, &mut entities_under_pinhole);
};
SubSpaceDimensionality::TwoD => {
// If this is 2D space, only display the origin entity itself.
// Everything else we have to assume requires some form of transformation.
VisualizableFilterContext3D {
entities_in_main_3d_space: std::iter::once(space_origin.clone()).collect(),
entities_under_pinholes: Default::default(),
}
}
}
});

Box::new(VisualizableFilterContext3D {
entities_under_pinhole,
})
Box::new(context.unwrap_or_default())
}

fn auto_spawn_heuristic(
Expand Down
Loading

0 comments on commit 083a851

Please sign in to comment.