Skip to content

Commit

Permalink
More context menu 1: refactor (#5392)
Browse files Browse the repository at this point in the history
### What

More like a rewrite actually. This PR completely reorganise the way
context menu actions are written and executed with two goals:
- Keep the complexity in check: the previous `summarize_selection`
mechanism was exposed to combinatorial explosion of possible item
subset.
- Improve locality: every details of if/when/how an action is displayed
and run is now local to the action `impl`s, not dispatched in various
functions such as `summarize_selection` etc.

All in all, this should ensure a O(1) effort to add new actions.

@ Reviewer: with the renames and rewrite, the diff a probably useless
and this PR is best reviewed by directly looking at `context_menu/*.rs`.

### 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/5392/index.html)
* Using examples from latest `main` build:
[app.rerun.io](https://app.rerun.io/pr/5392/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/5392/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/5392)
- [Docs
preview](https://rerun.io/preview/db9a05278dd4936e5224b6331143eea369ff61ad/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/db9a05278dd4936e5224b6331143eea369ff61ad/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 5, 2024
1 parent df7f46f commit 9d0d2a3
Show file tree
Hide file tree
Showing 8 changed files with 587 additions and 500 deletions.
20 changes: 20 additions & 0 deletions crates/re_viewport/src/container.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,26 @@ impl Contents {
}
}

impl TryFrom<Item> for Contents {
type Error = ();

fn try_from(item: Item) -> Result<Self, Self::Error> {
(&item).try_into()
}
}

impl TryFrom<&Item> for Contents {
type Error = ();

fn try_from(item: &Item) -> Result<Self, Self::Error> {
match item {
Item::Container(id) => Ok(Self::Container(*id)),
Item::SpaceView(id) => Ok(Self::SpaceView(*id)),
_ => Err(()),
}
}
}

#[inline]
pub fn blueprint_id_to_tile_id<T: BlueprintIdRegistry>(id: &BlueprintId<T>) -> TileId {
TileId::from_u64(id.hash())
Expand Down
311 changes: 311 additions & 0 deletions crates/re_viewport/src/context_menu/actions.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,311 @@
use re_log_types::{EntityPath, EntityPathFilter};
use re_space_view::{DataQueryBlueprint, SpaceViewBlueprint};
use re_viewer_context::{ContainerId, Item, SpaceViewClassIdentifier, SpaceViewId};

use super::{ContextMenuAction, ContextMenuContext};
use crate::Contents;

pub(super) struct ShowAction;

impl ContextMenuAction for ShowAction {
fn supports_selection(&self, ctx: &ContextMenuContext<'_>) -> bool {
ctx.selection.iter().any(|(item, _)| match item {
Item::SpaceView(space_view_id) => !ctx
.viewport_blueprint
.is_contents_visible(&Contents::SpaceView(*space_view_id)),
Item::Container(container_id) => {
!ctx.viewport_blueprint
.is_contents_visible(&Contents::Container(*container_id))
&& ctx.viewport_blueprint.root_container != Some(*container_id)
}
_ => false,
})
}

fn label(&self, ctx: &ContextMenuContext<'_>) -> String {
if ctx.selection.len() > 1 {
"Show All".to_owned()
} else {
"Show".to_owned()
}
}

fn process_container(&self, ctx: &ContextMenuContext<'_>, container_id: &ContainerId) {
ctx.viewport_blueprint.set_content_visibility(
ctx.viewer_context,
&Contents::Container(*container_id),
true,
);
}

fn process_space_view(&self, ctx: &ContextMenuContext<'_>, space_view_id: &SpaceViewId) {
ctx.viewport_blueprint.set_content_visibility(
ctx.viewer_context,
&Contents::SpaceView(*space_view_id),
true,
);
}
}

// ---

pub(super) struct HideAction;

impl ContextMenuAction for HideAction {
fn supports_selection(&self, ctx: &ContextMenuContext<'_>) -> bool {
ctx.selection.iter().any(|(item, _)| match item {
Item::SpaceView(space_view_id) => ctx
.viewport_blueprint
.is_contents_visible(&Contents::SpaceView(*space_view_id)),
Item::Container(container_id) => {
ctx.viewport_blueprint
.is_contents_visible(&Contents::Container(*container_id))
&& ctx.viewport_blueprint.root_container != Some(*container_id)
}
_ => false,
})
}

fn label(&self, ctx: &ContextMenuContext<'_>) -> String {
if ctx.selection.len() > 1 {
"Hide All".to_owned()
} else {
"Hide".to_owned()
}
}

fn process_container(&self, ctx: &ContextMenuContext<'_>, container_id: &ContainerId) {
ctx.viewport_blueprint.set_content_visibility(
ctx.viewer_context,
&Contents::Container(*container_id),
false,
);
}

fn process_space_view(&self, ctx: &ContextMenuContext<'_>, space_view_id: &SpaceViewId) {
ctx.viewport_blueprint.set_content_visibility(
ctx.viewer_context,
&Contents::SpaceView(*space_view_id),
false,
);
}
}

// ---

/// Remove a container or space view
pub(super) struct RemoveAction;

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

fn supports_item(&self, ctx: &ContextMenuContext<'_>, item: &Item) -> bool {
match item {
Item::SpaceView(_) => true,
Item::Container(container_id) => {
ctx.viewport_blueprint.root_container != Some(*container_id)
}
_ => false,
}
}

fn label(&self, _ctx: &ContextMenuContext<'_>) -> String {
"Remove".to_owned()
}

fn process_container(&self, ctx: &ContextMenuContext<'_>, container_id: &ContainerId) {
ctx.viewport_blueprint
.mark_user_interaction(ctx.viewer_context);
ctx.viewport_blueprint
.remove_contents(Contents::Container(*container_id));
}

fn process_space_view(&self, ctx: &ContextMenuContext<'_>, space_view_id: &SpaceViewId) {
ctx.viewport_blueprint
.mark_user_interaction(ctx.viewer_context);
ctx.viewport_blueprint
.remove_contents(Contents::SpaceView(*space_view_id));
}
}

// ---

/// Clone a single space view
pub(super) struct CloneSpaceViewAction;

impl ContextMenuAction for CloneSpaceViewAction {
fn supports_item(&self, _ctx: &ContextMenuContext<'_>, item: &Item) -> bool {
matches!(item, Item::SpaceView(_))
}

fn label(&self, _ctx: &ContextMenuContext<'_>) -> String {
"Clone".to_owned()
}

fn process_space_view(&self, ctx: &ContextMenuContext<'_>, space_view_id: &SpaceViewId) {
if let Some(new_space_view_id) = ctx
.viewport_blueprint
.duplicate_space_view(space_view_id, ctx.viewer_context)
{
ctx.viewer_context
.selection_state()
.set_selection(Item::SpaceView(new_space_view_id));
ctx.viewport_blueprint
.mark_user_interaction(ctx.viewer_context);
}
}
}

// ---

/// Add a container of a specific type
pub(super) struct AddContainerAction(pub egui_tiles::ContainerKind);

impl ContextMenuAction for AddContainerAction {
fn supports_selection(&self, ctx: &ContextMenuContext<'_>) -> bool {
if let Some(Item::Container(container_id)) = ctx.selection.single_item() {
if let Some(container) = ctx.viewport_blueprint.container(container_id) {
// same-kind linear containers cannot be nested
(container.container_kind != egui_tiles::ContainerKind::Vertical
&& container.container_kind != egui_tiles::ContainerKind::Horizontal)
|| container.container_kind != self.0
} else {
// unknown container
false
}
} else {
false
}
}

fn label(&self, _ctx: &ContextMenuContext<'_>) -> String {
format!("{:?}", self.0)
}

fn process_container(&self, ctx: &ContextMenuContext<'_>, container_id: &ContainerId) {
ctx.viewport_blueprint
.add_container(self.0, Some(*container_id));
ctx.viewport_blueprint
.mark_user_interaction(ctx.viewer_context);
}
}

// ---

/// Add a space view of the specific class
pub(super) struct AddSpaceViewAction(pub SpaceViewClassIdentifier);

impl ContextMenuAction for AddSpaceViewAction {
fn supports_item(&self, _ctx: &ContextMenuContext<'_>, item: &Item) -> bool {
matches!(item, Item::Container(_))
}

fn label(&self, ctx: &ContextMenuContext<'_>) -> String {
ctx.viewer_context
.space_view_class_registry
.get_class_or_log_error(&self.0)
.display_name()
.to_owned()
}

fn process_container(&self, ctx: &ContextMenuContext<'_>, container_id: &ContainerId) {
let space_view = SpaceViewBlueprint::new(
self.0,
&EntityPath::root(),
DataQueryBlueprint::new(self.0, EntityPathFilter::default()),
);

ctx.viewport_blueprint.add_space_views(
std::iter::once(space_view),
ctx.viewer_context,
Some(*container_id),
None,
);
ctx.viewport_blueprint
.mark_user_interaction(ctx.viewer_context);
}
}

// ---

/// Move the selected contents to a newly created container of the given kind
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;
}
}
}

ctx.selection.iter().all(|(item, _)| match item {
Item::SpaceView(_) => true,
Item::Container(container_id) => {
ctx.viewport_blueprint.root_container != Some(*container_id)
}
_ => false,
})
}

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

fn supports_item(&self, ctx: &ContextMenuContext<'_>, item: &Item) -> bool {
match item {
Item::SpaceView(_) => true,
Item::Container(container_id) => {
ctx.viewport_blueprint.root_container != Some(*container_id)
}
_ => false,
}
}

fn label(&self, _ctx: &ContextMenuContext<'_>) -> String {
format!("{:?}", self.0)
}

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 contents = ctx
.selection
.iter()
.filter_map(|(item, _)| item.try_into().ok())
.collect();

ctx.viewport_blueprint.move_contents_to_new_container(
contents,
self.0,
target_container_id,
target_position,
);

ctx.viewport_blueprint
.mark_user_interaction(ctx.viewer_context);
}
}
}

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))
}
}
Loading

0 comments on commit 9d0d2a3

Please sign in to comment.