Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Entity path query now shows simple statistics and warns if nothing is displayed #5693

Merged
merged 5 commits into from
Mar 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 25 additions & 8 deletions crates/re_space_view/src/space_view_contents.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,14 +223,22 @@ impl DataQuery for SpaceViewContents {
let executor =
QueryExpressionEvaluator::new(self, visualizable_entities_for_visualizer_systems);

let mut num_matching_entities = 0;
let mut num_visualized_entities = 0;
let root_handle = {
re_tracing::profile_scope!("add_entity_tree_to_data_results_recursive");
executor
.add_entity_tree_to_data_results_recursive(ctx.recording.tree(), &mut data_results)
executor.add_entity_tree_to_data_results_recursive(
ctx.recording.tree(),
&mut data_results,
&mut num_matching_entities,
&mut num_visualized_entities,
)
};

DataQueryResult {
tree: DataResultTree::new(data_results, root_handle),
num_matching_entities,
num_visualized_entities,
}
}
}
Expand Down Expand Up @@ -262,6 +270,8 @@ impl<'a> QueryExpressionEvaluator<'a> {
&self,
tree: &EntityTree,
data_results: &mut SlotMap<DataResultHandle, DataResultNode>,
num_matching_entities: &mut usize,
num_visualized_entities: &mut usize,
) -> Option<DataResultHandle> {
// Early-out optimization
if !self
Expand All @@ -275,27 +285,34 @@ impl<'a> QueryExpressionEvaluator<'a> {

let entity_path = &tree.path;

let tree_prefix_only = !self.entity_path_filter.is_included(entity_path);
let matches_filter = self.entity_path_filter.is_included(entity_path);
*num_matching_entities += matches_filter as usize;

// 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]> = if tree_prefix_only {
Default::default()
} else {
let visualizers: SmallVec<[_; 4]> = if matches_filter {
self.visualizable_entities_for_visualizer_systems
.iter()
.filter_map(|(visualizer, ents)| ents.contains(entity_path).then_some(*visualizer))
.collect()
} else {
Default::default()
};
*num_visualized_entities += !visualizers.is_empty() as usize;

let children: SmallVec<[_; 4]> = tree
.children
.values()
.filter_map(|subtree| {
self.add_entity_tree_to_data_results_recursive(subtree, data_results)
self.add_entity_tree_to_data_results_recursive(
subtree,
data_results,
num_matching_entities,
num_visualized_entities,
)
})
.collect();

Expand All @@ -308,7 +325,7 @@ impl<'a> QueryExpressionEvaluator<'a> {
data_result: DataResult {
entity_path: entity_path.clone(),
visualizers,
tree_prefix_only,
tree_prefix_only: !matches_filter,
property_overrides: None,
},
children,
Expand Down
43 changes: 32 additions & 11 deletions crates/re_viewer/src/ui/selection_panel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -819,11 +819,11 @@ fn blueprint_ui(
) {
match item {
Item::SpaceView(space_view_id) => {
blueprint_ui_for_space_view(ui, ctx, viewport, space_view_id);
blueprint_ui_for_space_view(ui, ctx, viewport, *space_view_id);
}

Item::DataResult(space_view_id, instance_path) => {
blueprint_ui_for_data_result(ui, ctx, viewport, space_view_id, instance_path);
blueprint_ui_for_data_result(ui, ctx, viewport, *space_view_id, instance_path);
}

Item::DataSource(_)
Expand All @@ -838,14 +838,16 @@ fn blueprint_ui_for_space_view(
ui: &mut Ui,
ctx: &ViewerContext<'_>,
viewport: &mut Viewport<'_, '_>,
space_view_id: &SpaceViewId,
space_view_id: SpaceViewId,
) {
if let Some(space_view) = viewport.blueprint.space_view(space_view_id) {
if let Some(space_view) = viewport.blueprint.space_view(&space_view_id) {
if let Some(new_entity_path_filter) = entity_path_filter_ui(
ui,
ctx,
viewport,
space_view_id,
&space_view.contents.entity_path_filter,
&space_view.space_origin,
) {
space_view
.contents
Expand All @@ -862,7 +864,8 @@ fn blueprint_ui_for_space_view(
)
.clicked()
{
if let Some(new_space_view_id) = viewport.blueprint.duplicate_space_view(space_view_id, ctx)
if let Some(new_space_view_id) =
viewport.blueprint.duplicate_space_view(&space_view_id, ctx)
{
ctx.selection_state()
.set_selection(Item::SpaceView(new_space_view_id));
Expand All @@ -874,7 +877,7 @@ fn blueprint_ui_for_space_view(
ReUi::full_span_separator(ui);
ui.add_space(ui.spacing().item_spacing.y / 2.0);

if let Some(space_view) = viewport.blueprint.space_view(space_view_id) {
if let Some(space_view) = viewport.blueprint.space_view(&space_view_id) {
let class_identifier = *space_view.class_identifier();

let space_view_state = viewport.state.space_view_state_mut(
Expand Down Expand Up @@ -925,10 +928,10 @@ fn blueprint_ui_for_data_result(
ui: &mut Ui,
ctx: &ViewerContext<'_>,
viewport: &Viewport<'_, '_>,
space_view_id: &SpaceViewId,
space_view_id: SpaceViewId,
instance_path: &InstancePath,
) {
if let Some(space_view) = viewport.blueprint.space_view(space_view_id) {
if let Some(space_view) = viewport.blueprint.space_view(&space_view_id) {
if instance_path.instance_key.is_splat() {
// splat - the whole entity
let space_view_class = *space_view.class_identifier();
Expand All @@ -948,7 +951,7 @@ fn blueprint_ui_for_data_result(
entity_props_ui(
ctx,
ui,
ctx.lookup_query_result(*space_view_id),
ctx.lookup_query_result(space_view_id),
&space_view_class,
entity_path,
&mut props,
Expand All @@ -962,9 +965,11 @@ fn blueprint_ui_for_data_result(
/// Returns a new filter when the editing is done, and there has been a change.
fn entity_path_filter_ui(
ui: &mut egui::Ui,
ctx: &ViewerContext<'_>,
viewport: &mut Viewport<'_, '_>,
space_view_id: &SpaceViewId,
space_view_id: SpaceViewId,
filter: &EntityPathFilter,
origin: &EntityPath,
) -> Option<EntityPathFilter> {
fn entity_path_filter_help_ui(ui: &mut egui::Ui) {
let markdown = r#"
Expand Down Expand Up @@ -1077,7 +1082,7 @@ The last rule matching `/world/house` is `+ /world/**`, so it is included.
.on_hover_text("Modify the entity query using the editor")
.clicked()
{
viewport.show_add_remove_entities_modal(*space_view_id);
viewport.show_add_remove_entities_modal(space_view_id);
}
},
);
Expand All @@ -1093,6 +1098,22 @@ The last rule matching `/world/house` is `+ /world/**`, so it is included.
ui.data_mut(|data| data.remove::<String>(filter_text_id));
}

// Show some statistics about the query, print a warning text if something seems off.
let query = ctx.lookup_query_result(space_view_id);
if query.num_matching_entities == 0 {
ui.label(ctx.re_ui.warning_text("Does not match any entity"));
} else if query.num_matching_entities == 1 {
ui.label("Matches 1 entity");
} else {
ui.label(format!("Matches {} entities", query.num_matching_entities));
}
if query.num_matching_entities != 0 && query.num_visualized_entities == 0 {
// TODO(andreas): Talk about this root bit only if it's a spatial view.
ui.label(ctx.re_ui.warning_text(
format!("This space view is not able to visualize any of the matched entities using the current root \"{origin:?}\"."),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could probably use a tooltip that explains it more in detail

));
}

// Apply the edit.
let new_filter = EntityPathFilter::parse_forgiving(&filter_string, &Default::default());
if &new_filter == filter {
Expand Down
11 changes: 11 additions & 0 deletions crates/re_viewer_context/src/query_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,15 @@ slotmap::new_key_type! {
pub struct DataQueryResult {
/// The [`DataResultTree`] for the query
pub tree: DataResultTree,

/// The number of entities that matched the query, including those that are not visualizable.
pub num_matching_entities: usize,

/// Of the matched queries, the number of entities that are visualizable by any given visualizer.
///
/// This does *not* take into account the actual selection of visualizers
/// which may be an explicit none for any given entity.
pub num_visualized_entities: usize,
}

impl DataQueryResult {
Expand All @@ -38,6 +47,8 @@ impl Clone for DataQueryResult {
re_tracing::profile_function!();
Self {
tree: self.tree.clone(),
num_matching_entities: self.num_matching_entities,
num_visualized_entities: self.num_visualized_entities,
}
}
}
Expand Down
Loading