diff --git a/doc/changelog.d/143.added.md b/doc/changelog.d/143.added.md new file mode 100644 index 00000000..b7247c34 --- /dev/null +++ b/doc/changelog.d/143.added.md @@ -0,0 +1 @@ +[Remote rendering 3.3d] sync camera to server on orientation widget diff --git a/src/ansys/visor/visor-client/src/VisorFrontend.tsx b/src/ansys/visor/visor-client/src/VisorFrontend.tsx index aa59dcc3..d78a6a33 100644 --- a/src/ansys/visor/visor-client/src/VisorFrontend.tsx +++ b/src/ansys/visor/visor-client/src/VisorFrontend.tsx @@ -52,7 +52,10 @@ export class VisorFrontend { } = getPromiseResolver(); 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); diff --git a/src/ansys/visor/visor-client/src/jest-tests/CameraGestureTracker.test.js b/src/ansys/visor/visor-client/src/jest-tests/CameraGestureTracker.test.js index 4895803e..3c638d11 100644 --- a/src/ansys/visor/visor-client/src/jest-tests/CameraGestureTracker.test.js +++ b/src/ansys/visor/visor-client/src/jest-tests/CameraGestureTracker.test.js @@ -211,6 +211,54 @@ describe('CameraGestureTracker', () => { expect(onSettled).toHaveBeenCalledWith('programmatic'); }); + // ---- a release on the wasm canvas --------------------------------------- + + // 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(); + + 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', () => { diff --git a/src/ansys/visor/visor-client/src/jest-tests/WasmRendererWidgetTriggers.test.tsx b/src/ansys/visor/visor-client/src/jest-tests/WasmRendererWidgetTriggers.test.tsx index 0ce6a698..7ac675ef 100644 --- a/src/ansys/visor/visor-client/src/jest-tests/WasmRendererWidgetTriggers.test.tsx +++ b/src/ansys/visor/visor-client/src/jest-tests/WasmRendererWidgetTriggers.test.tsx @@ -138,7 +138,9 @@ async function makeRenderer( render: jest.fn(), clearObserversAndEventListeners: 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; @@ -147,7 +149,7 @@ async function makeRenderer( default: return objects.widget; } - }, + }), }; const renderer = await WasmRenderer.createAsync( scene as unknown as VtkScene, @@ -156,7 +158,7 @@ async function makeRenderer( ); const sceneGraph = makeSceneGraphDouble(actorIds); renderer.attachSceneGraph(sceneGraph as unknown as VisorSceneNodeExtended); - return { renderer, camera, sceneGraph, ...objects }; + return { renderer, camera, scene, sceneGraph, ...objects }; } /** A sender that records its calls and resolves. */ @@ -336,3 +338,16 @@ describe('WasmRenderer reports the cross-section plane on the end-of-drag event' expect(sender).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()); + + const requestedIds = scene.getVtkObject.mock.calls.map((call) => call[0]); + + expect(requestedIds).not.toContain(ORIENTATION_WIDGET_ID); + }); +}); diff --git a/src/ansys/visor/visor-client/src/renderer/WasmRenderer.ts b/src/ansys/visor/visor-client/src/renderer/WasmRenderer.ts index d61fff6d..6b901986 100644 --- a/src/ansys/visor/visor-client/src/renderer/WasmRenderer.ts +++ b/src/ansys/visor/visor-client/src/renderer/WasmRenderer.ts @@ -96,6 +96,12 @@ export class WasmRenderer implements IRenderer { }); }); + // 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) // becomes available. diff --git a/src/ansys/visor/visor-client/src/wasm/CameraGestureTracker.js b/src/ansys/visor/visor-client/src/wasm/CameraGestureTracker.js index 694c2147..1307b941 100644 --- a/src/ansys/visor/visor-client/src/wasm/CameraGestureTracker.js +++ b/src/ansys/visor/visor-client/src/wasm/CameraGestureTracker.js @@ -25,6 +25,11 @@ * the camera event can arrive before the tracker has noticed the input. * To cover that, a wheel event or a z/r keydown arriving while a report is * 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, 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. */ /** @@ -73,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* @@ -190,6 +204,7 @@ export default class CameraGestureTracker { this.#settleTimer = setTimeout(this.#reportSettled, CAMERA_SETTLE_MS); }; + /** * @return {void} */