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

Entities can be dragged from the blueprint tree and streams tree to an existing view in the viewport #8431

Merged
merged 24 commits into from
Dec 17, 2024

Conversation

abey79
Copy link
Member

@abey79 abey79 commented Dec 12, 2024

Related

What

This PR makes it possible to drag one or more entities from the blueprint and streams tree to existing views in the viewport. This involves introducing and/or adjusting a whole bunch of semantics around selection and dragging.

DragAndDropFeedback

This PR introduced the notion of drag-and-drop feedback from the hovered ui element.

Feedback may be either of:

  • Ignore (default): hovered element is uninterested in the type of drag payload, or in any payload at all, or is outright oblivious to all that drag-and-drop stuff.
  • Reject: hovered element is compatible with the type of drag payload, but not its actual content. For example, a view might already contains the dragged entities.
  • Accept: hovered element is compatible with both the type and the content of the payload.

A drop should only ever happen in the latter case.

Payload visualisation

The payload pill display is now adjusted based on the feedback, both its opacity (ranging from 50 to 80%) and its colour (grey or blue). The mouse cursor is also adjusted based on the feedback.

Drop container visualisation

This PR slightly adjust the look of the drop container visualisation: the blue frame is now 2px wide.

Note that the drop container is not necessarily the thing that's hovered by the mouse, see e.g. containers in the blueprint.

Selection handling during drag-and-drop

This PR slightly alter the current behaviour. Now:

  • dragging a selected item drags the entire selection
  • dragging an unselected item with cmd held adds that item to the selection, and drags the entire selection
  • dragging an unselected item drags that item, without changing the selection (new)

Entity-to-viewport-view drag semantics

This is the original goal of this PR.

  • An existing view will accept a payload containing entities IFF any of these entities—or their children—is both visualisable and not already contained in that view.
  • An existing view will reject a payload containing entities IFF all of these entities are either non-visualisable or already contained.
  • An existing view will ignore a payload containing anything else.
  • When a drop succeeds:
    • The view will add an inclusive ("…/**") rule for each of the dropped entities that are both visualisable and not already included.
    • The view becomes selected.

Emphasis on that last point. This subtle UX behaviour (courtesy of @gavrelina) makes the drop success and impact on the entity path filter more explicit.

Theory of operation for drag-and-drop

With this PR, a "framework" slowly starts to emerge. For now, it's mainly this bit of documentation:

//! ## Theory of operation
//!
//! ### Setup
//!
//! A [`DragAndDropManager`] should be created at the start of the frame and made available to the
//! entire UI code.
//!
//!
//! ### Initiating a drag
//!
//! Any UI representation of an [`crate::Item`] may initiate a drag.
//! [`crate::ViewerContext::handle_select_hover_drag_interactions`] will handle that automatically
//! when passed `true` for its `draggable` argument.
//!
//!
//! ### Reacting to a drag and accepting a drop
//!
//! This part of the process is more involved and typically includes the following steps:
//!
//! 1. When hovered, the receiving UI element should check for a compatible payload using
//!    [`egui::DragAndDrop::payload`] and matching one or more variants of the returned
//!    [`DragAndDropPayload`], if any.
//!
//! 2. If an acceptable payload type is being dragged, the UI element should provide appropriate
//!    visual feedback. This includes:
//!    - Calling [`DragAndDropManager::set_feedback`] with the appropriate feedback.
//!    - Drawing a frame around the target container with
//!      [`re_ui::DesignToken::drop_target_container_stroke`].
//!    - Optionally provide more feedback, e.g., where exactly the payload will be inserted within
//!      the container.
//!
//! 3. If the mouse is released (using [`egui::PointerState::any_released`]), the payload must be
//!    actually transferred to the container and [`egui::DragAndDrop::clear_payload`] must be
//!    called.

TODO

  • release checklist to check the above semantics
entity_drag.mp4

@abey79 abey79 added enhancement New feature or request ui concerns graphical user interface include in changelog labels Dec 12, 2024
Copy link

github-actions bot commented Dec 12, 2024

Web viewer built successfully. If applicable, you should also test it:

  • I have tested the web viewer
Result Commit Link Manifest
925f682 https://rerun.io/viewer/pr/8431 +nightly +main

Note: This comment is updated whenever you push a commit.

@abey79 abey79 changed the title WIP: enable dragging entities from the Entities can be dragged from the blueprint tree and streams tree to an existing view in the viewport Dec 12, 2024
@abey79 abey79 marked this pull request as draft December 12, 2024 10:32
@abey79 abey79 marked this pull request as ready for review December 16, 2024 13:21
# Conflicts:
#	crates/viewer/re_viewer_context/src/viewer_context.rs
Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

love it!

Comment on lines 78 to 92
fn try_item_collection_to_contents(items: &ItemCollection) -> Option<Vec<Contents>> {
items.iter().map(|(item, _)| item.try_into().ok()).collect()
}

fn try_item_collection_to_entities(items: &ItemCollection) -> Option<Vec<EntityPath>> {
items
.iter()
.map(|(item, _)| match item {
Item::InstancePath(instance_path) | Item::DataResult(_, instance_path) => instance_path
.is_all()
.then(|| instance_path.entity_path.clone()),
_ => None,
})
.collect()
}
Copy link
Member

Choose a reason for hiding this comment

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

a comment why these aren't filter_map would be nice. Iirc right from the demo session that was a conscious choice but code wide it looks a like bug codewise
(and I reckon that's also the reason this isn't implemented as a TryInto)

Copy link
Member Author

Choose a reason for hiding this comment

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

I used to have a TryInto for part of that iirc, but I find TryInto to denote a degree of generality that doesn't really apply. In the context of drag-and-drop (and its specific semantics), an ItemCollection may or may not be converted to a, e.g., Vec<EntityPath>. The conversion might be otherwise in other contexts, e.g. DataResult and Instance might actually be differentiated.

Now this is not a filter_map because it uses the collect transpose trick that implies the all semantics: it returns Some<Vec<_>> only if all items are Some<_>. I'll add a comment to that effect.

crates/viewer/re_viewer_context/src/test_context.rs Outdated Show resolved Hide resolved
crates/viewer/re_viewer_context/src/test_context.rs Outdated Show resolved Hide resolved
crates/viewer/re_viewer_context/src/drag_and_drop.rs Outdated Show resolved Hide resolved
@Wumpf Wumpf merged commit 02ae9c5 into main Dec 17, 2024
32 checks passed
@Wumpf Wumpf deleted the antoine/dnd-entities-to-existing-views branch December 17, 2024 10:46
abey79 added a commit that referenced this pull request Dec 18, 2024
### Related

* Related to #8518
* Related to #8431

### What

Fix a bug in #8431 where multi-entity drag-and-drop would only add a
single entity due to a foot-gun in our entity path filter mutation API
(see #8518 for more). Also add a new checklist.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request include in changelog ui concerns graphical user interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support dragging entities to existing view to add them to their filter
2 participants