From 31d28598ae0c28398b7c12e32fe5f9f1b18e6173 Mon Sep 17 00:00:00 2001 From: Laura Kasian Date: Tue, 27 Oct 2026 10:14:04 -0700 Subject: [PATCH 1/5] feat: bump scene_version once per scene-changing server operation --- .../app/trame/server_state_publisher.py | 16 ++++ src/ansys/visor/viewer/app/visor_vtk.py | 37 +++++++- src/ansys/visor/viewer/app/visor_vtk_local.py | 3 + .../visor/viewer/app/visor_vtk_server.py | 12 ++- tests/unit/app/test_server_state_publisher.py | 34 ++++++- tests/unit/app/test_visor_vtk_local.py | 92 ++++++++++++++++++- tests/unit/app/test_visor_vtk_server.py | 21 +++-- 7 files changed, 200 insertions(+), 15 deletions(-) diff --git a/src/ansys/visor/viewer/app/trame/server_state_publisher.py b/src/ansys/visor/viewer/app/trame/server_state_publisher.py index e1c2e00b..b1a750c1 100644 --- a/src/ansys/visor/viewer/app/trame/server_state_publisher.py +++ b/src/ansys/visor/viewer/app/trame/server_state_publisher.py @@ -20,6 +20,9 @@ SERVER_CAMERA_STATE_KEY: str = "server_camera_state" """State key carrying the server's camera record, as its camelCase fields, or None.""" +SCENE_VERSION_KEY: str = "scene_version" +"""State key counting scene changes; absent until the first change, then 1, 2, ...""" + class ServerStatePublisher(CameraPublisher): """Writes server-originated state keys from any thread onto the trame event loop.""" @@ -41,6 +44,19 @@ def publish_camera(self, camera_state: VisorCameraState | None) -> None: camera_record = None if camera_state is None else camera_state.model_dump(by_alias=True) self._async_runner.call_soon(lambda: self._write(SERVER_CAMERA_STATE_KEY, camera_record)) + def bump_scene_version(self) -> None: + """Increment scene_version on the loop thread: read, add one, write and flush in one callback; callable from any thread. + + The read happens inside the callback, so bumps queued together from + any threads are serialised on the loop and each one counts. + """ + self._async_runner.call_soon(self._write_next_scene_version) + + def _write_next_scene_version(self) -> None: + """Read scene_version (absent counts as 0), write it plus one, and flush; loop thread only.""" + current = self._state[SCENE_VERSION_KEY] + self._write(SCENE_VERSION_KEY, (current or 0) + 1) + def _write(self, key: str, value: object) -> None: """Set one state key and flush it; loop thread only.""" self._state[key] = value diff --git a/src/ansys/visor/viewer/app/visor_vtk.py b/src/ansys/visor/viewer/app/visor_vtk.py index dbee29f0..303f4d38 100644 --- a/src/ansys/visor/viewer/app/visor_vtk.py +++ b/src/ansys/visor/viewer/app/visor_vtk.py @@ -81,6 +81,22 @@ def wrapper(*args, **kwargs): return decorator +def notifies_scene_change(func): + """Call self._post_scene_change_hook() once after func, whether func returns or raises. + + A failed operation can still have changed the scene, so it notifies too. + Placed directly above the method, so the argument and server checks above + it reject a call before anything is notified. + """ + @wraps(func) + def wrapper(self, *args, **kwargs): + try: + return func(self, *args, **kwargs) + finally: + self._post_scene_change_hook() + return wrapper + + class VisorVTK(Visor): """ VisorVTK is a class that provides an interface for initializing and managing a @@ -186,6 +202,11 @@ def _initialize_rendering(self, standalone: bool, trame_log_dir: str | None) -> Called from ``__init__`` after ``_server_manager`` and ``_file_io`` are in place. """ + @abstractmethod + def _post_scene_change_hook(self) -> None: + """Signal one finished scene-changing operation to connected clients.""" + raise NotImplementedError + """ Properties """ @property def running(self) -> bool: @@ -239,6 +260,7 @@ def stop(self): @require_server_on @require_input_is_not_none @validate_input_metadata_types + @notifies_scene_change def update(self, input: str | VisorDatasetType, metadata: Metadata | None = None @@ -266,6 +288,7 @@ def update(self, @handle_errors("add_dataset") @require_input_is_not_none @validate_input_metadata_types + @notifies_scene_change def add_dataset(self, input: str | VisorDatasetType, metadata: Metadata | None = None @@ -288,6 +311,7 @@ def add_dataset(self, return dataset_id @handle_errors("remove_dataset") + @notifies_scene_change def remove_dataset(self, dataset_id: int) -> None: """ Remove a dataset from the current scene based on its dataset ID. @@ -295,7 +319,7 @@ def remove_dataset(self, dataset_id: int) -> None: self._scene.remove_dataset(dataset_id) if self.dataset_count == 0: # if we've removed the last dataset, reset widgets and camera so the blank scene is normalized - self.clear() + self._clear_scene() else: self._scene.update_widgets() self._scene.render() @@ -311,14 +335,14 @@ def list_datasets(self) -> dict: return self._scene.list_all_dataset_info() @handle_errors("clear") + @notifies_scene_change def clear(self): """ Clear the current scene and reset the visualization pipeline. This function removes all actors from the scene and resets the state. """ logger.info("Clearing the scene") - self._scene.clear() - self._scene.finalize_scene() + self._clear_scene() logger.info("Scene cleared successfully") @handle_errors("list_variables") @@ -335,6 +359,7 @@ def list_variables(self, dataset_id: int) -> List[VisorPartVariables]: return self._scene.list_variables_for_dataset(dataset_id) @handle_errors("update_variables") + @notifies_scene_change def update_variables(self, dataset_id: int, variables: List[Dict[str, Any]]) -> None: """ Update the variables of a specific dataset in the scene. @@ -397,6 +422,7 @@ async def save_state(self, state_dir: str) -> None: self._file_io.delete_stale_snapshots(state_dir, state) @handle_errors("load state") + @notifies_scene_change def load_state(self, state_dir: str) -> None: """ Load viewer state from a directory. @@ -485,6 +511,11 @@ def _health(self) -> bool: return self._server_manager.health # Private Methods + def _clear_scene(self) -> None: + """Clear the scene and finalise it; shared by clear and remove_dataset.""" + self._scene.clear() + self._scene.finalize_scene() + def _initialize_server(self, trame_log_dir: str | None) -> TrameServerManager: """Initialize the Trame server manager.""" # Start Trame listening on the configured binding host (or the instance host if unset) diff --git a/src/ansys/visor/viewer/app/visor_vtk_local.py b/src/ansys/visor/viewer/app/visor_vtk_local.py index 465470cd..ea982ad9 100644 --- a/src/ansys/visor/viewer/app/visor_vtk_local.py +++ b/src/ansys/visor/viewer/app/visor_vtk_local.py @@ -57,3 +57,6 @@ def _initialize_rendering(self, standalone: bool, trame_log_dir: str | None) -> scene_mutation_api=self._scene, ) + def _post_scene_change_hook(self) -> None: + """Client mode signals through LocalView.update and set_state; nothing to bump.""" + diff --git a/src/ansys/visor/viewer/app/visor_vtk_server.py b/src/ansys/visor/viewer/app/visor_vtk_server.py index bdd93e5d..5ab43b3a 100644 --- a/src/ansys/visor/viewer/app/visor_vtk_server.py +++ b/src/ansys/visor/viewer/app/visor_vtk_server.py @@ -25,19 +25,21 @@ class VisorVTKServer(VisorVTK): """Visor interface for VTK SERVER mode: server-side off-screen rendering streamed through trame-rca.""" + _state_publisher: ServerStatePublisher + def _initialize_rendering(self, standalone: bool, trame_log_dir: str | None) -> None: """Wire the bridge, the state publisher, the server scene and ServerApp. One bridge serves both: the scene's renderer requests frames through it, and the app binds it to the view handler when the server is ready. The publisher writes the renderer's camera record into this viewer's - server state. + server state, and the scene_version bump after each scene change. """ bridge = ServerLoopBridge(self._server_manager.async_runner) - publisher = ServerStatePublisher(self._server_manager.async_runner, self.server.state) + self._state_publisher = ServerStatePublisher(self._server_manager.async_runner, self.server.state) self._scene = VisorServerScene( - self.server, dark_mode=self._dark_mode, bridge=bridge, camera_publisher=publisher + self.server, dark_mode=self._dark_mode, bridge=bridge, camera_publisher=self._state_publisher ) self._trame_app = ServerApp( @@ -52,3 +54,7 @@ def _initialize_rendering(self, standalone: bool, trame_log_dir: str | None) -> bridge=bridge, ) + def _post_scene_change_hook(self) -> None: + """Bump scene_version so connected clients rebuild; requests no frame, the operation already did.""" + self._state_publisher.bump_scene_version() + diff --git a/tests/unit/app/test_server_state_publisher.py b/tests/unit/app/test_server_state_publisher.py index f1d25476..274f51e9 100644 --- a/tests/unit/app/test_server_state_publisher.py +++ b/tests/unit/app/test_server_state_publisher.py @@ -42,11 +42,12 @@ def _camera() -> VisorCameraState: class _RecordingState: - """Records each key write and each flush, with the thread it ran on.""" + """Records each key write and each flush, with the thread it ran on; a read returns the last value set, or None.""" def __init__(self) -> None: self.events: list[tuple] = [] self.thread_ids: list[int] = [] + self._values: dict = {} self._changed = threading.Condition() def _record(self, event: tuple) -> None: @@ -55,7 +56,11 @@ def _record(self, event: tuple) -> None: self.thread_ids.append(threading.get_ident()) self._changed.notify_all() + def __getitem__(self, key): + return self._values.get(key) + def __setitem__(self, key, value) -> None: + self._values[key] = value self._record(("set", key, value)) def flush(self) -> None: @@ -113,3 +118,30 @@ def test_publish_camera_none_writes_none(runner, state): runner.callbacks[0]() assert state.events == [("set", "server_camera_state", None), ("flush",)] + +def test_bumps_queued_together_count_up(runner, state): + """Both bumps are queued before either runs, so each must read the value on the loop.""" + publisher = ServerStatePublisher(runner, state) + publisher.bump_scene_version() + publisher.bump_scene_version() + for callback in runner.callbacks: + callback() + assert state.events == [ + ("set", "scene_version", 1), + ("flush",), + ("set", "scene_version", 2), + ("flush",), + ] + + +def test_bump_writes_on_loop_thread(state): + async_runner = AsyncRunner() + try: + ServerStatePublisher(async_runner, state).bump_scene_version() + assert state.wait_for(2) + loop_thread_id = async_runner.thread.ident + assert state.events == [("set", "scene_version", 1), ("flush",)] + assert state.thread_ids == [loop_thread_id, loop_thread_id] + finally: + async_runner.dispose() + diff --git a/tests/unit/app/test_visor_vtk_local.py b/tests/unit/app/test_visor_vtk_local.py index 5ce7c719..e73e8706 100644 --- a/tests/unit/app/test_visor_vtk_local.py +++ b/tests/unit/app/test_visor_vtk_local.py @@ -125,7 +125,7 @@ def test_remove_dataset_clears_if_no_datasets(iface): type(iface._scene).dataset_count = PropertyMock(return_value=1) iface._scene.remove_dataset = MagicMock() - iface.clear = MagicMock() + iface._clear_scene = MagicMock() iface._scene.update_widgets = MagicMock() iface._scene.render = MagicMock() @@ -140,7 +140,7 @@ def side_effect_remove(dataset_id): # Assertions iface._scene.remove_dataset.assert_called_once_with(1) - iface.clear.assert_called_once() + iface._clear_scene.assert_called_once() def test_remove_dataset_updates_widgets_and_renders(iface): """Verify that removing a dataset updates widgets and triggers a render.""" @@ -540,6 +540,94 @@ def test_load_state_passes_correct_metadata_to_add_dataset(tmp_path, iface): iface._scene.add_dataset.assert_called_once_with(mock_data, built_meta) +# ================================================================== # +# Scene-change notification +# ================================================================== # + +def _run_update(iface): + iface._prepare_and_render_scene = MagicMock(return_value=1) + iface.update("input", "metadata") + + +def _run_add_dataset(iface): + iface._prepare_and_render_scene = MagicMock(return_value=1) + iface.add_dataset("input", "metadata") + + +def _run_remove_dataset_with_datasets_left(iface): + type(iface._scene).dataset_count = PropertyMock(return_value=1) + iface.remove_dataset(1) + + +def _run_remove_last_dataset(iface): + type(iface._scene).dataset_count = PropertyMock(return_value=0) + iface.remove_dataset(1) + + +def _run_clear(iface): + iface.clear() + + +def _run_update_variables(iface): + iface.update_variables(1, []) + + +def _run_load_state(iface): + iface._file_io.read_state = MagicMock(return_value=MagicMock()) + type(iface._scene).dataset_count = PropertyMock(return_value=1) + iface.load_state("state_dir") + + +def _run_start(iface): + iface._server_manager.running = False + iface._prepare_and_render_scene = MagicMock() + iface._server_manager.start = MagicMock() + iface.start("input", "metadata") + + +def _run_list_datasets(iface): + iface.list_datasets() + + +def _run_list_variables(iface): + iface.list_variables(1) + + +@pytest.mark.parametrize( + "run,expected_calls", + [ + pytest.param(_run_update, 1, id="update"), + pytest.param(_run_add_dataset, 1, id="add_dataset"), + pytest.param(_run_remove_dataset_with_datasets_left, 1, id="remove_dataset-left"), + pytest.param(_run_remove_last_dataset, 1, id="remove_dataset-last"), + pytest.param(_run_clear, 1, id="clear"), + pytest.param(_run_update_variables, 1, id="update_variables"), + pytest.param(_run_load_state, 1, id="load_state"), + pytest.param(_run_start, 0, id="start"), + pytest.param(_run_list_datasets, 0, id="list_datasets"), + pytest.param(_run_list_variables, 0, id="list_variables"), + ], +) +def test_each_scene_operation_notifies_once(iface, run, expected_calls): + """Each scene-changing operation notifies exactly once; start and the reads never do.""" + iface._post_scene_change_hook = MagicMock() + + run(iface) + + assert iface._post_scene_change_hook.call_count == expected_calls + + +def test_failed_scene_operation_still_notifies(iface): + """A scene operation that raises still notifies once, since the scene may have changed.""" + iface._post_scene_change_hook = MagicMock() + iface._scene.clear.side_effect = ValueError + + with pytest.raises(RuntimeError): + iface.clear() + + assert iface._post_scene_change_hook.call_count == 1 + + # ================================================================== # # LocalApp injection boundary # ================================================================== # diff --git a/tests/unit/app/test_visor_vtk_server.py b/tests/unit/app/test_visor_vtk_server.py index 4f8e366e..a8fb7a09 100644 --- a/tests/unit/app/test_visor_vtk_server.py +++ b/tests/unit/app/test_visor_vtk_server.py @@ -16,7 +16,7 @@ @pytest.fixture def wiring(): - """Build a VisorVTKServer against doubles; yield (app_cls, scene_cls, bridge_cls, mgr, publishers).""" + """Build a VisorVTKServer against doubles; yield (app_cls, scene_cls, bridge_cls, mgr, publishers, viewer).""" with patch("ansys.visor.viewer.app.visor_vtk.TrameServerManager") as mgr_cls, \ patch("ansys.visor.viewer.app.visor_vtk_server.ServerApp") as app_cls, \ patch("ansys.visor.viewer.app.visor_vtk_server.VisorServerScene") as scene_cls, \ @@ -35,20 +35,20 @@ def build_publisher(*args): return publishers[-1][1] publisher_cls.side_effect = build_publisher - VisorVTKServer() - yield app_cls, scene_cls, bridge_cls, mgr, publishers + viewer = VisorVTKServer() + yield app_cls, scene_cls, bridge_cls, mgr, publishers, viewer def test_scene_and_app_share_one_bridge(wiring): """The scene's renderer requests frames through the bridge the app binds.""" - app_cls, scene_cls, _bridge_cls, _mgr, _publishers = wiring + app_cls, scene_cls, _bridge_cls, _mgr, _publishers, _viewer = wiring assert scene_cls.call_args.kwargs["bridge"] is app_cls.call_args.kwargs["bridge"] def test_app_receives_the_scene_members(wiring): """The app is given the scene itself, its RCA window, and its pick and theme methods.""" - app_cls, scene_cls, _bridge_cls, _mgr, _publishers = wiring + app_cls, scene_cls, _bridge_cls, _mgr, _publishers, _viewer = wiring scene = scene_cls.return_value kwargs = app_cls.call_args.kwargs @@ -60,9 +60,18 @@ def test_app_receives_the_scene_members(wiring): def test_scene_publishes_camera_into_the_viewer_state(wiring): """The scene gets the one publisher, built over this viewer's runner and server state.""" - _app_cls, scene_cls, _bridge_cls, mgr, publishers = wiring + _app_cls, scene_cls, _bridge_cls, mgr, publishers, _viewer = wiring assert [args for args, _ in publishers] == [(mgr.async_runner, mgr.server.state)] assert scene_cls.call_args.kwargs["camera_publisher"] is publishers[0][1] +def test_scene_change_bumps_the_one_publisher(wiring): + """The scene-change hook bumps scene_version through the publisher the scene was given.""" + _app_cls, _scene_cls, _bridge_cls, _mgr, publishers, viewer = wiring + + viewer._post_scene_change_hook() + + publishers[0][1].bump_scene_version.assert_called_once_with() + + From 354f83aee12e202637ca1c9e6ada4587761d6a53 Mon Sep 17 00:00:00 2001 From: Laura Kasian Date: Tue, 27 Oct 2026 11:31:12 -0700 Subject: [PATCH 2/5] feat: add a frontend rebuilder and ServerRenderer releases frame listeners on dispose --- .../src/frontend/FrontendRebuilder.ts | 39 ++++++++++++ .../src/jest-tests/FrontendRebuilder.test.ts | 59 +++++++++++++++++++ .../jest-tests/ServerRendererDispose.test.ts | 46 +++++++++++++++ .../src/renderer/ServerRenderer.ts | 14 ++++- 4 files changed, 157 insertions(+), 1 deletion(-) create mode 100644 src/ansys/visor/visor-client/src/frontend/FrontendRebuilder.ts create mode 100644 src/ansys/visor/visor-client/src/jest-tests/FrontendRebuilder.test.ts create mode 100644 src/ansys/visor/visor-client/src/jest-tests/ServerRendererDispose.test.ts diff --git a/src/ansys/visor/visor-client/src/frontend/FrontendRebuilder.ts b/src/ansys/visor/visor-client/src/frontend/FrontendRebuilder.ts new file mode 100644 index 00000000..e6773f32 --- /dev/null +++ b/src/ansys/visor/visor-client/src/frontend/FrontendRebuilder.ts @@ -0,0 +1,39 @@ +/** Rebuilds the frontend on request, one build at a time; requests during a build collapse into one more build. */ +export class FrontendRebuilder { + /** Rebuild with build; a rejected build goes to onError. Both required. */ + constructor(build: () => Promise, onError: (err: unknown) => void) { + this.#build = build; + this.#onError = onError; + } + + readonly #build: () => Promise; + readonly #onError: (err: unknown) => void; + #building = false; + #requested = false; + + /** Build now if idle, otherwise build once more after the current build. */ + request(): void { + if (this.#building) { + this.#requested = true; + return; + } + void this.#buildAsync(); + } + + /** Build until no request arrived during the last build; the building flag is cleared in finally. */ + async #buildAsync(): Promise { + this.#building = true; + try { + do { + this.#requested = false; + try { + await this.#build(); + } catch (err) { + this.#onError(err); + } + } while (this.#requested); + } finally { + this.#building = false; + } + } +} diff --git a/src/ansys/visor/visor-client/src/jest-tests/FrontendRebuilder.test.ts b/src/ansys/visor/visor-client/src/jest-tests/FrontendRebuilder.test.ts new file mode 100644 index 00000000..a328341a --- /dev/null +++ b/src/ansys/visor/visor-client/src/jest-tests/FrontendRebuilder.test.ts @@ -0,0 +1,59 @@ +import { FrontendRebuilder } from '../frontend/FrontendRebuilder'; + +/** + * The rebuilder runs one build at a time: requests made during a build + * collapse into one more build, and a rejected build neither stops nor + * wedges it. + * + * Expected values are hand-written literals. + */ + +/** Let every pending promise callback run. */ +async function settle(): Promise { + await new Promise((resolve) => setTimeout(resolve, 0)); +} + +describe('FrontendRebuilder', () => { + test('requests during a held build collapse into one more build', async () => { + let releaseFirst: () => void = () => {}; + const firstBuild = new Promise((resolve) => { + releaseFirst = resolve; + }); + let builds = 0; + const rebuilder = new FrontendRebuilder(async () => { + builds += 1; + if (builds === 1) { + await firstBuild; + } + }, jest.fn()); + + rebuilder.request(); + rebuilder.request(); + rebuilder.request(); + rebuilder.request(); + releaseFirst(); + await settle(); + + expect(builds).toBe(2); + }); + + test('a rejected build is reported and the next request still builds', async () => { + const failure = new Error('build failed'); + let builds = 0; + const onError = jest.fn(); + const rebuilder = new FrontendRebuilder(async () => { + builds += 1; + if (builds === 1) { + throw failure; + } + }, onError); + + rebuilder.request(); + await settle(); + rebuilder.request(); + await settle(); + + expect(onError.mock.calls).toStrictEqual([[failure]]); + expect(builds).toBe(2); + }); +}); diff --git a/src/ansys/visor/visor-client/src/jest-tests/ServerRendererDispose.test.ts b/src/ansys/visor/visor-client/src/jest-tests/ServerRendererDispose.test.ts new file mode 100644 index 00000000..58d7ad11 --- /dev/null +++ b/src/ansys/visor/visor-client/src/jest-tests/ServerRendererDispose.test.ts @@ -0,0 +1,46 @@ +import { ServerRenderer, type ServerStateSource } from '../renderer/ServerRenderer'; +import type { IServerCanvas } from '../rca/ServerCanvas'; + +/** + * A disposed server renderer leaves no frame listener on the canvas, which + * outlives it: the canvas is shared by every renderer built over it. + * + * Expected values are hand-written literals. + */ + +const emptyState: ServerStateSource = { + watch: () => () => {}, +}; + +/** A canvas whose frame listener removers count their calls, in the order the listeners were added. */ +function recordingCanvas(): { canvas: IServerCanvas; removerCalls: number[] } { + const removerCalls: number[] = []; + const canvas: IServerCanvas = { + element: document.createElement('div'), + frameSize: null, + addFrameListener: () => { + const index = removerCalls.length; + removerCalls.push(0); + return () => { + removerCalls[index] += 1; + }; + }, + addClickListener: () => () => {}, + addResizeListener: () => () => {}, + }; + return { canvas, removerCalls }; +} + +describe('ServerRenderer dispose', () => { + test('dispose removes the frame listeners the renderer added', () => { + const { canvas, removerCalls } = recordingCanvas(); + const renderer = new ServerRenderer(canvas, emptyState, jest.fn()); + const removeFirst = renderer.addFrameRenderedListener(() => {}); + renderer.addFrameRenderedListener(() => {}); + + removeFirst(); + renderer.dispose(); + + expect(removerCalls).toStrictEqual([1, 1]); + }); +}); diff --git a/src/ansys/visor/visor-client/src/renderer/ServerRenderer.ts b/src/ansys/visor/visor-client/src/renderer/ServerRenderer.ts index 7a4f0449..97a303a5 100644 --- a/src/ansys/visor/visor-client/src/renderer/ServerRenderer.ts +++ b/src/ansys/visor/visor-client/src/renderer/ServerRenderer.ts @@ -166,6 +166,7 @@ export class ServerRenderer implements IRenderer { readonly #camera = new CameraRecordCache(); readonly #cameraChangedListeners = new Map<() => void, (state: VisorCameraState) => void>(); readonly #viewerClicks = new ViewerClickDispatcher(); + readonly #frameListenerRemovers = new Set<() => void>(); #crossSectionVisible = false; #boundingBoxVisible = false; #edgesVisible = false; @@ -286,6 +287,9 @@ export class ServerRenderer implements IRenderer { this.#removeResizeListener(); this.#cameraChangedListeners.clear(); this.#viewerClicks.clear(); + for (const remover of [...this.#frameListenerRemovers]) { + remover(); + } } // ---- Camera --------------------------------------------------------------- @@ -337,8 +341,16 @@ export class ServerRenderer implements IRenderer { return remover; } + /** Add a listener on the canvas's frames; returns its remover, which dispose also calls if it has not run. */ addFrameRenderedListener(callback: (fps: number) => void): () => void { - return this.#canvas.addFrameListener(callback); + const removeFromCanvas = this.#canvas.addFrameListener(callback); + const remover = () => { + if (this.#frameListenerRemovers.delete(remover)) { + removeFromCanvas(); + } + }; + this.#frameListenerRemovers.add(remover); + return remover; } /** Add a listener called once per click with the part pick_part_at found under it, or null; returns its remover. */ From d0659bac9dc7614bba00129aa00e21f17a775587 Mon Sep 17 00:00:00 2001 From: Laura Kasian Date: Tue, 27 Oct 2026 12:32:37 -0700 Subject: [PATCH 3/5] feat: wire scene_version rebuilds, theme resend, and mismatch handling into useServerVtk --- .../visor-client/src/hooks/useServerVtk.ts | 63 +++-- .../src/jest-tests/AppModeBranch.test.tsx | 7 + .../src/jest-tests/UseServerVtkBuild.test.tsx | 8 +- .../jest-tests/UseServerVtkRebuild.test.tsx | 232 ++++++++++++++++++ .../src/jest-tests/UseServerVtkTheme.test.tsx | 7 + 5 files changed, 302 insertions(+), 15 deletions(-) create mode 100644 src/ansys/visor/visor-client/src/jest-tests/UseServerVtkRebuild.test.tsx diff --git a/src/ansys/visor/visor-client/src/hooks/useServerVtk.ts b/src/ansys/visor/visor-client/src/hooks/useServerVtk.ts index dccfe836..80f0679a 100644 --- a/src/ansys/visor/visor-client/src/hooks/useServerVtk.ts +++ b/src/ansys/visor/visor-client/src/hooks/useServerVtk.ts @@ -6,15 +6,36 @@ import { RCA_AREA_NAME, ServerCanvas, type RcaSession } from '../rca/ServerCanva import { ServerRenderer, type ServerStateSource } from '../renderer/ServerRenderer'; import type { TrameTriggerSender } from '../renderer/IRenderer'; import { buildFrontendAsync } from '../buildFrontendAsync'; +import { FrontendRebuilder } from '../frontend/FrontendRebuilder'; import type { VisorArgs } from './useVisorArgs'; +/** State key the server bumps after each scene change; must equal server_state_publisher.py's. */ +export const SCENE_VERSION_KEY = 'scene_version'; + /** What the server-mode hook gives App. */ export type ServerVtk = Readonly<{ visorFrontend: VisorFrontend | null; schemaMismatchError: SchemaVersionMismatchError | null; }>; -/** Over the probe's session: send the page theme, stream frames and build the frontend; idle when trame is null. */ +/** A built server-mode frontend and the renderer it was built over, released together when replaced. */ +class ServerFrontendBuild { + constructor(frontend: VisorFrontend, renderer: ServerRenderer) { + this.#frontend = frontend; + this.#renderer = renderer; + } + + readonly #frontend: VisorFrontend; + readonly #renderer: ServerRenderer; + + /** Remove the frontend's renderer listeners, then dispose the renderer. */ + release(): void { + this.#frontend.release(); + this.#renderer.dispose(); + } +} + +/** Over the probe's session: stream frames, and rebuild the frontend on every scene_version change; idle when trame is null. */ export function useServerVtk(trame: Trame | null, visorArgs: VisorArgs): ServerVtk { const [visorFrontend, setVisorFrontend] = useState(null); const [schemaMismatchError, setSchemaMismatchError] = @@ -25,38 +46,42 @@ export function useServerVtk(trame: Trame | null, visorArgs: VisorArgs): ServerV return; } // Under StrictMode the effect runs twice in development: each run - // disposes only the canvas and renderer it created, and a cancelled - // run drops its frontend. + // disposes only the canvas and builds it created, and a build that + // finishes after cleanup disposes its own renderer. let cancelled = false; - let renderer: ServerRenderer | null = null; - const disposeRenderer = () => { - renderer?.dispose(); - renderer = null; - }; + let current: ServerFrontendBuild | null = null; + const state = stateOf(trame); // Subscribed before anything can make the server render, so the frame // that render produces reaches the canvas. const canvas = new ServerCanvas(sessionOf(trame), RCA_AREA_NAME); const sender: TrameTriggerSender = (triggerName, payload) => trame.trigger(triggerName, [payload]); - (async () => { + const buildAsync = async (): Promise => { + // A build requested before cleanup but started after it sends nothing. + if (cancelled) { + return; + } if (visorArgs.darkMode != null) { await sendDarkThemeAsync(trame, visorArgs.darkMode); } // No arguments: the trigger takes none, and a sender would pass one. const sceneDetailsJson = await trame.trigger('get_visor_scene_details_json'); + const built: { renderer: ServerRenderer | null } = { renderer: null }; let newFrontend: VisorFrontend; try { newFrontend = await buildFrontendAsync( sceneDetailsJson, async () => { - renderer = new ServerRenderer(canvas, stateOf(trame), sender); - return renderer; + built.renderer = new ServerRenderer(canvas, state, sender); + return built.renderer; }, sender, visorArgs.darkMode ); } catch (err) { + built.renderer?.dispose(); if (err instanceof SchemaVersionMismatchError) { + // The current frontend is kept, as in client mode. if (!cancelled) { setSchemaMismatchError(err); } @@ -64,17 +89,27 @@ export function useServerVtk(trame: Trame | null, visorArgs: VisorArgs): ServerV } throw err; } + const renderer = built.renderer!; if (cancelled) { // Built after cleanup ran, so cleanup could not dispose it. - disposeRenderer(); + renderer.dispose(); return; } + current?.release(); + current = new ServerFrontendBuild(newFrontend, renderer); setSchemaMismatchError(null); setVisorFrontend(newFrontend); - })(); + }; + const rebuilder = new FrontendRebuilder(buildAsync, (err) => + console.error('[VISOR] server-mode rebuild failed', err) + ); + // `watch` also calls back at once, which is the initial build. + const unwatch = state.watch([SCENE_VERSION_KEY], () => rebuilder.request()); return () => { cancelled = true; - disposeRenderer(); + unwatch(); + current?.release(); + current = null; canvas.dispose(); setVisorFrontend(null); setSchemaMismatchError(null); diff --git a/src/ansys/visor/visor-client/src/jest-tests/AppModeBranch.test.tsx b/src/ansys/visor/visor-client/src/jest-tests/AppModeBranch.test.tsx index b49fc400..be62f27e 100644 --- a/src/ansys/visor/visor-client/src/jest-tests/AppModeBranch.test.tsx +++ b/src/ansys/visor/visor-client/src/jest-tests/AppModeBranch.test.tsx @@ -16,6 +16,13 @@ const mockTrame = { getSession: () => ({ subscribe: mockSubscribe, call: async () => undefined }), }), }, + // Calls back at once with no values, as trame's watch does before any state arrives. + state: { + watch: (_keys: string[], fn: (...values: unknown[]) => void) => { + fn(); + return () => {}; + }, + }, // The scene-details fetch never answers, so the server frontend is never // built: this test is about which scene loads, not what the server sends. trigger: () => new Promise(() => {}), diff --git a/src/ansys/visor/visor-client/src/jest-tests/UseServerVtkBuild.test.tsx b/src/ansys/visor/visor-client/src/jest-tests/UseServerVtkBuild.test.tsx index e7b4d877..c9d23a25 100644 --- a/src/ansys/visor/visor-client/src/jest-tests/UseServerVtkBuild.test.tsx +++ b/src/ansys/visor/visor-client/src/jest-tests/UseServerVtkBuild.test.tsx @@ -42,7 +42,13 @@ const session = { }; const trame = { client: { getConnection: () => ({ getSession: () => session }) }, - state: { watch: () => () => {} }, + // Calls back at once with no values, as trame's watch does before any state arrives. + state: { + watch: (_keys: string[], fn: (...values: unknown[]) => void) => { + fn(); + return () => {}; + }, + }, trigger, } as unknown as Trame; diff --git a/src/ansys/visor/visor-client/src/jest-tests/UseServerVtkRebuild.test.tsx b/src/ansys/visor/visor-client/src/jest-tests/UseServerVtkRebuild.test.tsx new file mode 100644 index 00000000..046ddd05 --- /dev/null +++ b/src/ansys/visor/visor-client/src/jest-tests/UseServerVtkRebuild.test.tsx @@ -0,0 +1,232 @@ +import { act, renderHook, waitFor } from '@testing-library/react'; +import type { Trame } from '@kitware/trame/src/trame.js'; +import { useServerVtk } from '../hooks/useServerVtk'; +import type { VisorArgs } from '../hooks/useVisorArgs'; + +/** + * In server mode every change of scene_version rebuilds the frontend from + * freshly fetched scene details, one build at a time, and the replaced + * build's renderer stops listening. + * + * Expected values are hand-written literals. + */ + +/** A minimal v4 scene-details payload as a server-rendering server delivers it. */ +function sceneDetailsPayload() { + return { + schemaVersion: 4, + ui: {}, + scene: { unit: '', orthographicEnabled: null }, + vtkInfo: { + sceneGraph: { + id: 0, + dataArrays: [], + name: '', + isGroupNode: true, + isActorNode: false, + nodeType: 'root', + diffuseColor: [1, 1, 1], + bounds: [], + children: [], + }, + rendererAnnotation: null, + }, + }; +} + +type Watcher = (...values: unknown[]) => void; + +/** A trame state that records its live watchers per key and notifies them on demand. */ +class RecordingState { + readonly #watchers = new Map>(); + + watch(keys: string[], fn: Watcher): () => void { + for (const key of keys) { + if (!this.#watchers.has(key)) { + this.#watchers.set(key, new Set()); + } + this.#watchers.get(key)!.add(fn); + } + fn(); + return () => { + for (const key of keys) { + this.#watchers.get(key)?.delete(fn); + } + }; + } + + liveWatchers(key: string): number { + return this.#watchers.get(key)?.size ?? 0; + } + + notify(key: string): void { + for (const fn of [...(this.#watchers.get(key) ?? [])]) { + fn(1); + } + } +} + +/** Fetches of the scene details, each held until the test answers it. */ +let fetchAnswers: ((payload: unknown) => void)[]; +let triggerNames: string[]; +let state: RecordingState; +let trame: Trame; + +const session = { + subscribe: () => ({ unsubscribe: () => {} }), + call: async () => undefined, +}; + +function argsWith(darkMode: boolean | undefined): VisorArgs { + return { + host: 'localhost', + port: 8080, + aspectRatio: 1, + pixelDensity: 1000, + basePath: '', + darkMode, + } as VisorArgs; +} + +/** Answer the n-th scene-details fetch and let the build it feeds run. */ +async function answerFetchAsync(n: number, payload: unknown): Promise { + await waitFor(() => expect(fetchAnswers.length).toBeGreaterThan(n)); + await act(async () => { + fetchAnswers[n](payload); + }); +} + +const globals = globalThis as unknown as { ResizeObserver: unknown; fetch: unknown }; +let originalResizeObserver: unknown; +let originalFetch: unknown; + +beforeEach(() => { + fetchAnswers = []; + triggerNames = []; + state = new RecordingState(); + trame = { + client: { getConnection: () => ({ getSession: () => session }) }, + state, + trigger: async (name: string) => { + triggerNames.push(name); + if (name !== 'get_visor_scene_details_json') { + return undefined; + } + return new Promise((resolve) => fetchAnswers.push(resolve)); + }, + } as unknown as Trame; + originalResizeObserver = globals.ResizeObserver; + globals.ResizeObserver = class { + observe() {} + disconnect() {} + }; + // A dark theme makes the frontend load the theme stylesheets. + originalFetch = globals.fetch; + globals.fetch = async () => ({ text: async () => '' }); +}); + +afterEach(() => { + globals.ResizeObserver = originalResizeObserver; + globals.fetch = originalFetch; +}); + +describe('useServerVtk rebuild', () => { + test('a burst of scene_version changes during a build ends in one more build', async () => { + const { result } = renderHook(() => useServerVtk(trame, argsWith(undefined))); + act(() => { + state.notify('scene_version'); + state.notify('scene_version'); + }); + await answerFetchAsync(0, sceneDetailsPayload()); + await waitFor(() => expect(result.current.visorFrontend).not.toBeNull()); + const first = result.current.visorFrontend; + + await answerFetchAsync(1, sceneDetailsPayload()); + await waitFor(() => expect(result.current.visorFrontend).not.toBe(first)); + + expect(fetchAnswers.length).toBe(2); + }); + + test('a rebuild leaves one live server_camera_state watcher', async () => { + const { result } = renderHook(() => useServerVtk(trame, argsWith(undefined))); + await answerFetchAsync(0, sceneDetailsPayload()); + await waitFor(() => expect(result.current.visorFrontend).not.toBeNull()); + const first = result.current.visorFrontend; + + act(() => state.notify('scene_version')); + await answerFetchAsync(1, sceneDetailsPayload()); + await waitFor(() => expect(result.current.visorFrontend).not.toBe(first)); + + expect(state.liveWatchers('server_camera_state')).toBe(1); + }); + + test('a mismatch on rebuild sets schemaMismatchError and keeps the frontend', async () => { + const { result } = renderHook(() => useServerVtk(trame, argsWith(undefined))); + await answerFetchAsync(0, sceneDetailsPayload()); + await waitFor(() => expect(result.current.visorFrontend).not.toBeNull()); + const first = result.current.visorFrontend; + + act(() => state.notify('scene_version')); + await answerFetchAsync(1, { schemaVersion: 3 }); + await waitFor(() => expect(result.current.schemaMismatchError).not.toBeNull()); + + expect(result.current.visorFrontend).toBe(first); + }); + + test('each build sends set_dark_theme before its fetch', async () => { + const { result } = renderHook(() => useServerVtk(trame, argsWith(true))); + await answerFetchAsync(0, sceneDetailsPayload()); + await waitFor(() => expect(result.current.visorFrontend).not.toBeNull()); + const first = result.current.visorFrontend; + + act(() => state.notify('scene_version')); + await answerFetchAsync(1, sceneDetailsPayload()); + await waitFor(() => expect(result.current.visorFrontend).not.toBe(first)); + + expect( + triggerNames.filter( + (name) => name === 'set_dark_theme' || name === 'get_visor_scene_details_json' + ) + ).toEqual([ + 'set_dark_theme', + 'get_visor_scene_details_json', + 'set_dark_theme', + 'get_visor_scene_details_json', + ]); + }); + + test('unmounting during a build with a request pending fetches no more', async () => { + const { unmount } = renderHook(() => useServerVtk(trame, argsWith(undefined))); + await waitFor(() => expect(fetchAnswers.length).toBe(1)); + act(() => state.notify('scene_version')); + + unmount(); + await act(async () => { + fetchAnswers[0](sceneDetailsPayload()); + }); + + expect(fetchAnswers.length).toBe(1); + }); + + test('a build that fails after creating its renderer disposes it', async () => { + // The theme stylesheet load runs after the renderer exists, so its failure fails the build there. + globals.fetch = async () => { + throw new Error('stylesheet unavailable'); + }; + const consoleError = jest.spyOn(console, 'error').mockImplementation(() => {}); + try { + renderHook(() => useServerVtk(trame, argsWith(true))); + await answerFetchAsync(0, sceneDetailsPayload()); + await waitFor(() => + expect(consoleError).toHaveBeenCalledWith( + '[VISOR] server-mode rebuild failed', + expect.any(Error) + ) + ); + + expect(state.liveWatchers('server_camera_state')).toBe(0); + } finally { + consoleError.mockRestore(); + } + }); +}); diff --git a/src/ansys/visor/visor-client/src/jest-tests/UseServerVtkTheme.test.tsx b/src/ansys/visor/visor-client/src/jest-tests/UseServerVtkTheme.test.tsx index 1e5474b7..8eba69db 100644 --- a/src/ansys/visor/visor-client/src/jest-tests/UseServerVtkTheme.test.tsx +++ b/src/ansys/visor/visor-client/src/jest-tests/UseServerVtkTheme.test.tsx @@ -27,6 +27,13 @@ const session = { }; const trame = { client: { getConnection: () => ({ getSession: () => session }) }, + // Calls back at once with no values, as trame's watch does before any state arrives. + state: { + watch: (_keys: string[], fn: (...values: unknown[]) => void) => { + fn(); + return () => {}; + }, + }, trigger, } as unknown as Trame; From 78c449ad8c8087262f66ce8863cac59841119f89 Mon Sep 17 00:00:00 2001 From: pyansys-ci-bot <92810346+pyansys-ci-bot@users.noreply.github.com> Date: Tue, 27 Oct 2026 21:57:17 +0000 Subject: [PATCH 4/5] chore: adding changelog file 194.added.md [dependabot-skip] --- doc/changelog.d/194.added.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 doc/changelog.d/194.added.md diff --git a/doc/changelog.d/194.added.md b/doc/changelog.d/194.added.md new file mode 100644 index 00000000..253612c3 --- /dev/null +++ b/doc/changelog.d/194.added.md @@ -0,0 +1 @@ +[Remote rendering 4.4a] Update server mode panels when scene changes From 5c61d70afcce942b51f3469ff251586493bda058 Mon Sep 17 00:00:00 2001 From: pyansys-ci-bot <92810346+pyansys-ci-bot@users.noreply.github.com> Date: Thu, 29 Oct 2026 21:58:35 +0000 Subject: [PATCH 5/5] chore: adding changelog file 194.added.md [dependabot-skip] --- doc/changelog.d/194.added.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/changelog.d/194.added.md b/doc/changelog.d/194.added.md index 253612c3..f02a37d4 100644 --- a/doc/changelog.d/194.added.md +++ b/doc/changelog.d/194.added.md @@ -1 +1 @@ -[Remote rendering 4.4a] Update server mode panels when scene changes +Remote rendering 4.4a: Update server mode panels when scene changes