diff --git a/crates/re_space_view/src/data_query.rs b/crates/re_space_view/src/data_query.rs index 107752355439..6413f382abe1 100644 --- a/crates/re_space_view/src/data_query.rs +++ b/crates/re_space_view/src/data_query.rs @@ -1,7 +1,7 @@ use re_data_store::{EntityProperties, EntityPropertyMap}; use re_viewer_context::{DataQueryResult, EntitiesPerSystem, StoreContext}; -pub struct EntityOverrides { +pub struct EntityOverrideContext { pub root: EntityProperties, pub individual: EntityPropertyMap, pub group: EntityPropertyMap, @@ -12,23 +12,9 @@ pub struct EntityOverrides { /// The `SpaceViewBlueprint` is the only thing that likely implements this today /// but we use a trait here so we don't have to pick up a full dependency on `re_viewport`. pub trait PropertyResolver { - fn resolve_entity_overrides(&self, ctx: &StoreContext<'_>) -> EntityOverrides; + fn update_overrides(&self, ctx: &StoreContext<'_>, query_result: &mut DataQueryResult); } -pub struct NoopResolver {} - -impl PropertyResolver for NoopResolver { - fn resolve_entity_overrides(&self, _ctx: &StoreContext<'_>) -> EntityOverrides { - EntityOverrides { - root: EntityProperties::default(), - individual: EntityPropertyMap::default(), - group: EntityPropertyMap::default(), - } - } -} - -pub static NOOP_RESOLVER: NoopResolver = NoopResolver {}; - /// The common trait implemented for data queries /// /// Both interfaces return [`re_viewer_context::DataResult`]s, which are self-contained description of the data @@ -41,7 +27,6 @@ pub trait DataQuery { /// This is used when building up the contents for a `SpaceView`. fn execute_query( &self, - property_resolver: &impl PropertyResolver, ctx: &StoreContext<'_>, entities_per_system: &EntitiesPerSystem, ) -> DataQueryResult; diff --git a/crates/re_space_view/src/data_query_blueprint.rs b/crates/re_space_view/src/data_query_blueprint.rs index 98e22070e2b6..c8d75932b919 100644 --- a/crates/re_space_view/src/data_query_blueprint.rs +++ b/crates/re_space_view/src/data_query_blueprint.rs @@ -6,14 +6,14 @@ use re_log_types::{DataRow, EntityPath, EntityPathExpr, RowId, TimePoint}; use re_types_core::archetypes::Clear; use re_viewer_context::{ DataQueryId, DataQueryResult, DataResult, DataResultHandle, DataResultNode, DataResultTree, - EntitiesPerSystem, SpaceViewClassIdentifier, SpaceViewId, StoreContext, SystemCommand, - SystemCommandSender as _, ViewerContext, + EntitiesPerSystem, PropertyOverrides, SpaceViewClassIdentifier, SpaceViewId, StoreContext, + SystemCommand, SystemCommandSender as _, ViewerContext, }; use slotmap::SlotMap; use smallvec::SmallVec; use crate::{ - blueprint::components::QueryExpressions, DataQuery, EntityOverrides, PropertyResolver, + blueprint::components::QueryExpressions, DataQuery, EntityOverrideContext, PropertyResolver, }; /// An implementation of [`DataQuery`] that is built from a collection of [`QueryExpressions`] @@ -51,9 +51,6 @@ impl DataQueryBlueprint { } impl DataQueryBlueprint { - pub const INDIVIDUAL_OVERRIDES_PREFIX: &'static str = "individual_overrides"; - pub const RECURSIVE_OVERRIDES_PREFIX: &'static str = "recursive_overrides"; - /// Creates a new [`DataQueryBlueprint`]. /// /// This [`DataQueryBlueprint`] is ephemeral. It must be saved by calling @@ -116,17 +113,16 @@ impl DataQueryBlueprint { container: SpaceViewId, auto_properties: &'a EntityPropertyMap, ) -> DataQueryPropertyResolver<'a> { + let base_override_root = self.id.as_entity_path().clone(); + let individual_override_root = + base_override_root.join(&DataResult::INDIVIDUAL_OVERRIDES_PREFIX.into()); + let recursive_override_root = + base_override_root.join(&DataResult::RECURSIVE_OVERRIDES_PREFIX.into()); DataQueryPropertyResolver { auto_properties, default_stack: vec![container.as_entity_path(), self.id.as_entity_path()], - individual_override_root: self - .id - .as_entity_path() - .join(&Self::INDIVIDUAL_OVERRIDES_PREFIX.into()), - recursive_override_root: self - .id - .as_entity_path() - .join(&Self::RECURSIVE_OVERRIDES_PREFIX.into()), + individual_override_root, + recursive_override_root, } } @@ -275,9 +271,13 @@ impl DataQueryBlueprint { } impl DataQuery for DataQueryBlueprint { + /// Build up the initial [`DataQueryResult`] for this [`DataQueryBlueprint`] + /// + /// Note that this result will not have any resolved [`PropertyOverrides`]. Those can + /// be added by separately calling [`PropertyResolver::update_overrides`] on + /// the result. fn execute_query( &self, - property_resolver: &impl PropertyResolver, ctx: &re_viewer_context::StoreContext<'_>, entities_per_system: &EntitiesPerSystem, ) -> DataQueryResult { @@ -285,15 +285,11 @@ impl DataQuery for DataQueryBlueprint { let mut data_results = SlotMap::::default(); - let overrides = property_resolver.resolve_entity_overrides(ctx); - let executor = QueryExpressionEvaluator::new(self, entities_per_system); let root_handle = ctx.recording.and_then(|store| { executor.add_entity_tree_to_data_results_recursive( store.tree(), - &overrides, - &overrides.root, &mut data_results, false, ) @@ -312,7 +308,6 @@ impl DataQuery for DataQueryBlueprint { /// used to efficiently determine if we should continue the walk or switch /// to a pure recursive evaluation. struct QueryExpressionEvaluator<'a> { - blueprint: &'a DataQueryBlueprint, per_system_entity_list: &'a EntitiesPerSystem, exact_inclusions: IntSet, recursive_inclusions: IntSet, @@ -326,6 +321,7 @@ impl<'a> QueryExpressionEvaluator<'a> { blueprint: &'a DataQueryBlueprint, per_system_entity_list: &'a EntitiesPerSystem, ) -> Self { + re_tracing::profile_function!(); let inclusions: Vec = blueprint .expressions .inclusions @@ -368,7 +364,6 @@ impl<'a> QueryExpressionEvaluator<'a> { .collect(); Self { - blueprint, per_system_entity_list, exact_inclusions, recursive_inclusions, @@ -381,8 +376,6 @@ impl<'a> QueryExpressionEvaluator<'a> { fn add_entity_tree_to_data_results_recursive( &self, tree: &EntityTree, - overrides: &EntityOverrides, - inherited: &EntityProperties, data_results: &mut SlotMap, from_recursive: bool, ) -> Option { @@ -396,12 +389,12 @@ impl<'a> QueryExpressionEvaluator<'a> { return None; } - let entity_path = tree.path.clone(); + let entity_path = &tree.path; // Pre-compute our matches - let exact_include = self.exact_inclusions.contains(&entity_path); - let recursive_include = self.recursive_inclusions.contains(&entity_path) || from_recursive; - let exact_exclude = self.exact_exclusions.contains(&entity_path); + let exact_include = self.exact_inclusions.contains(entity_path); + let recursive_include = self.recursive_inclusions.contains(entity_path) || from_recursive; + let exact_exclude = self.exact_exclusions.contains(entity_path); let any_match = (exact_include || recursive_include) && !exact_exclude; // Only populate view_parts if this is a match @@ -410,7 +403,7 @@ impl<'a> QueryExpressionEvaluator<'a> { self.per_system_entity_list .iter() .filter_map(|(part, ents)| { - if ents.contains(&entity_path) { + if ents.contains(entity_path) { Some(*part) } else { None @@ -421,37 +414,14 @@ impl<'a> QueryExpressionEvaluator<'a> { Default::default() }; - let mut resolved_properties = inherited.clone(); - - if let Some(props) = overrides.group.get_opt(&entity_path) { - resolved_properties = resolved_properties.with_child(props); - } - - let base_entity_path = self.blueprint.id.as_entity_path().clone(); - - let individual_override_path = base_entity_path - .join(&DataQueryBlueprint::INDIVIDUAL_OVERRIDES_PREFIX.into()) - .join(&entity_path); - let recursive_override_path = base_entity_path - .join(&DataQueryBlueprint::RECURSIVE_OVERRIDES_PREFIX.into()) - .join(&entity_path); - let self_leaf = if !view_parts.is_empty() || exact_include { - let individual_props = overrides.individual.get_opt(&entity_path); - let mut leaf_resolved_properties = resolved_properties.clone(); - - if let Some(props) = individual_props { - leaf_resolved_properties = leaf_resolved_properties.with_child(props); - } Some(data_results.insert(DataResultNode { data_result: DataResult { entity_path: entity_path.clone(), view_parts, is_group: false, direct_included: any_match, - individual_properties: overrides.individual.get_opt(&entity_path).cloned(), - resolved_properties: leaf_resolved_properties, - override_path: individual_override_path, + property_overrides: None, }, children: Default::default(), })) @@ -465,33 +435,49 @@ impl<'a> QueryExpressionEvaluator<'a> { itertools::Either::Right(std::iter::empty()) }; - let children: SmallVec<_> = maybe_self_iter - .chain(tree.children.values().filter_map(|subtree| { - self.add_entity_tree_to_data_results_recursive( - subtree, - overrides, - &resolved_properties, - data_results, - recursive_include, // Once we have hit a recursive match, it's always propagated - ) - })) - .collect(); + let interesting_children: Vec<_> = { + if recursive_include { + tree.children + .values() + .filter(|subtree| { + !(self.recursive_exclusions.contains(&subtree.path) + || !(recursive_include + || self.allowed_prefixes.contains(&subtree.path))) + }) + .collect() + } else { + self.allowed_prefixes + .iter() + .filter(|prefix| prefix.is_child_of(entity_path)) + .filter_map(|prefix| prefix.last().and_then(|part| tree.children.get(part))) + .collect() + } + }; + + let children: SmallVec<_> = { + maybe_self_iter + .chain(interesting_children.iter().filter_map(|subtree| { + self.add_entity_tree_to_data_results_recursive( + subtree, + data_results, + recursive_include, // Once we have hit a recursive match, it's always propagated + ) + })) + .collect() + }; // If the only child is the self-leaf, then we don't need to create a group if children.is_empty() || children.len() == 1 && self_leaf.is_some() { self_leaf } else { // The 'individual' properties of a group are the group overrides - let individual_properties = overrides.group.get_opt(&entity_path).cloned(); Some(data_results.insert(DataResultNode { data_result: DataResult { - entity_path, + entity_path: entity_path.clone(), view_parts: Default::default(), is_group: true, direct_included: any_match, - individual_properties, - resolved_properties, - override_path: recursive_override_path, + property_overrides: None, }, children, })) @@ -507,72 +493,146 @@ pub struct DataQueryPropertyResolver<'a> { } impl DataQueryPropertyResolver<'_> { + /// Helper function to build the [`EntityOverrideContext`] for this [`DataQuery`] + /// + /// The context is made up of 3 parts: + /// - The root properties are build by merging a stack of paths from the Blueprint Tree. This + /// may include properties from the `SpaceView` or `DataQuery`. + /// - The individual overrides are found by walking an override subtree under the `data_query//individual_overrides` + /// - The group overrides are found by walking an override subtree under the `data_query//group_overrides` + fn build_override_context(&self, ctx: &StoreContext<'_>) -> EntityOverrideContext { + re_tracing::profile_function!(); + + let mut root: EntityProperties = Default::default(); + for prefix in &self.default_stack { + if let Some(overrides) = ctx + .blueprint + .store() + .query_timeless_component::(prefix) + { + root = root.with_child(&overrides.value.0); + } + } + + EntityOverrideContext { + root, + individual: self.resolve_entity_overrides_for_path(ctx, &self.individual_override_root), + group: self.resolve_entity_overrides_for_path(ctx, &self.recursive_override_root), + } + } + + /// Find all of the overrides for a given path. + /// + /// These overrides are full entity-paths prefixed by the override root. + /// + /// For example the individual override for `world/points` in the context of the query-id `1234` + /// would be found at: `data_query/1234/individual_overrides/world/points`. fn resolve_entity_overrides_for_path( &self, ctx: &StoreContext<'_>, - props_path: &EntityPath, + override_root: &EntityPath, ) -> EntityPropertyMap { re_tracing::profile_function!(); let blueprint = ctx.blueprint; let mut prop_map = self.auto_properties.clone(); - if let Some(tree) = blueprint.tree().subtree(props_path) { + if let Some(tree) = blueprint.tree().subtree(override_root) { tree.visit_children_recursively(&mut |path: &EntityPath| { if let Some(props) = blueprint .store() .query_timeless_component_quiet::(path) { let overridden_path = - EntityPath::from(&path.as_slice()[props_path.len()..path.len()]); + EntityPath::from(&path.as_slice()[override_root.len()..path.len()]); prop_map.update(overridden_path, props.value.0); } }); } prop_map } -} -impl<'a> PropertyResolver for DataQueryPropertyResolver<'a> { - /// Helper function to lookup the properties for a given entity path. + /// Recursively walk the [`DataResultTree`] and update the [`PropertyOverrides`] for each node. /// - /// We start with the auto properties for the `SpaceView` as the base layer and - /// then incrementally override from there. - fn resolve_entity_overrides(&self, ctx: &StoreContext<'_>) -> EntityOverrides { - re_tracing::profile_function!(); - let blueprint = ctx.blueprint; - - let mut root: EntityProperties = Default::default(); - for prefix in &self.default_stack { - if let Some(overrides) = ctx - .blueprint - .store() - .query_timeless_component::(prefix) - { - root = root.with_child(&overrides.value.0); - } - } - - let mut individual = self.auto_properties.clone(); + /// This will accumulate the group properties at each step down the tree, and then finally merge + /// with individual overrides at the leafs. + fn update_overrides_recursive( + &self, + query_result: &mut DataQueryResult, + override_context: &EntityOverrideContext, + accumulated: &EntityProperties, + handle: DataResultHandle, + ) { + if let Some((child_handles, accumulated)) = + query_result.tree.lookup_node_mut(handle).and_then(|node| { + if node.data_result.is_group { + let overridden_properties = override_context + .group + .get_opt(&node.data_result.entity_path); + + let accumulated_properties = if let Some(overridden) = overridden_properties { + accumulated.with_child(overridden) + } else { + accumulated.clone() + }; + + node.data_result.property_overrides = Some(PropertyOverrides { + individual_properties: overridden_properties.cloned(), + accumulated_properties: accumulated_properties.clone(), + override_path: self + .recursive_override_root + .join(&node.data_result.entity_path), + }); + + Some((node.children.clone(), accumulated_properties)) + } else { + let overridden_properties = override_context + .individual + .get_opt(&node.data_result.entity_path); - if let Some(tree) = blueprint.tree().subtree(&self.individual_override_root) { - tree.visit_children_recursively(&mut |path: &EntityPath| { - if let Some(props) = blueprint - .store() - .query_timeless_component::(path) - { - let overridden_path = EntityPath::from( - &path.as_slice()[self.individual_override_root.len()..path.len()], - ); - individual.update(overridden_path, props.value.0); + let accumulated_properties = if let Some(overridden) = overridden_properties { + accumulated.with_child(overridden) + } else { + accumulated.clone() + }; + + node.data_result.property_overrides = Some(PropertyOverrides { + individual_properties: overridden_properties.cloned(), + accumulated_properties: accumulated_properties.clone(), + override_path: self + .individual_override_root + .join(&node.data_result.entity_path), + }); + + None } - }); + }) + { + for child in child_handles { + self.update_overrides_recursive( + query_result, + override_context, + &accumulated, + child, + ); + } } + } +} - EntityOverrides { - root, - individual: self.resolve_entity_overrides_for_path(ctx, &self.individual_override_root), - group: self.resolve_entity_overrides_for_path(ctx, &self.recursive_override_root), +impl<'a> PropertyResolver for DataQueryPropertyResolver<'a> { + /// Recursively walk the [`DataResultTree`] and update the [`PropertyOverrides`] for each node. + fn update_overrides(&self, ctx: &StoreContext<'_>, query_result: &mut DataQueryResult) { + re_tracing::profile_function!(); + let entity_overrides = self.build_override_context(ctx); + + if let Some(root) = query_result.tree.root_handle() { + self.update_overrides_recursive( + query_result, + &entity_overrides, + &entity_overrides.root, + root, + ); } } } @@ -586,28 +646,11 @@ mod tests { use super::*; - struct StaticPropertyResolver { - overrides: EntityPropertyMap, - } - - impl PropertyResolver for StaticPropertyResolver { - fn resolve_entity_overrides(&self, _ctx: &StoreContext<'_>) -> EntityOverrides { - EntityOverrides { - root: Default::default(), - individual: self.overrides.clone(), - group: self.overrides.clone(), - } - } - } - #[test] fn test_query_results() { let mut recording = StoreDb::new(StoreId::random(re_log_types::StoreKind::Recording)); let blueprint = StoreDb::new(StoreId::random(re_log_types::StoreKind::Blueprint)); - let overrides = EntityPropertyMap::default(); - let resolver = StaticPropertyResolver { overrides }; - let timeline_frame = Timeline::new_sequence("frame"); let timepoint = TimePoint::from_iter([(timeline_frame, 10.into())]); @@ -750,7 +793,7 @@ mod tests { ), }; - let query_result = query.execute_query(&resolver, &ctx, &entities_per_system); + let query_result = query.execute_query(&ctx, &entities_per_system); let mut visited = vec![]; query_result.tree.visit(&mut |handle| { diff --git a/crates/re_space_view/src/lib.rs b/crates/re_space_view/src/lib.rs index af4d0b2efbb0..06724825d8e4 100644 --- a/crates/re_space_view/src/lib.rs +++ b/crates/re_space_view/src/lib.rs @@ -9,7 +9,7 @@ mod data_query_blueprint; mod screenshot; mod unreachable_transform_reason; -pub use data_query::{DataQuery, EntityOverrides, PropertyResolver, NOOP_RESOLVER}; +pub use data_query::{DataQuery, EntityOverrideContext, PropertyResolver}; pub use data_query_blueprint::DataQueryBlueprint; pub use screenshot::ScreenshotMode; pub use unreachable_transform_reason::UnreachableTransformReason; diff --git a/crates/re_space_view_dataframe/src/space_view_class.rs b/crates/re_space_view_dataframe/src/space_view_class.rs index 868512bb3eae..2aafa002ef41 100644 --- a/crates/re_space_view_dataframe/src/space_view_class.rs +++ b/crates/re_space_view_dataframe/src/space_view_class.rs @@ -87,7 +87,7 @@ impl SpaceViewClass for DataframeSpaceView { // These are the entity paths whose content we must display. let sorted_entity_paths: BTreeSet<_> = query .iter_all_data_results() - .filter(|data_result| data_result.resolved_properties.visible) + .filter(|data_result| data_result.accumulated_properties().visible) .map(|data_result| &data_result.entity_path) .cloned() .collect(); diff --git a/crates/re_space_view_spatial/src/contexts/non_interactive_entities.rs b/crates/re_space_view_spatial/src/contexts/non_interactive_entities.rs index f08c93dbc4d8..d1842b1a99e6 100644 --- a/crates/re_space_view_spatial/src/contexts/non_interactive_entities.rs +++ b/crates/re_space_view_spatial/src/contexts/non_interactive_entities.rs @@ -28,7 +28,7 @@ impl ViewContextSystem for NonInteractiveEntities { self.0 = query .iter_all_data_results() .filter_map(|data_result| { - if data_result.resolved_properties.interactive { + if data_result.accumulated_properties().interactive { None } else { Some(data_result.entity_path.hash()) diff --git a/crates/re_space_view_spatial/src/contexts/transform_context.rs b/crates/re_space_view_spatial/src/contexts/transform_context.rs index 34b81319d1b6..0a9213129da5 100644 --- a/crates/re_space_view_spatial/src/contexts/transform_context.rs +++ b/crates/re_space_view_spatial/src/contexts/transform_context.rs @@ -99,7 +99,7 @@ impl ViewContextSystem for TransformContext { .map(|results| { results .iter() - .map(|r| (r.entity_path.clone(), r.resolved_properties.clone())) + .map(|r| (r.entity_path.clone(), r.accumulated_properties().clone())) .collect() }) .unwrap_or_default(); diff --git a/crates/re_space_view_spatial/src/parts/cameras.rs b/crates/re_space_view_spatial/src/parts/cameras.rs index 22b741bbf821..9cc726bffb53 100644 --- a/crates/re_space_view_spatial/src/parts/cameras.rs +++ b/crates/re_space_view_spatial/src/parts/cameras.rs @@ -216,7 +216,7 @@ impl ViewPartSystem for CamerasPart { transforms, shared_render_builders, &data_result.entity_path, - &data_result.resolved_properties, + data_result.accumulated_properties(), &pinhole, store .query_latest_component::( diff --git a/crates/re_space_view_spatial/src/parts/entity_iterator.rs b/crates/re_space_view_spatial/src/parts/entity_iterator.rs index 977755e196b8..027cf4d584bf 100644 --- a/crates/re_space_view_spatial/src/parts/entity_iterator.rs +++ b/crates/re_space_view_spatial/src/parts/entity_iterator.rs @@ -78,7 +78,7 @@ where ctx.store_db.store(), &query.timeline, &query.latest_at, - &data_result.resolved_properties.visible_history, + &data_result.accumulated_properties().visible_history, &data_result.entity_path, ) .and_then(|entity_views| { @@ -91,7 +91,7 @@ where fun( ctx, &data_result.entity_path, - &data_result.resolved_properties, + data_result.accumulated_properties(), ent_view, &entity_context, )?; diff --git a/crates/re_space_view_spatial/src/parts/transform3d_arrows.rs b/crates/re_space_view_spatial/src/parts/transform3d_arrows.rs index b373f8a562f1..31db5a5e3eef 100644 --- a/crates/re_space_view_spatial/src/parts/transform3d_arrows.rs +++ b/crates/re_space_view_spatial/src/parts/transform3d_arrows.rs @@ -67,7 +67,7 @@ impl ViewPartSystem for Transform3DArrowsPart { continue; } - if !*data_result.resolved_properties.transform_3d_visible { + if !*data_result.accumulated_properties().transform_3d_visible { continue; } @@ -91,7 +91,7 @@ impl ViewPartSystem for Transform3DArrowsPart { &mut line_builder, world_from_obj, Some(&data_result.entity_path), - *data_result.resolved_properties.transform_3d_size, + *data_result.accumulated_properties().transform_3d_size, query .highlights .entity_outline_mask(data_result.entity_path.hash()) diff --git a/crates/re_space_view_time_series/src/view_part_system.rs b/crates/re_space_view_time_series/src/view_part_system.rs index 671f235c1929..0d1fd78d1492 100644 --- a/crates/re_space_view_time_series/src/view_part_system.rs +++ b/crates/re_space_view_time_series/src/view_part_system.rs @@ -132,14 +132,17 @@ impl TimeSeriesSystem { let visible_history = match query.timeline.typ() { re_log_types::TimeType::Time => { - data_result.resolved_properties.visible_history.nanos + data_result.accumulated_properties().visible_history.nanos } re_log_types::TimeType::Sequence => { - data_result.resolved_properties.visible_history.sequences + data_result + .accumulated_properties() + .visible_history + .sequences } }; - let (from, to) = if data_result.resolved_properties.visible_history.enabled { + let (from, to) = if data_result.accumulated_properties().visible_history.enabled { ( visible_history.from(query.latest_at), visible_history.to(query.latest_at), diff --git a/crates/re_viewer/src/app_state.rs b/crates/re_viewer/src/app_state.rs index 26d5c15e3cd0..f0c491b10848 100644 --- a/crates/re_viewer/src/app_state.rs +++ b/crates/re_viewer/src/app_state.rs @@ -3,7 +3,7 @@ use ahash::HashMap; use re_data_store::StoreDb; use re_log_types::{LogMsg, StoreId, TimeRangeF}; use re_smart_channel::ReceiveSet; -use re_space_view::DataQuery as _; +use re_space_view::{DataQuery as _, PropertyResolver as _}; use re_viewer_context::{ AppOptions, Caches, CommandSender, ComponentUiRegistry, PlayState, RecordingConfig, SelectionState, SpaceViewClassRegistry, StoreContext, SystemCommandSender as _, ViewerContext, @@ -139,7 +139,7 @@ impl AppState { ); // Execute the queries for every `SpaceView` - let query_results = { + let mut query_results = { re_tracing::profile_scope!("query_results"); viewport .blueprint @@ -147,18 +147,12 @@ impl AppState { .values() .flat_map(|space_view| { space_view.queries.iter().filter_map(|query| { - let props = viewport.state.space_view_props(space_view.id); - let resolver = query.build_resolver(space_view.id, props); entities_per_system_per_class .get(&query.space_view_class_identifier) .map(|entities_per_system| { ( query.id, - query.execute_query( - &resolver, - store_context, - entities_per_system, - ), + query.execute_query(store_context, entities_per_system), ) }) }) @@ -166,7 +160,7 @@ impl AppState { .collect::<_>() }; - let mut ctx = ViewerContext { + let ctx = ViewerContext { app_options, cache, space_view_class_registry, @@ -188,36 +182,35 @@ impl AppState { viewport.on_frame_start(&ctx, &spaces_info); - // TODO(jleibs): Running the queries a second time is annoying, but we need - // to do this or else the auto_properties aren't right since they get populated - // in on_frame_start, but on_frame_start also needs the queries. - let updated_query_results = { + { re_tracing::profile_scope!("updated_query_results"); - viewport - .blueprint - .space_views - .values() - .flat_map(|space_view| { - space_view.queries.iter().filter_map(|query| { + for space_view in viewport.blueprint.space_views.values() { + for query in &space_view.queries { + if let Some(query_result) = query_results.get_mut(&query.id) { let props = viewport.state.space_view_props(space_view.id); let resolver = query.build_resolver(space_view.id, props); - entities_per_system_per_class - .get(&query.space_view_class_identifier) - .map(|entities_per_system| { - ( - query.id, - query.execute_query( - &resolver, - store_context, - entities_per_system, - ), - ) - }) - }) - }) - .collect::<_>() + resolver.update_overrides(store_context, query_result); + } + } + } + }; + + // TODO(jleibs): The need to rebuild this after updating the queries is kind of annoying, + // but it's just a bunch of refs so not really that big of a deal in practice. + let ctx = ViewerContext { + app_options, + cache, + space_view_class_registry, + component_ui_registry, + store_db, + store_context, + entities_per_system_per_class: &entities_per_system_per_class, + query_results: &query_results, + rec_cfg, + re_ui, + render_ctx, + command_sender, }; - ctx.query_results = &updated_query_results; time_panel.show_panel(&ctx, ui, app_blueprint.time_panel_expanded); selection_panel.show_panel( diff --git a/crates/re_viewer/src/ui/selection_panel.rs b/crates/re_viewer/src/ui/selection_panel.rs index 227656fa51b3..5479ddc7e021 100644 --- a/crates/re_viewer/src/ui/selection_panel.rs +++ b/crates/re_viewer/src/ui/selection_panel.rs @@ -603,8 +603,8 @@ fn blueprint_ui( let root_data_result = space_view.root_data_result(ctx.store_context); let mut props = root_data_result - .individual_properties - .clone() + .individual_properties() + .cloned() .unwrap_or(resolved_entity_props.clone()); let cursor = ui.cursor(); @@ -667,8 +667,8 @@ fn blueprint_ui( .cloned() { let mut props = data_result - .individual_properties - .clone() + .individual_properties() + .cloned() .unwrap_or_default(); entity_props_ui( ctx, @@ -676,7 +676,7 @@ fn blueprint_ui( &space_view_class, Some(entity_path), &mut props, - &data_result.resolved_properties, + data_result.accumulated_properties(), ); data_result.save_override(Some(props), ctx); } @@ -697,8 +697,8 @@ fn blueprint_ui( { let space_view_class = *space_view.class_identifier(); let mut props = data_result - .individual_properties - .clone() + .individual_properties() + .cloned() .unwrap_or_default(); entity_props_ui( @@ -707,7 +707,7 @@ fn blueprint_ui( &space_view_class, None, &mut props, - &data_result.resolved_properties, + data_result.accumulated_properties(), ); data_result.save_override(Some(props), ctx); } diff --git a/crates/re_viewer_context/src/lib.rs b/crates/re_viewer_context/src/lib.rs index da3045fd3afb..bb34fba38cf3 100644 --- a/crates/re_viewer_context/src/lib.rs +++ b/crates/re_viewer_context/src/lib.rs @@ -44,9 +44,9 @@ pub use selection_state::{ pub use space_view::{ default_heuristic_filter, AutoSpawnHeuristic, DataResult, DynSpaceViewClass, HeuristicFilterContext, IdentifiedViewSystem, PerSystemDataResults, PerSystemEntities, - SpaceViewClass, SpaceViewClassIdentifier, SpaceViewClassLayoutPriority, SpaceViewClassRegistry, - SpaceViewClassRegistryError, SpaceViewEntityHighlight, SpaceViewHighlights, - SpaceViewOutlineMasks, SpaceViewState, SpaceViewSystemExecutionError, + PropertyOverrides, SpaceViewClass, SpaceViewClassIdentifier, SpaceViewClassLayoutPriority, + SpaceViewClassRegistry, SpaceViewClassRegistryError, SpaceViewEntityHighlight, + SpaceViewHighlights, SpaceViewOutlineMasks, SpaceViewState, SpaceViewSystemExecutionError, SpaceViewSystemRegistrator, SystemExecutionOutput, ViewContextCollection, ViewContextSystem, ViewPartCollection, ViewPartSystem, ViewQuery, ViewSystemIdentifier, }; diff --git a/crates/re_viewer_context/src/query_context.rs b/crates/re_viewer_context/src/query_context.rs index 81180e51ec91..4caba9946e4b 100644 --- a/crates/re_viewer_context/src/query_context.rs +++ b/crates/re_viewer_context/src/query_context.rs @@ -136,6 +136,11 @@ impl DataResultTree { self.data_results.get(handle) } + /// Look up a [`DataResultNode`] in the tree based on its handle. + pub fn lookup_node_mut(&mut self, handle: DataResultHandle) -> Option<&mut DataResultNode> { + self.data_results.get_mut(handle) + } + /// Look up a [`DataResultNode`] in the tree based on an [`EntityPath`]. pub fn lookup_result_by_path_and_group( &self, diff --git a/crates/re_viewer_context/src/selection_state.rs b/crates/re_viewer_context/src/selection_state.rs index 5502da850cf7..c02d31f4dc04 100644 --- a/crates/re_viewer_context/src/selection_state.rs +++ b/crates/re_viewer_context/src/selection_state.rs @@ -117,7 +117,8 @@ pub struct SelectionState { impl SelectionState { /// Called at the start of each frame pub fn on_frame_start(&mut self, item_retain_condition: impl Fn(&Item) -> bool) { - re_tracing::profile_function!(); + // Use a different name so we don't get a collision in puffin. + re_tracing::profile_scope!("SelectionState::on_frame_start"); let history = self.history.get_mut(); history.retain(&item_retain_condition); diff --git a/crates/re_viewer_context/src/space_view/mod.rs b/crates/re_viewer_context/src/space_view/mod.rs index 2063e4efc492..d4a5385167c8 100644 --- a/crates/re_viewer_context/src/space_view/mod.rs +++ b/crates/re_viewer_context/src/space_view/mod.rs @@ -32,7 +32,7 @@ pub use view_context_system::{ViewContextCollection, ViewContextSystem}; pub use view_part_system::{ default_heuristic_filter, HeuristicFilterContext, ViewPartCollection, ViewPartSystem, }; -pub use view_query::{DataResult, PerSystemDataResults, ViewQuery}; +pub use view_query::{DataResult, PerSystemDataResults, PropertyOverrides, ViewQuery}; // --------------------------------------------------------------------------- diff --git a/crates/re_viewer_context/src/space_view/view_query.rs b/crates/re_viewer_context/src/space_view/view_query.rs index 737ac30e571c..5e787aabf281 100644 --- a/crates/re_viewer_context/src/space_view/view_query.rs +++ b/crates/re_viewer_context/src/space_view/view_query.rs @@ -1,6 +1,7 @@ use std::collections::BTreeMap; use itertools::Itertools; +use once_cell::sync::Lazy; use re_arrow_store::LatestAtQuery; use re_data_store::{EntityPath, EntityProperties, EntityPropertiesComponent, TimeInt, Timeline}; use re_log_types::{DataCell, DataRow, RowId, TimePoint}; @@ -12,12 +13,26 @@ use crate::{ ViewSystemIdentifier, ViewerContext, }; +#[derive(Clone, Debug, PartialEq)] +pub struct PropertyOverrides { + /// The accumulated properties (including any hierarchical flattening) to apply. + // TODO(jleibs): Eventually this goes away and becomes implicit as an override layer in the StoreView. + // For now, bundling this here acts as a good proxy for that future data-override mechanism. + pub accumulated_properties: EntityProperties, + + /// The individual property set in this `DataResult`, if any. + pub individual_properties: Option, + + /// `EntityPath` in the Blueprint store where updated overrides should be written back. + pub override_path: EntityPath, +} + /// This is the primary mechanism through which data is passed to a `SpaceView`. /// /// It contains everything necessary to properly use this data in the context of the /// `ViewSystem`s that it is a part of. /// -/// In the future `resolved_properties` will be replaced by a `StoreView` that contains +/// In the future `accumulated_properties` will be replaced by a `StoreView` that contains /// the relevant data overrides for the given query. #[derive(Clone, Debug, PartialEq)] pub struct DataResult { @@ -37,44 +52,59 @@ pub struct DataResult { // exists due to a common prefix. pub direct_included: bool, - /// The resolved properties (including any hierarchical flattening) to apply. - // TODO(jleibs): Eventually this goes away and becomes implicit as an override layer in the StoreView. - // For now, bundling this here acts as a good proxy for that future data-override mechanism. - pub resolved_properties: EntityProperties, - - /// The individual property set in this `DataResult`, if any. - pub individual_properties: Option, - - /// `EntityPath` in the Blueprint store where updated overrides should be written back. - pub override_path: EntityPath, + /// The accumulated property overrides for this `DataResult`. + pub property_overrides: Option, } +static DEFAULT_PROPS: Lazy = Lazy::::new(Default::default); + impl DataResult { + pub const INDIVIDUAL_OVERRIDES_PREFIX: &'static str = "individual_overrides"; + pub const RECURSIVE_OVERRIDES_PREFIX: &'static str = "recursive_overrides"; + + #[inline] + pub fn override_path(&self) -> Option<&EntityPath> { + self.property_overrides.as_ref().map(|p| &p.override_path) + } + /// Write the [`EntityProperties`] for this result back to the Blueprint store. pub fn save_override(&self, props: Option, ctx: &ViewerContext<'_>) { + // TODO(jleibs): Make it impossible for this to happen with different type structure + // This should never happen unless we're doing something with a partially processed + // query. + let Some(override_path) = self.override_path() else { + re_log::warn!( + "Tried to save override for {:?} but it has no override path", + self.entity_path + ); + return; + }; + + // This should never happen if the above didn't return early. + let Some(property_overrides) = &self.property_overrides else { + return; + }; + let cell = match props { None => { - if self.individual_properties.is_some() { - re_log::debug!("Clearing {:?}", self.override_path); + re_log::debug!("Clearing {:?}", override_path); - Some(DataCell::from_arrow_empty( - EntityPropertiesComponent::name(), - EntityPropertiesComponent::arrow_datatype(), - )) - } else { - None - } + Some(DataCell::from_arrow_empty( + EntityPropertiesComponent::name(), + EntityPropertiesComponent::arrow_datatype(), + )) } Some(props) => { // A value of `None` in the data store means "use the default value", so if // `self.individual_properties` is `None`, we only must save if `props` is different // from the default. if props.has_edits( - self.individual_properties + property_overrides + .individual_properties .as_ref() .unwrap_or(&EntityProperties::default()), ) { - re_log::debug!("Overriding {:?} with {:?}", self.override_path, props); + re_log::debug!("Overriding {:?} with {:?}", override_path, props); let component = EntityPropertiesComponent(props); @@ -91,7 +121,7 @@ impl DataResult { let row = DataRow::from_cells1_sized( RowId::new(), - self.override_path.clone(), + override_path.clone(), TimePoint::timeless(), 1, cell, @@ -104,6 +134,29 @@ impl DataResult { vec![row], )); } + + #[inline] + pub fn accumulated_properties(&self) -> &EntityProperties { + // TODO(jleibs): Make it impossible for this to happen with different type structure + // This should never happen unless we're doing something with a partially processed + // query. + let Some(property_overrides) = &self.property_overrides else { + re_log::warn!( + "Tried to get accumulated properties for {:?} but it has no property overrides", + self.entity_path + ); + return &DEFAULT_PROPS; + }; + + &property_overrides.accumulated_properties + } + + #[inline] + pub fn individual_properties(&self) -> Option<&EntityProperties> { + self.property_overrides + .as_ref() + .and_then(|p| p.individual_properties.as_ref()) + } } pub type PerSystemDataResults<'a> = BTreeMap>; @@ -144,7 +197,7 @@ impl<'s> ViewQuery<'s> { itertools::Either::Right( results .iter() - .filter(|result| result.resolved_properties.visible) + .filter(|result| result.accumulated_properties().visible) .copied(), ) }, diff --git a/crates/re_viewport/src/space_view.rs b/crates/re_viewport/src/space_view.rs index 306d135b2e4c..fa71a3319a41 100644 --- a/crates/re_viewport/src/space_view.rs +++ b/crates/re_viewport/src/space_view.rs @@ -11,9 +11,9 @@ use re_types::blueprint::components::{EntitiesDeterminedByUser, Name, SpaceViewO use re_types_core::archetypes::Clear; use re_viewer_context::{ DataQueryId, DataResult, DynSpaceViewClass, PerSystemDataResults, PerSystemEntities, - SpaceViewClass, SpaceViewClassIdentifier, SpaceViewHighlights, SpaceViewId, SpaceViewState, - StoreContext, SystemCommand, SystemCommandSender as _, SystemExecutionOutput, ViewQuery, - ViewerContext, + PropertyOverrides, SpaceViewClass, SpaceViewClassIdentifier, SpaceViewHighlights, SpaceViewId, + SpaceViewState, StoreContext, SystemCommand, SystemCommandSender as _, SystemExecutionOutput, + ViewQuery, ViewerContext, }; use crate::system_execution::create_and_run_space_view_systems; @@ -372,8 +372,8 @@ impl SpaceViewBlueprint { let root_data_result = self.root_data_result(ctx.store_context); let props = root_data_result - .individual_properties - .clone() + .individual_properties() + .cloned() .unwrap_or_default(); ui.scope(|ui| { @@ -403,7 +403,7 @@ impl SpaceViewBlueprint { .query_timeless_component_quiet::(&self.entity_path()) .map(|result| result.value.0); - let resolved_properties = individual_properties.clone().unwrap_or_else(|| { + let accumulated_properties = individual_properties.clone().unwrap_or_else(|| { let mut props = EntityProperties::default(); // better defaults for the time series space view // TODO(#4194, jleibs, ab): Per-space-view-class property defaults should be factored in @@ -419,9 +419,11 @@ impl SpaceViewBlueprint { view_parts: Default::default(), is_group: true, direct_included: true, - resolved_properties, - individual_properties, - override_path: entity_path, + property_overrides: Some(PropertyOverrides { + accumulated_properties, + individual_properties, + override_path: entity_path, + }), } } @@ -461,7 +463,7 @@ impl SpaceViewBlueprint { mod tests { use re_data_store::StoreDb; use re_log_types::{DataCell, DataRow, RowId, StoreId, TimePoint}; - use re_space_view::DataQuery as _; + use re_space_view::{DataQuery as _, PropertyResolver as _}; use re_types::archetypes::Points3D; use re_viewer_context::{EntitiesPerSystem, StoreContext}; @@ -539,7 +541,8 @@ mod tests { all_recordings: vec![], }; - let query_result = query.execute_query(&resolver, &ctx, &entities_per_system); + let mut query_result = query.execute_query(&ctx, &entities_per_system); + resolver.update_overrides(&ctx, &mut query_result); let parent = query_result .tree @@ -555,14 +558,17 @@ mod tests { .unwrap(); for result in [parent, child1, child2] { - assert_eq!(result.resolved_properties, EntityProperties::default(),); + assert_eq!( + result.accumulated_properties(), + &EntityProperties::default(), + ); } // Now, override visibility on parent but not group - let mut overrides = parent.individual_properties.clone().unwrap_or_default(); + let mut overrides = parent.individual_properties().cloned().unwrap_or_default(); overrides.visible = false; - save_override(overrides, &parent.override_path, &mut blueprint); + save_override(overrides, parent.override_path().unwrap(), &mut blueprint); } // Parent is not visible, but children are @@ -573,7 +579,8 @@ mod tests { all_recordings: vec![], }; - let query_result = query.execute_query(&resolver, &ctx, &entities_per_system); + let mut query_result = query.execute_query(&ctx, &entities_per_system); + resolver.update_overrides(&ctx, &mut query_result); let parent_group = query_result .tree @@ -592,20 +599,24 @@ mod tests { .lookup_result_by_path_and_group(&EntityPath::from("parent/skip/child2"), false) .unwrap(); - assert!(!parent.resolved_properties.visible); + assert!(!parent.accumulated_properties().visible); for result in [child1, child2] { - assert!(result.resolved_properties.visible); + assert!(result.accumulated_properties().visible); } // Override visibility on parent group let mut overrides = parent_group - .individual_properties - .clone() + .individual_properties() + .cloned() .unwrap_or_default(); overrides.visible = false; - save_override(overrides, &parent_group.override_path, &mut blueprint); + save_override( + overrides, + parent_group.override_path().unwrap(), + &mut blueprint, + ); } // Nobody is visible @@ -616,7 +627,8 @@ mod tests { all_recordings: vec![], }; - let query_result = query.execute_query(&resolver, &ctx, &entities_per_system); + let mut query_result = query.execute_query(&ctx, &entities_per_system); + resolver.update_overrides(&ctx, &mut query_result); let parent = query_result .tree @@ -632,7 +644,7 @@ mod tests { .unwrap(); for result in [parent, child1, child2] { - assert!(!result.resolved_properties.visible); + assert!(!result.accumulated_properties().visible); } } @@ -643,11 +655,11 @@ mod tests { recording: Some(&recording), all_recordings: vec![], }); - let mut overrides = root.individual_properties.clone().unwrap_or_default(); + let mut overrides = root.individual_properties().cloned().unwrap_or_default(); overrides.visible_history.enabled = true; overrides.visible_history.nanos = VisibleHistory::ALL; - save_override(overrides, &root.override_path, &mut blueprint); + save_override(overrides, root.override_path().unwrap(), &mut blueprint); } // Everyone has visible history @@ -658,7 +670,8 @@ mod tests { all_recordings: vec![], }; - let query_result = query.execute_query(&resolver, &ctx, &entities_per_system); + let mut query_result = query.execute_query(&ctx, &entities_per_system); + resolver.update_overrides(&ctx, &mut query_result); let parent = query_result .tree @@ -674,17 +687,17 @@ mod tests { .unwrap(); for result in [parent, child1, child2] { - assert!(result.resolved_properties.visible_history.enabled); + assert!(result.accumulated_properties().visible_history.enabled); assert_eq!( - result.resolved_properties.visible_history.nanos, + result.accumulated_properties().visible_history.nanos, VisibleHistory::ALL ); } - let mut overrides = child2.individual_properties.clone().unwrap_or_default(); + let mut overrides = child2.individual_properties().cloned().unwrap_or_default(); overrides.visible_history.enabled = true; - save_override(overrides, &child2.override_path, &mut blueprint); + save_override(overrides, child2.override_path().unwrap(), &mut blueprint); } // Child2 has its own visible history @@ -695,7 +708,8 @@ mod tests { all_recordings: vec![], }; - let query_result = query.execute_query(&resolver, &ctx, &entities_per_system); + let mut query_result = query.execute_query(&ctx, &entities_per_system); + resolver.update_overrides(&ctx, &mut query_result); let parent = query_result .tree @@ -711,16 +725,16 @@ mod tests { .unwrap(); for result in [parent, child1] { - assert!(result.resolved_properties.visible_history.enabled); + assert!(result.accumulated_properties().visible_history.enabled); assert_eq!( - result.resolved_properties.visible_history.nanos, + result.accumulated_properties().visible_history.nanos, VisibleHistory::ALL ); } - assert!(child2.resolved_properties.visible_history.enabled); + assert!(child2.accumulated_properties().visible_history.enabled); assert_eq!( - child2.resolved_properties.visible_history.nanos, + child2.accumulated_properties().visible_history.nanos, VisibleHistory::OFF ); } diff --git a/crates/re_viewport/src/space_view_heuristics.rs b/crates/re_viewport/src/space_view_heuristics.rs index 93002b1ecd9f..bf64526e2527 100644 --- a/crates/re_viewport/src/space_view_heuristics.rs +++ b/crates/re_viewport/src/space_view_heuristics.rs @@ -5,7 +5,7 @@ use nohash_hasher::{IntMap, IntSet}; use re_arrow_store::{LatestAtQuery, Timeline}; use re_data_store::{EntityPath, EntityTree}; use re_log_types::{EntityPathExpr, TimeInt}; -use re_space_view::{DataQuery as _, DataQueryBlueprint, NOOP_RESOLVER}; +use re_space_view::{DataQuery as _, DataQueryBlueprint}; use re_types::components::{DisconnectedSpace, TensorData}; use re_types::ComponentNameSet; use re_viewer_context::{ @@ -58,18 +58,6 @@ pub fn all_possible_space_views( ) -> Vec<(SpaceViewBlueprint, DataQueryResult)> { re_tracing::profile_function!(); - for (class_identifier, entities_per_system) in entities_per_system_per_class { - for (system_name, entities) in entities_per_system { - if entities.is_empty() { - re_log::debug!( - "SpaceViewClassRegistry: No entities for system {:?} of class {:?}", - system_name, - class_identifier - ); - } - } - } - // For each candidate, create space views for all possible classes. candidate_space_view_paths(ctx, spaces_info) .flat_map(|candidate_space_path| { @@ -98,11 +86,8 @@ pub fn all_possible_space_views( std::iter::once(EntityPathExpr::Recursive(candidate_space_path.clone())), ); - let results = candidate_query.execute_query( - &NOOP_RESOLVER, - ctx.store_context, - entities_per_system, - ); + let results = + candidate_query.execute_query(ctx.store_context, entities_per_system); if !results.is_empty() { Some(( diff --git a/crates/re_viewport/src/viewport_blueprint_ui.rs b/crates/re_viewport/src/viewport_blueprint_ui.rs index 44dffa4ab821..43e742533ecb 100644 --- a/crates/re_viewport/src/viewport_blueprint_ui.rs +++ b/crates/re_viewport/src/viewport_blueprint_ui.rs @@ -222,7 +222,7 @@ impl Viewport<'_, '_> { }; let group_is_visible = - top_node.data_result.resolved_properties.visible && space_view_visible; + top_node.data_result.accumulated_properties().visible && space_view_visible; // Always real children ahead of groups for child in top_node @@ -265,8 +265,8 @@ impl Viewport<'_, '_> { ctx.selection_state().highlight_for_ui_element(&item) == HoverHighlight::Hovered; let mut properties = data_result - .individual_properties - .clone() + .individual_properties() + .cloned() .unwrap_or_default(); let name = entity_path