Merge pull request #3091 from ZIDAN44/fix/1826-marker-bin-double-click

Timeline: a double-click on the composition marker bin adds a marker at the current time (#1826)

crates/ui-egui/src/panels/markers_ui.rs

@@ -428,9 +428,18 @@ pub(crate) fn comp_markers(app: &mut EffectcraftApp, ui: &mut egui::Ui, comp: &C
 /// (#275): drag a marker out of it and it follows the pointer along the marker strip `strip`
 /// (snapping like a dragged marker); dropping it there adds a composition marker at that time,
 /// one undo step. Dropped back on the bin or outside the time graph's width, nothing is added.
+/// A double-click on the bin adds one at the current time (#1826).
 pub(crate) fn marker_bin(app: &mut EffectcraftApp, ui: &mut egui::Ui, comp: &Comp, tm: TMap, bin: Rect, strip: Rect) {
     let t = app.tokens;
     let resp = ui.interact(bin, egui::Id::unique("marker-bin"), Sense::drag()).on_hover_text(tr("Composition marker bin: drag a marker to the time ruler"));
+    // The bin senses drags only, so a drag starts as soon as it is pressed: sensing clicks too
+    // would hold that back until the pointer moved, and a drag out released within that frame
+    // would add nothing. The double-click is read from the pointer instead.
+    if bin_double_clicked(ui, resp.id, bin, resp.enabled() && (resp.contains_pointer() || resp.dragged() || resp.drag_stopped()))
+        && let Err(e) = app.session.execute("comp.addMarker", json!({}))
+    {
+        app.ui.status = e.to_string();
+    }
     app.auto.add("timeline.markerBin", bin, "Composition marker bin");
     let hot = resp.hovered() || resp.dragged();
     if hot {
@@ -455,6 +464,64 @@ pub(crate) fn marker_bin(app: &mut EffectcraftApp, ui: &mut egui::Ui, comp: &Com
     }
 }
 
+/// What the marker bin keeps of the primary button between frames: the press being held (when,
+/// where, and whether it has moved too far since to be a click), and the last click (when,
+/// where, and whether it was on the bin: both pressed and released there).
+#[derive(Clone, Copy, Default)]
+struct BinClicks {
+    press: Option<(f64, Pos2, bool)>,
+    last: Option<(f64, Pos2, bool)>,
+}
+
+/// Whether the marker bin `bin` (widget `id`; `over` when it is enabled and has the pointer, not
+/// something above it) was double-clicked this frame: two clicks in a row, both on the bin, each
+/// in a frame when it was `over`. egui's own
+/// double-click takes any click within a few points of the last for the first one, so a click on
+/// the ruler right beside the bin and then one on the bin would count. The bin pairs the clicks
+/// itself, in the order they came, by egui's rules: a click is released within `max_click_dist`
+/// of where it was pressed and within `max_click_duration`, and the second of a double-click comes
+/// within `max_double_click_delay` and `max_click_dist` of the first. A third click starts over.
+fn bin_double_clicked(ui: &egui::Ui, id: egui::Id, bin: Rect, over: bool) -> bool {
+    use egui::{Event, PointerButton};
+    let (dist, duration, delay) = ui.ctx().options(|o| {
+        let i = &o.input_options;
+        (i.max_click_dist, i.max_click_duration, i.max_double_click_delay)
+    });
+    let id = id.with("clicks");
+    let mut s = ui.data(|d| d.get_temp::<BinClicks>(id)).unwrap_or_default();
+    let double = ui.input(|i| {
+        let mut double = false;
+        for e in &i.events {
+            match *e {
+                Event::PointerMoved(p) => {
+                    if let Some((_, at, moved)) = &mut s.press {
+                        *moved |= at.distance(p) > dist;
+                    }
+                }
+                Event::PointerButton { button: PointerButton::Primary, pressed: true, pos, .. } => s.press = Some((i.time, pos, false)),
+                Event::PointerButton { button: PointerButton::Primary, pressed: false, pos, .. } => {
+                    let Some((pressed_at, at, moved)) = s.press.take() else { continue };
+                    if moved || at.distance(pos) > dist || i.time - pressed_at >= duration {
+                        continue;
+                    }
+                    let on = over && bin.contains(at) && bin.contains(pos);
+                    let pair = s.last.is_some_and(|(t, p, was_on)| was_on && on && i.time - t < delay && p.distance(pos) < dist);
+                    if pair {
+                        double = true;
+                        s.last = None;
+                    } else {
+                        s.last = Some((i.time, pos, on));
+                    }
+                }
+                _ => {}
+            }
+        }
+        double
+    });
+    ui.data_mut(|d| d.insert_temp(id, s));
+    double
+}
+
 /// Layer markers on a layer's row `r`; on a precomp layer, its comp's markers too (read-only,
 /// outlined: hover names them, a double-click opens the nested comp at the marker).
 pub(crate) fn layer_markers(app: &mut EffectcraftApp, ui: &mut egui::Ui, clip: Rect, comp: &Comp, layer: &Layer, tm: TMap, r: Rect) {

crates/ui-egui/tests/it/ui_timeline_zoom.rs

@@ -376,6 +376,122 @@ fn dragging_out_of_the_marker_bin_adds_a_comp_marker() {
     assert_eq!(markers(&h), vec![13.0, 20.0]);
 }
 
+/// #1826: as in After Effects, a double-click on the composition marker bin adds a composition
+/// marker at the current time, one undo step; a single click adds none, and a marker dragged
+/// out of the bin still lands where it is dropped.
+#[test]
+fn double_clicking_the_marker_bin_adds_a_comp_marker_at_the_current_time() {
+    let (mut h, _) = harness();
+    h.state_mut().ui.timeline.pps = Some(200.0);
+    h.state_mut().ui.timeline.start = 10.0;
+    h.state_mut().session.execute("edit.deselectAll", json!({})).unwrap();
+    h.state_mut().session.execute("time.set", json!({"time": 21.5})).unwrap();
+    h.run_steps(2);
+    let markers = |h: &Harness<'_, EffectcraftApp>| -> Vec<f64> { h.state().session.active_comp().unwrap().markers.iter().map(|m| m.time.seconds()).collect() };
+    let bin = rect(&h, "timeline.markerBin");
+    let ruler = rect(&h, "timeline.ruler");
+    // Presses and releases in one frame each, so a pair falls inside egui's double-click time.
+    let clicks = |h: &mut Harness<'_, EffectcraftApp>, n: usize| {
+        h.event(Event::PointerMoved(bin.center()));
+        h.step();
+        for _ in 0..n {
+            for pressed in [true, false] {
+                h.input_mut().events.push(Event::PointerButton {
+                    pos: bin.center(),
+                    button: egui::PointerButton::Primary,
+                    pressed,
+                    modifiers: Modifiers::NONE,
+                });
+            }
+        }
+        h.step();
+        h.run_steps(3);
+    };
+    clicks(&mut h, 1);
+    assert!(markers(&h).is_empty(), "a single click: {:?}", markers(&h));
+    let undo = h.state().session.history.undo.len();
+    clicks(&mut h, 2);
+    assert_eq!(markers(&h), vec![21.5], "at the current time");
+    let s = &h.state().session;
+    assert_eq!(s.active_comp().unwrap().markers[0].time, s.time(), "exactly the current time");
+    let history = &h.state().session.history.undo;
+    assert_eq!((history.len(), history.last().map(|u| u.0.as_str())), (undo + 1, Some("Add Marker")), "one undo step");
+    assert_eq!(h.state().session.time().seconds(), 21.5, "the current time stays");
+    // The second press in one frame and its release in the next (a frame here is 0.25 s, inside
+    // egui's 0.3 s double-click time): the bin is dragged by then, and still a marker at 22 s.
+    h.state_mut().session.execute("time.set", json!({"time": 22.0})).unwrap();
+    h.event(Event::PointerMoved(bin.center()));
+    h.step();
+    for pressed in [true, false, true] {
+        h.input_mut().events.push(Event::PointerButton { pos: bin.center(), button: egui::PointerButton::Primary, pressed, modifiers: Modifiers::NONE });
+    }
+    h.step();
+    assert!(h.ctx.dragged_id().is_some(), "the second press drags the bin");
+    h.event(Event::PointerButton { pos: bin.center(), button: egui::PointerButton::Primary, pressed: false, modifiers: Modifiers::NONE });
+    h.step();
+    h.run_steps(3);
+    assert_eq!(markers(&h), vec![21.5, 22.0], "a double-click released while the bin is dragged");
+    // A click on the ruler right beside the bin and then one on the bin, in one frame: egui
+    // takes them for a double-click (they are a few points apart), but only one was on the bin.
+    let (beside, on) = (pos2(bin.min.x - 2.0, bin.center().y), pos2(bin.min.x + 2.0, bin.center().y));
+    assert!(ruler.contains(beside) && !bin.contains(beside) && bin.contains(on));
+    h.event(Event::PointerMoved(beside));
+    h.step();
+    for (pos, pressed) in [(beside, true), (beside, false), (on, true), (on, false)] {
+        h.input_mut().events.push(Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed, modifiers: Modifiers::NONE });
+    }
+    h.step();
+    h.run_steps(3);
+    assert_eq!(markers(&h), vec![21.5, 22.0], "one click beside the bin and one on it");
+    // On the bin, beside it, on it again: the two in a row are never both on the bin.
+    h.run_steps(3);
+    for pos in [on, beside, on] {
+        for pressed in [true, false] {
+            h.input_mut().events.push(Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed, modifiers: Modifiers::NONE });
+        }
+    }
+    h.step();
+    h.run_steps(3);
+    assert_eq!(markers(&h), vec![21.5, 22.0], "on, beside, on");
+    // Twice pressed beside the bin and released on it, a few points away: clicks, not on the bin.
+    h.run_steps(3);
+    for _ in 0..2 {
+        for (pos, pressed) in [(beside, true), (on, false)] {
+            h.input_mut().events.push(Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed, modifiers: Modifiers::NONE });
+        }
+    }
+    h.step();
+    h.run_steps(3);
+    assert_eq!(markers(&h), vec![21.5, 22.0], "pressed beside, released on");
+    // Dragged out 3 s into the visible span (from 10 s at 200 px/s): a marker at 13 s too.
+    drag(&mut h, bin.center(), pos2(ruler.min.x + 6.0 + 600.0, ruler.center().y));
+    assert_eq!(markers(&h), vec![13.0, 21.5, 22.0]);
+}
+
+/// A marker dragged out of the bin and dropped within one frame (a fast drag, or a slow frame)
+/// still lands where it is dropped: the bin starts its drag as soon as it is pressed.
+#[test]
+fn a_marker_dragged_out_of_the_bin_and_dropped_in_one_frame_is_added() {
+    let (mut h, _) = harness();
+    h.state_mut().ui.timeline.pps = Some(200.0);
+    h.state_mut().ui.timeline.start = 10.0;
+    h.run_steps(2);
+    let bin = rect(&h, "timeline.markerBin");
+    let ruler = rect(&h, "timeline.ruler");
+    let at = pos2(ruler.min.x + 6.0 + 600.0, ruler.center().y);
+    h.event(Event::PointerMoved(bin.center()));
+    h.step();
+    h.event(Event::PointerButton { pos: bin.center(), button: egui::PointerButton::Primary, pressed: true, modifiers: Modifiers::NONE });
+    h.step();
+    // Both in one frame (`event` would give each a frame of its own).
+    h.input_mut().events.push(Event::PointerMoved(at));
+    h.input_mut().events.push(Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed: false, modifiers: Modifiers::NONE });
+    h.step();
+    h.run_steps(2);
+    let markers: Vec<f64> = h.state().session.active_comp().unwrap().markers.iter().map(|m| m.time.seconds()).collect();
+    assert_eq!(markers, vec![13.0]);
+}
+
 /// #976: as in After Effects, Numpad * marks the selected layers at the current time; only with
 /// no layer selected does it add a composition marker.
 #[test]