Skip to content

Commit

Permalink
Less restrictive visualizability constraints of 2d entitites, improve…
Browse files Browse the repository at this point in the history
…d space view generation heuristics (#5188)

### What

* Fixes #4926
* it does not address child-of-root heuristics for time series, but they
weren't as much of a concern
* Fixes heuristic issues observed in
rerun-io/cpp-example-opencv-eigen#20

Two changes in here:
* we no longer determine the dimensionality of a topological subspace,
instead we only determine whether it can't display 2d/3d
   * 2d: we now state we can _always_ display 2d objects
* 3d: we can display it unless the parent of the active subspace is a
pinhole in which case we only make the entity _at_ the origin as
visualizable
* make `ViewCoordinates` add "heuristic hints" to spatial topology
* this is all that's needed to get rid of child-of-root heuristics in 3d
without regressing existing examples
   
OpenCV example that didn't work before:

![image](https://github.com/rerun-io/rerun/assets/1220815/571ad089-98cd-4295-ba5e-3b81c39c93c2)

New release check:

![image](https://github.com/rerun-io/rerun/assets/1220815/d4d577e1-7fc1-41e5-90b4-6764c9db2f0d)



### 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/5188/index.html)
* Using examples from latest `main` build:
[app.rerun.io](https://app.rerun.io/pr/5188/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/5188/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
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!

- [PR Build Summary](https://build.rerun.io/pr/5188)
- [Docs
preview](https://rerun.io/preview/cc3735c02cd092df5843f7fe97bfa998db2e459d/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/cc3735c02cd092df5843f7fe97bfa998db2e459d/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
  • Loading branch information
Wumpf committed Feb 21, 2024
1 parent f6bc63b commit 91da2b9
Show file tree
Hide file tree
Showing 6 changed files with 336 additions and 371 deletions.
70 changes: 1 addition & 69 deletions crates/re_space_view_spatial/src/heuristics.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
use std::collections::BTreeSet;

use ahash::HashMap;
use egui::NumExt as _;
use nohash_hasher::IntSet;

use re_data_ui::image_meaning_for_entity;
use re_entity_db::EditableAutoValue;
use re_log_types::{EntityPath, EntityPathPart};
use re_log_types::EntityPath;
use re_types::{
components::{DepthMeter, TensorData},
tensor_data::TensorDataMeaning,
Expand All @@ -18,7 +17,6 @@ use re_viewer_context::{

use crate::{
query_pinhole,
spatial_topology::{SpatialTopology, SubSpace, SubSpaceDimensionality},
view_kind::SpatialSpaceViewKind,
visualizers::{
CamerasVisualizer, ImageVisualizer, SpatialViewVisualizerData, Transform3DArrowsVisualizer,
Expand Down Expand Up @@ -254,69 +252,3 @@ pub fn default_visualized_entities_for_visualizer_kind(
.cloned()
.collect()
}

/// Splits the root space into subspaces under certain circumstances.
///
/// TODO(#4926): This seems to be unnecessarily complicated.
/// #4926 describes the rationale and how we might be able to remove this.
pub fn root_space_split_heuristic(
topo: &SpatialTopology,
relevant_entities: &IntSet<EntityPath>,
dimensionality: SubSpaceDimensionality,
) -> HashMap<EntityPathPart, SubSpace> {
re_tracing::profile_function!();

// We want to split the root space if…
//
// … there's a root space in the first place.
let Some(root_space) = topo.subspace_for_subspace_origin(EntityPath::root().hash()) else {
return Default::default();
};
// … and that root space doesn't have a different dimensionality.
if root_space.dimensionality != dimensionality
&& root_space.dimensionality != SubSpaceDimensionality::Unknown
{
return Default::default();
}
// … the root space is not empty.
if root_space.entities.is_empty() {
return Default::default();
}
// … nothing relevant logged directly at the root.
if relevant_entities.contains(&EntityPath::root()) {
return Default::default();
}

let mut interesting_children_of_root_spaces = HashMap::<EntityPathPart, SubSpace>::default();
for entity in &root_space.entities {
let Some(root_child) = entity.iter().next() else {
continue;
};
interesting_children_of_root_spaces
.entry(root_child.clone())
.or_insert_with(|| {
let root_child_path: EntityPath = [root_child.clone()].as_slice().into();
SubSpace {
origin: root_child_path.clone(),
dimensionality,
entities: Default::default(), // Filled as we go.
child_spaces: root_space
.child_spaces
.iter()
.filter_map(|(child, connection)| {
if child.is_descendant_of(&root_child_path) {
Some((child.clone(), *connection))
} else {
None
}
})
.collect(),
parent_space: Some(EntityPath::root().hash()),
}
})
.entities
.insert(entity.clone());
}

interesting_children_of_root_spaces
}
86 changes: 33 additions & 53 deletions crates/re_space_view_spatial/src/space_view_2d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::{
default_visualized_entities_for_visualizer_kind, update_object_property_heuristics,
},
max_image_dimension_subscriber::{ImageDimensions, MaxImageDimensions},
spatial_topology::{SpatialTopology, SubSpaceDimensionality},
spatial_topology::{SpatialTopology, SubSpaceConnectionFlags},
ui::SpatialSpaceViewState,
view_kind::SpatialSpaceViewKind,
visualizers::register_2d_spatial_visualizers,
Expand Down Expand Up @@ -90,51 +90,29 @@ impl SpaceViewClass for SpatialSpaceView2D {

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(),
if !primary_space.supports_2d_content() {
// If this is strict 3D space, only display the origin entity itself.
// Everything else we have to assume requires some form of transformation.
return VisualizableFilterContext2D {
entities_in_main_2d_space: std::iter::once(space_origin.clone()).collect(),
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,
}
}

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(),
}
}
};
}

// All 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 reprojectable_3d_entities =
if primary_space.connection_to_parent.is_connected_pinhole() {
topo.subspace_for_subspace_origin(primary_space.parent_space)
.map(|parent_space| parent_space.entities.clone())
.unwrap_or_default()
} else {
Default::default()
};

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

Expand Down Expand Up @@ -182,7 +160,7 @@ impl SpaceViewClass for SpatialSpaceView2D {
recommended_space_views: topo
.iter_subspaces()
.flat_map(|subspace| {
if subspace.dimensionality == SubSpaceDimensionality::ThreeD
if !subspace.supports_2d_content()
|| subspace.entities.is_empty()
|| indicated_entities.is_disjoint(&subspace.entities)
{
Expand All @@ -197,14 +175,16 @@ impl SpaceViewClass for SpatialSpaceView2D {
.cloned()
.collect();

// For explicit 2D spaces (typically indicated by a pinhole or similar) start at the origin, otherwise start at the common ancestor.
// For explicit 2D spaces with a pinhole at the origin, otherwise start at the common ancestor.
// This generally avoids the `/` root entity unless it's required as a common ancestor.
let recommended_root =
if subspace.dimensionality == SubSpaceDimensionality::TwoD {
subspace.origin.clone()
} else {
EntityPath::common_ancestor_of(relevant_entities.iter())
};
let recommended_root = if subspace
.connection_to_parent
.contains(SubSpaceConnectionFlags::Pinhole)
{
subspace.origin.clone()
} else {
EntityPath::common_ancestor_of(relevant_entities.iter())
};

let mut recommended_space_views = Vec::<RecommendedSpaceView>::new();

Expand Down
136 changes: 65 additions & 71 deletions crates/re_space_view_spatial/src/space_view_3d.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use itertools::Itertools;
use nohash_hasher::IntSet;
use re_entity_db::EntityProperties;
use re_log_types::{EntityPath, EntityPathFilter};
Expand All @@ -11,10 +12,9 @@ use re_viewer_context::{
use crate::{
contexts::{register_spatial_contexts, PrimitiveCounter},
heuristics::{
default_visualized_entities_for_visualizer_kind, root_space_split_heuristic,
update_object_property_heuristics,
default_visualized_entities_for_visualizer_kind, update_object_property_heuristics,
},
spatial_topology::{SpatialTopology, SubSpaceDimensionality},
spatial_topology::{HeuristicHints, SpatialTopology, SubSpaceConnectionFlags},
ui::SpatialSpaceViewState,
view_kind::SpatialSpaceViewKind,
visualizers::register_3d_spatial_visualizers,
Expand Down Expand Up @@ -84,46 +84,40 @@ impl SpaceViewClass for SpatialSpaceView3D {

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(),
if !primary_space.supports_3d_content() {
// If this is strict 2D space, only display the origin entity itself.
// Everything else we have to assume requires some form of transformation.
return VisualizableFilterContext3D {
entities_in_main_3d_space: std::iter::once(space_origin.clone()).collect(),
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,
}
// 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 in &primary_space.child_spaces {
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;
};

if child_space
.connection_to_parent
.contains(SubSpaceConnectionFlags::Pinhole)
{
// Note that for this the connection to the parent is allowed to contain the disconnected flag.
// 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());
}
}

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(),
}
}
VisualizableFilterContext3D {
entities_in_main_3d_space,
entities_under_pinholes,
}
});

Expand Down Expand Up @@ -161,40 +155,40 @@ impl SpaceViewClass for SpatialSpaceView3D {
// Spawn a space view at each subspace that has any potential 3D content.
// Note that visualizability filtering is all about being in the right subspace,
// so we don't need to call the visualizers' filter functions here.
SpatialTopology::access(ctx.entity_db.store_id(), |topo| {
let split_root_spaces = root_space_split_heuristic(
topo,
&indicated_entities,
SubSpaceDimensionality::ThreeD,
);

SpaceViewSpawnHeuristics {
recommended_space_views: topo
.iter_subspaces()
.flat_map(|subspace| {
if subspace.origin.is_root() && !split_root_spaces.is_empty() {
itertools::Either::Left(split_root_spaces.values())
} else {
itertools::Either::Right(std::iter::once(subspace))
}
})
.filter_map(|subspace| {
if subspace.dimensionality == SubSpaceDimensionality::TwoD
|| subspace.entities.is_empty()
|| indicated_entities.is_disjoint(&subspace.entities)
SpatialTopology::access(ctx.entity_db.store_id(), |topo| SpaceViewSpawnHeuristics {
recommended_space_views: topo
.iter_subspaces()
.filter_map(|subspace| {
if !subspace.supports_3d_content() || subspace.entities.is_empty() {
None
} else {
// Creates space views at each view coordinates if there's any.
// (yes, we do so even if they're empty at the moment!)
let mut roots = subspace
.heuristic_hints
.iter()
.filter(|(_, hint)| hint.contains(HeuristicHints::ViewCoordinates3d))
.map(|(root, _)| root.clone())
.collect::<Vec<_>>();

// If there's no view coordinates or there are still some entities not covered,
// create a view at the subspace origin.
if !roots.iter().contains(&subspace.origin)
&& indicated_entities
.intersection(&subspace.entities)
.any(|e| roots.iter().all(|root| !e.starts_with(root)))
{
None
} else {
Some(RecommendedSpaceView {
root: subspace.origin.clone(),
query_filter: EntityPathFilter::subtree_entity_filter(
&subspace.origin,
),
})
roots.push(subspace.origin.clone());
}
})
.collect(),
}

Some(roots.into_iter().map(|root| RecommendedSpaceView {
query_filter: EntityPathFilter::subtree_entity_filter(&root),
root,
}))
}
})
.flatten()
.collect(),
})
.unwrap_or_default()
}
Expand Down
Loading

0 comments on commit 91da2b9

Please sign in to comment.