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/Cargo.lock b/Cargo.lock index f5c66cbb3c42..336780852773 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", @@ -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/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/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) } 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, 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",