replace orientation widget observer with canvas mouseup mark

Author: LKasianAnsys

src/ansys/visor/visor-client/src/VisorFrontend.tsx

@@ -52,7 +52,10 @@ export class VisorFrontend {
         } = getPromiseResolver<UiScaffoldUtil>();
         let darkMode: boolean = true;
 
-        // TODO: uncomment these lines when the dataset addition bug is fixed
+        // TODO: uncomment these lines once VTK is upgraded past 9.6.1. getVtkObject
+        // on the orientation widget serializes the widget's graph, and the client-only
+        // ids the proxy allocates then collide with the next add_dataset's objects.
+        // Fixed after 9.6.1 by SetAllocateIdsDescending.
         // const orientationWidget = vtkScene.getVtkObject(vtkInfo.orientationWidgetWasmId);
         // use "void" here to suppress the "no await" IDE warning
         // void orientationWidget.SetShouldResetCamera(false);

src/ansys/visor/visor-client/src/jest-tests/CameraGestureTracker.test.js

@@ -211,23 +211,54 @@ describe('CameraGestureTracker', () => {
         expect(onSettled).toHaveBeenCalledWith('programmatic');
     });
 
-    // ---- the orientation widget's mark --------------------------------------
+    // ---- a release on the wasm canvas ---------------------------------------
 
-    test('a camera event followed by the widget mark within 300 ms reports gesture', () => {
-        // The widget's mark can arrive after the camera events it belongs
-        // to, so this exercises the retroactive branch of noteWidgetGesture.
-        // The mark-first order is covered by the wheel test above, via the
-        // same #markImpulse code path.
-        tracker.noteCameraEvent();
+    // A face click reaches the camera only after the release, with no button
+    // held, so the release is what arms the window. All three raise the camera
+    // event at 299 ms, inside the 300 ms a release arms, so what separates
+    // them is solely whether the release armed it.
+
+    test('a release on the canvas, then a camera event within 300 ms, reports gesture', () => {
+        canvas.dispatchEvent(new MouseEvent('mouseup', { button: 0, bubbles: true }));
         jest.advanceTimersByTime(299);
+        tracker.noteCameraEvent();
 
-        tracker.noteWidgetGesture();
         jest.advanceTimersByTime(300);
 
         expect(onSettled).toHaveBeenCalledTimes(1);
         expect(onSettled).toHaveBeenCalledWith('gesture');
     });
 
+    test('a release whose target is not the canvas, then the same, reports programmatic', () => {
+        // A child of canvasDiv, so it is on the capture path and reaches the
+        // same listener: only the target check can tell it from the canvas.
+        const overlayButton = document.createElement('button');
+        canvasDiv.appendChild(overlayButton);
+
+        overlayButton.dispatchEvent(new MouseEvent('mouseup', { button: 0, bubbles: true }));
+        jest.advanceTimersByTime(299);
+        tracker.noteCameraEvent();
+
+        jest.advanceTimersByTime(300);
+
+        expect(onSettled).toHaveBeenCalledTimes(1);
+        expect(onSettled).toHaveBeenCalledWith('programmatic');
+    });
+
+    test('a non-bubbling release on the canvas, then the same, reports programmatic', () => {
+        // Exactly what applyMouseEvent fires at the canvas on every press,
+        // release and mouseout. Its target is the canvas, so without the
+        // bubbles check a press alone would mark this settle a gesture.
+        canvas.dispatchEvent(new MouseEvent('mouseup', { button: 0, bubbles: false }));
+        jest.advanceTimersByTime(299);
+        tracker.noteCameraEvent();
+
+        jest.advanceTimersByTime(300);
+
+        expect(onSettled).toHaveBeenCalledTimes(1);
+        expect(onSettled).toHaveBeenCalledWith('programmatic');
+    });
+
     // ---- listener management and teardown ----------------------------------
 
     test('the remover returned by addSettledListener stops reports', () => {

src/ansys/visor/visor-client/src/jest-tests/WasmRendererWidgetTriggers.test.tsx

@@ -103,14 +103,7 @@ function makeFakeWasmObjects() {
         SetOrigin: jest.fn(async () => undefined),
         SetNormal: jest.fn(async () => undefined),
     };
-    // The orientation widget is its own double, not the shared `widget`
-    // above. Both are observed on `EndInteractionEvent`, and one double for
-    // both cannot tell those two registrations apart: the plane's own test
-    // asserts exactly one such registration, and would see two.
-    const orientationWidget = {
-        observe: jest.fn(),
-    };
-    return { actor, property, widget, orientationWidget };
+    return { actor, property, widget };
 }
 
 /**
@@ -144,23 +137,19 @@ async function makeRenderer(
         canvasDiv: document.createElement('div'),
         render: jest.fn(),
         clearObserversAndEventListeners: jest.fn(),
-        // The tracker itself is `#private` to VtkScene, so what the renderer
-        // can reach is this one passthrough, and this is what the orientation
-        // registration is pinned against.
-        noteWidgetGesture: jest.fn(),
         camera,
-        getVtkObject: (wasmId: number) => {
+        // A jest.fn, so the ids the renderer asks for are recorded: the
+        // orientation widget's id must never be among them.
+        getVtkObject: jest.fn((wasmId: number) => {
             switch (wasmId) {
                 case ACTOR_ID:
                     return objects.actor;
                 case PROPERTY_ID:
                     return objects.property;
-                case ORIENTATION_WIDGET_ID:
-                    return objects.orientationWidget;
                 default:
                     return objects.widget;
             }
-        },
+        }),
     };
     const renderer = await WasmRenderer.createAsync(
         scene as unknown as VtkScene,
@@ -350,27 +339,15 @@ describe('WasmRenderer reports the cross-section plane on the end-of-drag event'
     });
 });
 
-describe('WasmRenderer marks an orientation-widget move as a gesture', () => {
-    // As above, the event does not exist under jsdom, so what is pinned is
-    // the *registration*: which event the mark is bound to, that there is
-    // exactly one of it on the orientation widget, and that the callback
-    // marks and sends nothing. Whether the wasm widget invokes that event at
-    // all is MC-I6's subject and no gate reaches it.
-    test('the orientation widget is observed once on EndInteractionEvent and the callback marks a widget gesture', async () => {
-        const sender = makeSender();
-        const { scene, orientationWidget } = await makeRenderer(sender);
-
-        const endCalls = orientationWidget.observe.mock.calls.filter(
-            (call) => call[0] === 'EndInteractionEvent'
-        );
-        expect(endCalls).toHaveLength(1);
-        expect(scene.noteWidgetGesture).not.toHaveBeenCalled();
+describe('WasmRenderer builds no proxy of the orientation widget', () => {
+    // Building a proxy of the orientation widget serializes its graph, and the
+    // client-only ids that allocates collide with the next add_dataset's on
+    // VTK 9.6.1. The gesture mark is taken at the DOM level instead.
+    test('the orientation widget id is never requested through getVtkObject', async () => {
+        const { scene } = await makeRenderer(makeSender());
 
-        await endCalls[0][1]();
+        const requestedIds = scene.getVtkObject.mock.calls.map((call) => call[0]);
 
-        expect(scene.noteWidgetGesture).toHaveBeenCalledTimes(1);
-        // The mark carries no payload and triggers no send: the report stays
-        // the settle's, through the unchanged sync_camera path.
-        expect(sender).not.toHaveBeenCalled();
+        expect(requestedIds).not.toContain(ORIENTATION_WIDGET_ID);
     });
 });

src/ansys/visor/visor-client/src/renderer/WasmRenderer.ts

@@ -96,21 +96,11 @@ export class WasmRenderer implements IRenderer {
             });
         });
 
-        /**
-         * Attribute an orientation-widget camera move to the user.
-         *
-         * Clicking a face of the cube moves the camera entirely inside wasm,
-         * with no DOM input `CameraGestureTracker` can see, so without this
-         * mark the move settles as `programmatic` and the server drops it.
-         * `EndInteractionEvent` (not `InteractionEvent`) fires once per
-         * interaction rather than per animation frame. The callback only
-         * marks the gesture; the report itself is still the settle's,
-         * unchanged, via `sync_camera`.
-         */
-        const orientationWidget = vtkScene.getVtkObject(annotation.widgets.orientationWidgetId);
-        orientationWidget.observe('EndInteractionEvent', () => {
-            vtkScene.noteWidgetGesture();
-        });
+        // No proxy of the orientation widget: getVtkObject on it serializes the
+        // widget's graph, and the client-only ids that allocates collide with the
+        // next add_dataset's objects. Fixed after VTK 9.6.1 by
+        // SetAllocateIdsDescending; on that upgrade the gesture mark can move back
+        // onto the widget's EndInteractionEvent, from CameraGestureTracker's.
 
         // Bounding-box ids are stashed for attachSceneGraph, which is the
         // point at which the live sceneGraph (needed by BoundingBoxWidget)

src/ansys/visor/visor-client/src/wasm/CameraGestureTracker.js

@@ -27,9 +27,9 @@
  *   pending marks that report `gesture` as well.
  *
  * - The orientation widget is not a DOM input at all: its face click reaches
- *   the camera inside wasm. It is marked explicitly, through
- *   `noteWidgetGesture`, from the observer `WasmRenderer` registers on the
- *   widget's `EndInteractionEvent`.
+ *   the camera inside wasm, and only after the release. A real, bubbling
+ *   release on the canvas itself therefore arms the window too, which covers
+ *   that face click without the tracker knowing the widget exists.
  */
 
 /**
@@ -78,6 +78,15 @@ export default class CameraGestureTracker {
         };
         const onMouseUp = /**@param {MouseEvent} e*/ (e) => {
             this.#heldButtons.delete(e.button);
+            // A release on the canvas arms the window, so a move that only
+            // reaches the camera afterwards -- an orientation-widget face
+            // click -- still reports as a gesture. Both checks are needed:
+            // UI over the canvas is not the canvas, and `applyMouseEvent`
+            // fires non-bubbling `mouseup`s at the canvas on every press,
+            // release and mouseout, which must not arm anything.
+            if (e.target === canvas && e.bubbles) {
+                this.#markImpulse();
+            }
         };
         // A `mouseout` is the existing sticky-mousedown release, and a window
         // `blur` means the page no longer owns the input. Both clear *every*
@@ -195,21 +204,6 @@ export default class CameraGestureTracker {
         this.#settleTimer = setTimeout(this.#reportSettled, CAMERA_SETTLE_MS);
     };
 
-    /**
-     * The orientation widget's end-of-interaction mark, called via
-     * `VtkScene.noteWidgetGesture` from the `EndInteractionEvent` observer in
-     * `WasmRenderer`. A face click involves no button, wheel or z/r key, so
-     * without this mark the move settles as `programmatic`.
-     *
-     * Delegates to `#markImpulse` to reuse the same window and retroactive
-     * stickiness as a wheel notch or z/r press, so a mark landing before or
-     * after the camera events it belongs to is still caught.
-     *
-     * @return {void}
-     */
-    noteWidgetGesture = () => {
-        this.#markImpulse();
-    };
 
     /**
      * @return {void}

src/ansys/visor/visor-client/src/wasm/VtkScene.js

@@ -159,17 +159,6 @@ export default class VtkScene {
     addCameraSettledListener = (handler) => {
         return this.#cameraGestureTracker.addSettledListener(handler);
     };
-    /**
-     * Mark the settle window now open as a user gesture, on behalf of a wasm
-     * widget (the orientation widget) whose interaction is not a DOM input
-     * this scene can see. `WasmRenderer` calls this from its
-     * `EndInteractionEvent` observer, since `#cameraGestureTracker` is private.
-     *
-     * @return {void}
-     */
-    noteWidgetGesture = () => {
-        this.#cameraGestureTracker?.noteWidgetGesture();
-    };
     /**
      * @param {(actorId:number,ctrlKey:boolean,shiftKey:boolean,normX:number,normY:number)=>void} handler
      * @return {()=>void}