From ab3ef67faa4da872fd354b1b6d85b1c3b7c58784 Mon Sep 17 00:00:00 2001 From: Laura Kasian Date: Thu, 24 Sep 2026 14:16:17 -0700 Subject: [PATCH 01/14] feat: server-owned UI panel and theme state on save and load --- src/ansys/visor/viewer/app/trame/local_app.py | 64 ++++ .../runtime/requests/widget_state_payloads.py | 45 +++ .../models/runtime/scene/runtime_app_state.py | 13 +- .../models/runtime/visor_scene_details.py | 12 +- src/ansys/visor/viewer/vtk/scene/base.py | 137 +++++++- .../viewer/vtk/scene/visor_state_mapper.py | 6 +- tests/integration/test_save_load_state.py | 4 +- .../app/test_local_app_ui_panel_triggers.py | 150 +++++++++ tests/unit/vtk/scene/test_base.py | 306 +++++++++++++++++- tests/unit/vtk/scene/test_local_scene.py | 13 + 10 files changed, 724 insertions(+), 26 deletions(-) create mode 100644 tests/unit/app/test_local_app_ui_panel_triggers.py diff --git a/src/ansys/visor/viewer/app/trame/local_app.py b/src/ansys/visor/viewer/app/trame/local_app.py index f702b1ab..8e24fc99 100644 --- a/src/ansys/visor/viewer/app/trame/local_app.py +++ b/src/ansys/visor/viewer/app/trame/local_app.py @@ -17,6 +17,10 @@ SetBoundingBoxVisibilityPayload, SetCrossSectionVisibilityPayload, SetEdgesVisiblePayload, + SetPanelTopLeftPanelCollapsedPayload, + SetPanelTopRightLegendCollapsedPayload, + SetPanelTopRightPanelCollapsedPayload, + SetPanelTopRightTabIndexPayload, SetProjectionPayload, SyncCrossSectionPlanePayload, ) @@ -76,6 +80,14 @@ def sync_cross_section_plane( self, origin: List[float], normal: List[float] ) -> None: ... + def set_panel_top_left_panel_collapsed(self, collapsed: bool) -> None: ... + + def set_panel_top_right_panel_collapsed(self, collapsed: bool) -> None: ... + + def set_panel_top_right_legend_collapsed(self, collapsed: bool) -> None: ... + + def set_panel_top_right_tab_index(self, tab_index: int) -> None: ... + # ---------------------------------------------------------------------- # Trigger payload models @@ -209,6 +221,10 @@ class LocalApp: set_bounding_box_visibility: shows or hides the bounding-box outline set_projection: sets parallel or perspective projection on the camera record sync_cross_section_plane: records a settled cross-section plane reported by the frontend + set_panel_top_left_panel_collapsed: records whether the top-left panel is collapsed + set_panel_top_right_panel_collapsed: records whether the top-right panel is collapsed + set_panel_top_right_legend_collapsed: records whether the top-right legend is collapsed + set_panel_top_right_tab_index: records which top-right tab is active set_only_cookie: sets a cookie on the server (note: Trame server only allows a single cookie header) Protected Methods: _cleanup(): Cleans up the active actor in the visualization pipeline. @@ -504,6 +520,13 @@ def sync_camera(self, payload) -> None: # ``set_projection`` lives here too: it is delivered the same way, but # it writes the camera record's projection field rather than a toggle # of its own. + # + # The last four carry UI panel layout rather than widget state. They + # have no renderer half at all -- nothing the server renders depends on + # which panel is collapsed or which tab is active -- so each coordinator + # writes one store field and stops. Their echoes are idempotent on the + # same terms as the toggles': after a delivered apply the client reports + # back the value the server just sent it. # ------------------------------------------------------------------ @trigger("set_cross_section_visibility") @@ -567,6 +590,47 @@ def sync_cross_section_plane(self, payload) -> None: return api.sync_cross_section_plane(payload.origin, payload.normal) + @trigger("set_panel_top_left_panel_collapsed") + @parse_payload(SetPanelTopLeftPanelCollapsedPayload) + def set_panel_top_left_panel_collapsed(self, payload) -> None: + """Frontend -> Backend: the top-left panel reports its collapsed state.""" + api = self._mutation_api("set_panel_top_left_panel_collapsed", payload) + if api is None: + return + api.set_panel_top_left_panel_collapsed(payload.collapsed) + + @trigger("set_panel_top_right_panel_collapsed") + @parse_payload(SetPanelTopRightPanelCollapsedPayload) + def set_panel_top_right_panel_collapsed(self, payload) -> None: + """Frontend -> Backend: the top-right panel reports its collapsed state.""" + api = self._mutation_api("set_panel_top_right_panel_collapsed", payload) + if api is None: + return + api.set_panel_top_right_panel_collapsed(payload.collapsed) + + @trigger("set_panel_top_right_legend_collapsed") + @parse_payload(SetPanelTopRightLegendCollapsedPayload) + def set_panel_top_right_legend_collapsed(self, payload) -> None: + """Frontend -> Backend: the top-right legend reports its collapsed state.""" + api = self._mutation_api("set_panel_top_right_legend_collapsed", payload) + if api is None: + return + api.set_panel_top_right_legend_collapsed(payload.collapsed) + + @trigger("set_panel_top_right_tab_index") + @parse_payload(SetPanelTopRightTabIndexPayload) + def set_panel_top_right_tab_index(self, payload) -> None: + """Frontend -> Backend: the top-right panel reports its active tab. + + The index is forwarded verbatim and is never range-checked here; the + client's ``selectTab`` is the guard, so a future third tab is a UI + change rather than a validation failure at this boundary. + """ + api = self._mutation_api("set_panel_top_right_tab_index", payload) + if api is None: + return + api.set_panel_top_right_tab_index(payload.tab_index) + def set_only_cookie(self, key: str, value: str): """ Sets a cookie on the server. NOTE: there is a limitation diff --git a/src/ansys/visor/viewer/models/runtime/requests/widget_state_payloads.py b/src/ansys/visor/viewer/models/runtime/requests/widget_state_payloads.py index 610f1b98..1fdda2f3 100644 --- a/src/ansys/visor/viewer/models/runtime/requests/widget_state_payloads.py +++ b/src/ansys/visor/viewer/models/runtime/requests/widget_state_payloads.py @@ -18,6 +18,18 @@ carries two three-component vectors rather than a boolean. It lives here rather than beside the per-part models for the same reason the four above do: it is scene-wide and carries no ``nodeId``. + +The four ``SetPanelTopLeft*`` / ``SetPanelTopRight*`` models carry UI panel +layout rather than widget state. They are here on the same terms: each is +scene-wide, carries no ``nodeId``, and carries the absolute value of +exactly one panel field. A panel echo is idempotent, as a toggle echo is: +the value the client reports after a delivered apply is the value the +server already holds. + +``tab_index`` is deliberately unbounded. The client sends ``0`` or ``1`` +and nothing else, and ``Panel_TopRight_Util.selectTab`` already ignores any +other value, so the bound lives there. A bound here would make a future +third tab a validation failure at the boundary rather than a UI change. """ from typing import List @@ -65,3 +77,36 @@ class SyncCrossSectionPlanePayload(BaseModel): origin: List[float] = Field(min_length=3, max_length=3) normal: List[float] = Field(min_length=3, max_length=3) + +class SetPanelTopLeftPanelCollapsedPayload(BaseModel): + """Payload of the ``set_panel_top_left_panel_collapsed`` trigger.""" + + model_config = ConfigDict(populate_by_name=True) + + collapsed: bool + + +class SetPanelTopRightPanelCollapsedPayload(BaseModel): + """Payload of the ``set_panel_top_right_panel_collapsed`` trigger.""" + + model_config = ConfigDict(populate_by_name=True) + + collapsed: bool + + +class SetPanelTopRightLegendCollapsedPayload(BaseModel): + """Payload of the ``set_panel_top_right_legend_collapsed`` trigger.""" + + model_config = ConfigDict(populate_by_name=True) + + collapsed: bool + + +class SetPanelTopRightTabIndexPayload(BaseModel): + """Payload of the ``set_panel_top_right_tab_index`` trigger.""" + + model_config = ConfigDict(populate_by_name=True) + + tab_index: int = Field(alias="tabIndex") + + diff --git a/src/ansys/visor/viewer/models/runtime/scene/runtime_app_state.py b/src/ansys/visor/viewer/models/runtime/scene/runtime_app_state.py index 4eb9512d..b5bfba6b 100644 --- a/src/ansys/visor/viewer/models/runtime/scene/runtime_app_state.py +++ b/src/ansys/visor/viewer/models/runtime/scene/runtime_app_state.py @@ -30,7 +30,7 @@ class RuntimeAppState(BaseModel): @classmethod def from_components( cls, - dark_mode: bool, + ui: VisorUIState, unit: str | None, dataset_states: Dict[int, RuntimeDatasetState], orthographic_enabled: bool | None = None, @@ -41,8 +41,13 @@ def from_components( camera: VisorCameraState | None = None, variable_states: Dict[str, VisorVariableState] | None = None, ) -> "RuntimeAppState": - """Construct a RuntimeAppState from the given components.""" - ui_state = VisorUIState(dark_theme=dark_mode) + """Construct a RuntimeAppState from the given components. + + The UI record arrives whole rather than as a bare ``dark_mode``: the + server owns every field of it -- the theme and the four panel-layout + fields -- and assembles them together, so there is one place that + decides what the client is told about the UI rather than two. + """ runtime_scene_state = RuntimeSceneState( unit=unit, camera=camera, @@ -55,6 +60,6 @@ def from_components( spectrum_states=variable_states or {}, ) return cls( - ui=ui_state, + ui=ui, scene=runtime_scene_state, ) diff --git a/src/ansys/visor/viewer/models/runtime/visor_scene_details.py b/src/ansys/visor/viewer/models/runtime/visor_scene_details.py index 80a45ca5..a65b0889 100644 --- a/src/ansys/visor/viewer/models/runtime/visor_scene_details.py +++ b/src/ansys/visor/viewer/models/runtime/visor_scene_details.py @@ -4,6 +4,7 @@ from pydantic import BaseModel, ConfigDict, Field +from ansys.visor.viewer.models.common.visor_ui_state import VisorUIState from ansys.visor.viewer.models.runtime.dataset.runtime_dataset_state import RuntimeDatasetState from ansys.visor.viewer.models.runtime.scene.runtime_app_state import RuntimeAppState from ansys.visor.viewer.models.runtime.vtk.renderer_annotation import RendererAnnotation @@ -26,7 +27,7 @@ class VisorSceneDetails(BaseModel): @classmethod def from_components( cls, - dark_mode: bool, + ui: VisorUIState, unit: str | None, dataset_states: Dict[int, RuntimeDatasetState], scene_graph_state: SceneGraphNodeInfo | None = None, @@ -36,13 +37,18 @@ def from_components( edges_enabled: bool | None = None, bounding_box_enabled: bool | None = None, ) -> "VisorSceneDetails": - """Construct an instance from components.""" + """Construct an instance from components. + + The UI record is forwarded whole to + :meth:`RuntimeAppState.from_components`; this class does not build one + and does not read any field of it. + """ vtk_info = RuntimeVTKInfo( scene_graph=scene_graph_state, renderer_annotation=renderer_annotation, ) app_state = RuntimeAppState.from_components( - dark_mode=dark_mode, + ui=ui, unit=unit, dataset_states=dataset_states, orthographic_enabled=orthographic_enabled, diff --git a/src/ansys/visor/viewer/vtk/scene/base.py b/src/ansys/visor/viewer/vtk/scene/base.py index 04c9ca91..1e5ce173 100644 --- a/src/ansys/visor/viewer/vtk/scene/base.py +++ b/src/ansys/visor/viewer/vtk/scene/base.py @@ -14,6 +14,7 @@ from ansys.visor.viewer.core.visor_logging import VisorDefaultLogger from ansys.visor.viewer.core.visor_types import VisorDatasetType from ansys.visor.viewer.models.common.visor_camera_state import VisorCameraState +from ansys.visor.viewer.models.common.visor_ui_state import VisorUIState from ansys.visor.viewer.models.persist.persisted_viewer_state import PersistedViewerStateV1 from ansys.visor.viewer.models.runtime.visor_scene_details import VisorSceneDetails from ansys.visor.viewer.renderer.base import IRenderer @@ -67,6 +68,10 @@ class VisorSceneBase(ABC): _cross_section_enabled: bool _edges_enabled: bool _bounding_box_enabled: bool + _panel_top_left_panel_collapsed: bool + _panel_top_right_panel_collapsed: bool + _panel_top_right_legend_collapsed: bool + _panel_top_right_tab_index: int def __init__( self, @@ -88,6 +93,22 @@ def __init__( self._edges_enabled: bool = False self._bounding_box_enabled: bool = False + # Server-tracked UI panel layout. Absolute values, never toggles, and + # written only by the four panel triggers. Initialised to the client + # panels' own mount defaults -- Panel_TopLeft's ``isPanelCollapsed`` + # and both of Panel_TopRight's ``isCollapsed`` closures start false and + # are mount-clicked to false, and its ``tabIndex`` starts at 0 -- for + # the reason the toggle store gives above, and for a second one: these + # are delivered through a dump that excludes ``None``, so a field left + # unset here is simply omitted from the payload and the client shows + # its own defaults instead of the server's record. There is + # deliberately no ``_dark_theme`` field: the theme is ``dark_mode``, + # and a second copy of it is the divergence set_projection refuses. + self._panel_top_left_panel_collapsed: bool = False + self._panel_top_right_panel_collapsed: bool = False + self._panel_top_right_legend_collapsed: bool = False + self._panel_top_right_tab_index: int = 0 + self._server = server self._scene_graph = None self._pipelines = {} @@ -178,6 +199,17 @@ async def get_state(self, timeout: float) -> PersistedViewerStateV1: this object's own store, and the camera from the renderer's record; the reply is consulted for none of the three. + The UI record is the fourth. ``runtime_state.ui`` is replaced + wholesale with this object's own record -- the theme from + ``dark_mode`` and the four panel-layout fields from the store the + panel triggers write -- so the browser's ``ui`` block is discarded + entire. The assignment is unconditional, as the camera and toggle + assignments are, and it is what makes the saved theme the server's + rather than the embedding host's: under Dash the host prop overrides + the delivered theme in the browser, so a reply that was trusted here + would write the host's value into the file and, on the next load, into + ``dark_mode``. + The camera comes from the renderer's record, which is authoritative, rather than from the reply or from the pipeline ``vtkCamera``: the pipeline is the record's projection, and reading it back would re-import whatever drift @@ -214,6 +246,7 @@ async def get_state(self, timeout: float) -> PersistedViewerStateV1: runtime_state.scene.cross_section_enabled = self._cross_section_enabled runtime_state.scene.edges_enabled = self._edges_enabled runtime_state.scene.bounding_box_enabled = self._bounding_box_enabled + runtime_state.ui = self._build_ui_state() runtime_state.scene.dataset_states = registry_dataset_states persisted = self._state_mapper.runtime_to_persisted(runtime_state) @@ -242,8 +275,9 @@ def apply_state(self, state: PersistedViewerStateV1): # One call per state class: updates the server's stored state and its VTK objects. self._restore_part_states(runtime_app_state) self._restore_widget_state(runtime_app_state) + self._restore_ui_state(runtime_app_state) self._restore_camera_state(runtime_app_state) - # TODO: restore UI state and variable states when they are synced back to the server. + # TODO: restore variable states when they are synced back to the server. # Finalize here, not on the load path. On a cold load -- viewer started with no dataset, # then a state loaded -- load_state adds the datasets and only then calls apply_state, so a @@ -282,11 +316,11 @@ def get_scene_details(self) -> VisorSceneDetails: stored, so it can't disagree with the camera. ``None`` means "nothing was ever written." - No ``_vtk_lock``: the reads here (three booleans, one camera field) - aren't consumed as a mutually consistent snapshot, and locking a - request-path read against the trigger thread belongs with the - round-trip/thread-affinity work, not here. Accepted exposure: one - stale field in a delivered payload. + No ``_vtk_lock``: the reads here (three booleans, one camera field, + and the five the UI record is assembled from) aren't consumed as a + mutually consistent snapshot, and locking a request-path read against + the trigger thread belongs with the round-trip/thread-affinity work, + not here. Accepted exposure: one stale field in a delivered payload. """ if self._scene_graph is None: self._initialize_scene_graph() @@ -294,7 +328,7 @@ def get_scene_details(self) -> VisorSceneDetails: scene_graph_state = self._build_scene_graph_state() camera_record = self._renderer.get_camera_state() return VisorSceneDetails.from_components( - dark_mode=self.dark_mode, + ui=self._build_ui_state(), unit=self._dataset_registry.unit, dataset_states=self._dataset_registry.runtime_state_dict, scene_graph_state=scene_graph_state, @@ -743,6 +777,48 @@ def set_projection(self, parallel: bool) -> None: self._renderer.set_projection(parallel) self._renderer.serialize_camera_state() + # ========================================================================= + # UI panel layout — coordinator surface + # + # The same shape as the widget toggles above, minus the renderer half: + # each writes one store field under ``_vtk_lock`` and stops. There is no + # ``IRenderer`` seam to fill, because nothing the server renders depends + # on panel layout -- it is browser-side chrome whose only server-side job + # is to survive a refresh. + # + # No notify, for the reason the surfaces above give: no ``render()``, no + # ``flush_wasm_state()``, no ``set_state``. + # + # Every value that arrives here is absolute, never relative, and an echo + # is idempotent: after a delivered apply the client reports back the value + # the server just sent it. + # ========================================================================= + + def set_panel_top_left_panel_collapsed(self, collapsed: bool) -> None: + """Record whether the top-left panel is collapsed.""" + with self._vtk_lock: + self._panel_top_left_panel_collapsed = collapsed + + def set_panel_top_right_panel_collapsed(self, collapsed: bool) -> None: + """Record whether the top-right panel is collapsed.""" + with self._vtk_lock: + self._panel_top_right_panel_collapsed = collapsed + + def set_panel_top_right_legend_collapsed(self, collapsed: bool) -> None: + """Record whether the top-right legend overlay is collapsed.""" + with self._vtk_lock: + self._panel_top_right_legend_collapsed = collapsed + + def set_panel_top_right_tab_index(self, tab_index: int) -> None: + """Record which top-right tab is active. + + The value is stored opaquely and is never interpreted here; it is + only ever handed back to the client's ``selectTab``, which is where + the range guard lives. + """ + with self._vtk_lock: + self._panel_top_right_tab_index = tab_index + def _restore_part_states(self, runtime_app_state: "RuntimeAppState") -> None: """ Restore per-part state from a runtime app state, on the load path. @@ -837,6 +913,31 @@ def _restore_widget_state(self, runtime_app_state: "RuntimeAppState") -> None: self._renderer.sync_cross_section_plane(cs.origin, cs.normal) self._renderer.serialize_cross_section_state() + def _restore_ui_state(self, runtime_app_state: "RuntimeAppState") -> None: + """ + Restore the UI panel layout from a runtime app state, on the load path. + + Absent says nothing: a state that does not carry a panel field leaves + the server's value alone. Four independent guards and not one, because + a file can carry any subset -- anything written before this record + existed carries none of them. + + ``dark_theme`` is deliberately not read here. It is + :meth:`apply_state`'s own line, and two readers of one property is the + divergence this epic has been removing. + + Callers must hold ``_vtk_lock``. + """ + ui = runtime_app_state.ui + if ui.panel_top_left_panel_collapsed is not None: + self._panel_top_left_panel_collapsed = ui.panel_top_left_panel_collapsed + if ui.panel_top_right_panel_collapsed is not None: + self._panel_top_right_panel_collapsed = ui.panel_top_right_panel_collapsed + if ui.panel_top_right_legend_collapsed is not None: + self._panel_top_right_legend_collapsed = ui.panel_top_right_legend_collapsed + if ui.panel_top_right_tab_index is not None: + self._panel_top_right_tab_index = ui.panel_top_right_tab_index + def _restore_one_part_state( self, part_id: int, @@ -1021,6 +1122,28 @@ def _update_actor_count(self): """Update the bounding box widget with the current part-node count.""" self._renderer.update_actor_count(self._scene_graph.descendant_part_count()) + def _build_ui_state(self) -> VisorUIState: + """Assemble the server's UI record: the theme and the four panel fields. + + Every field is concrete by construction -- ``dark_mode`` is a ``bool`` + and the four panel attributes are initialised to the client's mount + defaults -- so the record never carries a ``None``. That matters on + the way out: the scene-details dump excludes ``None``, so an unset + field would be omitted from the payload and the client would fall back + to its own defaults rather than the server's record. + + No lock of its own. A caller that needs a mutually consistent + snapshot holds ``_vtk_lock`` around the call; :meth:`get_state` does, + :meth:`get_scene_details` deliberately does not. + """ + return VisorUIState( + dark_theme=self.dark_mode, + panel_top_left_panel_collapsed=self._panel_top_left_panel_collapsed, + panel_top_right_panel_collapsed=self._panel_top_right_panel_collapsed, + panel_top_right_legend_collapsed=self._panel_top_right_legend_collapsed, + panel_top_right_tab_index=self._panel_top_right_tab_index, + ) + def _build_scene_graph_state(self): """Assemble the pure scene-graph ``SceneGraphNodeInfo`` tree. diff --git a/src/ansys/visor/viewer/vtk/scene/visor_state_mapper.py b/src/ansys/visor/viewer/vtk/scene/visor_state_mapper.py index e22c0ffb..00c5c9d5 100644 --- a/src/ansys/visor/viewer/vtk/scene/visor_state_mapper.py +++ b/src/ansys/visor/viewer/vtk/scene/visor_state_mapper.py @@ -79,7 +79,9 @@ def persisted_to_runtime(self, state: PersistedViewerStateV1) -> RuntimeAppState :meth:`VisorSceneBase._restore_part_states`. """ - # UI settings + # UI settings. The record crosses whole: the four panel fields are as + # much the server's as the theme is, and splitting one field out here + # is what dropped them on the load path before. ui_state = state.ui # scene state @@ -108,7 +110,7 @@ def persisted_to_runtime(self, state: PersistedViewerStateV1) -> RuntimeAppState runtime_dataset_states[dataset.id] = runtime_state return RuntimeAppState.from_components( - dark_mode=ui_state.dark_theme, + ui=ui_state, unit=unit, camera=camera, cross_section=scene_state.cross_section, diff --git a/tests/integration/test_save_load_state.py b/tests/integration/test_save_load_state.py index 1d966a11..ce7f0e24 100644 --- a/tests/integration/test_save_load_state.py +++ b/tests/integration/test_save_load_state.py @@ -448,7 +448,7 @@ def test_saved_visor_json_carries_registry_part_state_keyed_by_name(self, iface, # the same dataset. A pass therefore proves the file came from the # registry rather than from the frontend round trip. frontend_state = RuntimeAppState.from_components( - dark_mode=False, + ui=VisorUIState(dark_theme=False), unit="m", dataset_states={ dataset_id: RuntimeDatasetState( @@ -547,7 +547,7 @@ def test_saved_visor_json_carries_the_camera_record_not_the_browsers(self, iface # The browser answers getState with a different camera in every field. # A pass therefore proves the file came from the record. frontend_state = RuntimeAppState.from_components( - dark_mode=False, + ui=VisorUIState(dark_theme=False), unit="m", dataset_states={}, camera=_reply_camera(), diff --git a/tests/unit/app/test_local_app_ui_panel_triggers.py b/tests/unit/app/test_local_app_ui_panel_triggers.py new file mode 100644 index 00000000..501d6b1a --- /dev/null +++ b/tests/unit/app/test_local_app_ui_panel_triggers.py @@ -0,0 +1,150 @@ +"""Unit tests for LocalApp's four UI panel triggers. + +A module of its own rather than an addition to +``test_local_app_widget_triggers.py``: that module's tests are written +against a single ``visible`` boolean shared by all three widget toggles, +and its registration test names those three. These four are neither +toggles nor uniform -- three carry ``collapsed`` and the fourth carries +``tabIndex``, an ``int`` behind a camelCase alias. + +Coverage targets +---------------- +1. Each of the four triggers delegates the value it arrived with, exactly + once, to the identically-named coordinator method. Four tests and not + one: a wrong trigger name and a delegation to the wrong coordinator + method are distinct failures that a single test could not tell apart. +2. A payload missing ``tabIndex`` is a logged no-op: nothing is delegated. + +What is deliberately **not** re-tested here. ``@parse_payload`` is one +shared decorator, already pinned four times over in +``test_local_app_widget_triggers.py``; a second, third and fourth +malformed-payload case here would restate it. The "no coordinator +injected" path and the "trigger names are registered after decoration" +case are likewise already pinned in that module, against the same +``_mutation_api`` helper and the same registration loop these four +handlers use unchanged. + +Every payload here is a hand-written literal. ``tabIndex`` is written in +its wire spelling, so a model that lost the alias fails here rather than +passing on a snake_case key the client never sends. +""" + +from unittest.mock import MagicMock, patch + +import pytest + +from ansys.visor.viewer.app.trame.local_app import LocalApp + +# Hand-written literals, each the opposite of the server's own initial value +# (``False`` for the three collapse flags, ``0`` for the tab index), so a +# handler that delegated a default rather than its payload would fail. +COLLAPSED = True +TAB_INDEX = 1 + + +class MockController: + """Minimal trame controller stand-in that records added handlers.""" + + def __init__(self): + self.handlers = {} + self.add_call_count = 0 + + def add(self, event): + self.add_call_count += 1 + + def decorator(fn): + self.handlers[event] = fn + return fn + + return decorator + + +@pytest.fixture +def mock_server(): + """Provide a mock Trame server.""" + server = MagicMock() + server.controller = MockController() + server.http_headers.set_header = MagicMock() + server.name = "TestServer" + server._www = None + return server + + +@pytest.fixture +def api(): + """Stand-in for the injected scene coordinator.""" + return MagicMock(name="scene_mutation_api") + + +@pytest.fixture +def app(mock_server, api): + """LocalApp with a coordinator injected.""" + return LocalApp( + server=mock_server, + get_scene_details_json=MagicMock(), + handle_save_state_response=MagicMock(), + standalone=True, + scene_mutation_api=api, + ) + + +# =========================================================================== +# Delegation +# =========================================================================== + +def test_set_panel_top_left_panel_collapsed_delegates_the_collapsed_value(app, api): + """The top-left panel trigger hands the coordinator the value it received.""" + result = app.set_panel_top_left_panel_collapsed({"collapsed": COLLAPSED}) + + assert result is None + api.set_panel_top_left_panel_collapsed.assert_called_once_with(True) + + +def test_set_panel_top_right_panel_collapsed_delegates_the_collapsed_value(app, api): + """The top-right panel trigger hands the coordinator the value it received.""" + result = app.set_panel_top_right_panel_collapsed({"collapsed": COLLAPSED}) + + assert result is None + api.set_panel_top_right_panel_collapsed.assert_called_once_with(True) + + +def test_set_panel_top_right_legend_collapsed_delegates_the_collapsed_value(app, api): + """The legend trigger hands the coordinator the value it received.""" + result = app.set_panel_top_right_legend_collapsed({"collapsed": COLLAPSED}) + + assert result is None + api.set_panel_top_right_legend_collapsed.assert_called_once_with(True) + + +def test_set_panel_top_right_tab_index_delegates_the_tab_index(app, api): + """The tab trigger hands the coordinator the index it received. + + The payload key is the wire spelling ``tabIndex``; the coordinator is + called with the snake_case value, which is what the alias is for. + """ + result = app.set_panel_top_right_tab_index({"tabIndex": TAB_INDEX}) + + assert result is None + api.set_panel_top_right_tab_index.assert_called_once_with(1) + + +# =========================================================================== +# Validation -- a malformed payload reaches no handler body +# =========================================================================== + +def test_set_panel_top_right_tab_index_missing_tab_index_is_a_logged_no_op(app, api): + """An empty payload never reaches the handler body. + + Asserted against the whole mock, so a leak under any other method name + still fails. One such test for the four triggers rather than four: they + share one decorator, and reverting ``@parse_payload`` on this one is the + case this test exists to report. + """ + with patch("ansys.visor.viewer.app.trame.local_app.logger") as log: + result = app.set_panel_top_right_tab_index({}) + + assert result is None + assert api.mock_calls == [] + assert log.warning.call_count == 1 + + diff --git a/tests/unit/vtk/scene/test_base.py b/tests/unit/vtk/scene/test_base.py index 3ac40a2a..c6fa3933 100644 --- a/tests/unit/vtk/scene/test_base.py +++ b/tests/unit/vtk/scene/test_base.py @@ -599,7 +599,7 @@ def mocked_scene(renderer): # must return a real RuntimeAppState rather than a MagicMock (whose # dataset_states would be a MagicMock and raise on iteration). s._state_mapper.persisted_to_runtime.return_value = RuntimeAppState.from_components( - dark_mode=False, unit="m", dataset_states={} + ui=VisorUIState(dark_theme=False), unit="m", dataset_states={} ) s._vtk_lock = _LockSpy() return s @@ -803,7 +803,7 @@ def _seed_part_variables(registry, variables, part_id=NODE_ID, dataset_id=1): def _runtime_state(part_states, variable_states=None, dataset_id=1): """A real RuntimeAppState carrying the given per-part records.""" return RuntimeAppState.from_components( - dark_mode=False, + ui=VisorUIState(dark_theme=False), unit="m", dataset_states={ dataset_id: RuntimeDatasetState(id=dataset_id, part_states=part_states) @@ -820,7 +820,7 @@ def _frontend_state(): output from frontend-sourced output. """ return RuntimeAppState.from_components( - dark_mode=False, + ui=VisorUIState(dark_theme=False), unit="m", dataset_states={ FRONTEND_DATASET_ID: RuntimeDatasetState( @@ -1075,7 +1075,7 @@ def _save_scene(scene, record, reply_camera): async def _get_runtime_state_async(timeout): return RuntimeAppState.from_components( - dark_mode=False, + ui=VisorUIState(dark_theme=False), unit="m", dataset_states={}, camera=reply_camera, @@ -2069,7 +2069,7 @@ def _toggle_save_scene(scene, reply_toggle): async def _get_runtime_state_async(timeout): return RuntimeAppState.from_components( - dark_mode=False, + ui=VisorUIState(dark_theme=False), unit="m", dataset_states={}, cross_section_enabled=reply_toggle, @@ -2235,7 +2235,7 @@ def test_get_state_discards_the_toggles_the_browser_returned(scene): def _toggle_runtime_state(**toggles): """A real RuntimeAppState carrying only the toggles named.""" return RuntimeAppState.from_components( - dark_mode=False, + ui=VisorUIState(dark_theme=False), unit="m", dataset_states={}, **toggles, @@ -2452,7 +2452,7 @@ def _projection_save_scene(scene, record, reply_orthographic): async def _get_runtime_state_async(timeout): return RuntimeAppState.from_components( - dark_mode=False, + ui=VisorUIState(dark_theme=False), unit="m", dataset_states={}, orthographic_enabled=reply_orthographic, @@ -2698,7 +2698,7 @@ def _plane_save_scene(scene, record_plane, reply_plane): async def _get_runtime_state_async(timeout): return RuntimeAppState.from_components( - dark_mode=False, + ui=VisorUIState(dark_theme=False), unit="m", dataset_states={}, cross_section=reply_plane, @@ -2886,3 +2886,293 @@ def _sync(origin, normal): ] +# =========================================================================== +# UI panel state -- the store, the read, the restore +# +# The four panel fields are the browser's chrome, recorded on the server so +# that a refresh restores them. They have no renderer half: nothing the +# server renders depends on which panel is collapsed, so each coordinator +# writes one store field and stops. That is why there is no apply-half test +# here to match the toggles' pair above -- there is no apply. +# +# Every expected value below is a hand-written literal. ``True, True, True, +# 1`` is used as the value the client reports, because the server's own +# initial values are the literals ``False, False, False, 0``, so a body that +# wrote a default rather than its argument fails. +# =========================================================================== + +PANEL_COLLAPSED = True +PANEL_TAB_INDEX = 1 + +# What a browser that was asked would answer. Deliberately the opposite of +# what the store holds in the derivation test, so that "read the store" and +# "read the reply" cannot both pass. +REPLY_PANEL_COLLAPSED = False +REPLY_PANEL_TAB_INDEX = 0 + +# The theme pair. The server holds one value and the browser reports the +# other, so that "wrote dark_mode" and "passed the reply through" are +# distinguishable in a single assertion. +SERVER_DARK_MODE = True +REPLY_DARK_THEME = False + + +class _PanelStoreLockSpy(_LockSpy): + """Lock spy that reads the tab-index store field as the lock is released. + + The panel coordinators make no renderer call, so there is no inner call + to probe the way ``_ToggleSpyRenderer`` probes the toggles. What can be + observed instead is the store itself at the moment the critical section + ends: if the write happened inside ``with self._vtk_lock:`` the field + already carries the new value when the lock is released, and the depth + read here is the depth the write ran at. With the ``with`` removed the + lock is never entered at all and both recordings stay ``None``. + """ + + def __init__(self, scene): + super().__init__() + self._scene = scene + self.value_at_release = None + self.depth_at_release = None + + def __exit__(self, exc_type, exc, tb): + self.value_at_release = self._scene._panel_top_right_tab_index + self.depth_at_release = self.depth + return super().__exit__(exc_type, exc, tb) + + +def _reply_ui( + panel_top_left_panel_collapsed, + panel_top_right_panel_collapsed, + panel_top_right_legend_collapsed, + panel_top_right_tab_index, + dark_theme=REPLY_DARK_THEME, +): + """The ``ui`` block a browser round trip answers with.""" + return VisorUIState( + dark_theme=dark_theme, + panel_top_left_panel_collapsed=panel_top_left_panel_collapsed, + panel_top_right_panel_collapsed=panel_top_right_panel_collapsed, + panel_top_right_legend_collapsed=panel_top_right_legend_collapsed, + panel_top_right_tab_index=panel_top_right_tab_index, + ) + + +def _ui_save_scene(scene, reply_ui): + """Wire *scene* for a save whose browser reply carries *reply_ui*. + + The same shape as ``_toggle_save_scene`` above; the registry is emptied so + the mapper's per-dataset loop contributes nothing, and what is under test + is the ``ui`` block of the persisted state. + """ + scene._dataset_registry = VisorDatasetRegistry() + + async def _get_runtime_state_async(timeout): + return RuntimeAppState.from_components( + ui=reply_ui, + unit="m", + dataset_states={}, + ) + + scene._get_runtime_state_async = _get_runtime_state_async + + +def _panel_runtime_state(ui): + """A real RuntimeAppState carrying only the UI record named.""" + return RuntimeAppState.from_components( + ui=ui, + unit="m", + dataset_states={}, + ) + + +# --------------------------------------------------------------------------- +# The coordinator surface +# --------------------------------------------------------------------------- + +def test_panel_coordinators_write_the_store(scene): + """Each of the four coordinators records what it was told. + + One test and not four: the four bodies are the same statement against + four fields, and a swap between two of them fails this test exactly as + visibly as four separate ones would. The four literals are distinct from + the four initial values, so a body that wrote its default rather than its + argument fails here too. + """ + scene.set_panel_top_left_panel_collapsed(PANEL_COLLAPSED) + scene.set_panel_top_right_panel_collapsed(PANEL_COLLAPSED) + scene.set_panel_top_right_legend_collapsed(PANEL_COLLAPSED) + scene.set_panel_top_right_tab_index(PANEL_TAB_INDEX) + + assert scene._panel_top_left_panel_collapsed is True + assert scene._panel_top_right_panel_collapsed is True + assert scene._panel_top_right_legend_collapsed is True + assert scene._panel_top_right_tab_index == 1 + + +def test_set_panel_top_right_tab_index_holds_the_lock_at_the_store_write(scene): + """The lock is *held* at the moment the store is written. + + One of the four and not all four: the precedent pins the lock once per + surface rather than once per method. Remove ``with self._vtk_lock:`` from + the coordinator and the spy records nothing at all -- the value and the + depth both stay ``None`` -- because the lock is never entered. + + The trigger handler runs on trame's daemon thread while this store is read + on the caller's thread by ``get_state``; a write outside the lock would + interleave with the assembly of the record being saved. That failure is + intermittent and never reproduces under a gate. + """ + spy = _PanelStoreLockSpy(scene) + scene._vtk_lock = spy + + scene.set_panel_top_right_tab_index(PANEL_TAB_INDEX) + + assert spy.depth_at_release >= 1 + assert spy.value_at_release == 1 + assert spy.depth == 0 + assert spy.enter_count == spy.exit_count + + +# --------------------------------------------------------------------------- +# get_state -- the UI record comes from the server, not from the reply +# --------------------------------------------------------------------------- + +def test_get_state_takes_the_ui_panel_state_from_the_store(scene): + """The saved panel state is the server's, with the browser saying otherwise. + + This is the assertion that pins the change. The store holds the + hand-written literals ``True, True, True, 1`` while the reply carries + ``False, False, False, 0``; delete the ``runtime_state.ui`` assignment in + get_state and every assertion below reports the reply's value instead. + + Asserted on what get_state RETURNS -- the object that reaches the writer -- + not on the runtime state it was built from. + """ + scene.set_panel_top_left_panel_collapsed(PANEL_COLLAPSED) + scene.set_panel_top_right_panel_collapsed(PANEL_COLLAPSED) + scene.set_panel_top_right_legend_collapsed(PANEL_COLLAPSED) + scene.set_panel_top_right_tab_index(PANEL_TAB_INDEX) + _ui_save_scene( + scene, + _reply_ui( + REPLY_PANEL_COLLAPSED, + REPLY_PANEL_COLLAPSED, + REPLY_PANEL_COLLAPSED, + REPLY_PANEL_TAB_INDEX, + ), + ) + + persisted = asyncio.run(scene.get_state(timeout=1.0)) + + assert persisted.ui.panel_top_left_panel_collapsed is True + assert persisted.ui.panel_top_right_panel_collapsed is True + assert persisted.ui.panel_top_right_legend_collapsed is True + assert persisted.ui.panel_top_right_tab_index == 1 + + +def test_get_state_discards_the_ui_panel_state_the_browser_returned(scene): + """The browser's panel state does not survive into the persisted state. + + The mirror of the test above and its own test for the same reason the + camera and toggle pairs are split: "wrote the store" and "did not write + the reply" are the same only while the round trip still carries a ``ui`` + block at all, and the round trip is not being removed. Here the store is + left at its initial values and the reply carries ``True, True, True, 1``. + + The four expected values are the hand-written literals ``False``, + ``False``, ``False`` and ``0``, not a read of the store: a store + initialised to ``None`` -- the silent failure the delivery decision + turns on -- fails this test on value. + """ + _ui_save_scene( + scene, + _reply_ui( + PANEL_COLLAPSED, + PANEL_COLLAPSED, + PANEL_COLLAPSED, + PANEL_TAB_INDEX, + ), + ) + + persisted = asyncio.run(scene.get_state(timeout=1.0)) + + assert persisted.ui.panel_top_left_panel_collapsed is False + assert persisted.ui.panel_top_right_panel_collapsed is False + assert persisted.ui.panel_top_right_legend_collapsed is False + assert persisted.ui.panel_top_right_tab_index == 0 + + +def test_get_state_takes_dark_theme_from_the_server_not_the_browser(scene): + """The saved theme is ``dark_mode``, with the browser saying otherwise. + + Its own test rather than a fifth assertion above, because it is a + different source: the other four come from the panel store, this one from + the public ``dark_mode`` attribute, and no trigger writes it. The reply + carries the opposite literal, which is what a Dash host prop override + produces in the browser today; before this change that value reached the + file and, on the next load, the server's own ``dark_mode``. + """ + scene.dark_mode = SERVER_DARK_MODE + _ui_save_scene( + scene, + _reply_ui( + REPLY_PANEL_COLLAPSED, + REPLY_PANEL_COLLAPSED, + REPLY_PANEL_COLLAPSED, + REPLY_PANEL_TAB_INDEX, + dark_theme=REPLY_DARK_THEME, + ), + ) + + persisted = asyncio.run(scene.get_state(timeout=1.0)) + + assert persisted.ui.dark_theme is True + + +# --------------------------------------------------------------------------- +# apply_state -- the load path writes the store +# --------------------------------------------------------------------------- + +def test_apply_state_restores_the_ui_panel_state_to_the_store(scene): + """A state carrying panel fields becomes the server's store.""" + _apply( + scene, + _panel_runtime_state( + VisorUIState( + dark_theme=False, + panel_top_left_panel_collapsed=PANEL_COLLAPSED, + panel_top_right_panel_collapsed=PANEL_COLLAPSED, + panel_top_right_legend_collapsed=PANEL_COLLAPSED, + panel_top_right_tab_index=PANEL_TAB_INDEX, + ) + ), + ) + + assert scene._panel_top_left_panel_collapsed is True + assert scene._panel_top_right_panel_collapsed is True + assert scene._panel_top_right_legend_collapsed is True + assert scene._panel_top_right_tab_index == 1 + + +def test_apply_state_leaves_an_absent_ui_panel_field_alone(scene): + """Absent says nothing: a state with no panel fields is not a state of defaults. + + The guard belongs to this path and not to get_state, and this is the test + that says so. The store is seeded through the coordinators first, so a + body that wrote the model's own ``None`` default through would be caught; + a file written before this record existed carries exactly this shape. + """ + scene.set_panel_top_left_panel_collapsed(PANEL_COLLAPSED) + scene.set_panel_top_right_panel_collapsed(PANEL_COLLAPSED) + scene.set_panel_top_right_legend_collapsed(PANEL_COLLAPSED) + scene.set_panel_top_right_tab_index(PANEL_TAB_INDEX) + + _apply(scene, _panel_runtime_state(VisorUIState(dark_theme=False))) + + assert scene._panel_top_left_panel_collapsed is True + assert scene._panel_top_right_panel_collapsed is True + assert scene._panel_top_right_legend_collapsed is True + assert scene._panel_top_right_tab_index == 1 + + diff --git a/tests/unit/vtk/scene/test_local_scene.py b/tests/unit/vtk/scene/test_local_scene.py index f5a01404..3692caf8 100644 --- a/tests/unit/vtk/scene/test_local_scene.py +++ b/tests/unit/vtk/scene/test_local_scene.py @@ -541,6 +541,19 @@ def test_get_scene_details_json_is_valid(pipeline_instance): assert payload["appState"]["scene"]["unit"] == "m" assert payload["appState"]["ui"]["darkTheme"] == pipeline_instance.dark_mode + # The four panel keys, at the client's own mount defaults. The gate for + # the push path: this dump is ``exclude_none=True``, so a store left at + # ``None`` would omit these keys entirely, the client would skip all four + # of its apply branches, and a refresh would show the panels' own + # defaults -- which is indistinguishable from working at a glance. The + # expected values are hand-written literals, so a store initialised to + # ``None`` fails here on presence and a store initialised to the wrong + # values fails on value. + assert payload["appState"]["ui"]["panelTopLeftPanelCollapsed"] is False + assert payload["appState"]["ui"]["panelTopRightPanelCollapsed"] is False + assert payload["appState"]["ui"]["panelTopRightLegendCollapsed"] is False + assert payload["appState"]["ui"]["panelTopRightTabIndex"] == 0 + # Widget IDs are ints, now under rendererAnnotation.widgets widgets = payload["vtkInfo"]["rendererAnnotation"]["widgets"] assert widgets["orientationWidgetId"] == 10 From 4552603499a584f3d0addf9bdaafc5408d54b081 Mon Sep 17 00:00:00 2001 From: Laura Kasian Date: Thu, 24 Sep 2026 15:10:41 -0700 Subject: [PATCH 02/14] feat: panels report collapse and tab changes to the server --- .../visor/visor-client/src/VisorFrontend.tsx | 42 +++ .../components/ui-panels/Panel_TopLeft.tsx | 9 + .../components/ui-panels/Panel_TopRight.tsx | 21 ++ .../src/jest-tests/UiPanelStateSends.test.tsx | 308 ++++++++++++++++++ 4 files changed, 380 insertions(+) create mode 100644 src/ansys/visor/visor-client/src/jest-tests/UiPanelStateSends.test.tsx diff --git a/src/ansys/visor/visor-client/src/VisorFrontend.tsx b/src/ansys/visor/visor-client/src/VisorFrontend.tsx index aa59dcc3..c674ae9f 100644 --- a/src/ansys/visor/visor-client/src/VisorFrontend.tsx +++ b/src/ansys/visor/visor-client/src/VisorFrontend.tsx @@ -215,6 +215,40 @@ export class VisorFrontend { uiScaffoldUtilSet = true; uiScaffoldUtilResolve(uiScaffoldUtil); }; + // UI panel state -- the four send methods below report one panel field + // each to its server trigger. They close over the `triggerSender` + // constructor parameter, which is required and supplied at every + // construction site, so there is no no-transport state to guard. + // + // A rejection is logged under one fixed, greppable prefix and + // swallowed, never rethrown: these run inside synchronous UI handlers + // that behaved a certain way before the call existed, and the client + // applies its own change independently of the report. + const sendUiPanelTriggerAsync = async ( + triggerName: string, + payload: Record + ): Promise => { + try { + await triggerSender(triggerName, payload); + } catch (err) { + console.error(`[VISOR] ui panel trigger send failed: trigger='${triggerName}'`, err); + } + }; + // Each of the four forwards the argument it was given and reads + // nothing back. This is deliberately the opposite of `WasmRenderer`'s + // widget sends, which read their widget back because the toolbar calls + // them with no argument: here the caller is the panel handler that has + // just written the closure, so the argument is the settled value by + // construction, and the util it would be read back from may not exist + // yet. + this.sendPanelTopLeftPanelCollapsedAsync = (collapsed) => + sendUiPanelTriggerAsync('set_panel_top_left_panel_collapsed', { collapsed }); + this.sendPanelTopRightPanelCollapsedAsync = (collapsed) => + sendUiPanelTriggerAsync('set_panel_top_right_panel_collapsed', { collapsed }); + this.sendPanelTopRightLegendCollapsedAsync = (collapsed) => + sendUiPanelTriggerAsync('set_panel_top_right_legend_collapsed', { collapsed }); + this.sendPanelTopRightTabIndexAsync = (tabIndex) => + sendUiPanelTriggerAsync('set_panel_top_right_tab_index', { tabIndex }); this.toggleFullScreenAsync = () => renderer.toggleFullScreenAsync(); this.setEdgeVisibilityAsync = (visible) => renderer.setEdgeVisibilityGlobalAsync(visible); this.setCrossSectionVisibilityAsync = (visible) => @@ -552,6 +586,14 @@ export class VisorFrontend { setPanelTopLeftUtil: (panelTopRightUtil: Panel_TopLeft_Util) => void; setPanelTopRightUtil: (panelTopRightUtil: Panel_TopRight_Util) => void; setUiScaffoldUtil: (uiScaffoldUtil: UiScaffoldUtil) => void; + /** + * Report one panel-layout field to the server. Each carries the absolute + * value it was given for exactly one field; no send reads any other field. + */ + sendPanelTopLeftPanelCollapsedAsync: (collapsed: boolean) => Promise; + sendPanelTopRightPanelCollapsedAsync: (collapsed: boolean) => Promise; + sendPanelTopRightLegendCollapsedAsync: (collapsed: boolean) => Promise; + sendPanelTopRightTabIndexAsync: (tabIndex: number) => Promise; treeViewUtilPromise: Promise>; panelTopLeftUtilPromise: Promise; panelTopRightUtilPromise: Promise; diff --git a/src/ansys/visor/visor-client/src/components/ui-panels/Panel_TopLeft.tsx b/src/ansys/visor/visor-client/src/components/ui-panels/Panel_TopLeft.tsx index 4d4a1a94..b03bef5e 100644 --- a/src/ansys/visor/visor-client/src/components/ui-panels/Panel_TopLeft.tsx +++ b/src/ansys/visor/visor-client/src/components/ui-panels/Panel_TopLeft.tsx @@ -57,12 +57,16 @@ export const Panel_TopLeft: FC<{ 'theme-hover-background-3', ]) as HTMLButtonElement; let isPanelCollapsed = false; + let sendEnabled = false; collapseButton.onclick = (e) => { reactComponentContainer.style.width = `${reactComponentContainer.offsetWidth}px`; treeViewContainer.style.display = 'none'; collapseButton.remove(); collapseButtonContainer.appendChild(expandButton); isPanelCollapsed = true; + if (sendEnabled) { + void visorState.sendPanelTopLeftPanelCollapsedAsync(isPanelCollapsed); + } }; expandButton.onclick = (e) => { reactComponentContainer.style.removeProperty('width'); @@ -70,6 +74,9 @@ export const Panel_TopLeft: FC<{ expandButton.remove(); collapseButtonContainer.appendChild(collapseButton); isPanelCollapsed = false; + if (sendEnabled) { + void visorState.sendPanelTopLeftPanelCollapsedAsync(isPanelCollapsed); + } }; expandButton.click(); visorState.setTreeViewUtil(treeViewUtil.current); @@ -86,6 +93,8 @@ export const Panel_TopLeft: FC<{ } ); visorState.setPanelTopLeftUtil(util); + // Must stay on the line after the util handoff: it suppresses the mount click above and is open before any delivered apply awaiting the util promise can click; scaffolding, removed when delivery is separated from mutation. + sendEnabled = true; onLoad(util); }, []); return ( diff --git a/src/ansys/visor/visor-client/src/components/ui-panels/Panel_TopRight.tsx b/src/ansys/visor/visor-client/src/components/ui-panels/Panel_TopRight.tsx index 42f45b92..d68c4543 100644 --- a/src/ansys/visor/visor-client/src/components/ui-panels/Panel_TopRight.tsx +++ b/src/ansys/visor/visor-client/src/components/ui-panels/Panel_TopRight.tsx @@ -230,6 +230,7 @@ export const Panel_TopRight: FC<{ let collapseLegend: () => void; let expandLegend: () => void; let getIsLegendCollapsed: () => boolean; + let sendEnabled = false; { // set up top right panel collapse/expand buttons @@ -248,6 +249,9 @@ export const Panel_TopRight: FC<{ collapseButton.remove(); collapseButtonContainer.appendChild(expandButton); isCollapsed = true; + if (sendEnabled) { + void visorState.sendPanelTopRightPanelCollapsedAsync(isCollapsed); + } }; expandButton.onclick = (e) => { componentContainer.style.removeProperty('width'); @@ -256,6 +260,9 @@ export const Panel_TopRight: FC<{ expandButton.remove(); collapseButtonContainer.appendChild(collapseButton); isCollapsed = false; + if (sendEnabled) { + void visorState.sendPanelTopRightPanelCollapsedAsync(isCollapsed); + } }; expandButton.click(); collapsePanel = () => { @@ -283,6 +290,9 @@ export const Panel_TopRight: FC<{ collapseButton.remove(); legendCollapseButtonContainer.appendChild(expandButton); isCollapsed = true; + if (sendEnabled) { + void visorState.sendPanelTopRightLegendCollapsedAsync(isCollapsed); + } }; expandButton.onclick = (e) => { legendOverlayElem.style.removeProperty('width'); @@ -290,6 +300,9 @@ export const Panel_TopRight: FC<{ expandButton.remove(); legendCollapseButtonContainer.appendChild(collapseButton); isCollapsed = false; + if (sendEnabled) { + void visorState.sendPanelTopRightLegendCollapsedAsync(isCollapsed); + } }; expandButton.click(); collapseLegend = () => { @@ -317,6 +330,9 @@ export const Panel_TopRight: FC<{ legendPanelElem.style.display = 'none'; legendTabTextContainer.style.display = 'none'; tabIndex = 0; + if (sendEnabled) { + void visorState.sendPanelTopRightTabIndexAsync(tabIndex); + } }; legendTabElem.onclick = () => { propertyTabElem.classList.remove('theme-background-1'); @@ -326,6 +342,9 @@ export const Panel_TopRight: FC<{ legendPanelElem.style.removeProperty('display'); legendTabTextContainer.style.removeProperty('display'); tabIndex = 1; + if (sendEnabled) { + void visorState.sendPanelTopRightTabIndexAsync(tabIndex); + } }; propertyTabElem.onclick(null!); @@ -472,6 +491,8 @@ export const Panel_TopRight: FC<{ () => tabIndex ); visorState.setPanelTopRightUtil(util); + // Must stay on the line after the util handoff: it suppresses the three mount writes above and is open before any delivered apply awaiting the util promise can click; scaffolding, removed when delivery is separated from mutation. + sendEnabled = true; onLoad(util); })(); diff --git a/src/ansys/visor/visor-client/src/jest-tests/UiPanelStateSends.test.tsx b/src/ansys/visor/visor-client/src/jest-tests/UiPanelStateSends.test.tsx new file mode 100644 index 00000000..d07b42ed --- /dev/null +++ b/src/ansys/visor/visor-client/src/jest-tests/UiPanelStateSends.test.tsx @@ -0,0 +1,308 @@ +import { render, fireEvent, act } from '@testing-library/react'; +import { VisorFrontend } from '../VisorFrontend'; +import { Panel_TopLeft } from '../components/ui-panels/Panel_TopLeft'; +import { Panel_TopRight } from '../components/ui-panels/Panel_TopRight'; +import { CreateVisorSceneGraph, VisorSceneNodeExtended } from '../state/VisorSceneGraph.tsx'; +import type { IRenderer } from '../renderer/IRenderer'; +import type VisorVtkSceneNode from '../state/appstate/vtkInfo/VisorVtkSceneNode.tsx'; +import type { Panel_TopLeft_Util } from '../components/ui-panels/Panel_TopLeft_Util.tsx'; +import type { Panel_TopRight_Util } from '../components/ui-panels/Panel_TopRight_Util.tsx'; + +/** + * The client half of server-owned UI panel state: the four `VisorFrontend` + * send methods, and the per-panel mount gate that keeps a freshly mounted + * panel silent. + * + * Two subjects, and they fail differently. + * + * 1. Each send method puts its own trigger name and the value it was given + * on the wire. A name mismatched by one character between the two stacks + * routes nowhere on the server, logs nothing and fails nothing, so the + * name strings are pinned here as hand-written literals -- as is every + * expected payload. + * + * 2. Every rebuild remounts every panel, and each mount runs + * `expandButton.click()` (both panels, twice in the top-right) and + * `propertyTabElem.onclick(null!)`. Those writes run *before* the util is + * registered. Were the gate open there, every refresh and every rebuild + * would report the client's mount defaults over the record the server + * just delivered, and a save in that window would record them -- correct + * under every other test, and wrong in the running application only. + * Tests 5 and 6 pin the gate closed at mount for each panel separately, + * because the two gate differently: the top-left gate is purely + * synchronous, the top-right gate spans two awaits. + * + * Tests 5 and 6 cannot see a gate that never opens -- a panel that reports + * nothing at mount and nothing afterwards passes both. Test 7 is that pin, + * on the top-right panel, whose gate is the one behind the awaits. + * + * jsdom has no `ResizeObserver` and jest here runs with no `setupFiles` + * (`jest.config.cjs`), so the stub below is this module's own. + */ + +class ResizeObserverStub { + observe(): void {} + unobserve(): void {} + disconnect(): void {} +} +(globalThis as unknown as { ResizeObserver: unknown }).ResizeObserver = ResizeObserverStub; + +/** Hand-written literals. Every expected payload below is built from these. */ +const TOP_LEFT_PANEL_COLLAPSED = true; +const TOP_RIGHT_PANEL_COLLAPSED = true; +const TOP_RIGHT_LEGEND_COLLAPSED = false; +const TOP_RIGHT_TAB_INDEX = 1; + +/** + * The minimum root scene-graph node `CreateVisorSceneGraph` accepts, which + * `VisorFrontend`'s constructor builds the live graph from. Same shape + * `AppStateProjectionLoad.test.ts` uses, and no parts: nothing here reads one. + */ +function makeSceneGraphNode() { + return { + id: 0, + dataArrays: [], + name: '', + isGroupNode: true, + isActorNode: false, + nodeType: 'root', + diffuseColor: [1, 1, 1], + bounds: [], + children: [], + }; +} + +/** + * Every renderer member `VisorFrontend`'s constructor reaches. The sends are + * the subject; these are here so construction completes. + */ +function makeRendererDouble() { + return { + attachSceneGraph: jest.fn(), + domElement: document.createElement('div'), + addCameraSettledListener: jest.fn(() => jest.fn()), + addViewerClickedListener: jest.fn(() => jest.fn()), + }; +} + +/** + * A real `VisorFrontend` over a sender double. Real, not a double: the trigger + * name strings these tests pin are in the frontend's own source, and a double + * would restate them instead of reading them. + */ +function makeFrontend(): { + frontend: VisorFrontend; + triggerSender: jest.Mock, [string, unknown]>; +} { + const triggerSender = jest.fn(async () => undefined) as unknown as jest.Mock< + Promise, + [string, unknown] + >; + const frontend = new VisorFrontend( + makeRendererDouble() as unknown as IRenderer, + makeSceneGraphNode() as unknown as VisorVtkSceneNode, + triggerSender + ); + return { frontend, triggerSender }; +} + +/** The four sends, as jest mocks, for the two panels to call. */ +function makeSendDoubles() { + return { + sendPanelTopLeftPanelCollapsedAsync: jest.fn(async () => undefined), + sendPanelTopRightPanelCollapsedAsync: jest.fn(async () => undefined), + sendPanelTopRightLegendCollapsedAsync: jest.fn(async () => undefined), + sendPanelTopRightTabIndexAsync: jest.fn(async () => undefined), + }; +} + +/** A real scene graph, which `Panel_TopLeft` hands to its `TreeView`. */ +function makeSceneGraph(): VisorSceneNodeExtended { + return CreateVisorSceneGraph({ + id: 0, + dataArrays: [], + name: '', + isGroupNode: true, + isActorNode: false, + nodeType: 'root', + diffuseColor: [1, 1, 1], + bounds: [], + children: [ + { + id: 1, + dataArrays: [], + name: 'some-polydata-file.vtp', + isGroupNode: false, + isActorNode: true, + nodeType: 'vtkUnstructuredGrid', + diffuseColor: [1, 1, 1], + bounds: [], + children: [], + }, + ], + }); +} + +/** + * A `VisorFrontend` stand-in carrying only what `Panel_TopLeft`'s effect + * reaches, plus a promise that settles when the panel hands over its util -- + * which is the moment the gate is expected to open. + */ +function makeTopLeftFrontendDouble() { + const sends = makeSendDoubles(); + let registered: (util: Panel_TopLeft_Util) => void = () => {}; + const utilRegistered = new Promise((resolve) => { + registered = resolve; + }); + const visorState = { + ...sends, + sceneGraph: makeSceneGraph(), + render: jest.fn(async () => undefined), + setTreeViewUtil: jest.fn(), + setPanelTopLeftUtil: jest.fn((util: Panel_TopLeft_Util) => registered(util)), + }; + return { visorState, sends, utilRegistered }; +} + +/** + * The same for `Panel_TopRight`, whose effect additionally awaits the tree-view + * util and runs a selection pass before it registers. + */ +function makeTopRightFrontendDouble() { + const sends = makeSendDoubles(); + let registered: (util: Panel_TopRight_Util) => void = () => {}; + const utilRegistered = new Promise((resolve) => { + registered = resolve; + }); + const treeViewUtil = { + addSelectionChangeListener: jest.fn(() => jest.fn()), + rows: [], + rowUtilsMap: new Map(), + selectedNodes: [], + updateSelectedNodesArray: jest.fn(), + }; + const visorState = { + ...sends, + treeViewUtilPromise: Promise.resolve(treeViewUtil), + addViewerClickedListener: jest.fn(() => jest.fn()), + addSelectionModeChangedListener: jest.fn(() => jest.fn()), + getSelectionMode: jest.fn(() => 'part' as const), + render: jest.fn(async () => undefined), + setSpectrumRangeAsync: jest.fn(async () => undefined), + setPanelTopRightUtil: jest.fn((util: Panel_TopRight_Util) => registered(util)), + }; + return { visorState, sends, utilRegistered }; +} + +describe('VisorFrontend UI panel send surface', () => { + test('sendPanelTopLeftPanelCollapsedAsync sends set_panel_top_left_panel_collapsed with the value it was given', async () => { + const { frontend, triggerSender } = makeFrontend(); + + await frontend.sendPanelTopLeftPanelCollapsedAsync(TOP_LEFT_PANEL_COLLAPSED); + + expect(triggerSender).toHaveBeenCalledTimes(1); + expect(triggerSender).toHaveBeenCalledWith('set_panel_top_left_panel_collapsed', { + collapsed: true, + }); + }); + + test('sendPanelTopRightPanelCollapsedAsync sends set_panel_top_right_panel_collapsed with the value it was given', async () => { + const { frontend, triggerSender } = makeFrontend(); + + await frontend.sendPanelTopRightPanelCollapsedAsync(TOP_RIGHT_PANEL_COLLAPSED); + + expect(triggerSender).toHaveBeenCalledTimes(1); + expect(triggerSender).toHaveBeenCalledWith('set_panel_top_right_panel_collapsed', { + collapsed: true, + }); + }); + + test('sendPanelTopRightLegendCollapsedAsync sends set_panel_top_right_legend_collapsed with the value it was given', async () => { + const { frontend, triggerSender } = makeFrontend(); + + await frontend.sendPanelTopRightLegendCollapsedAsync(TOP_RIGHT_LEGEND_COLLAPSED); + + expect(triggerSender).toHaveBeenCalledTimes(1); + expect(triggerSender).toHaveBeenCalledWith('set_panel_top_right_legend_collapsed', { + collapsed: false, + }); + }); + + test('sendPanelTopRightTabIndexAsync sends set_panel_top_right_tab_index with the value it was given', async () => { + const { frontend, triggerSender } = makeFrontend(); + + await frontend.sendPanelTopRightTabIndexAsync(TOP_RIGHT_TAB_INDEX); + + expect(triggerSender).toHaveBeenCalledTimes(1); + expect(triggerSender).toHaveBeenCalledWith('set_panel_top_right_tab_index', { + tabIndex: 1, + }); + }); +}); + +describe('the panel mount gate', () => { + test('Panel_TopLeft sends nothing while mounting', async () => { + const { visorState, sends, utilRegistered } = makeTopLeftFrontendDouble(); + + await act(async () => { + render( + {}} + /> + ); + }); + await utilRegistered; + + // The mount click has run and the util has been handed over, so the + // gate is open now -- and nothing was reported on the way there. + expect(visorState.setPanelTopLeftUtil).toHaveBeenCalledTimes(1); + expect(sends.sendPanelTopLeftPanelCollapsedAsync).not.toHaveBeenCalled(); + }); + + test('Panel_TopRight sends nothing while mounting', async () => { + const { visorState, sends, utilRegistered } = makeTopRightFrontendDouble(); + + await act(async () => { + render( + {}} + /> + ); + }); + await utilRegistered; + + // Three mount writes here, not one: both blocks' expand clicks and the + // tab call, all before the registration this has now awaited. + expect(visorState.setPanelTopRightUtil).toHaveBeenCalledTimes(1); + expect(sends.sendPanelTopRightPanelCollapsedAsync).not.toHaveBeenCalled(); + expect(sends.sendPanelTopRightLegendCollapsedAsync).not.toHaveBeenCalled(); + expect(sends.sendPanelTopRightTabIndexAsync).not.toHaveBeenCalled(); + }); + + test('a click on a Panel_TopRight control after its util is registered sends exactly once', async () => { + const { visorState, sends, utilRegistered } = makeTopRightFrontendDouble(); + const { container } = render( + {}} /> + ); + await act(async () => { + await utilRegistered; + }); + + // The panel's collapse button, located by structure: the ids in this + // component are `randomId()`-generated, and the collapse-button cell is + // the only `td.shrink.padding-all` in it. The legend's own collapse + // button lives in a plain `td.shrink` inside the legend overlay. + const collapseButton = container.querySelector( + 'td.shrink.padding-all > button' + ) as HTMLButtonElement; + expect(collapseButton).not.toBeNull(); + + fireEvent.click(collapseButton); + + expect(sends.sendPanelTopRightPanelCollapsedAsync).toHaveBeenCalledTimes(1); + expect(sends.sendPanelTopRightPanelCollapsedAsync).toHaveBeenCalledWith(true); + }); +}); + From b8db83798dd0e6a9fca7d0521a10a459ed1ddf15 Mon Sep 17 00:00:00 2001 From: Laura Kasian Date: Fri, 25 Sep 2026 08:52:57 -0700 Subject: [PATCH 03/14] fix: properties panel shows a part selected before it mounts --- .../components/ui-panels/Panel_TopRight.tsx | 9 +- .../src/jest-tests/UiPanelStateSends.test.tsx | 124 ++++++++++++++---- 2 files changed, 105 insertions(+), 28 deletions(-) diff --git a/src/ansys/visor/visor-client/src/components/ui-panels/Panel_TopRight.tsx b/src/ansys/visor/visor-client/src/components/ui-panels/Panel_TopRight.tsx index d68c4543..d92e41cd 100644 --- a/src/ansys/visor/visor-client/src/components/ui-panels/Panel_TopRight.tsx +++ b/src/ansys/visor/visor-client/src/components/ui-panels/Panel_TopRight.tsx @@ -473,7 +473,14 @@ export const Panel_TopRight: FC<{ await visorState.render(); }); - await onSelectionChangeAsync([]); + // Initialize from the tree's current selection, not from an empty + // list: a selection delivered before this panel mounted -- a + // refresh, a rebuild -- is already on the rows and on the mesh, and + // `synchronize()` reaches it without running the selection-change + // listeners, so nothing else would ever hand it to this panel. + // Read here, after the util promise settled, so the array is the + // live one the listener above is subscribed to. + await onSelectionChangeAsync(treeViewUtil.selectedNodes); const util = new Panel_TopRight_Util( expandPanel, collapsePanel, diff --git a/src/ansys/visor/visor-client/src/jest-tests/UiPanelStateSends.test.tsx b/src/ansys/visor/visor-client/src/jest-tests/UiPanelStateSends.test.tsx index d07b42ed..3817945c 100644 --- a/src/ansys/visor/visor-client/src/jest-tests/UiPanelStateSends.test.tsx +++ b/src/ansys/visor/visor-client/src/jest-tests/UiPanelStateSends.test.tsx @@ -36,6 +36,13 @@ import type { Panel_TopRight_Util } from '../components/ui-panels/Panel_TopRight * nothing at mount and nothing afterwards passes both. Test 7 is that pin, * on the top-right panel, whose gate is the one behind the awaits. * + * Test 8 is a third subject and not a send at all: the top-right panel + * initializes its properties panel from the tree view's current selection, + * rather than from an empty list. A selection delivered before the panel + * mounts reaches the rows and the mesh through `TreeViewUtil.synchronize()`, + * which runs no selection-change listener, so an empty-list initialization + * leaves the properties panel blank until the user clicks a part. + * * jsdom has no `ResizeObserver` and jest here runs with no `setupFiles` * (`jest.config.cjs`), so the stub below is this module's own. */ @@ -116,31 +123,54 @@ function makeSendDoubles() { }; } -/** A real scene graph, which `Panel_TopLeft` hands to its `TreeView`. */ -function makeSceneGraph(): VisorSceneNodeExtended { - return CreateVisorSceneGraph({ - id: 0, - dataArrays: [], - name: '', - isGroupNode: true, - isActorNode: false, - nodeType: 'root', - diffuseColor: [1, 1, 1], - bounds: [], - children: [ - { - id: 1, - dataArrays: [], - name: 'some-polydata-file.vtp', - isGroupNode: false, - isActorNode: true, - nodeType: 'vtkUnstructuredGrid', - diffuseColor: [1, 1, 1], - bounds: [], - children: [], - }, - ], - }); +/** + * A real scene graph, which `Panel_TopLeft` hands to its `TreeView`. + * + * The renderer is optional and defaults to absent, which is what the two + * mount-gate tests want: they never reach a node method. A test whose selection + * is non-empty does reach one -- the properties panel clears the colour + * variable on a part with no data arrays -- and supplies the double below. + */ +function makeSceneGraph(renderer?: IRenderer): VisorSceneNodeExtended { + return CreateVisorSceneGraph( + { + id: 0, + dataArrays: [], + name: '', + isGroupNode: true, + isActorNode: false, + nodeType: 'root', + diffuseColor: [1, 1, 1], + bounds: [], + children: [ + { + id: 1, + dataArrays: [], + name: 'some-polydata-file.vtp', + isGroupNode: false, + isActorNode: true, + nodeType: 'vtkUnstructuredGrid', + diffuseColor: [1, 1, 1], + bounds: [], + children: [], + }, + ], + }, + undefined, + renderer + ); +} + +/** + * The renderer members a selected part's own methods reach while the + * properties panel initializes. Nothing here is asserted on: the subject is + * what the panel shows, and these exist so the node's calls complete. + */ +function makeNodeRendererDouble(): IRenderer { + return { + clearColorVariableAsync: jest.fn(async () => undefined), + sendClearPartColorVariableAsync: jest.fn(async () => undefined), + } as unknown as IRenderer; } /** @@ -167,8 +197,12 @@ function makeTopLeftFrontendDouble() { /** * The same for `Panel_TopRight`, whose effect additionally awaits the tree-view * util and runs a selection pass before it registers. + * + * `selectedNodes` is what the tree view util holds when the panel's effect + * reaches it. It defaults to empty -- the state a tree with nothing selected is + * in -- so the two mount-gate tests below are unaffected by its presence. */ -function makeTopRightFrontendDouble() { +function makeTopRightFrontendDouble(selectedNodes: VisorSceneNodeExtended[] = []) { const sends = makeSendDoubles(); let registered: (util: Panel_TopRight_Util) => void = () => {}; const utilRegistered = new Promise((resolve) => { @@ -178,7 +212,7 @@ function makeTopRightFrontendDouble() { addSelectionChangeListener: jest.fn(() => jest.fn()), rows: [], rowUtilsMap: new Map(), - selectedNodes: [], + selectedNodes, updateSelectedNodesArray: jest.fn(), }; const visorState = { @@ -306,3 +340,39 @@ describe('the panel mount gate', () => { }); }); +describe('the properties panel at mount', () => { + test('Panel_TopRight shows the tree view\'s current selection at mount', async () => { + // The selection is already on the tree view util before the panel + // mounts, which is what a refresh or a rebuild leaves behind: the rows + // and the mesh carry it, and `synchronize()` puts it in this array + // without running any selection-change listener. Nothing in this test + // clicks anything, and nothing fires a selection event -- the handler + // the panel registers with `addSelectionChangeListener` is captured by + // the double and never invoked here. + const sceneGraph = makeSceneGraph(makeNodeRendererDouble()); + const part = sceneGraph.children[0]; + const { visorState, utilRegistered } = makeTopRightFrontendDouble([part]); + + let container: HTMLElement = null!; + await act(async () => { + container = render( + {}} + /> + ).container; + }); + await utilRegistered; + + // The name field, located by structure: the ids in this component are + // `randomId()`-generated, and this is the component's only read-only + // text input. It lives inside the property panel's body, which the + // no-selection branch hides. + const nameInput = container.querySelector( + 'input[type="text"][readonly]' + ) as HTMLInputElement; + expect(nameInput).not.toBeNull(); + expect(nameInput.value).toBe('some-polydata-file.vtp'); + }); +}); + From b048cf5503fbd17877fcebea3b11cffcdcb666f8 Mon Sep 17 00:00:00 2001 From: pyansys-ci-bot <92810346+pyansys-ci-bot@users.noreply.github.com> Date: Fri, 25 Sep 2026 16:53:16 +0000 Subject: [PATCH 04/14] chore: adding changelog file 144.added.md [dependabot-skip] --- doc/changelog.d/144.added.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 doc/changelog.d/144.added.md diff --git a/doc/changelog.d/144.added.md b/doc/changelog.d/144.added.md new file mode 100644 index 00000000..bcd851f4 --- /dev/null +++ b/doc/changelog.d/144.added.md @@ -0,0 +1 @@ +[Remote rendering 3.4] server-owned UI panel and theme state From 6edb99049324b5eeed9b3d7337133e3a28c73cc5 Mon Sep 17 00:00:00 2001 From: Laura Kasian Date: Fri, 25 Sep 2026 09:54:07 -0700 Subject: [PATCH 05/14] fix: prettier fixes --- src/ansys/visor/visor-client/src/VisorFrontend.tsx | 5 ++++- .../visor-client/src/jest-tests/UiPanelStateSends.test.tsx | 3 +-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/ansys/visor/visor-client/src/VisorFrontend.tsx b/src/ansys/visor/visor-client/src/VisorFrontend.tsx index c674ae9f..f9b6e042 100644 --- a/src/ansys/visor/visor-client/src/VisorFrontend.tsx +++ b/src/ansys/visor/visor-client/src/VisorFrontend.tsx @@ -231,7 +231,10 @@ export class VisorFrontend { try { await triggerSender(triggerName, payload); } catch (err) { - console.error(`[VISOR] ui panel trigger send failed: trigger='${triggerName}'`, err); + console.error( + `[VISOR] ui panel trigger send failed: trigger='${triggerName}'`, + err + ); } }; // Each of the four forwards the argument it was given and reads diff --git a/src/ansys/visor/visor-client/src/jest-tests/UiPanelStateSends.test.tsx b/src/ansys/visor/visor-client/src/jest-tests/UiPanelStateSends.test.tsx index 3817945c..8f4d4e1f 100644 --- a/src/ansys/visor/visor-client/src/jest-tests/UiPanelStateSends.test.tsx +++ b/src/ansys/visor/visor-client/src/jest-tests/UiPanelStateSends.test.tsx @@ -341,7 +341,7 @@ describe('the panel mount gate', () => { }); describe('the properties panel at mount', () => { - test('Panel_TopRight shows the tree view\'s current selection at mount', async () => { + test("Panel_TopRight shows the tree view's current selection at mount", async () => { // The selection is already on the tree view util before the panel // mounts, which is what a refresh or a rebuild leaves behind: the rows // and the mesh carry it, and `synchronize()` puts it in this array @@ -375,4 +375,3 @@ describe('the properties panel at mount', () => { expect(nameInput.value).toBe('some-polydata-file.vtp'); }); }); - From 01209895adb574cf37d0b3ae4ed87f8466c31927 Mon Sep 17 00:00:00 2001 From: Laura Kasian Date: Fri, 25 Sep 2026 11:05:05 -0700 Subject: [PATCH 06/14] refactor: store ui model on scene class instead of individual properties --- src/ansys/visor/viewer/vtk/scene/base.py | 142 +++++++++++++---------- tests/unit/vtk/scene/test_base.py | 78 ++++++++++--- 2 files changed, 148 insertions(+), 72 deletions(-) diff --git a/src/ansys/visor/viewer/vtk/scene/base.py b/src/ansys/visor/viewer/vtk/scene/base.py index 1e5ce173..7e099bb4 100644 --- a/src/ansys/visor/viewer/vtk/scene/base.py +++ b/src/ansys/visor/viewer/vtk/scene/base.py @@ -68,10 +68,7 @@ class VisorSceneBase(ABC): _cross_section_enabled: bool _edges_enabled: bool _bounding_box_enabled: bool - _panel_top_left_panel_collapsed: bool - _panel_top_right_panel_collapsed: bool - _panel_top_right_legend_collapsed: bool - _panel_top_right_tab_index: int + _ui_state: VisorUIState def __init__( self, @@ -81,7 +78,36 @@ def __init__( ): """Initialize the scene coordinator and its local renderer backend.""" logger.debug("Initializing %s", type(self).__name__) - self.dark_mode: bool = dark_mode + + # The server's UI record, held as its model rather than as loose + # scalars -- the same shape the camera record is held in. Built first, + # because ``dark_mode`` is a property over its ``dark_theme`` field and + # every later read or write of the theme goes through it. + # + # The theme is the constructor's; the four panel fields are initialised + # to the client panels' own mount defaults -- Panel_TopLeft's + # ``isPanelCollapsed`` and both of Panel_TopRight's ``isCollapsed`` + # closures start false and are mount-clicked to false, and its + # ``tabIndex`` starts at 0. Initialised rather than left unset for two + # reasons: a get_state before the client has ever spoken then reports + # what the client would report, and the record is delivered through a + # dump that excludes ``None``, so a field left unset here is simply + # omitted from the payload and the client shows its own defaults + # instead of the server's record. + # + # Written only by the four panel triggers and by the load path. + # Absolute values, never toggles. There is still exactly one copy of + # the theme: ``dark_theme`` is it, and ``dark_mode`` is the property + # over it, not a second field -- a second copy is the divergence + # set_projection refuses. Projection is deliberately absent for the + # same reason: it is derived from the camera record. + self._ui_state = VisorUIState( + dark_theme=dark_mode, + panel_top_left_panel_collapsed=False, + panel_top_right_panel_collapsed=False, + panel_top_right_legend_collapsed=False, + panel_top_right_tab_index=0, + ) # Server-tracked widget toggles. Absolute values, never toggles. # Initialised to the client widgets' own constructor defaults so a @@ -93,21 +119,6 @@ def __init__( self._edges_enabled: bool = False self._bounding_box_enabled: bool = False - # Server-tracked UI panel layout. Absolute values, never toggles, and - # written only by the four panel triggers. Initialised to the client - # panels' own mount defaults -- Panel_TopLeft's ``isPanelCollapsed`` - # and both of Panel_TopRight's ``isCollapsed`` closures start false and - # are mount-clicked to false, and its ``tabIndex`` starts at 0 -- for - # the reason the toggle store gives above, and for a second one: these - # are delivered through a dump that excludes ``None``, so a field left - # unset here is simply omitted from the payload and the client shows - # its own defaults instead of the server's record. There is - # deliberately no ``_dark_theme`` field: the theme is ``dark_mode``, - # and a second copy of it is the divergence set_projection refuses. - self._panel_top_left_panel_collapsed: bool = False - self._panel_top_right_panel_collapsed: bool = False - self._panel_top_right_legend_collapsed: bool = False - self._panel_top_right_tab_index: int = 0 self._server = server self._scene_graph = None @@ -172,6 +183,31 @@ def dataset_count(self) -> int: """Number of datasets registered in the scene.""" return self._dataset_registry.count + @property + def dark_mode(self) -> bool | None: + """Whether the viewer is in dark theme. + + A property over the UI record's ``dark_theme`` field rather than an + attribute of its own: the theme has exactly one holder, and a second + copy of it is the divergence :meth:`set_projection` refuses. Every + existing reader and writer of ``scene.dark_mode`` -- the construction + sites, the scene-details read, and :meth:`apply_state`'s own line -- + goes through this pair unchanged. + + Typed as the record's field is, ``bool | None`` rather than ``bool``: + the constructor always supplies a ``bool``, but :meth:`apply_state` + writes ``state.ui.dark_theme`` through unguarded, and that field is + optional. The previous plain attribute was annotated ``bool`` and + took the same value; the annotation is what changes here, not what + can arrive. + """ + return self._ui_state.dark_theme + + @dark_mode.setter + def dark_mode(self, dark_mode: bool | None) -> None: + """Set the theme on the UI record.""" + self._ui_state.dark_theme = dark_mode + @property def datasets(self) -> dict[int, VisorDataset]: """List of datasets registered in the scene.""" @@ -200,10 +236,15 @@ async def get_state(self, timeout: float) -> PersistedViewerStateV1: reply is consulted for none of the three. The UI record is the fourth. ``runtime_state.ui`` is replaced - wholesale with this object's own record -- the theme from - ``dark_mode`` and the four panel-layout fields from the store the - panel triggers write -- so the browser's ``ui`` block is discarded - entire. The assignment is unconditional, as the camera and toggle + wholesale with a copy of this object's own record -- the theme in its + ``dark_theme`` field and the four panel-layout fields the panel + triggers write -- so the browser's ``ui`` block is discarded + entire. A copy and never the instance: the record is handed to the + mapper and survives by identity onto the object the writer receives, + so passing the instance would let a panel trigger landing after this + read mutate the state being saved. That is the registry snapshot's + reason, applied to the one other live object that leaves here. The + assignment is unconditional, as the camera and toggle assignments are, and it is what makes the saved theme the server's rather than the embedding host's: under Dash the host prop overrides the delivered theme in the browser, so a reply that was trusted here @@ -246,7 +287,7 @@ async def get_state(self, timeout: float) -> PersistedViewerStateV1: runtime_state.scene.cross_section_enabled = self._cross_section_enabled runtime_state.scene.edges_enabled = self._edges_enabled runtime_state.scene.bounding_box_enabled = self._bounding_box_enabled - runtime_state.ui = self._build_ui_state() + runtime_state.ui = self._ui_state.model_copy() runtime_state.scene.dataset_states = registry_dataset_states persisted = self._state_mapper.runtime_to_persisted(runtime_state) @@ -317,10 +358,14 @@ def get_scene_details(self) -> VisorSceneDetails: "nothing was ever written." No ``_vtk_lock``: the reads here (three booleans, one camera field, - and the five the UI record is assembled from) aren't consumed as a - mutually consistent snapshot, and locking a request-path read against - the trigger thread belongs with the round-trip/thread-affinity work, - not here. Accepted exposure: one stale field in a delivered payload. + and the UI record's five) aren't consumed as a mutually consistent + snapshot, and locking a request-path read against the trigger thread + belongs with the round-trip/thread-affinity work, not here. Accepted + exposure: one stale field in a delivered payload. + + The UI record is passed as a copy and never as the instance, for the + reason :meth:`get_state` gives: a panel trigger landing after this + call would otherwise mutate a payload already served. """ if self._scene_graph is None: self._initialize_scene_graph() @@ -328,7 +373,7 @@ def get_scene_details(self) -> VisorSceneDetails: scene_graph_state = self._build_scene_graph_state() camera_record = self._renderer.get_camera_state() return VisorSceneDetails.from_components( - ui=self._build_ui_state(), + ui=self._ui_state.model_copy(), unit=self._dataset_registry.unit, dataset_states=self._dataset_registry.runtime_state_dict, scene_graph_state=scene_graph_state, @@ -797,17 +842,17 @@ def set_projection(self, parallel: bool) -> None: def set_panel_top_left_panel_collapsed(self, collapsed: bool) -> None: """Record whether the top-left panel is collapsed.""" with self._vtk_lock: - self._panel_top_left_panel_collapsed = collapsed + self._ui_state.panel_top_left_panel_collapsed = collapsed def set_panel_top_right_panel_collapsed(self, collapsed: bool) -> None: """Record whether the top-right panel is collapsed.""" with self._vtk_lock: - self._panel_top_right_panel_collapsed = collapsed + self._ui_state.panel_top_right_panel_collapsed = collapsed def set_panel_top_right_legend_collapsed(self, collapsed: bool) -> None: """Record whether the top-right legend overlay is collapsed.""" with self._vtk_lock: - self._panel_top_right_legend_collapsed = collapsed + self._ui_state.panel_top_right_legend_collapsed = collapsed def set_panel_top_right_tab_index(self, tab_index: int) -> None: """Record which top-right tab is active. @@ -817,7 +862,7 @@ def set_panel_top_right_tab_index(self, tab_index: int) -> None: the range guard lives. """ with self._vtk_lock: - self._panel_top_right_tab_index = tab_index + self._ui_state.panel_top_right_tab_index = tab_index def _restore_part_states(self, runtime_app_state: "RuntimeAppState") -> None: """ @@ -930,13 +975,13 @@ def _restore_ui_state(self, runtime_app_state: "RuntimeAppState") -> None: """ ui = runtime_app_state.ui if ui.panel_top_left_panel_collapsed is not None: - self._panel_top_left_panel_collapsed = ui.panel_top_left_panel_collapsed + self._ui_state.panel_top_left_panel_collapsed = ui.panel_top_left_panel_collapsed if ui.panel_top_right_panel_collapsed is not None: - self._panel_top_right_panel_collapsed = ui.panel_top_right_panel_collapsed + self._ui_state.panel_top_right_panel_collapsed = ui.panel_top_right_panel_collapsed if ui.panel_top_right_legend_collapsed is not None: - self._panel_top_right_legend_collapsed = ui.panel_top_right_legend_collapsed + self._ui_state.panel_top_right_legend_collapsed = ui.panel_top_right_legend_collapsed if ui.panel_top_right_tab_index is not None: - self._panel_top_right_tab_index = ui.panel_top_right_tab_index + self._ui_state.panel_top_right_tab_index = ui.panel_top_right_tab_index def _restore_one_part_state( self, @@ -1122,27 +1167,6 @@ def _update_actor_count(self): """Update the bounding box widget with the current part-node count.""" self._renderer.update_actor_count(self._scene_graph.descendant_part_count()) - def _build_ui_state(self) -> VisorUIState: - """Assemble the server's UI record: the theme and the four panel fields. - - Every field is concrete by construction -- ``dark_mode`` is a ``bool`` - and the four panel attributes are initialised to the client's mount - defaults -- so the record never carries a ``None``. That matters on - the way out: the scene-details dump excludes ``None``, so an unset - field would be omitted from the payload and the client would fall back - to its own defaults rather than the server's record. - - No lock of its own. A caller that needs a mutually consistent - snapshot holds ``_vtk_lock`` around the call; :meth:`get_state` does, - :meth:`get_scene_details` deliberately does not. - """ - return VisorUIState( - dark_theme=self.dark_mode, - panel_top_left_panel_collapsed=self._panel_top_left_panel_collapsed, - panel_top_right_panel_collapsed=self._panel_top_right_panel_collapsed, - panel_top_right_legend_collapsed=self._panel_top_right_legend_collapsed, - panel_top_right_tab_index=self._panel_top_right_tab_index, - ) def _build_scene_graph_state(self): """Assemble the pure scene-graph ``SceneGraphNodeInfo`` tree. diff --git a/tests/unit/vtk/scene/test_base.py b/tests/unit/vtk/scene/test_base.py index c6fa3933..6ef1acef 100644 --- a/tests/unit/vtk/scene/test_base.py +++ b/tests/unit/vtk/scene/test_base.py @@ -2936,7 +2936,7 @@ def __init__(self, scene): self.depth_at_release = None def __exit__(self, exc_type, exc, tb): - self.value_at_release = self._scene._panel_top_right_tab_index + self.value_at_release = self._scene._ui_state.panel_top_right_tab_index self.depth_at_release = self.depth return super().__exit__(exc_type, exc, tb) @@ -3004,10 +3004,10 @@ def test_panel_coordinators_write_the_store(scene): scene.set_panel_top_right_legend_collapsed(PANEL_COLLAPSED) scene.set_panel_top_right_tab_index(PANEL_TAB_INDEX) - assert scene._panel_top_left_panel_collapsed is True - assert scene._panel_top_right_panel_collapsed is True - assert scene._panel_top_right_legend_collapsed is True - assert scene._panel_top_right_tab_index == 1 + assert scene._ui_state.panel_top_left_panel_collapsed is True + assert scene._ui_state.panel_top_right_panel_collapsed is True + assert scene._ui_state.panel_top_right_legend_collapsed is True + assert scene._ui_state.panel_top_right_tab_index == 1 def test_set_panel_top_right_tab_index_holds_the_lock_at_the_store_write(scene): @@ -3149,10 +3149,10 @@ def test_apply_state_restores_the_ui_panel_state_to_the_store(scene): ), ) - assert scene._panel_top_left_panel_collapsed is True - assert scene._panel_top_right_panel_collapsed is True - assert scene._panel_top_right_legend_collapsed is True - assert scene._panel_top_right_tab_index == 1 + assert scene._ui_state.panel_top_left_panel_collapsed is True + assert scene._ui_state.panel_top_right_panel_collapsed is True + assert scene._ui_state.panel_top_right_legend_collapsed is True + assert scene._ui_state.panel_top_right_tab_index == 1 def test_apply_state_leaves_an_absent_ui_panel_field_alone(scene): @@ -3170,9 +3170,61 @@ def test_apply_state_leaves_an_absent_ui_panel_field_alone(scene): _apply(scene, _panel_runtime_state(VisorUIState(dark_theme=False))) - assert scene._panel_top_left_panel_collapsed is True - assert scene._panel_top_right_panel_collapsed is True - assert scene._panel_top_right_legend_collapsed is True - assert scene._panel_top_right_tab_index == 1 + assert scene._ui_state.panel_top_left_panel_collapsed is True + assert scene._ui_state.panel_top_right_panel_collapsed is True + assert scene._ui_state.panel_top_right_legend_collapsed is True + assert scene._ui_state.panel_top_right_tab_index == 1 + + +# --------------------------------------------------------------------------- +# get_state -- the record is handed out as a copy +# --------------------------------------------------------------------------- + +# A later write, distinct from both the store's seeded values and the reply's, +# so a returned state that moved reports a value belonging to neither. +LATER_PANEL_TAB_INDEX = 9 +LATER_PANEL_COLLAPSED = False + + +def test_get_state_hands_out_a_copy_of_the_ui_record(scene): + """A panel write landing after the read does not change what was returned. + + This is the assertion that pins the copy. The scene now holds the UI + record as one model, and that model survives by identity through the + state mapper onto the object ``get_state`` returns, so handing out + ``self._ui_state`` itself would let a trigger arriving on trame's daemon + thread mutate a state the writer already has -- the file would then carry + a layout the user reached after they asked to save. Pass the instance + instead of the copy and the two value assertions below report ``9`` and + ``False``. + + The registry's snapshot is pinned the same way, in + ``test_get_state_snapshots_the_registry_rather_than_referencing_it``; this + is that pin for the one other live object that leaves here. + + Asserted on what get_state RETURNS -- the object that reaches the writer + -- not on the runtime state it was built from. + """ + scene.set_panel_top_left_panel_collapsed(PANEL_COLLAPSED) + scene.set_panel_top_right_panel_collapsed(PANEL_COLLAPSED) + scene.set_panel_top_right_legend_collapsed(PANEL_COLLAPSED) + scene.set_panel_top_right_tab_index(PANEL_TAB_INDEX) + _ui_save_scene( + scene, + _reply_ui( + REPLY_PANEL_COLLAPSED, + REPLY_PANEL_COLLAPSED, + REPLY_PANEL_COLLAPSED, + REPLY_PANEL_TAB_INDEX, + ), + ) + + persisted = asyncio.run(scene.get_state(timeout=1.0)) + scene.set_panel_top_right_tab_index(LATER_PANEL_TAB_INDEX) + scene.set_panel_top_left_panel_collapsed(LATER_PANEL_COLLAPSED) + + assert persisted.ui.panel_top_right_tab_index == 1 + assert persisted.ui.panel_top_left_panel_collapsed is True + assert persisted.ui is not scene._ui_state From d3c0bc5a64c0b80201bb6214ab9491719522bea9 Mon Sep 17 00:00:00 2001 From: Laura Kasian Date: Fri, 25 Sep 2026 11:19:53 -0700 Subject: [PATCH 07/14] doc: clean up comments --- src/ansys/visor/viewer/app/trame/local_app.py | 9 +++------ .../runtime/requests/widget_state_payloads.py | 17 +++++++---------- .../models/runtime/scene/runtime_app_state.py | 7 +++---- .../viewer/vtk/scene/visor_state_mapper.py | 6 +++--- 4 files changed, 16 insertions(+), 23 deletions(-) diff --git a/src/ansys/visor/viewer/app/trame/local_app.py b/src/ansys/visor/viewer/app/trame/local_app.py index 8e24fc99..ce7e7f00 100644 --- a/src/ansys/visor/viewer/app/trame/local_app.py +++ b/src/ansys/visor/viewer/app/trame/local_app.py @@ -521,12 +521,9 @@ def sync_camera(self, payload) -> None: # it writes the camera record's projection field rather than a toggle # of its own. # - # The last four carry UI panel layout rather than widget state. They - # have no renderer half at all -- nothing the server renders depends on - # which panel is collapsed or which tab is active -- so each coordinator - # writes one store field and stops. Their echoes are idempotent on the - # same terms as the toggles': after a delivered apply the client reports - # back the value the server just sent it. + # The last four carry UI panel layout, not widget state: no renderer + # depends on them, so each coordinator just writes one store field. + # Echoes are idempotent for the same reason as the toggles'. # ------------------------------------------------------------------ @trigger("set_cross_section_visibility") diff --git a/src/ansys/visor/viewer/models/runtime/requests/widget_state_payloads.py b/src/ansys/visor/viewer/models/runtime/requests/widget_state_payloads.py index 1fdda2f3..2f757b3c 100644 --- a/src/ansys/visor/viewer/models/runtime/requests/widget_state_payloads.py +++ b/src/ansys/visor/viewer/models/runtime/requests/widget_state_payloads.py @@ -20,16 +20,13 @@ do: it is scene-wide and carries no ``nodeId``. The four ``SetPanelTopLeft*`` / ``SetPanelTopRight*`` models carry UI panel -layout rather than widget state. They are here on the same terms: each is -scene-wide, carries no ``nodeId``, and carries the absolute value of -exactly one panel field. A panel echo is idempotent, as a toggle echo is: -the value the client reports after a delivered apply is the value the -server already holds. - -``tab_index`` is deliberately unbounded. The client sends ``0`` or ``1`` -and nothing else, and ``Panel_TopRight_Util.selectTab`` already ignores any -other value, so the bound lives there. A bound here would make a future -third tab a validation failure at the boundary rather than a UI change. +layout, not widget state, but are here on the same terms: scene-wide, no +``nodeId``, one absolute field each. Their echoes are idempotent like a +toggle's. + +``tab_index`` is deliberately unbounded: the client only ever sends ``0`` +or ``1``, and ``Panel_TopRight_Util.selectTab`` already ignores anything +else, so the bound lives there, not here. """ from typing import List diff --git a/src/ansys/visor/viewer/models/runtime/scene/runtime_app_state.py b/src/ansys/visor/viewer/models/runtime/scene/runtime_app_state.py index b5bfba6b..63781a06 100644 --- a/src/ansys/visor/viewer/models/runtime/scene/runtime_app_state.py +++ b/src/ansys/visor/viewer/models/runtime/scene/runtime_app_state.py @@ -43,10 +43,9 @@ def from_components( ) -> "RuntimeAppState": """Construct a RuntimeAppState from the given components. - The UI record arrives whole rather than as a bare ``dark_mode``: the - server owns every field of it -- the theme and the four panel-layout - fields -- and assembles them together, so there is one place that - decides what the client is told about the UI rather than two. + ``ui`` is taken whole and stored as-is: the server owns every one of + its fields (theme plus the four panel-layout fields) and assembles + them in one place, so this constructor never reaches into it. """ runtime_scene_state = RuntimeSceneState( unit=unit, diff --git a/src/ansys/visor/viewer/vtk/scene/visor_state_mapper.py b/src/ansys/visor/viewer/vtk/scene/visor_state_mapper.py index 00c5c9d5..88167884 100644 --- a/src/ansys/visor/viewer/vtk/scene/visor_state_mapper.py +++ b/src/ansys/visor/viewer/vtk/scene/visor_state_mapper.py @@ -79,9 +79,9 @@ def persisted_to_runtime(self, state: PersistedViewerStateV1) -> RuntimeAppState :meth:`VisorSceneBase._restore_part_states`. """ - # UI settings. The record crosses whole: the four panel fields are as - # much the server's as the theme is, and splitting one field out here - # is what dropped them on the load path before. + # UI settings. Crossed whole: the server owns every field on the + # record (theme and the four panel-layout fields alike), so it is + # never split here. ui_state = state.ui # scene state From 0834b5b6438f63023d1464eabfb50a63826a66ff Mon Sep 17 00:00:00 2001 From: Laura Kasian Date: Fri, 25 Sep 2026 11:36:57 -0700 Subject: [PATCH 08/14] refactor: move dark_mode application to _restore_ui_state() --- src/ansys/visor/viewer/vtk/scene/base.py | 101 +++++++++-------------- 1 file changed, 37 insertions(+), 64 deletions(-) diff --git a/src/ansys/visor/viewer/vtk/scene/base.py b/src/ansys/visor/viewer/vtk/scene/base.py index 7e099bb4..b51578cf 100644 --- a/src/ansys/visor/viewer/vtk/scene/base.py +++ b/src/ansys/visor/viewer/vtk/scene/base.py @@ -79,28 +79,21 @@ def __init__( """Initialize the scene coordinator and its local renderer backend.""" logger.debug("Initializing %s", type(self).__name__) - # The server's UI record, held as its model rather than as loose - # scalars -- the same shape the camera record is held in. Built first, - # because ``dark_mode`` is a property over its ``dark_theme`` field and - # every later read or write of the theme goes through it. + # The server's UI record, held as a model like the camera record. + # Built first because ``dark_mode`` is a property over its + # ``dark_theme`` field. # - # The theme is the constructor's; the four panel fields are initialised - # to the client panels' own mount defaults -- Panel_TopLeft's - # ``isPanelCollapsed`` and both of Panel_TopRight's ``isCollapsed`` - # closures start false and are mount-clicked to false, and its - # ``tabIndex`` starts at 0. Initialised rather than left unset for two - # reasons: a get_state before the client has ever spoken then reports - # what the client would report, and the record is delivered through a - # dump that excludes ``None``, so a field left unset here is simply - # omitted from the payload and the client shows its own defaults + # The theme comes from the constructor; the four panel fields match + # the client panels' own mount defaults (collapsed=False, + # tab_index=0), so a get_state before the client has spoken reports + # what the client would. They're initialised rather than left unset + # because the payload dump excludes ``None``, and an unset field + # would be omitted, letting the client fall back to its own default # instead of the server's record. # - # Written only by the four panel triggers and by the load path. - # Absolute values, never toggles. There is still exactly one copy of - # the theme: ``dark_theme`` is it, and ``dark_mode`` is the property - # over it, not a second field -- a second copy is the divergence - # set_projection refuses. Projection is deliberately absent for the - # same reason: it is derived from the camera record. + # Written only by the four panel triggers and the load path, always + # as absolute values, never toggles. Projection is deliberately + # absent here: it's derived from the camera record, not stored twice. self._ui_state = VisorUIState( dark_theme=dark_mode, panel_top_left_panel_collapsed=False, @@ -187,19 +180,12 @@ def dataset_count(self) -> int: def dark_mode(self) -> bool | None: """Whether the viewer is in dark theme. - A property over the UI record's ``dark_theme`` field rather than an - attribute of its own: the theme has exactly one holder, and a second - copy of it is the divergence :meth:`set_projection` refuses. Every - existing reader and writer of ``scene.dark_mode`` -- the construction - sites, the scene-details read, and :meth:`apply_state`'s own line -- - goes through this pair unchanged. - - Typed as the record's field is, ``bool | None`` rather than ``bool``: - the constructor always supplies a ``bool``, but :meth:`apply_state` - writes ``state.ui.dark_theme`` through unguarded, and that field is - optional. The previous plain attribute was annotated ``bool`` and - took the same value; the annotation is what changes here, not what - can arrive. + A property over the UI record's ``dark_theme`` field rather than a + separate attribute, so the theme has exactly one holder. + + Typed ``bool | None`` to match that field: the constructor always + supplies a ``bool``, but :meth:`apply_state` writes + ``state.ui.dark_theme`` through unguarded, and that field is optional. """ return self._ui_state.dark_theme @@ -236,20 +222,17 @@ async def get_state(self, timeout: float) -> PersistedViewerStateV1: reply is consulted for none of the three. The UI record is the fourth. ``runtime_state.ui`` is replaced - wholesale with a copy of this object's own record -- the theme in its - ``dark_theme`` field and the four panel-layout fields the panel - triggers write -- so the browser's ``ui`` block is discarded - entire. A copy and never the instance: the record is handed to the - mapper and survives by identity onto the object the writer receives, - so passing the instance would let a panel trigger landing after this - read mutate the state being saved. That is the registry snapshot's - reason, applied to the one other live object that leaves here. The - assignment is unconditional, as the camera and toggle - assignments are, and it is what makes the saved theme the server's - rather than the embedding host's: under Dash the host prop overrides - the delivered theme in the browser, so a reply that was trusted here - would write the host's value into the file and, on the next load, into - ``dark_mode``. + wholesale with a copy of this object's own record, so the browser's + ``ui`` block is discarded entirely. A copy and never the instance: + the mapper holds onto the object it's given, so passing the instance + would let a panel trigger landing after this read mutate the state + being saved. + + The assignment is unconditional, which is what makes the saved theme + the server's rather than the embedding host's: under Dash the host + prop overrides the delivered theme in the browser, so a reply that + was trusted here would write the host's value into the file and, + on the next load, into ``dark_mode``. The camera comes from the renderer's record, which is authoritative, rather than from the reply or from the pipeline ``vtkCamera``: the pipeline is the @@ -307,9 +290,6 @@ def apply_state(self, state: PersistedViewerStateV1): Holds ``_vtk_lock`` for the whole body, including the delegated render step. """ with self._vtk_lock: - # Apply UI settings - self.dark_mode = state.ui.dark_theme - # Transform the frontend PersistedViewerStateV1 -> RuntimeAppState runtime_app_state = self._state_mapper.persisted_to_runtime(state) @@ -825,18 +805,12 @@ def set_projection(self, parallel: bool) -> None: # ========================================================================= # UI panel layout — coordinator surface # - # The same shape as the widget toggles above, minus the renderer half: - # each writes one store field under ``_vtk_lock`` and stops. There is no - # ``IRenderer`` seam to fill, because nothing the server renders depends - # on panel layout -- it is browser-side chrome whose only server-side job - # is to survive a refresh. - # - # No notify, for the reason the surfaces above give: no ``render()``, no - # ``flush_wasm_state()``, no ``set_state``. + # Same shape as the widget toggles above, minus the renderer half: each + # writes one store field under ``_vtk_lock`` and stops. No ``IRenderer`` + # seam to fill, no notify — panel layout is browser-side chrome, and the + # reasons are the same as above. # - # Every value that arrives here is absolute, never relative, and an echo - # is idempotent: after a delivered apply the client reports back the value - # the server just sent it. + # Every value that arrives here is absolute, never relative. # ========================================================================= def set_panel_top_left_panel_collapsed(self, collapsed: bool) -> None: @@ -967,13 +941,12 @@ def _restore_ui_state(self, runtime_app_state: "RuntimeAppState") -> None: a file can carry any subset -- anything written before this record existed carries none of them. - ``dark_theme`` is deliberately not read here. It is - :meth:`apply_state`'s own line, and two readers of one property is the - divergence this epic has been removing. - Callers must hold ``_vtk_lock``. """ ui = runtime_app_state.ui + + if ui.dark_theme is not None: + self.dark_mode = ui.dark_theme if ui.panel_top_left_panel_collapsed is not None: self._ui_state.panel_top_left_panel_collapsed = ui.panel_top_left_panel_collapsed if ui.panel_top_right_panel_collapsed is not None: From cccb97502b51b42243f4ef4ea3fada460d0dd7bd Mon Sep 17 00:00:00 2001 From: Laura Kasian Date: Fri, 25 Sep 2026 11:42:45 -0700 Subject: [PATCH 09/14] doc: clean up docs --- src/ansys/visor/viewer/vtk/scene/base.py | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/ansys/visor/viewer/vtk/scene/base.py b/src/ansys/visor/viewer/vtk/scene/base.py index b51578cf..ced52498 100644 --- a/src/ansys/visor/viewer/vtk/scene/base.py +++ b/src/ansys/visor/viewer/vtk/scene/base.py @@ -222,17 +222,7 @@ async def get_state(self, timeout: float) -> PersistedViewerStateV1: reply is consulted for none of the three. The UI record is the fourth. ``runtime_state.ui`` is replaced - wholesale with a copy of this object's own record, so the browser's - ``ui`` block is discarded entirely. A copy and never the instance: - the mapper holds onto the object it's given, so passing the instance - would let a panel trigger landing after this read mutate the state - being saved. - - The assignment is unconditional, which is what makes the saved theme - the server's rather than the embedding host's: under Dash the host - prop overrides the delivered theme in the browser, so a reply that - was trusted here would write the host's value into the file and, - on the next load, into ``dark_mode``. + wholesale with a copy of this object's own record. The camera comes from the renderer's record, which is authoritative, rather than from the reply or from the pipeline ``vtkCamera``: the pipeline is the From 752d1e53d1158642ee74ff90e78515c1f62344c1 Mon Sep 17 00:00:00 2001 From: Laura Kasian Date: Fri, 25 Sep 2026 11:48:11 -0700 Subject: [PATCH 10/14] clean up docs --- .../src/components/ui-panels/Panel_TopLeft.tsx | 4 +++- .../src/components/ui-panels/Panel_TopRight.tsx | 16 ++++++++-------- .../src/wasm/CameraGestureTracker.js | 1 + 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/ansys/visor/visor-client/src/components/ui-panels/Panel_TopLeft.tsx b/src/ansys/visor/visor-client/src/components/ui-panels/Panel_TopLeft.tsx index b03bef5e..8d9eadd3 100644 --- a/src/ansys/visor/visor-client/src/components/ui-panels/Panel_TopLeft.tsx +++ b/src/ansys/visor/visor-client/src/components/ui-panels/Panel_TopLeft.tsx @@ -93,7 +93,9 @@ export const Panel_TopLeft: FC<{ } ); visorState.setPanelTopLeftUtil(util); - // Must stay on the line after the util handoff: it suppresses the mount click above and is open before any delivered apply awaiting the util promise can click; scaffolding, removed when delivery is separated from mutation. + // Must stay on the line after the util handoff: it suppresses the mount click above and is + // open before any delivered apply awaiting the util promise can click; scaffolding, + // removed when delivery is separated from mutation. sendEnabled = true; onLoad(util); }, []); diff --git a/src/ansys/visor/visor-client/src/components/ui-panels/Panel_TopRight.tsx b/src/ansys/visor/visor-client/src/components/ui-panels/Panel_TopRight.tsx index d92e41cd..9473293e 100644 --- a/src/ansys/visor/visor-client/src/components/ui-panels/Panel_TopRight.tsx +++ b/src/ansys/visor/visor-client/src/components/ui-panels/Panel_TopRight.tsx @@ -473,13 +473,11 @@ export const Panel_TopRight: FC<{ await visorState.render(); }); - // Initialize from the tree's current selection, not from an empty - // list: a selection delivered before this panel mounted -- a - // refresh, a rebuild -- is already on the rows and on the mesh, and - // `synchronize()` reaches it without running the selection-change - // listeners, so nothing else would ever hand it to this panel. - // Read here, after the util promise settled, so the array is the - // live one the listener above is subscribed to. + // Seed from the tree's current selection instead of an empty list: + // a selection made before this panel mounted (e.g. during a + // refresh/rebuild) is applied via `synchronize()`, which doesn't + // fire the selection-change listener, so it would otherwise never + // reach this panel. await onSelectionChangeAsync(treeViewUtil.selectedNodes); const util = new Panel_TopRight_Util( expandPanel, @@ -498,7 +496,9 @@ export const Panel_TopRight: FC<{ () => tabIndex ); visorState.setPanelTopRightUtil(util); - // Must stay on the line after the util handoff: it suppresses the three mount writes above and is open before any delivered apply awaiting the util promise can click; scaffolding, removed when delivery is separated from mutation. + // Must stay on the line after the util handoff: it suppresses the three mount writes + // above and is open before any delivered apply awaiting the util promise can click; + // scaffolding, removed when delivery is separated from mutation. sendEnabled = true; onLoad(util); })(); diff --git a/src/ansys/visor/visor-client/src/wasm/CameraGestureTracker.js b/src/ansys/visor/visor-client/src/wasm/CameraGestureTracker.js index 694c2147..ad8611f9 100644 --- a/src/ansys/visor/visor-client/src/wasm/CameraGestureTracker.js +++ b/src/ansys/visor/visor-client/src/wasm/CameraGestureTracker.js @@ -190,6 +190,7 @@ export default class CameraGestureTracker { this.#settleTimer = setTimeout(this.#reportSettled, CAMERA_SETTLE_MS); }; + /** * @return {void} */ From a1863ad26ac83fbe4a7777f66a7cdbd4ef895ffe Mon Sep 17 00:00:00 2001 From: Laura Kasian Date: Fri, 25 Sep 2026 11:52:14 -0700 Subject: [PATCH 11/14] doc: clean up comments --- .../visor/visor-client/src/VisorFrontend.tsx | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/src/ansys/visor/visor-client/src/VisorFrontend.tsx b/src/ansys/visor/visor-client/src/VisorFrontend.tsx index f9b6e042..e6f3bbea 100644 --- a/src/ansys/visor/visor-client/src/VisorFrontend.tsx +++ b/src/ansys/visor/visor-client/src/VisorFrontend.tsx @@ -215,15 +215,8 @@ export class VisorFrontend { uiScaffoldUtilSet = true; uiScaffoldUtilResolve(uiScaffoldUtil); }; - // UI panel state -- the four send methods below report one panel field - // each to its server trigger. They close over the `triggerSender` - // constructor parameter, which is required and supplied at every - // construction site, so there is no no-transport state to guard. - // - // A rejection is logged under one fixed, greppable prefix and - // swallowed, never rethrown: these run inside synchronous UI handlers - // that behaved a certain way before the call existed, and the client - // applies its own change independently of the report. + // Reports one panel field per call; errors are logged, not rethrown, + // since the client already applied the change locally. const sendUiPanelTriggerAsync = async ( triggerName: string, payload: Record @@ -237,13 +230,6 @@ export class VisorFrontend { ); } }; - // Each of the four forwards the argument it was given and reads - // nothing back. This is deliberately the opposite of `WasmRenderer`'s - // widget sends, which read their widget back because the toolbar calls - // them with no argument: here the caller is the panel handler that has - // just written the closure, so the argument is the settled value by - // construction, and the util it would be read back from may not exist - // yet. this.sendPanelTopLeftPanelCollapsedAsync = (collapsed) => sendUiPanelTriggerAsync('set_panel_top_left_panel_collapsed', { collapsed }); this.sendPanelTopRightPanelCollapsedAsync = (collapsed) => From 7d8a3f558dc80df00bbfdf3b83b4b5609a175b31 Mon Sep 17 00:00:00 2001 From: Laura Kasian Date: Fri, 25 Sep 2026 11:53:35 -0700 Subject: [PATCH 12/14] doc: clean up comments --- src/ansys/visor/visor-client/src/VisorFrontend.tsx | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/ansys/visor/visor-client/src/VisorFrontend.tsx b/src/ansys/visor/visor-client/src/VisorFrontend.tsx index e6f3bbea..9f69f7e5 100644 --- a/src/ansys/visor/visor-client/src/VisorFrontend.tsx +++ b/src/ansys/visor/visor-client/src/VisorFrontend.tsx @@ -575,10 +575,6 @@ export class VisorFrontend { setPanelTopLeftUtil: (panelTopRightUtil: Panel_TopLeft_Util) => void; setPanelTopRightUtil: (panelTopRightUtil: Panel_TopRight_Util) => void; setUiScaffoldUtil: (uiScaffoldUtil: UiScaffoldUtil) => void; - /** - * Report one panel-layout field to the server. Each carries the absolute - * value it was given for exactly one field; no send reads any other field. - */ sendPanelTopLeftPanelCollapsedAsync: (collapsed: boolean) => Promise; sendPanelTopRightPanelCollapsedAsync: (collapsed: boolean) => Promise; sendPanelTopRightLegendCollapsedAsync: (collapsed: boolean) => Promise; From fa6c45d81e8166c0181b01378777e83b5b1d2e4c Mon Sep 17 00:00:00 2001 From: Laura Kasian Date: Fri, 25 Sep 2026 11:56:36 -0700 Subject: [PATCH 13/14] doc: clean up comments --- src/ansys/visor/viewer/vtk/scene/base.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/ansys/visor/viewer/vtk/scene/base.py b/src/ansys/visor/viewer/vtk/scene/base.py index ced52498..b0e2529e 100644 --- a/src/ansys/visor/viewer/vtk/scene/base.py +++ b/src/ansys/visor/viewer/vtk/scene/base.py @@ -327,11 +327,11 @@ def get_scene_details(self) -> VisorSceneDetails: stored, so it can't disagree with the camera. ``None`` means "nothing was ever written." - No ``_vtk_lock``: the reads here (three booleans, one camera field, - and the UI record's five) aren't consumed as a mutually consistent - snapshot, and locking a request-path read against the trigger thread - belongs with the round-trip/thread-affinity work, not here. Accepted - exposure: one stale field in a delivered payload. + No ``_vtk_lock``: the reads here (three booleans, one camera field) + aren't consumed as a mutually consistent snapshot, and locking a + request-path read against the trigger thread belongs with the + round-trip/thread-affinity work, not here. Accepted exposure: one + stale field in a delivered payload. The UI record is passed as a copy and never as the instance, for the reason :meth:`get_state` gives: a panel trigger landing after this From 41fd0e3ea89d8b3046431d6e3fcfbfca5a8611c6 Mon Sep 17 00:00:00 2001 From: Laura Kasian Date: Fri, 25 Sep 2026 12:00:21 -0700 Subject: [PATCH 14/14] fix: prettier fix --- src/ansys/visor/visor-client/src/wasm/CameraGestureTracker.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ansys/visor/visor-client/src/wasm/CameraGestureTracker.js b/src/ansys/visor/visor-client/src/wasm/CameraGestureTracker.js index ad8611f9..694c2147 100644 --- a/src/ansys/visor/visor-client/src/wasm/CameraGestureTracker.js +++ b/src/ansys/visor/visor-client/src/wasm/CameraGestureTracker.js @@ -190,7 +190,6 @@ export default class CameraGestureTracker { this.#settleTimer = setTimeout(this.#reportSettled, CAMERA_SETTLE_MS); }; - /** * @return {void} */