Allow root space views transforms to other roots (#1075)

* Allow adding entities from arbitrary space paths
Fix bug in space info reachability check (which drives the entity picker)
* improve group naming for now unrestricted object addings

Author: Wumpf

crates/re_log_types/src/component_types/transform.rs

@@ -51,6 +51,16 @@ pub struct Rigid3 {
 
 #[cfg(feature = "glam")]
 impl Rigid3 {
+    pub const IDENTITY: Rigid3 = Rigid3 {
+        rotation: Quaternion {
+            x: 1.0,
+            y: 0.0,
+            z: 0.0,
+            w: 0.0,
+        },
+        translation: Vec3D([0.0, 0.0, 0.0]),
+    };
+
     #[inline]
     pub fn new_parent_from_child(parent_from_child: macaw::IsoTransform) -> Self {
         Self {

crates/re_viewer/src/misc/space_info.rs

@@ -98,7 +98,7 @@ impl SpaceInfo {
 /// Information about all spaces.
 ///
 /// This is gathered by analyzing the transform hierarchy of the entities:
-/// For every child of the root there is a space info.
+/// For every child of the root there is a space info, as well as the root itself.
 /// Each of these we walk down recursively, every time a transform is encountered, we create another space info.
 ///
 /// Expected to be recreated every frame (or whenever new data is available).
@@ -163,20 +163,39 @@ impl SpaceInfoCollection {
 
         let mut spaces_info = Self::default();
 
+        // The root itself.
+        // To make our heuristics work we pretend direct child of the root has a transform,
+        // breaking the pattern applied for everything else where we create a SpaceInfo once we hit a transform.
+        //
+        // TODO(andreas): Our dependency on SpaceInfo in this way is quite telling - we should be able to create a SpaceView without having a corresponding SpaceInfo
+        //                Currently too many things depend on every SpaceView being backed up by a concrete SpaceInfo on its space path.
+        if query_transform(entity_db, &EntityPath::root(), &query).is_some() {
+            re_log::warn_once!("The root entity has a 'transform' component! This will have no effect. Did you mean to apply the transform elsewhere?");
+        }
+        let mut root_space_info = SpaceInfo::new(EntityPath::root());
+        root_space_info
+            .descendants_without_transform
+            .insert(EntityPath::root());
+
         for tree in entity_db.tree.children.values() {
-            // Each root entity is its own space (or should be)
+            let mut space_info = SpaceInfo::new(tree.path.clone());
+            let transform = query_transform(entity_db, &EntityPath::root(), &query)
+                .unwrap_or(Transform::Rigid3(re_log_types::Rigid3::IDENTITY));
+            space_info.parent = Some((EntityPath::root(), transform.clone()));
+            space_info
+                .descendants_without_transform
+                .insert(tree.path.clone());
 
-            if query_transform(entity_db, &tree.path, &query).is_some() {
-                re_log::warn_once!(
-                    "Root entity '{}' has a _transform - this is not allowed!",
-                    tree.path
-                );
-            }
+            root_space_info
+                .child_spaces
+                .insert(tree.path.clone(), transform);
 
-            let mut space_info = SpaceInfo::new(tree.path.clone());
             add_children(entity_db, &mut spaces_info, &mut space_info, tree, &query);
             spaces_info.spaces.insert(tree.path.clone(), space_info);
         }
+        spaces_info
+            .spaces
+            .insert(EntityPath::root(), root_space_info);
 
         for (entity_path, space_info) in &mut spaces_info.spaces {
             space_info.coordinates = query_view_coordinates(entity_db, entity_path, &query);
@@ -209,40 +228,36 @@ impl SpaceInfoCollection {
     ///
     /// For how, you need to check [`crate::misc::TransformCache`]!
     /// Note that in any individual frame, entities may or may not be reachable.
-    ///
-    /// If `from` and `to_reference` are not under the same root branch, they are regarded as [`UnreachableTransform::Unconnected`]
     pub fn is_reachable_by_transform(
         &self,
         from: &EntityPath,
         to_reference: &EntityPath,
     ) -> Result<(), UnreachableTransform> {
         crate::profile_function!();
 
-        // By convention we regard the global hierarchy as a forest - don't allow breaking out of the current tree.
-        if from.iter().next() != to_reference.iter().next() {
-            return Err(UnreachableTransform::Unconnected);
-        }
-
         // Get closest space infos for the given entity paths.
         let Some(mut from_space) = self.get_first_parent_with_info(from) else {
             re_log::warn_once!("{} not part of space infos", from);
-            return Err(UnreachableTransform::Unconnected);
+            return Err(UnreachableTransform::UnknownSpaceInfo);
         };
         let Some(mut to_reference_space) = self.get_first_parent_with_info(to_reference) else {
             re_log::warn_once!("{} not part of space infos", to_reference);
-            return Err(UnreachableTransform::Unconnected);
+            return Err(UnreachableTransform::UnknownSpaceInfo);
         };
 
-        // If this is not true, the path we're querying, `from`, is outside of the tree the reference node.
-        // Note that this means that all transforms on the way are inversed!
-        let from_is_child_of_reference = from.is_descendant_of(to_reference);
-
         // Reachability is (mostly) commutative!
-        // This means we can simply walk from the lower node to the parent until we're on the same node
+        // This means we can simply walk from both nodes up until we find a common ancestor!
         // If we haven't encountered any obstacles, we're fine!
         let mut encountered_pinhole = false;
         while from_space.path != to_reference_space.path {
-            let parent = if from_is_child_of_reference {
+            // Decide if we should walk up "from" or "to_reference"
+            // If "from" is a descendant of "to_reference", we walk up "from"
+            // Otherwise we walk up on "to_reference".
+            //
+            // If neither is a descendant of the other it doesn't matter which one we walk up, since we eventually going to hit common ancestor!
+            let walk_up_from = from_space.path.is_descendant_of(&to_reference_space.path);
+
+            let parent = if walk_up_from {
                 &from_space.parent
             } else {
                 &to_reference_space.parent
@@ -258,7 +273,7 @@ impl SpaceInfoCollection {
                             Err(UnreachableTransform::NestedPinholeCameras)
                         } else {
                             encountered_pinhole = true;
-                            if pinhole.resolution.is_none() && !from_is_child_of_reference {
+                            if pinhole.resolution.is_none() && !walk_up_from {
                                 Err(UnreachableTransform::InversePinholeCameraWithoutResolution)
                             } else {
                                 Ok(())
@@ -269,10 +284,10 @@ impl SpaceInfoCollection {
 
                 let Some(parent_space) = self.get(parent_path) else {
                     re_log::warn_once!("{} not part of space infos", parent_path);
-                    return Err(UnreachableTransform::Unconnected);
+                    return Err(UnreachableTransform::UnknownSpaceInfo);
                 };
 
-                if from_is_child_of_reference {
+                if walk_up_from {
                     from_space = parent_space;
                 } else {
                     to_reference_space = parent_space;
@@ -283,7 +298,7 @@ impl SpaceInfoCollection {
                     from,
                     to_reference
                 );
-                return Err(UnreachableTransform::Unconnected);
+                return Err(UnreachableTransform::UnknownSpaceInfo);
             }
         }
 

crates/re_viewer/src/misc/transform_cache.rs

@@ -24,16 +24,15 @@ pub struct TransformCache {
     unreachable_descendants: Vec<(EntityPath, UnreachableTransform)>,
 
     /// The first parent of reference_path that is no longer reachable.
-    first_unreachable_parent: (EntityPath, UnreachableTransform),
+    first_unreachable_parent: Option<(EntityPath, UnreachableTransform)>,
 }
 
 #[derive(Clone, Copy)]
 pub enum UnreachableTransform {
-    /// Not part of the hierarchy at all.
-    /// TODO(andreas): This is only needed for the "breaking out of the root" case that we want to scrap.
-    ///                 After that it's impossible to be unreachable since there can always be an identity matrix.
-    ///                 We already allow walking over unlogged paths (because why not)
-    Unconnected,
+    /// [`super::space_info::SpaceInfoCollection`] is outdated and can't find a corresponding space info for the given path.
+    ///
+    /// If at all, this should only happen for a single frame until space infos are rebuilt.
+    UnknownSpaceInfo,
     /// More than one pinhole camera between this and the reference space.
     NestedPinholeCameras,
     /// Exiting out of a space with a pinhole camera that doesn't have a resolution is not supported.
@@ -45,8 +44,8 @@ pub enum UnreachableTransform {
 impl std::fmt::Display for UnreachableTransform {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         f.write_str(match self {
-            Self::Unconnected =>
-                "No entity path connection from this space view.",
+            Self::UnknownSpaceInfo =>
+                "Can't determine transform because internal data structures are not in a valid state. Please file an issue on https://github.com/rerun-io/rerun/",
             Self::NestedPinholeCameras =>
                 "Can't display entities under nested pinhole cameras.",
             Self::UnknownTransform =>
@@ -75,7 +74,7 @@ impl TransformCache {
             reference_path: root_path.clone(),
             reference_from_entity_per_entity: Default::default(),
             unreachable_descendants: Default::default(),
-            first_unreachable_parent: (EntityPath::root(), UnreachableTransform::Unconnected),
+            first_unreachable_parent: None,
         };
 
         // Find the entity path tree for the root.
@@ -113,13 +112,6 @@ impl TransformCache {
         let mut encountered_pinhole = false;
         let mut reference_from_ancestor = glam::Mat4::IDENTITY;
         while let Some(parent_tree) = parent_tree_stack.pop() {
-            // By convention we regard the global hierarchy as a forest - don't allow breaking out of the current tree.
-            if parent_tree.path.is_root() {
-                transforms.first_unreachable_parent =
-                    (parent_tree.path.clone(), UnreachableTransform::Unconnected);
-                break;
-            }
-
             // Note that the transform at the reference is the first that needs to be inverted to "break out" of its hierarchy.
             // Generally, the transform _at_ a node isn't relevant to it's children, but only to get to its parent in turn!
             match inverse_transform_at(
@@ -130,7 +122,7 @@ impl TransformCache {
             ) {
                 Err(unreachable_reason) => {
                     transforms.first_unreachable_parent =
-                        (parent_tree.path.clone(), unreachable_reason);
+                        Some((parent_tree.path.clone(), unreachable_reason));
                     break;
                 }
                 Ok(None) => {}

crates/re_viewer/src/ui/data_blueprint.rs

@@ -221,7 +221,7 @@ impl DataBlueprintTree {
             } else {
                 // Otherwise, create a new group which only contains this entity and add the group to the hierarchy.
                 let new_group = self.groups.insert(DataBlueprintGroup {
-                    display_name: path_to_group_name(path, base_path),
+                    display_name: path_to_group_name(path),
                     ..Default::default()
                 });
                 self.add_group_to_hierarchy_recursively(new_group, path, base_path);
@@ -271,8 +271,7 @@ impl DataBlueprintTree {
 
         // Short circuit to the root group at base_path.
         // If the entity is outside of the base path we would walk up all the way to the root
-        // That's ok but we want to stop one element short (since a space view can only show elements under a shared path)
-        if &parent_path == base_path || parent_path.iter().count() == 1 {
+        if &parent_path == base_path {
             parent_path = EntityPath::root();
         }
 
@@ -289,7 +288,7 @@ impl DataBlueprintTree {
 
             std::collections::hash_map::Entry::Vacant(vacant_mapping) => {
                 let parent_group = self.groups.insert(DataBlueprintGroup {
-                    display_name: path_to_group_name(&parent_path, base_path),
+                    display_name: path_to_group_name(&parent_path),
                     children: smallvec![new_group],
                     ..Default::default()
                 });
@@ -398,18 +397,6 @@ impl DataBlueprintTree {
     }
 }
 
-fn path_to_group_name(path: &EntityPath, base_path: &EntityPath) -> String {
-    // Root should never be pasesd in here, but handle it gracefully regardless.
-    let name = path.iter().last().map_or("/".to_owned(), |c| c.to_string());
-
-    // How many steps until a common ancestor?
-    let mut num_steps_until_common_ancestor = 0;
-    let mut common_ancestor = base_path.clone();
-    while !path.is_descendant_of(&common_ancestor) {
-        // Can never go beyond root because everything is a descendent of root, therefore this is safe.
-        common_ancestor = common_ancestor.parent().unwrap();
-        num_steps_until_common_ancestor += 1;
-    }
-
-    format!("{}{}", "../".repeat(num_steps_until_common_ancestor), name)
+fn path_to_group_name(path: &EntityPath) -> String {
+    path.iter().last().map_or("/".to_owned(), |c| c.to_string())
 }

crates/re_viewer/src/ui/space_view.rs

@@ -41,11 +41,6 @@ pub struct SpaceView {
     pub id: SpaceViewId,
     pub name: String,
 
-    /// Everything under this root *can* be shown in the space view.
-    ///
-    /// TODO(andreas): We decided to remove this concept.
-    pub root_path: EntityPath,
-
     /// The "anchor point" of this space view.
     /// It refers to a [`SpaceInfo`] which forms our reference point for all scene->world transforms in this space view.
     /// I.e. the position of this entity path in space forms the origin of the coordinate system in this space view.
@@ -71,11 +66,6 @@ impl SpaceView {
         space_info: &SpaceInfo,
         queries_entities: &[EntityPath],
     ) -> Self {
-        let root_path = space_info.path.iter().next().map_or_else(
-            || space_info.path.clone(),
-            |c| EntityPath::from(vec![c.clone()]),
-        );
-
         let name = if queries_entities.len() == 1 {
             // a single entity in this space-view - name the space after it
             queries_entities[0].to_string()
@@ -90,7 +80,6 @@ impl SpaceView {
         Self {
             name,
             id: SpaceViewId::random(),
-            root_path,
             space_path: space_info.path.clone(),
             data_blueprint: data_blueprint_tree,
             view_state: ViewState::default(),

crates/re_viewer/src/ui/space_view_entity_picker.rs

@@ -75,20 +75,18 @@ impl SpaceViewEntityPicker {
 
 fn add_entities_ui(ctx: &mut ViewerContext<'_>, ui: &mut egui::Ui, space_view: &mut SpaceView) {
     let spaces_info = SpaceInfoCollection::new(&ctx.log_db.entity_db);
-    // TODO(andreas): remove use space_view.root_path, just show everything
-    if let Some(tree) = ctx.log_db.entity_db.tree.subtree(&space_view.root_path) {
-        let entities_add_info = create_entity_add_info(ctx, tree, space_view, &spaces_info);
+    let tree = &ctx.log_db.entity_db.tree;
+    let entities_add_info = create_entity_add_info(ctx, tree, space_view, &spaces_info);
 
-        add_entities_tree_ui(
-            ctx,
-            ui,
-            &spaces_info,
-            &tree.path.to_string(),
-            tree,
-            space_view,
-            &entities_add_info,
-        );
-    }
+    add_entities_tree_ui(
+        ctx,
+        ui,
+        &spaces_info,
+        &tree.path.to_string(),
+        tree,
+        space_view,
+        &entities_add_info,
+    );
 }
 
 fn add_entities_tree_ui(

crates/re_viewer/src/ui/viewport.rs

@@ -521,24 +521,27 @@ impl Viewport {
         let mut space_views = Vec::new();
 
         for space_view_candidate in Self::all_possible_space_views(ctx, spaces_info) {
+            // Skip root space for now, messes things up.
+            if space_view_candidate.space_path.is_root() {
+                continue;
+            }
+
             let Some(space_info) = spaces_info.get(&space_view_candidate.space_path) else {
                 // Should never happen.
                 continue;
             };
 
-            // If it doesn't contain anything but the transform itself, skip,
-            if space_info.descendants_without_transform.is_empty() {
-                continue;
-            }
-
             if space_view_candidate.category == ViewCategory::Spatial {
-                // Skip if connection to parent is via rigid (too trivial for a new space view!)
-                if let Some(parent_transform) = space_info.parent_transform() {
-                    match parent_transform {
-                        re_log_types::Transform::Rigid3(_) => {
-                            continue;
+                // For every item that isn't a direct descendant of the root, skip if connection to parent is via rigid (too trivial for a new space view!)
+                if space_info.path.parent() != Some(EntityPath::root()) {
+                    if let Some(parent_transform) = space_info.parent_transform() {
+                        match parent_transform {
+                            re_log_types::Transform::Rigid3(_) => {
+                                continue;
+                            }
+                            re_log_types::Transform::Pinhole(_)
+                            | re_log_types::Transform::Unknown => {}
                         }
-                        re_log_types::Transform::Pinhole(_) | re_log_types::Transform::Unknown => {}
                     }
                 }