Skip to content

Commit

Permalink
Various picking bugfixes (#15293)
Browse files Browse the repository at this point in the history 
# Objective

- Intended to resolve #15290.
- Fix four duplicate `DragEnd` firing when drag finished.
- Fix redundant `DragStart` firing when dragging across pick-able
entities.
- Fix `Click` coming after `Drop` and obliterating finished drag
interactions.

Big thanks to B. Reinhart for testing picking in their codebase and
identifying these issues early.

## Solution

- Fix press & drag state being cleared after the first entity is read
from the hover map on pointer release, rather than after all entities
are read. This caused only the first hovered entity to receive `Up` and
`Click` events.
- Fixes `Down` being determined using the `previous_hover_map` rather
than `hover_map`, a regression compared to `bevy_mod_picking`. I think
this is what was messing up drag events.
- Fixes and issue where `PointerEnd` would fire multiple times and
`PointerStart` would fire when dragging onto a new entity.
- Re-orders events to make them easier to handle. `Out` now fired before
`DragLeave` and `Click/Up` now fire before `DragDrop`.
- Generally refactors the picking event code to be more clean and sane. 

## Testing

These changes are currently sporadically tested.
  • Loading branch information
@NthTensor
NthTensor authored Sep 20, 2024
1 parent 13ca08f commit d7ea5b6
Showing 1 changed file with 141 additions and 120 deletions.
261 changes: 141 additions & 120 deletions crates/bevy_picking/src/events.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ pub struct DragEntry {
#[derive(Debug, Clone, Default)]
pub struct PointerState {
/// Stores the press location and start time for each button currently being pressed by the pointer.
pub pressing: HashMap<Entity, (Location, Instant)>,
pub pressing: HashMap<Entity, (Location, Instant, HitData)>,
/// Stores the the starting and current locations for each entity currently being dragged by the pointer.
pub dragging: HashMap<Entity, DragEntry>,
/// Stores the hit data for each entity currently being dragged over by the pointer.
Expand All @@ -266,9 +266,9 @@ pub struct PointerState {
/// + The sequence [`DragEnter`], [`Over`].
/// + Any number of any of the following:
/// + For each movement: The sequence [`DragStart`], [`Drag`], [`DragOver`], [`Move`].
/// + For each button press: Either [`Down`], or the sequence [`DragDrop`], [`DragEnd`], [`DragLeave`], [`Click`], [`Up`].
/// + For each button press: Either [`Down`], or the sequence [`Click`], [`Up`], [`DragDrop`], [`DragEnd`], [`DragLeave`].
/// + For each pointer cancellation: Simply [`Cancel`].
/// + Finally the sequence [`DragLeave`], [`Out`].
/// + Finally the sequence [`Out`], [`DragLeave`].
///
/// Only the last event in a given sequence is garenteed to be present.
///
Expand Down Expand Up @@ -326,6 +326,7 @@ pub fn pointer_events(
);
continue;
};

// Possibly send DragEnter events
for button in PointerButton::iter() {
let state = pointer_state.entry((pointer_id, button)).or_default();
Expand All @@ -350,6 +351,7 @@ pub fn pointer_events(
);
}
}

// Always send Over events
commands.trigger_targets(
Pointer::new(pointer_id, location.clone(), Over { hit: hit.clone() }),
Expand All @@ -370,76 +372,39 @@ pub fn pointer_events(
PointerAction::Pressed { direction, button } => {
let state = pointer_state.entry((pointer_id, button)).or_default();

// Possibly emit DragEnd, DragDrop, DragLeave on button releases
if direction == PressDirection::Up {
// For each currently dragged entity
for (drag_target, drag) in state.dragging.drain() {
// Emit DragDrop
for (dragged_over, hit) in state.dragging_over.iter() {
commands.trigger_targets(
Pointer::new(
pointer_id,
location.clone(),
DragDrop {
button,
dropped: drag_target,
hit: hit.clone(),
},
),
*dragged_over,
);
}
// Emit DragEnd
commands.trigger_targets(
Pointer::new(
// The sequence of events emitted depends on if this is a press or a release
match direction {
PressDirection::Down => {
// If it's a press, emit a Down event and mark the hovered entities as pressed
for (hovered_entity, hit) in hover_map
.get(&pointer_id)
.iter()
.flat_map(|h| h.iter().map(|(entity, data)| (*entity, data.clone())))
{
let event = Pointer::new(
pointer_id,
location.clone(),
DragEnd {
Down {
button,
distance: drag.latest_pos - drag.start_pos,
hit: hit.clone(),
},
),
drag_target,
);
// Emit DragLeave
for (dragged_over, hit) in state.dragging_over.iter() {
commands.trigger_targets(
Pointer::new(
pointer_id,
location.clone(),
DragLeave {
button,
dragged: drag_target,
hit: hit.clone(),
},
),
*dragged_over,
);
}
}
}

// Send a Down or possibly a Click and an Up button events
for (hovered_entity, hit) in previous_hover_map
.get(&pointer_id)
.iter()
.flat_map(|h| h.iter().map(|(entity, data)| (*entity, data.clone())))
{
match direction {
PressDirection::Down => {
// Send the Down event first
let event =
Pointer::new(pointer_id, location.clone(), Down { button, hit });
commands.trigger_targets(event.clone(), hovered_entity);
commands.trigger_targets(event, hovered_entity);
// Also insert the press into the state
state
.pressing
.insert(hovered_entity, (event.pointer_location, now));
.insert(hovered_entity, (location.clone(), now, hit));
}
PressDirection::Up => {
// If this pointer previously pressed the hovered entity, first send a Click event
if let Some((_location, press_instant)) =
state.pressing.get(&hovered_entity)
}
PressDirection::Up => {
// Emit Click and Up events on all the previously hovered entities.
for (hovered_entity, hit) in previous_hover_map
.get(&pointer_id)
.iter()
.flat_map(|h| h.iter().map(|(entity, data)| (*entity, data.clone())))
{
// If this pointer previously pressed the hovered entity, emit a Click event
if let Some((_, press_instant, _)) = state.pressing.get(&hovered_entity)
{
commands.trigger_targets(
Pointer::new(
Expand All @@ -466,77 +431,132 @@ pub fn pointer_events(
),
hovered_entity,
);
// Also clear the state
state.pressing.clear();
state.dragging.clear();
state.dragging_over.clear();
}
};
}
}
// Moved
PointerAction::Moved { delta } => {
for (hovered_entity, hit) in hover_map
.get(&pointer_id)
.iter()
.flat_map(|h| h.iter().map(|(entity, data)| (*entity, data.to_owned())))
{
// Send drag events to entities being dragged or dragged over
for button in PointerButton::iter() {
let state = pointer_state.entry((pointer_id, button)).or_default();

// Emit a DragStart the first time the pointer moves while pressing an entity
for (location, _instant) in state.pressing.values() {
if state.dragging.contains_key(&hovered_entity) {
continue; // this entity is already logged as being dragged

// Then emit the drop events.
for (drag_target, drag) in state.dragging.drain() {
// Emit DragDrop
for (dragged_over, hit) in state.dragging_over.iter() {
commands.trigger_targets(
Pointer::new(
pointer_id,
location.clone(),
DragDrop {
button,
dropped: drag_target,
hit: hit.clone(),
},
),
*dragged_over,
);
}
state.dragging.insert(
hovered_entity,
DragEntry {
start_pos: location.position,
latest_pos: location.position,
},
);
// Emit DragEnd
commands.trigger_targets(
Pointer::new(
pointer_id,
location.clone(),
DragStart {
DragEnd {
button,
hit: hit.clone(),
distance: drag.latest_pos - drag.start_pos,
},
),
hovered_entity,
drag_target,
);
// Emit DragLeave
for (dragged_over, hit) in state.dragging_over.iter() {
commands.trigger_targets(
Pointer::new(
pointer_id,
location.clone(),
DragLeave {
button,
dragged: drag_target,
hit: hit.clone(),
},
),
*dragged_over,
);
}
}

// Emit a Drag event to the dragged entity when it is dragged over another entity.
for (dragged_entity, drag) in state.dragging.iter_mut() {
let drag_event = Drag {
button,
distance: location.position - drag.start_pos,
delta: location.position - drag.latest_pos,
};
drag.latest_pos = location.position;
let target = *dragged_entity;
let event = Pointer::new(pointer_id, location.clone(), drag_event);
commands.trigger_targets(event, target);
// Finally, we can clear the state of everything relating to presses or drags.
state.pressing.clear();
state.dragging.clear();
state.dragging_over.clear();
}
}
}
// Moved
PointerAction::Moved { delta } => {
// Triggers during movement even if not over an entity
for button in PointerButton::iter() {
let state = pointer_state.entry((pointer_id, button)).or_default();

// Emit DragEntry and DragStart the first time we move while pressing an entity
for (press_target, (location, _, hit)) in state.pressing.iter() {
if state.dragging.contains_key(press_target) {
continue; // This entity is already logged as being dragged
}
state.dragging.insert(
*press_target,
DragEntry {
start_pos: location.position,
latest_pos: location.position,
},
);
commands.trigger_targets(
Pointer::new(
pointer_id,
location.clone(),
DragStart {
button,
hit: hit.clone(),
},
),
*press_target,
);
}

// Emit a DragOver to the hovered entity when dragging a different entity over it.
for drag_target in state.dragging.keys()
.filter(
|&&drag_target| hovered_entity != drag_target, /* can't drag over itself */
)
// Emit Drag events to the entities we are dragging
for (drag_target, drag) in state.dragging.iter_mut() {
let drag_event = Drag {
button,
distance: location.position - drag.start_pos,
delta: location.position - drag.latest_pos,
};
drag.latest_pos = location.position;
let event = Pointer::new(pointer_id, location.clone(), drag_event);
commands.trigger_targets(event, *drag_target);

// Emit corresponding DragOver to the hovered entities
for (hovered_entity, hit) in hover_map
.get(&pointer_id)
.iter()
.flat_map(|h| h.iter().map(|(entity, data)| (*entity, data.to_owned())))
.filter(|(hovered_entity, _)| *hovered_entity != *drag_target)
{
commands.trigger_targets(
Pointer::new(pointer_id, location.clone(), DragOver { button, dragged: *drag_target, hit: hit.clone() }),
Pointer::new(
pointer_id,
location.clone(),
DragOver {
button,
dragged: *drag_target,
hit: hit.clone(),
},
),
hovered_entity,
);
}
}
}

// Always send Move event
for (hovered_entity, hit) in hover_map
.get(&pointer_id)
.iter()
.flat_map(|h| h.iter().map(|(entity, data)| (*entity, data.to_owned())))
{
// Emit Move events to the entities we are hovering
commands.trigger_targets(
Pointer::new(
pointer_id,
Expand Down Expand Up @@ -593,6 +613,13 @@ pub fn pointer_events(
);
continue;
};

// Always send Out events
commands.trigger_targets(
Pointer::new(pointer_id, location.clone(), Out { hit: hit.clone() }),
hovered_entity,
);

// Possibly send DragLeave events
for button in PointerButton::iter() {
let state = pointer_state.entry((pointer_id, button)).or_default();
Expand All @@ -612,12 +639,6 @@ pub fn pointer_events(
);
}
}

// Always send Out events
commands.trigger_targets(
Pointer::new(pointer_id, location.clone(), Out { hit: hit.clone() }),
hovered_entity,
);
}
}
}

0 comments on commit d7ea5b6

Please sign in to comment.