Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow root space views transforms to other roots #1075

Merged
merged 4 commits into from
Feb 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions crates/re_log_types/src/component_types/transform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
71 changes: 43 additions & 28 deletions crates/re_viewer/src/misc/space_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand All @@ -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(())
Expand All @@ -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;
Expand All @@ -283,7 +298,7 @@ impl SpaceInfoCollection {
from,
to_reference
);
return Err(UnreachableTransform::Unconnected);
return Err(UnreachableTransform::UnknownSpaceInfo);
}
}

Expand Down
26 changes: 9 additions & 17 deletions crates/re_viewer/src/misc/transform_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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 =>
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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(
Expand All @@ -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) => {}
Expand Down
23 changes: 5 additions & 18 deletions crates/re_viewer/src/ui/data_blueprint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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();
}

Expand All @@ -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()
});
Expand Down Expand Up @@ -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())
}
11 changes: 0 additions & 11 deletions crates/re_viewer/src/ui/space_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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()
Expand All @@ -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(),
Expand Down
24 changes: 11 additions & 13 deletions crates/re_viewer/src/ui/space_view_entity_picker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
25 changes: 14 additions & 11 deletions crates/re_viewer/src/ui/viewport.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {}
}
}

Expand Down