Skip to content

Commit

Permalink
More context menu 4: create a new space view with selected entities (#…
Browse files Browse the repository at this point in the history
…5411)

### What

As the title says ☝🏻 

* Fixes #5299

Also adds a "no action available for this selection" notice that got
lost in #5392.

Makes it very easy to bump into:
- #5410


https://github.com/rerun-io/rerun/assets/49431240/d3bafbf4-3755-4f6f-b236-bd1d022b172f


#### Design decisions

- The origin of the newly created space view is set to "/"
- Alternative 1: set it to the clicked item. Strong reject: too arcane,
different results for the same multi-selection depending on which item
is actually clicked.
- Alternative 2: set it to the common ancestor of all selected entities.
Weak reject: less predictable, occasionally wrong (but works around some
visualisable issue we have with some space views).
- We show a list of suggested space view classes.
- The list is the *intersection* of the suggested classes for each of
the selected entities.
- For each entity, the suggested classes are determined based on the
*union* of suggested classes for the entity itself, *and for every
entity of its subtree*. This enables meaningful suggestion when
selecting a pure TreePrefix.
- The newly created space view is selected.

#### Known "phenomenons"

- 2D space views are rarely suggested, because of the origin is set to
"/" and that's outside of a pinhole transform.
  - TODO: issue number?
- Text Document and Text Log are often suggested for time series scalar,
because of the `Text` document.
- Tensor is recommended but will (sometime?) display nothing, e.g.
`structure_from_motion` -> `/camera/image`
- If enabled, Dataframe is always the top-most suggested Space View,
because of the lexicographic sorting.

### 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/5411/index.html)
* Using examples from latest `main` build:
[app.rerun.io](https://app.rerun.io/pr/5411/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/5411/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/5411)
- [Docs
preview](https://rerun.io/preview/979f2768f87bd3e72b45c80ffd319b56661f138e/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/979f2768f87bd3e72b45c80ffd319b56661f138e/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
  • Loading branch information
abey79 authored Mar 7, 2024
1 parent 2a59c08 commit 5eebbac
Show file tree
Hide file tree
Showing 3 changed files with 264 additions and 25 deletions.
189 changes: 167 additions & 22 deletions crates/re_viewport/src/context_menu/actions.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
use egui::{Response, Ui};
use itertools::Itertools;
use nohash_hasher::IntSet;

use re_entity_db::InstancePath;
use re_log_types::{EntityPath, EntityPathFilter, EntityPathRule};
use re_space_view::SpaceViewBlueprint;
use re_log_types::{EntityPath, EntityPathFilter, EntityPathRule, RuleEffect};
use re_space_view::{determine_visualizable_entities, SpaceViewBlueprint};
use re_viewer_context::{ContainerId, Item, SpaceViewClassIdentifier, SpaceViewId};

use super::{ContextMenuAction, ContextMenuContext};
Expand Down Expand Up @@ -312,15 +316,12 @@ pub(super) struct MoveContentsToNewContainerAction(pub egui_tiles::ContainerKind

impl ContextMenuAction for MoveContentsToNewContainerAction {
fn supports_selection(&self, ctx: &ContextMenuContext<'_>) -> bool {
if let Some((parent_container_id, _)) = Self::target_container_id_and_position(ctx) {
if let Some(parent_container) = ctx.viewport_blueprint.container(&parent_container_id) {
// same-kind linear containers cannot be nested
if (parent_container.container_kind == egui_tiles::ContainerKind::Vertical
|| parent_container.container_kind == egui_tiles::ContainerKind::Horizontal)
&& parent_container.container_kind == self.0
{
return false;
}
if let Some((parent_container, _)) = ctx.clicked_item_enclosing_container_and_position() {
if (parent_container.container_kind == egui_tiles::ContainerKind::Vertical
|| parent_container.container_kind == egui_tiles::ContainerKind::Horizontal)
&& parent_container.container_kind == self.0
{
return false;
}
}

Expand Down Expand Up @@ -353,8 +354,9 @@ impl ContextMenuAction for MoveContentsToNewContainerAction {

fn process_selection(&self, ctx: &ContextMenuContext<'_>) {
if let Some(root_container_id) = ctx.viewport_blueprint.root_container {
let (target_container_id, target_position) =
Self::target_container_id_and_position(ctx).unwrap_or((root_container_id, 0));
let (target_container_id, target_position) = ctx
.clicked_item_enclosing_container_id_and_position()
.unwrap_or((root_container_id, 0));

let contents = ctx
.selection
Expand All @@ -375,14 +377,157 @@ impl ContextMenuAction for MoveContentsToNewContainerAction {
}
}

impl MoveContentsToNewContainerAction {
fn target_container_id_and_position(
ctx: &ContextMenuContext<'_>,
) -> Option<(ContainerId, usize)> {
ctx.clicked_item
.clone()
.try_into()
.ok()
.and_then(|c| ctx.viewport_blueprint.find_parent_and_position_index(&c))
// ---

/// Create a new space view containing the selected entities.
///
/// The space view is created next to the clicked item's parent view (if a data result was clicked).
pub(super) struct AddEntitiesToNewSpaceViewAction;

impl ContextMenuAction for AddEntitiesToNewSpaceViewAction {
fn supports_multi_selection(&self, _ctx: &ContextMenuContext<'_>) -> bool {
true
}

fn supports_item(&self, _ctx: &ContextMenuContext<'_>, item: &Item) -> bool {
matches!(item, Item::DataResult(_, _) | Item::InstancePath(_))
}

fn ui(&self, ctx: &ContextMenuContext<'_>, ui: &mut Ui) -> Response {
let space_view_class_registry = ctx.viewer_context.space_view_class_registry;

let recommended_space_view_classes = recommended_space_views_for_selection(ctx);
let other_space_view_classes: IntSet<_> = space_view_class_registry
.iter_registry()
.map(|entry| entry.class.identifier())
.collect::<IntSet<SpaceViewClassIdentifier>>()
.difference(&recommended_space_view_classes)
.cloned()
.collect();

ui.menu_button("Add to new space view", |ui| {
let buttons_for_space_view_classes =
|ui: &mut egui::Ui, space_view_classes: &IntSet<SpaceViewClassIdentifier>| {
for (identifier, display_name) in space_view_classes
.iter()
.map(|identifier| {
(
identifier,
space_view_class_registry
.get_class_or_log_error(identifier)
.display_name(),
)
})
.sorted_by_key(|(_, display_name)| display_name.to_owned())
{
if ui.button(display_name).clicked() {
create_space_view_for_selected_entities(ctx, *identifier);
ui.close_menu();
}
}
};

ui.label(egui::WidgetText::from("Recommended:").italics());
if recommended_space_view_classes.is_empty() {
ui.label("None");
} else {
buttons_for_space_view_classes(ui, &recommended_space_view_classes);
}

if !other_space_view_classes.is_empty() {
ui.label(egui::WidgetText::from("Others:").italics());
buttons_for_space_view_classes(ui, &other_space_view_classes);
}
})
.response
}
}

/// Builds a list of compatible space views for the provided selection.
fn recommended_space_views_for_selection(
ctx: &ContextMenuContext<'_>,
) -> IntSet<SpaceViewClassIdentifier> {
re_tracing::profile_function!();

let entities_of_interest = ctx
.selection
.iter()
.filter_map(|(item, _)| item.entity_path())
.collect::<Vec<_>>();

let mut output: IntSet<SpaceViewClassIdentifier> = IntSet::default();

let space_view_class_registry = ctx.viewer_context.space_view_class_registry;
let entity_db = ctx.viewer_context.entity_db;
let applicable_entities_per_visualizer =
space_view_class_registry.applicable_entities_for_visualizer_systems(entity_db.store_id());

for entry in space_view_class_registry.iter_registry() {
let visualizable_entities = determine_visualizable_entities(
&applicable_entities_per_visualizer,
entity_db,
&space_view_class_registry.new_visualizer_collection(entry.class.identifier()),
&*entry.class,
&EntityPath::root(),
);

// We consider a space view class to be recommended if all selected entities are
// "visualizable" with it. By "visualizable" we mean that either the entity itself, or any
// of its sub-entities, are visualizable.

let covered = entities_of_interest.iter().all(|entity| {
visualizable_entities.0.iter().any(|(_, entities)| {
entities
.0
.iter()
.any(|visualizable_entity| visualizable_entity.starts_with(entity))
})
});

if covered {
output.insert(entry.class.identifier());
}
}

output
}

/// Creates a space view of the given class, with root set as origin, and a filter set to include all
/// selected entities. Then, the selection is set to the new space view.
fn create_space_view_for_selected_entities(
ctx: &ContextMenuContext<'_>,
identifier: SpaceViewClassIdentifier,
) {
let origin = EntityPath::root();

let mut filter = EntityPathFilter::default();
ctx.selection
.iter()
.filter_map(|(item, _)| item.entity_path())
.for_each(|path| {
filter.add_rule(
RuleEffect::Include,
EntityPathRule::including_subtree(path.clone()),
);
});

let target_container_id = ctx
.clicked_item_enclosing_container_id_and_position()
.map(|(id, _)| id);

let space_view = SpaceViewBlueprint::new(identifier, &origin, filter);

let new_space_view = ctx.viewport_blueprint.add_space_views(
std::iter::once(space_view),
ctx.viewer_context,
target_container_id,
None,
);
if let Some(space_view_id) = new_space_view.first() {
ctx.viewer_context
.selection_state()
.set_selection(Item::SpaceView(*space_view_id));
}
ctx.viewport_blueprint
.mark_user_interaction(ctx.viewer_context);
}
45 changes: 42 additions & 3 deletions crates/re_viewport/src/context_menu/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@ use once_cell::sync::OnceCell;
use re_entity_db::InstancePath;
use re_viewer_context::{ContainerId, Item, Selection, SpaceViewId, ViewerContext};

use crate::ViewportBlueprint;
use crate::{ContainerBlueprint, Contents, ViewportBlueprint};

mod actions;
mod sub_menu;

use actions::{
AddContainerAction, AddSpaceViewAction, CloneSpaceViewAction, HideAction,
MoveContentsToNewContainerAction, RemoveAction, ShowAction,
AddContainerAction, AddEntitiesToNewSpaceViewAction, AddSpaceViewAction, CloneSpaceViewAction,
HideAction, MoveContentsToNewContainerAction, RemoveAction, ShowAction,
};
use sub_menu::SubMenu;

Expand Down Expand Up @@ -143,6 +143,7 @@ fn action_list(
)),
],
})],
vec![Box::new(AddEntitiesToNewSpaceViewAction)],
]
})
}
Expand Down Expand Up @@ -173,6 +174,12 @@ fn show_context_menu_for_selection(ctx: &ContextMenuContext<'_>, ui: &mut egui::

should_display_separator |= any_action_displayed;
}

// If anything was shown, then `should_display_separator` has to be true. We can therefore
// recycle this flag for the empty menu message.
if !should_display_separator {
ui.label(egui::RichText::from("No action available for the current selection").italics());
}
}

/// Context information provided to context menu actions
Expand All @@ -183,6 +190,38 @@ struct ContextMenuContext<'a> {
clicked_item: &'a Item,
}

impl<'a> ContextMenuContext<'a> {
/// Return the clicked item's parent container id and position within it.
///
/// Valid only for space views, containers, and data results. For data results, the parent and
/// position of the enclosing space view is considered.
pub fn clicked_item_enclosing_container_id_and_position(&self) -> Option<(ContainerId, usize)> {
match self.clicked_item {
Item::SpaceView(space_view_id) | Item::DataResult(space_view_id, _) => {
Some(Contents::SpaceView(*space_view_id))
}
Item::Container(container_id) => Some(Contents::Container(*container_id)),
_ => None,
}
.and_then(|c: Contents| self.viewport_blueprint.find_parent_and_position_index(&c))
}

/// Return the clicked item's parent container and position within it.
///
/// Valid only for space views, containers, and data results. For data results, the parent and
/// position of the enclosing space view is considered.
pub fn clicked_item_enclosing_container_and_position(
&self,
) -> Option<(&'a ContainerBlueprint, usize)> {
self.clicked_item_enclosing_container_id_and_position()
.and_then(|(container_id, pos)| {
self.viewport_blueprint
.container(&container_id)
.map(|container| (container, pos))
})
}
}

/// Context menu actions must implement this trait.
///
/// Actions must do three things, corresponding to three core methods:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
from __future__ import annotations

import os
import random
from argparse import Namespace
from uuid import uuid4

import numpy as np
import rerun as rr

README = """
# Context Menu - Add entity to new space view
* Reset the blueprint.
* Expend all space views and data result.
* Right-click on the `boxes3d` entity and select "Add to new space view" -> "3D". Check a new space view is created _and selected_ with the boxes3d entity and origin set to root.
* In each space view, right-click on the leaf entity, and check that "Add to new space view" recommends at least space views of the same kind.
* Select both the `boxes3d` entity and the `text_logs` entity. Check no space view is recommended (except Dataframe if enabled).
"""


def log_readme() -> None:
rr.log("readme", rr.TextDocument(README, media_type=rr.MediaType.MARKDOWN), timeless=True)


def log_some_space_views() -> None:
rr.set_time_sequence("frame_nr", 0)

rr.log("boxes3d", rr.Boxes3D(centers=[[0, 0, 0], [1, 1.5, 1.15], [3, 2, 1]], half_sizes=[0.5, 1, 0.5] * 3))
rr.log("boxes2d", rr.Boxes2D(centers=[[0, 0], [1.3, 0.5], [3, 2]], half_sizes=[0.5, 1] * 3))
rr.log("text_logs", rr.TextLog("Hello, world!", level=rr.TextLogLevel.INFO))
rr.log("bars", rr.BarChart([1, 2, 3, 4, 5]))
rr.log("tensor", rr.Tensor(np.random.rand(3, 4, 5)))

for i in range(10):
rr.set_time_sequence("frame_nr", i)
rr.log("timeseries", rr.TimeSeriesScalar(random.randint(0, 100)))


def run(args: Namespace) -> None:
# TODO(cmc): I have no idea why this works without specifying a `recording_id`, but
# I'm not gonna rely on it anyway.
rr.script_setup(args, f"{os.path.basename(__file__)}", recording_id=uuid4())

log_readme()
log_some_space_views()


if __name__ == "__main__":
import argparse

parser = argparse.ArgumentParser(description="Interactive release checklist")
rr.script_add_args(parser)
args = parser.parse_args()
run(args)

0 comments on commit 5eebbac

Please sign in to comment.