diff --git a/src/viser/_scene_handles.py b/src/viser/_scene_handles.py index 792a1b845..23d9daae7 100644 --- a/src/viser/_scene_handles.py +++ b/src/viser/_scene_handles.py @@ -474,10 +474,19 @@ class SceneNodeDragEvent(Generic[TSceneNodeHandle]): target: TSceneNodeHandle """Scene node that is being dragged.""" phase: DragPhase - """Drag lifecycle phase: ``"start"`` at press, ``"update"`` on - every throttled pointermove (~20Hz), ``"end"`` at release. A - single drag fires exactly one ``"start"``, zero or more - ``"update"``s, and exactly one ``"end"``.""" + """Drag lifecycle phase: ``"start"`` once a press is confirmed as a + drag (the pointer travels past a small motion threshold -- a + stationary press/release fires nothing), ``"update"`` on every + throttled pointermove (~20Hz), ``"end"`` at release. + + A gesture is partitioned into one *segment* per held modifier-combo. + Each segment fires exactly one ``"start"``, zero or more + ``"update"``s, and exactly one ``"end"``. If the user changes the + held modifier mid-drag, the current segment ends and a new one + starts under the new modifier (see :attr:`modifier`) -- so a single + physical drag can produce more than one ``start``/``end`` pair. When + the modifier doesn't change, this collapses to the common case of a + single ``start`` ... ``end`` per gesture.""" instance_index: int | None """Instance index within a batched scene node (e.g. batched meshes, batched GLBs, batched axes); ``None`` for non-batched nodes. Frozen @@ -498,9 +507,12 @@ class SceneNodeDragEvent(Generic[TSceneNodeHandle]): button: Literal["left", "middle", "right"] """Mouse button that initiated the drag.""" modifier: _messages.KeyModifier | None - """Modifier-combo held at drag-start (frozen for the drag's - lifetime). ``None`` if no modifiers were held; otherwise a - canonical :data:`KeyModifier` string.""" + """Modifier-combo that owns the current drag segment. Constant within + a segment and matches the binding this callback was registered for; + if the user changes the held modifier mid-drag the segment ends and a + new one begins under the new combo (see :attr:`phase`). ``None`` if no + modifiers are held; otherwise a canonical :data:`KeyModifier` + string.""" _VALID_DRAG_BUTTONS: Tuple[_messages.DragButton, ...] = get_args(_messages.DragButton) @@ -611,12 +623,32 @@ def on_drag( ) -> Any: """Attach a callback for the full drag lifecycle. - Fires three times per gesture: once with - ``event.phase == "start"`` at press, zero or more times with - ``"update"`` (throttled pointermove), once with ``"end"`` at - release. ``end`` fires even on cancellation paths (window - blur, pointer cancel, node removed mid-drag) so per-drag - state can be released. + Fires once with ``event.phase == "start"`` when a press is + confirmed as a drag (the pointer travels past a small motion + threshold; a stationary press/release fires nothing), zero or + more times with ``"update"`` (throttled pointermove), and once + with ``"end"`` at release. ``end`` fires even on cancellation + paths (window blur, pointer cancel, node removed mid-drag) so + per-drag state can be released. + + Modifiers are live: if the user changes the held modifier + mid-drag, the current segment ends and a new one begins under + the new combo, routing to whichever callback that combo is bound + to. A callback therefore sees a clean ``start`` ... ``end`` pair + for *its* modifier each time that modifier is engaged, and a + single physical drag may fire more than one such pair. To switch + behavior mid-drag (e.g. changing the drag plane), register a + separate ``on_drag`` for each modifier combo. ``event.modifier`` + identifies the active segment. + + A switch-created segment's ``start`` is confirmed briefly + (~100ms, or sooner on pointer motion) before it fires; releasing + the mouse button within that window discards the segment + entirely. In particular, releasing the modifier a beat before + the button -- the natural way to end a modifier-drag -- does + *not* fire a spurious start/end pair on the combo left behind + (e.g. a bare ``on_drag`` registered alongside a modifier + binding). Usable as a bare decorator (``@handle.on_drag``, defaults to ``button="left"`` and no modifiers) or with arguments @@ -629,9 +661,12 @@ def on_drag( ordered ``"+"``-separated string like ``"cmd/ctrl"``, ``"shift"``, or ``"cmd/ctrl+shift"``. ``None`` matches "no modifiers held". Matching is exact: listed modifiers - must be held and others must not be. Left-drag on this - node intercepts the gesture -- the camera only orbits on - empty-space drags. + must be held and others must not be. The match is + re-evaluated whenever the held modifier changes mid-drag, + so this callback is entered and exited as its combo is + engaged and released. Left-drag on this node intercepts + the gesture -- the camera only orbits on empty-space + drags. Note on ordering: synchronous (``def``) callbacks are submitted to a thread pool fire-and-forget and can run out of order -- an diff --git a/src/viser/client/src/DragLayer.tsx b/src/viser/client/src/DragLayer.tsx index c5e6aedc4..ece09be86 100644 --- a/src/viser/client/src/DragLayer.tsx +++ b/src/viser/client/src/DragLayer.tsx @@ -31,9 +31,15 @@ import { useThrottledMessageSender } from "./WebsocketUtils"; import { ActiveDragState, DragScratches, + KeyModifier, anyBindingMatches, computeInstanceWorldMatrix, isInstancedMesh2VendoredMessage, + keyModifierFromEvent, + motionExceedsThreshold, + planDragStart, + planModifierTransition, + SWITCH_START_DELAY_MS, } from "./dragUtils"; import { DragLayerApi, DragLayerContext } from "./dragLayerContext"; import { useDragArrow } from "./useDragArrow"; @@ -42,6 +48,15 @@ import { useDragArrow } from "./useDragArrow"; // DragLayer component. // ============================================================================= +/** Discard a not-yet-confirmed switch-created ``start`` (no-op when none + * is pending). No wire message was ever sent for the pending segment, so + * nothing needs ending -- the segment simply never existed. */ +function discardPendingStart(activeDrag: ActiveDragState): void { + if (activeDrag.pendingStart === null) return; + clearTimeout(activeDrag.pendingStart.timerId); + activeDrag.pendingStart = null; +} + /** In playback / embedded / static viewers ``sendMessage`` is a no-op, * so dragging makes no sense (it would disable camera controls and * swallow left-clicks with nothing on the receiving end). The outer @@ -225,21 +240,101 @@ function DragLayerActive({ children }: { children?: React.ReactNode }) { // Build + send a drag message in one call, no-op if ``buildDragMessage`` // returns null (live grab point unavailable). Wraps the duplicated - // null-check pattern at the three send sites. */ + // null-check pattern at the send sites. Returns whether a message was + // actually sent -- callers use this to keep ``segmentActive`` honest + // (a ``start`` that couldn't build must not be paired with a later + // ``end``). */ const sendDragMessage = React.useCallback( ( activeDrag: ActiveDragState, phase: "start" | "update" | "end", throttle: boolean, - ) => { + ): boolean => { const message = buildDragMessage(activeDrag, phase); - if (message === null) return; + if (message === null) return false; if (throttle) sendDragsThrottled(message); else viewerMutable.sendMessage(message); + return true; }, [buildDragMessage, sendDragsThrottled, viewerMutable], ); + // Send a pending switch-created ``start`` now (no-op when none is + // pending). Called when the switch is confirmed: the deferral timer + // fires, or the pointer moves past the threshold from the switch + // point. */ + const flushPendingStart = React.useCallback( + (activeDrag: ActiveDragState) => { + if (activeDrag.pendingStart === null) return; + clearTimeout(activeDrag.pendingStart.timerId); + activeDrag.pendingStart = null; + activeDrag.segmentActive = sendDragMessage(activeDrag, "start", false); + }, + [sendDragMessage], + ); + + // Apply a mid-drag modifier change. Ends the current segment (if any) + // immediately, switches ownership to ``nextModifier``, and -- when the + // new combo is bound -- schedules a fresh segment whose ``start`` is + // DEFERRED until confirmed (``SWITCH_START_DELAY_MS`` timer, or pointer + // motion; see ``flushPendingStart``). Releasing the button first + // discards the pending start, so releasing the modifier a beat before + // the mouse button never fires a degenerate start/end pair on the + // combo left behind. Geometry is untouched, so the drag continues + // without a visual jump -- only the addressed callback set changes. + // Dormant (unbound) modifiers send nothing; see + // ``planModifierTransition``. */ + const transitionDragModifier = React.useCallback( + (activeDrag: ActiveDragState, nextModifier: KeyModifier | null) => { + // Bindings are read live from the scene-tree store (not a + // drag-start snapshot): a callback the server registers or removes + // *mid-drag* takes effect at the next modifier switch. A missing + // node (removed mid-gesture) reads as no bindings -- the switch + // goes dormant, and the revoke path ends the drag separately. + const liveBindings = + viewer.useSceneTree.get(activeDrag.nodeName)?.dragBindings ?? []; + const plan = planModifierTransition( + activeDrag.input.modifier, + nextModifier, + liveBindings, + activeDrag.input.button, + activeDrag.segmentActive, + ); + if (plan === null) return; + // Switching away from a combo whose start is still pending + // abandons that segment before it ever hit the wire -- a sub-delay + // pass-through combo (e.g. key rollover while releasing a + // multi-key combo) produces no messages at all. + discardPendingStart(activeDrag); + if (plan.emitEnd) { + // Flush queued throttled updates so the old segment's pending + // update lands *before* its synthetic end -- preserves wire + // ordering across the segment boundary. + flushDragsThrottled(); + sendDragMessage(activeDrag, "end", false); + } + // Switch ownership before scheduling the new ``start`` so the + // start message carries the new modifier. ``button`` is unchanged. + activeDrag.input = { ...activeDrag.input, modifier: nextModifier }; + activeDrag.segmentActive = false; + if (plan.emitStart) { + activeDrag.pendingStart = { + switchPointerXy: [ + activeDrag.endPointerXy[0], + activeDrag.endPointerXy[1], + ], + timerId: setTimeout(() => { + // The drag may have ended since (stopActiveDrag clears the + // timer, but guard against a same-tick race anyway). + if (activeDragRef.current !== activeDrag) return; + flushPendingStart(activeDrag); + }, SWITCH_START_DELAY_MS), + }; + } + }, + [flushDragsThrottled, flushPendingStart, sendDragMessage, viewer], + ); + type EndInfo = { clientX: number; clientY: number; @@ -250,6 +345,13 @@ function DragLayerActive({ children }: { children?: React.ReactNode }) { const activeDrag = activeDragRef.current; if (activeDrag === null) return; + // A release (or cancel/blur/node-removal) while a switch-start is + // still pending discards it: the modifier change and the release + // were one gesture. No start was sent, so no end is owed -- this + // is the deferral's whole purpose (no degenerate start/end pair + // when the modifier lifts a beat before the button). + discardPendingStart(activeDrag); + if (endInfo !== undefined) { // Refresh end fields from the final pointer position; if the // ray misses the plane (rare grazing case) we keep whatever was @@ -258,15 +360,14 @@ function DragLayerActive({ children }: { children?: React.ReactNode }) { } flushDragsThrottled(); - if (sendEndMessage) { - // Modifier state is frozen at drag_start: a drag is "owned" by - // whichever (button, modifiers) combo was held when the user - // pressed the mouse, and stays owned by that combo until release - // regardless of modifier changes mid-drag. This guarantees the - // drag_start / drag_end callbacks see the same dispatch and - // avoids a class of footguns where a key-up arrives a beat - // before mouse-up (downgrading the gesture at the last moment) - // or a modifier is accidentally pressed mid-drag. + if (sendEndMessage && activeDrag.segmentActive) { + // End the currently-active segment. A drag is partitioned into + // one segment per (button, modifier) combo; the modifier can + // switch mid-drag (see ``transitionDragModifier``), and each + // switch already emitted the prior segment's ``end``. So here we + // only emit when a segment is still active -- a release while + // dormant (current modifier matches no binding) has nothing left + // to end. sendDragMessage(activeDrag, "end", false); } @@ -295,10 +396,24 @@ function DragLayerActive({ children }: { children?: React.ReactNode }) { pointerId, input, bindings, + promotionModifier, }) => { if (activeDragRef.current !== null) return false; if (!anyBindingMatches(bindings, input)) return false; + // The gate above validates the POINTERDOWN input against the + // POINTERDOWN bindings (the combo that made this gesture a drag + // candidate). The opening segment is instead attributed to the + // PROMOTION-TIME modifier, planned against the LIVE bindings -- + // both may have changed inside the pointerdown->promotion window + // (a binding-clear cancels the candidate before promotion, but a + // partial edit doesn't). An unbound promotion-time combo begins + // the drag dormant; the key/pointermove listeners below pick up + // the next switch. + const liveBindings = + viewer.useSceneTree.get(nodeName)?.dragBindings ?? []; + const opening = planDragStart(input, promotionModifier, liveBindings); + // Convert the raycast hit point to world coords. The frame of // ``eventPoint`` depends on which raycast produced it: // - ``BatchedMeshesMessage`` / ``BatchedGlbMessage`` use the @@ -354,16 +469,57 @@ function DragLayerActive({ children }: { children?: React.ReactNode }) { if (event.pointerId !== activeDrag.pointerId) return; if (!updateActiveDragEnd(event.clientX, event.clientY)) return; - // Modifier/button state is frozen at drag_start and reused on - // every update/end -- see the note in `stopActiveDrag` above. + // A modifier change carried on this move ends the current + // segment and starts a new one under the new combo. Run it + // *after* refreshing the end position so the synthetic end + // reports the latest pointer location, and *before* the update + // so the update is attributed to the new segment. + const liveModifier = keyModifierFromEvent(event); + if (liveModifier !== activeDrag.input.modifier) { + transitionDragModifier(activeDrag, liveModifier); + } + + // Real motion confirms a pending switch-start early: the + // pointer leaving the switch point past the promotion + // threshold proves the new segment is a deliberate + // continuation, not the leading edge of a release. + if ( + activeDrag.pendingStart !== null && + motionExceedsThreshold( + activeDrag.pendingStart.switchPointerXy, + activeDrag.endPointerXy, + ) + ) { + flushPendingStart(activeDrag); + } + // ``start_position`` is recomputed live inside buildDragMessage, // so the wire payload always reflects the click point's - // current world position (tracking the moving object). - sendDragMessage(activeDrag, "update", true); + // current world position (tracking the moving object). Skip + // while dormant -- the current modifier matches no binding -- + // or while a switch-start is still pending confirmation. + if (activeDrag.segmentActive) { + sendDragMessage(activeDrag, "update", true); + } // The per-frame useFrame updates the arrow tail from target // transforms; no manual update needed here. }; + // Modifier changes can also arrive with the pointer stationary + // (the user taps a modifier key without moving the mouse). Listen + // for key transitions during the drag and re-evaluate ownership + // using the last-known pointer position. ``keyModifierFromEvent`` + // reads ``ctrl/meta/shift/alt`` off the KeyboardEvent, so both + // keydown and keyup resolve the current combo. + const handleWindowKeyChange = (event: KeyboardEvent) => { + const activeDrag = activeDragRef.current; + if (activeDrag === null) return; + const liveModifier = keyModifierFromEvent(event); + if (liveModifier !== activeDrag.input.modifier) { + transitionDragModifier(activeDrag, liveModifier); + } + }; + const handleWindowPointerUp = (event: PointerEvent) => { // Ignore mismatched pointers -- we only end the drag when the // *same* pointer that started it lifts up (or cancels). @@ -385,6 +541,8 @@ function DragLayerActive({ children }: { children?: React.ReactNode }) { window.removeEventListener("pointerup", handleWindowPointerUp); window.removeEventListener("pointercancel", handleWindowPointerUp); window.removeEventListener("blur", handleWindowBlur); + window.removeEventListener("keydown", handleWindowKeyChange); + window.removeEventListener("keyup", handleWindowKeyChange); }; // Plane parallel to the camera image plane, through the start @@ -415,7 +573,16 @@ function DragLayerActive({ children }: { children?: React.ReactNode }) { // / end_* fields agree). endPointWorld: startWorld.clone(), endPointerXy: [pointerXy[0], pointerXy[1]], - input, + input: opening.input, + // Set from the initial ``start`` send below. The opening + // segment is dormant when the promotion-time combo is unbound + // (``opening.emitStart`` false); even when bound, the send can + // still fail if the live grab point is unavailable, so we + // trust its return value rather than assuming ``true``. + segmentActive: false, + // The OPENING segment is never deferred -- crossing the + // motion threshold at promotion is already its confirmation. + pendingStart: null, releaseCameraLock: null, cleanup, }; @@ -426,7 +593,11 @@ function DragLayerActive({ children }: { children?: React.ReactNode }) { window.addEventListener("pointerup", handleWindowPointerUp); window.addEventListener("pointercancel", handleWindowPointerUp); window.addEventListener("blur", handleWindowBlur); - sendDragMessage(activeDragRef.current, "start", false); + window.addEventListener("keydown", handleWindowKeyChange); + window.addEventListener("keyup", handleWindowKeyChange); + activeDragRef.current.segmentActive = opening.emitStart + ? sendDragMessage(activeDragRef.current, "start", false) + : false; return true; }, stopIfNodeIs: (nodeName) => { @@ -441,6 +612,7 @@ function DragLayerActive({ children }: { children?: React.ReactNode }) { frameScratches, sendDragMessage, stopActiveDrag, + transitionDragModifier, updateActiveDragEnd, viewer, viewerMutable, diff --git a/src/viser/client/src/SceneTree.tsx b/src/viser/client/src/SceneTree.tsx index f3164e29f..536b29a75 100644 --- a/src/viser/client/src/SceneTree.tsx +++ b/src/viser/client/src/SceneTree.tsx @@ -1053,7 +1053,11 @@ export function SceneNodeThreeObject(props: { name: string }) { onPromote: beginDragArgs === null || dragLayer === null ? null - : () => dragLayer.beginDrag(beginDragArgs), + : (promotionModifier) => + dragLayer.beginDrag({ + ...beginDragArgs, + promotionModifier, + }), }); if (dragMatches) e.nativeEvent.preventDefault(); } diff --git a/src/viser/client/src/dragLayerContext.ts b/src/viser/client/src/dragLayerContext.ts index a6d821fa1..9218f0691 100644 --- a/src/viser/client/src/dragLayerContext.ts +++ b/src/viser/client/src/dragLayerContext.ts @@ -6,7 +6,7 @@ import React from "react"; import * as THREE from "three"; -import { DragBinding, DragInput } from "./dragUtils"; +import { DragBinding, DragInput, KeyModifier } from "./dragUtils"; export type BeginDragArgs = { nodeName: string; @@ -21,6 +21,13 @@ export type BeginDragArgs = { pointerId: number; input: DragInput; bindings: DragBinding[]; + /** Modifier held at promotion time (the threshold-crossing + * pointermove). May differ from ``input.modifier``, which was sampled + * at pointerdown: DragLayer's key listeners only install at promotion, + * so a change inside the pointerdown-to-promotion window is only + * visible through this value. The opening segment is attributed to + * this combo (dormant when it's unbound). */ + promotionModifier: KeyModifier | null; }; export interface DragLayerApi { diff --git a/src/viser/client/src/dragUtils.test.ts b/src/viser/client/src/dragUtils.test.ts index 7bafc5761..615dd5a0e 100644 --- a/src/viser/client/src/dragUtils.test.ts +++ b/src/viser/client/src/dragUtils.test.ts @@ -7,6 +7,8 @@ import { anyBindingMatches, hasCmdCtrl, motionExceedsThreshold, + planDragStart, + planModifierTransition, MOTION_THRESHOLD_PX, } from "./dragUtils"; @@ -91,6 +93,97 @@ describe("hasCmdCtrl", () => { }); }); +describe("planModifierTransition", () => { + // mjviser-style setup: two bound combos on the left button, used to + // switch the drag plane mid-gesture. + const bindings = [ + { button: "left" as const, modifier: "cmd/ctrl" as const }, + { button: "left" as const, modifier: "cmd/ctrl+shift" as const }, + ]; + + it("is a no-op when the modifier is unchanged", () => { + expect( + planModifierTransition("cmd/ctrl", "cmd/ctrl", bindings, "left", true), + ).toBeNull(); + // Even when nothing is held and nothing changes. + expect( + planModifierTransition(null, null, bindings, "left", false), + ).toBeNull(); + }); + + it("ends the old segment and starts a new one between two bound combos", () => { + expect( + planModifierTransition( + "cmd/ctrl", + "cmd/ctrl+shift", + bindings, + "left", + true, + ), + ).toEqual({ emitEnd: true, emitStart: true }); + }); + + it("ends the segment and goes dormant when the new combo is unbound", () => { + // ctrl is bound, shift-only is not -- releasing ctrl mid-drag drops + // into a dormant gap rather than starting a spurious segment. + expect( + planModifierTransition("cmd/ctrl", "shift", bindings, "left", true), + ).toEqual({ emitEnd: true, emitStart: false }); + }); + + it("starts a fresh segment when re-entering a bound combo from dormant", () => { + // Already dormant (segmentActive=false): no end to emit, just the + // new start. + expect( + planModifierTransition("shift", "cmd/ctrl", bindings, "left", false), + ).toEqual({ emitEnd: false, emitStart: true }); + }); + + it("stays dormant when moving between two unbound combos", () => { + expect( + planModifierTransition("shift", "alt", bindings, "left", false), + ).toEqual({ emitEnd: false, emitStart: false }); + }); + + it("respects the button when matching the new combo", () => { + // The same modifier on a button with no binding is unbound. + expect( + planModifierTransition(null, "cmd/ctrl", bindings, "right", false), + ).toEqual({ emitEnd: false, emitStart: false }); + }); +}); + +describe("planDragStart", () => { + const bindings = [ + { button: "left" as const, modifier: "cmd/ctrl" as const }, + { button: "left" as const, modifier: "cmd/ctrl+shift" as const }, + ]; + const down = { button: "left" as const, modifier: "cmd/ctrl" as const }; + + it("keeps the pointerdown input when the modifier is unchanged", () => { + const plan = planDragStart(down, "cmd/ctrl", bindings); + expect(plan.input).toBe(down); // same reference -- no realloc + expect(plan.emitStart).toBe(true); + }); + + it("attributes the opening segment to the promotion-time modifier", () => { + // Shift added inside the pointerdown->promotion window: the opening + // start carries cmd/ctrl+shift, never a stale cmd/ctrl. + const plan = planDragStart(down, "cmd/ctrl+shift", bindings); + expect(plan.input).toEqual({ button: "left", modifier: "cmd/ctrl+shift" }); + expect(plan.emitStart).toBe(true); + }); + + it("begins dormant when the promotion-time combo is unbound", () => { + // Ctrl released before the threshold crossing: no segment starts (no + // degenerate stale-modifier start/end pair), but the drag itself + // begins -- a later switch back to a bound combo resumes it. + const plan = planDragStart(down, null, bindings); + expect(plan.input).toEqual({ button: "left", modifier: null }); + expect(plan.emitStart).toBe(false); + }); +}); + describe("motionExceedsThreshold", () => { it("is false for movement at or under the threshold (L-infinity)", () => { expect(motionExceedsThreshold([0, 0], [0, 0])).toBe(false); diff --git a/src/viser/client/src/dragUtils.ts b/src/viser/client/src/dragUtils.ts index 47c976fdc..a88c15e4d 100644 --- a/src/viser/client/src/dragUtils.ts +++ b/src/viser/client/src/dragUtils.ts @@ -97,6 +97,69 @@ export function anyBindingMatches( return bindings.some((b) => matchesDragBinding(b, input)); } +/** Plan emitted by :func:`planModifierTransition`: which lifecycle + * messages a mid-drag modifier change should produce. */ +export type DragModifierTransition = { + /** Send a ``phase="end"`` for the *current* (pre-switch) modifier + * before switching ownership. True iff a segment is currently active. */ + emitEnd: boolean; + /** Send a ``phase="start"`` under the *new* modifier after switching. + * True iff the new combo matches a registered binding. */ + emitStart: boolean; +}; + +/** Decide how an in-progress drag reacts to a mid-gesture modifier change. + * + * A single physical drag is partitioned into one logical segment per + * (button, modifier) combo. When the held modifier changes, the current + * segment is ended and -- if the new combo matches a registered binding -- + * a fresh segment is started under it. The grab geometry (plane, grab + * point, instance) is preserved across the boundary by the caller, so the + * drag continues without a visual jump; only which callback set is + * addressed changes. + * + * If the new combo matches no binding the drag goes *dormant*: the + * physical gesture stays alive (camera locked, geometry retained) but no + * messages are sent, so the user's callbacks see properly paired + * start/end per bound combo. Re-entering a bound combo before release + * starts a fresh segment. + * + * Returns ``null`` when the modifier is unchanged (a no-op). */ +export function planModifierTransition( + current: KeyModifier | null, + next: KeyModifier | null, + bindings: DragBinding[], + button: PointerButton, + segmentActive: boolean, +): DragModifierTransition | null { + if (next === current) return null; + return { + emitEnd: segmentActive, + emitStart: anyBindingMatches(bindings, { button, modifier: next }), + }; +} + +/** Decide the opening segment for a drag promoted at the motion + * threshold. ``pointerdownInput`` was sampled at pointerdown, but the + * held modifier may have changed before the threshold-crossing + * pointermove (the promotion event) -- the drag layer's key listeners + * only install at promotion, so that window is otherwise invisible. The + * opening segment is attributed to the promotion-time modifier; when + * that combo is unbound the drag begins *dormant* (no ``start`` sent), + * exactly like a mid-drag switch to an unbound combo (see + * :func:`planModifierTransition`). */ +export function planDragStart( + pointerdownInput: DragInput, + promotionModifier: KeyModifier | null, + bindings: DragBinding[], +): { input: DragInput; emitStart: boolean } { + const input = + promotionModifier === pointerdownInput.modifier + ? pointerdownInput + : { ...pointerdownInput, modifier: promotionModifier }; + return { input, emitStart: anyBindingMatches(bindings, input) }; +} + /** True when the held modifier includes cmd/ctrl. Used to gate browser * context-menu suppression: macOS raises a ``contextmenu`` event on * ctrl+click, and we only want to suppress it when the gesture is a @@ -114,6 +177,19 @@ export function hasCmdCtrl(modifier: KeyModifier | null): boolean { * sites by reference, not by repeated literal. */ export const MOTION_THRESHOLD_PX = 3; +/** How long a *switch-created* drag segment's ``start`` is held back + * before being sent. A mid-drag modifier change ends the old segment + * immediately, but the new segment's ``start`` only goes out once the + * switch is confirmed: by this timer expiring, or sooner by pointer + * motion past :data:`MOTION_THRESHOLD_PX` from the switch point. If the + * button is released first, the pending ``start`` is discarded -- + * releasing the modifier a beat before the mouse button (the natural + * order for ending a modifier-drag) must not fire a degenerate + * ``start``/``end`` pair on whatever combo remains. The same philosophy + * as the motion threshold at drag promotion, applied at segment + * boundaries. */ +export const SWITCH_START_DELAY_MS = 100; + /** ``true`` when the L∞ distance between ``start`` and ``end`` exceeds * :data:`MOTION_THRESHOLD_PX`. Equivalent to the duplicated inline * ``Math.abs(end[0] - start[0]) > N || Math.abs(end[1] - start[1]) > N`` @@ -166,7 +242,28 @@ export type ActiveDragState = { * compute ``end_screen_pos`` and re-cast the pointer ray each * pointermove. */ endPointerXy: [number, number]; + /** Current (button, modifier) the drag is owned by. ``button`` is + * frozen at drag-start; ``modifier`` is *live* -- it switches when the + * held modifier changes mid-drag, partitioning the gesture into one + * segment per combo (see :func:`planModifierTransition`). */ input: DragInput; + /** Whether an in-flight segment is currently active (a ``start`` was + * sent and its ``end`` hasn't). ``false`` while dormant -- the gesture + * is physically held but the current modifier matches no binding, so + * no messages are sent -- and while a switch-created ``start`` is + * still pending confirmation (see ``pendingStart``). */ + segmentActive: boolean; + /** A switch-created segment whose ``start`` has not been sent yet + * (see :data:`SWITCH_START_DELAY_MS`). Confirmed -- and the ``start`` + * sent -- by the timer or by pointer motion past the threshold from + * ``switchPointerXy`` (canvas-relative, captured at the switch); + * discarded wholesale if the drag ends or the modifier changes again + * first, in which case no message was ever sent for the segment. + * ``null`` when no start is pending. */ + pendingStart: { + timerId: ReturnType; + switchPointerXy: [number, number]; + } | null; /** Release for the camera-control lock held for the lifetime of * this drag. Called in `stopActiveDrag` (and on every cancel * path). Routing through `cameraLock` keeps a concurrent diff --git a/src/viser/client/src/pointer/gestures.ts b/src/viser/client/src/pointer/gestures.ts index 257a34ae4..4064aabb0 100644 --- a/src/viser/client/src/pointer/gestures.ts +++ b/src/viser/client/src/pointer/gestures.ts @@ -1,4 +1,5 @@ import { + keyModifierFromEvent, matchesModifierFilter, motionExceedsThreshold, pointerButtonFromNative, @@ -274,7 +275,10 @@ type NodeCandidate = { startClientXy: [number, number]; release: (() => void) | null; cleanup: () => void; - onPromote: (() => boolean) | null; + /** Called with the modifier held on the promoting pointermove -- the + * modifier may have changed since pointerdown, and the drag layer's + * own key listeners only install once the drag begins. */ + onPromote: ((promotionModifier: KeyModifier | null) => boolean) | null; }; export class NodeGestureController { @@ -300,7 +304,7 @@ export class NodeGestureController { nodeKey: string; startClientXy: [number, number]; lockCamera: boolean; - onPromote: (() => boolean) | null; + onPromote: ((promotionModifier: KeyModifier | null) => boolean) | null; }): void { this.cancelCandidate(); @@ -315,7 +319,7 @@ export class NodeGestureController { ) { return; } - this.promoteOrCancelCandidate(); + this.promoteOrCancelCandidate(keyModifierFromEvent(event)); }; const handlePointerUp = (event: PointerEvent) => { const candidate = this.candidate; @@ -384,12 +388,14 @@ export class NodeGestureController { this.cancelAny(); } - private promoteOrCancelCandidate(): void { + private promoteOrCancelCandidate( + promotionModifier: KeyModifier | null, + ): void { const candidate = this.candidate; if (candidate === null) return; const promote = candidate.onPromote; this.cancelCandidate(); - if (promote !== null) promote(); + if (promote !== null) promote(promotionModifier); } private cancelCandidate(): void { diff --git a/tests/e2e/test_scene_node_drag.py b/tests/e2e/test_scene_node_drag.py index 2a1b68925..bc921dce5 100644 --- a/tests/e2e/test_scene_node_drag.py +++ b/tests/e2e/test_scene_node_drag.py @@ -205,6 +205,591 @@ def _(event: viser.SceneNodeDragEvent[viser.BoxHandle]) -> None: assert event.modifier == "cmd/ctrl", event.modifier +def test_scene_node_drag_modifier_switch_mid_drag( + page: Page, + viser_server: viser.ViserServer, +) -> None: + """Changing the held modifier mid-drag ends the current segment and + starts a new one under the new combo, routing each segment to the + callback bound to it. Holding Ctrl, dragging, then adding Shift while + still dragging should: end the ``cmd/ctrl`` segment, then run a full + ``cmd/ctrl+shift`` segment -- all within one physical button press.""" + viser_server.initial_camera.position = (0.0, 0.0, 4.0) + viser_server.initial_camera.look_at = (0.0, 0.0, 0.0) + + ctrl_started = threading.Event() + ctrl_ended = threading.Event() + ctrl_shift_started = threading.Event() + ctrl_shift_ended = threading.Event() + + box = viser_server.scene.add_box( + "/switch_box", + dimensions=(4.0, 4.0, 0.2), + color=(200, 100, 255), + ) + + # The server routes each segment to the callback bound to its combo, + # so a fired event is itself proof the segment carried that modifier. + _on_drag_phase(box, "start", "left", modifier="cmd/ctrl")( + lambda _: ctrl_started.set() + ) + _on_drag_phase(box, "end", "left", modifier="cmd/ctrl")(lambda _: ctrl_ended.set()) + _on_drag_phase(box, "start", "left", modifier="cmd/ctrl+shift")( + lambda _: ctrl_shift_started.set() + ) + _on_drag_phase(box, "end", "left", modifier="cmd/ctrl+shift")( + lambda _: ctrl_shift_ended.set() + ) + + wait_for_connection(page, viser_server.get_port()) + wait_for_scene_node(page, "/switch_box") + + (start_x, start_y), (end_x, end_y) = _get_canvas_drag_points(page) + mid_x = (start_x + end_x) / 2 + mid_y = (start_y + end_y) / 2 + + # cmd/ctrl segment: press and drag halfway. + page.keyboard.down("Control") + page.mouse.move(start_x, start_y) + page.mouse.down() + page.mouse.move(mid_x, mid_y, steps=8) + assert ctrl_started.wait(timeout=5.0), "cmd/ctrl segment didn't start" + + # Add Shift mid-drag -- the keydown alone (no further motion) ends the + # cmd/ctrl segment and starts the cmd/ctrl+shift one. Asserting + # ctrl_ended *before* mouse-up is the crux: it proves the segment + # ended from the modifier switch, not from the release. + page.keyboard.down("Shift") + assert ctrl_ended.wait(timeout=5.0), "cmd/ctrl segment didn't end on modifier add" + assert ctrl_shift_started.wait(timeout=5.0), ( + "cmd/ctrl+shift segment didn't start when Shift was added mid-drag" + ) + + # Keep dragging under the new combo, then release. + page.mouse.move(end_x, end_y, steps=8) + page.mouse.up() + page.keyboard.up("Shift") + page.keyboard.up("Control") + assert ctrl_shift_ended.wait(timeout=5.0), ( + "cmd/ctrl+shift segment didn't end on release" + ) + + +def test_scene_node_drag_dormant_then_resume( + page: Page, + viser_server: viser.ViserServer, +) -> None: + """Switching to an UNBOUND modifier combo mid-drag suspends the gesture + (dormant) rather than ending it: the active segment's ``end`` fires, but + the physical drag stays alive and a *new* segment starts when a bound + combo is re-entered -- all within one button press. + + Only ``cmd/ctrl`` is bound, so adding Shift lands in a dormant gap. The + crux is the resume: if pressing Shift had torn the drag down, releasing + it could NOT produce a second ``cmd/ctrl`` start without a fresh + mouse-down. Both transitions here are driven by key events with the + pointer stationary, so this also exercises the keydown/keyup path.""" + viser_server.initial_camera.position = (0.0, 0.0, 4.0) + viser_server.initial_camera.look_at = (0.0, 0.0, 0.0) + + starts = [0] + ends = [0] + first_start = threading.Event() + first_end = threading.Event() + resumed_start = threading.Event() + second_end = threading.Event() + lock = threading.Lock() + + box = viser_server.scene.add_box( + "/dormant_box", + dimensions=(4.0, 4.0, 0.2), + color=(120, 220, 160), + ) + + def _on_start(_event: viser.SceneNodeDragEvent[viser.BoxHandle]) -> None: + with lock: + starts[0] += 1 + n = starts[0] + (first_start if n == 1 else resumed_start).set() + + def _on_end(_event: viser.SceneNodeDragEvent[viser.BoxHandle]) -> None: + with lock: + ends[0] += 1 + n = ends[0] + (first_end if n == 1 else second_end).set() + + _on_drag_phase(box, "start", "left", modifier="cmd/ctrl")(_on_start) + _on_drag_phase(box, "end", "left", modifier="cmd/ctrl")(_on_end) + + wait_for_connection(page, viser_server.get_port()) + wait_for_scene_node(page, "/dormant_box") + + (start_x, start_y), (end_x, end_y) = _get_canvas_drag_points(page) + mid_x = (start_x + end_x) / 2 + mid_y = (start_y + end_y) / 2 + + # cmd/ctrl segment: press and drag partway. + page.keyboard.down("Control") + page.mouse.move(start_x, start_y) + page.mouse.down() + page.mouse.move(mid_x, mid_y, steps=8) + assert first_start.wait(timeout=5.0), "cmd/ctrl segment didn't start" + + # Add Shift (cmd/ctrl+shift is unbound) -> dormant. The active segment + # ends now, BEFORE any release -- proving the switch, not the mouse-up, + # ended it. + page.keyboard.down("Shift") + assert first_end.wait(timeout=5.0), "segment didn't end when going dormant" + with lock: + assert ends[0] == 1 and starts[0] == 1, (starts[0], ends[0]) + + # Drag while dormant -- no segment is active, so nothing should fire. + page.mouse.move(end_x, end_y, steps=8) + + # Release Shift -> back to the bound cmd/ctrl combo -> RESUME with a + # fresh segment, even though the pointer is stationary and the button + # was never released. + page.keyboard.up("Shift") + assert resumed_start.wait(timeout=5.0), ( + "drag did not resume on re-entering cmd/ctrl -- dormant was treated " + "as a full teardown instead of a suspend" + ) + + page.mouse.up() + page.keyboard.up("Control") + assert second_end.wait(timeout=5.0), "resumed segment didn't end on release" + + # Exactly two clean segments, no churn from the dormant gap. + with lock: + assert starts[0] == 2, starts[0] + assert ends[0] == 2, ends[0] + + +def test_scene_node_drag_modifier_change_before_promotion( + page: Page, + viser_server: viser.ViserServer, +) -> None: + """A modifier change inside the pointerdown->promotion window is honored. + + The drag candidate samples its modifier at pointerdown, but the drag + only *promotes* at the motion threshold -- and the drag layer's key + listeners install at promotion. Releasing Ctrl in that window used to + be invisible: the promoted drag opened a stale ``cmd/ctrl`` segment + (start + end) even though no modifier was held for any of the actual + motion. + + Now the opening segment is attributed to the promotion-time modifier: + with Ctrl released before the threshold crossing, the drag begins + DORMANT -- no start fires -- and re-pressing Ctrl mid-gesture resumes + it with a properly-attributed segment (proving the gesture began as a + live drag rather than being dropped).""" + viser_server.initial_camera.position = (0.0, 0.0, 4.0) + viser_server.initial_camera.look_at = (0.0, 0.0, 0.0) + + starts = [0] + ends = [0] + started = threading.Event() + ended = threading.Event() + lock = threading.Lock() + + box = viser_server.scene.add_box( + "/prepromo_box", + dimensions=(4.0, 4.0, 0.2), + color=(255, 180, 90), + ) + + def _on_start(_event: viser.SceneNodeDragEvent[viser.BoxHandle]) -> None: + with lock: + starts[0] += 1 + started.set() + + def _on_end(_event: viser.SceneNodeDragEvent[viser.BoxHandle]) -> None: + with lock: + ends[0] += 1 + ended.set() + + _on_drag_phase(box, "start", "left", modifier="cmd/ctrl")(_on_start) + _on_drag_phase(box, "end", "left", modifier="cmd/ctrl")(_on_end) + + wait_for_connection(page, viser_server.get_port()) + wait_for_scene_node(page, "/prepromo_box") + + (start_x, start_y), (end_x, end_y) = _get_canvas_drag_points(page) + + # Pointerdown under the bound combo, then release Ctrl BEFORE any + # motion -- the change lands inside the promotion blind spot. + page.keyboard.down("Control") + page.mouse.move(start_x, start_y) + page.mouse.down() + page.keyboard.up("Control") + + # Cross the threshold and keep dragging with nothing held. The drag + # must begin dormant: no stale cmd/ctrl start (the old bug fired a + # degenerate start/end pair here). + page.mouse.move(end_x, end_y, steps=12) + assert not started.wait(timeout=1.0), ( + "a cmd/ctrl segment started even though Ctrl was released before " + "the promotion threshold (stale pointerdown modifier)" + ) + + # Re-press Ctrl with the button still down: the dormant drag resumes + # with a real cmd/ctrl segment -- distinguishing "began dormant" from + # "never began at all". + page.keyboard.down("Control") + assert started.wait(timeout=5.0), ( + "re-pressing the bound modifier mid-gesture did not resume the " + "dormant drag -- was the pre-promotion gesture dropped entirely?" + ) + + page.mouse.up() + page.keyboard.up("Control") + assert ended.wait(timeout=5.0), "resumed segment didn't end on release" + with lock: + assert starts[0] == 1 and ends[0] == 1, (starts[0], ends[0]) + + +def test_scene_node_drag_binding_added_mid_drag( + page: Page, + viser_server: viser.ViserServer, +) -> None: + """A binding registered while a drag is in progress takes effect at the + next modifier switch. + + Bindings are read live at each switch (not snapshotted at drag + start): registering a ``cmd/ctrl+shift`` callback mid-``cmd/ctrl``-drag + and then pressing Shift must end the cmd/ctrl segment and START a + cmd/ctrl+shift one. With a drag-start snapshot, the switch would land + in a dormant gap and the new callback would never fire without a fresh + mouse-down.""" + viser_server.initial_camera.position = (0.0, 0.0, 4.0) + viser_server.initial_camera.look_at = (0.0, 0.0, 0.0) + + ctrl_started = threading.Event() + ctrl_ended = threading.Event() + ctrl_shift_started = threading.Event() + ctrl_shift_ended = threading.Event() + + box = viser_server.scene.add_box( + "/live_bindings_box", + dimensions=(4.0, 4.0, 0.2), + color=(90, 180, 255), + ) + + _on_drag_phase(box, "start", "left", modifier="cmd/ctrl")( + lambda _: ctrl_started.set() + ) + _on_drag_phase(box, "end", "left", modifier="cmd/ctrl")(lambda _: ctrl_ended.set()) + + wait_for_connection(page, viser_server.get_port()) + wait_for_scene_node(page, "/live_bindings_box") + + (start_x, start_y), (end_x, end_y) = _get_canvas_drag_points(page) + mid_x = (start_x + end_x) / 2 + mid_y = (start_y + end_y) / 2 + + # cmd/ctrl segment: press and drag partway. + page.keyboard.down("Control") + page.mouse.move(start_x, start_y) + page.mouse.down() + page.mouse.move(mid_x, mid_y, steps=8) + assert ctrl_started.wait(timeout=5.0), "cmd/ctrl segment didn't start" + + # Register the cmd/ctrl+shift binding MID-DRAG, and give the update + # message a beat to reach the client. + _on_drag_phase(box, "start", "left", modifier="cmd/ctrl+shift")( + lambda _: ctrl_shift_started.set() + ) + _on_drag_phase(box, "end", "left", modifier="cmd/ctrl+shift")( + lambda _: ctrl_shift_ended.set() + ) + page.wait_for_timeout(500) + + # Shift lands the switch on the freshly-registered combo. + page.keyboard.down("Shift") + assert ctrl_ended.wait(timeout=5.0), "cmd/ctrl segment didn't end on switch" + assert ctrl_shift_started.wait(timeout=5.0), ( + "the mid-drag-registered cmd/ctrl+shift binding didn't own the new " + "segment -- bindings were snapshotted at drag start instead of read live" + ) + + page.mouse.move(end_x, end_y, steps=8) + page.mouse.up() + page.keyboard.up("Shift") + page.keyboard.up("Control") + assert ctrl_shift_ended.wait(timeout=5.0), ( + "cmd/ctrl+shift segment didn't end on release" + ) + + +def test_scene_node_drag_modifier_release_before_button( + page: Page, + viser_server: viser.ViserServer, +) -> None: + """Releasing the modifier a beat before the mouse button must not fire + a degenerate segment on the combo left behind. + + With both a bare binding (no modifier) and ``cmd/ctrl`` registered, a + Ctrl keyup mid-drag switches to the bare combo -- but the + switch-created segment's ``start`` is deferred + (``SWITCH_START_DELAY_MS``) and discarded if the button comes up + inside the window. Two gestures pin both halves: + + A. CONFIRM: Ctrl keyup with the button HELD. The ctrl segment ends on + the keyup (asserted before any release), and the pending bare + start fires on the deferral timer -- proving the keyup really + creates a pending segment (gesture B's zero isn't vacuous: the + identical keyup, un-released, produces a bare segment). + B. DISCARD: Ctrl keyup + pointerup dispatched in the SAME JS task + (both listeners are window-level, so synthetic events reach them; + the same-task dispatch makes the sub-window gap deterministic -- + real key/mouse calls race the 100ms timer on slow CI). The keyup + schedules the pending bare start; the pointerup discards it before + any timer can fire. Bare must not budge.""" + viser_server.initial_camera.position = (0.0, 0.0, 4.0) + viser_server.initial_camera.look_at = (0.0, 0.0, 0.0) + + bare_starts = [0] + bare_ends = [0] + ctrl_starts = [0] + ctrl_ends = [0] + lock = threading.Lock() + ctrl_started = threading.Event() + ctrl_ended = threading.Event() + bare_started = threading.Event() + bare_ended = threading.Event() + + box = viser_server.scene.add_box( + "/release_order_box", + dimensions=(4.0, 4.0, 0.2), + color=(240, 120, 120), + ) + + def _bare_start(_event: viser.SceneNodeDragEvent[viser.BoxHandle]) -> None: + with lock: + bare_starts[0] += 1 + bare_started.set() + + def _bare_end(_event: viser.SceneNodeDragEvent[viser.BoxHandle]) -> None: + with lock: + bare_ends[0] += 1 + bare_ended.set() + + def _ctrl_start(_event: viser.SceneNodeDragEvent[viser.BoxHandle]) -> None: + with lock: + ctrl_starts[0] += 1 + ctrl_started.set() + + def _ctrl_end(_event: viser.SceneNodeDragEvent[viser.BoxHandle]) -> None: + with lock: + ctrl_ends[0] += 1 + ctrl_ended.set() + + _on_drag_phase(box, "start", "left")(_bare_start) + _on_drag_phase(box, "end", "left")(_bare_end) + _on_drag_phase(box, "start", "left", modifier="cmd/ctrl")(_ctrl_start) + _on_drag_phase(box, "end", "left", modifier="cmd/ctrl")(_ctrl_end) + + wait_for_connection(page, viser_server.get_port()) + wait_for_scene_node(page, "/release_order_box") + + (start_x, start_y), (end_x, end_y) = _get_canvas_drag_points(page) + + # Record the mouse pointerId for gesture B's same-task synthetic + # release (handleWindowPointerUp filters on it). + page.evaluate( + """() => { window.__pid = null; + window.addEventListener('pointermove', + (e) => { window.__pid = e.pointerId; }, true); }""" + ) + + # --- Gesture A (CONFIRM): Ctrl keyup with the button HELD. + page.keyboard.down("Control") + page.mouse.move(start_x, start_y) + page.mouse.down() + page.mouse.move(end_x, end_y, steps=8) + assert ctrl_started.wait(timeout=5.0), "cmd/ctrl segment didn't start" + page.keyboard.up("Control") + # The ctrl end comes from the KEYUP -- asserted before any release. + assert ctrl_ended.wait(timeout=5.0), ( + "cmd/ctrl segment didn't end on keyup (button still held)" + ) + # The keyup scheduled a pending bare start; held stationary past the + # deferral window, the timer confirms it. + assert bare_started.wait(timeout=5.0), ( + "held past the deferral window, the pending bare start should have " + "fired on the timer" + ) + page.mouse.up() + assert bare_ended.wait(timeout=5.0), "bare segment didn't end on release" + with lock: + assert ctrl_starts[0] == 1 and ctrl_ends[0] == 1, ( + ctrl_starts[0], + ctrl_ends[0], + ) + assert bare_starts[0] == 1 and bare_ends[0] == 1, ( + bare_starts[0], + bare_ends[0], + ) + + # --- Gesture B (DISCARD): the identical keyup, but the button comes + # up in the SAME JS task -- inside the window, deterministically. + page.keyboard.down("Control") + page.mouse.move(start_x, start_y) + page.mouse.down() + page.mouse.move(end_x, end_y, steps=8) + for _ in range(50): + with lock: + if ctrl_starts[0] == 2: + break + page.wait_for_timeout(100) + with lock: + assert ctrl_starts[0] == 2, "second cmd/ctrl segment didn't start" + pid = page.evaluate("() => window.__pid") + assert pid is not None, "pointer id not observed" + page.evaluate( + """([x, y, pid]) => { + window.dispatchEvent(new KeyboardEvent('keyup', { bubbles: true })); + window.dispatchEvent(new PointerEvent('pointerup', { + bubbles: true, clientX: x, clientY: y, button: 0, + pointerId: pid, pointerType: 'mouse', isPrimary: true })); + }""", + [end_x, end_y, pid], + ) + # Reset the real input state (the drag already ended; both no-op). + page.mouse.up() + page.keyboard.up("Control") + + for _ in range(50): + with lock: + if ctrl_ends[0] == 2: + break + page.wait_for_timeout(100) + # Ample time for any (buggy) surviving pending start to fire. + page.wait_for_timeout(600) + with lock: + assert ctrl_starts[0] == 2 and ctrl_ends[0] == 2, ( + ctrl_starts[0], + ctrl_ends[0], + ) + assert bare_starts[0] == 1 and bare_ends[0] == 1, ( + "modifier-first release fired a degenerate segment on the bare " + f"binding: {bare_starts[0]} starts, {bare_ends[0]} ends " + "(expected the gesture-A pair only)" + ) + + +def test_scene_node_drag_many_stages_one_drag( + page: Page, + viser_server: viser.ViserServer, +) -> None: + """A single drag that cycles through several segments AND a dormant gap: + cmd/ctrl -> cmd/ctrl+shift -> (dormant) -> cmd/ctrl+shift -> cmd/ctrl, + all within one button press. Both combos are entered twice, with an + unbound (dormant) interval in the middle, exercising repeated switching, + re-entry, and resume-after-dormant in one gesture. Each callback must + see exactly two clean start...end pairs.""" + viser_server.initial_camera.position = (0.0, 0.0, 4.0) + viser_server.initial_camera.look_at = (0.0, 0.0, 0.0) + + ctrl_starts = [0] + ctrl_ends = [0] + cs_starts = [0] + cs_ends = [0] + lock = threading.Lock() + # One event per milestone we gate the gesture on. + ev = { + k: threading.Event() for k in ("cs_s1", "cs_e1", "cs_s2", "ctrl_s2", "ctrl_e2") + } + + box = viser_server.scene.add_box( + "/many_stage_box", + dimensions=(4.0, 4.0, 0.2), + color=(160, 140, 240), + ) + + # Async callbacks are awaited in dispatch order on the event loop, so + # the counters settle deterministically (sync callbacks fire-and-forget + # on a thread pool and would race the final count assertions -- step 4 + # below fires two callbacks at once). + async def _ctrl_start(_e: viser.SceneNodeDragEvent[viser.BoxHandle]) -> None: + with lock: + ctrl_starts[0] += 1 + n = ctrl_starts[0] + if n == 2: + ev["ctrl_s2"].set() + + async def _ctrl_end(_e: viser.SceneNodeDragEvent[viser.BoxHandle]) -> None: + with lock: + ctrl_ends[0] += 1 + n = ctrl_ends[0] + if n == 2: + ev["ctrl_e2"].set() + + async def _cs_start(_e: viser.SceneNodeDragEvent[viser.BoxHandle]) -> None: + with lock: + cs_starts[0] += 1 + n = cs_starts[0] + ev["cs_s1" if n == 1 else "cs_s2"].set() + + async def _cs_end(_e: viser.SceneNodeDragEvent[viser.BoxHandle]) -> None: + with lock: + cs_ends[0] += 1 + n = cs_ends[0] + if n == 1: + ev["cs_e1"].set() + + _on_drag_phase(box, "start", "left", modifier="cmd/ctrl")(_ctrl_start) + _on_drag_phase(box, "end", "left", modifier="cmd/ctrl")(_ctrl_end) + _on_drag_phase(box, "start", "left", modifier="cmd/ctrl+shift")(_cs_start) + _on_drag_phase(box, "end", "left", modifier="cmd/ctrl+shift")(_cs_end) + + wait_for_connection(page, viser_server.get_port()) + wait_for_scene_node(page, "/many_stage_box") + + (start_x, start_y), (end_x, end_y) = _get_canvas_drag_points(page) + q_x = start_x + (end_x - start_x) / 3 + q_y = start_y + (end_y - start_y) / 3 + + # Segment 1: cmd/ctrl. + page.keyboard.down("Control") + page.mouse.move(start_x, start_y) + page.mouse.down() + page.mouse.move(q_x, q_y, steps=6) + + # Segment 2: add Shift -> cmd/ctrl+shift (bound). + page.keyboard.down("Shift") + assert ev["cs_s1"].wait(timeout=5.0), "cmd/ctrl+shift didn't start on Shift add" + + # Dormant: release Ctrl -> shift-only (unbound). cmd/ctrl+shift ends. + page.keyboard.up("Control") + assert ev["cs_e1"].wait(timeout=5.0), "cmd/ctrl+shift didn't end going dormant" + + # Drag while dormant (nothing should fire). + page.mouse.move(end_x, end_y, steps=6) + + # Resume: press Ctrl -> cmd/ctrl+shift again (bound). + page.keyboard.down("Control") + assert ev["cs_s2"].wait(timeout=5.0), "cmd/ctrl+shift didn't resume after dormant" + + # Segment 4: release Shift -> back to cmd/ctrl (bound). + page.keyboard.up("Shift") + assert ev["ctrl_s2"].wait(timeout=5.0), "cmd/ctrl didn't restart on Shift release" + + # Release. + page.mouse.up() + page.keyboard.up("Control") + assert ev["ctrl_e2"].wait(timeout=5.0), ( + "final cmd/ctrl segment didn't end on release" + ) + + # Exactly two clean segments per combo across the whole gesture. + with lock: + assert ctrl_starts[0] == 2, ctrl_starts[0] + assert ctrl_ends[0] == 2, ctrl_ends[0] + assert cs_starts[0] == 2, cs_starts[0] + assert cs_ends[0] == 2, cs_ends[0] + + def test_scene_node_drag_filter_rejects_wrong_modifier( page: Page, viser_server: viser.ViserServer, diff --git a/tests/test_drag_modifier_segments.py b/tests/test_drag_modifier_segments.py new file mode 100644 index 000000000..d801515d5 --- /dev/null +++ b/tests/test_drag_modifier_segments.py @@ -0,0 +1,227 @@ +"""Unit tests for mid-drag modifier switching (drag "segments"). + +A single physical drag is partitioned into one *segment* per held +(button, modifier) combo. When the held modifier changes mid-drag the +client ends the current segment and starts a new one under the new +combo; the server routes each segment's start/update/end to whichever +callback that combo is bound to (``_handle_node_drag`` -> +``_dispatch_drag_callbacks``). + +These tests drive ``_handle_node_drag`` directly with a crafted message +sequence -- no browser -- to lock down that the server bookkeeping +(``_active_drag_handles``) and per-modifier dispatch stay consistent +across the synthetic end/start boundary the client emits. The real +client-side segmentation (key listeners, geometry preservation) is +covered by the e2e suite and the ``planModifierTransition`` unit tests. +""" + +from __future__ import annotations + +import asyncio +from typing import Generator, cast +from unittest.mock import Mock + +import pytest + +import viser +from viser import _messages +from viser.infra import ClientId + + +@pytest.fixture +def server() -> Generator[viser.ViserServer, None, None]: + s = viser.ViserServer() + try: + yield s + finally: + s.stop() + + +def _drag_msg( + name: str, + phase: _messages._DragPhase, + modifier: _messages.KeyModifier | None, +) -> _messages.SceneNodeDragMessage: + return _messages.SceneNodeDragMessage( + phase=phase, + name=name, + instance_index=None, + start_position=(0.0, 0.0, 0.0), + start_screen_pos=(0.0, 0.0), + end_position=(1.0, 1.0, 1.0), + end_screen_pos=(1.0, 1.0), + button="left", + modifier=modifier, + ) + + +def _dispatch( + server: viser.ViserServer, + client_id: ClientId, + message: _messages.SceneNodeDragMessage, +) -> None: + """Run ``_handle_node_drag`` on the server's event loop and block.""" + asyncio.run_coroutine_threadsafe( + server.scene._handle_node_drag(client_id, message), + server._event_loop, + ).result() + + +def test_modifier_switch_routes_segments_to_each_binding( + server: viser.ViserServer, +) -> None: + """Switching the held modifier mid-drag routes each segment's + start/update/end to the callback bound to that combo, and leaves the + other combo's callback untouched.""" + box = server.scene.add_box("/seg_box", dimensions=(1.0, 1.0, 1.0)) + + ctrl_phases: list[str] = [] + ctrl_shift_phases: list[str] = [] + + # Async callbacks are awaited in dispatch order on the event loop, so + # the phase lists are complete and deterministic once ``_dispatch`` + # returns. (Sync callbacks are fire-and-forget on a thread pool and + # would race the assertions.) + @box.on_drag("left", modifier="cmd/ctrl") + async def _(event: viser.SceneNodeDragEvent) -> None: + ctrl_phases.append(event.phase) + # The active segment's modifier always matches this binding. + assert event.modifier == "cmd/ctrl" + + @box.on_drag("left", modifier="cmd/ctrl+shift") + async def _(event: viser.SceneNodeDragEvent) -> None: + ctrl_shift_phases.append(event.phase) + assert event.modifier == "cmd/ctrl+shift" + + client = cast(ClientId, 42) + server._connected_clients[client] = Mock() + + # The client emits this sequence when the user holds Ctrl, drags, + # then adds Shift mid-drag and keeps dragging before releasing: the + # Ctrl segment is ended and a Ctrl+Shift segment is started, with the + # grab geometry preserved across the boundary. + _dispatch(server, client, _drag_msg("/seg_box", "start", "cmd/ctrl")) + _dispatch(server, client, _drag_msg("/seg_box", "update", "cmd/ctrl")) + _dispatch(server, client, _drag_msg("/seg_box", "end", "cmd/ctrl")) + _dispatch(server, client, _drag_msg("/seg_box", "start", "cmd/ctrl+shift")) + _dispatch(server, client, _drag_msg("/seg_box", "update", "cmd/ctrl+shift")) + _dispatch(server, client, _drag_msg("/seg_box", "end", "cmd/ctrl+shift")) + + # Each callback sees exactly one clean start...end for its own combo, + # and nothing from the other segment. + assert ctrl_phases == ["start", "update", "end"] + assert ctrl_shift_phases == ["start", "update", "end"] + + # Bookkeeping released after the final end. + assert not server.scene._is_drag_active_for("/seg_box") + + +def test_modifier_switch_keeps_drag_active_across_boundary( + server: viser.ViserServer, +) -> None: + """The node stays "actively dragged" across the segment boundary -- + the end of one segment is immediately followed by the start of the + next, so a concurrent ``remove()`` would still preserve callbacks + until the gesture truly finishes.""" + box = server.scene.add_box("/active_box", dimensions=(1.0, 1.0, 1.0)) + box.on_drag("left", modifier="cmd/ctrl")(lambda _: None) + box.on_drag("left", modifier="cmd/ctrl+shift")(lambda _: None) + + client = cast(ClientId, 7) + server._connected_clients[client] = Mock() + + _dispatch(server, client, _drag_msg("/active_box", "start", "cmd/ctrl")) + assert server.scene._is_drag_active_for("/active_box") + + # Old segment ends... + _dispatch(server, client, _drag_msg("/active_box", "end", "cmd/ctrl")) + # ...new segment starts. The node is active again immediately. + _dispatch(server, client, _drag_msg("/active_box", "start", "cmd/ctrl+shift")) + assert server.scene._is_drag_active_for("/active_box") + + _dispatch(server, client, _drag_msg("/active_box", "end", "cmd/ctrl+shift")) + assert not server.scene._is_drag_active_for("/active_box") + + +def test_unbound_segment_does_not_dispatch( + server: viser.ViserServer, +) -> None: + """A segment whose modifier matches no binding dispatches to nobody + (server-side filter), but still cycles the active-drag bookkeeping + cleanly. In practice the client suppresses these entirely (dormant + state); this pins the server's filter as a backstop.""" + box = server.scene.add_box("/unbound_box", dimensions=(1.0, 1.0, 1.0)) + + ctrl_phases: list[str] = [] + + @box.on_drag("left", modifier="cmd/ctrl") + async def _(event: viser.SceneNodeDragEvent) -> None: + ctrl_phases.append(event.phase) + + client = cast(ClientId, 99) + server._connected_clients[client] = Mock() + + _dispatch(server, client, _drag_msg("/unbound_box", "start", "cmd/ctrl")) + _dispatch(server, client, _drag_msg("/unbound_box", "end", "cmd/ctrl")) + # Shift-only is unbound: no callback fires for this segment. + _dispatch(server, client, _drag_msg("/unbound_box", "start", "shift")) + _dispatch(server, client, _drag_msg("/unbound_box", "update", "shift")) + _dispatch(server, client, _drag_msg("/unbound_box", "end", "shift")) + + assert ctrl_phases == ["start", "end"] + assert not server.scene._is_drag_active_for("/unbound_box") + + +def test_many_segments_one_drag_routes_and_cycles( + server: viser.ViserServer, +) -> None: + """A single drag that switches modifiers SEVERAL times -- including + re-entering a previously-used combo -- routes each segment to its + callback as a clean, ordered start...end pair, and the active-drag + bookkeeping registers/pops correctly across every boundary. + + Dormant (unbound) gaps produce no wire messages (the client suppresses + them), so a realistic multi-stage stream is just the bound segments + back to back. We interleave cmd/ctrl and cmd/ctrl+shift four times to + exercise repeated switching and re-entry of both combos in one drag.""" + box = server.scene.add_box("/many_box", dimensions=(1.0, 1.0, 1.0)) + + ctrl_phases: list[str] = [] + ctrl_shift_phases: list[str] = [] + + @box.on_drag("left", modifier="cmd/ctrl") + async def _(event: viser.SceneNodeDragEvent) -> None: + ctrl_phases.append(event.phase) + assert event.modifier == "cmd/ctrl" + + @box.on_drag("left", modifier="cmd/ctrl+shift") + async def _(event: viser.SceneNodeDragEvent) -> None: + ctrl_shift_phases.append(event.phase) + assert event.modifier == "cmd/ctrl+shift" + + client = cast(ClientId, 13) + server._connected_clients[client] = Mock() + + # ctrl -> ctrl+shift -> ctrl -> ctrl+shift, all in one physical drag. + sequence: list[tuple[_messages._DragPhase, _messages.KeyModifier]] = [ + ("start", "cmd/ctrl"), + ("update", "cmd/ctrl"), + ("end", "cmd/ctrl"), + ("start", "cmd/ctrl+shift"), + ("update", "cmd/ctrl+shift"), + ("end", "cmd/ctrl+shift"), + ("start", "cmd/ctrl"), # re-enter ctrl + ("end", "cmd/ctrl"), + ("start", "cmd/ctrl+shift"), # re-enter ctrl+shift + ("end", "cmd/ctrl+shift"), + ] + for phase, modifier in sequence: + _dispatch(server, client, _drag_msg("/many_box", phase, modifier)) + + # Each callback sees its two segments as clean, ordered, paired phases + # -- no cross-talk, no dropped or duplicated start/end across the four + # switches. + assert ctrl_phases == ["start", "update", "end", "start", "end"] + assert ctrl_shift_phases == ["start", "update", "end", "start", "end"] + # Bookkeeping fully released after the final end. + assert not server.scene._is_drag_active_for("/many_box")