From 91bee819d7c26aa1ef40fe9cc3e37a180c771304 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler <49431240+abey79@users.noreply.github.com> Date: Thu, 18 Jan 2024 08:57:35 +0100 Subject: [PATCH 1/4] Bump h2 to 0.3.24 (#4850) ### What Fixes cargo deny error ### 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/4850/index.html) * Using examples from latest `main` build: [app.rerun.io](https://app.rerun.io/pr/4850/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/4850/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/4850) - [Docs preview](https://rerun.io/preview/492bad328c02e40f8d2d9610db77eb03939ea36e/docs) - [Examples preview](https://rerun.io/preview/492bad328c02e40f8d2d9610db77eb03939ea36e/examples) - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html) --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f5c66cbb3c42..1811f9a53198 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2511,9 +2511,9 @@ dependencies = [ [[package]] name = "h2" -version = "0.3.22" +version = "0.3.24" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4d6250322ef6e60f93f9a2162799302cd6f68f79f6e5d85c8c16f14d1d958178" +checksum = "bb2c4422095b67ee78da96fbb51a4cc413b3b25883c7717ff7ca1ab31022c9c9" dependencies = [ "bytes", "fnv", From a789ecac5cb8c4ab6267ba888181bb35b0fbf244 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 18 Jan 2024 10:56:52 +0100 Subject: [PATCH 2/4] Update changelog for 0.12.1 (#4862) ### What ### 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/4862/index.html) * Using examples from latest `main` build: [app.rerun.io](https://app.rerun.io/pr/4862/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/4862/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/4862) - [Docs preview](https://rerun.io/preview/bdc20b661c8fb947adfdd1947f3521c4aa9c31d7/docs) - [Examples preview](https://rerun.io/preview/bdc20b661c8fb947adfdd1947f3521c4aa9c31d7/examples) - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html) --- CHANGELOG.md | 32 +++++++++++++++++++++++++++++--- RELEASES.md | 4 ++-- docs/cspell.json | 1 + 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 576e93fcf7cf..2de4507fe046 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,30 @@ ## [Unreleased](https://github.com/rerun-io/rerun/compare/latest...HEAD) -## [0.12.0](https://github.com/rerun-io/rerun/compare/0.11.0...0.12.0) + +## [0.12.1](https://github.com/rerun-io/rerun/compare/0.12.0...0.12.1) - 2024-01-17 - Data loader bug fixes + +#### 🌊 C++ API +- Fix CMake trying to pick up test folders outside of the Rerun project/zip [#4770](https://github.com/rerun-io/rerun/pull/4770) (thanks [@KevinGliewe](https://github.com/KevinGliewe)!) +- Document that `Mat3x3` and `Mat4x4` constructors are column major [#4842](https://github.com/rerun-io/rerun/pull/4842) + +#### 🦀 Rust API +- Fix `entity_path_vec!` and `entity_path!` depending on `ToString` being in scope [#4766](https://github.com/rerun-io/rerun/pull/4766) (thanks [@kpreid](https://github.com/kpreid)!) + +#### 🪳 Bug Fixes +- Fix external data loader plugins on Windows [#4840](https://github.com/rerun-io/rerun/pull/4840) +- Reduce latency when loading data from external loaders [#4797](https://github.com/rerun-io/rerun/pull/4797) +- Always point to versioned manifest when building a versioned binary [#4781](https://github.com/rerun-io/rerun/pull/4781) + +#### 🧑‍💻 Dev-experience +- External loaders: remove warnings on duplicated binary on `$PATH` [#4833](https://github.com/rerun-io/rerun/pull/4833) + +#### 🤷‍♂️ Other +- Include `Cargo.lock` in `rerun-cli` crate [#4750](https://github.com/rerun-io/rerun/pull/4750) +- Replace `atty` dependency with `std::io::IsTerminal` [#4790](https://github.com/rerun-io/rerun/pull/4790) (thanks [@kpreid](https://github.com/kpreid)!) + + +## [0.12.0](https://github.com/rerun-io/rerun/compare/0.11.0...0.12.0) - Data Loaders, Container-editing, Python-3.12 - 2024-01-09 ### Overview & Highlights - 🌁 The Rerun Viewer now supports a plugin system for creating [arbitrary external data loaders](https://www.rerun.io/docs/howto/open-any-file). @@ -142,7 +165,8 @@ - File-like entity paths [#4476](https://github.com/rerun-io/rerun/pull/4476) - Make the new container blueprints the default behavior [#4642](https://github.com/rerun-io/rerun/pull/4642) -## [0.11.0](https://github.com/rerun-io/rerun/compare/0.10.1...0.11.0) + +## [0.11.0](https://github.com/rerun-io/rerun/compare/0.10.1...0.11.0) - C++ improvements & better Visible History - 2023-11-28 ### Overview & Highlights @@ -226,7 +250,8 @@ Special thanks to @dvad & @dangush for contributing! - `DataStore` introduce `StoreEvent`s [#4203](https://github.com/rerun-io/rerun/pull/4203) - `DataStore` introduce `StoreView`s [#4205](https://github.com/rerun-io/rerun/pull/4205) -## [0.10.1](https://github.com/rerun-io/rerun/compare/0.10.0...0.10.1) + +## [0.10.1](https://github.com/rerun-io/rerun/compare/0.10.0...0.10.1) - 2023-11-02 ### Overview & Highlights This is a small release primarily to tie up some loose ends for our C++ SDK. @@ -241,6 +266,7 @@ This is a small release primarily to tie up some loose ends for our C++ SDK. - C++ Windows CI [#4110](https://github.com/rerun-io/rerun/pull/4110) - Add MacOS C++ CI, add Linux C++20 CI [#4120](https://github.com/rerun-io/rerun/pull/4120) + ## [0.10.0](https://github.com/rerun-io/rerun/compare/0.9.1...0.10.0) - C++ SDK - 2023-10-30 [Rerun](https://www.rerun.io/) is an easy-to-use visualization toolbox for computer vision and robotics. diff --git a/RELEASES.md b/RELEASES.md index 6d93e2aa9661..ea45e1884991 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -37,8 +37,8 @@ We tag all data files (`.rrd` files) and communication protocols with the rerun ## Releases -Release builds of the Python Wheels are triggered by pushing a release tag to GitHub in the form `v0.2.0`. -If we are doing a patch release, we do a branch off of the latest release tag (e.g. `v0.3.0`) and cherry-pick any fixes we want into that branch. +Release builds of the Python Wheels are triggered by pushing a release tag to GitHub in the form `0.2.0`. +If we are doing a patch release, we do a branch off of the latest release tag (e.g. `0.3.0`) and cherry-pick any fixes we want into that branch. # Release process diff --git a/docs/cspell.json b/docs/cspell.json index ab8f62f67141..8288ae0c1460 100644 --- a/docs/cspell.json +++ b/docs/cspell.json @@ -42,6 +42,7 @@ "Georgios", "Girshick", "Gkioxari", + "Gliewe", "Gokay", "Gustafson", "Hanzi", From 083a8518091ae463b00318089131c57c7a76a6cb Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Thu, 18 Jan 2024 12:19:26 +0100 Subject: [PATCH 3/4] More rigorious evaluation of visualizable entities in spatial views using 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) - [Examples preview](https://rerun.io/preview/4e908b26a2323f145b06a7938a44cc178019d3bf/examples) - [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 --- Cargo.lock | 2 + crates/re_space_view_spatial/Cargo.toml | 2 + crates/re_space_view_spatial/src/lib.rs | 1 + .../src/space_view_2d.rs | 100 ++- .../src/space_view_3d.rs | 101 +-- .../src/spatial_topology.rs | 702 ++++++++++++++++++ .../src/visualizers/arrows3d.rs | 1 + .../src/visualizers/assets3d.rs | 1 + .../src/visualizers/boxes3d.rs | 1 + .../src/visualizers/cameras.rs | 1 + .../src/visualizers/lines3d.rs | 1 + .../src/visualizers/meshes.rs | 1 + .../src/visualizers/mod.rs | 63 +- .../src/visualizers/points3d.rs | 1 + 14 files changed, 865 insertions(+), 113 deletions(-) create mode 100644 crates/re_space_view_spatial/src/spatial_topology.rs diff --git a/Cargo.lock b/Cargo.lock index 1811f9a53198..336780852773 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5034,6 +5034,7 @@ version = "0.13.0-alpha.2" dependencies = [ "ahash", "anyhow", + "bitflags 2.4.1", "bytemuck", "criterion", "egui", @@ -5042,6 +5043,7 @@ dependencies = [ "macaw", "mimalloc", "nohash-hasher", + "once_cell", "parking_lot 0.12.1", "rayon", "re_data_store", diff --git a/crates/re_space_view_spatial/Cargo.toml b/crates/re_space_view_spatial/Cargo.toml index 61b9d6a62091..ab06a6071133 100644 --- a/crates/re_space_view_spatial/Cargo.toml +++ b/crates/re_space_view_spatial/Cargo.toml @@ -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 diff --git a/crates/re_space_view_spatial/src/lib.rs b/crates/re_space_view_spatial/src/lib.rs index a09b0846412d..33edb33a2b85 100644 --- a/crates/re_space_view_spatial/src/lib.rs +++ b/crates/re_space_view_spatial/src/lib.rs @@ -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; 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 1808497de5aa..d442fc6a7f49 100644 --- a/crates/re_space_view_spatial/src/space_view_2d.rs +++ b/crates/re_space_view_spatial/src/space_view_2d.rs @@ -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, @@ -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, + // TODO(andreas): Would be nice to use `EntityPathHash` in order to avoid bumping reference counters. + pub entities_in_main_2d_space: IntSet, + pub reprojectable_3d_entities: IntSet, } impl VisualizableFilterContext for VisualizableFilterContext2D { @@ -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)?; @@ -73,41 +75,61 @@ impl SpaceViewClass for SpatialSpaceView2D { ) -> Box { 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) { - 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( diff --git a/crates/re_space_view_spatial/src/space_view_3d.rs b/crates/re_space_view_spatial/src/space_view_3d.rs index 92ff8f0294e4..6c287e9d06b2 100644 --- a/crates/re_space_view_spatial/src/space_view_3d.rs +++ b/crates/re_space_view_spatial/src/space_view_3d.rs @@ -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, @@ -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, + // TODO(andreas): Would be nice to use `EntityPathHash` in order to avoid bumping reference counters. + pub entities_in_main_3d_space: IntSet, + pub entities_under_pinholes: IntSet, } impl VisualizableFilterContext for VisualizableFilterContext3D { @@ -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)?; @@ -71,46 +74,56 @@ impl SpaceViewClass for SpatialSpaceView3D { ) -> Box { 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, - ) { - 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::::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( diff --git a/crates/re_space_view_spatial/src/spatial_topology.rs b/crates/re_space_view_spatial/src/spatial_topology.rs new file mode 100644 index 000000000000..0e70a2a04782 --- /dev/null +++ b/crates/re_space_view_spatial/src/spatial_topology.rs @@ -0,0 +1,702 @@ +use once_cell::sync::OnceCell; + +use ahash::HashMap; +use nohash_hasher::{IntMap, IntSet}; +use re_data_store::{StoreSubscriber, StoreSubscriberHandle}; +use re_log_types::{EntityPath, EntityPathHash, StoreId}; +use re_types::{ + components::{DisconnectedSpace, PinholeProjection}, + Loggable, +}; + +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub enum SubSpaceDimensionality { + /// We don't know if this space is a 2D or 3D space. + /// + /// This is the most common case and happens whenever there's no projection that + /// establishes a clear distinction between 2D and 3D spaces. + /// + /// Note that this can both mean "there are both 2D and 3D relationships within the space" + /// as well as "there are not spatial relationships at all within the space". + Unknown, + + /// The space is definitively a 2D space. + /// + /// This conclusion is usually reached by the presence of a projection operation. + TwoD, + + /// The space is definitively a 3D space. + /// + /// This conclusion is usually reached by the presence of a projection operation. + ThreeD, +} + +bitflags::bitflags! { + #[derive(PartialEq, Eq, Debug, Copy, Clone)] + pub struct SubSpaceConnectionFlags: u8 { + const Disconnected = 0b0000001; + const Pinhole = 0b0000010; + } +} + +impl SubSpaceConnectionFlags { + /// Pinhole flag but not disconnected + #[inline] + pub fn is_connected_pinhole(&self) -> bool { + self.contains(SubSpaceConnectionFlags::Pinhole) + && !self.contains(SubSpaceConnectionFlags::Disconnected) + } +} + +/// Spatial subspace within we typically expect a homogeneous dimensionality without any projections & disconnects. +/// +/// Subspaces are separated by projections or explicit disconnects. +/// +/// A subspace may contain internal transforms, but any such transforms must be invertible such +/// that all data can be represented regardless of choice of origin. +/// +/// Within the tree of all subspaces, every entity is contained in exactly one subspace. +/// The subtree at (and including) the `origin` minus the +/// subtrees of all child spaces are considered to be contained in a subspace. +pub struct SubSpace { + /// The transform root of this subspace. + /// + /// This is also used to uniquely identify the space. + pub origin: EntityPath, + + pub dimensionality: SubSpaceDimensionality, + + /// All entities that were logged at any point in time and are part of this subspace. + /// + /// Contains the origin entity as well, unless the origin is the `EntityPath::root()` and nothing was logged to it directly. + /// + /// Note that we this is merely here to speed up queries. + /// Instead, we could check if an entity is equal to or a descendent of the + /// origin and not equal or descendent of any child space. + /// The problem with that is that it's common for a 3d space to have many 2d spaces as children, + /// which would make this an expensive query. + pub entities: IntSet, + + /// Origin paths of child spaces and how they're connected. + /// + /// This implies that there is a either an explicit disconnect or + /// a projection at the origin of the child space. + /// How it is connected is implied by `parent_space` in the child space. + /// + /// Any path in `child_spaces` is *not* contained in `entities`. + /// This implies that the camera itself is not part of its 3D space even when it may still have a 3D transform. + pub child_spaces: IntMap, + + /// Origin of the parent space if any. + pub parent_space: Option, + // + // TODO(andreas): + // We could (and should) add here additional transform hierarchy information within this space. + // This would be useful in order to speed up determining the transforms for a given frame. +} + +impl SubSpace { + /// Updates dimensionality based on a new connection to a child space. + fn add_or_update_child_connection( + &mut self, + child_path: &EntityPath, + new_connections_to_child: SubSpaceConnectionFlags, + ) { + if new_connections_to_child.contains(SubSpaceConnectionFlags::Pinhole) { + match self.dimensionality { + SubSpaceDimensionality::Unknown => { + self.dimensionality = SubSpaceDimensionality::ThreeD; + } + SubSpaceDimensionality::TwoD => { + // For the moment the only way to get a 2D space is by defining a pinhole, + // but in the future other projections may also cause a space to be defined as 2D space. + // TODO(#3849, #4301): We should be able to tag the source entity as having an invalid transform so we can display a permanent warning in the ui. + re_log::warn_once!("There was already a pinhole logged at {:?}. +The new pinhole at {:?} is nested under it, implying an invalid projection from a 2D space to a 2D space.", self.origin, child_path); + // We keep 2D. + } + SubSpaceDimensionality::ThreeD => { + // Already 3D. + } + } + } + + self.child_spaces + .entry(child_path.clone()) + .or_insert(new_connections_to_child) // insert into child spaces in the first place + .insert(new_connections_to_child); // insert into connection flags + } +} + +#[derive(Default)] +pub struct SpatialTopologyStoreSubscriber { + topologies: HashMap, +} + +impl SpatialTopologyStoreSubscriber { + /// Accesses the global store subscriber. + /// + /// Lazily registers the subscriber if it hasn't been registered yet. + pub fn subscription_handle() -> StoreSubscriberHandle { + static SUBSCRIPTION: OnceCell = OnceCell::new(); + *SUBSCRIPTION.get_or_init(|| { + re_data_store::DataStore::register_subscriber( + Box::::default(), + ) + }) + } +} + +impl StoreSubscriber for SpatialTopologyStoreSubscriber { + fn name(&self) -> String { + "SpatialTopologyStoreSubscriber".to_owned() + } + + fn as_any(&self) -> &dyn std::any::Any { + self + } + + fn as_any_mut(&mut self) -> &mut dyn std::any::Any { + self + } + + fn on_events(&mut self, events: &[re_data_store::StoreEvent]) { + re_tracing::profile_function!(); + + for event in events { + if event.diff.kind != re_data_store::StoreDiffKind::Addition { + // Topology is only additive, don't care about removals. + continue; + } + + // Possible optimization: + // only update topologies if an entity is logged the first time or a new relevant component was added. + self.topologies + .entry(event.store_id.clone()) + .or_default() + .on_store_diff(&event.diff.entity_path, event.diff.cells.keys()); + } + } +} + +/// Spatial toopological information about a store. +/// +/// Describes how 2D & 3D spaces are connected/disconnected. +/// +/// Used to determine whether 2D/3D visualizers are applicable and to inform +/// space view generation heuristics. +/// +/// Spatial topology is time independent but may change as new data comes in. +/// Generally, the assumption is that topological cuts stay constant over time. +pub struct SpatialTopology { + /// All subspaces, identified by their origin-hash. + subspaces: IntMap, + + /// Maps each logged entity to the origin of a subspace. + /// + /// This is purely an optimization to speed up searching for `subspaces`. + subspace_origin_per_logged_entity: IntMap, +} + +impl Default for SpatialTopology { + fn default() -> Self { + Self { + subspaces: std::iter::once(( + EntityPath::root().hash(), + SubSpace { + origin: EntityPath::root(), + dimensionality: SubSpaceDimensionality::Unknown, + entities: IntSet::default(), // Note that this doesn't contain the root entity. + child_spaces: IntMap::default(), + parent_space: None, + }, + )) + .collect(), + + subspace_origin_per_logged_entity: Default::default(), + } + } +} + +impl SpatialTopology { + /// Accesses the spatial topology for a given store. + pub fn access(store_id: &StoreId, f: impl FnOnce(&SpatialTopology) -> T) -> Option { + re_data_store::DataStore::with_subscriber_once( + SpatialTopologyStoreSubscriber::subscription_handle(), + move |topology_subscriber: &SpatialTopologyStoreSubscriber| { + topology_subscriber.topologies.get(store_id).map(f) + }, + ) + .flatten() + } + + /// Returns the subspace an entity belongs to. + #[inline] + pub fn subspace_for_entity(&self, entity: &EntityPath) -> &SubSpace { + self.subspaces + .get(&self.subspace_origin_hash_for_entity(entity)) + .expect("unknown subspace origin, `SpatialTopology` is in an invalid state") + } + + fn subspace_origin_hash_for_entity(&self, entity: &EntityPath) -> EntityPathHash { + let mut entity_reference = entity; + let mut entity_storage: EntityPath; // Only needed if we actually have to walk up the tree. Unused on the happy path. + + loop { + // It's enough to check in`self.subspace_origin_per_logged_entity`, we don't have to check `self.subspaces` + // since every origin of a subspace is also a logged entity (except the root which we checked initially), + // making the keys of `self.subspace_origin_per_logged_entity` a superset of the keys of `self.subspaces`. + if let Some(origin_hash) = self + .subspace_origin_per_logged_entity + .get(&entity_reference.hash()) + { + return *origin_hash; + } + + if let Some(parent) = entity_reference.parent() { + entity_storage = parent; + entity_reference = &entity_storage; + } else { + return EntityPath::root().hash(); + }; + } + } + + /// Returns the subspace for a given origin. + /// + /// None if the origin doesn't identify its own subspace. + #[inline] + pub fn subspace_for_subspace_origin(&self, origin: EntityPathHash) -> Option<&SubSpace> { + self.subspaces.get(&origin) + } + + fn on_store_diff<'a>( + &mut self, + entity_path: &EntityPath, + added_components: impl Iterator, + ) { + re_tracing::profile_function!(); + + let mut new_subspace_connections = SubSpaceConnectionFlags::empty(); + for added_component in added_components { + if added_component == &DisconnectedSpace::name() { + new_subspace_connections.insert(SubSpaceConnectionFlags::Disconnected); + } else if added_component == &PinholeProjection::name() { + new_subspace_connections.insert(SubSpaceConnectionFlags::Pinhole); + }; + } + + // Do we already know about this entity in general? + if let Some(subspace_origin_hash) = self + .subspace_origin_per_logged_entity + .get(&entity_path.hash()) + { + // In that case, this causes only changes if there's a change in connection. + if !new_subspace_connections.is_empty() { + self.update_space_with_new_connections( + entity_path, + *subspace_origin_hash, + new_subspace_connections, + ); + } + } else { + self.add_new_entity(entity_path, new_subspace_connections); + }; + } + + fn update_space_with_new_connections( + &mut self, + entity_path: &EntityPath, + subspace_origin_hash: EntityPathHash, + new_connections: SubSpaceConnectionFlags, + ) { + if subspace_origin_hash == entity_path.hash() { + // If this is the origin of a space we can't split it. + // Instead we have to update connectivity & dimensionality. + let subspace = self + .subspaces + .get_mut(&subspace_origin_hash) + .expect("Subspace origin not part of origin->subspace map."); + + // (see also `split_subspace`)` + if new_connections.contains(SubSpaceConnectionFlags::Pinhole) { + subspace.dimensionality = SubSpaceDimensionality::TwoD; + } + + if let Some(parent_origin_hash) = subspace.parent_space { + self.subspaces + .get_mut(&parent_origin_hash) + .expect("Parent origin not part of origin->subspace map.") + .add_or_update_child_connection(entity_path, new_connections); + } + } else { + // Split the existing subspace. + self.split_subspace(subspace_origin_hash, entity_path, new_connections); + } + } + + /// Adds a new entity to the spatial topology that wasn't known before. + fn add_new_entity( + &mut self, + entity_path: &EntityPath, + subspace_connections: SubSpaceConnectionFlags, + ) { + let subspace_origin_hash = self.subspace_origin_hash_for_entity(entity_path); + + let target_space_origin_hash = if subspace_connections.is_empty() { + // Add entity to the existing space. + let subspace = self + .subspaces + .get_mut(&subspace_origin_hash) + .expect("Subspace origin not part of origin->subspace map."); + subspace.entities.insert(entity_path.clone()); + subspace.origin.hash() + } else { + // Create a new subspace with this entity as its origin & containing this entity. + self.split_subspace(subspace_origin_hash, entity_path, subspace_connections); + entity_path.hash() + }; + + self.subspace_origin_per_logged_entity + .insert(entity_path.hash(), target_space_origin_hash); + } + + fn split_subspace( + &mut self, + split_subspace_origin_hash: EntityPathHash, + new_space_origin: &EntityPath, + connections: SubSpaceConnectionFlags, + ) { + let split_subspace = self + .subspaces + .get_mut(&split_subspace_origin_hash) + .expect("Subspace origin not part of origin->subspace map."); + debug_assert!(new_space_origin.is_descendant_of(&split_subspace.origin)); + + // Determine the space dimensionality of the new space and update the current space's space type if necessary. + // (see also `update_space_with_new_connections`) + let new_space_dimensionality = if connections.contains(SubSpaceConnectionFlags::Pinhole) { + SubSpaceDimensionality::TwoD + } else { + SubSpaceDimensionality::Unknown + }; + + let mut new_space = SubSpace { + origin: new_space_origin.clone(), + dimensionality: new_space_dimensionality, + entities: std::iter::once(new_space_origin.clone()).collect(), + child_spaces: Default::default(), + parent_space: Some(split_subspace_origin_hash), + }; + + // Transfer entities from self to the new space if they're children of the new space. + split_subspace.entities.retain(|e| { + if e.starts_with(new_space_origin) { + self.subspace_origin_per_logged_entity + .insert(e.hash(), new_space.origin.hash()); + new_space.entities.insert(e.clone()); + false + } else { + true + } + }); + + // Transfer any child spaces from self to the new space if they're children of the new space. + split_subspace + .child_spaces + .retain(|child_origin, connections| { + debug_assert!(child_origin != new_space_origin); + + if child_origin.is_descendant_of(new_space_origin) { + new_space + .child_spaces + .insert(child_origin.clone(), *connections); + false + } else { + true + } + }); + + // Note that the new connection information may change the known dimensionality of the space that we're splitting. + split_subspace.add_or_update_child_connection(new_space_origin, connections); + + // Patch parents of the child spaces that were moved to the new space. + for child_origin in new_space.child_spaces.keys() { + let child_space = self + .subspaces + .get_mut(&child_origin.hash()) + .expect("Child origin not part of origin->subspace map."); + child_space.parent_space = Some(new_space.origin.hash()); + } + + self.subspaces.insert(new_space.origin.hash(), new_space); + } +} + +#[cfg(test)] +mod tests { + use re_log_types::EntityPath; + use re_types::{ + components::{DisconnectedSpace, PinholeProjection}, + ComponentName, Loggable as _, + }; + + use crate::spatial_topology::{SubSpaceConnectionFlags, SubSpaceDimensionality}; + + use super::SpatialTopology; + + #[test] + fn no_splits() { + let mut topo = SpatialTopology::default(); + + // Initialized with root space. + assert_eq!(topo.subspaces.len(), 1); + assert_eq!(topo.subspace_origin_per_logged_entity.len(), 0); + + // Add a simple tree without any splits for now. + add_diff(&mut topo, "robo", &[]); + add_diff(&mut topo, "robo/arm", &[]); + add_diff(&mut topo, "robo/eyes/cam", &[]); + + // Check that all entities are in the same space. + check_paths_in_space(&topo, &["robo", "robo/arm", "robo/eyes/cam"], "/"); + + // .. and that space has no children and no parent. + let subspace = topo.subspace_for_entity(&"robo".into()); + assert!(subspace.child_spaces.is_empty()); + assert!(subspace.parent_space.is_none()); + + // If we make up entities that weren't logged we get the closest space + assert_eq!( + topo.subspace_for_entity(&EntityPath::root()).origin, + EntityPath::root() + ); + assert_eq!( + topo.subspace_for_entity(&"robo/eyes".into()).origin, + EntityPath::root() + ); + assert_eq!( + topo.subspace_for_entity(&"robo/leg".into()).origin, + EntityPath::root() + ); + } + + #[test] + fn valid_splits() { + let mut topo = SpatialTopology::default(); + + // Two cameras, one delayed for later. + add_diff(&mut topo, "robo", &[]); + add_diff(&mut topo, "robo/eyes/left/cam/annotation", &[]); + add_diff(&mut topo, "robo/arm", &[]); + add_diff( + &mut topo, + "robo/eyes/left/cam", + &[PinholeProjection::name()], + ); + add_diff(&mut topo, "robo/eyes/right/cam/annotation", &[]); + add_diff(&mut topo, "robo/eyes/right/cam", &[]); + { + check_paths_in_space( + &topo, + &[ + "robo", + "robo/arm", + "robo/eyes/right/cam", + "robo/eyes/right/cam/annotation", + ], + "/", + ); + check_paths_in_space( + &topo, + &["robo/eyes/left/cam", "robo/eyes/left/cam/annotation"], + "robo/eyes/left/cam", + ); + + let root = topo.subspace_for_entity(&"robo".into()); + let left_camera = topo.subspace_for_entity(&"robo/eyes/left/cam".into()); + + assert_eq!(left_camera.origin, "robo/eyes/left/cam".into()); + assert_eq!(left_camera.parent_space, Some(root.origin.hash())); + assert_eq!(left_camera.dimensionality, SubSpaceDimensionality::TwoD); + + assert_eq!(root.dimensionality, SubSpaceDimensionality::ThreeD); + assert_eq!( + root.child_spaces, + std::iter::once((left_camera.origin.clone(), SubSpaceConnectionFlags::Pinhole)) + .collect() + ); + assert!(root.parent_space.is_none()); + } + + // Introduce a third space at the right camera. + add_diff( + &mut topo, + "robo/eyes/right/cam", + &[PinholeProjection::name()], + ); + { + check_paths_in_space(&topo, &["robo", "robo/arm"], "/"); + check_paths_in_space( + &topo, + &["robo/eyes/right/cam", "robo/eyes/right/cam/annotation"], + "robo/eyes/right/cam", + ); + + let root = topo.subspace_for_entity(&"robo".into()); + let left_camera = topo.subspace_for_entity(&"robo/eyes/left/cam".into()); + let right_camera = topo.subspace_for_entity(&"robo/eyes/right/cam".into()); + + assert_eq!(right_camera.origin, "robo/eyes/right/cam".into()); + assert_eq!(right_camera.parent_space, Some(root.origin.hash())); + assert_eq!(right_camera.dimensionality, SubSpaceDimensionality::TwoD); + assert_eq!( + root.child_spaces, + [ + (left_camera.origin.clone(), SubSpaceConnectionFlags::Pinhole), + ( + right_camera.origin.clone(), + SubSpaceConnectionFlags::Pinhole + ) + ] + .into_iter() + .collect() + ); + + // If we make up entities that weren't logged we get the closest space + assert_eq!( + topo.subspace_for_entity(&"robo/eyes/right/cam/unheard".into()) + .origin, + "robo/eyes/right/cam".into() + ); + assert_eq!( + topo.subspace_for_entity(&"bonkers".into()).origin, + EntityPath::root() + ); + } + + // Disconnect the left camera. + add_diff( + &mut topo, + "robo/eyes/left/cam", + &[DisconnectedSpace::name()], + ); + { + let root = topo.subspace_for_entity(&"robo".into()); + let left_camera = topo.subspace_for_entity(&"robo/eyes/left/cam".into()); + let right_camera = topo.subspace_for_entity(&"robo/eyes/right/cam".into()); + + assert_eq!(left_camera.origin, "robo/eyes/left/cam".into()); + assert_eq!(left_camera.parent_space, Some(root.origin.hash())); + assert_eq!(left_camera.dimensionality, SubSpaceDimensionality::TwoD); + assert_eq!(root.dimensionality, SubSpaceDimensionality::ThreeD); + assert!(root.parent_space.is_none()); + assert_eq!( + root.child_spaces, + [ + ( + left_camera.origin.clone(), + SubSpaceConnectionFlags::Disconnected | SubSpaceConnectionFlags::Pinhole + ), + ( + right_camera.origin.clone(), + SubSpaceConnectionFlags::Pinhole + ) + ] + .into_iter() + .collect() + ); + } + } + + #[test] + fn handle_invalid_splits_gracefully() { + for nested_first in [false, true] { + let mut topo = SpatialTopology::default(); + + // Two nested cameras. Try both orderings + if nested_first { + add_diff(&mut topo, "cam0/cam1", &[PinholeProjection::name()]); + add_diff(&mut topo, "cam0", &[PinholeProjection::name()]); + } else { + add_diff(&mut topo, "cam0", &[PinholeProjection::name()]); + add_diff(&mut topo, "cam0/cam1", &[PinholeProjection::name()]); + } + + check_paths_in_space(&topo, &["cam0"], "cam0"); + check_paths_in_space(&topo, &["cam0/cam1"], "cam0/cam1"); + + let root = topo.subspace_for_entity(&EntityPath::root()); + let cam0 = topo.subspace_for_entity(&"cam0".into()); + let cam1 = topo.subspace_for_entity(&"cam0/cam1".into()); + + assert_eq!(cam0.dimensionality, SubSpaceDimensionality::TwoD); + assert_eq!(cam1.dimensionality, SubSpaceDimensionality::TwoD); + assert_eq!(cam0.parent_space, Some(EntityPath::root().hash())); + assert_eq!(cam1.parent_space, Some(cam0.origin.hash())); + + assert_eq!( + root.child_spaces, + std::iter::once((cam0.origin.clone(), SubSpaceConnectionFlags::Pinhole)).collect() + ); + + assert_eq!( + cam0.child_spaces, + std::iter::once((cam1.origin.clone(), SubSpaceConnectionFlags::Pinhole)).collect() + ); + assert!(cam1.child_spaces.is_empty()); + } + } + + #[test] + fn disconnected_pinhole() { + let mut topo = SpatialTopology::default(); + + add_diff(&mut topo, "stuff", &[]); + add_diff( + &mut topo, + "camera", + &[PinholeProjection::name(), DisconnectedSpace::name()], + ); + add_diff(&mut topo, "camera/image", &[]); + + check_paths_in_space(&topo, &["stuff"], "/"); + check_paths_in_space(&topo, &["camera", "camera/image"], "camera"); + + let cam = topo.subspace_for_entity(&"camera".into()); + assert_eq!(cam.dimensionality, SubSpaceDimensionality::TwoD); + assert_eq!(cam.parent_space, Some(EntityPath::root().hash())); + + let root = topo.subspace_for_entity(&"stuff".into()); + assert_eq!(root.dimensionality, SubSpaceDimensionality::ThreeD); + assert_eq!( + root.child_spaces, + std::iter::once(( + cam.origin.clone(), + SubSpaceConnectionFlags::Disconnected | SubSpaceConnectionFlags::Pinhole + )) + .collect() + ); + } + + fn add_diff(topo: &mut SpatialTopology, path: &str, components: &[ComponentName]) { + topo.on_store_diff(&path.into(), components.iter()); + } + + fn check_paths_in_space(topo: &SpatialTopology, paths: &[&str], expected_origin: &str) { + for path in paths { + let path = *path; + assert_eq!( + topo.subspace_for_entity(&path.into()).origin, + expected_origin.into() + ); + } + + let space = topo.subspace_for_entity(&paths[0].into()); + for path in paths { + let path = *path; + assert!(space.entities.contains(&path.into())); + } + assert_eq!(space.entities.len(), paths.len()); + } +} diff --git a/crates/re_space_view_spatial/src/visualizers/arrows3d.rs b/crates/re_space_view_spatial/src/visualizers/arrows3d.rs index 3ec1ec6a9751..c14afacb1335 100644 --- a/crates/re_space_view_spatial/src/visualizers/arrows3d.rs +++ b/crates/re_space_view_spatial/src/visualizers/arrows3d.rs @@ -186,6 +186,7 @@ impl VisualizerSystem for Arrows3DVisualizer { entities: ApplicableEntities, context: &dyn VisualizableFilterContext, ) -> VisualizableEntities { + re_tracing::profile_function!(); filter_visualizable_3d_entities(entities, context) } diff --git a/crates/re_space_view_spatial/src/visualizers/assets3d.rs b/crates/re_space_view_spatial/src/visualizers/assets3d.rs index af0d40a13636..ec5b6a6d3893 100644 --- a/crates/re_space_view_spatial/src/visualizers/assets3d.rs +++ b/crates/re_space_view_spatial/src/visualizers/assets3d.rs @@ -122,6 +122,7 @@ impl VisualizerSystem for Asset3DVisualizer { entities: ApplicableEntities, context: &dyn VisualizableFilterContext, ) -> VisualizableEntities { + re_tracing::profile_function!(); filter_visualizable_3d_entities(entities, context) } diff --git a/crates/re_space_view_spatial/src/visualizers/boxes3d.rs b/crates/re_space_view_spatial/src/visualizers/boxes3d.rs index ed20f5b49ffb..c39ae17c8310 100644 --- a/crates/re_space_view_spatial/src/visualizers/boxes3d.rs +++ b/crates/re_space_view_spatial/src/visualizers/boxes3d.rs @@ -143,6 +143,7 @@ impl VisualizerSystem for Boxes3DVisualizer { entities: ApplicableEntities, context: &dyn VisualizableFilterContext, ) -> VisualizableEntities { + re_tracing::profile_function!(); filter_visualizable_3d_entities(entities, context) } diff --git a/crates/re_space_view_spatial/src/visualizers/cameras.rs b/crates/re_space_view_spatial/src/visualizers/cameras.rs index ce805f5dd3c8..d7911ba4d135 100644 --- a/crates/re_space_view_spatial/src/visualizers/cameras.rs +++ b/crates/re_space_view_spatial/src/visualizers/cameras.rs @@ -205,6 +205,7 @@ impl VisualizerSystem for CamerasVisualizer { entities: ApplicableEntities, context: &dyn VisualizableFilterContext, ) -> VisualizableEntities { + re_tracing::profile_function!(); filter_visualizable_3d_entities(entities, context) } diff --git a/crates/re_space_view_spatial/src/visualizers/lines3d.rs b/crates/re_space_view_spatial/src/visualizers/lines3d.rs index 6313da5ca88d..b8e28d9a56eb 100644 --- a/crates/re_space_view_spatial/src/visualizers/lines3d.rs +++ b/crates/re_space_view_spatial/src/visualizers/lines3d.rs @@ -188,6 +188,7 @@ impl VisualizerSystem for Lines3DVisualizer { entities: ApplicableEntities, context: &dyn VisualizableFilterContext, ) -> VisualizableEntities { + re_tracing::profile_function!(); filter_visualizable_3d_entities(entities, context) } diff --git a/crates/re_space_view_spatial/src/visualizers/meshes.rs b/crates/re_space_view_spatial/src/visualizers/meshes.rs index f0d5dec86e01..05f79d3e0c4a 100644 --- a/crates/re_space_view_spatial/src/visualizers/meshes.rs +++ b/crates/re_space_view_spatial/src/visualizers/meshes.rs @@ -153,6 +153,7 @@ impl VisualizerSystem for Mesh3DVisualizer { entities: ApplicableEntities, context: &dyn VisualizableFilterContext, ) -> VisualizableEntities { + re_tracing::profile_function!(); filter_visualizable_3d_entities(entities, context) } diff --git a/crates/re_space_view_spatial/src/visualizers/mod.rs b/crates/re_space_view_spatial/src/visualizers/mod.rs index 628e07d60f16..e574d2b0c6e6 100644 --- a/crates/re_space_view_spatial/src/visualizers/mod.rs +++ b/crates/re_space_view_spatial/src/visualizers/mod.rs @@ -401,61 +401,64 @@ pub fn image_view_coordinates() -> re_types::components::ViewCoordinates { re_types::components::ViewCoordinates::RDF } -/// If 2d object is shown in a 3d space view, it is only then visualizable, if it is under a pinhole camera. fn filter_visualizable_2d_entities( entities: ApplicableEntities, context: &dyn VisualizableFilterContext, ) -> VisualizableEntities { - // `VisualizableFilterContext3D` will only be available if we're in a 3D space view. if let Some(context) = context .as_any() - .downcast_ref::() + .downcast_ref::() { VisualizableEntities( - entities - .0 - .into_iter() - .filter(|ent_path| context.entities_under_pinhole.contains(&ent_path.hash())) + context + .entities_in_main_2d_space + .intersection(&entities.0) + .cloned() .collect(), ) } else if let Some(context) = context .as_any() - .downcast_ref::() + .downcast_ref::() { - if !context.invalid_subtrees.is_empty() { - VisualizableEntities( - entities - .0 - .into_iter() - .filter(|ent_path| { - !context - .invalid_subtrees - .iter() - .any(|invalid_subtree| ent_path.starts_with(invalid_subtree)) - }) - .collect(), - ) - } else { - VisualizableEntities(entities.0) - } + VisualizableEntities( + context + .entities_under_pinholes + .intersection(&entities.0) + .cloned() + .collect(), + ) } else { VisualizableEntities(entities.0) } } -/// If 3d object is shown in a 2d space view, it is only visualizable, if the origin of the space view has a pinhole camera. fn filter_visualizable_3d_entities( entities: ApplicableEntities, context: &dyn VisualizableFilterContext, ) -> VisualizableEntities { - // `VisualizableFilterContext2D` will only be available if we're in a 2D space view. - if context + if let Some(context) = context .as_any() .downcast_ref::() - .map_or(true, |c| c.has_pinhole_at_origin) { - VisualizableEntities(entities.0) + VisualizableEntities( + context + .reprojectable_3d_entities + .intersection(&entities.0) + .cloned() + .collect(), + ) + } else if let Some(context) = context + .as_any() + .downcast_ref::() + { + VisualizableEntities( + context + .entities_in_main_3d_space + .intersection(&entities.0) + .cloned() + .collect(), + ) } else { - VisualizableEntities::default() + VisualizableEntities(entities.0) } } diff --git a/crates/re_space_view_spatial/src/visualizers/points3d.rs b/crates/re_space_view_spatial/src/visualizers/points3d.rs index 2fa4cce6c7e7..5b23536013c6 100644 --- a/crates/re_space_view_spatial/src/visualizers/points3d.rs +++ b/crates/re_space_view_spatial/src/visualizers/points3d.rs @@ -181,6 +181,7 @@ impl VisualizerSystem for Points3DVisualizer { entities: ApplicableEntities, context: &dyn VisualizableFilterContext, ) -> VisualizableEntities { + re_tracing::profile_function!(); filter_visualizable_3d_entities(entities, context) } From ebc49a7932b76e2642c8d444ee9ffb443288c5fd Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Thu, 18 Jan 2024 13:06:54 +0100 Subject: [PATCH 4/4] Use new egui_tiles `active_tiles` api to simplify space view multithreading (#4864) ### What Removes a workaround where we had track the tiles that were visible last frame. Resolved now due to * https://github.com/rerun-io/egui_tiles/pull/42 ### 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/4864/index.html) * Using examples from latest `main` build: [app.rerun.io](https://app.rerun.io/pr/4864/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/4864/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/4864) - [Docs preview](https://rerun.io/preview/56cae2d11d8d4c5bd8df2a07e2814992c2d200f7/docs) - [Examples preview](https://rerun.io/preview/56cae2d11d8d4c5bd8df2a07e2814992c2d200f7/examples) - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html) --- crates/re_viewport/src/system_execution.rs | 23 +++++++---- crates/re_viewport/src/viewport.rs | 46 +++++++--------------- 2 files changed, 29 insertions(+), 40 deletions(-) diff --git a/crates/re_viewport/src/system_execution.rs b/crates/re_viewport/src/system_execution.rs index a3a2769a974f..aaadc1fc6a4a 100644 --- a/crates/re_viewport/src/system_execution.rs +++ b/crates/re_viewport/src/system_execution.rs @@ -60,7 +60,7 @@ pub fn create_and_run_space_view_systems( pub fn execute_systems_for_space_views<'a>( ctx: &'a ViewerContext<'a>, - mut space_views_to_execute: Vec, + tree: &egui_tiles::Tree, space_views: &'a BTreeMap, ) -> HashMap, SystemExecutionOutput)> { let Some(time_int) = ctx.rec_cfg.time_ctrl.read().time_int() else { @@ -69,13 +69,20 @@ pub fn execute_systems_for_space_views<'a>( re_tracing::profile_wait!("execute_systems"); - space_views_to_execute - .par_drain(..) - .filter_map(|space_view_id| { - let space_view_blueprint = space_views.get(&space_view_id)?; - let highlights = highlights_for_space_view(ctx, space_view_id); - let output = space_view_blueprint.execute_systems(ctx, time_int, highlights); - Some((space_view_id, output)) + tree.active_tiles() + .into_par_iter() + .filter_map(|tile_id| { + tree.tiles.get(tile_id).and_then(|tile| match tile { + egui_tiles::Tile::Pane(space_view_id) => { + space_views.get(space_view_id).map(|space_view_blueprint| { + let highlights = highlights_for_space_view(ctx, *space_view_id); + let output = + space_view_blueprint.execute_systems(ctx, time_int, highlights); + (*space_view_id, output) + }) + } + egui_tiles::Tile::Container(_) => None, + }) }) .collect::>() } diff --git a/crates/re_viewport/src/viewport.rs b/crates/re_viewport/src/viewport.rs index db73fc518b3a..837402c2054a 100644 --- a/crates/re_viewport/src/viewport.rs +++ b/crates/re_viewport/src/viewport.rs @@ -20,7 +20,6 @@ use re_viewer_context::{ use crate::{ space_view_entity_picker::SpaceViewEntityPicker, space_view_heuristics::default_created_space_views, - space_view_highlights::highlights_for_space_view, system_execution::execute_systems_for_space_views, SpaceInfoCollection, SpaceViewBlueprint, ViewportBlueprint, }; @@ -39,11 +38,6 @@ pub struct PerSpaceViewState { pub struct ViewportState { space_view_entity_window: SpaceViewEntityPicker, space_view_states: HashMap, - - /// List of all space views that were visible *on screen* (excluding e.g. unselected tabs) the last frame. - /// - /// TODO(rerun-io/egui_tiles#34): This is needed because we don't know which space views will be visible until we have drawn them. - space_views_displayed_last_frame: Vec, } static DEFAULT_PROPS: Lazy = Lazy::::new(Default::default); @@ -213,11 +207,8 @@ impl<'a, 'b> Viewport<'a, 'b> { &mut self.tree }; - let executed_systems_per_space_view = execute_systems_for_space_views( - ctx, - std::mem::take(&mut state.space_views_displayed_last_frame), - &blueprint.space_views, - ); + let executed_systems_per_space_view = + execute_systems_for_space_views(ctx, tree, &blueprint.space_views); ui.scope(|ui| { ui.spacing_mut().item_spacing.x = re_ui::ReUi::view_padding(); @@ -230,7 +221,6 @@ impl<'a, 'b> Viewport<'a, 'b> { space_views: &blueprint.space_views, maximized: &mut maximized, edited: false, - space_views_displayed_current_frame: Vec::new(), executed_systems_per_space_view, }; @@ -251,8 +241,6 @@ impl<'a, 'b> Viewport<'a, 'b> { // TODO(#4687): Be extra careful here. If we mark edited inappropriately we can create an infinite edit loop. self.edited |= tab_viewer.edited; - - state.space_views_displayed_last_frame = tab_viewer.space_views_displayed_current_frame; }); self.blueprint.set_maximized(maximized, ctx); @@ -472,11 +460,6 @@ struct TabViewer<'a, 'b> { /// List of query & system execution results for each space view. executed_systems_per_space_view: HashMap, SystemExecutionOutput)>, - /// List of all space views drawn this frame. - /// - /// TODO(rerun-io/egui_tiles#34): It should be possible to predict which space views will be drawn. - space_views_displayed_current_frame: Vec, - /// The user edited the tree. edited: bool, } @@ -499,22 +482,24 @@ impl<'a, 'b> egui_tiles::Behavior for TabViewer<'a, 'b> { return Default::default(); } - let Some(latest_at) = self.ctx.rec_cfg.time_ctrl.read().time_int() else { + if self.ctx.rec_cfg.time_ctrl.read().time_int().is_none() { ui.centered_and_justified(|ui| { ui.weak("No time selected"); }); return Default::default(); }; - // TODO(rerun-io/egui_tiles#34): If we haven't executed the system yet ahead of time, we should do so now. - // This is needed because we merely "guess" which systems we are going to need. - let (query, system_output) = - if let Some(result) = self.executed_systems_per_space_view.remove(space_view_id) { - result - } else { - let highlights = highlights_for_space_view(self.ctx, *space_view_id); - space_view_blueprint.execute_systems(self.ctx, latest_at, highlights) - }; + let Some((query, system_output)) = + self.executed_systems_per_space_view.remove(space_view_id) + else { + // The space view's systems haven't been executed. + // This should never happen, but if it does anyways we can't display the space view. + re_log::error_once!( + "Visualizers for space view {:?} haven't been executed prior to display. This should never happen, please report a bug.", + space_view_blueprint.display_name_or_default() + ); // TODO(#4433): This should go to analytics + return Default::default(); + }; let PerSpaceViewState { auto_properties: _, @@ -525,9 +510,6 @@ impl<'a, 'b> egui_tiles::Behavior for TabViewer<'a, 'b> { space_view_blueprint.class_identifier(), ); - self.space_views_displayed_current_frame - .push(space_view_blueprint.id); - space_view_blueprint.scene_ui( space_view_state.as_mut(), self.ctx,