Skip to content

Commit b2dbf9f

Browse files
authored
Support mid-drag modifier changes (#733)
* Support live modifier switching mid-drag Previously a scene-node drag froze its (button, modifier) combo at drag-start: changing the held modifier mid-gesture had no effect. This partitions a physical drag into one logical segment per combo. When the held modifier changes, the client ends the current segment and -- if the new combo is bound -- starts a fresh one under it, preserving the grab geometry (plane, grab point, instance) so the drag continues without a visual jump; only the addressed callback set changes. Modifier changes are picked up from both pointermove and (for stationary switches) window keydown/keyup during the drag. A switch to an unbound combo drops the gesture into a dormant gap (no messages) so callbacks see properly paired start/end per bound combo; re-entering a bound combo starts a new segment. The wire message already carries `modifier`, so this is client-side segmentation plus docs -- no message-schema change. `SceneNodeDragEvent` keeps a single `modifier` field that now identifies the active segment. Tests: planModifierTransition unit cases (vitest), a server-side multi-segment dispatch/bookkeeping test, and an e2e test that adds Shift mid-drag and asserts the cmd/ctrl segment ends before release. * Add e2e coverage for dormant-then-resume drag Covers the gap surfaced in review: switching to an unbound modifier combo mid-drag must suspend the gesture (dormant), not end it. Asserts the active segment ends on the switch (before release), and that re-entering a bound combo resumes with a fresh segment within the same button press -- which only holds if dormant is a suspend rather than a teardown. Both transitions are driven by key events with the pointer stationary, so this also exercises the keydown/keyup path. * Add multi-stage drag coverage (many switches + dormant cycles) Existing tests each exercised a single modifier switch. These add sequences within one drag: - server: one drag cycling cmd/ctrl -> cmd/ctrl+shift -> cmd/ctrl -> cmd/ctrl+shift (re-entering both combos), asserting each callback sees clean ordered start...end pairs and the active-drag map fully releases. - e2e: one continuous press cycling cmd/ctrl -> cmd/ctrl+shift -> dormant -> cmd/ctrl+shift -> cmd/ctrl, asserting exactly two segments per combo across repeated switching, re-entry, and resume-after-dormant. Uses async callbacks so the counters settle deterministically. * Prettier-format dragUtils.test.ts (CI eslint gate) * Honor pre-promotion modifier changes; read drag bindings live at switches Two review findings on the mid-drag modifier-switch mechanism: 1. The opening segment is now attributed to the modifier held on the promoting pointermove, not the one sampled at pointerdown. The drag layer's key listeners only install at promotion, so a change inside the pointerdown->threshold window was invisible: a promote-and-release gesture opened (and closed) a segment under a stale modifier. When the promotion-time combo is unbound the drag now begins dormant and resumes on the next bound combo (new planDragStart, unit-pinned). 2. transitionDragModifier reads bindings live from the scene-tree store instead of a drag-start snapshot, so callbacks registered or removed mid-drag take effect at the next switch. The snapshot field is gone from ActiveDragState. E2E pins for both: a pre-promotion Ctrl release must not fire a stale segment (and must resume on re-press), and a binding registered mid-drag must own the post-switch segment. * Plan the opening drag segment against live bindings too The pointerdown->promotion window fix attributed the opening segment to the promotion-time modifier but still planned it against the bindings array captured at pointerdown. A binding added or removed inside that window was invisible to the opening plan (e.g. a combo bound mid-window began dormant instead of owning the segment). beginDrag now reads dragBindings live from the scene-tree store for the opening plan; the pointerdown snapshot remains only as the candidacy gate. * Defer switch-created segment starts until confirmed (100ms or motion) Ending a modifier-drag the natural way -- modifier up a beat before the button -- previously fired a degenerate start/end pair on whatever combo remained bound (e.g. a bare on_drag registered alongside a modifier binding), since humans can't release both within the same frame. This reintroduced exactly the degenerate pair the promotion motion-threshold eliminated, one boundary later. A mid-drag switch now ends the old segment immediately but holds the new segment's start PENDING until confirmed: a 100ms timer (SWITCH_START_DELAY_MS), or sooner by pointer motion past the promotion threshold from the switch point. Release/cancel/blur inside the window discards the pending start outright -- no message was sent, none is owed -- and a further modifier change abandons it silently, which also swallows key-rollover pass-through combos when releasing multi-key combos one key at a time. Stationary switches still fire (timer path, imperceptibly later); the opening segment is never deferred (promotion's motion threshold is its confirmation). E2E pins the release order: bare + cmd/ctrl both bound, ctrl-drag ended modifier-first -> bare sees nothing, ctrl sees one clean pair, and a follow-up bare drag proves the binding was live. * Strengthen the release-order e2e; fix start-at-press docstrings The discard pin alone was satisfiable with the keyup path broken entirely (the pointerup would end the ctrl segment and bare would still read zero), and its keyup->mouseup gap raced the 100ms deferral timer on slow CI. Split into two gestures: A holds the button through the window so the keyup's pending bare start CONFIRMS on the timer (proving the keyup creates a pending segment, and doubling as the bare-binding positive control); B dispatches keyup + pointerup in one JS task -- both listeners are window-level -- making the inside-the-window release deterministic before asserting bare never budged. Also correct the on_drag docs: start fires when the press is confirmed as a drag (motion threshold), not at press.
1 parent d04c1cd commit b2dbf9f

9 files changed

Lines changed: 1267 additions & 41 deletions

File tree

src/viser/_scene_handles.py

Lines changed: 51 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -474,10 +474,19 @@ class SceneNodeDragEvent(Generic[TSceneNodeHandle]):
474474
target: TSceneNodeHandle
475475
"""Scene node that is being dragged."""
476476
phase: DragPhase
477-
"""Drag lifecycle phase: ``"start"`` at press, ``"update"`` on
478-
every throttled pointermove (~20Hz), ``"end"`` at release. A
479-
single drag fires exactly one ``"start"``, zero or more
480-
``"update"``s, and exactly one ``"end"``."""
477+
"""Drag lifecycle phase: ``"start"`` once a press is confirmed as a
478+
drag (the pointer travels past a small motion threshold -- a
479+
stationary press/release fires nothing), ``"update"`` on every
480+
throttled pointermove (~20Hz), ``"end"`` at release.
481+
482+
A gesture is partitioned into one *segment* per held modifier-combo.
483+
Each segment fires exactly one ``"start"``, zero or more
484+
``"update"``s, and exactly one ``"end"``. If the user changes the
485+
held modifier mid-drag, the current segment ends and a new one
486+
starts under the new modifier (see :attr:`modifier`) -- so a single
487+
physical drag can produce more than one ``start``/``end`` pair. When
488+
the modifier doesn't change, this collapses to the common case of a
489+
single ``start`` ... ``end`` per gesture."""
481490
instance_index: int | None
482491
"""Instance index within a batched scene node (e.g. batched meshes,
483492
batched GLBs, batched axes); ``None`` for non-batched nodes. Frozen
@@ -498,9 +507,12 @@ class SceneNodeDragEvent(Generic[TSceneNodeHandle]):
498507
button: Literal["left", "middle", "right"]
499508
"""Mouse button that initiated the drag."""
500509
modifier: _messages.KeyModifier | None
501-
"""Modifier-combo held at drag-start (frozen for the drag's
502-
lifetime). ``None`` if no modifiers were held; otherwise a
503-
canonical :data:`KeyModifier` string."""
510+
"""Modifier-combo that owns the current drag segment. Constant within
511+
a segment and matches the binding this callback was registered for;
512+
if the user changes the held modifier mid-drag the segment ends and a
513+
new one begins under the new combo (see :attr:`phase`). ``None`` if no
514+
modifiers are held; otherwise a canonical :data:`KeyModifier`
515+
string."""
504516

505517

506518
_VALID_DRAG_BUTTONS: Tuple[_messages.DragButton, ...] = get_args(_messages.DragButton)
@@ -611,12 +623,32 @@ def on_drag(
611623
) -> Any:
612624
"""Attach a callback for the full drag lifecycle.
613625
614-
Fires three times per gesture: once with
615-
``event.phase == "start"`` at press, zero or more times with
616-
``"update"`` (throttled pointermove), once with ``"end"`` at
617-
release. ``end`` fires even on cancellation paths (window
618-
blur, pointer cancel, node removed mid-drag) so per-drag
619-
state can be released.
626+
Fires once with ``event.phase == "start"`` when a press is
627+
confirmed as a drag (the pointer travels past a small motion
628+
threshold; a stationary press/release fires nothing), zero or
629+
more times with ``"update"`` (throttled pointermove), and once
630+
with ``"end"`` at release. ``end`` fires even on cancellation
631+
paths (window blur, pointer cancel, node removed mid-drag) so
632+
per-drag state can be released.
633+
634+
Modifiers are live: if the user changes the held modifier
635+
mid-drag, the current segment ends and a new one begins under
636+
the new combo, routing to whichever callback that combo is bound
637+
to. A callback therefore sees a clean ``start`` ... ``end`` pair
638+
for *its* modifier each time that modifier is engaged, and a
639+
single physical drag may fire more than one such pair. To switch
640+
behavior mid-drag (e.g. changing the drag plane), register a
641+
separate ``on_drag`` for each modifier combo. ``event.modifier``
642+
identifies the active segment.
643+
644+
A switch-created segment's ``start`` is confirmed briefly
645+
(~100ms, or sooner on pointer motion) before it fires; releasing
646+
the mouse button within that window discards the segment
647+
entirely. In particular, releasing the modifier a beat before
648+
the button -- the natural way to end a modifier-drag -- does
649+
*not* fire a spurious start/end pair on the combo left behind
650+
(e.g. a bare ``on_drag`` registered alongside a modifier
651+
binding).
620652
621653
Usable as a bare decorator (``@handle.on_drag``, defaults to
622654
``button="left"`` and no modifiers) or with arguments
@@ -629,9 +661,12 @@ def on_drag(
629661
ordered ``"+"``-separated string like ``"cmd/ctrl"``,
630662
``"shift"``, or ``"cmd/ctrl+shift"``. ``None`` matches
631663
"no modifiers held". Matching is exact: listed modifiers
632-
must be held and others must not be. Left-drag on this
633-
node intercepts the gesture -- the camera only orbits on
634-
empty-space drags.
664+
must be held and others must not be. The match is
665+
re-evaluated whenever the held modifier changes mid-drag,
666+
so this callback is entered and exited as its combo is
667+
engaged and released. Left-drag on this node intercepts
668+
the gesture -- the camera only orbits on empty-space
669+
drags.
635670
636671
Note on ordering: synchronous (``def``) callbacks are submitted
637672
to a thread pool fire-and-forget and can run out of order -- an

0 commit comments

Comments
 (0)