diff --git a/crates/re_data_ui/src/item_ui.rs b/crates/re_data_ui/src/item_ui.rs index 9ea707ada529..ff1409729987 100644 --- a/crates/re_data_ui/src/item_ui.rs +++ b/crates/re_data_ui/src/item_ui.rs @@ -5,9 +5,7 @@ use re_entity_db::{EntityTree, InstancePath}; use re_log_types::{ComponentPath, EntityPath, TimeInt, Timeline}; use re_ui::SyntaxHighlighting; -use re_viewer_context::{ - DataQueryId, HoverHighlight, Item, SpaceViewId, UiVerbosity, ViewerContext, -}; +use re_viewer_context::{HoverHighlight, Item, SpaceViewId, UiVerbosity, ViewerContext}; use super::DataUi; @@ -31,9 +29,6 @@ use super::DataUi; // Item::InstancePath(space_view_id, instance_path) => { // instance_path_button_to(ctx, ui, *space_view_id, instance_path, text) // } -// Item::DataBlueprintGroup(space_view_id, group_handle) => { -// data_blueprint_group_button_to(ctx, ui, text, *space_view_id, *group_handle) -// } // } // } @@ -311,28 +306,6 @@ pub fn component_path_button_to( cursor_interact_with_selectable(ctx, response, item) } -pub fn data_blueprint_group_button_to( - ctx: &ViewerContext<'_>, - ui: &mut egui::Ui, - text: impl Into, - space_view_id: SpaceViewId, - query_id: DataQueryId, - entity_path: EntityPath, -) -> egui::Response { - let item = Item::DataBlueprintGroup(space_view_id, query_id, entity_path); - let response = ctx - .re_ui - .selectable_label_with_icon( - ui, - &re_ui::icons::GROUP, - text, - ctx.selection().contains_item(&item), - re_ui::LabelStyle::Normal, - ) - .on_hover_text("Group"); - cursor_interact_with_selectable(ctx, response, item) -} - pub fn data_blueprint_button_to( ctx: &ViewerContext<'_>, query: &re_data_store::LatestAtQuery, diff --git a/crates/re_space_view/src/data_query.rs b/crates/re_space_view/src/data_query.rs index 09638dcebbd9..68f3d1c02f73 100644 --- a/crates/re_space_view/src/data_query.rs +++ b/crates/re_space_view/src/data_query.rs @@ -4,7 +4,7 @@ use re_viewer_context::{DataQueryResult, PerVisualizer, StoreContext, Visualizab pub struct EntityOverrideContext { pub root: EntityProperties, pub individual: EntityPropertyMap, - pub group: EntityPropertyMap, + pub recursive: EntityPropertyMap, } /// Trait for resolving properties needed by most implementations of [`DataQuery`] diff --git a/crates/re_space_view/src/data_query_blueprint.rs b/crates/re_space_view/src/data_query_blueprint.rs index af86f145eae2..58964269e0ee 100644 --- a/crates/re_space_view/src/data_query_blueprint.rs +++ b/crates/re_space_view/src/data_query_blueprint.rs @@ -315,76 +315,41 @@ impl<'a> QueryExpressionEvaluator<'a> { let entity_path = &tree.path; - // Pre-compute our matches - let any_match = self.entity_path_filter.is_included(entity_path); - - // Check for visualizers if this is a match. - // Note that allowed prefixes that aren't matches can still create groups. - let visualizable = any_match - && self - .visualizable_entities_for_visualizer_systems - .values() - .any(|ents| ents.contains(entity_path)); - - let self_leaf = if visualizable || self.entity_path_filter.is_exact_included(entity_path) { - // TODO(#5067): For now, we always start by setting visualizers to the full list of available visualizers. - // This is currently important for evaluating auto-properties during the space-view `on_frame_start`, which - // is called before the property-overrider has a chance to update this list. - // This list will be updated below during `update_overrides_recursive` by calling `choose_default_visualizers` - // on the space view. - let available_visualizers = self - .visualizable_entities_for_visualizer_systems - .iter() - .filter_map(|(visualizer, ents)| { - if ents.contains(entity_path) { - Some(*visualizer) - } else { - None - } - }) - .collect(); - - Some(data_results.insert(DataResultNode { - data_result: DataResult { - entity_path: entity_path.clone(), - visualizers: available_visualizers, - is_group: false, - direct_included: any_match, - property_overrides: None, - }, - children: Default::default(), - })) - } else { - None - }; - - let maybe_self_iter = if let Some(self_leaf) = self_leaf { - itertools::Either::Left(std::iter::once(self_leaf)) - } else { - itertools::Either::Right(std::iter::empty()) - }; + // TODO(#5067): For now, we always start by setting visualizers to the full list of available visualizers. + // This is currently important for evaluating auto-properties during the space-view `on_frame_start`, which + // is called before the property-overrider has a chance to update this list. + // This list will be updated below during `update_overrides_recursive` by calling `choose_default_visualizers` + // on the space view. + let visualizers: SmallVec<[_; 4]> = self + .visualizable_entities_for_visualizer_systems + .iter() + .filter_map(|(visualizer, ents)| ents.contains(entity_path).then_some(*visualizer)) + .collect(); - let children: SmallVec<_> = maybe_self_iter - .chain(tree.children.values().filter_map(|subtree| { + let children: SmallVec<[_; 4]> = tree + .children + .values() + .filter_map(|subtree| { self.add_entity_tree_to_data_results_recursive(subtree, data_results) - })) + }) .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 + // Ignore empty nodes. + // Since we recurse downwards, this prunes any branches that don't have anything to contribute to the scene + // and aren't directly included. + let direct_included = self.entity_path_filter.is_exact_included(entity_path); + if direct_included || !children.is_empty() || !visualizers.is_empty() { Some(data_results.insert(DataResultNode { data_result: DataResult { entity_path: entity_path.clone(), - visualizers: Default::default(), - is_group: true, - direct_included: any_match, + visualizers, + direct_included, property_overrides: None, }, children, })) + } else { + None } } } @@ -407,7 +372,7 @@ impl DataQueryPropertyResolver<'_> { /// - 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` + /// - The recursive overrides are found by walking an override subtree under the `data_query//recursive_overrides` fn build_override_context( &self, ctx: &StoreContext<'_>, @@ -444,7 +409,7 @@ impl DataQueryPropertyResolver<'_> { query, &self.individual_override_root, ), - group: self.resolve_entity_overrides_for_path( + recursive: self.resolve_entity_overrides_for_path( ctx, query, &self.recursive_override_root, @@ -486,8 +451,8 @@ impl DataQueryPropertyResolver<'_> { /// Recursively walk the [`DataResultTree`] and update the [`PropertyOverrides`] for each node. /// - /// This will accumulate the group properties at each step down the tree, and then finally merge - /// with individual overrides at the leaves. + /// This will accumulate the recursive properties at each step down the tree, and then merge + /// with individual overrides on each step. fn update_overrides_recursive( &self, ctx: &StoreContext<'_>, @@ -498,97 +463,95 @@ impl DataQueryPropertyResolver<'_> { 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 { + query_result.tree.lookup_node_mut(handle).map(|node| { + let recursive_properties = override_context + .recursive + .get_opt(&node.data_result.entity_path); + let accumulated_recursive_properties = + if let Some(overridden) = recursive_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(), - component_overrides: Default::default(), - 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); - - let accumulated_properties = if let Some(overridden) = overridden_properties { - accumulated.with_child(overridden) + let individual_properties = override_context + .individual + .get_opt(&node.data_result.entity_path); + let accumulated_properties = + if let Some(individual_override) = individual_properties { + accumulated_recursive_properties.with_child(individual_override) } else { - accumulated.clone() + accumulated_recursive_properties.clone() }; - let override_path = self - .individual_override_root - .join(&node.data_result.entity_path); - + let individual_override_path = self + .individual_override_root + .join(&node.data_result.entity_path); + let recursive_override_path = self + .recursive_override_root + .join(&node.data_result.entity_path); + + if !node.data_result.visualizers.is_empty() { + re_tracing::profile_scope!("Update visualizers from overrides"); + + // If the user has overridden the visualizers, update which visualizers are used. + if let Some(viz_override) = ctx + .blueprint + .store() + .query_latest_component::( + &individual_override_path, + query, + ) + .map(|c| c.value) { - re_tracing::profile_scope!("Update visualizers from overrides"); + node.data_result.visualizers = + viz_override.0.iter().map(|v| v.as_str().into()).collect(); + } else { + // Otherwise ask the `SpaceViewClass` to choose. + node.data_result.visualizers = self + .space_view + .class(self.space_view_class_registry) + .choose_default_visualizers( + &node.data_result.entity_path, + self.visualizable_entities_per_visualizer, + self.indicated_entities_per_visualizer, + ); + } + } - // If the user has overridden the visualizers, update which visualizers are used. - if let Some(viz_override) = ctx + let mut component_overrides: HashMap = + Default::default(); + + if let Some(override_subtree) = + ctx.blueprint.tree().subtree(&individual_override_path) + { + for component in override_subtree.entity.components.keys() { + if let Some(component_data) = ctx .blueprint .store() - .query_latest_component::(&override_path, query) - .map(|c| c.value) + .latest_at(query, &individual_override_path, *component, &[*component]) + .and_then(|(_, _, cells)| cells[0].clone()) { - node.data_result.visualizers = - viz_override.0.iter().map(|v| v.as_str().into()).collect(); - } else { - // Otherwise ask the `SpaceViewClass` to choose. - node.data_result.visualizers = self - .space_view - .class(self.space_view_class_registry) - .choose_default_visualizers( - &node.data_result.entity_path, - self.visualizable_entities_per_visualizer, - self.indicated_entities_per_visualizer, + if !component_data.is_empty() { + component_overrides.insert( + *component, + (StoreKind::Blueprint, individual_override_path.clone()), ); - } - } - let mut component_overrides: HashMap = - Default::default(); - - if let Some(override_subtree) = ctx.blueprint.tree().subtree(&override_path) { - for component in override_subtree.entity.components.keys() { - if let Some(component_data) = ctx - .blueprint - .store() - .latest_at(query, &override_path, *component, &[*component]) - .and_then(|(_, _, cells)| cells[0].clone()) - { - if !component_data.is_empty() { - component_overrides.insert( - *component, - (StoreKind::Blueprint, override_path.clone()), - ); - } } } } + } - node.data_result.property_overrides = Some(PropertyOverrides { - individual_properties: overridden_properties.cloned(), - accumulated_properties: accumulated_properties.clone(), - component_overrides, - override_path, - }); + node.data_result.property_overrides = Some(PropertyOverrides { + accumulated_properties, + individual_properties: individual_properties.cloned(), + recursive_properties: recursive_properties.cloned(), + component_overrides, + recursive_override_path, + individual_override_path, + }); - None - } + (node.children.clone(), accumulated_recursive_properties) }) { for child in child_handles { diff --git a/crates/re_space_view/src/space_view.rs b/crates/re_space_view/src/space_view.rs index aeee6eab3e09..2c9a0618fc74 100644 --- a/crates/re_space_view/src/space_view.rs +++ b/crates/re_space_view/src/space_view.rs @@ -475,13 +475,14 @@ impl SpaceViewBlueprint { DataResult { entity_path: entity_path.clone(), visualizers: Default::default(), - is_group: true, direct_included: true, property_overrides: Some(PropertyOverrides { accumulated_properties, individual_properties, + recursive_properties: Default::default(), component_overrides: Default::default(), - override_path: entity_path, + recursive_override_path: entity_path.clone(), + individual_override_path: entity_path, }), } } @@ -642,15 +643,15 @@ mod tests { let parent = query_result .tree - .lookup_result_by_path_and_group(&EntityPath::from("parent"), false) + .lookup_result_by_path(&EntityPath::from("parent")) .unwrap(); let child1 = query_result .tree - .lookup_result_by_path_and_group(&EntityPath::from("parent/skip/child1"), false) + .lookup_result_by_path(&EntityPath::from("parent/skip/child1")) .unwrap(); let child2 = query_result .tree - .lookup_result_by_path_and_group(&EntityPath::from("parent/skip/child2"), false) + .lookup_result_by_path(&EntityPath::from("parent/skip/child2")) .unwrap(); for result in [parent, child1, child2] { @@ -660,11 +661,15 @@ mod tests { ); } - // Now, override visibility on parent but not group + // Now, override visibility on parent individually. let mut overrides = parent.individual_properties().cloned().unwrap_or_default(); overrides.visible = false; - save_override(overrides, parent.override_path().unwrap(), &mut blueprint); + save_override( + overrides, + parent.individual_override_path().unwrap(), + &mut blueprint, + ); } // Parent is not visible, but children are @@ -680,19 +685,19 @@ mod tests { let parent_group = query_result .tree - .lookup_result_by_path_and_group(&EntityPath::from("parent"), true) + .lookup_result_by_path(&EntityPath::from("parent")) .unwrap(); let parent = query_result .tree - .lookup_result_by_path_and_group(&EntityPath::from("parent"), false) + .lookup_result_by_path(&EntityPath::from("parent")) .unwrap(); let child1 = query_result .tree - .lookup_result_by_path_and_group(&EntityPath::from("parent/skip/child1"), false) + .lookup_result_by_path(&EntityPath::from("parent/skip/child1")) .unwrap(); let child2 = query_result .tree - .lookup_result_by_path_and_group(&EntityPath::from("parent/skip/child2"), false) + .lookup_result_by_path(&EntityPath::from("parent/skip/child2")) .unwrap(); assert!(!parent.accumulated_properties().visible); @@ -701,7 +706,7 @@ mod tests { assert!(result.accumulated_properties().visible); } - // Override visibility on parent group + // Override visibility on parent recursively. let mut overrides = parent_group .individual_properties() .cloned() @@ -710,7 +715,7 @@ mod tests { save_override( overrides, - parent_group.override_path().unwrap(), + parent_group.recursive_override_path().unwrap(), &mut blueprint, ); } @@ -728,15 +733,15 @@ mod tests { let parent = query_result .tree - .lookup_result_by_path_and_group(&EntityPath::from("parent"), false) + .lookup_result_by_path(&EntityPath::from("parent")) .unwrap(); let child1 = query_result .tree - .lookup_result_by_path_and_group(&EntityPath::from("parent/skip/child1"), false) + .lookup_result_by_path(&EntityPath::from("parent/skip/child1")) .unwrap(); let child2 = query_result .tree - .lookup_result_by_path_and_group(&EntityPath::from("parent/skip/child2"), false) + .lookup_result_by_path(&EntityPath::from("parent/skip/child2")) .unwrap(); for result in [parent, child1, child2] { @@ -758,7 +763,11 @@ mod tests { overrides.visible_history.enabled = true; overrides.visible_history.nanos = VisibleHistory::ALL; - save_override(overrides, root.override_path().unwrap(), &mut blueprint); + save_override( + overrides, + root.recursive_override_path().unwrap(), + &mut blueprint, + ); } // Everyone has visible history @@ -773,15 +782,15 @@ mod tests { let parent = query_result .tree - .lookup_result_by_path_and_group(&EntityPath::from("parent"), false) + .lookup_result_by_path(&EntityPath::from("parent")) .unwrap(); let child1 = query_result .tree - .lookup_result_by_path_and_group(&EntityPath::from("parent/skip/child1"), false) + .lookup_result_by_path(&EntityPath::from("parent/skip/child1")) .unwrap(); let child2 = query_result .tree - .lookup_result_by_path_and_group(&EntityPath::from("parent/skip/child2"), false) + .lookup_result_by_path(&EntityPath::from("parent/skip/child2")) .unwrap(); for result in [parent, child1, child2] { @@ -795,7 +804,11 @@ mod tests { let mut overrides = child2.individual_properties().cloned().unwrap_or_default(); overrides.visible_history.enabled = true; - save_override(overrides, child2.override_path().unwrap(), &mut blueprint); + save_override( + overrides, + child2.individual_override_path().unwrap(), + &mut blueprint, + ); } // Child2 has its own visible history @@ -811,15 +824,15 @@ mod tests { let parent = query_result .tree - .lookup_result_by_path_and_group(&EntityPath::from("parent"), false) + .lookup_result_by_path(&EntityPath::from("parent")) .unwrap(); let child1 = query_result .tree - .lookup_result_by_path_and_group(&EntityPath::from("parent/skip/child1"), false) + .lookup_result_by_path(&EntityPath::from("parent/skip/child1")) .unwrap(); let child2 = query_result .tree - .lookup_result_by_path_and_group(&EntityPath::from("parent/skip/child2"), false) + .lookup_result_by_path(&EntityPath::from("parent/skip/child2")) .unwrap(); for result in [parent, child1] { diff --git a/crates/re_space_view_spatial/src/ui_3d.rs b/crates/re_space_view_spatial/src/ui_3d.rs index f876cf260a40..7c7ac747147d 100644 --- a/crates/re_space_view_spatial/src/ui_3d.rs +++ b/crates/re_space_view_spatial/src/ui_3d.rs @@ -550,7 +550,7 @@ pub fn view_3d( // Track focused entity if any. if let Some(focused_item) = ctx.focused_item { let focused_entity = match focused_item { - Item::StoreId(_) | Item::DataBlueprintGroup(_, _, _) | Item::Container(_) => None, + Item::StoreId(_) | Item::Container(_) => None, Item::SpaceView(space_view_id) => { if space_view_id == &query.space_view_id { diff --git a/crates/re_viewer/src/ui/override_ui.rs b/crates/re_viewer/src/ui/override_ui.rs index bab48f4146f4..78936aac7ff2 100644 --- a/crates/re_viewer/src/ui/override_ui.rs +++ b/crates/re_viewer/src/ui/override_ui.rs @@ -38,7 +38,7 @@ pub fn override_ui( let query_result = ctx.lookup_query_result(space_view.query_id()); let Some(data_result) = query_result .tree - .lookup_result_by_path_and_group(entity_path, false) + .lookup_result_by_path(entity_path) .cloned() else { ui.label(ctx.re_ui.error_text("Entity not found in view.")); @@ -125,7 +125,7 @@ pub fn override_ui( // not exist in the recording store. ctx.save_empty_blueprint_component_name( ctx.store_context.blueprint.store(), - &overrides.override_path, + &overrides.individual_override_path, *component_name, ); } @@ -166,7 +166,7 @@ pub fn override_ui( &query, store, entity_path, - &overrides.override_path, + &overrides.individual_override_path, &component_data, instance_key, ); @@ -214,7 +214,7 @@ pub fn add_new_override( } // If we don't have an override_path we can't set up an initial override // this shouldn't happen if the `DataResult` is valid. - let Some(override_path) = data_result.override_path() else { + let Some(override_path) = data_result.individual_override_path() else { if cfg!(debug_assertions) { re_log::error!("No override path for: {}", component); } @@ -327,14 +327,14 @@ pub fn override_visualizer_ui( let query_result = ctx.lookup_query_result(space_view.query_id()); let Some(data_result) = query_result .tree - .lookup_result_by_path_and_group(entity_path, false) + .lookup_result_by_path(entity_path) .cloned() else { ui.label(ctx.re_ui.error_text("Entity not found in view.")); return; }; - let Some(override_path) = data_result.override_path() else { + let Some(override_path) = data_result.individual_override_path() else { if cfg!(debug_assertions) { re_log::error!("No override path for entity: {}", data_result.entity_path); } @@ -402,7 +402,7 @@ pub fn add_new_visualizer( ) { // If we don't have an override_path we can't set up an initial override // this shouldn't happen if the `DataResult` is valid. - let Some(override_path) = data_result.override_path() else { + let Some(override_path) = data_result.individual_override_path() else { if cfg!(debug_assertions) { re_log::error!("No override path for entity: {}", data_result.entity_path); } diff --git a/crates/re_viewer/src/ui/selection_history_ui.rs b/crates/re_viewer/src/ui/selection_history_ui.rs index dc0e3df1d660..b0bfbabccd94 100644 --- a/crates/re_viewer/src/ui/selection_history_ui.rs +++ b/crates/re_viewer/src/ui/selection_history_ui.rs @@ -191,7 +191,6 @@ fn item_to_string(blueprint: &ViewportBlueprint, item: &Item) -> String { } } Item::InstancePath(_, entity_path) => entity_path.to_string(), - Item::DataBlueprintGroup(_sid, _qid, entity_path) => entity_path.to_string(), Item::ComponentPath(path) => { format!("{}:{}", path.entity_path, path.component_name.short_name(),) } diff --git a/crates/re_viewer/src/ui/selection_panel.rs b/crates/re_viewer/src/ui/selection_panel.rs index 202fc813dec0..92aacdbac366 100644 --- a/crates/re_viewer/src/ui/selection_panel.rs +++ b/crates/re_viewer/src/ui/selection_panel.rs @@ -15,8 +15,8 @@ use re_ui::list_item::ListItem; use re_ui::{ReUi, SyntaxHighlighting as _}; use re_viewer_context::{ blueprint_timepoint_for_writes, gpu_bridge::colormap_dropdown_button_ui, ContainerId, - DataQueryId, HoverHighlight, Item, SpaceViewClass, SpaceViewClassIdentifier, SpaceViewId, - SystemCommand, SystemCommandSender as _, UiVerbosity, ViewerContext, + HoverHighlight, Item, SpaceViewClass, SpaceViewClassIdentifier, SpaceViewId, SystemCommand, + SystemCommandSender as _, UiVerbosity, ViewerContext, }; use re_viewport::{ external::re_space_view::blueprint::components::QueryExpressions, icon_for_container_kind, @@ -287,7 +287,7 @@ fn data_section_ui(item: &Item) -> Option> { Item::ComponentPath(component_path) => Some(Box::new(component_path.clone())), Item::InstancePath(_, instance_path) => Some(Box::new(instance_path.clone())), // Skip data ui since we don't know yet what to show for these. - Item::SpaceView(_) | Item::DataBlueprintGroup(_, _, _) | Item::Container(_) => None, + Item::SpaceView(_) | Item::Container(_) => None, } } @@ -484,25 +484,6 @@ fn what_is_selected_ui( list_existing_data_blueprints(ui, ctx, &instance_path.entity_path, viewport); } } - Item::DataBlueprintGroup(space_view_id, _query_id, entity_path) => { - if let Some(space_view) = viewport.space_view(space_view_id) { - item_title_ui( - ctx.re_ui, - ui, - &entity_path.to_string(), - Some(&re_ui::icons::GROUP), - &format!( - "Group {:?} as shown in Space View {:?}", - entity_path, space_view.display_name - ), - ); - - ui.horizontal(|ui| { - ui.label("in"); - space_view_button(ctx, ui, space_view); - }); - } - } } } @@ -803,7 +784,7 @@ fn has_blueprint_section(item: &Item) -> bool { Item::InstancePath(space_view_id, instance_path) => { space_view_id.is_some() && instance_path.instance_key.is_splat() } - _ => true, + Item::SpaceView(_) => true, } } @@ -829,10 +810,6 @@ fn blueprint_ui( ); } - Item::DataBlueprintGroup(space_view_id, query_id, group_path) => { - blueprint_ui_for_group(ui, ctx, viewport, space_view_id, query_id, group_path); - } - Item::StoreId(_) | Item::ComponentPath(_) | Item::Container(_) => {} } } @@ -930,7 +907,7 @@ fn blueprint_ui_for_space_view( &mut props, ); - root_data_result.save_override(Some(props), ctx); + root_data_result.save_individual_override(Some(props), ctx); } } @@ -947,18 +924,18 @@ fn blueprint_ui_for_instance_path( // splat - the whole entity let space_view_class = *space_view.class_identifier(); let entity_path = &instance_path.entity_path; - let as_group = false; let query_result = ctx.lookup_query_result(space_view.query_id()); if let Some(data_result) = query_result .tree - .lookup_result_by_path_and_group(entity_path, as_group) + .lookup_result_by_path(entity_path) .cloned() { let mut props = data_result .individual_properties() .cloned() .unwrap_or_default(); + entity_props_ui( ctx, ui, @@ -967,51 +944,13 @@ fn blueprint_ui_for_instance_path( &mut props, data_result.accumulated_properties(), ); - data_result.save_override(Some(props), ctx); + data_result.save_individual_override(Some(props), ctx); } } } } } -fn blueprint_ui_for_group( - ui: &mut Ui, - ctx: &ViewerContext<'_>, - viewport: &Viewport<'_, '_>, - space_view_id: &SpaceViewId, - query_id: &DataQueryId, - group_path: &EntityPath, -) { - if let Some(space_view) = viewport.blueprint.space_view(space_view_id) { - let as_group = true; - - let query_result = ctx.lookup_query_result(*query_id); - if let Some(data_result) = query_result - .tree - .lookup_result_by_path_and_group(group_path, as_group) - .cloned() - { - let space_view_class = *space_view.class_identifier(); - let mut props = data_result - .individual_properties() - .cloned() - .unwrap_or_default(); - - entity_props_ui( - ctx, - ui, - &space_view_class, - None, - &mut props, - data_result.accumulated_properties(), - ); - data_result.save_override(Some(props), ctx); - } - } else { - ctx.selection_state().clear_current(); - } -} - /// Returns a new filter when the editing is done, and there has been a change. fn entity_path_filter_ui( ui: &mut egui::Ui, diff --git a/crates/re_viewer_context/src/item.rs b/crates/re_viewer_context/src/item.rs index 05e673e5f7bf..d62d445382b5 100644 --- a/crates/re_viewer_context/src/item.rs +++ b/crates/re_viewer_context/src/item.rs @@ -1,7 +1,7 @@ use re_entity_db::InstancePath; use re_log_types::{ComponentPath, DataPath, EntityPath}; -use crate::{ContainerId, DataQueryId, SpaceViewId}; +use crate::{ContainerId, SpaceViewId}; /// One "thing" in the UI. /// @@ -13,7 +13,6 @@ pub enum Item { ComponentPath(ComponentPath), SpaceView(SpaceViewId), InstancePath(Option, InstancePath), - DataBlueprintGroup(SpaceViewId, DataQueryId, EntityPath), Container(ContainerId), } @@ -23,7 +22,6 @@ impl Item { Item::ComponentPath(component_path) => Some(&component_path.entity_path), Item::SpaceView(_) | Item::Container(_) | Item::StoreId(_) => None, Item::InstancePath(_, instance_path) => Some(&instance_path.entity_path), - Item::DataBlueprintGroup(_, _, entity_path) => Some(entity_path), } } } @@ -96,9 +94,6 @@ impl std::fmt::Debug for Item { Item::ComponentPath(s) => s.fmt(f), Item::SpaceView(s) => write!(f, "{s:?}"), Item::InstancePath(sid, path) => write!(f, "({sid:?}, {path})"), - Item::DataBlueprintGroup(sid, qid, entity_path) => { - write!(f, "({sid:?}, {qid:?}, {entity_path:?})") - } Item::Container(tile_id) => write!(f, "(tile: {tile_id:?})"), } } @@ -124,7 +119,6 @@ impl Item { } Item::ComponentPath(_) => "Entity Component", Item::SpaceView(_) => "Space View", - Item::DataBlueprintGroup(_, _, _) => "Group", Item::Container(_) => "Container", } } @@ -142,11 +136,9 @@ pub fn resolve_mono_instance_path_item( *space_view, resolve_mono_instance_path(query, store, instance), ), - Item::StoreId(_) - | Item::ComponentPath(_) - | Item::SpaceView(_) - | Item::DataBlueprintGroup(_, _, _) - | Item::Container(_) => item.clone(), + Item::StoreId(_) | Item::ComponentPath(_) | Item::SpaceView(_) | Item::Container(_) => { + item.clone() + } } } diff --git a/crates/re_viewer_context/src/lib.rs b/crates/re_viewer_context/src/lib.rs index 1d05f1b99e11..8dd9dcde7c5c 100644 --- a/crates/re_viewer_context/src/lib.rs +++ b/crates/re_viewer_context/src/lib.rs @@ -74,11 +74,6 @@ pub mod external { // --------------------------------------------------------------------------- -slotmap::new_key_type! { - /// Identifier for a blueprint group. - pub struct DataBlueprintGroupHandle; -} - #[derive(Copy, Clone, Debug, PartialEq, Eq)] pub enum NeedsRepaint { Yes, diff --git a/crates/re_viewer_context/src/query_context.rs b/crates/re_viewer_context/src/query_context.rs index 2d457b1390ae..b5f51f32d72e 100644 --- a/crates/re_viewer_context/src/query_context.rs +++ b/crates/re_viewer_context/src/query_context.rs @@ -1,9 +1,10 @@ use ahash::HashMap; use once_cell::sync::Lazy; -use re_log_types::EntityPath; use slotmap::SlotMap; use smallvec::SmallVec; +use re_log_types::{EntityPath, EntityPathHash}; + use crate::{DataQueryId, DataResult, ViewerContext}; slotmap::new_key_type! { @@ -29,14 +30,14 @@ impl DataQueryResult { #[inline] pub fn contains_entity(&self, path: &EntityPath) -> bool { self.tree - .lookup_result_by_path_and_group(path, false) + .lookup_result_by_path(path) .map_or(false, |result| result.direct_included) } #[inline] pub fn contains_group(&self, path: &EntityPath) -> bool { self.tree - .lookup_result_by_path_and_group(path, true) + .lookup_result_by_path(path) .map_or(false, |result| result.direct_included) } @@ -73,7 +74,7 @@ pub struct DataResultTree { // at the moment we only look up a single path per frame for the selection panel. It's probably // less over-head to just walk the tree once instead of pre-computing an entire map we use for // a single lookup. - data_results_by_path: HashMap<(EntityPath, bool), DataResultHandle>, + data_results_by_path: HashMap, root_handle: Option, } @@ -92,15 +93,7 @@ impl DataResultTree { re_tracing::profile_function!(); let data_results_by_path = data_results .iter() - .map(|(handle, node)| { - ( - ( - node.data_result.entity_path.clone(), - node.data_result.is_group, - ), - handle, - ) - }) + .map(|(handle, node)| (node.data_result.entity_path.hash(), handle)) .collect(); Self { @@ -119,30 +112,6 @@ impl DataResultTree { .and_then(|handle| self.data_results.get(handle)) } - pub fn first_interesting_root(&self) -> Option<&DataResultNode> { - let mut next_node = self.root_node(); - - while let Some(node) = next_node { - // If both this node is trivial we can skip it. - // A trivial node is a node which is a group, with a single child, - // where that child still has children. - if node.data_result.is_group && node.children.len() == 1 { - if let Some(child_handle) = node.children.first() { - if let Some(child) = self.data_results.get(*child_handle) { - if !child.children.is_empty() { - next_node = Some(child); - continue; - } - } - } - } - - break; - } - - next_node - } - /// Depth-first traversal of the tree, calling `visitor` on each result. pub fn visit(&self, visitor: &mut impl FnMut(DataResultHandle)) { if let Some(root_handle) = self.root_handle { @@ -150,17 +119,6 @@ impl DataResultTree { } } - /// Depth-first traversal of a subtree, starting with the given group entity-path, calling `visitor` on each result. - pub fn visit_group( - &self, - entity_path: &EntityPath, - visitor: &mut impl FnMut(DataResultHandle), - ) { - if let Some(subtree_handle) = self.data_results_by_path.get(&(entity_path.clone(), true)) { - self.visit_recursive(*subtree_handle, visitor); - } - } - /// Look up a [`DataResult`] in the tree based on its handle. pub fn lookup_result(&self, handle: DataResultHandle) -> Option<&DataResult> { self.data_results.get(handle).map(|node| &node.data_result) @@ -177,13 +135,9 @@ impl DataResultTree { } /// Look up a [`DataResultNode`] in the tree based on an [`EntityPath`]. - pub fn lookup_result_by_path_and_group( - &self, - path: &EntityPath, - is_group: bool, - ) -> Option<&DataResult> { + pub fn lookup_result_by_path(&self, path: &EntityPath) -> Option<&DataResult> { self.data_results_by_path - .get(&(path.clone(), is_group)) + .get(&path.hash()) .and_then(|handle| self.lookup_result(*handle)) } diff --git a/crates/re_viewer_context/src/selection_state.rs b/crates/re_viewer_context/src/selection_state.rs index 3751dfcf9f6e..8a8867105c68 100644 --- a/crates/re_viewer_context/src/selection_state.rs +++ b/crates/re_viewer_context/src/selection_state.rs @@ -321,16 +321,10 @@ impl ApplicationSelectionState { .hovered_previous_frame .iter_items() .any(|current| match current { - Item::StoreId(_) - | Item::SpaceView(_) - | Item::DataBlueprintGroup(_, _, _) - | Item::Container(_) => current == test, + Item::StoreId(_) | Item::SpaceView(_) | Item::Container(_) => current == test, Item::ComponentPath(component_path) => match test { - Item::StoreId(_) - | Item::SpaceView(_) - | Item::DataBlueprintGroup(_, _, _) - | Item::Container(_) => false, + Item::StoreId(_) | Item::SpaceView(_) | Item::Container(_) => false, Item::ComponentPath(test_component_path) => { test_component_path == component_path 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 a983dc4a2f07..0a11d6998d6e 100644 --- a/crates/re_viewer_context/src/space_view/view_query.rs +++ b/crates/re_viewer_context/src/space_view/view_query.rs @@ -24,14 +24,22 @@ pub struct PropertyOverrides { /// The individual property set in this `DataResult`, if any. pub individual_properties: Option, + /// The recursive property set in this `DataResult`, if any. + pub recursive_properties: Option, + /// An alternative store and entity path to use for the specified component. // NOTE: StoreKind is easier to work with than a `StoreId`` or full `DataStore` but // might still be ambiguous when we have multiple stores active at a time. // TODO(jleibs): Consider something like `tinymap` for this. pub component_overrides: HashMap, - /// `EntityPath` in the Blueprint store where updated overrides should be written back. - pub override_path: EntityPath, + /// `EntityPath` in the Blueprint store where updated overrides should be written back\ + /// for properties that apply recursively. + pub recursive_override_path: EntityPath, + + /// `EntityPath` in the Blueprint store where updated overrides should be written back + /// for properties that apply to the individual entity only. + pub individual_override_path: EntityPath, } pub type SmallVisualizerSet = SmallVec<[ViewSystemIdentifier; 4]>; @@ -53,11 +61,7 @@ pub struct DataResult { /// Which `ViewSystems`s to pass the `DataResult` to. pub visualizers: SmallVisualizerSet, - /// This DataResult represents a group - // TODO(jleibs): Maybe make this an enum instead? - pub is_group: bool, - - // This result was actually in the query results, not just a group that + // This result was actually in the query results, not just a path that // exists due to a common prefix. pub direct_included: bool, @@ -72,16 +76,66 @@ impl DataResult { 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) + pub fn recursive_override_path(&self) -> Option<&EntityPath> { + self.property_overrides + .as_ref() + .map(|p| &p.recursive_override_path) + } + + #[inline] + pub fn individual_override_path(&self) -> Option<&EntityPath> { + self.property_overrides + .as_ref() + .map(|p| &p.individual_override_path) + } + + /// Write the [`EntityProperties`] for this result back to the Blueprint store on the recursive override. + /// + /// Setting `new_recursive_props` to `None` will always clear the override. + /// Otherwise, writes only if the recursive properties aren't already the same value. + /// (does *not* take into account what the accumulated properties are which are a combination of recursive and individual overwrites) + pub fn save_recursive_override( + &self, + ctx: &ViewerContext<'_>, + new_recursive_props: Option, + ) { + self.save_override_internal( + ctx, + new_recursive_props, + self.recursive_override_path(), + self.recursive_properties(), + ); + } + + /// Write the [`EntityProperties`] for this result back to the Blueprint store on the individual override. + /// + /// Setting `new_individual_props` to `None` will always clear the override. + /// Otherwise, writes only if the individual properties aren't already the same value. + /// (does *not* take into account what the accumulated properties are which are a combination of recursive and individual overwrites) + pub fn save_individual_override( + &self, + new_individual_props: Option, + ctx: &ViewerContext<'_>, + ) { + self.save_override_internal( + ctx, + new_individual_props, + self.individual_override_path(), + self.individual_properties(), + ); } - /// Write the [`EntityProperties`] for this result back to the Blueprint store. - pub fn save_override(&self, props: Option, ctx: &ViewerContext<'_>) { + fn save_override_internal( + &self, + ctx: &ViewerContext<'_>, + new_individual_props: Option, + override_path: Option<&EntityPath>, + properties: Option<&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(override_path) = self.override_path() else { + let Some(override_path) = override_path else { re_log::warn!( "Tried to save override for {:?} but it has no override path", self.entity_path @@ -89,12 +143,7 @@ impl DataResult { 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 { + let cell = match new_individual_props { None => { re_log::debug!("Clearing {:?}", override_path); @@ -105,14 +154,9 @@ impl DataResult { } 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 + // the properties are `None`, we only must save if `props` is different // from the default. - if props.has_edits( - property_overrides - .individual_properties - .as_ref() - .unwrap_or(&EntityProperties::default()), - ) { + if props.has_edits(properties.unwrap_or(&DEFAULT_PROPS)) { re_log::debug!("Overriding {:?} with {:?}", override_path, props); let component = EntityPropertiesComponent(props); @@ -124,21 +168,19 @@ impl DataResult { } }; - let Some(cell) = cell else { - return; - }; - - let timepoint = blueprint_timepoint_for_writes(); + if let Some(cell) = cell { + let timepoint = blueprint_timepoint_for_writes(); - let row = - DataRow::from_cells1_sized(RowId::new(), override_path.clone(), timepoint, 1, cell) - .unwrap(); + let row = + DataRow::from_cells1_sized(RowId::new(), override_path.clone(), timepoint, 1, cell) + .unwrap(); - ctx.command_sender - .send_system(SystemCommand::UpdateBlueprint( - ctx.store_context.blueprint.store_id().clone(), - vec![row], - )); + ctx.command_sender + .send_system(SystemCommand::UpdateBlueprint( + ctx.store_context.blueprint.store_id().clone(), + vec![row], + )); + } } #[inline] @@ -157,6 +199,13 @@ impl DataResult { &property_overrides.accumulated_properties } + #[inline] + pub fn recursive_properties(&self) -> Option<&EntityProperties> { + self.property_overrides + .as_ref() + .and_then(|p| p.recursive_properties.as_ref()) + } + #[inline] pub fn individual_properties(&self) -> Option<&EntityProperties> { self.property_overrides diff --git a/crates/re_viewport/src/space_view_highlights.rs b/crates/re_viewport/src/space_view_highlights.rs index ef9e8e3d5c0b..462c402ecee1 100644 --- a/crates/re_viewport/src/space_view_highlights.rs +++ b/crates/re_viewport/src/space_view_highlights.rs @@ -33,36 +33,6 @@ pub fn highlights_for_space_view( match current_selection { Item::StoreId(_) | Item::SpaceView(_) | Item::Container(_) => {} - Item::DataBlueprintGroup(group_space_view_id, query_id, group_entity_path) => { - // Unlike for selected objects/data we are more picky for data blueprints with our hover highlights - // since they are truly local to a space view. - if *group_space_view_id == space_view_id { - // Everything in the same group should receive the same selection outline. - // (Due to the way outline masks work in re_renderer, we can't leave the hover channel empty) - let selection_mask = next_selection_mask(); - - let query_result = ctx.lookup_query_result(*query_id).clone(); - - query_result - .tree - .visit_group(group_entity_path, &mut |handle| { - if let Some(result) = query_result.tree.lookup_result(handle) { - let entity_hash = result.entity_path.hash(); - let instance = result.entity_path.clone().into(); - - highlighted_entity_paths - .entry(entity_hash) - .or_default() - .add_selection(&instance, SelectionHighlight::SiblingSelection); - outlines_masks - .entry(entity_hash) - .or_default() - .add(&instance, selection_mask); - } - }); - } - } - Item::ComponentPath(component_path) => { let entity_hash = component_path.entity_path.hash(); let instance = component_path.entity_path.clone().into(); @@ -108,33 +78,6 @@ pub fn highlights_for_space_view( match current_hover { Item::StoreId(_) | Item::SpaceView(_) | Item::Container(_) => {} - Item::DataBlueprintGroup(group_space_view_id, query_id, group_entity_path) => { - // Unlike for selected objects/data we are more picky for data blueprints with our hover highlights - // since they are truly local to a space view. - if *group_space_view_id == space_view_id { - // Everything in the same group should receive the same hover outline. - let hover_mask = next_hover_mask(); - - let query_result = ctx.lookup_query_result(*query_id).clone(); - - query_result - .tree - .visit_group(group_entity_path, &mut |handle| { - if let Some(result) = query_result.tree.lookup_result(handle) { - let instance = result.entity_path.clone().into(); - highlighted_entity_paths - .entry(result.entity_path.hash()) - .or_default() - .add_hover(&instance, HoverHighlight::Hovered); - outlines_masks - .entry(result.entity_path.hash()) - .or_default() - .add(&instance, hover_mask); - } - }); - } - } - Item::ComponentPath(component_path) => { let entity_hash = component_path.entity_path.hash(); let instance = component_path.entity_path.clone().into(); diff --git a/crates/re_viewport/src/viewport_blueprint.rs b/crates/re_viewport/src/viewport_blueprint.rs index db47ab97c615..f6b158217fd4 100644 --- a/crates/re_viewport/src/viewport_blueprint.rs +++ b/crates/re_viewport/src/viewport_blueprint.rs @@ -267,10 +267,6 @@ impl ViewportBlueprint { .map(|space_view_id| self.space_view(&space_view_id).is_some()) .unwrap_or(true), Item::SpaceView(space_view_id) => self.space_view(space_view_id).is_some(), - Item::DataBlueprintGroup(space_view_id, query_id, _entity_path) => self - .space_views - .get(space_view_id) - .map_or(false, |sv| sv.queries.iter().any(|q| q.id == *query_id)), Item::Container(container_id) => self.container(container_id).is_some(), } } @@ -622,11 +618,7 @@ impl ViewportBlueprint { .iter() .filter_map(|(space_view_id, space_view)| { let query_result = ctx.lookup_query_result(space_view.query_id()); - if query_result - .tree - .lookup_result_by_path_and_group(path, false) - .is_some() - { + if query_result.tree.lookup_result_by_path(path).is_some() { Some(*space_view_id) } else { None diff --git a/crates/re_viewport/src/viewport_blueprint_ui.rs b/crates/re_viewport/src/viewport_blueprint_ui.rs index 7616f6d2d111..e5d68cde51f1 100644 --- a/crates/re_viewport/src/viewport_blueprint_ui.rs +++ b/crates/re_viewport/src/viewport_blueprint_ui.rs @@ -1,9 +1,10 @@ use egui::{Response, Ui}; +use itertools::Itertools; + use re_entity_db::InstancePath; use re_log_types::EntityPathRule; -use re_space_view::SpaceViewBlueprint; -use re_space_view::SpaceViewName; +use re_space_view::{SpaceViewBlueprint, SpaceViewName}; use re_ui::{drag_and_drop::DropTarget, list_item::ListItem, ReUi}; use re_viewer_context::{ ContainerId, DataQueryResult, DataResultNode, HoverHighlight, Item, SpaceViewId, ViewerContext, @@ -205,7 +206,7 @@ impl Viewport<'_, '_> { let space_view_visible = visible && container_visible; let item = Item::SpaceView(space_view.id); - let root_node = result_tree.first_interesting_root(); + let root_node = result_tree.root_node(); // empty space views should display as open by default to highlight the fact that they are empty let default_open = root_node.map_or(true, Self::default_open_for_data_result); @@ -289,26 +290,12 @@ impl Viewport<'_, '_> { let query = ctx.current_query(); let store = ctx.entity_db.store(); - let group_is_visible = - top_node.data_result.accumulated_properties().visible && space_view_visible; - - // Always real children ahead of groups - for child in top_node - .children - .iter() - .filter(|c| { - query_result - .tree - .lookup_result(**c) - .map_or(false, |c| !c.is_group) - }) - .chain(top_node.children.iter().filter(|c| { - query_result - .tree - .lookup_result(**c) - .map_or(false, |c| c.is_group) - })) - { + for child in top_node.children.iter().sorted_by_key(|c| { + query_result + .tree + .lookup_result(**c) + .map_or(&space_view.space_origin, |c| &c.entity_path) + }) { let Some(child_node) = query_result.tree.lookup_node(*child) else { debug_assert!(false, "DataResultNode {top_node:?} has an invalid child"); continue; @@ -317,23 +304,19 @@ impl Viewport<'_, '_> { let data_result = &child_node.data_result; let entity_path = &child_node.data_result.entity_path; - let item = if data_result.is_group { - // If we can't find a group_handle for some reason, use the default, null handle. - Item::DataBlueprintGroup(space_view.id, query_result.id, entity_path.clone()) - } else { - Item::InstancePath( - Some(space_view.id), - InstancePath::entity_splat(entity_path.clone()), - ) - }; + let item = Item::InstancePath( + Some(space_view.id), + InstancePath::entity_splat(entity_path.clone()), + ); let is_selected = ctx.selection().contains_item(&item); let is_item_hovered = ctx.selection_state().highlight_for_ui_element(&item) == HoverHighlight::Hovered; - let mut properties = data_result - .individual_properties() + let visible = data_result.accumulated_properties().visible; + let mut recursive_properties = data_result + .recursive_properties() .cloned() .unwrap_or_default(); @@ -342,77 +325,53 @@ impl Viewport<'_, '_> { .last() .map_or("unknown".to_owned(), |e| e.ui_string()); + let subdued = !space_view_visible + || !visible + || (data_result.visualizers.is_empty() && child_node.children.is_empty()); + + let mut remove_entity = false; + let buttons = |re_ui: &_, ui: &mut egui::Ui| { + let vis_response = visibility_button_ui( + re_ui, + ui, + space_view_visible, + &mut recursive_properties.visible, + ); + + let response = remove_button_ui( + re_ui, + ui, + "Remove Group and all its children from the Space View", + ); + remove_entity = response.clicked(); + + response | vis_response + }; + let response = if child_node.children.is_empty() { ListItem::new(ctx.re_ui, name) .selected(is_selected) .with_icon(&re_ui::icons::ENTITY) - .subdued( - !group_is_visible - || !properties.visible - || data_result.visualizers.is_empty(), - ) + .subdued(subdued) .force_hovered(is_item_hovered) - .with_buttons(|re_ui, ui| { - let vis_response = visibility_button_ui( - re_ui, - ui, - group_is_visible, - &mut properties.visible, - ); - - let response = - remove_button_ui(re_ui, ui, "Remove Entity from the Space View"); - if response.clicked() { - space_view.add_entity_exclusion( - ctx, - EntityPathRule::exact(entity_path.clone()), - ); - } - - response | vis_response - }) + .with_buttons(buttons) .show(ui) .on_hover_ui(|ui| { - if data_result.is_group { - ui.label("Group"); - } else { - re_data_ui::item_ui::entity_hover_card_ui( - ui, - ctx, - &query, - store, - entity_path, - ); - } + re_data_ui::item_ui::entity_hover_card_ui( + ui, + ctx, + &query, + store, + entity_path, + ); }) } else { let default_open = Self::default_open_for_data_result(child_node); - let mut remove_group = false; - let response = ListItem::new(ctx.re_ui, name) .selected(is_selected) - .subdued(!properties.visible || !group_is_visible) + .subdued(subdued) .force_hovered(is_item_hovered) - .with_icon(&re_ui::icons::GROUP) - .with_buttons(|re_ui, ui| { - let vis_response = visibility_button_ui( - re_ui, - ui, - group_is_visible, - &mut properties.visible, - ); - - let response = remove_button_ui( - re_ui, - ui, - "Remove Group and all its children from the Space View", - ); - if response.clicked() { - remove_group = true; - } - - response | vis_response - }) + .with_buttons(buttons) .show_collapsing( ui, ui.id().with(&child_node.data_result.entity_path), @@ -430,29 +389,26 @@ impl Viewport<'_, '_> { ) .item_response .on_hover_ui(|ui| { - if data_result.is_group { - ui.label("Group"); - } else { - re_data_ui::item_ui::entity_hover_card_ui( - ui, - ctx, - &query, - store, - entity_path, - ); - } + re_data_ui::item_ui::entity_hover_card_ui( + ui, + ctx, + &query, + store, + entity_path, + ); }); - if remove_group { - space_view.add_entity_exclusion( - ctx, - EntityPathRule::including_subtree(entity_path.clone()), - ); - } - response }; - data_result.save_override(Some(properties), ctx); + + if remove_entity { + space_view.add_entity_exclusion( + ctx, + EntityPathRule::including_subtree(entity_path.clone()), + ); + } + + data_result.save_recursive_override(ctx, Some(recursive_properties)); ctx.select_hovered_on_click(&response, item); }