Skip to content

Commit

Permalink
move disconnected space detection into transform cache as well
Browse files Browse the repository at this point in the history
  • Loading branch information
Wumpf committed Dec 13, 2024
1 parent 152dff6 commit f170086
Show file tree
Hide file tree
Showing 7 changed files with 108 additions and 175 deletions.
26 changes: 24 additions & 2 deletions crates/viewer/re_view/src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,12 @@ pub trait DataResultQuery {
latest_at_query: &'a LatestAtQuery,
) -> HybridLatestAtResults<'a>;

fn latest_at_with_blueprint_resolved_data_for_component<'a, C: re_types_core::Component>(
&'a self,
ctx: &'a ViewContext<'a>,
latest_at_query: &'a LatestAtQuery,
) -> HybridLatestAtResults<'a>;

fn query_archetype_with_history<'a, A: re_types_core::Archetype>(
&'a self,
ctx: &'a ViewContext<'a>,
Expand All @@ -234,14 +240,30 @@ impl DataResultQuery for DataResult {
ctx: &'a ViewContext<'a>,
latest_at_query: &'a LatestAtQuery,
) -> HybridLatestAtResults<'a> {
let query_shadowed_defaults = false;
let query_shadowed_components = false;
latest_at_with_blueprint_resolved_data(
ctx,
None,
latest_at_query,
self,
A::all_components().iter().map(|descr| descr.component_name),
query_shadowed_defaults,
query_shadowed_components,
)
}

fn latest_at_with_blueprint_resolved_data_for_component<'a, C: re_types_core::Component>(
&'a self,
ctx: &'a ViewContext<'a>,
latest_at_query: &'a LatestAtQuery,
) -> HybridLatestAtResults<'a> {
let query_shadowed_components = false;
latest_at_with_blueprint_resolved_data(
ctx,
None,
latest_at_query,
self,
std::iter::once(C::name()),
query_shadowed_components,
)
}

Expand Down
110 changes: 51 additions & 59 deletions crates/viewer/re_view_spatial/src/contexts/transform_context.rs
Original file line number Diff line number Diff line change
@@ -1,25 +1,20 @@
use itertools::Either;
use nohash_hasher::IntMap;

use re_chunk_store::LatestAtQuery;
use re_entity_db::{EntityDb, EntityPath, EntityTree};
use re_types::{
archetypes::{InstancePoses3D, Pinhole, Transform3D},
components::{
DisconnectedSpace, ImagePlaneDistance, PinholeProjection, PoseRotationAxisAngle,
PoseRotationQuat, PoseScale3D, PoseTransformMat3x3, PoseTranslation3D, ViewCoordinates,
},
archetypes::{InstancePoses3D, Transform3D},
components::{DisconnectedSpace, ImagePlaneDistance, PinholeProjection},
Archetype, Component as _, ComponentNameSet,
};
use re_view::DataResultQuery as _;
use re_viewer_context::{IdentifiedViewSystem, ViewContext, ViewContextSystem};
use re_viewer_context::{DataResultTree, IdentifiedViewSystem, ViewContext, ViewContextSystem};
use vec1::smallvec_v1::SmallVec1;

use crate::{
transform_cache::{
CachedTransformsPerTimeline, ResolvedPinholeProjection, TransformCacheStoreSubscriber,
},
transform_component_tracker::TransformComponentTrackerStoreSubscriber,
visualizers::image_view_coordinates,
};

Expand Down Expand Up @@ -190,6 +185,8 @@ impl ViewContextSystem for TransformContext {
debug_assert_transform_field_order(ctx.viewer_ctx.reflection);

let entity_tree = ctx.recording().tree();
let query_result = ctx.viewer_ctx.lookup_query_result(query.view_id);
let data_result_tree = &query_result.tree;

self.space_origin = query.space_origin.clone();

Expand All @@ -212,21 +209,25 @@ impl ViewContextSystem for TransformContext {
};

// Child transforms of this space
self.gather_descendants_transforms(
ctx,
query,
current_tree,
ctx.recording(),
&time_query,
// Ignore potential pinhole camera at the root of the view, since it regarded as being "above" this root.
TransformInfo::default(),
transforms_per_timeline,
);
{
re_tracing::profile_scope!("gather_descendants_transforms");

self.gather_descendants_transforms(
ctx,
data_result_tree,
current_tree,
ctx.recording(),
&time_query,
// Ignore potential pinhole camera at the root of the view, since it regarded as being "above" this root.
TransformInfo::default(),
transforms_per_timeline,
);
}

// Walk up from the reference to the highest reachable parent.
self.gather_parent_transforms(
ctx,
query,
data_result_tree,
current_tree,
&time_query,
transforms_per_timeline,
Expand All @@ -245,7 +246,7 @@ impl TransformContext {
fn gather_parent_transforms<'a>(
&mut self,
ctx: &'a ViewContext<'a>,
query: &re_viewer_context::ViewQuery<'_>,
data_result_tree: &DataResultTree,
mut current_tree: &'a EntityTree,
time_query: &LatestAtQuery,
transforms_per_timeline: &mut CachedTransformsPerTimeline,
Expand All @@ -260,9 +261,8 @@ impl TransformContext {
let Some(parent_tree) = entity_tree.subtree(&parent_path) else {
// Unlike not having the space path in the hierarchy, this should be impossible.
re_log::error_once!(
"Path {} is not part of the global entity tree whereas its child {} is",
parent_path,
query.space_origin
"Path {} is not part of the global entity tree whereas its child is",
parent_path
);
return;
};
Expand Down Expand Up @@ -295,7 +295,7 @@ impl TransformContext {
// (this skips over everything at and under `current_tree` automatically)
self.gather_descendants_transforms(
ctx,
query,
data_result_tree,
parent_tree,
ctx.recording(),
time_query,
Expand All @@ -311,7 +311,7 @@ impl TransformContext {
fn gather_descendants_transforms(
&mut self,
ctx: &ViewContext<'_>,
view_query: &re_viewer_context::ViewQuery<'_>,
data_result_tree: &DataResultTree,
subtree: &EntityTree,
entity_db: &EntityDb,
query: &LatestAtQuery,
Expand All @@ -332,22 +332,8 @@ impl TransformContext {
for child_tree in subtree.children.values() {
let child_path = &child_tree.path;

let lookup_image_plane = |p: &_| {
let query_result = ctx.viewer_ctx.lookup_query_result(view_query.view_id);

query_result
.tree
.lookup_result_by_path(p)
.cloned()
.map(|data_result| {
let results = data_result
.latest_at_with_blueprint_resolved_data::<Pinhole>(ctx, query);

results.get_mono_with_fallback::<ImagePlaneDistance>()
})
.unwrap_or_default()
.into()
};
let lookup_image_plane =
|p: &_| lookup_image_plane_distance(ctx, data_result_tree, p, query);

let mut encountered_pinhole = twod_in_threed_info
.as_ref()
Expand Down Expand Up @@ -376,7 +362,7 @@ impl TransformContext {

self.gather_descendants_transforms(
ctx,
view_query,
data_result_tree,
child_tree,
entity_db,
query,
Expand All @@ -398,6 +384,28 @@ impl TransformContext {
}
}

fn lookup_image_plane_distance(
ctx: &ViewContext<'_>,
data_result_tree: &DataResultTree,
entity_path: &EntityPath,
query: &LatestAtQuery,
) -> f32 {
re_tracing::profile_function!();

data_result_tree
.lookup_result_by_path(entity_path)
.cloned()
.map(|data_result| {
data_result
.latest_at_with_blueprint_resolved_data_for_component::<ImagePlaneDistance>(
ctx, query,
)
.get_mono_with_fallback::<ImagePlaneDistance>()
})
.unwrap_or_default()
.into()
}

/// Compute transform info for when we walk up the tree from the reference.
fn transform_info_for_upward_propagation(
reference_from_ancestor: glam::Affine3A,
Expand Down Expand Up @@ -622,18 +630,7 @@ fn transforms_at(
) -> Result<TransformsAtEntity, UnreachableTransformReason> {
// This is called very frequently, don't put a profile scope here.

let potential_transform_components =
TransformComponentTrackerStoreSubscriber::access(&entity_db.store_id(), |tracker| {
tracker.potential_transform_components(entity_path).cloned()
})
.flatten()
.unwrap_or_default();

// TODO: That's a lot of locking.
// TODO: pose transforms?

let Some(entity_transforms) = transforms_per_timeline.entity_transforms(entity_path) else {
// TODO: this skips over disconnected.
return Ok(TransformsAtEntity::default());
};

Expand Down Expand Up @@ -677,12 +674,7 @@ fn transforms_at(
.instance_from_pinhole_image_plane
.is_none();

if no_other_transforms
&& potential_transform_components.disconnected_space
&& entity_db
.latest_at_component::<DisconnectedSpace>(entity_path, query)
.map_or(false, |(_index, res)| **res)
{
if no_other_transforms && entity_transforms.disconnected_space() {
Err(UnreachableTransformReason::DisconnectedSpace)
} else {
Ok(transforms_at_entity)
Expand Down
1 change: 0 additions & 1 deletion crates/viewer/re_view_spatial/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ mod proc_mesh;
mod scene_bounding_boxes;
mod space_camera_3d;
mod spatial_topology;
mod transform_component_tracker;
mod ui;
mod ui_2d;
mod ui_3d;
Expand Down
39 changes: 32 additions & 7 deletions crates/viewer/re_view_spatial/src/transform_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,12 @@ pub struct CachedTransformsPerTimeline {
pub struct PerTimelinePerEntityTransforms {
timeline: Timeline,
entity_path: EntityPath,

// TODO: some of these are exceedingly rare. do we need all that memory?
tree_transforms: BTreeMap<TimeInt, CacheEntry<glam::Affine3A>>,
pose_transforms: BTreeMap<TimeInt, CacheEntry<Vec<glam::Affine3A>>>,
pinhole_projections: BTreeMap<TimeInt, CacheEntry<ResolvedPinholeProjection>>,
disconnected_space: bool,
}

enum CacheEntry<T> {
Expand Down Expand Up @@ -181,6 +184,11 @@ impl PerTimelinePerEntityTransforms {
CacheEntry::None => None,
}
}

#[inline]
pub fn disconnected_space(&self) -> bool {
self.disconnected_space
}
}

impl TransformCacheStoreSubscriber {
Expand Down Expand Up @@ -220,16 +228,17 @@ impl PerStoreChunkSubscriber for TransformCacheStoreSubscriber {
re_tracing::profile_function!();

for event in events {
// TODO:???
// if event.compacted.is_some() {
// // Compactions don't change the data.
// continue;
// }
if event.compacted.is_some() {
// Compactions don't change the data.
continue;
}
if event.kind == re_chunk_store::ChunkStoreDiffKind::Deletion {
// Not participating in GC for now.
continue;
}

// TODO: do a quicker check for nothing happening.

let has_tree_transforms = event
.chunk
.component_names()
Expand All @@ -238,14 +247,22 @@ impl PerStoreChunkSubscriber for TransformCacheStoreSubscriber {
.chunk
.component_names()
.any(|component_name| self.pose_components.contains(&component_name));

let has_pinhole_or_view_coordinates =
event.chunk.component_names().any(|component_name| {
component_name == components::PinholeProjection::name()
|| component_name == components::ViewCoordinates::name()
});
// TODO(#7868): Disconnected space should be deprecated.
let has_disconnected_space = event
.chunk
.component_names()
.any(|component_name| component_name == components::DisconnectedSpace::name());

if !has_instance_poses && !has_tree_transforms && !has_pinhole_or_view_coordinates {
if !has_instance_poses
&& !has_tree_transforms
&& !has_pinhole_or_view_coordinates
&& !has_disconnected_space
{
continue;
}

Expand All @@ -270,8 +287,16 @@ impl PerStoreChunkSubscriber for TransformCacheStoreSubscriber {
tree_transforms: Default::default(),
pose_transforms: Default::default(),
pinhole_projections: Default::default(),
disconnected_space: has_disconnected_space,
});

// TODO: this still isn't quite right. Need to check if this was set to true?
// TODO(#7868): Disconnected space should be deprecated.
// TODO(#7121): Once a space is disconnected, it can't be re-connected again.
if has_disconnected_space {
per_entity.disconnected_space = true;
}

// Cache lazily since all of these require complex latest-at queries that...
// - we don't want to do more often than needed
// - would require a lot more context (we could inject that here, but it's not entirely straight forward)
Expand Down
Loading

0 comments on commit f170086

Please sign in to comment.