Skip to content

Commit

Permalink
Entity path query now shows simple statistics and warns if nothing is…
Browse files Browse the repository at this point in the history
… displayed (#5693)

### What

* Fixes  #5669


https://github.com/rerun-io/rerun/assets/1220815/91c63fbb-4b6e-4eb9-a5b5-3d14ea41d412

a small step towards making it easy to debug why no data might be shown


### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using newly built examples:
[app.rerun.io](https://app.rerun.io/pr/5693/index.html)
* Using examples from latest `main` build:
[app.rerun.io](https://app.rerun.io/pr/5693/index.html?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[app.rerun.io](https://app.rerun.io/pr/5693/index.html?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!

- [PR Build Summary](https://build.rerun.io/pr/5693)
- [Docs
preview](https://rerun.io/preview/b9425379e1b41bf8f1891ffada4e6e0d15edc3cc/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/b9425379e1b41bf8f1891ffada4e6e0d15edc3cc/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

---------

Co-authored-by: Emil Ernerfeldt <[email protected]>
  • Loading branch information
Wumpf and emilk authored Mar 27, 2024
1 parent a06cbdf commit a1f59db
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 19 deletions.
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:?}\"."),
));
}

// 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

0 comments on commit a1f59db

Please sign in to comment.