From 7c19f79b62303986dbd748b61000012c8ff5d0ba Mon Sep 17 00:00:00 2001 From: Laura Kasian Date: Fri, 18 Sep 2026 09:35:34 -0700 Subject: [PATCH 01/13] feat: server-tracked widget toggles with triggers, coordinator and delivery --- src/ansys/visor/viewer/app/trame/local_app.py | 76 ++- .../runtime/requests/widget_state_payloads.py | 43 ++ .../models/runtime/visor_scene_details.py | 18 +- src/ansys/visor/viewer/renderer/base.py | 15 +- .../visor/viewer/renderer/local_renderer.py | 20 +- .../visor/viewer/renderer/null_renderer.py | 5 +- src/ansys/visor/viewer/vtk/node_pipeline.py | 13 + src/ansys/visor/viewer/vtk/scene/base.py | 148 +++++- .../app/test_local_app_widget_triggers.py | 234 ++++++++++ tests/unit/renderer/test_local_renderer.py | 45 ++ tests/unit/vtk/scene/test_base.py | 442 ++++++++++++++++++ tests/unit/vtk/scene/test_local_scene.py | 5 + tests/unit/vtk/test_node_pipeline.py | 44 ++ tests/unit/vtk/test_wire_format_identity.py | 11 + 14 files changed, 1103 insertions(+), 16 deletions(-) create mode 100644 src/ansys/visor/viewer/models/runtime/requests/widget_state_payloads.py create mode 100644 tests/unit/app/test_local_app_widget_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 cf5c0202..bd03a0f3 100644 --- a/src/ansys/visor/viewer/app/trame/local_app.py +++ b/src/ansys/visor/viewer/app/trame/local_app.py @@ -13,6 +13,11 @@ from ansys.visor.viewer.core.visor_logging import VisorDefaultLogger from ansys.visor.viewer.models.common.visor_camera_state import VisorCameraState from ansys.visor.viewer.models.runtime.requests.sync_camera_payload import SyncCameraPayload +from ansys.visor.viewer.models.runtime.requests.widget_state_payloads import ( + SetBoundingBoxVisibilityPayload, + SetCrossSectionVisibilityPayload, + SetEdgesVisiblePayload, +) logger = VisorDefaultLogger(__name__) @@ -21,16 +26,19 @@ class ScenePartStateApi(Protocol): - """Structural type of the per-part coordinator surface LocalApp calls. + """Structural type of the coordinator surface LocalApp calls. Typing only: there is no ``runtime_checkable`` decoration and no ``isinstance`` check anywhere against it. Declaring it here rather than importing the scene keeps this module free of any scene type, so the injected object remains LocalApp's only route to the scene. - ``sync_camera`` is not per-part, and it is declared here anyway: the one - production injection site passes the whole scene coordinator, so a second - protocol would be the same object under a second name. + Not all of it is per-part, and the name is historical. ``sync_camera`` + and the three widget-state toggles are scene-wide, and they are declared + here anyway: the one production injection site passes the whole scene + coordinator, so a second protocol would be the same object under a second + name. Read this as the whole coordinator surface LocalApp calls, not only + the per-part part of it. """ def set_part_visibility(self, node_id: int, visible: bool) -> None: ... @@ -56,6 +64,12 @@ def clear_part_color_variable(self, node_id: int) -> None: ... def sync_camera(self, camera_state: VisorCameraState) -> None: ... + def set_cross_section_visibility(self, visible: bool) -> None: ... + + def set_edges_visible(self, visible: bool) -> None: ... + + def set_bounding_box_visibility(self, visible: bool) -> None: ... + # ---------------------------------------------------------------------- # Trigger payload models @@ -184,6 +198,9 @@ class LocalApp: set_part_color_variable: colours one part by a scalar variable clear_part_color_variable: stops colouring one part by a scalar variable sync_camera: records a settled camera reported by the frontend + set_cross_section_visibility: shows or hides the cross-section plane + set_edges_visible: shows or hides edges on every part + set_bounding_box_visibility: shows or hides the bounding-box outline 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. @@ -467,6 +484,57 @@ def sync_camera(self, payload) -> None: ) api.sync_camera(payload.camera) + # ------------------------------------------------------------------ + # Widget-state triggers + # + # Frontend -> Backend. One trigger per server-tracked toggle. Each + # carries the absolute target value the client's widget settled on, + # never a toggle and never a delta: the toolbar buttons are toggles, so + # the client reads its widget back after the local write and sends the + # result. A message the client suppresses as redundant is therefore + # indistinguishable from one that set a value the server already held, + # and both are correct. + # + # No ``origin`` field (RS-1). The camera needed one because a + # programmatic echo re-applied a *stale* camera over a newer one; a + # toggle echo carries the same boolean the server already holds, so the + # write is idempotent and the client's own widgets damp it with their + # value guards. It is the same echo the per-part deliveries already + # produce on load. Separating delivery from mutation is a later story's + # work, not this one's. + # + # Payloads are validated at this boundary by ``@parse_payload`` exactly + # as the per-part ones are. The wire key is ``visible`` on all three and + # carries no pydantic alias: snake_case and camelCase coincide. + # ------------------------------------------------------------------ + + @trigger("set_cross_section_visibility") + @parse_payload(SetCrossSectionVisibilityPayload) + def set_cross_section_visibility(self, payload) -> None: + """Frontend -> Backend: show or hide the cross-section plane.""" + api = self._part_state_api("set_cross_section_visibility") + if api is None: + return + api.set_cross_section_visibility(payload.visible) + + @trigger("set_edges_visible") + @parse_payload(SetEdgesVisiblePayload) + def set_edges_visible(self, payload) -> None: + """Frontend -> Backend: show or hide edges on every part.""" + api = self._part_state_api("set_edges_visible") + if api is None: + return + api.set_edges_visible(payload.visible) + + @trigger("set_bounding_box_visibility") + @parse_payload(SetBoundingBoxVisibilityPayload) + def set_bounding_box_visibility(self, payload) -> None: + """Frontend -> Backend: show or hide the bounding-box outline.""" + api = self._part_state_api("set_bounding_box_visibility") + if api is None: + return + api.set_bounding_box_visibility(payload.visible) + 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 new file mode 100644 index 00000000..a2badacd --- /dev/null +++ b/src/ansys/visor/viewer/models/runtime/requests/widget_state_payloads.py @@ -0,0 +1,43 @@ +"""Models for the widget-state trigger payloads. + +One model per server-tracked widget toggle. Field names are already +identical in snake_case and camelCase, so **no** ``Field(alias=...)`` is +needed and none is to be added: the wire key is exactly ``visible``. + +These live here rather than inline in ``local_app.py`` beside the six +per-part payload models, whose own block comment scopes itself to +per-part triggers carrying camelCase aliases. These are neither. The +precedent is ``sync_camera_payload.py``, the one existing non-per-part +trigger, whose model lives in this package. + +No ``origin`` field (RS-1). The camera needed one because a programmatic +echo re-applied a *stale* camera over a newer one; a toggle echo carries +the same boolean the server already holds, so the write is idempotent. +""" + +from pydantic import BaseModel, ConfigDict + + +class SetCrossSectionVisibilityPayload(BaseModel): + """Payload of the ``set_cross_section_visibility`` trigger.""" + + model_config = ConfigDict(populate_by_name=True) + + visible: bool + + +class SetEdgesVisiblePayload(BaseModel): + """Payload of the ``set_edges_visible`` trigger.""" + + model_config = ConfigDict(populate_by_name=True) + + visible: bool + + +class SetBoundingBoxVisibilityPayload(BaseModel): + """Payload of the ``set_bounding_box_visibility`` trigger.""" + + model_config = ConfigDict(populate_by_name=True) + + visible: bool + 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 6f4a25cf..ffc02de4 100644 --- a/src/ansys/visor/viewer/models/runtime/visor_scene_details.py +++ b/src/ansys/visor/viewer/models/runtime/visor_scene_details.py @@ -31,8 +31,20 @@ def from_components( dataset_states: Dict[int, RuntimeDatasetState], scene_graph_state: SceneGraphNodeInfo | None = None, renderer_annotation: RendererAnnotation | None = None, + orthographic_enabled: bool | None = None, + cross_section_enabled: bool | None = None, + edges_enabled: bool | None = None, + bounding_box_enabled: bool | None = None, ) -> "VisorSceneDetails": - """Construct an instance from components.""" + """Construct an instance from components. + + The four widget-state keywords are forwarded, not stored here: this + payload is how a rebuilt or reconnecting client learns the server's + toggles, and ``RuntimeAppState.from_components`` already accepts every + one of them. No new model and no new field -- what was missing was + the argument passing, and a keyword dropped from the call below is + delivered as ``None`` and reverts that toggle in the browser. + """ vtk_info = RuntimeVTKInfo( scene_graph=scene_graph_state, renderer_annotation=renderer_annotation, @@ -41,6 +53,10 @@ def from_components( dark_mode=dark_mode, unit=unit, dataset_states=dataset_states, + orthographic_enabled=orthographic_enabled, + cross_section_enabled=cross_section_enabled, + edges_enabled=edges_enabled, + bounding_box_enabled=bounding_box_enabled, ) return cls( app_state=app_state, diff --git a/src/ansys/visor/viewer/renderer/base.py b/src/ansys/visor/viewer/renderer/base.py index 9f2f1ac5..b69e925f 100644 --- a/src/ansys/visor/viewer/renderer/base.py +++ b/src/ansys/visor/viewer/renderer/base.py @@ -8,8 +8,8 @@ without touching scene coordination. Contract covers node lifecycle, per-part visual mutations, -camera, widget control (cross-section, bounding box), widget fan-out (scene -bounds, actor count), picking, and render/flush. Not covered yet: +camera, widget control (cross-section, bounding box, edges), widget fan-out +(scene bounds, actor count), picking, and render/flush. Not covered yet: state-authority hooks, trigger-facing methods, round-trip additions. """ @@ -220,9 +220,18 @@ def serialize_camera_state(self) -> None: """ # ------------------------------------------------------------------------ - # Widget control (cross-section, bounding box) + # Widget control (cross-section, bounding box, edges) # ------------------------------------------------------------------------ + @abstractmethod + def set_edges_visible(self, visible: bool) -> None: + """Show or hide edges on every part in the scene. + + Scene-wide, not per-node: edges are a single global toggle and the + per-part surface that once mirrored it had no reader, no sender and + no model field. + """ + @abstractmethod def set_cross_section_visibility(self, visible: bool) -> None: """Show or hide the cross-section clipping plane.""" diff --git a/src/ansys/visor/viewer/renderer/local_renderer.py b/src/ansys/visor/viewer/renderer/local_renderer.py index 1cb77ff7..b83815ff 100644 --- a/src/ansys/visor/viewer/renderer/local_renderer.py +++ b/src/ansys/visor/viewer/renderer/local_renderer.py @@ -365,11 +365,27 @@ def _apply_to_pipeline_camera(self, camera_state: "VisorCameraState") -> None: camera.SetParallelScale(camera_state.parallel_scale) # ------------------------------------------------------------------ - # IRenderer: widget control (cross-section, bounding box) + # IRenderer: widget control (cross-section, bounding box, edges) # - # No coordinator caller on this branch. Phase 3 populates. + # ``set_edges_visible`` has a coordinator caller and a body. The two + # visibility verbs do not, and deliberately stay no-ops: the server's + # cross-section and bounding-box widget objects are driven by the client + # through the wasm mirror, nothing in LOCAL reads their enablement, and a + # server-side body would be a second writer with no reader. Edge + # visibility is different in kind -- it is an actor property on this + # renderer's own pipelines, and it has a reader here. # ------------------------------------------------------------------ + def set_edges_visible(self, visible: bool) -> None: + """See :meth:`IRenderer.set_edges_visible`. + + Fans out over every registered pipeline. Scene-wide, so there is no + node id to resolve and no logged-no-op branch: a scene with no + pipelines is a no-op by iteration, not by guard. + """ + for pipe in self._pipelines.values(): + pipe.set_edge_visibility(visible) + def set_cross_section_visibility(self, visible: bool) -> None: """No-op in Story 1.2. Phase 3 populates.""" diff --git a/src/ansys/visor/viewer/renderer/null_renderer.py b/src/ansys/visor/viewer/renderer/null_renderer.py index 3500510c..cb9880f4 100644 --- a/src/ansys/visor/viewer/renderer/null_renderer.py +++ b/src/ansys/visor/viewer/renderer/null_renderer.py @@ -135,9 +135,12 @@ def serialize_camera_state(self) -> None: # ------------------------------------------------------------------ - # Widget control (cross-section, bounding box) + # Widget control (cross-section, bounding box, edges) # ------------------------------------------------------------------ + def set_edges_visible(self, visible: bool) -> None: + """No pipelines to fan out over; nothing to do.""" + def set_cross_section_visibility(self, visible: bool) -> None: pass diff --git a/src/ansys/visor/viewer/vtk/node_pipeline.py b/src/ansys/visor/viewer/vtk/node_pipeline.py index fa611e0b..d40de33b 100644 --- a/src/ansys/visor/viewer/vtk/node_pipeline.py +++ b/src/ansys/visor/viewer/vtk/node_pipeline.py @@ -153,6 +153,19 @@ def set_visibility(self, visible: bool) -> None: """ self.actor.SetVisibility(1 if visible else 0) + def set_edge_visibility(self, visible: bool) -> None: + """Show or hide this part's edges. + + Mutates the actor's property, not the actor: edge visibility is a + property flag, where :meth:`set_visibility` is an actor flag. + + Parameters + ---------- + visible: + Target state. Absolute, never a toggle. + """ + self.actor.GetProperty().SetEdgeVisibility(1 if visible else 0) + def set_opacity(self, opacity: float) -> None: """Set this part's opacity. diff --git a/src/ansys/visor/viewer/vtk/scene/base.py b/src/ansys/visor/viewer/vtk/scene/base.py index 2a066af2..04606946 100644 --- a/src/ansys/visor/viewer/vtk/scene/base.py +++ b/src/ansys/visor/viewer/vtk/scene/base.py @@ -64,6 +64,9 @@ class VisorSceneBase(ABC): _dataset_registry: VisorDatasetRegistry _renderer: IRenderer _state_mapper: VisorStateMapper + _cross_section_enabled: bool + _edges_enabled: bool + _bounding_box_enabled: bool def __init__( self, @@ -74,6 +77,17 @@ def __init__( """Initialize the scene coordinator and its local renderer backend.""" logger.debug("Initializing %s", type(self).__name__) self.dark_mode: bool = dark_mode + + # Server-tracked widget toggles. Absolute values, never toggles. + # Initialised to the client widgets' own constructor defaults so a + # get_state before the client has ever spoken reports what the client + # would report. Projection is deliberately absent: it is derived from + # the camera record, so a fourth field here would be the second source + # that derivation exists to remove. + self._cross_section_enabled: bool = False + self._edges_enabled: bool = False + self._bounding_box_enabled: bool = False + self._server = server self._scene_graph = None self._pipelines = {} @@ -159,8 +173,12 @@ async def get_state(self, timeout: float) -> PersistedViewerStateV1: setters mutate from the trame daemon thread, so each one is deep-copied under ``_vtk_lock``. The lock is taken after the ``await`` and never held across one. - The camera is the second thing the browser's reply does not get to supply. - It comes from the renderer's record, which is authoritative, rather than + The camera and the widget toggles are what the browser's reply does not + get to supply. Per-part state comes from the registry, the toggles from + this object's own store, and the camera from the renderer's record; the + reply is consulted for none of the three. + + 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 VTK introduced -- ``ResetCamera`` rewrites ``clipping_range``. The @@ -177,6 +195,9 @@ async def get_state(self, timeout: float) -> PersistedViewerStateV1: for dataset_id, dataset_state in self._dataset_registry.runtime_state_dict.items() } runtime_state.scene.camera = self._renderer.get_camera_state() + 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.scene.dataset_states = registry_dataset_states persisted = self._state_mapper.runtime_to_persisted(runtime_state) @@ -204,8 +225,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_camera_state(runtime_app_state) - # TODO: restore widget state, UI state, and variable states when they are synced back to the server. + # TODO: restore UI state and variable states when they are synced back to the server. # The server's copy is now current; deliver it to the rendering backend. # wasm: set_state() to the browser; RCA: a rendered frame; headless: no-op. @@ -215,17 +237,44 @@ def apply_state(self, state: PersistedViewerStateV1): # at this point races the client's rebuild against a half-written object graph. def get_scene_details(self) -> VisorSceneDetails: - """Return the VisorState.""" + """Return the VisorState. + + This payload is how a rebuilt or reconnecting client learns the + server's widget toggles. Everything else in this story makes the + server *authoritative*; this method is what makes it *deliver*, and + the three client branches that apply these fields already exist and + were dead only because nothing ever populated them. + + ``orthographic_enabled`` is derived from the camera record rather than + stored, so that the projection has exactly one source. A ``None`` + record delivers ``None``, which says "nothing was ever written" and + leaves the client's own flag alone. + + No ``_vtk_lock`` (RS-8). What is read here is three independent + boolean loads and one field off the camera record; nothing consumes + them as a mutually consistent snapshot, and the two larger reads + already in this method are already unlocked. Locking a request-path + read against the trigger thread, with the lock ordering on that path + untraced, belongs with the round-trip and thread-affinity work. The + accepted exposure is one stale field in a delivered payload. + """ if self._scene_graph is None: self._initialize_scene_graph() annotation = self._renderer.build_renderer_annotation() scene_graph_state = self._build_scene_graph_state() + camera_record = self._renderer.get_camera_state() return VisorSceneDetails.from_components( dark_mode=self.dark_mode, unit=self._dataset_registry.unit, dataset_states=self._dataset_registry.runtime_state_dict, scene_graph_state=scene_graph_state, renderer_annotation=annotation, + orthographic_enabled=( + camera_record.parallel_projection if camera_record is not None else None + ), + cross_section_enabled=self._cross_section_enabled, + edges_enabled=self._edges_enabled, + bounding_box_enabled=self._bounding_box_enabled, ) def get_scene_details_json(self) -> str: @@ -466,12 +515,28 @@ def pick_geometry(self, actor_wasm_id, cell_id, mode, world_x, world_y, world_z) # ========================================================================= def set_part_visibility(self, node_id: int, visible: bool) -> None: - """Set whether the part identified by *node_id* is visible.""" + """Set whether the part identified by *node_id* is visible. + + Fans out to the widget layer after the apply, so that hiding a part + reaches the bounds-consuming widgets at all. The two private helpers + rather than :meth:`update_widgets`: that method raises ``RuntimeError`` + when the scene graph is ``None``, and a trigger thread is where a raise + has no caller to handle it, so a path that today logs at debug and + returns would start raising. The cost is that a later addition to + ``update_widgets``' body will not reach here. + + The box will not change size when a part is hidden. The server's root + bounds are computed across all loaded datasets with no visibility + filter, so the same numbers arrive at the widget. That is the current + bounds semantics, not a defect in this fan-out. + """ with self._vtk_lock: if not self._dataset_registry.set_part_visibility(node_id, visible): logger.debug("set_part_visibility: no dataset owns node %s; skipping.", node_id) return self._renderer.apply_visibility(node_id, visible) + self._update_widget_bounds() + self._update_actor_count() def set_part_opacity(self, node_id: int, opacity: float) -> None: """Set the opacity of the part identified by *node_id*.""" @@ -565,6 +630,50 @@ def clear_part_color_variable(self, node_id: int) -> None: return self._renderer.clear_color_variable(node_id) + # ========================================================================= + # Widget state — coordinator surface + # + # Each method does both halves of its trigger, in this order and all under + # ``_vtk_lock``: write the server's record, apply to the server's VTK + # objects. Nothing is pushed to the client from here, for the same reason + # the per-part surface pushes nothing: the client applied its own change + # before it sent, and a push rebuilds the client, which re-delivers state + # and fires further triggers. + # + # Every value that arrives here is absolute, never relative. + # + # None of these is abstract. The subclasses differ on state authority, + # not on widget state, and the abstract set is asserted by equality. + # ========================================================================= + + def set_cross_section_visibility(self, visible: bool) -> None: + """Set whether the cross-section plane is shown. + + The renderer call is a no-op today and is made anyway: the server's + cross-section widget is driven by the client through the wasm mirror, + so the store is what is authoritative and delivered, and the call is + the seam a server-rendering mode would fill. + """ + with self._vtk_lock: + self._cross_section_enabled = visible + self._renderer.set_cross_section_visibility(visible) + + def set_edges_visible(self, visible: bool) -> None: + """Set whether edges are shown on every part.""" + with self._vtk_lock: + self._edges_enabled = visible + self._renderer.set_edges_visible(visible) + + def set_bounding_box_visibility(self, visible: bool) -> None: + """Set whether the bounding-box outline is shown. + + As with the cross-section, the renderer call is a no-op today and the + store is the authority. + """ + with self._vtk_lock: + self._bounding_box_enabled = visible + self._renderer.set_bounding_box_visibility(visible) + def _restore_part_states(self, runtime_app_state: "RuntimeAppState") -> None: """ Restore per-part state from a runtime app state, on the load path. @@ -617,6 +726,35 @@ def _restore_camera_state(self, runtime_app_state: "RuntimeAppState") -> None: self._renderer.sync_camera(runtime_app_state.scene.camera) self._renderer.serialize_camera_state() + def _restore_widget_state(self, runtime_app_state: "RuntimeAppState") -> None: + """ + Restore the camera state from a runtime app state, on the load path. + + The widget toggles. Absent says nothing: a state that does not + carry a toggle leaves the server's value alone, which is this + path's guard and not get_state's. Store first, renderer second, + matching the coordinator surface below. + + + ``orthographic_enabled`` is deliberately not read. Projection + arrives on the camera, whose sync_camera writes it to the + record and the pipeline; the persisted toggle is emitted for + compatibility and ignored here, because two readers of one + property is the divergence this story removed. + + Callers must hold ``_vtk_lock``. + """ + scene = runtime_app_state.scene + if scene.cross_section_enabled is not None: + self._cross_section_enabled = scene.cross_section_enabled + self._renderer.set_cross_section_visibility(scene.cross_section_enabled) + if scene.edges_enabled is not None: + self._edges_enabled = scene.edges_enabled + self._renderer.set_edges_visible(scene.edges_enabled) + if scene.bounding_box_enabled is not None: + self._bounding_box_enabled = scene.bounding_box_enabled + self._renderer.set_bounding_box_visibility(scene.bounding_box_enabled) + def _restore_one_part_state( self, part_id: int, diff --git a/tests/unit/app/test_local_app_widget_triggers.py b/tests/unit/app/test_local_app_widget_triggers.py new file mode 100644 index 00000000..6e8d8a7e --- /dev/null +++ b/tests/unit/app/test_local_app_widget_triggers.py @@ -0,0 +1,234 @@ +"""Unit tests for LocalApp's three widget-state triggers. + +A module of its own rather than an addition to ``test_local_app.py``: that +module's ``TRIGGER_NAMES`` list drives parametrised tests whose meaning is +"one of the six per-part triggers", and every per-part payload model carries +``node_id: int = Field(alias="nodeId")``. None of these three carries a node +id -- each is scene-wide and carries a single ``visible`` boolean -- so adding +a name there would multiply the per-part tests by three and rewrite them. + +Coverage targets +---------------- +1. Each trigger delegates the value it arrived with, exactly once, to the + identically-named coordinator method. +2. A payload missing ``visible`` is a logged no-op: nothing is delegated. +3. A payload that is not a mapping at all is a logged no-op, not a + ``TypeError`` -- the ``model_validate`` posture the payload decorator + documents. +4. With no coordinator injected, each trigger logs and returns. +5. All three trigger names survive decoration and are registered with the + server. + +Every payload here is a hand-written literal. The wire key is ``visible`` on +all three and carries no alias: snake_case and camelCase coincide, which is +itself asserted by the delegation tests passing a literal ``{"visible": ...}``. +""" + +from unittest.mock import MagicMock, patch + +import pytest + +from ansys.visor.viewer.app.trame.local_app import LocalApp + +# The three trigger names, written out rather than imported, so that a rename +# on the production side fails here by name instead of following along. +CROSS_SECTION_TRIGGER = "set_cross_section_visibility" +EDGES_TRIGGER = "set_edges_visible" +BOUNDING_BOX_TRIGGER = "set_bounding_box_visibility" + + +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_part_state_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_part_state_api=api, + ) + + +@pytest.fixture +def app_without_api(mock_server): + """LocalApp with no coordinator injected.""" + return LocalApp( + server=mock_server, + get_scene_details_json=MagicMock(), + handle_save_state_response=MagicMock(), + standalone=True, + ) + + +# =========================================================================== +# Delegation +# +# ``True`` is sent where the server's own default is ``False``, so a handler +# that delegated a default rather than the payload's value would fail. +# =========================================================================== + +def test_set_cross_section_visibility_delegates_the_visible_value(app, api): + """The cross-section trigger hands the coordinator the value it received.""" + result = app.set_cross_section_visibility({"visible": True}) + + assert result is None + api.set_cross_section_visibility.assert_called_once_with(True) + + +def test_set_edges_visible_delegates_the_visible_value(app, api): + """The edges trigger hands the coordinator the value it received.""" + result = app.set_edges_visible({"visible": True}) + + assert result is None + api.set_edges_visible.assert_called_once_with(True) + + +def test_set_bounding_box_visibility_delegates_the_visible_value(app, api): + """The bounding-box trigger hands the coordinator the value it received.""" + result = app.set_bounding_box_visibility({"visible": True}) + + assert result is None + api.set_bounding_box_visibility.assert_called_once_with(True) + + +# =========================================================================== +# Validation -- a malformed payload reaches no handler body +# =========================================================================== + +def test_set_cross_section_visibility_missing_visible_is_a_logged_no_op(app, api): + """An empty payload never reaches the handler body.""" + with patch("ansys.visor.viewer.app.trame.local_app.logger") as log: + result = app.set_cross_section_visibility({}) + + assert result is None + assert api.mock_calls == [] + assert log.warning.call_count == 1 + + +def test_set_edges_visible_missing_visible_is_a_logged_no_op(app, api): + """An empty payload never reaches the handler body.""" + with patch("ansys.visor.viewer.app.trame.local_app.logger") as log: + result = app.set_edges_visible({}) + + assert result is None + assert api.mock_calls == [] + assert log.warning.call_count == 1 + + +def test_set_bounding_box_visibility_missing_visible_is_a_logged_no_op(app, api): + """An empty payload never reaches the handler body.""" + with patch("ansys.visor.viewer.app.trame.local_app.logger") as log: + result = app.set_bounding_box_visibility({}) + + assert result is None + assert api.mock_calls == [] + assert log.warning.call_count == 1 + + +def test_set_edges_visible_non_mapping_payload_is_a_logged_no_op(app, api): + """A bare string is a ValidationError, not a TypeError. + + The payload decorator uses ``model_validate`` rather than ``model(**payload)`` + precisely so that a payload which is not a mapping at all is caught by the + same guard as a payload with a missing key. Asserted against the whole + mock, so a leak under any other method name still fails. + """ + with patch("ansys.visor.viewer.app.trame.local_app.logger") as log: + result = app.set_edges_visible("visible") + + assert result is None + assert api.mock_calls == [] + assert log.warning.call_count == 1 + + +# =========================================================================== +# No coordinator injected +# =========================================================================== + +def test_set_cross_section_visibility_is_a_logged_no_op_when_no_coordinator_injected( + app_without_api, +): + """With nothing injected the trigger logs and returns without raising.""" + with patch("ansys.visor.viewer.app.trame.local_app.logger") as log: + result = app_without_api.set_cross_section_visibility({"visible": True}) + + assert result is None + assert log.debug.call_count == 1 + + +def test_set_edges_visible_is_a_logged_no_op_when_no_coordinator_injected(app_without_api): + """With nothing injected the trigger logs and returns without raising.""" + with patch("ansys.visor.viewer.app.trame.local_app.logger") as log: + result = app_without_api.set_edges_visible({"visible": True}) + + assert result is None + assert log.debug.call_count == 1 + + +def test_set_bounding_box_visibility_is_a_logged_no_op_when_no_coordinator_injected( + app_without_api, +): + """With nothing injected the trigger logs and returns without raising.""" + with patch("ansys.visor.viewer.app.trame.local_app.logger") as log: + result = app_without_api.set_bounding_box_visibility({"visible": True}) + + assert result is None + assert log.debug.call_count == 1 + + +# =========================================================================== +# Registration +# =========================================================================== + +def test_widget_trigger_names_are_registered_after_decoration(app, mock_server): + """All three names survive the payload decorator and take a raw dict. + + ``@trigger`` is outermost above ``@parse_payload``; this asserts the pair + registers under the name rather than under the wrapper, and that the + registered callable accepts the raw dict the client sends. + """ + names = [call.args[0] for call in mock_server.trigger.call_args_list] + functions = [ + call.args[0] for call in mock_server.trigger.return_value.call_args_list + ] + registered = dict(zip(names, functions)) + + assert CROSS_SECTION_TRIGGER in registered + assert EDGES_TRIGGER in registered + assert BOUNDING_BOX_TRIGGER in registered + assert registered[EDGES_TRIGGER]({"visible": False}) is None + diff --git a/tests/unit/renderer/test_local_renderer.py b/tests/unit/renderer/test_local_renderer.py index 09073046..e17d604e 100644 --- a/tests/unit/renderer/test_local_renderer.py +++ b/tests/unit/renderer/test_local_renderer.py @@ -453,6 +453,51 @@ def test_refresh_color_variable_range(self, renderer): ) +# =========================================================================== +# 3a-bis. Scene-wide widget state: set_edges_visible +# +# The VTK effect lives on VtkNodePipeline and is asserted in +# tests/unit/vtk/test_node_pipeline.py. What is asserted here is the +# fan-out: every registered pipeline, no node-id resolution, and no +# guard branch for an empty registry. +# =========================================================================== + +class TestGlobalEdgeVisibility: + + def test_set_edges_visible_fans_out_over_every_pipeline(self, renderer): + """Every registered pipeline is told, with the value as given. + + Three pipelines under non-contiguous ids, so a body that iterated a + range or resolved a node id rather than iterating the registry's + values fails here. + """ + pipes = {4: MagicMock(name="pipe-4"), 9: MagicMock(name="pipe-9"), + 17: MagicMock(name="pipe-17")} + renderer._pipelines.update(pipes) + + assert renderer.set_edges_visible(True) is None + + for pipe in pipes.values(): + pipe.set_edge_visibility.assert_called_once_with(True) + + def test_set_edges_visible_with_no_pipelines_is_a_no_op(self, renderer): + """An empty scene is a no-op by iteration, not by guard. + + Pinned because the contract says there is no logged-no-op branch here: + a later session adding one would be adding a branch that can only ever + be wrong, and this test says the empty case is already handled. + """ + renderer._pipelines.clear() + + with patch( + "ansys.visor.viewer.renderer.local_renderer.logger" + ) as mock_logger: + assert renderer.set_edges_visible(True) is None + + assert mock_logger.debug.call_count == 0 + assert mock_logger.warning.call_count == 0 + + # =========================================================================== # 3b. Delegated apply bodies: visibility, opacity, diffuse colour, # selection, colour variable diff --git a/tests/unit/vtk/scene/test_base.py b/tests/unit/vtk/scene/test_base.py index b4cda515..77b5bfdc 100644 --- a/tests/unit/vtk/scene/test_base.py +++ b/tests/unit/vtk/scene/test_base.py @@ -1912,4 +1912,446 @@ def test_apply_state_component_without_variable_id_does_not_clear( assert pipeline.mapper.GetScalarVisibility() == 1 +# =========================================================================== +# Widget state -- the server store, the coordinator surface, and delivery +# +# Three toggles the server now holds: cross-section, edges, bounding box. +# Each has a coordinator method that writes the store and applies to the +# renderer under ``_vtk_lock``; ``get_state`` reads the store rather than the +# browser's reply; ``apply_state`` restores it; ``get_scene_details`` delivers +# it to a rebuilt or reconnecting client. +# +# Every expected value below is a hand-written literal. ``True`` is used +# throughout as the value the client reports, because the server's own default +# is the literal ``False``, so a body that wrote a default rather than its +# argument fails. +# =========================================================================== + +TOGGLE_ON = True +TOGGLE_OFF = False + +# 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_TOGGLE = False + +DELIVERED_PARALLEL_PROJECTION = True + + +class _ToggleSpyRenderer: + """Records the three widget-state calls, with the lock depth at each. + + Hand-written rather than a MagicMock so that a call under some other name + is an AttributeError here rather than a silently absorbed no-op, and so + that the lock depth can be read at the moment of the call rather than + after the fact. + """ + + def __init__(self, scene): + self._scene = scene + self.calls = [] + self.depths = {} + + def _record(self, name, visible): + self.calls.append((name, visible)) + self.depths[name] = getattr(self._scene._vtk_lock, "depth", None) + + def set_cross_section_visibility(self, visible): + self._record("set_cross_section_visibility", visible) + + def set_edges_visible(self, visible): + self._record("set_edges_visible", visible) + + def set_bounding_box_visibility(self, visible): + self._record("set_bounding_box_visibility", visible) + + +class _SceneDetailsRenderer: + """Renderer double for the scene-details delivery path. + + Answers only what ``get_scene_details`` asks of a renderer: the annotation + (``None`` -- wire absence is the contract for a renderer with no handles) + and the camera record. A ``MagicMock`` cannot stand in here: the values it + returns are validated by pydantic, not merely called. + """ + + def __init__(self, camera_record): + self._camera_record = camera_record + self.camera_reads = 0 + + def build_renderer_annotation(self): + return None + + def get_camera_state(self): + self.camera_reads += 1 + return self._camera_record + + +def _delivered_camera() -> VisorCameraState: + """A camera record whose projection is a hand-written literal.""" + return VisorCameraState( + position=[41.0, 42.0, 43.0], + focal_point=[44.0, 45.0, 46.0], + view_up=[0.0, 1.0, 0.0], + clipping_range=[47.0, 48.0], + parallel_projection=DELIVERED_PARALLEL_PROJECTION, + view_angle=36.0, + parallel_scale=49.0, + ) + + +def _toggle_save_scene(scene, reply_toggle): + """Wire *scene* for a save whose browser reply carries *reply_toggle*. + + The registry is emptied so the mapper's per-dataset loop contributes + nothing; what is under test is the scene block of the persisted state. + """ + scene._dataset_registry = VisorDatasetRegistry() + + async def _get_runtime_state_async(timeout): + return RuntimeAppState.from_components( + dark_mode=False, + unit="m", + dataset_states={}, + cross_section_enabled=reply_toggle, + edges_enabled=reply_toggle, + bounding_box_enabled=reply_toggle, + ) + + scene._get_runtime_state_async = _get_runtime_state_async + + +# --------------------------------------------------------------------------- +# The store's initial value +# --------------------------------------------------------------------------- + +def test_widget_toggles_default_to_false_before_any_client_speaks(scene): + """All three start at the client widgets' own constructor default. + + ``False`` and not ``None``: ``None`` would mean "nobody has said", which is + only ever true before the first save, and it would push a three-way branch + into get_state for a state that resolves itself on the first toolbar click. + The literal here is hand-written and is never read from a widget. + """ + assert scene._cross_section_enabled is False + assert scene._edges_enabled is False + assert scene._bounding_box_enabled is False + + +# --------------------------------------------------------------------------- +# The coordinator surface -- store half and renderer half, separately +# --------------------------------------------------------------------------- + +def test_set_cross_section_visibility_writes_the_store(scene): + """Store half: the coordinator records what it was told.""" + scene.set_cross_section_visibility(TOGGLE_ON) + + assert scene._cross_section_enabled is True + + +def test_set_cross_section_visibility_applies_to_the_renderer(scene): + """Renderer half: the value is passed through, once. + + Its own test rather than an extra assertion above: either half can + silently do nothing while the other succeeds, which is the posture the + per-part surface above is tested with. + """ + spy = _ToggleSpyRenderer(scene) + scene._renderer = spy + + scene.set_cross_section_visibility(TOGGLE_ON) + + assert spy.calls == [("set_cross_section_visibility", True)] + + +def test_set_edges_visible_writes_the_store(scene): + """Store half: the coordinator records what it was told.""" + scene.set_edges_visible(TOGGLE_ON) + + assert scene._edges_enabled is True + + +def test_set_edges_visible_applies_to_the_renderer(scene): + """Renderer half: the value reaches the renderer's scene-wide verb.""" + spy = _ToggleSpyRenderer(scene) + scene._renderer = spy + + scene.set_edges_visible(TOGGLE_ON) + + assert spy.calls == [("set_edges_visible", True)] + + +def test_set_bounding_box_visibility_writes_the_store(scene): + """Store half: the coordinator records what it was told.""" + scene.set_bounding_box_visibility(TOGGLE_ON) + + assert scene._bounding_box_enabled is True + + +def test_set_bounding_box_visibility_applies_to_the_renderer(scene): + """Renderer half: the value is passed through, once.""" + spy = _ToggleSpyRenderer(scene) + scene._renderer = spy + + scene.set_bounding_box_visibility(TOGGLE_ON) + + assert spy.calls == [("set_bounding_box_visibility", True)] + + +def test_set_edges_visible_holds_the_lock_at_the_renderer_call(scene): + """The lock is *held* at the moment the renderer is called. + + This is the assertion that pins AC-2's lock half, and it is the only + assertion in this increment that sees the lock at all: the trigger module + sees delegation, and get_state and apply_state take the lock on paths that + already had it. Remove ``with self._vtk_lock:`` from the coordinator + method and the probe records 0. + + The trigger handler runs on trame's daemon thread while the VTK objects it + mutates belong to the caller's thread; an apply outside the lock would + interleave with another thread's mutation. That failure is intermittent + and never reproduces under a gate. + """ + scene._vtk_lock = _LockSpy() + spy = _ToggleSpyRenderer(scene) + scene._renderer = spy + + scene.set_edges_visible(TOGGLE_ON) + + assert spy.depths["set_edges_visible"] >= 1 + assert scene._vtk_lock.depth == 0 + assert scene._vtk_lock.enter_count == scene._vtk_lock.exit_count + + +# --------------------------------------------------------------------------- +# get_state -- the toggles come from the store, not from the reply +# --------------------------------------------------------------------------- + +def test_get_state_takes_the_toggles_from_the_store(scene): + """The saved toggles are the server's, with the browser saying otherwise. + + This is the assertion that pins the change. The reply carries the + hand-written literal ``False`` for all three while the store holds ``True`` + for all three; revert the three get_state assignments and every assertion + below reports ``False``. That single difference is what separates + "server-authoritative" from "round-trips the client's answer". + + Asserted on what get_state RETURNS -- the object that reaches the writer -- + not on the runtime state it was built from. + """ + scene.set_cross_section_visibility(TOGGLE_ON) + scene.set_edges_visible(TOGGLE_ON) + scene.set_bounding_box_visibility(TOGGLE_ON) + _toggle_save_scene(scene, REPLY_TOGGLE) + + persisted = asyncio.run(scene.get_state(timeout=1.0)) + + assert persisted.scene.cross_section_enabled is True + assert persisted.scene.edges_enabled is True + assert persisted.scene.bounding_box_enabled is True + + +def test_get_state_discards_the_toggles_the_browser_returned(scene): + """The browser's toggles do not survive into the persisted state. + + The mirror of the test above and its own test for the same reason the + camera pair is split: "wrote the store" and "did not write the reply" are + the same only while the round trip still carries toggles at all, and the + round trip is not being removed. Here the store is left at its default + ``False`` and the reply carries ``True``. + """ + _toggle_save_scene(scene, True) + + persisted = asyncio.run(scene.get_state(timeout=1.0)) + + assert persisted.scene.cross_section_enabled is False + assert persisted.scene.edges_enabled is False + assert persisted.scene.bounding_box_enabled is False + + +# --------------------------------------------------------------------------- +# apply_state -- the load path writes the store and the renderer +# --------------------------------------------------------------------------- + +def _toggle_runtime_state(**toggles): + """A real RuntimeAppState carrying only the toggles named.""" + return RuntimeAppState.from_components( + dark_mode=False, + unit="m", + dataset_states={}, + **toggles, + ) + + +def test_apply_state_writes_the_toggles_to_the_store(scene): + """A state carrying toggles becomes the server's store.""" + _apply( + scene, + _toggle_runtime_state( + cross_section_enabled=True, + edges_enabled=True, + bounding_box_enabled=True, + ), + ) + + assert scene._cross_section_enabled is True + assert scene._edges_enabled is True + assert scene._bounding_box_enabled is True + + +def test_apply_state_applies_the_toggles_to_the_renderer(scene): + """The load path applies as well as records, in that order.""" + spy = _ToggleSpyRenderer(scene) + scene._renderer = spy + + _apply( + scene, + _toggle_runtime_state( + cross_section_enabled=True, + edges_enabled=True, + bounding_box_enabled=True, + ), + ) + + assert spy.calls == [ + ("set_cross_section_visibility", True), + ("set_edges_visible", True), + ("set_bounding_box_visibility", True), + ] + + +def test_apply_state_absent_toggles_leave_the_store_untouched(scene): + """Absent says nothing: a state with no toggles is not a state of False. + + The guard belongs to this path and not to get_state, and this is the test + that says so. The store is seeded to ``True`` first, so a body that wrote + the model's own ``None`` default through would be caught. + """ + scene.set_cross_section_visibility(TOGGLE_ON) + scene.set_edges_visible(TOGGLE_ON) + scene.set_bounding_box_visibility(TOGGLE_ON) + spy = _ToggleSpyRenderer(scene) + scene._renderer = spy + + _apply(scene, _toggle_runtime_state()) + + assert scene._cross_section_enabled is True + assert scene._edges_enabled is True + assert scene._bounding_box_enabled is True + assert spy.calls == [] + + +def test_apply_state_ignores_orthographic_enabled(scene): + """The persisted projection toggle is emitted, never read back. + + Two independent fields writing one camera property is the divergence this + story removes; projection arrives on the camera and nowhere else. Pinned + so that a later session does not restore the read as a bug fix: nothing on + the scene stores it, and a state carrying only that field changes nothing. + """ + spy = _ToggleSpyRenderer(scene) + scene._renderer = spy + + _apply(scene, _toggle_runtime_state(orthographic_enabled=True)) + + assert spy.calls == [] + assert not hasattr(scene, "_orthographic_enabled") + + +# --------------------------------------------------------------------------- +# get_scene_details -- delivery to a rebuilt or reconnecting client +# --------------------------------------------------------------------------- + +def test_get_scene_details_delivers_the_four_widget_keywords(scene): + """All four keywords reach the payload the client is served. + + This is the assertion that pins delivery, and it covers all four rather + than one: revert any single keyword out of the ``from_components`` call and + that field is delivered as ``None``, which reverts its toggle in the + browser on the next rebuild. Without this, only a manual check would see + it. + + ``orthographic_enabled`` is derived from the camera record, so the double + answers with a record whose ``parallel_projection`` is a hand-written + literal; nothing here is read from a vtkCamera. + """ + scene.set_cross_section_visibility(TOGGLE_ON) + scene.set_edges_visible(TOGGLE_ON) + scene.set_bounding_box_visibility(TOGGLE_ON) + scene._dataset_registry = VisorDatasetRegistry() + double = _SceneDetailsRenderer(_delivered_camera()) + scene._renderer = double + + delivered = scene.get_scene_details().app_state.scene + + assert delivered.cross_section_enabled is True + assert delivered.edges_enabled is True + assert delivered.bounding_box_enabled is True + assert delivered.orthographic_enabled is DELIVERED_PARALLEL_PROJECTION + assert double.camera_reads == 1 + + +def test_get_scene_details_delivers_no_orthographic_flag_when_the_camera_record_is_empty( + scene, +): + """A ``None`` record delivers ``None``, not a fabricated ``False``. + + ``None`` on the wire says "nothing was ever written" and leaves the + client's own flag alone; ``False`` would assert perspective over a client + that may be parallel. The three store toggles still deliver, because they + do not depend on the camera. + """ + scene.set_edges_visible(TOGGLE_ON) + scene._dataset_registry = VisorDatasetRegistry() + scene._renderer = _SceneDetailsRenderer(None) + + delivered = scene.get_scene_details().app_state.scene + + assert delivered.orthographic_enabled is None + assert delivered.edges_enabled is True + + +# --------------------------------------------------------------------------- +# set_part_visibility -- the bounds fan-out (R4) +# --------------------------------------------------------------------------- + +def test_set_part_visibility_fans_out_to_update_bounds_and_actor_count(scene, pipeline): + """Hiding a part reaches the widget layer, after the apply. + + This is the assertion that pins AC-4. Revert the two calls and the + recorder is empty. Asserted on the renderer's ``update_bounds`` and + ``update_actor_count`` -- the surface MC-3 reads in the log -- reached + through the scene's real private helpers, which are deliberately not + patched: patching them would move the assertion off that surface. + + Order matters and is asserted: a fan-out before the apply would push the + pre-mutation actor count. + """ + order = [] + scene._renderer.update_bounds = lambda bounds: order.append("update_bounds") + scene._renderer.update_actor_count = lambda count: order.append("update_actor_count") + real_apply = scene._renderer.apply_visibility + scene._renderer.apply_visibility = lambda node_id, visible: ( + order.append("apply_visibility"), real_apply(node_id, visible) + )[1] + + scene.set_part_visibility(NODE_ID, False) + + assert order == ["apply_visibility", "update_bounds", "update_actor_count"] + + +def test_set_part_visibility_unknown_node_id_does_not_fan_out(scene): + """The early return still skips the fan-out. + + The two calls sit inside the lock block after the renderer write, so a node + no dataset owns costs nothing. A fan-out hoisted above the guard would + refresh the widgets on every stray trigger. + """ + calls = [] + scene._renderer.update_bounds = lambda bounds: calls.append("update_bounds") + scene._renderer.update_actor_count = lambda count: calls.append("update_actor_count") + + with patch("ansys.visor.viewer.vtk.scene.base.logger"): + scene.set_part_visibility(UNKNOWN_NODE_ID, False) + assert calls == [] diff --git a/tests/unit/vtk/scene/test_local_scene.py b/tests/unit/vtk/scene/test_local_scene.py index d3a36dd9..f5a01404 100644 --- a/tests/unit/vtk/scene/test_local_scene.py +++ b/tests/unit/vtk/scene/test_local_scene.py @@ -114,6 +114,11 @@ def pipeline_instance(): bounding_box_axes_actor_id=32, ), ) + # get_camera_state() must return a real record or None, since + # get_scene_details derives orthographic_enabled from it and the result is + # validated as ``bool | None``. A bare MagicMock absorbs method calls + # silently, but not values pydantic validates. + mock_renderer.get_camera_state.return_value = None # frontend_ref_name must be a real str, since it is passed to # VisorFrontendBridge's constructor. mock_renderer.frontend_ref_name = "test-ref-name" diff --git a/tests/unit/vtk/test_node_pipeline.py b/tests/unit/vtk/test_node_pipeline.py index 5a03955a..546f3590 100644 --- a/tests/unit/vtk/test_node_pipeline.py +++ b/tests/unit/vtk/test_node_pipeline.py @@ -246,6 +246,50 @@ def test_set_visibility_false_hides_actor(poly_dataset): assert pipe.actor.GetVisibility() == 0 +# --------------------------------------------------------------------------- +# set_edge_visibility +# +# The property flag, not the actor flag. Both literals are hand-written: VTK +# stores edge visibility as an int, and the assertions are against 1 and 0 +# rather than against a bool, so a body that relied on bool-to-int coercion +# and a body that wrote the wrong object are distinguishable. +# --------------------------------------------------------------------------- + +def test_set_edge_visibility_true_sets_property_edge_visibility(poly_dataset): + """set_edge_visibility(True) sets the property's edge-visibility flag.""" + pipe = VtkNodePipeline.from_dataset(poly_dataset) + pipe.actor.GetProperty().SetEdgeVisibility(0) + + pipe.set_edge_visibility(True) + + assert pipe.actor.GetProperty().GetEdgeVisibility() == 1 + + +def test_set_edge_visibility_false_clears_property_edge_visibility(poly_dataset): + """set_edge_visibility(False) clears the property's edge-visibility flag.""" + pipe = VtkNodePipeline.from_dataset(poly_dataset) + pipe.actor.GetProperty().SetEdgeVisibility(1) + + pipe.set_edge_visibility(False) + + assert pipe.actor.GetProperty().GetEdgeVisibility() == 0 + + +def test_set_edge_visibility_does_not_touch_actor_visibility(poly_dataset): + """Edges are a property flag; the part's own visibility is untouched. + + The two are one keystroke apart on the actor and this is the test that + separates them: writing ``SetVisibility`` instead would hide the part and + still pass a test that only read the edge flag back. + """ + pipe = VtkNodePipeline.from_dataset(poly_dataset) + pipe.actor.SetVisibility(0) + + pipe.set_edge_visibility(True) + + assert pipe.actor.GetVisibility() == 0 + + # --------------------------------------------------------------------------- # set_opacity # --------------------------------------------------------------------------- diff --git a/tests/unit/vtk/test_wire_format_identity.py b/tests/unit/vtk/test_wire_format_identity.py index 084efaee..81ea3757 100644 --- a/tests/unit/vtk/test_wire_format_identity.py +++ b/tests/unit/vtk/test_wire_format_identity.py @@ -120,6 +120,17 @@ def build_renderer_annotation(self) -> WasmRendererAnnotation: ) return WasmRendererAnnotation(nodes=nodes, widgets=widgets) + def get_camera_state(self): + """``None``: this stub has no camera record. + + ``get_scene_details`` derives the delivered ``orthographicEnabled`` + from the camera record, so it reads this even though nothing in this + module is about the camera. ``None`` is the honest answer for a + renderer that never wrote a camera, and it is what the real renderer + answers before the first reset. + """ + return None + class _WireFormatScene(VisorSceneBase): """Minimal concrete ``VisorSceneBase`` for exercising the real From ce25308d7f65db6a6d806f55484bb684444885ae Mon Sep 17 00:00:00 2001 From: Laura Kasian Date: Fri, 18 Sep 2026 10:22:45 -0700 Subject: [PATCH 02/13] feat: logging triggers on backend --- src/ansys/visor/viewer/app/trame/local_app.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ansys/visor/viewer/app/trame/local_app.py b/src/ansys/visor/viewer/app/trame/local_app.py index bd03a0f3..5f13a492 100644 --- a/src/ansys/visor/viewer/app/trame/local_app.py +++ b/src/ansys/visor/viewer/app/trame/local_app.py @@ -352,6 +352,7 @@ def perf_report_server_update(self, payload: dict): def _part_state_api(self, trigger_name: str) -> ScenePartStateApi | None: """Return the injected coordinator, or ``None`` after logging.""" + logger.debug("[trigger] %s arrived.", trigger_name) if self._scene_part_state_api is None: logger.debug("%s: no scene part-state API injected; ignoring.", trigger_name) return None From 0fb590120802a6e0bcdbffe0356cac9c527d65c0 Mon Sep 17 00:00:00 2001 From: Laura Kasian Date: Fri, 18 Sep 2026 12:15:08 -0700 Subject: [PATCH 03/13] feat: set_projection trigger writes the camera record --- src/ansys/visor/viewer/app/trame/local_app.py | 38 +++- .../runtime/requests/widget_state_payloads.py | 20 +- src/ansys/visor/viewer/renderer/base.py | 19 ++ .../visor/viewer/renderer/local_renderer.py | 14 ++ .../visor/viewer/renderer/null_renderer.py | 15 ++ src/ansys/visor/viewer/vtk/scene/base.py | 41 +++- .../unit/app/test_local_app_set_projection.py | 209 +++++++++++++++++ tests/unit/renderer/test_local_renderer.py | 116 ++++++++++ tests/unit/renderer/test_null_renderer.py | 44 ++++ tests/unit/vtk/scene/test_base.py | 213 ++++++++++++++++++ 10 files changed, 718 insertions(+), 11 deletions(-) create mode 100644 tests/unit/app/test_local_app_set_projection.py diff --git a/src/ansys/visor/viewer/app/trame/local_app.py b/src/ansys/visor/viewer/app/trame/local_app.py index 5f13a492..6a7fa0e3 100644 --- a/src/ansys/visor/viewer/app/trame/local_app.py +++ b/src/ansys/visor/viewer/app/trame/local_app.py @@ -17,6 +17,7 @@ SetBoundingBoxVisibilityPayload, SetCrossSectionVisibilityPayload, SetEdgesVisiblePayload, + SetProjectionPayload, ) logger = VisorDefaultLogger(__name__) @@ -33,11 +34,11 @@ class ScenePartStateApi(Protocol): importing the scene keeps this module free of any scene type, so the injected object remains LocalApp's only route to the scene. - Not all of it is per-part, and the name is historical. ``sync_camera`` - and the three widget-state toggles are scene-wide, and they are declared - here anyway: the one production injection site passes the whole scene - coordinator, so a second protocol would be the same object under a second - name. Read this as the whole coordinator surface LocalApp calls, not only + Not all of it is per-part, and the name is historical. ``sync_camera``, + the three widget-state toggles and ``set_projection`` are scene-wide, and + they are declared here anyway: the one production injection site passes + the whole scene coordinator, so a second protocol would be the same object + under a second name. Read this as the whole coordinator surface LocalApp calls, not only the per-part part of it. """ @@ -70,6 +71,8 @@ def set_edges_visible(self, visible: bool) -> None: ... def set_bounding_box_visibility(self, visible: bool) -> None: ... + def set_projection(self, parallel: bool) -> None: ... + # ---------------------------------------------------------------------- # Trigger payload models @@ -201,6 +204,7 @@ class LocalApp: set_cross_section_visibility: shows or hides the cross-section plane set_edges_visible: shows or hides edges on every part set_bounding_box_visibility: shows or hides the bounding-box outline + set_projection: sets parallel or perspective projection on the camera record 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. @@ -505,8 +509,14 @@ def sync_camera(self, payload) -> None: # work, not this one's. # # Payloads are validated at this boundary by ``@parse_payload`` exactly - # as the per-part ones are. The wire key is ``visible`` on all three and - # carries no pydantic alias: snake_case and camelCase coincide. + # as the per-part ones are. The wire key is ``visible`` on the three + # visibility triggers and ``parallel`` on ``set_projection``; none carries + # a pydantic alias, snake_case and camelCase coinciding on all four. + # + # ``set_projection`` sits in this block because it arrives from the same + # toolbar and under the same absolute-value rule, but it is not a fourth + # toggle: the server keeps no projection field, the camera record holds + # it, and the coordinator re-serialises the camera as part of the write. # ------------------------------------------------------------------ @trigger("set_cross_section_visibility") @@ -536,6 +546,20 @@ def set_bounding_box_visibility(self, payload) -> None: return api.set_bounding_box_visibility(payload.visible) + @trigger("set_projection") + @parse_payload(SetProjectionPayload) + def set_projection(self, payload) -> None: + """Frontend -> Backend: set parallel or perspective projection. + + The projection is the camera record's field, not a toggle of its + own: the coordinator writes the record and re-serialises the + camera in one critical section. + """ + api = self._part_state_api("set_projection") + if api is None: + return + api.set_projection(payload.parallel) + 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 a2badacd..5f37cd19 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 @@ -1,8 +1,15 @@ """Models for the widget-state trigger payloads. -One model per server-tracked widget toggle. Field names are already -identical in snake_case and camelCase, so **no** ``Field(alias=...)`` is -needed and none is to be added: the wire key is exactly ``visible``. +One model per server-tracked widget toggle, plus the projection. Field +names are already identical in snake_case and camelCase, so **no** +``Field(alias=...)`` is needed and none is to be added: the wire key is +exactly ``visible`` on the three visibility payloads and exactly +``parallel`` on ``SetProjectionPayload``. + +Projection is not a fourth toggle. It has no store field on the scene: it +is the camera record's ``parallel_projection``, written through the +renderer and derived back out in ``get_state``, so that the projection has +exactly one source. These live here rather than inline in ``local_app.py`` beside the six per-part payload models, whose own block comment scopes itself to @@ -41,3 +48,10 @@ class SetBoundingBoxVisibilityPayload(BaseModel): visible: bool + +class SetProjectionPayload(BaseModel): + """Payload of the ``set_projection`` trigger.""" + + model_config = ConfigDict(populate_by_name=True) + + parallel: bool diff --git a/src/ansys/visor/viewer/renderer/base.py b/src/ansys/visor/viewer/renderer/base.py index b69e925f..108b12cb 100644 --- a/src/ansys/visor/viewer/renderer/base.py +++ b/src/ansys/visor/viewer/renderer/base.py @@ -202,6 +202,25 @@ def sync_camera(self, camera_state: "VisorCameraState") -> None: on object identity through :meth:`get_camera_state`. """ + @abstractmethod + def set_projection(self, parallel: bool) -> None: + """Set parallel projection on the camera record, and project it. + + Writes ``parallel_projection`` on the existing record **in place**, + preserving the object identity :meth:`sync_camera` documents, then + applies to the pipeline camera. Record first, pipeline second, so a + raising VTK setter still leaves the record holding what the caller + asked for. + + A ``None`` record is not seeded here. The write to the pipeline + still happens and the record stays ``None``, logged at debug; the + next :meth:`reset_camera` reads the pipeline and imports it. The + cost is named: a projection set before any camera has been written + is not saved until then. + + Re-serialisation is the coordinator's, not this method's. + """ + @abstractmethod def serialize_camera_state(self) -> None: """Make the state served to the client current for the camera. diff --git a/src/ansys/visor/viewer/renderer/local_renderer.py b/src/ansys/visor/viewer/renderer/local_renderer.py index b83815ff..76178e49 100644 --- a/src/ansys/visor/viewer/renderer/local_renderer.py +++ b/src/ansys/visor/viewer/renderer/local_renderer.py @@ -302,6 +302,20 @@ def sync_camera(self, camera_state: "VisorCameraState") -> None: self._last_camera_state = camera_state self._apply_to_pipeline_camera(camera_state) + def set_projection(self, parallel: bool) -> None: + """See :meth:`IRenderer.set_projection`. + + Record first, pipeline second, for the reason :meth:`sync_camera` + gives. + """ + if self._last_camera_state is not None: + self._last_camera_state.parallel_projection = parallel + else: + logger.debug( + "set_projection: no camera record; applying to the pipeline only." + ) + self._vtk_renderer.GetActiveCamera().SetParallelProjection(parallel) + def serialize_camera_state(self) -> None: """See :meth:`IRenderer.serialize_camera_state`. diff --git a/src/ansys/visor/viewer/renderer/null_renderer.py b/src/ansys/visor/viewer/renderer/null_renderer.py index cb9880f4..44c3b278 100644 --- a/src/ansys/visor/viewer/renderer/null_renderer.py +++ b/src/ansys/visor/viewer/renderer/null_renderer.py @@ -18,6 +18,7 @@ from typing import TYPE_CHECKING, Optional +from ansys.visor.viewer.core.visor_logging import VisorDefaultLogger from ansys.visor.viewer.renderer.base import IRenderer if TYPE_CHECKING: @@ -26,6 +27,8 @@ from ansys.visor.viewer.models.common.visor_camera_state import VisorCameraState from ansys.visor.viewer.vtk.scene_graph import VisorSceneGraphPartNode +logger = VisorDefaultLogger(__name__) + class NullRenderer(IRenderer): """Null-object implementation of :class:`IRenderer` for use in tests.""" @@ -127,6 +130,18 @@ def sync_camera(self, camera_state: "VisorCameraState") -> None: """See :meth:`IRenderer.sync_camera`.""" self._last_camera_state = camera_state + def set_projection(self, parallel: bool) -> None: + """See :meth:`IRenderer.set_projection`. + + Record only: there is no pipeline camera to project onto. + """ + if self._last_camera_state is not None: + self._last_camera_state.parallel_projection = parallel + else: + logger.debug( + "set_projection: no camera record; nothing to write." + ) + def serialize_camera_state(self) -> None: """See :meth:`IRenderer.serialize_camera_state`. diff --git a/src/ansys/visor/viewer/vtk/scene/base.py b/src/ansys/visor/viewer/vtk/scene/base.py index 04606946..55467e56 100644 --- a/src/ansys/visor/viewer/vtk/scene/base.py +++ b/src/ansys/visor/viewer/vtk/scene/base.py @@ -186,6 +186,18 @@ async def get_state(self, timeout: float) -> PersistedViewerStateV1: written, and writing that ``None`` through is what says so; the guard for "absent says nothing" belongs to the load path, in :meth:`apply_state`, not here. + + ``orthographic_enabled`` is derived from that same record rather than + stored, and it is **emitted here and not read on load**. The record + is bound once and read once: the camera and the projection cannot + disagree because there is nothing for them to disagree about. The + load path takes projection off ``camera.parallel_projection`` alone + and ignores this field, because two independent fields writing one + camera property is exactly the divergence that made a saved file + report one projection while the view showed the other. A ``None`` + record emits ``None``, which says "nothing was ever written" rather + than asserting perspective. Restoring the load-side read would + reopen the divergence; it is not a missing feature. """ runtime_state = await self._get_runtime_state_async(timeout) @@ -194,7 +206,11 @@ async def get_state(self, timeout: float) -> PersistedViewerStateV1: dataset_id: dataset_state.model_copy(deep=True) for dataset_id, dataset_state in self._dataset_registry.runtime_state_dict.items() } - runtime_state.scene.camera = self._renderer.get_camera_state() + camera_record = self._renderer.get_camera_state() + runtime_state.scene.camera = camera_record + runtime_state.scene.orthographic_enabled = ( + camera_record.parallel_projection if camera_record is not None else None + ) 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 @@ -674,6 +690,29 @@ def set_bounding_box_visibility(self, visible: bool) -> None: self._bounding_box_enabled = visible self._renderer.set_bounding_box_visibility(visible) + def set_projection(self, parallel: bool) -> None: + """Set parallel or perspective projection on the camera record. + + No store field, and that is the point: projection lives on the camera + record and nowhere else, so ``get_state`` derives it rather than + reading a second copy that could disagree. + + Both halves run in one critical section and the re-serialisation is + part of the write, exactly as in :meth:`sync_camera`: the backend + advertises a version number read from the live VTK object while + serving content from a cache, so a write with no re-serialise + publishes a new version against old content and the client fetches + and re-applies the pre-write camera. Because a projection flip is + visually obvious, omitting the re-serialise shows up as the view + snapping back. + + No notify. No ``render()``, no ``flush_wasm_state()``, no + ``set_state``. + """ + with self._vtk_lock: + self._renderer.set_projection(parallel) + self._renderer.serialize_camera_state() + def _restore_part_states(self, runtime_app_state: "RuntimeAppState") -> None: """ Restore per-part state from a runtime app state, on the load path. diff --git a/tests/unit/app/test_local_app_set_projection.py b/tests/unit/app/test_local_app_set_projection.py new file mode 100644 index 00000000..0b89f9e2 --- /dev/null +++ b/tests/unit/app/test_local_app_set_projection.py @@ -0,0 +1,209 @@ +"""Unit tests for ``LocalApp.set_projection`` -- the projection trigger. + +A module of its own rather than an addition to ``test_local_app.py``, on the +precedent ``test_local_app_sync_camera.py`` set: that module's +``TRIGGER_NAMES`` list drives parametrised tests whose meaning is "one of the +six per-part triggers", and every per-part payload model carries +``node_id: int = Field(alias="nodeId")``. ``set_projection`` carries no node +id -- it carries a single ``parallel`` boolean and is scene-wide -- so adding a +name there would multiply the per-part tests by one more and rewrite them. + +It is separate from ``test_local_app_widget_triggers.py`` as well, and +deliberately: those three are toggles the server stores, this one is the +camera record's field, and the two increments that added them have different +failure modes. + +Coverage targets +---------------- +1. The trigger delegates the value it arrived with, exactly once, to the + identically-named coordinator method -- for ``True`` and for ``False``, so + a handler that forwarded a constant fails. +2. A payload missing ``parallel`` is a logged no-op: nothing is delegated. +3. A payload that is not a mapping at all is a logged no-op, not a + ``TypeError`` -- the ``model_validate`` posture the payload decorator + documents. +4. With no coordinator injected, the trigger logs and returns. +5. The trigger name survives decoration and is registered with the server. + +Every payload here is a hand-written literal. The wire key is ``parallel`` +and carries no alias: snake_case and camelCase coincide, which is itself +asserted by the delegation tests passing a literal ``{"parallel": ...}``. +""" + +from unittest.mock import MagicMock, patch + +import pytest + +from ansys.visor.viewer.app.trame.local_app import LocalApp + +# The trigger name, written out rather than imported, so that a rename on the +# production side fails here by name instead of following along. +PROJECTION_TRIGGER = "set_projection" + + +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_part_state_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_part_state_api=api, + ) + + +@pytest.fixture +def app_without_api(mock_server): + """LocalApp with no coordinator injected.""" + return LocalApp( + server=mock_server, + get_scene_details_json=MagicMock(), + handle_save_state_response=MagicMock(), + standalone=True, + ) + + +# =========================================================================== +# Delegation +# +# Both values are exercised. Projection has no server-side store and so no +# default to differ from, which is exactly why the pair is needed here: a +# handler that forwarded a constant would pass either test alone. +# =========================================================================== + +def test_set_projection_delegates_the_parallel_value(app, api): + """The trigger hands the coordinator the value it received.""" + result = app.set_projection({"parallel": True}) + + assert result is None + api.set_projection.assert_called_once_with(True) + + +def test_set_projection_delegates_a_false_value(app, api): + """Perspective travels the same path as parallel, and is not a no-op. + + The absolute-value rule applies here as everywhere on this boundary: the + client sends the value its widget settled on, never a toggle, so "turn it + off" is a message with a value and not an absent message. + """ + result = app.set_projection({"parallel": False}) + + assert result is None + api.set_projection.assert_called_once_with(False) + + +# =========================================================================== +# Validation -- a malformed payload reaches no handler body +# =========================================================================== + +def test_set_projection_missing_parallel_is_a_logged_no_op(app, api): + """An empty payload never reaches the handler body. + + Asserted against the whole mock rather than one method name, so a leak + under any other name still fails. + """ + with patch("ansys.visor.viewer.app.trame.local_app.logger") as log: + result = app.set_projection({}) + + assert result is None + assert api.mock_calls == [] + assert log.warning.call_count == 1 + + +def test_set_projection_non_mapping_payload_is_a_logged_no_op(app, api): + """A bare string is a ValidationError, not a TypeError. + + The payload decorator uses ``model_validate`` rather than + ``model(**payload)`` precisely so that a payload which is not a mapping at + all is caught by the same guard as a payload with a missing key. + """ + with patch("ansys.visor.viewer.app.trame.local_app.logger") as log: + result = app.set_projection("parallel") + + assert result is None + assert api.mock_calls == [] + assert log.warning.call_count == 1 + + +# =========================================================================== +# No coordinator injected +# =========================================================================== + +def test_set_projection_is_a_logged_no_op_when_no_coordinator_injected(app_without_api): + """With nothing injected the trigger logs and returns without raising. + + Two debug lines, both from ``_part_state_api`` and neither from this + handler: the arrival line every trigger emits, and the "no scene + part-state API injected" line that says why nothing was delegated. The + handler adds no logging of its own, so counting arrivals in the server log + stays a sound measurement, and the trigger name appears on both lines so + the count is attributable. + + The literal ``2`` is hand-written from what ``_part_state_api`` does, not + copied from the neighbouring trigger modules, which assert ``1`` and are + red at this increment's base commit for exactly that reason. + """ + with patch("ansys.visor.viewer.app.trame.local_app.logger") as log: + result = app_without_api.set_projection({"parallel": True}) + + assert result is None + assert log.debug.call_count == 2 + assert all(PROJECTION_TRIGGER in call.args for call in log.debug.call_args_list) + assert log.warning.call_count == 0 + + +# =========================================================================== +# Registration +# =========================================================================== + +def test_set_projection_trigger_name_is_registered_after_decoration(app, mock_server): + """The name survives the payload decorator and takes a raw dict. + + ``@trigger`` is outermost above ``@parse_payload``; this asserts the pair + registers under the name rather than under the wrapper, and that the + registered callable accepts the raw dict the client sends. + """ + names = [call.args[0] for call in mock_server.trigger.call_args_list] + functions = [ + call.args[0] for call in mock_server.trigger.return_value.call_args_list + ] + registered = dict(zip(names, functions)) + + assert PROJECTION_TRIGGER in registered + assert registered[PROJECTION_TRIGGER]({"parallel": False}) is None + diff --git a/tests/unit/renderer/test_local_renderer.py b/tests/unit/renderer/test_local_renderer.py index e17d604e..67e1591e 100644 --- a/tests/unit/renderer/test_local_renderer.py +++ b/tests/unit/renderer/test_local_renderer.py @@ -817,6 +817,122 @@ def test_serialize_camera_state_does_not_notify_the_client(self, renderer): renderer._local_view.update.assert_not_called() + # -- set_projection ----------------------------------------------------- + # + # The record half and the pipeline half are separate tests, and the + # None-record path is two more: either half can silently do nothing while + # the other succeeds, and the None path's two halves fail for different + # reasons -- a seeded record is data loss deferred, a guarded pipeline + # write is a projection that never reaches the view. + + def test_set_projection_writes_the_record_in_place_preserving_identity( + self, renderer + ): + """Record half: the same object, mutated, never replaced. + + This is the assertion that pins Option A against Option B. Object + identity through ``get_camera_state`` is contractual -- the + ``sync_camera`` docstring says callers rely on it -- so a body that + rebuilt the record with ``model_copy`` would satisfy the value + assertion and fail the identity one, which is what makes the two + separable here. + + The record is seeded through ``sync_camera`` from a hand-written + camera whose ``parallel_projection`` is the literal ``False``, so the + write is a visible transition rather than a coincidence. + """ + seeded = VisorCameraState( + position=[1.0, 2.0, 3.0], + focal_point=[4.0, 5.0, 6.0], + view_up=[0.0, 0.0, 1.0], + clipping_range=[7.0, 8.0], + parallel_projection=False, + view_angle=31.0, + parallel_scale=9.0, + ) + renderer.sync_camera(seeded) + before = renderer.get_camera_state() + + renderer.set_projection(True) + + after = renderer.get_camera_state() + assert before is after + assert after is seeded + assert after.parallel_projection is True + + def test_set_projection_writes_false_to_the_record(self, renderer): + """The other direction, so a body hard-coding ``True`` fails. + + Record half only. The seed carries ``True`` so that ``False`` is a + transition and not the value that was already there. + """ + seeded = VisorCameraState( + position=[1.0, 2.0, 3.0], + focal_point=[4.0, 5.0, 6.0], + view_up=[0.0, 0.0, 1.0], + clipping_range=[7.0, 8.0], + parallel_projection=True, + view_angle=31.0, + parallel_scale=9.0, + ) + renderer.sync_camera(seeded) + + renderer.set_projection(False) + + assert renderer.get_camera_state().parallel_projection is False + + def test_set_projection_applies_to_the_pipeline_camera(self, renderer): + """Pipeline half: exactly one setter call, with the argument. + + Asserted as the whole call list, so a body that also wrote some other + camera property fails rather than passing on the one call that was + looked for. The camera double's setters are recorded after the + ``sync_camera`` seed is cleared, so the seven projection writes that + seed performs do not appear here. + """ + renderer._last_camera_state = None + camera = renderer._vtk_renderer.GetActiveCamera.return_value + camera.calls.clear() + + renderer.set_projection(True) + + assert camera.calls == [("SetParallelProjection", True)] + + def test_set_projection_with_no_record_leaves_the_record_none(self, renderer): + """The ``None`` record is not seeded here, and that is deliberate. + + Sub-option (ii), pinned so a later session does not quietly seed it: a + projection set before any camera has been written is not saved until + the next ``reset_camera`` reads the pipeline and imports it. That + cost is named in the interface docstring, and this is the assertion + that holds the tree to it. + """ + assert renderer.get_camera_state() is None + + renderer.set_projection(True) + + assert renderer.get_camera_state() is None + + def test_set_projection_with_no_record_still_applies_to_the_pipeline( + self, renderer + ): + """...and the pipeline write happens anyway, with one debug line. + + The pipeline write is unconditional, on both branches. Its own test + rather than another assertion above: "did not seed the record" and + "still projected" are different failures, and a body that returned + early on a ``None`` record would pass the first while leaving the + toolbar click with no visible effect at all. + """ + camera = renderer._vtk_renderer.GetActiveCamera.return_value + camera.calls.clear() + + with patch("ansys.visor.viewer.renderer.local_renderer.logger") as log: + renderer.set_projection(True) + + assert camera.calls == [("SetParallelProjection", True)] + assert log.debug.call_count == 1 + # =========================================================================== # 5. Render / flush delegation diff --git a/tests/unit/renderer/test_null_renderer.py b/tests/unit/renderer/test_null_renderer.py index 4b96eecc..41809e93 100644 --- a/tests/unit/renderer/test_null_renderer.py +++ b/tests/unit/renderer/test_null_renderer.py @@ -9,6 +9,8 @@ from __future__ import annotations +from unittest.mock import patch + from ansys.visor.viewer.models.common.visor_camera_state import VisorCameraState from ansys.visor.viewer.renderer.null_renderer import NullRenderer @@ -118,4 +120,46 @@ def test_serialize_camera_state_is_a_no_op_and_leaves_the_record_alone(): assert renderer.get_camera_state() is cam +# =========================================================================== +# Projection: the record half owed in full, the pipeline half not at all +# =========================================================================== + +def test_set_projection_writes_the_record_in_place_preserving_identity(): + """The record is mutated, not replaced, exactly as on the local renderer. + + The record half of the projection contract is not optional on any + implementation. Identity is asserted as well as value because callers + rely on object identity through ``get_camera_state``, and a body that + rebuilt the record would satisfy the value assertion alone. + + The seed carries the literal ``False`` so the write is a transition. + """ + renderer = NullRenderer() + cam = _camera_state() + renderer.sync_camera(cam) + + renderer.set_projection(True) + + assert renderer.get_camera_state() is cam + assert cam.parallel_projection is True + + +def test_set_projection_with_no_record_is_a_logged_no_op(): + """With no record there is nothing to write, and nothing to project onto. + + This renderer has no pipeline camera, so unlike the local renderer there + is no second half to fall through to: the whole method is the record, and + an empty record makes the whole method a logged no-op. Asserted as "still + ``None``" rather than "did not raise", because a body that seeded a record + here would also not raise. + """ + renderer = NullRenderer() + + with patch("ansys.visor.viewer.renderer.null_renderer.logger") as log: + renderer.set_projection(True) + + assert renderer.get_camera_state() is None + assert log.debug.call_count == 1 + + diff --git a/tests/unit/vtk/scene/test_base.py b/tests/unit/vtk/scene/test_base.py index 77b5bfdc..e51f98e8 100644 --- a/tests/unit/vtk/scene/test_base.py +++ b/tests/unit/vtk/scene/test_base.py @@ -2355,3 +2355,216 @@ def test_set_part_visibility_unknown_node_id_does_not_fan_out(scene): scene.set_part_visibility(UNKNOWN_NODE_ID, False) assert calls == [] + + +# =========================================================================== +# Projection -- the camera record's field, written by its own trigger +# +# Projection is not a fourth toggle and there is no store field for it. The +# camera record is the only holder, ``get_state`` derives the persisted +# ``orthographic_enabled`` from it, and the coordinator re-serialises the +# camera as part of the write. +# +# That re-serialisation is the story's one quiet failure: dropped, the record +# is right, the pipeline camera is right, every other test in this module +# passes, and the browser snaps back to the pre-toggle framing on the next +# fetch. The probe below is the only assertion in the suite that sees it. +# =========================================================================== + +PROJECTION_ON = True +PROJECTION_OFF = False + +# What a browser that was asked would answer for the persisted toggle. +# Deliberately the opposite of the record's parallel_projection, so "derived +# from the record" and "passed through from the reply" cannot both pass. +REPLY_ORTHOGRAPHIC = False + + +def _projection_save_scene(scene, record, reply_orthographic): + """Wire *scene* for a save: renderer record *record*, reply *orthographic*. + + The same shape as ``_save_scene`` above, but the reply carries the + persisted projection toggle rather than a camera, because that is the + field whose source is under test. + """ + double = _CameraRecordRenderer(record, _pipeline_camera(), scene=scene) + scene._renderer = double + scene._dataset_registry = VisorDatasetRegistry() + + async def _get_runtime_state_async(timeout): + return RuntimeAppState.from_components( + dark_mode=False, + unit="m", + dataset_states={}, + orthographic_enabled=reply_orthographic, + ) + + scene._get_runtime_state_async = _get_runtime_state_async + return double + + +# --------------------------------------------------------------------------- +# The coordinator +# --------------------------------------------------------------------------- + +def test_set_projection_applies_to_the_renderer_and_then_serializes(scene): + """The re-serialisation follows the write, and carries production's id. + + Reverted -- the write kept and the re-serialisation dropped -- the record + is right, the pipeline camera is right, and the client is served the + pre-toggle camera on its next fetch. + + Also pins *where* the re-serialisation lives. Written inside the + renderer's own ``set_projection`` instead of here, the lock probe below + would still pass; this spy sits on the coordinator's two calls, so a + renderer that serialised for itself would record the pair in the wrong + order or twice. + + The spy appends ``("set_projection", )`` for the write and the + two-tuple ``("serialize", )`` for the re-serialisation; the tuple is + the recording format, not the argument. + """ + order = [] + real_set = scene._renderer.set_projection + + def _set(parallel): + order.append(("set_projection", parallel)) + return real_set(parallel) + + scene._renderer.set_projection = _set + scene._renderer._object_manager.UpdateStateFromObject = ( + lambda object_id: order.append(("serialize", object_id)) + ) + + scene.set_projection(PROJECTION_ON) + + assert order == [("set_projection", True), ("serialize", ACTIVE_CAMERA_WASM_ID)] + + +def test_set_projection_holds_the_lock_across_both_halves(scene): + """Both halves run inside ONE critical section, at the same depth. + + This is the assertion that pins the increment, and it is the only one in + the story that catches the quiet failure. Three reverts, three distinct + signatures: + + * drop ``serialize_camera_state()`` -> ``serialize_depth`` is never + recorded and this fails on the missing key; + * move it below the ``with`` block -> ``serialize_depth`` is 0 while + ``write_depth`` is 1, so both the non-zero and the equality assertions + fail, which is what separates "outside the lock" from "absent"; + * drop the ``with`` entirely -> both depths are 0. + + The trigger handler runs on trame's daemon thread while the VTK objects it + mutates belong to the caller's thread, so a re-serialisation outside the + lock reads the object graph while another thread is free to mutate it. + That failure is intermittent and never reproduces under a gate. + """ + scene._vtk_lock = _LockSpy() + observed = {} + real_set = scene._renderer.set_projection + + def _set(parallel): + observed["write_depth"] = scene._vtk_lock.depth + return real_set(parallel) + + scene._renderer.set_projection = _set + scene._renderer._object_manager.UpdateStateFromObject = ( + lambda object_id: observed.update(serialize_depth=scene._vtk_lock.depth) + ) + + scene.set_projection(PROJECTION_ON) + + assert observed["write_depth"] >= 1 + assert observed["serialize_depth"] >= 1 + assert observed["write_depth"] == observed["serialize_depth"] + assert scene._vtk_lock.depth == 0 + assert scene._vtk_lock.enter_count == scene._vtk_lock.exit_count + + +def test_set_projection_writes_no_store_field(scene): + """The camera record is the only holder, and the scene stores nothing. + + The three widget toggles have store fields because no server VTK object + backs them. Projection has one, so a fourth store field would be the + second source the derivation exists to remove -- and a second source is + invisible until the two disagree, which is a save away. + + Asserted both ways: the record carries the value, and the scene grew no + attribute to carry it as well. + """ + seeded = _record_camera() + scene._renderer.sync_camera(seeded) + + scene.set_projection(PROJECTION_OFF) + + assert scene._renderer.get_camera_state().parallel_projection is False + assert not hasattr(scene, "_orthographic_enabled") + assert not hasattr(scene, "_parallel_projection") + assert scene._cross_section_enabled is False + assert scene._edges_enabled is False + assert scene._bounding_box_enabled is False + + +def test_set_projection_does_not_notify_the_client(scene): + """Serialise only. No render, no flush, no delegated push. + + A notify here would look correct and would be a loop: the push rebuilds + the client, the rebuild re-delivers state, the reapply fires further + triggers, and each one pushes again. Every gate passes with a notify in + place and the symptom in the browser looks like a network problem. + """ + notifications = [] + scene._renderer.render = lambda: notifications.append("render") + scene._renderer.render_window_only = lambda: notifications.append("render_window") + scene._renderer.flush_wasm_state = lambda: notifications.append("flush") + scene._apply_runtime_state_to_render = lambda state: notifications.append("bridge") + + scene.set_projection(PROJECTION_ON) + + assert notifications == [] + + +# --------------------------------------------------------------------------- +# get_state -- orthographic_enabled is derived from the record +# --------------------------------------------------------------------------- + +def test_get_state_derives_orthographic_enabled_from_the_camera_record(scene): + """The saved projection is the record's, with the browser saying otherwise. + + This is the assertion that closes AC-5. The record carries the + hand-written literal ``True`` while the reply carries the hand-written + literal ``False``; revert the derivation and the assertion reports + ``False``, which is the reply's answer passed through -- the behaviour + before this increment. + + ``record_reads == 1`` is asserted here too: the record is bound once and + read once, so the camera and the projection are answers to a single + question and cannot disagree with each other. + + Asserted on what get_state RETURNS -- the object that reaches the writer + -- not on the runtime state it was built from. + """ + double = _projection_save_scene(scene, _record_camera(), REPLY_ORTHOGRAPHIC) + + persisted = asyncio.run(scene.get_state(timeout=1.0)) + + assert persisted.scene.orthographic_enabled is True + assert persisted.scene.camera.parallel_projection is True + assert double.record_reads == 1 + + +def test_get_state_orthographic_enabled_is_none_when_the_record_is_empty(scene): + """An empty record emits ``None``, not a fabricated ``False``. + + ``None`` says "nothing was ever written"; ``False`` would assert + perspective over a client that may be parallel. The reply carries + ``True`` here, so a derivation that dropped its guard and fell back to the + reply would be visible rather than coincide. + """ + _projection_save_scene(scene, None, True) + + persisted = asyncio.run(scene.get_state(timeout=1.0)) + + assert persisted.scene.orthographic_enabled is None + assert persisted.scene.camera is None From 34709684d0391215d48974b69326ad57633c73c2 Mon Sep 17 00:00:00 2001 From: Laura Kasian Date: Fri, 18 Sep 2026 12:41:23 -0700 Subject: [PATCH 04/13] feat: update tests after extra debug logging added --- tests/unit/app/test_local_app.py | 4 ++-- tests/unit/app/test_local_app_sync_camera.py | 4 ++-- tests/unit/app/test_local_app_widget_triggers.py | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/unit/app/test_local_app.py b/tests/unit/app/test_local_app.py index 25ea4498..b3f8154a 100644 --- a/tests/unit/app/test_local_app.py +++ b/tests/unit/app/test_local_app.py @@ -289,7 +289,7 @@ def test_trigger_is_a_logged_no_op_when_no_coordinator_injected(app_without_api, with patch("ansys.visor.viewer.app.trame.local_app.logger") as mock_logger: assert getattr(app_without_api, name)(PAYLOADS[name]) is None - assert mock_logger.debug.call_count == 1 + assert mock_logger.debug.call_count == 2 # =========================================================================== @@ -450,7 +450,7 @@ def test_missing_coordinator_logs_debug_and_not_warning(app_without_api, name): with patch("ansys.visor.viewer.app.trame.local_app.logger") as mock_logger: assert getattr(app_without_api, name)(PAYLOADS[name]) is None - assert mock_logger.debug.call_count == 1 + assert mock_logger.debug.call_count == 2 assert mock_logger.warning.call_count == 0 diff --git a/tests/unit/app/test_local_app_sync_camera.py b/tests/unit/app/test_local_app_sync_camera.py index 9cb2ed81..347ea322 100644 --- a/tests/unit/app/test_local_app_sync_camera.py +++ b/tests/unit/app/test_local_app_sync_camera.py @@ -163,7 +163,7 @@ def test_sync_camera_gesture_logs_one_debug_line_with_origin_and_position(app): with patch("ansys.visor.viewer.app.trame.local_app.logger") as log: app.sync_camera({"origin": "gesture", "camera": _camera()}) - assert log.debug.call_count == 1 + assert log.debug.call_count == 2 args = log.debug.call_args.args assert "gesture" in args assert CAMERA_POSITION in args @@ -256,7 +256,7 @@ def test_sync_camera_is_a_logged_no_op_when_no_coordinator_injected(app_without_ result = app_without_api.sync_camera({"origin": "gesture", "camera": _camera()}) assert result is None - assert log.debug.call_count == 1 + assert log.debug.call_count == 2 def test_sync_camera_trigger_name_is_registered_after_decoration(app, mock_server): diff --git a/tests/unit/app/test_local_app_widget_triggers.py b/tests/unit/app/test_local_app_widget_triggers.py index 6e8d8a7e..058b1c79 100644 --- a/tests/unit/app/test_local_app_widget_triggers.py +++ b/tests/unit/app/test_local_app_widget_triggers.py @@ -187,7 +187,7 @@ def test_set_cross_section_visibility_is_a_logged_no_op_when_no_coordinator_inje result = app_without_api.set_cross_section_visibility({"visible": True}) assert result is None - assert log.debug.call_count == 1 + assert log.debug.call_count == 2 def test_set_edges_visible_is_a_logged_no_op_when_no_coordinator_injected(app_without_api): @@ -196,7 +196,7 @@ def test_set_edges_visible_is_a_logged_no_op_when_no_coordinator_injected(app_wi result = app_without_api.set_edges_visible({"visible": True}) assert result is None - assert log.debug.call_count == 1 + assert log.debug.call_count == 2 def test_set_bounding_box_visibility_is_a_logged_no_op_when_no_coordinator_injected( @@ -207,7 +207,7 @@ def test_set_bounding_box_visibility_is_a_logged_no_op_when_no_coordinator_injec result = app_without_api.set_bounding_box_visibility({"visible": True}) assert result is None - assert log.debug.call_count == 1 + assert log.debug.call_count == 2 # =========================================================================== From cf256271c3b99f9453550e343e6e6cc347dc0966 Mon Sep 17 00:00:00 2001 From: Laura Kasian Date: Fri, 18 Sep 2026 14:54:03 -0700 Subject: [PATCH 05/13] feat: client reports widget toggles, seeds the projection flag, loads from the camera --- .../visor/visor-client/src/VisorFrontend.tsx | 19 +- .../jest-tests/AppStateProjectionLoad.test.ts | 144 ++++++++++ .../WasmRendererPartTriggers.test.tsx | 8 + .../WasmRendererWidgetTriggers.test.tsx | 259 ++++++++++++++++++ .../visor-client/src/renderer/IRenderer.ts | 47 ++++ .../visor-client/src/renderer/WasmRenderer.ts | 82 +++++- .../src/widgets/orthographicWidget.ts | 16 ++ 7 files changed, 567 insertions(+), 8 deletions(-) create mode 100644 src/ansys/visor/visor-client/src/jest-tests/AppStateProjectionLoad.test.ts create mode 100644 src/ansys/visor/visor-client/src/jest-tests/WasmRendererWidgetTriggers.test.tsx diff --git a/src/ansys/visor/visor-client/src/VisorFrontend.tsx b/src/ansys/visor/visor-client/src/VisorFrontend.tsx index 9841b837..df818481 100644 --- a/src/ansys/visor/visor-client/src/VisorFrontend.tsx +++ b/src/ansys/visor/visor-client/src/VisorFrontend.tsx @@ -378,10 +378,12 @@ export class VisorFrontend { uiScaffold.setUnit(unit); })(); } - if (sceneState.orthographicEnabled !== undefined) { - const promise = renderer.setOrthographicModeAsync(sceneState.orthographicEnabled); - promises.push(promise); - } + // `sceneState.orthographicEnabled` is deliberately not read here. + // Projection arrives on the camera, below, through the one call + // that also sets the widget flag `getAppStateAsync` reads back. + // The persisted toggle is still emitted for compatibility and is + // ignored on load: two independent fields writing one wasm camera + // property, in unspecified order, is the divergence this removed. if (sceneState.crossSectionEnabled !== undefined) { const promise = renderer.setCrossSectionVisibilityAsync( sceneState.crossSectionEnabled @@ -416,9 +418,12 @@ export class VisorFrontend { promises.push(promise); } if (cameraState.parallelProjection !== undefined) { - const promise = renderer.setCameraParallelProjectionAsync( - cameraState.parallelProjection - ); + // The sole writer of projection on this path. Routed through + // setOrthographicModeAsync rather than + // setCameraParallelProjectionAsync because that one also sets + // the widget flag; the camera-only call would leave the flag + // stale and the next save would write the stale value. + const promise = renderer.setOrthographicModeAsync(cameraState.parallelProjection); promises.push(promise); } if (cameraState.viewAngle !== undefined) { diff --git a/src/ansys/visor/visor-client/src/jest-tests/AppStateProjectionLoad.test.ts b/src/ansys/visor/visor-client/src/jest-tests/AppStateProjectionLoad.test.ts new file mode 100644 index 00000000..0cbfd507 --- /dev/null +++ b/src/ansys/visor/visor-client/src/jest-tests/AppStateProjectionLoad.test.ts @@ -0,0 +1,144 @@ +import { VisorFrontend } from '../VisorFrontend'; +import type { IRenderer } from '../renderer/IRenderer'; +import type VisorVtkSceneNode from '../state/appstate/vtkInfo/VisorVtkSceneNode.tsx'; + +/** + * The projection load path: `setAppStateAsync` applies projection from + * `camera.parallelProjection` alone, through the one call that also sets the + * widget flag. + * + * Two persisted fields used to write the same wasm camera property -- + * `scene.orthographicEnabled` and `scene.camera.parallelProjection` -- both + * pushed into one unordered promise array, with only the first also setting + * the cached flag `getAppStateAsync` reads back. Which one won was + * unspecified. This module pins that there is now exactly one writer, that it + * is the one that sets the flag, and that the persisted toggle is ignored on + * load. + * + * The fixture is deliberately self-contradictory: the camera says `true` and + * the toggle says `false`. That is what makes the assertion pin *authority* + * rather than plumbing -- with both branches live the double is called twice, + * once with each value, and the call count fails. + * + * Two constraints the fixture depends on, stated because they are load-bearing + * and not merely convenient: + * + * 1. It omits `ui.darkTheme`. The theme branch of `setAppStateAsync` + * `fetch`es two CSS files and appends to `document.head`; it is gated on + * `darkTheme !== undefined`, and omitting the key is what keeps this test + * off the network. + * 2. It passes `updateUI: false`. With `true`, `setAppStateAsync` awaits + * `treeViewUtilPromise`, which nothing in this test ever resolves, and + * the call would hang rather than fail. + */ + +/** Hand-written literals. The camera and the toggle disagree on purpose. */ +const CAMERA_PARALLEL_PROJECTION = true; +const PERSISTED_ORTHOGRAPHIC_ENABLED = false; + +/** + * The minimum root scene-graph node `CreateVisorSceneGraph` accepts, which + * `VisorFrontend`'s constructor builds the live graph from. No parts: the + * per-part half of `setAppStateAsync` is not the subject here. + */ +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 and `setAppStateAsync` + * reach on this fixture's path. The two projection setters are the subject; + * the rest are here so the call completes. + */ +function makeRendererDouble() { + return { + // Touched by the constructor. + attachSceneGraph: jest.fn(), + domElement: document.createElement('div'), + addCameraSettledListener: jest.fn(() => jest.fn()), + addViewerClickedListener: jest.fn(() => jest.fn()), + // The two projection writers. Exactly one of these may be called. + setOrthographicModeAsync: jest.fn(async () => undefined), + setCameraParallelProjectionAsync: jest.fn(async () => undefined), + // The other widget toggles, unused by this fixture but part of the + // surface `setAppStateAsync` branches over. + setCrossSectionVisibilityAsync: jest.fn(async () => undefined), + setEdgeVisibilityGlobalAsync: jest.fn(async () => undefined), + setBoundingBoxVisibilityAsync: jest.fn(async () => undefined), + // The remaining camera setters. + setCameraPositionAsync: jest.fn(async () => undefined), + setCameraFocalPointAsync: jest.fn(async () => undefined), + setCameraViewUpAsync: jest.fn(async () => undefined), + setCameraClippingRangeAsync: jest.fn(async () => undefined), + setCameraViewAngleAsync: jest.fn(async () => undefined), + setCameraParallelScaleAsync: jest.fn(async () => undefined), + // The cross-section plane setters. + setCrossSectionOriginAsync: jest.fn(async () => undefined), + setCrossSectionNormalAsync: jest.fn(async () => undefined), + // Awaited at the end of setAppStateAsync. + resizeAsync: jest.fn(async () => undefined), + }; +} + +type RendererDouble = ReturnType; + +function makeFrontend(): { frontend: VisorFrontend; renderer: RendererDouble } { + const renderer = makeRendererDouble(); + const frontend = new VisorFrontend( + renderer as unknown as IRenderer, + makeSceneGraphNode() as unknown as VisorVtkSceneNode, + jest.fn(async () => undefined) + ); + return { frontend, renderer }; +} + +describe('setAppStateAsync applies projection from the camera alone', () => { + test('a parallel camera with a contradictory orthographicEnabled applies projection once, with true, through setOrthographicModeAsync', async () => { + const { frontend, renderer } = makeFrontend(); + + await frontend.setAppStateAsync( + { + scene: { + orthographicEnabled: PERSISTED_ORTHOGRAPHIC_ENABLED, + camera: { parallelProjection: CAMERA_PARALLEL_PROJECTION }, + }, + }, + false + ); + + expect(renderer.setOrthographicModeAsync).toHaveBeenCalledTimes(1); + expect(renderer.setOrthographicModeAsync).toHaveBeenCalledWith(true); + expect(renderer.setCameraParallelProjectionAsync).not.toHaveBeenCalled(); + }); + + test('a state with no camera.parallelProjection applies no projection at all', async () => { + // The persisted toggle is not a fallback. Reinstating it as one -- + // "read the camera, and the toggle when the camera is silent" -- + // passes the test above and fails here. + const { frontend, renderer } = makeFrontend(); + + await frontend.setAppStateAsync( + { + scene: { + orthographicEnabled: true, + camera: {}, + }, + }, + false + ); + + expect(renderer.setOrthographicModeAsync).not.toHaveBeenCalled(); + expect(renderer.setCameraParallelProjectionAsync).not.toHaveBeenCalled(); + }); +}); + diff --git a/src/ansys/visor/visor-client/src/jest-tests/WasmRendererPartTriggers.test.tsx b/src/ansys/visor/visor-client/src/jest-tests/WasmRendererPartTriggers.test.tsx index 26714ad2..9c0adbc9 100644 --- a/src/ansys/visor/visor-client/src/jest-tests/WasmRendererPartTriggers.test.tsx +++ b/src/ansys/visor/visor-client/src/jest-tests/WasmRendererPartTriggers.test.tsx @@ -105,6 +105,14 @@ async function makeRenderer(sender: TrameTriggerSender | null) { canvasDiv: document.createElement('div'), render: jest.fn(), clearObserversAndEventListeners: jest.fn(), + // createAsync seeds the orthographic flag from the wasm camera, so a + // double that cannot answer GetParallelProjection fails construction + // before any test in this module runs. Perspective (0) is the value a + // scene that has never been made parallel reports; nothing here reads + // it back. + camera: { + GetParallelProjection: jest.fn(async () => 0), + }, getVtkObject: (wasmId: number) => { switch (wasmId) { case ACTOR_ID: diff --git a/src/ansys/visor/visor-client/src/jest-tests/WasmRendererWidgetTriggers.test.tsx b/src/ansys/visor/visor-client/src/jest-tests/WasmRendererWidgetTriggers.test.tsx new file mode 100644 index 00000000..a09566c0 --- /dev/null +++ b/src/ansys/visor/visor-client/src/jest-tests/WasmRendererWidgetTriggers.test.tsx @@ -0,0 +1,259 @@ +import { WasmRenderer } from '../renderer/WasmRenderer'; +import type { WasmRendererAnnotation } from '../renderer/RendererAnnotation'; +import type VtkScene from '../wasm/VtkScene'; +import type { VisorSceneNodeExtended } from '../state/VisorSceneGraph.tsx'; +import type { TrameTriggerSender } from '../renderer/IRenderer'; + +/** + * WasmRenderer's view-level widget *send* surface: the four methods that + * carry a scene-wide toggle to the matching server trigger, and the + * orthographic flag seed that `createAsync` performs. + * + * Two things are pinned here, and they are separate on purpose. + * + * 1. Each of the four methods, **called with no argument**, sends its + * trigger once carrying the value the widget settled on -- not the + * argument it was passed. The toolbar calls all four with no argument, + * so the argument is `undefined` and only the widget knows the value. + * Forwarding the argument instead of reading the widget back would put + * `undefined` on the wire and fail nowhere a gate can see it. Every + * expected payload below is a hand-written literal. + * + * 2. `createAsync` leaves `isOrthographicEnabled()` agreeing with the wasm + * camera before any setter has run. Without the seed the flag is the + * literal `false` its field initialiser gives it, whatever the camera + * says, and the fault appears only in the running application -- on a + * rebuild, where `getAppStateAsync` reads that flag and saves it. + * + * Also pinned: with no sender injected the four sends are no-ops that do not + * throw, and a failed widget send is logged under its own prefix, naming the + * trigger and carrying no node id. + * + * Modelled on `WasmRendererPartTriggers.test.tsx`, which does the same for + * the six per-part sends. + */ + +const NODE_ID = 7; +const ACTOR_ID = 101; +const PROPERTY_ID = 102; +const MAPPER_ID = 103; +const ORIENTATION_WIDGET_ID = 201; +const PLANE_ID = 202; +const PLANE_WIDGET_ID = 203; +const PLANE_REPRESENTATION_ID = 204; +const BOUNDING_BOX_ALGORITHM_ID = 205; +const BOUNDING_BOX_OUTLINE_ID = 206; +const BOUNDING_BOX_AXES_ID = 207; + +/** What the wasm camera reports for `GetParallelProjection()`. */ +const PARALLEL = 1; +const PERSPECTIVE = 0; + +function makeAnnotation(): WasmRendererAnnotation { + return { + rendererKind: 'wasm', + nodes: { + [String(NODE_ID)]: { + actorId: ACTOR_ID, + propertyId: PROPERTY_ID, + mapperId: MAPPER_ID, + }, + }, + widgets: { + orientationWidgetId: ORIENTATION_WIDGET_ID, + crossSectionPlaneId: PLANE_ID, + crossSectionPlaneWidgetId: PLANE_WIDGET_ID, + crossSectionPlaneRepresentationId: PLANE_REPRESENTATION_ID, + boundingBoxAlgorithmId: BOUNDING_BOX_ALGORITHM_ID, + boundingBoxOutlineActorId: BOUNDING_BOX_OUTLINE_ID, + boundingBoxAxesActorId: BOUNDING_BOX_AXES_ID, + }, + }; +} + +function makeFakeWasmObjects() { + const actor = { + SetVisibility: jest.fn(async () => undefined), + }; + const property = { + SetEdgeColor: jest.fn(async () => undefined), + EdgeVisibilityOn: jest.fn(async () => undefined), + EdgeVisibilityOff: jest.fn(async () => undefined), + }; + // Stands in for the plane, the plane representation, the plane widget, + // the bounding-box outline and axes actors, and the box algorithm. The + // bounding-box widget queries `GetVisibility` when toggling with no + // argument, which is the path every test here takes. + const widget = { + observe: jest.fn(), + On: jest.fn(async () => undefined), + Off: jest.fn(async () => undefined), + GetVisibility: jest.fn(async () => 0), + SetVisibility: jest.fn(async () => undefined), + SetBounds: jest.fn(async () => undefined), + GetOrigin: jest.fn(async () => [0, 0, 0]), + GetNormal: jest.fn(async () => [0, 0, 1]), + SetOrigin: jest.fn(async () => undefined), + SetNormal: jest.fn(async () => undefined), + }; + return { actor, property, widget }; +} + +/** + * A scene-graph stand-in for `attachSceneGraph`. + * + * `EdgesWidget` fans out through the graph node's own + * `setEdgeVisibilityAsync`; `BoundingBoxWidget` enumerates + * `descendantActorNodesOrSelfArray` when recomputing bounds. Neither is the + * subject here -- what is pinned is the *send* that follows the local write. + */ +function makeSceneGraphDouble() { + return { + setEdgeVisibilityAsync: jest.fn(async () => undefined), + descendantActorNodesOrSelfArray: [], + }; +} + +async function makeRenderer( + sender: TrameTriggerSender | null, + parallelProjection: number = PERSPECTIVE +) { + const objects = makeFakeWasmObjects(); + const camera = { + GetParallelProjection: jest.fn(async () => parallelProjection), + ParallelProjectionOn: jest.fn(async () => undefined), + ParallelProjectionOff: jest.fn(async () => undefined), + }; + const scene = { + canvasDiv: document.createElement('div'), + render: jest.fn(), + clearObserversAndEventListeners: jest.fn(), + camera, + getVtkObject: (wasmId: number) => { + switch (wasmId) { + case ACTOR_ID: + return objects.actor; + case PROPERTY_ID: + return objects.property; + default: + return objects.widget; + } + }, + }; + const renderer = await WasmRenderer.createAsync( + scene as unknown as VtkScene, + makeAnnotation(), + sender + ); + const sceneGraph = makeSceneGraphDouble(); + renderer.attachSceneGraph(sceneGraph as unknown as VisorSceneNodeExtended); + return { renderer, camera, sceneGraph, ...objects }; +} + +/** A sender that records its calls and resolves. */ +function makeSender() { + return jest.fn(async () => undefined) as unknown as jest.Mock & TrameTriggerSender; +} + +describe('WasmRenderer widget sends: trigger name and settled value', () => { + // Every widget starts at its constructor default of `false`, so a call + // with no argument settles on `true`. `true` is the hand-written literal + // expected below; nothing here asks a widget what it settled on. + test('setCrossSectionVisibilityAsync with no argument sends set_cross_section_visibility with the settled value', async () => { + const sender = makeSender(); + const { renderer } = await makeRenderer(sender); + + await renderer.setCrossSectionVisibilityAsync(); + + expect(sender).toHaveBeenCalledTimes(1); + expect(sender).toHaveBeenCalledWith('set_cross_section_visibility', { + visible: true, + }); + }); + + test('setEdgeVisibilityGlobalAsync with no argument sends set_edges_visible with the settled value', async () => { + const sender = makeSender(); + const { renderer } = await makeRenderer(sender); + + await renderer.setEdgeVisibilityGlobalAsync(); + + expect(sender).toHaveBeenCalledTimes(1); + expect(sender).toHaveBeenCalledWith('set_edges_visible', { + visible: true, + }); + }); + + test('setBoundingBoxVisibilityAsync with no argument sends set_bounding_box_visibility with the settled value', async () => { + const sender = makeSender(); + const { renderer } = await makeRenderer(sender); + + await renderer.setBoundingBoxVisibilityAsync(); + + expect(sender).toHaveBeenCalledTimes(1); + expect(sender).toHaveBeenCalledWith('set_bounding_box_visibility', { + visible: true, + }); + }); + + test('setOrthographicModeAsync with no argument sends set_projection with the settled value', async () => { + const sender = makeSender(); + const { renderer } = await makeRenderer(sender); + + await renderer.setOrthographicModeAsync(); + + expect(sender).toHaveBeenCalledTimes(1); + expect(sender).toHaveBeenCalledWith('set_projection', { + parallel: true, + }); + }); +}); + +describe('WasmRenderer widget sends: no sender, and a failing sender', () => { + test('the four widget sends are no-ops that do not throw with no sender injected', async () => { + const { renderer } = await makeRenderer(null); + + await expect(renderer.setCrossSectionVisibilityAsync(true)).resolves.toBeUndefined(); + await expect(renderer.setEdgeVisibilityGlobalAsync(true)).resolves.toBeUndefined(); + await expect(renderer.setBoundingBoxVisibilityAsync(true)).resolves.toBeUndefined(); + await expect(renderer.setOrthographicModeAsync(true)).resolves.toBeUndefined(); + }); + + test('a failed widget send is logged with the trigger name and carries no node id', async () => { + // This log line is the only signal a failed widget send produces. It + // is deliberately a different prefix from the per-part helper's, and + // it names no node, because a widget trigger has none -- a sentinel + // id here would be fiction in the one message that has to be true. + const consoleError = jest.spyOn(console, 'error').mockImplementation(() => {}); + const sender = jest.fn(async () => { + throw new Error('socket closed'); + }) as unknown as TrameTriggerSender; + const { renderer } = await makeRenderer(sender); + + await expect(renderer.setOrthographicModeAsync(true)).resolves.toBeUndefined(); + + expect(consoleError).toHaveBeenCalledTimes(1); + const message = String(consoleError.mock.calls[0][0]); + expect(message).toContain('[VISOR] widget trigger send failed:'); + expect(message).toContain("trigger='set_projection'"); + expect(message).not.toContain('nodeId'); + + consoleError.mockRestore(); + }); +}); + +describe('WasmRenderer.createAsync seeds the orthographic flag from the wasm camera', () => { + test('isOrthographicEnabled is true when the wasm camera reports parallel, before any setter runs', async () => { + const { renderer } = await makeRenderer(makeSender(), PARALLEL); + + expect(renderer.isOrthographicEnabled()).toBe(true); + }); + + test('isOrthographicEnabled is false when the wasm camera reports perspective', async () => { + // The negative half. Without it, a seed hard-written to `true` would + // pass the test above and be wrong on every perspective camera. + const { renderer } = await makeRenderer(makeSender(), PERSPECTIVE); + + expect(renderer.isOrthographicEnabled()).toBe(false); + }); +}); + diff --git a/src/ansys/visor/visor-client/src/renderer/IRenderer.ts b/src/ansys/visor/visor-client/src/renderer/IRenderer.ts index 6cb94001..3ecd71d7 100644 --- a/src/ansys/visor/visor-client/src/renderer/IRenderer.ts +++ b/src/ansys/visor/visor-client/src/renderer/IRenderer.ts @@ -211,6 +211,17 @@ export interface IRenderer { // ---- View-level widgets (state is renderer-owned; see arch rule (a)) --- setCrossSectionVisibilityAsync(visible?: boolean): Promise; + /** + * Whether the cross-section plane is shown. + * + * The value is the last one the server delivered, or the last one this + * client set locally, whichever happened later. The widget's cached flag + * is a projection of that value, not an independent source. + * + * A write followed by an immediate read returns the **pre-push** value: + * the write is local and the server's confirmation arrives on a later + * fetch, so this getter is not a read-after-write on the server's store. + */ isCrossSectionVisible(): boolean; // cached bool, sync updateCrossSectionBoundsAsync(): Promise; getCrossSectionOriginAsync(): Promise; @@ -219,13 +230,49 @@ export interface IRenderer { setCrossSectionNormalAsync(normal: readonly number[]): Promise; setBoundingBoxVisibilityAsync(visible?: boolean): Promise; + /** + * Whether the bounding-box outline is shown. + * + * The value is the last one the server delivered, or the last one this + * client set locally, whichever happened later. The bounding-box widget's + * cached flag is a projection of that value, not an independent source. + * + * A write followed by an immediate read returns the **pre-push** value: + * the write is local and the server's confirmation arrives on a later + * fetch, so this getter is not a read-after-write on the server's store. + */ isBoundingBoxVisible(): boolean; updateBoundingBoxBoundsAsync(): Promise; setOrthographicModeAsync(enable?: boolean): Promise; + /** + * Whether the view is in parallel (orthographic) projection. + * + * The value is the last one the server delivered -- seeded from the wasm + * camera when the renderer is built, which is the point at which the + * delivered camera is already in place -- or the last one this client set + * locally, whichever happened later. The orthographic widget's cached flag + * is a projection of that camera, not an independent source: the camera is + * the single place projection lives, and this flag only reflects it. + * + * A write followed by an immediate read returns the **pre-push** value: + * the write is local and the server's confirmation arrives on a later + * fetch, so this getter is not a read-after-write on the server's store. + */ isOrthographicEnabled(): boolean; setEdgeVisibilityGlobalAsync(visible?: boolean): Promise; + /** + * Whether edges are shown on every part in the scene. + * + * The value is the last one the server delivered, or the last one this + * client set locally, whichever happened later. The edges widget's cached + * flag is a projection of that value, not an independent source. + * + * A write followed by an immediate read returns the **pre-push** value: + * the write is local and the server's confirmation arrives on a later + * fetch, so this getter is not a read-after-write on the server's store. + */ areEdgesVisibleGlobally(): boolean; toggleFullScreenAsync(): Promise; diff --git a/src/ansys/visor/visor-client/src/renderer/WasmRenderer.ts b/src/ansys/visor/visor-client/src/renderer/WasmRenderer.ts index d4471d8b..1134dd7d 100644 --- a/src/ansys/visor/visor-client/src/renderer/WasmRenderer.ts +++ b/src/ansys/visor/visor-client/src/renderer/WasmRenderer.ts @@ -77,7 +77,28 @@ export class WasmRenderer implements IRenderer { annotation: WasmRendererAnnotation, triggerSender: TrameTriggerSender | null = null ): Promise { - return new WasmRenderer(vtkScene, annotation, triggerSender); + const renderer = new WasmRenderer(vtkScene, annotation, triggerSender); + await renderer.#seedOrthographicFlagAsync(); + return renderer; + } + + /** + * Read the orthographic flag out of the wasm camera, once, at construction. + * + * This is the only asynchronous seam in building a renderer, and it exists + * because the orthographic widget's cached flag is otherwise `false` no + * matter what the camera says. By the time `createAsync` runs, the wasm + * state fetch has already completed, so the camera read here is a read of + * the *delivered* camera -- which is what makes `isOrthographicEnabled()` + * a projection of server state rather than an independent source. + * + * Deliberately not defensive: a seed that swallowed its own failure would + * leave the flag at `false` against a parallel camera, which is precisely + * the save-corrupting fault this seed was added to remove, and it would be + * invisible to every gate. + */ + async #seedOrthographicFlagAsync(): Promise { + await this.#orthographicWidget.seedFromCameraAsync(); } readonly #vtkScene: VtkScene; @@ -393,6 +414,32 @@ export class WasmRenderer implements IRenderer { } } + /** + * Send one view-level widget payload to its server trigger. + * + * The sibling of `#sendTriggerAsync` above, whose rationale -- no sender + * means no send, a rejection is logged and swallowed, the catch is + * unnarrowed -- applies here unchanged and is not repeated. + * + * It exists separately only because a widget trigger has no `nodeId`. + * That helper takes one purely to name it in the error line, and passing a + * sentinel would put a fictitious node id in the one message a failed send + * produces. The prefix here is correspondingly distinct and greppable. + */ + async #sendWidgetTriggerAsync( + triggerName: string, + payload: Record + ): Promise { + if (this.#triggerSender == null) { + return; + } + try { + await this.#triggerSender(triggerName, payload); + } catch (err) { + console.error(`[VISOR] widget trigger send failed: trigger='${triggerName}'`, err); + } + } + async sendPartVisibilityAsync(nodeId: NodeId, visible: boolean): Promise { await this.#sendTriggerAsync('set_part_visibility', nodeId, { nodeId, @@ -454,8 +501,32 @@ export class WasmRenderer implements IRenderer { } // ---- View-level widgets ------------------------------------------------- + /** + * The four methods below each report to the server *after* the local + * write, and each reads its own widget back rather than forwarding the + * argument it was given. + * + * That read-back is required, not stylistic. The toolbar calls all four + * with **no argument** -- see `Panel_BottomMiddle.tsx` -- and each widget + * resolves the absent argument itself by negating its cached flag. The + * argument is therefore not the value; only the widget knows what it + * settled on. Forwarding the argument would put `undefined` on the wire + * on every toolbar click, which type-checks and lints and fails only at + * the server's payload boundary. + * + * Each reads back through the same getter `getAppStateAsync` uses, so the + * value sent and the value saved cannot disagree. + * + * Every payload is absolute, never a toggle and never a delta, so a send + * that is suppressed, duplicated or reordered is harmless. A delivered + * state therefore echoes back one send per toggle; that echo is + * idempotent by construction and is accepted. + */ async setCrossSectionVisibilityAsync(visible?: boolean): Promise { await this.#crossSectionWidget.setVisibilityAsync(visible); + await this.#sendWidgetTriggerAsync('set_cross_section_visibility', { + visible: this.isCrossSectionVisible(), + }); } isCrossSectionVisible(): boolean { @@ -485,6 +556,9 @@ export class WasmRenderer implements IRenderer { async setBoundingBoxVisibilityAsync(visible?: boolean): Promise { this.#requireSceneGraphAttached('setBoundingBoxVisibilityAsync'); await this.#boundingBoxWidget!.setVisibilityAsync(visible); + await this.#sendWidgetTriggerAsync('set_bounding_box_visibility', { + visible: this.isBoundingBoxVisible(), + }); } isBoundingBoxVisible(): boolean { @@ -499,6 +573,9 @@ export class WasmRenderer implements IRenderer { async setOrthographicModeAsync(enable?: boolean): Promise { await this.#orthographicWidget.setOrthographicModeAsync(enable); + await this.#sendWidgetTriggerAsync('set_projection', { + parallel: this.isOrthographicEnabled(), + }); } isOrthographicEnabled(): boolean { @@ -508,6 +585,9 @@ export class WasmRenderer implements IRenderer { async setEdgeVisibilityGlobalAsync(visible?: boolean): Promise { this.#requireSceneGraphAttached('setEdgeVisibilityGlobalAsync'); await this.#edgesWidget!.setEdgesVisibleAsync(visible); + await this.#sendWidgetTriggerAsync('set_edges_visible', { + visible: this.#edgesWidget!.enabled, + }); } areEdgesVisibleGlobally(): boolean { diff --git a/src/ansys/visor/visor-client/src/widgets/orthographicWidget.ts b/src/ansys/visor/visor-client/src/widgets/orthographicWidget.ts index e7efcd4e..43e7dd2f 100644 --- a/src/ansys/visor/visor-client/src/widgets/orthographicWidget.ts +++ b/src/ansys/visor/visor-client/src/widgets/orthographicWidget.ts @@ -23,6 +23,22 @@ export class OrthographicWidget { } }; + /** + * Seed the cached flag from the wasm camera. + * + * `#enabled` is initialised to `false` unconditionally at construction, + * and nothing else reads the real camera except the toggle path. A + * renderer built against a camera the server has already made parallel + * would therefore report perspective, and `getAppStateAsync` reads that + * cached flag -- so on a rebuild the wrong value is what gets saved. + * + * Awaited from `WasmRenderer.createAsync`, after the wasm state fetch has + * completed, which is what makes the value read here the delivered one. + */ + seedFromCameraAsync = async (): Promise => { + this.#enabled = await this.isOrthographicAsync(); + }; + isOrthographicAsync = async (): Promise => { const result = await this.#vtkScene.camera.GetParallelProjection(); return result === 1 || result === true; From 813dc5c73e71657930b79ba31d24267ad39fc862 Mon Sep 17 00:00:00 2001 From: Laura Kasian Date: Mon, 21 Sep 2026 14:12:48 -0700 Subject: [PATCH 06/13] feat: trigger arrival line carries the parsed payload --- src/ansys/visor/viewer/app/trame/local_app.py | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/ansys/visor/viewer/app/trame/local_app.py b/src/ansys/visor/viewer/app/trame/local_app.py index 6a7fa0e3..6a4e7b96 100644 --- a/src/ansys/visor/viewer/app/trame/local_app.py +++ b/src/ansys/visor/viewer/app/trame/local_app.py @@ -354,9 +354,9 @@ def perf_report_server_update(self, payload: dict): # body. # ------------------------------------------------------------------ - def _part_state_api(self, trigger_name: str) -> ScenePartStateApi | None: + def _part_state_api(self, trigger_name: str, payload: BaseModel) -> ScenePartStateApi | None: """Return the injected coordinator, or ``None`` after logging.""" - logger.debug("[trigger] %s arrived.", trigger_name) + logger.debug("[trigger] %s arrived: %s.", trigger_name, payload) if self._scene_part_state_api is None: logger.debug("%s: no scene part-state API injected; ignoring.", trigger_name) return None @@ -366,7 +366,7 @@ def _part_state_api(self, trigger_name: str) -> ScenePartStateApi | None: @parse_payload(SetPartVisibilityPayload) def set_part_visibility(self, payload) -> None: """Frontend -> Backend: set whether one part is visible.""" - api = self._part_state_api("set_part_visibility") + api = self._part_state_api("set_part_visibility", payload) if api is None: return api.set_part_visibility(payload.node_id, payload.visible) @@ -379,7 +379,7 @@ def set_part_opacity(self, payload) -> None: An opacity outside ``[0.0, 1.0]`` fails validation and is a logged no-op; it does not reach VTK to be clamped. """ - api = self._part_state_api("set_part_opacity") + api = self._part_state_api("set_part_opacity", payload) if api is None: return api.set_part_opacity(payload.node_id, payload.opacity) @@ -394,7 +394,7 @@ def set_part_diffuse_color(self, payload) -> None: or a colour that is not exactly three components, is a logged no-op -- nothing is delegated, so nothing is written to the store. """ - api = self._part_state_api("set_part_diffuse_color") + api = self._part_state_api("set_part_diffuse_color", payload) if api is None: return api.set_part_diffuse_color(payload.node_id, payload.diffuse_rgb) @@ -407,7 +407,7 @@ def set_part_selected(self, payload) -> None: No colour crosses this trigger: the server reads the part's stored diffuse colour from its own record. """ - api = self._part_state_api("set_part_selected") + api = self._part_state_api("set_part_selected", payload) if api is None: return api.set_part_selected(payload.node_id, payload.selected) @@ -425,7 +425,7 @@ def set_part_color_variable(self, payload) -> None: name. ``variableId`` is forwarded verbatim and is never parsed by the server. """ - api = self._part_state_api("set_part_color_variable") + api = self._part_state_api("set_part_color_variable", payload) if api is None: return api.set_part_color_variable( @@ -442,7 +442,7 @@ def set_part_color_variable(self, payload) -> None: @parse_payload(ClearPartColorVariablePayload) def clear_part_color_variable(self, payload) -> None: """Frontend -> Backend: stop colouring one part by a scalar variable.""" - api = self._part_state_api("clear_part_color_variable") + api = self._part_state_api("clear_part_color_variable", payload) if api is None: return api.clear_part_color_variable(payload.node_id) @@ -479,7 +479,7 @@ def sync_camera(self, payload) -> None: if payload.origin != "gesture": logger.debug("sync_camera: origin=%s; dropping.", payload.origin) return - api = self._part_state_api("sync_camera") + api = self._part_state_api("sync_camera", payload) if api is None: return logger.debug( @@ -523,7 +523,7 @@ def sync_camera(self, payload) -> None: @parse_payload(SetCrossSectionVisibilityPayload) def set_cross_section_visibility(self, payload) -> None: """Frontend -> Backend: show or hide the cross-section plane.""" - api = self._part_state_api("set_cross_section_visibility") + api = self._part_state_api("set_cross_section_visibility", payload) if api is None: return api.set_cross_section_visibility(payload.visible) @@ -532,7 +532,7 @@ def set_cross_section_visibility(self, payload) -> None: @parse_payload(SetEdgesVisiblePayload) def set_edges_visible(self, payload) -> None: """Frontend -> Backend: show or hide edges on every part.""" - api = self._part_state_api("set_edges_visible") + api = self._part_state_api("set_edges_visible", payload) if api is None: return api.set_edges_visible(payload.visible) @@ -541,7 +541,7 @@ def set_edges_visible(self, payload) -> None: @parse_payload(SetBoundingBoxVisibilityPayload) def set_bounding_box_visibility(self, payload) -> None: """Frontend -> Backend: show or hide the bounding-box outline.""" - api = self._part_state_api("set_bounding_box_visibility") + api = self._part_state_api("set_bounding_box_visibility", payload) if api is None: return api.set_bounding_box_visibility(payload.visible) @@ -555,7 +555,7 @@ def set_projection(self, payload) -> None: own: the coordinator writes the record and re-serialises the camera in one critical section. """ - api = self._part_state_api("set_projection") + api = self._part_state_api("set_projection", payload) if api is None: return api.set_projection(payload.parallel) From 763506ce30cfb39b79ab530dbdb9663696e9f0b4 Mon Sep 17 00:00:00 2001 From: pyansys-ci-bot <92810346+pyansys-ci-bot@users.noreply.github.com> Date: Tue, 22 Sep 2026 21:43:30 +0000 Subject: [PATCH 07/13] chore: adding changelog file 137.added.md [dependabot-skip] --- doc/changelog.d/137.added.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 doc/changelog.d/137.added.md diff --git a/doc/changelog.d/137.added.md b/doc/changelog.d/137.added.md new file mode 100644 index 00000000..3db59953 --- /dev/null +++ b/doc/changelog.d/137.added.md @@ -0,0 +1 @@ +[Remote rendering 3.3a] server tracked widget toggles From 2ba0f7de34733c23fac1894267fe9e0384eb2477 Mon Sep 17 00:00:00 2001 From: pyansys-ci-bot <92810346+pyansys-ci-bot@users.noreply.github.com> Date: Tue, 22 Sep 2026 21:45:05 +0000 Subject: [PATCH 08/13] chore: adding changelog file 137.added.md [dependabot-skip] --- doc/changelog.d/137.added.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/changelog.d/137.added.md b/doc/changelog.d/137.added.md index 3db59953..c831698a 100644 --- a/doc/changelog.d/137.added.md +++ b/doc/changelog.d/137.added.md @@ -1 +1 @@ -[Remote rendering 3.3a] server tracked widget toggles +[Remote rendering 3.3a] server-authoritative widget toggles and projection From 3c1fed351ff17de35f0f622fb5a28393b19daeba Mon Sep 17 00:00:00 2001 From: Laura Kasian Date: Tue, 22 Sep 2026 14:47:19 -0700 Subject: [PATCH 09/13] prettier fix --- .../visor-client/src/jest-tests/AppStateProjectionLoad.test.ts | 1 - .../src/jest-tests/WasmRendererWidgetTriggers.test.tsx | 1 - 2 files changed, 2 deletions(-) diff --git a/src/ansys/visor/visor-client/src/jest-tests/AppStateProjectionLoad.test.ts b/src/ansys/visor/visor-client/src/jest-tests/AppStateProjectionLoad.test.ts index 0cbfd507..382762ee 100644 --- a/src/ansys/visor/visor-client/src/jest-tests/AppStateProjectionLoad.test.ts +++ b/src/ansys/visor/visor-client/src/jest-tests/AppStateProjectionLoad.test.ts @@ -141,4 +141,3 @@ describe('setAppStateAsync applies projection from the camera alone', () => { expect(renderer.setCameraParallelProjectionAsync).not.toHaveBeenCalled(); }); }); - diff --git a/src/ansys/visor/visor-client/src/jest-tests/WasmRendererWidgetTriggers.test.tsx b/src/ansys/visor/visor-client/src/jest-tests/WasmRendererWidgetTriggers.test.tsx index a09566c0..28cdc63d 100644 --- a/src/ansys/visor/visor-client/src/jest-tests/WasmRendererWidgetTriggers.test.tsx +++ b/src/ansys/visor/visor-client/src/jest-tests/WasmRendererWidgetTriggers.test.tsx @@ -256,4 +256,3 @@ describe('WasmRenderer.createAsync seeds the orthographic flag from the wasm cam expect(renderer.isOrthographicEnabled()).toBe(false); }); }); - From dbe93d8f0971b60d9d8ad3afc7c9b6bfa4a648bb Mon Sep 17 00:00:00 2001 From: Laura Kasian Date: Wed, 23 Sep 2026 15:11:42 -0700 Subject: [PATCH 10/13] rename ScenePartStateApi -> SceneMutationApi and clean up docs --- src/ansys/visor/viewer/app/trame/local_app.py | 55 ++++++------------- src/ansys/visor/viewer/app/visor_vtk_local.py | 4 +- tests/unit/app/test_local_app.py | 4 +- .../unit/app/test_local_app_set_projection.py | 4 +- tests/unit/app/test_local_app_sync_camera.py | 4 +- .../app/test_local_app_widget_triggers.py | 4 +- tests/unit/app/test_visor_vtk_local.py | 4 +- 7 files changed, 30 insertions(+), 49 deletions(-) diff --git a/src/ansys/visor/viewer/app/trame/local_app.py b/src/ansys/visor/viewer/app/trame/local_app.py index 6a4e7b96..c86b88fa 100644 --- a/src/ansys/visor/viewer/app/trame/local_app.py +++ b/src/ansys/visor/viewer/app/trame/local_app.py @@ -26,7 +26,7 @@ vtkObject.GlobalWarningDisplayOff() -class ScenePartStateApi(Protocol): +class SceneMutationApi(Protocol): """Structural type of the coordinator surface LocalApp calls. Typing only: there is no ``runtime_checkable`` decoration and no @@ -34,12 +34,10 @@ class ScenePartStateApi(Protocol): importing the scene keeps this module free of any scene type, so the injected object remains LocalApp's only route to the scene. - Not all of it is per-part, and the name is historical. ``sync_camera``, - the three widget-state toggles and ``set_projection`` are scene-wide, and - they are declared here anyway: the one production injection site passes - the whole scene coordinator, so a second protocol would be the same object - under a second name. Read this as the whole coordinator surface LocalApp calls, not only - the per-part part of it. + Covers both per-part mutations (visibility, opacity, colour, selection) + and scene-wide ones (camera sync, the widget-state toggles, projection, + cross-section plane). The one production injection site passes the + whole scene coordinator, so this protocol describes that whole surface. """ def set_part_visibility(self, node_id: int, visible: bool) -> None: ... @@ -217,7 +215,7 @@ def __init__( standalone: bool = True, trame_logger: Logger | None = None, pick_geometry=None, - scene_part_state_api: ScenePartStateApi | None = None, + scene_mutation_api: SceneMutationApi | None = None, ): self.server = server # Callable to get the scene details in JSON format @@ -226,10 +224,10 @@ def __init__( self._handle_save_state_response = handle_save_state_response # Callable for sub-geometry picking (optional) self._pick_geometry = pick_geometry - # Per-part visual state coordinator (see ScenePartStateApi). The one + # Per-part visual state coordinator (see SceneMutationApi). The one # production construction site always supplies it; it is optional so # that the class stays constructible without a scene. - self._scene_part_state_api = scene_part_state_api + self._scene_mutation_api = scene_mutation_api # logger for logging trame server lifecycle info self.__trame_logger = trame_logger @@ -354,13 +352,13 @@ def perf_report_server_update(self, payload: dict): # body. # ------------------------------------------------------------------ - def _part_state_api(self, trigger_name: str, payload: BaseModel) -> ScenePartStateApi | None: + def _part_state_api(self, trigger_name: str, payload: BaseModel) -> SceneMutationApi | None: """Return the injected coordinator, or ``None`` after logging.""" logger.debug("[trigger] %s arrived: %s.", trigger_name, payload) - if self._scene_part_state_api is None: + if self._scene_mutation_api is None: logger.debug("%s: no scene part-state API injected; ignoring.", trigger_name) return None - return self._scene_part_state_api + return self._scene_mutation_api @trigger("set_part_visibility") @parse_payload(SetPartVisibilityPayload) @@ -492,31 +490,14 @@ def sync_camera(self, payload) -> None: # ------------------------------------------------------------------ # Widget-state triggers # - # Frontend -> Backend. One trigger per server-tracked toggle. Each - # carries the absolute target value the client's widget settled on, - # never a toggle and never a delta: the toolbar buttons are toggles, so - # the client reads its widget back after the local write and sends the - # result. A message the client suppresses as redundant is therefore - # indistinguishable from one that set a value the server already held, - # and both are correct. + # Frontend -> Backend. One trigger per server-tracked toggle. Each + # carries the absolute target value, not a delta, so a redundant + # message is indistinguishable from a no-op one, and both are fine. + # No ``origin`` field: unlike the camera, a toggle echo is idempotent. # - # No ``origin`` field (RS-1). The camera needed one because a - # programmatic echo re-applied a *stale* camera over a newer one; a - # toggle echo carries the same boolean the server already holds, so the - # write is idempotent and the client's own widgets damp it with their - # value guards. It is the same echo the per-part deliveries already - # produce on load. Separating delivery from mutation is a later story's - # work, not this one's. - # - # Payloads are validated at this boundary by ``@parse_payload`` exactly - # as the per-part ones are. The wire key is ``visible`` on the three - # visibility triggers and ``parallel`` on ``set_projection``; none carries - # a pydantic alias, snake_case and camelCase coinciding on all four. - # - # ``set_projection`` sits in this block because it arrives from the same - # toolbar and under the same absolute-value rule, but it is not a fourth - # toggle: the server keeps no projection field, the camera record holds - # it, and the coordinator re-serialises the camera as part of the write. + # ``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. # ------------------------------------------------------------------ @trigger("set_cross_section_visibility") diff --git a/src/ansys/visor/viewer/app/visor_vtk_local.py b/src/ansys/visor/viewer/app/visor_vtk_local.py index d61b4483..0a1258e8 100644 --- a/src/ansys/visor/viewer/app/visor_vtk_local.py +++ b/src/ansys/visor/viewer/app/visor_vtk_local.py @@ -52,9 +52,9 @@ def _initialize_rendering(self, standalone: bool, trame_log_dir: str | None) -> world_x, world_y, world_z: self._scene.pick_geometry(actor_wasm_id, cell_id, mode, world_x, world_y, world_z), - # The scene satisfies LocalApp's ScenePartStateApi protocol + # The scene satisfies LocalApp's SceneMutationApi protocol # structurally: the six per-part coordinator methods carry exactly # the names and signatures the triggers call. - scene_part_state_api=self._scene, + scene_mutation_api=self._scene, ) diff --git a/tests/unit/app/test_local_app.py b/tests/unit/app/test_local_app.py index b3f8154a..abf9fb2b 100644 --- a/tests/unit/app/test_local_app.py +++ b/tests/unit/app/test_local_app.py @@ -97,7 +97,7 @@ def mock_server(): @pytest.fixture def api(): """Stand-in for the injected per-part coordinator object.""" - return MagicMock(name="scene_part_state_api") + return MagicMock(name="scene_mutation_api") @pytest.fixture @@ -108,7 +108,7 @@ def app(mock_server, api): get_scene_details_json=MagicMock(), handle_save_state_response=MagicMock(), standalone=True, - scene_part_state_api=api, + scene_mutation_api=api, ) diff --git a/tests/unit/app/test_local_app_set_projection.py b/tests/unit/app/test_local_app_set_projection.py index 0b89f9e2..4a72ac4c 100644 --- a/tests/unit/app/test_local_app_set_projection.py +++ b/tests/unit/app/test_local_app_set_projection.py @@ -72,7 +72,7 @@ def mock_server(): @pytest.fixture def api(): """Stand-in for the injected scene coordinator.""" - return MagicMock(name="scene_part_state_api") + return MagicMock(name="scene_mutation_api") @pytest.fixture @@ -83,7 +83,7 @@ def app(mock_server, api): get_scene_details_json=MagicMock(), handle_save_state_response=MagicMock(), standalone=True, - scene_part_state_api=api, + scene_mutation_api=api, ) diff --git a/tests/unit/app/test_local_app_sync_camera.py b/tests/unit/app/test_local_app_sync_camera.py index 347ea322..9d06ac67 100644 --- a/tests/unit/app/test_local_app_sync_camera.py +++ b/tests/unit/app/test_local_app_sync_camera.py @@ -91,7 +91,7 @@ def mock_server(): @pytest.fixture def api(): """Stand-in for the injected scene coordinator.""" - return MagicMock(name="scene_part_state_api") + return MagicMock(name="scene_mutation_api") @pytest.fixture @@ -102,7 +102,7 @@ def app(mock_server, api): get_scene_details_json=MagicMock(), handle_save_state_response=MagicMock(), standalone=True, - scene_part_state_api=api, + scene_mutation_api=api, ) diff --git a/tests/unit/app/test_local_app_widget_triggers.py b/tests/unit/app/test_local_app_widget_triggers.py index 058b1c79..9dec8cfe 100644 --- a/tests/unit/app/test_local_app_widget_triggers.py +++ b/tests/unit/app/test_local_app_widget_triggers.py @@ -68,7 +68,7 @@ def mock_server(): @pytest.fixture def api(): """Stand-in for the injected scene coordinator.""" - return MagicMock(name="scene_part_state_api") + return MagicMock(name="scene_mutation_api") @pytest.fixture @@ -79,7 +79,7 @@ def app(mock_server, api): get_scene_details_json=MagicMock(), handle_save_state_response=MagicMock(), standalone=True, - scene_part_state_api=api, + scene_mutation_api=api, ) diff --git a/tests/unit/app/test_visor_vtk_local.py b/tests/unit/app/test_visor_vtk_local.py index 434f7fb5..eeb76d6d 100644 --- a/tests/unit/app/test_visor_vtk_local.py +++ b/tests/unit/app/test_visor_vtk_local.py @@ -560,8 +560,8 @@ def test_local_app_receives_the_scene_as_the_part_state_api(local_app_call): """The scene itself is injected, not a wrapper or a set of lambdas.""" call, scene, instance = local_app_call - assert call.kwargs["scene_part_state_api"] is scene - assert call.kwargs["scene_part_state_api"] is instance._scene + assert call.kwargs["scene_mutation_api"] is scene + assert call.kwargs["scene_mutation_api"] is instance._scene def test_local_app_still_receives_the_pre_existing_arguments(local_app_call): From 984498089389ff3b270fa5bce3e385283fb269bd Mon Sep 17 00:00:00 2001 From: Laura Kasian Date: Wed, 23 Sep 2026 15:22:48 -0700 Subject: [PATCH 11/13] clean up docs --- .../runtime/requests/widget_state_payloads.py | 10 +-- .../models/runtime/visor_scene_details.py | 10 +-- src/ansys/visor/viewer/renderer/base.py | 24 +++---- .../visor/viewer/renderer/local_renderer.py | 12 ++-- src/ansys/visor/viewer/vtk/scene/base.py | 45 +++++-------- .../visor/visor-client/src/VisorFrontend.tsx | 16 ++--- .../visor-client/src/renderer/IRenderer.ts | 63 ++++++------------- .../visor-client/src/renderer/WasmRenderer.ts | 56 +++++++---------- .../src/widgets/orthographicWidget.ts | 13 ++-- 9 files changed, 82 insertions(+), 167 deletions(-) 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 5f37cd19..3101e16b 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 @@ -1,10 +1,6 @@ """Models for the widget-state trigger payloads. -One model per server-tracked widget toggle, plus the projection. Field -names are already identical in snake_case and camelCase, so **no** -``Field(alias=...)`` is needed and none is to be added: the wire key is -exactly ``visible`` on the three visibility payloads and exactly -``parallel`` on ``SetProjectionPayload``. +One model per server-tracked widget toggle, plus the projection. Projection is not a fourth toggle. It has no store field on the scene: it is the camera record's ``parallel_projection``, written through the @@ -16,10 +12,6 @@ per-part triggers carrying camelCase aliases. These are neither. The precedent is ``sync_camera_payload.py``, the one existing non-per-part trigger, whose model lives in this package. - -No ``origin`` field (RS-1). The camera needed one because a programmatic -echo re-applied a *stale* camera over a newer one; a toggle echo carries -the same boolean the server already holds, so the write is idempotent. """ from pydantic import BaseModel, ConfigDict 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 ffc02de4..80a45ca5 100644 --- a/src/ansys/visor/viewer/models/runtime/visor_scene_details.py +++ b/src/ansys/visor/viewer/models/runtime/visor_scene_details.py @@ -36,15 +36,7 @@ def from_components( edges_enabled: bool | None = None, bounding_box_enabled: bool | None = None, ) -> "VisorSceneDetails": - """Construct an instance from components. - - The four widget-state keywords are forwarded, not stored here: this - payload is how a rebuilt or reconnecting client learns the server's - toggles, and ``RuntimeAppState.from_components`` already accepts every - one of them. No new model and no new field -- what was missing was - the argument passing, and a keyword dropped from the call below is - delivered as ``None`` and reverts that toggle in the browser. - """ + """Construct an instance from components.""" vtk_info = RuntimeVTKInfo( scene_graph=scene_graph_state, renderer_annotation=renderer_annotation, diff --git a/src/ansys/visor/viewer/renderer/base.py b/src/ansys/visor/viewer/renderer/base.py index 108b12cb..878594c2 100644 --- a/src/ansys/visor/viewer/renderer/base.py +++ b/src/ansys/visor/viewer/renderer/base.py @@ -204,21 +204,15 @@ def sync_camera(self, camera_state: "VisorCameraState") -> None: @abstractmethod def set_projection(self, parallel: bool) -> None: - """Set parallel projection on the camera record, and project it. - - Writes ``parallel_projection`` on the existing record **in place**, - preserving the object identity :meth:`sync_camera` documents, then - applies to the pipeline camera. Record first, pipeline second, so a - raising VTK setter still leaves the record holding what the caller - asked for. - - A ``None`` record is not seeded here. The write to the pipeline - still happens and the record stays ``None``, logged at debug; the - next :meth:`reset_camera` reads the pipeline and imports it. The - cost is named: a projection set before any camera has been written - is not saved until then. - - Re-serialisation is the coordinator's, not this method's. + """Set parallel projection on the camera record, then project it. + + Writes ``parallel_projection`` on the existing record in place + (preserving :meth:`sync_camera`'s identity contract) before applying + to the pipeline camera, so a raising VTK setter still leaves the + record holding what was asked. A ``None`` record is not seeded: the + pipeline write still happens, but the value is lost until the next + :meth:`reset_camera` imports it. Does not re-serialise; that is the + coordinator's job. """ @abstractmethod diff --git a/src/ansys/visor/viewer/renderer/local_renderer.py b/src/ansys/visor/viewer/renderer/local_renderer.py index 76178e49..5195b78a 100644 --- a/src/ansys/visor/viewer/renderer/local_renderer.py +++ b/src/ansys/visor/viewer/renderer/local_renderer.py @@ -381,13 +381,11 @@ def _apply_to_pipeline_camera(self, camera_state: "VisorCameraState") -> None: # ------------------------------------------------------------------ # IRenderer: widget control (cross-section, bounding box, edges) # - # ``set_edges_visible`` has a coordinator caller and a body. The two - # visibility verbs do not, and deliberately stay no-ops: the server's - # cross-section and bounding-box widget objects are driven by the client - # through the wasm mirror, nothing in LOCAL reads their enablement, and a - # server-side body would be a second writer with no reader. Edge - # visibility is different in kind -- it is an actor property on this - # renderer's own pipelines, and it has a reader here. + # ``set_cross_section_visibility`` and ``set_bounding_box_visibility`` + # stay no-ops: those widgets are driven client-side via the wasm mirror, + # and nothing in LOCAL reads their enablement. Edge visibility differs -- + # it's an actor property on this renderer's own pipelines, with a reader + # here. # ------------------------------------------------------------------ def set_edges_visible(self, visible: bool) -> None: diff --git a/src/ansys/visor/viewer/vtk/scene/base.py b/src/ansys/visor/viewer/vtk/scene/base.py index 55467e56..68739357 100644 --- a/src/ansys/visor/viewer/vtk/scene/base.py +++ b/src/ansys/visor/viewer/vtk/scene/base.py @@ -187,17 +187,9 @@ async def get_state(self, timeout: float) -> PersistedViewerStateV1: "absent says nothing" belongs to the load path, in :meth:`apply_state`, not here. - ``orthographic_enabled`` is derived from that same record rather than - stored, and it is **emitted here and not read on load**. The record - is bound once and read once: the camera and the projection cannot - disagree because there is nothing for them to disagree about. The - load path takes projection off ``camera.parallel_projection`` alone - and ignores this field, because two independent fields writing one - camera property is exactly the divergence that made a saved file - report one projection while the view showed the other. A ``None`` - record emits ``None``, which says "nothing was ever written" rather - than asserting perspective. Restoring the load-side read would - reopen the divergence; it is not a missing feature. + ``orthographic_enabled`` is derived from that same record, not stored + separately, so it can't disagree with the camera. ``None`` means + "nothing was ever written." """ runtime_state = await self._get_runtime_state_async(timeout) @@ -255,24 +247,19 @@ def apply_state(self, state: PersistedViewerStateV1): def get_scene_details(self) -> VisorSceneDetails: """Return the VisorState. - This payload is how a rebuilt or reconnecting client learns the - server's widget toggles. Everything else in this story makes the - server *authoritative*; this method is what makes it *deliver*, and - the three client branches that apply these fields already exist and - were dead only because nothing ever populated them. - - ``orthographic_enabled`` is derived from the camera record rather than - stored, so that the projection has exactly one source. A ``None`` - record delivers ``None``, which says "nothing was ever written" and - leaves the client's own flag alone. - - No ``_vtk_lock`` (RS-8). What is read here is three independent - boolean loads and one field off the camera record; nothing consumes - them as a mutually consistent snapshot, and the two larger reads - already in this method are already unlocked. Locking a request-path - read against the trigger thread, with the lock ordering on that path - untraced, belongs with the round-trip and thread-affinity work. The - accepted exposure is one stale field in a delivered payload. + How a rebuilt or reconnecting client learns the server's widget + toggles; the client branches that apply these fields already existed + and were dead only because nothing populated them. + + ``orthographic_enabled`` is derived from the camera record, not + 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. """ if self._scene_graph is None: self._initialize_scene_graph() diff --git a/src/ansys/visor/visor-client/src/VisorFrontend.tsx b/src/ansys/visor/visor-client/src/VisorFrontend.tsx index df818481..aa59dcc3 100644 --- a/src/ansys/visor/visor-client/src/VisorFrontend.tsx +++ b/src/ansys/visor/visor-client/src/VisorFrontend.tsx @@ -378,12 +378,9 @@ export class VisorFrontend { uiScaffold.setUnit(unit); })(); } - // `sceneState.orthographicEnabled` is deliberately not read here. - // Projection arrives on the camera, below, through the one call - // that also sets the widget flag `getAppStateAsync` reads back. - // The persisted toggle is still emitted for compatibility and is - // ignored on load: two independent fields writing one wasm camera - // property, in unspecified order, is the divergence this removed. + // `sceneState.orthographicEnabled` is deliberately not read; + // projection's sole writer is the camera write below. See + // AppStateProjectionLoad.test.ts for why. if (sceneState.crossSectionEnabled !== undefined) { const promise = renderer.setCrossSectionVisibilityAsync( sceneState.crossSectionEnabled @@ -418,11 +415,8 @@ export class VisorFrontend { promises.push(promise); } if (cameraState.parallelProjection !== undefined) { - // The sole writer of projection on this path. Routed through - // setOrthographicModeAsync rather than - // setCameraParallelProjectionAsync because that one also sets - // the widget flag; the camera-only call would leave the flag - // stale and the next save would write the stale value. + // Routed through setOrthographicModeAsync, not setCameraParallelProjectionAsync: only + // the former also sets the widget flag, so the latter would leave it stale for the next save. const promise = renderer.setOrthographicModeAsync(cameraState.parallelProjection); promises.push(promise); } diff --git a/src/ansys/visor/visor-client/src/renderer/IRenderer.ts b/src/ansys/visor/visor-client/src/renderer/IRenderer.ts index 3ecd71d7..705140d1 100644 --- a/src/ansys/visor/visor-client/src/renderer/IRenderer.ts +++ b/src/ansys/visor/visor-client/src/renderer/IRenderer.ts @@ -210,18 +210,19 @@ export interface IRenderer { sendClearPartColorVariableAsync(nodeId: NodeId): Promise; // ---- View-level widgets (state is renderer-owned; see arch rule (a)) --- + // + // The four cached-bool getters below (isCrossSectionVisible, + // isBoundingBoxVisible, isOrthographicEnabled, areEdgesVisibleGlobally) + // share one contract: each returns the last value the server delivered, + // or the last value this client set locally, whichever happened later. + // Each widget's cached flag is a projection of that value, not an + // independent source. A write followed by an immediate read returns the + // **pre-push** value -- the write is local and the server's confirmation + // arrives on a later fetch, so none of these is a read-after-write on the + // server's store. + setCrossSectionVisibilityAsync(visible?: boolean): Promise; - /** - * Whether the cross-section plane is shown. - * - * The value is the last one the server delivered, or the last one this - * client set locally, whichever happened later. The widget's cached flag - * is a projection of that value, not an independent source. - * - * A write followed by an immediate read returns the **pre-push** value: - * the write is local and the server's confirmation arrives on a later - * fetch, so this getter is not a read-after-write on the server's store. - */ + /** Whether the cross-section plane is shown. See contract note above. */ isCrossSectionVisible(): boolean; // cached bool, sync updateCrossSectionBoundsAsync(): Promise; getCrossSectionOriginAsync(): Promise; @@ -230,49 +231,21 @@ export interface IRenderer { setCrossSectionNormalAsync(normal: readonly number[]): Promise; setBoundingBoxVisibilityAsync(visible?: boolean): Promise; - /** - * Whether the bounding-box outline is shown. - * - * The value is the last one the server delivered, or the last one this - * client set locally, whichever happened later. The bounding-box widget's - * cached flag is a projection of that value, not an independent source. - * - * A write followed by an immediate read returns the **pre-push** value: - * the write is local and the server's confirmation arrives on a later - * fetch, so this getter is not a read-after-write on the server's store. - */ + /** Whether the bounding-box outline is shown. See contract note above. */ isBoundingBoxVisible(): boolean; updateBoundingBoxBoundsAsync(): Promise; setOrthographicModeAsync(enable?: boolean): Promise; /** - * Whether the view is in parallel (orthographic) projection. - * - * The value is the last one the server delivered -- seeded from the wasm - * camera when the renderer is built, which is the point at which the - * delivered camera is already in place -- or the last one this client set - * locally, whichever happened later. The orthographic widget's cached flag - * is a projection of that camera, not an independent source: the camera is - * the single place projection lives, and this flag only reflects it. - * - * A write followed by an immediate read returns the **pre-push** value: - * the write is local and the server's confirmation arrives on a later - * fetch, so this getter is not a read-after-write on the server's store. + * Whether the view is in parallel (orthographic) projection. See contract + * note above; additionally, the server-delivered value is seeded from the + * wasm camera when the renderer is built. The camera is the single place + * projection lives -- this flag only reflects it. */ isOrthographicEnabled(): boolean; setEdgeVisibilityGlobalAsync(visible?: boolean): Promise; - /** - * Whether edges are shown on every part in the scene. - * - * The value is the last one the server delivered, or the last one this - * client set locally, whichever happened later. The edges widget's cached - * flag is a projection of that value, not an independent source. - * - * A write followed by an immediate read returns the **pre-push** value: - * the write is local and the server's confirmation arrives on a later - * fetch, so this getter is not a read-after-write on the server's store. - */ + /** Whether edges are shown on every part in the scene. See contract note above. */ areEdgesVisibleGlobally(): boolean; toggleFullScreenAsync(): Promise; diff --git a/src/ansys/visor/visor-client/src/renderer/WasmRenderer.ts b/src/ansys/visor/visor-client/src/renderer/WasmRenderer.ts index 1134dd7d..126710a9 100644 --- a/src/ansys/visor/visor-client/src/renderer/WasmRenderer.ts +++ b/src/ansys/visor/visor-client/src/renderer/WasmRenderer.ts @@ -85,17 +85,15 @@ export class WasmRenderer implements IRenderer { /** * Read the orthographic flag out of the wasm camera, once, at construction. * - * This is the only asynchronous seam in building a renderer, and it exists - * because the orthographic widget's cached flag is otherwise `false` no - * matter what the camera says. By the time `createAsync` runs, the wasm - * state fetch has already completed, so the camera read here is a read of - * the *delivered* camera -- which is what makes `isOrthographicEnabled()` - * a projection of server state rather than an independent source. + * The only async seam in building a renderer: without it, the + * orthographic widget's cached flag starts `false` regardless of what the + * camera says. `createAsync` runs after the wasm state fetch completes, + * so this reads the *delivered* camera -- making `isOrthographicEnabled()` + * a projection of server state, not an independent source. * - * Deliberately not defensive: a seed that swallowed its own failure would - * leave the flag at `false` against a parallel camera, which is precisely - * the save-corrupting fault this seed was added to remove, and it would be - * invisible to every gate. + * Deliberately not defensive: swallowing a failure here would leave the + * flag at `false` against a parallel camera -- the exact save-corrupting + * fault this seed exists to remove -- invisibly to every gate. */ async #seedOrthographicFlagAsync(): Promise { await this.#orthographicWidget.seedFromCameraAsync(); @@ -417,14 +415,11 @@ export class WasmRenderer implements IRenderer { /** * Send one view-level widget payload to its server trigger. * - * The sibling of `#sendTriggerAsync` above, whose rationale -- no sender - * means no send, a rejection is logged and swallowed, the catch is - * unnarrowed -- applies here unchanged and is not repeated. + * Sibling of `#sendTriggerAsync` above; same no-sender/no-op and + * logged-and-swallowed-rejection rationale, not repeated here. * - * It exists separately only because a widget trigger has no `nodeId`. - * That helper takes one purely to name it in the error line, and passing a - * sentinel would put a fictitious node id in the one message a failed send - * produces. The prefix here is correspondingly distinct and greppable. + * Separate only because a widget trigger has no `nodeId` to name in the + * error line, so the log prefix here is distinct and greppable on its own. */ async #sendWidgetTriggerAsync( triggerName: string, @@ -502,25 +497,16 @@ export class WasmRenderer implements IRenderer { // ---- View-level widgets ------------------------------------------------- /** - * The four methods below each report to the server *after* the local - * write, and each reads its own widget back rather than forwarding the - * argument it was given. + * The four methods below report to the server *after* the local write, + * reading their own widget back rather than forwarding the argument they + * were given -- required, not stylistic: the toolbar calls all four with + * **no argument** (`Panel_BottomMiddle.tsx`), so each widget resolves the + * absent argument by negating its own cached flag, and only it knows what + * it settled on. The read-back uses the same getter `getAppStateAsync` + * does, so the value sent and the value saved cannot disagree. * - * That read-back is required, not stylistic. The toolbar calls all four - * with **no argument** -- see `Panel_BottomMiddle.tsx` -- and each widget - * resolves the absent argument itself by negating its cached flag. The - * argument is therefore not the value; only the widget knows what it - * settled on. Forwarding the argument would put `undefined` on the wire - * on every toolbar click, which type-checks and lints and fails only at - * the server's payload boundary. - * - * Each reads back through the same getter `getAppStateAsync` uses, so the - * value sent and the value saved cannot disagree. - * - * Every payload is absolute, never a toggle and never a delta, so a send - * that is suppressed, duplicated or reordered is harmless. A delivered - * state therefore echoes back one send per toggle; that echo is - * idempotent by construction and is accepted. + * Every payload is absolute, never a toggle or delta, so a send that is + * suppressed, duplicated, or reordered is harmless. */ async setCrossSectionVisibilityAsync(visible?: boolean): Promise { await this.#crossSectionWidget.setVisibilityAsync(visible); diff --git a/src/ansys/visor/visor-client/src/widgets/orthographicWidget.ts b/src/ansys/visor/visor-client/src/widgets/orthographicWidget.ts index 43e7dd2f..ae970d2d 100644 --- a/src/ansys/visor/visor-client/src/widgets/orthographicWidget.ts +++ b/src/ansys/visor/visor-client/src/widgets/orthographicWidget.ts @@ -26,14 +26,13 @@ export class OrthographicWidget { /** * Seed the cached flag from the wasm camera. * - * `#enabled` is initialised to `false` unconditionally at construction, - * and nothing else reads the real camera except the toggle path. A - * renderer built against a camera the server has already made parallel - * would therefore report perspective, and `getAppStateAsync` reads that - * cached flag -- so on a rebuild the wrong value is what gets saved. + * `#enabled` starts `false` unconditionally, and only the toggle path + * otherwise reads the real camera. Without this seed, a renderer built + * against a camera the server already made parallel would report + * perspective, and `getAppStateAsync` would save that wrong cached value. * - * Awaited from `WasmRenderer.createAsync`, after the wasm state fetch has - * completed, which is what makes the value read here the delivered one. + * Awaited from `WasmRenderer.createAsync`, after the wasm state fetch + * completes, so the value read here is the delivered one. */ seedFromCameraAsync = async (): Promise => { this.#enabled = await this.isOrthographicAsync(); From eb2237cfd799b574279d74f2baa22b5b2e8c183c Mon Sep 17 00:00:00 2001 From: Laura Kasian Date: Thu, 24 Sep 2026 11:04:30 -0700 Subject: [PATCH 12/13] rename _part_state_api -> _mutation_api --- src/ansys/visor/viewer/app/trame/local_app.py | 24 +++++++++---------- .../unit/app/test_local_app_set_projection.py | 4 ++-- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/ansys/visor/viewer/app/trame/local_app.py b/src/ansys/visor/viewer/app/trame/local_app.py index c86b88fa..4f2c25be 100644 --- a/src/ansys/visor/viewer/app/trame/local_app.py +++ b/src/ansys/visor/viewer/app/trame/local_app.py @@ -352,7 +352,7 @@ def perf_report_server_update(self, payload: dict): # body. # ------------------------------------------------------------------ - def _part_state_api(self, trigger_name: str, payload: BaseModel) -> SceneMutationApi | None: + def _mutation_api(self, trigger_name: str, payload: BaseModel) -> SceneMutationApi | None: """Return the injected coordinator, or ``None`` after logging.""" logger.debug("[trigger] %s arrived: %s.", trigger_name, payload) if self._scene_mutation_api is None: @@ -364,7 +364,7 @@ def _part_state_api(self, trigger_name: str, payload: BaseModel) -> SceneMutatio @parse_payload(SetPartVisibilityPayload) def set_part_visibility(self, payload) -> None: """Frontend -> Backend: set whether one part is visible.""" - api = self._part_state_api("set_part_visibility", payload) + api = self._mutation_api("set_part_visibility", payload) if api is None: return api.set_part_visibility(payload.node_id, payload.visible) @@ -377,7 +377,7 @@ def set_part_opacity(self, payload) -> None: An opacity outside ``[0.0, 1.0]`` fails validation and is a logged no-op; it does not reach VTK to be clamped. """ - api = self._part_state_api("set_part_opacity", payload) + api = self._mutation_api("set_part_opacity", payload) if api is None: return api.set_part_opacity(payload.node_id, payload.opacity) @@ -392,7 +392,7 @@ def set_part_diffuse_color(self, payload) -> None: or a colour that is not exactly three components, is a logged no-op -- nothing is delegated, so nothing is written to the store. """ - api = self._part_state_api("set_part_diffuse_color", payload) + api = self._mutation_api("set_part_diffuse_color", payload) if api is None: return api.set_part_diffuse_color(payload.node_id, payload.diffuse_rgb) @@ -405,7 +405,7 @@ def set_part_selected(self, payload) -> None: No colour crosses this trigger: the server reads the part's stored diffuse colour from its own record. """ - api = self._part_state_api("set_part_selected", payload) + api = self._mutation_api("set_part_selected", payload) if api is None: return api.set_part_selected(payload.node_id, payload.selected) @@ -423,7 +423,7 @@ def set_part_color_variable(self, payload) -> None: name. ``variableId`` is forwarded verbatim and is never parsed by the server. """ - api = self._part_state_api("set_part_color_variable", payload) + api = self._mutation_api("set_part_color_variable", payload) if api is None: return api.set_part_color_variable( @@ -440,7 +440,7 @@ def set_part_color_variable(self, payload) -> None: @parse_payload(ClearPartColorVariablePayload) def clear_part_color_variable(self, payload) -> None: """Frontend -> Backend: stop colouring one part by a scalar variable.""" - api = self._part_state_api("clear_part_color_variable", payload) + api = self._mutation_api("clear_part_color_variable", payload) if api is None: return api.clear_part_color_variable(payload.node_id) @@ -477,7 +477,7 @@ def sync_camera(self, payload) -> None: if payload.origin != "gesture": logger.debug("sync_camera: origin=%s; dropping.", payload.origin) return - api = self._part_state_api("sync_camera", payload) + api = self._mutation_api("sync_camera", payload) if api is None: return logger.debug( @@ -504,7 +504,7 @@ def sync_camera(self, payload) -> None: @parse_payload(SetCrossSectionVisibilityPayload) def set_cross_section_visibility(self, payload) -> None: """Frontend -> Backend: show or hide the cross-section plane.""" - api = self._part_state_api("set_cross_section_visibility", payload) + api = self._mutation_api("set_cross_section_visibility", payload) if api is None: return api.set_cross_section_visibility(payload.visible) @@ -513,7 +513,7 @@ def set_cross_section_visibility(self, payload) -> None: @parse_payload(SetEdgesVisiblePayload) def set_edges_visible(self, payload) -> None: """Frontend -> Backend: show or hide edges on every part.""" - api = self._part_state_api("set_edges_visible", payload) + api = self._mutation_api("set_edges_visible", payload) if api is None: return api.set_edges_visible(payload.visible) @@ -522,7 +522,7 @@ def set_edges_visible(self, payload) -> None: @parse_payload(SetBoundingBoxVisibilityPayload) def set_bounding_box_visibility(self, payload) -> None: """Frontend -> Backend: show or hide the bounding-box outline.""" - api = self._part_state_api("set_bounding_box_visibility", payload) + api = self._mutation_api("set_bounding_box_visibility", payload) if api is None: return api.set_bounding_box_visibility(payload.visible) @@ -536,7 +536,7 @@ def set_projection(self, payload) -> None: own: the coordinator writes the record and re-serialises the camera in one critical section. """ - api = self._part_state_api("set_projection", payload) + api = self._mutation_api("set_projection", payload) if api is None: return api.set_projection(payload.parallel) diff --git a/tests/unit/app/test_local_app_set_projection.py b/tests/unit/app/test_local_app_set_projection.py index 4a72ac4c..77218b3f 100644 --- a/tests/unit/app/test_local_app_set_projection.py +++ b/tests/unit/app/test_local_app_set_projection.py @@ -167,14 +167,14 @@ def test_set_projection_non_mapping_payload_is_a_logged_no_op(app, api): def test_set_projection_is_a_logged_no_op_when_no_coordinator_injected(app_without_api): """With nothing injected the trigger logs and returns without raising. - Two debug lines, both from ``_part_state_api`` and neither from this + Two debug lines, both from ``_mutation_api`` and neither from this handler: the arrival line every trigger emits, and the "no scene part-state API injected" line that says why nothing was delegated. The handler adds no logging of its own, so counting arrivals in the server log stays a sound measurement, and the trigger name appears on both lines so the count is attributable. - The literal ``2`` is hand-written from what ``_part_state_api`` does, not + The literal ``2`` is hand-written from what ``_mutation_api`` does, not copied from the neighbouring trigger modules, which assert ``1`` and are red at this increment's base commit for exactly that reason. """ From 490b545fe1a57dc0749e591ef89472bdcf6da572 Mon Sep 17 00:00:00 2001 From: Laura Kasian Date: Thu, 24 Sep 2026 11:11:48 -0700 Subject: [PATCH 13/13] fix naming in tests --- tests/unit/app/test_visor_vtk_local.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit/app/test_visor_vtk_local.py b/tests/unit/app/test_visor_vtk_local.py index eeb76d6d..3a71d91a 100644 --- a/tests/unit/app/test_visor_vtk_local.py +++ b/tests/unit/app/test_visor_vtk_local.py @@ -523,7 +523,7 @@ def test_load_state_passes_correct_metadata_to_add_dataset(tmp_path, iface): # LocalApp injection boundary # ================================================================== # -PART_STATE_API_METHODS = [ +SCENE_MUTATION_API_METHODS = [ "set_part_visibility", "set_part_opacity", "set_part_diffuse_color", @@ -556,7 +556,7 @@ def local_app_call(): return mock_local_app.call_args, mock_scene_inst, instance -def test_local_app_receives_the_scene_as_the_part_state_api(local_app_call): +def test_local_app_receives_the_scene_as_the_scene_mutation_api(local_app_call): """The scene itself is injected, not a wrapper or a set of lambdas.""" call, scene, instance = local_app_call @@ -591,7 +591,7 @@ def test_the_pre_existing_lambdas_still_delegate_to_the_scene(local_app_call): scene.pick_geometry.assert_called_once_with(2, 3, "vertex", 0.0, 1.0, 2.0) -@pytest.mark.parametrize("name", PART_STATE_API_METHODS) +@pytest.mark.parametrize("name", SCENE_MUTATION_API_METHODS) def test_local_scene_satisfies_the_part_state_protocol(name): """VisorLocalScene structurally provides every method the triggers call.""" from ansys.visor.viewer.vtk.scene.local_scene import VisorLocalScene