From b5092bb40ce95e66f927d08ffeba19c0aeb9eefd Mon Sep 17 00:00:00 2001 From: Laura Kasian Date: Mon, 21 Sep 2026 21:30:51 -0700 Subject: [PATCH 1/8] feat: server-authoritative cross-section plane # Conflicts: # src/ansys/visor/viewer/models/runtime/requests/widget_state_payloads.py # src/ansys/visor/viewer/vtk/scene/base.py --- src/ansys/visor/viewer/app/trame/local_app.py | 26 ++ .../runtime/requests/widget_state_payloads.py | 20 +- src/ansys/visor/viewer/renderer/base.py | 21 ++ .../visor/viewer/renderer/local_renderer.py | 78 ++++- .../visor/viewer/renderer/null_renderer.py | 37 ++- src/ansys/visor/viewer/vtk/scene/base.py | 39 +++ .../viewer/vtk/widgets/visor_cross_section.py | 6 + ...test_local_app_sync_cross_section_plane.py | 137 +++++++++ tests/unit/renderer/test_local_renderer.py | 260 +++++++++++++++++ tests/unit/vtk/scene/test_base.py | 276 +++++++++++++++++- .../test_visor_cross_section_widget.py | 12 + 11 files changed, 902 insertions(+), 10 deletions(-) create mode 100644 tests/unit/app/test_local_app_sync_cross_section_plane.py diff --git a/src/ansys/visor/viewer/app/trame/local_app.py b/src/ansys/visor/viewer/app/trame/local_app.py index 4f2c25be..ea637b35 100644 --- a/src/ansys/visor/viewer/app/trame/local_app.py +++ b/src/ansys/visor/viewer/app/trame/local_app.py @@ -18,6 +18,7 @@ SetCrossSectionVisibilityPayload, SetEdgesVisiblePayload, SetProjectionPayload, + SyncCrossSectionPlanePayload, ) logger = VisorDefaultLogger(__name__) @@ -71,6 +72,10 @@ def set_bounding_box_visibility(self, visible: bool) -> None: ... def set_projection(self, parallel: bool) -> None: ... + def sync_cross_section_plane( + self, origin: List[float], normal: List[float] + ) -> None: ... + # ---------------------------------------------------------------------- # Trigger payload models @@ -203,6 +208,7 @@ class LocalApp: 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 + sync_cross_section_plane: records a settled cross-section plane reported by the frontend 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. @@ -541,6 +547,26 @@ def set_projection(self, payload) -> None: return api.set_projection(payload.parallel) + @trigger("sync_cross_section_plane") + @parse_payload(SyncCrossSectionPlanePayload) + def sync_cross_section_plane(self, payload) -> None: + """Frontend -> Backend: a settled drag reports the cross-section plane. + + Not a toggle: it carries the origin and the normal the widget settled + on, both required and both exactly three components. A half-plane -- + a missing vector, or one of the wrong length -- fails validation and + is a logged no-op at this boundary rather than a half-applied plane, + which is ``sync_camera``'s stated posture for the same reason. + + The coordinator writes the record and both server VTK objects and + re-serialises them in one critical section; nothing is pushed from + here. + """ + api = self._part_state_api("sync_cross_section_plane", payload) + if api is None: + return + api.sync_cross_section_plane(payload.origin, payload.normal) + 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 3101e16b..610f1b98 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 @@ -12,9 +12,17 @@ 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. + +``SyncCrossSectionPlanePayload`` is not a toggle either. It is the plane +the client's drag settled on, reported at end of interaction, and it +carries two three-component vectors rather than a boolean. It lives here +rather than beside the per-part models for the same reason the four above +do: it is scene-wide and carries no ``nodeId``. """ -from pydantic import BaseModel, ConfigDict +from typing import List + +from pydantic import BaseModel, ConfigDict, Field class SetCrossSectionVisibilityPayload(BaseModel): @@ -47,3 +55,13 @@ class SetProjectionPayload(BaseModel): model_config = ConfigDict(populate_by_name=True) parallel: bool + + +class SyncCrossSectionPlanePayload(BaseModel): + """Payload of the ``sync_cross_section_plane`` trigger.""" + + model_config = ConfigDict(populate_by_name=True) + + origin: List[float] = Field(min_length=3, max_length=3) + normal: List[float] = Field(min_length=3, max_length=3) + diff --git a/src/ansys/visor/viewer/renderer/base.py b/src/ansys/visor/viewer/renderer/base.py index 5821b150..a5de4a9b 100644 --- a/src/ansys/visor/viewer/renderer/base.py +++ b/src/ansys/visor/viewer/renderer/base.py @@ -23,6 +23,9 @@ from vtkmodules.vtkCommonDataModel import vtkDataObject from ansys.visor.viewer.models.common.visor_camera_state import VisorCameraState + from ansys.visor.viewer.models.common.visor_cross_section_state import ( + VisorCrossSectionState, + ) from ansys.visor.viewer.models.runtime.vtk.renderer_annotation import RendererAnnotation from ansys.visor.viewer.vtk.scene_graph import VisorSceneGraphPartNode @@ -254,6 +257,24 @@ def sync_cross_section_plane( to server-side VTK objects. """ + @abstractmethod + def get_cross_section_plane(self) -> "VisorCrossSectionState | None": + """ + Return the cross-section plane record, or ``None`` if nothing has + written one yet. + """ + + @abstractmethod + def serialize_cross_section_state(self) -> None: + """Make the state served to the client current for the plane. + + Names its ids explicitly and never relies on a render following. + + **Serialize only; do not notify.** + + No-op on a renderer that serves the client no VTK object state. + """ + @abstractmethod def set_bounding_box_visibility(self, visible: bool) -> None: """Show or hide the bounding-box outline.""" diff --git a/src/ansys/visor/viewer/renderer/local_renderer.py b/src/ansys/visor/viewer/renderer/local_renderer.py index e2d28c85..92ba18c6 100644 --- a/src/ansys/visor/viewer/renderer/local_renderer.py +++ b/src/ansys/visor/viewer/renderer/local_renderer.py @@ -29,6 +29,7 @@ from ansys.visor.viewer.core.visor_colors import VisorColors 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.common.visor_cross_section_state import VisorCrossSectionState from ansys.visor.viewer.models.runtime.vtk.renderer_annotation import ( WasmNodeHandles, WasmRendererAnnotation, @@ -62,12 +63,14 @@ class VisorLocalRenderer(IRenderer): # Increment I5; mutated exclusively via register_node / deregister_node. _pipelines: dict[int, VtkNodePipeline] _last_camera_state: Optional["VisorCameraState"] + _last_cross_section_state: Optional["VisorCrossSectionState"] def __init__(self, server: Server): """Build the full local-mode VTK infrastructure and seed wasm state.""" self._server = server self._pipelines = {} self._last_camera_state = None + self._last_cross_section_state = None self._vtk_renderer = self._initialize_vtk_renderer() self._render_window = self._initialize_render_window() @@ -401,7 +404,47 @@ def set_cross_section_visibility(self, visible: bool) -> None: def sync_cross_section_plane( self, origin: list[float], normal: list[float] ) -> None: - """No-op in Story 1.2. Phase 3 populates.""" + """See :meth:`IRenderer.sync_cross_section_plane`. + + Writes the record, then both VTK objects. ``set_origin`` and + ``set_normal`` each write the plane *and* the representation, so the + clip function and the draggable handle cannot diverge on this path -- + which is the defect the client's own set/get asymmetry has. + """ + self._last_cross_section_state = VisorCrossSectionState( + origin=list(origin), normal=list(normal) + ) + self._cross_section_widget.set_origin(origin) + self._cross_section_widget.set_normal(normal) + + def get_cross_section_plane(self) -> Optional["VisorCrossSectionState"]: + """See :meth:`IRenderer.get_cross_section_plane`.""" + return self._last_cross_section_state + + def serialize_cross_section_state(self) -> None: + """See :meth:`IRenderer.serialize_cross_section_state`. + + Two ids, named one at a time, each derived the way + :meth:`serialize_camera_state` derives the camera's: ``GetId`` on the + object itself, not the id ``register_vtk_object`` returned. Deriving + it this way means the equivalence of those two id spaces never has to + be established. + + The plane is the clip function every pipeline holds; the + representation is the visible handle and is also what the client + reads on save. A plane-only re-serialise leaves the client saving a + stale handle. + + The widget id is deliberately not named: ``set_origin`` and + ``set_normal`` do not write the widget, whose changing state is + enablement. See MC-6. + """ + self._object_manager.UpdateStateFromObject( + self._object_manager.GetId(self._cross_section_widget.plane) + ) + self._object_manager.UpdateStateFromObject( + self._object_manager.GetId(self._cross_section_widget.plane_representation) + ) def set_bounding_box_visibility(self, visible: bool) -> None: """No-op in Story 1.2. Phase 3 populates.""" @@ -411,9 +454,40 @@ def set_bounding_box_visibility(self, visible: bool) -> None: # ------------------------------------------------------------------ def update_bounds(self, bounds: list[float]) -> None: - """See :meth:`IRenderer.update_bounds`.""" + """See :meth:`IRenderer.update_bounds`. + + Seeds the cross-section record from the values the widget's own + ``update_bounds`` just wrote through ``set_origin``/``set_normal``, so + the record is never ``None`` once a scene has been populated and + ``get_state`` can assign it unconditionally, as it does the camera's. + + **The seed runs only when there is no record.** This method is not + reached once per scene: ``set_part_visibility`` fans out to + ``_update_widget_bounds``, so a part toggle or a dataset add reaches + here too. Seeding unconditionally would therefore discard a plane the + user had dragged, on the next toggle, and the widget's own + ``update_bounds`` would already have overwritten both VTK objects with + the default plane. With a record present this writes the record + *back* to the widget instead -- after the widget's own call, never + before -- and leaves the record itself untouched. + + Ordering is load-bearing in both branches. Seeded before the widget + call, the record holds the previous plane and is one populate behind; + written back before it, the widget's defaults win and the drag is + lost. Either compiles, and either passes any test that does not + assert the order. + """ self._cross_section_widget.update_bounds(bounds) self._bounding_box_widget.update_bounds(bounds) + if self._last_cross_section_state is None: + plane = self._cross_section_widget.plane + self._last_cross_section_state = VisorCrossSectionState( + origin=list(plane.GetOrigin()), + normal=list(plane.GetNormal()), + ) + else: + self._cross_section_widget.set_origin(self._last_cross_section_state.origin) + self._cross_section_widget.set_normal(self._last_cross_section_state.normal) def update_actor_count(self, count: int) -> None: """See :meth:`IRenderer.update_actor_count`.""" diff --git a/src/ansys/visor/viewer/renderer/null_renderer.py b/src/ansys/visor/viewer/renderer/null_renderer.py index 2b4b2065..7995ce20 100644 --- a/src/ansys/visor/viewer/renderer/null_renderer.py +++ b/src/ansys/visor/viewer/renderer/null_renderer.py @@ -8,10 +8,11 @@ annotated return the simplest valid empty value for that type; all others are ``pass``. No VTK imports, no local view, no side effects. -One exception to "no state": the camera record, ``_last_camera_state``. The -record half of the :class:`IRenderer` camera contract is not optional on any -implementation -- only the projection half is, and here it is a no-op because -there is no pipeline camera to project onto. +One exception to "no state": the camera record, ``_last_camera_state``, and +the cross-section plane record, ``_last_cross_section_state``. The record +half of the :class:`IRenderer` camera and plane contracts is not optional on +any implementation -- only the projection half is, and here it is a no-op +because there is no pipeline camera and no widget to project onto. """ from __future__ import annotations @@ -19,6 +20,7 @@ from typing import TYPE_CHECKING, Optional from ansys.visor.viewer.core.visor_logging import VisorDefaultLogger +from ansys.visor.viewer.models.common.visor_cross_section_state import VisorCrossSectionState from ansys.visor.viewer.renderer.base import IRenderer if TYPE_CHECKING: @@ -34,10 +36,12 @@ class NullRenderer(IRenderer): """Null-object implementation of :class:`IRenderer` for use in tests.""" _last_camera_state: Optional["VisorCameraState"] + _last_cross_section_state: Optional["VisorCrossSectionState"] def __init__(self) -> None: - """Initialize the camera record.""" + """Initialize the camera and cross-section records.""" self._last_camera_state = None + self._last_cross_section_state = None # ------------------------------------------------------------------ # Wire contract @@ -159,7 +163,28 @@ def set_cross_section_visibility(self, visible: bool) -> None: def sync_cross_section_plane( self, origin: list[float], normal: list[float] ) -> None: - pass + """See :meth:`IRenderer.sync_cross_section_plane`. + + Record only: there are no VTK widget objects to project onto. The + record takes a copy of each list, as :class:`VisorLocalRenderer` does. + """ + self._last_cross_section_state = VisorCrossSectionState( + origin=list(origin), normal=list(normal) + ) + + def get_cross_section_plane(self) -> "VisorCrossSectionState | None": + """See :meth:`IRenderer.get_cross_section_plane`. + + ``None`` unless :meth:`sync_cross_section_plane` wrote one: there is + no widget here for :meth:`update_bounds` to seed a record from. + """ + return self._last_cross_section_state + + def serialize_cross_section_state(self) -> None: + """See :meth:`IRenderer.serialize_cross_section_state`. + + No-op: this renderer serves the client no VTK object state. + """ def set_bounding_box_visibility(self, visible: bool) -> None: pass diff --git a/src/ansys/visor/viewer/vtk/scene/base.py b/src/ansys/visor/viewer/vtk/scene/base.py index 68739357..b5c0bb05 100644 --- a/src/ansys/visor/viewer/vtk/scene/base.py +++ b/src/ansys/visor/viewer/vtk/scene/base.py @@ -190,6 +190,15 @@ async def get_state(self, timeout: float) -> PersistedViewerStateV1: ``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." + + The cross-section plane is the camera's twin and is taken from the + renderer's record on exactly the same terms. The assignment is + unconditional: the renderer seeds the record from its own widget the + first time bounds are pushed, so in normal operation there is no + ``None`` case to guard, and a ``None`` record is written through as + ``None`` because that is what says "no plane was ever written". The + guard for "absent says nothing" belongs to the load path, as it does + for the camera. """ runtime_state = await self._get_runtime_state_async(timeout) @@ -203,6 +212,7 @@ async def get_state(self, timeout: float) -> PersistedViewerStateV1: runtime_state.scene.orthographic_enabled = ( camera_record.parallel_projection if camera_record is not None else None ) + runtime_state.scene.cross_section = self._renderer.get_cross_section_plane() 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 @@ -493,6 +503,22 @@ def sync_camera(self, camera_state: VisorCameraState) -> None: self._renderer.sync_camera(camera_state) self._renderer.serialize_camera_state() + def sync_cross_section_plane( + self, origin: list[float], normal: list[float] + ) -> None: + """Record a cross-section plane the frontend reported, and project it. + + Both halves in one critical section, re-serialisation part of the + write, no notify -- for the reasons :meth:`sync_camera` gives. The + plane has exactly one delivery channel to a rebuilt or reconnecting + client, the wasm state fetch that follows this re-serialisation, so an + id that is not re-serialised here is simply not delivered and the + user's drag reappears where it started after a page reload. + """ + with self._vtk_lock: + self._renderer.sync_cross_section_plane(origin, normal) + self._renderer.serialize_cross_section_state() + def pick_geometry(self, actor_wasm_id, cell_id, mode, world_x, world_y, world_z) -> dict: """ Frontend-trigger entry point for cell picking. Packs the world-space @@ -768,6 +794,14 @@ def _restore_widget_state(self, runtime_app_state: "RuntimeAppState") -> None: compatibility and ignored here, because two readers of one property is the divergence this story removed. + The cross-section plane is restored here too, and both-or-neither: + ``sync_cross_section_plane`` takes an origin and a normal together + and the model allows either to be absent, so a half-plane says + nothing rather than half-applying. The re-serialisation follows the + write for the reason :meth:`sync_cross_section_plane` gives -- a + loaded plane that is written but not re-serialised leaves the client + fetching the pre-load one. + Callers must hold ``_vtk_lock``. """ scene = runtime_app_state.scene @@ -780,6 +814,11 @@ def _restore_widget_state(self, runtime_app_state: "RuntimeAppState") -> None: 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) + if scene.cross_section is not None: + cs = scene.cross_section + if cs.origin is not None and cs.normal is not None: + self._renderer.sync_cross_section_plane(cs.origin, cs.normal) + self._renderer.serialize_cross_section_state() def _restore_one_part_state( self, diff --git a/src/ansys/visor/viewer/vtk/widgets/visor_cross_section.py b/src/ansys/visor/viewer/vtk/widgets/visor_cross_section.py index 9a28f450..209ad39f 100644 --- a/src/ansys/visor/viewer/vtk/widgets/visor_cross_section.py +++ b/src/ansys/visor/viewer/vtk/widgets/visor_cross_section.py @@ -40,6 +40,7 @@ class VisorCrossSectionWidget: Properties: plane: Returns the VTK plane object. + plane_representation: Returns the VTK plane representation (the draggable handle). algorithm_filter: Returns the VTK algorithm filter. plane_wasm_id: Returns the WebAssembly ID for the plane object. plane_widget_wasm_id: Returns the WebAssembly ID for the plane widget. @@ -121,6 +122,11 @@ def plane(self) -> vtkPlane: """Returns the widget VTK plane object.""" return self._plane + @property + def plane_representation(self) -> vtkImplicitPlaneRepresentation: + """The draggable handle's representation.""" + return self._plane_representation + @property def algorithm_filter(self): """Returns the VTK algorithm filter.""" diff --git a/tests/unit/app/test_local_app_sync_cross_section_plane.py b/tests/unit/app/test_local_app_sync_cross_section_plane.py new file mode 100644 index 00000000..193de09b --- /dev/null +++ b/tests/unit/app/test_local_app_sync_cross_section_plane.py @@ -0,0 +1,137 @@ +"""Unit tests for ``LocalApp.sync_cross_section_plane`` -- the plane 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")``. This trigger carries no node id -- +it carries two three-component vectors and is scene-wide -- so adding a name +there would multiply the per-part tests by one more and rewrite them. + +Two tests, not six. The no-coordinator path and the not-a-mapping path run +through ``_part_state_api`` and ``parse_payload``, which three sibling trigger +modules already pin against the same two functions; repeating them here would +be a third and fourth copy of one failure mode rather than a new one. What is +unique to this trigger is that it forwards *two* vectors in a fixed order, and +that its name has to survive the payload decorator. + +Every payload here is a hand-written literal. The wire keys are ``origin`` +and ``normal`` and carry no alias: snake_case and camelCase coincide, which is +itself asserted by the delegation test passing a literal dict. +""" + +from unittest.mock import MagicMock + +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. +PLANE_TRIGGER = "sync_cross_section_plane" + +# Hand-written literals. No component is shared between the origin and the +# normal and neither is a permutation of the other, so a handler that swapped +# the two arguments fails on value rather than coinciding. +REPORTED_ORIGIN = [1.5, 2.5, 3.5] +REPORTED_NORMAL = [0.0, 1.0, 0.0] + + +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, + ) + + +# =========================================================================== +# Delegation +# =========================================================================== + +def test_sync_cross_section_plane_delegates_the_origin_and_normal(app, api): + """The trigger hands the coordinator both vectors, in that order. + + Positional, and asserted positionally, because the coordinator's signature + is ``(origin, normal)`` and a handler that passed them the other way round + would type-check, validate and run -- and would leave the plane at right + angles to where the user dragged it. The two literals share no component, + so the swap fails on value. + """ + result = app.sync_cross_section_plane( + {"origin": REPORTED_ORIGIN, "normal": REPORTED_NORMAL} + ) + + assert result is None + api.sync_cross_section_plane.assert_called_once_with( + REPORTED_ORIGIN, REPORTED_NORMAL + ) + + +# =========================================================================== +# Registration +# =========================================================================== + +def test_sync_cross_section_plane_trigger_name_is_registered_after_decoration( + app, mock_server +): + """The name survives the payload decorator, once, 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. + + Registered exactly once: a second registration under the same name would + leave which handler the client reaches dependent on registration order, + and nothing else in the suite would see it. + """ + 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 names.count(PLANE_TRIGGER) == 1 + assert PLANE_TRIGGER in registered + assert registered[PLANE_TRIGGER]( + {"origin": REPORTED_ORIGIN, "normal": REPORTED_NORMAL} + ) is None + diff --git a/tests/unit/renderer/test_local_renderer.py b/tests/unit/renderer/test_local_renderer.py index ca9119a1..1b9e012e 100644 --- a/tests/unit/renderer/test_local_renderer.py +++ b/tests/unit/renderer/test_local_renderer.py @@ -1022,3 +1022,263 @@ def test_pick_unknown_mode_returns_not_found(self, renderer): res = renderer.pick_geometry(1, 0, "INVALID_MODE", (0.0, 0.0, 0.0)) assert res == {"found": False} + +# =========================================================================== +# 7. The cross-section plane record +# +# Hand-written widget double rather than a MagicMock, on the precedent +# ``_CameraDouble`` sets in this module: the seed reads ``GetOrigin`` and +# ``GetNormal`` back into a pydantic model, which rejects Mock attributes, and +# the ordering tests need one shared call list across three widget methods +# rather than three independent recorders. +# +# Every literal below is hand-written. None is computed the way +# ``_get_default_plane_info`` computes it and none originates from VTK, so no +# assertion here can be satisfied by a VTK-constructed default. +# =========================================================================== + +# What the widget's own update_bounds left on the plane, read back by the seed. +SEEDED_PLANE_ORIGIN = [1.0, 2.0, 3.0] +SEEDED_PLANE_NORMAL = [0.0, 0.0, 1.0] + +# What a settled drag reports through sync_cross_section_plane. +REPORTED_PLANE_ORIGIN = [4.0, 5.0, 6.0] +REPORTED_PLANE_NORMAL = [0.0, 1.0, 0.0] + +# What an already-present record holds when update_bounds runs again. Every +# component differs from both sets above, so "wrote the record back" and +# "re-seeded from the widget" cannot both pass. +RECORDED_PLANE_ORIGIN = [7.0, 8.0, 9.0] +RECORDED_PLANE_NORMAL = [1.0, 0.0, 0.0] + +# One literal per object, so asserting these two fails -- rather than +# coincides -- if production names the widget, the render window or anything +# else. Distinct from the camera's literals above for the same reason. +CROSS_SECTION_PLANE_WASM_ID = 11 +CROSS_SECTION_REPRESENTATION_WASM_ID = 12 + + +class _PlaneDouble: + """Stand-in for the widget's ``vtkPlane``. + + Answers the two getters the seed reads with hand-written literals, so the + seeded record is asserted against values that never passed through VTK. + """ + + def GetOrigin(self): # noqa: N802 + return tuple(SEEDED_PLANE_ORIGIN) + + def GetNormal(self): # noqa: N802 + return tuple(SEEDED_PLANE_NORMAL) + + +class _CrossSectionWidgetDouble: + """Stand-in for ``VisorCrossSectionWidget``. + + ``calls`` is one shared, ordered list across ``update_bounds``, + ``set_origin`` and ``set_normal``, because what has to be pinned is their + order relative to each other and not merely that each happened. + + ``plane`` and ``plane_representation`` are two distinct objects, so an id + lookup keyed on identity can tell them apart and a re-serialise that named + the same object twice fails. + """ + + def __init__(self): + self.calls = [] + self._plane = _PlaneDouble() + self._plane_representation = object() + + @property + def plane(self): + return self._plane + + @property + def plane_representation(self): + return self._plane_representation + + def update_bounds(self, bounds): + self.calls.append(("update_bounds", list(bounds))) + + def set_origin(self, origin): + self.calls.append(("set_origin", list(origin))) + + def set_normal(self, normal): + self.calls.append(("set_normal", list(normal))) + + +@pytest.fixture +def widget(renderer): + """Install the hand-written cross-section widget double on *renderer*.""" + double = _CrossSectionWidgetDouble() + renderer._cross_section_widget = double + return double + + +BOUNDS = [-1.0, 1.0, -2.0, 2.0, -3.0, 3.0] + + +class TestCrossSectionPlane: + """The record, the two VTK writes, the two-id re-serialise, and the seed.""" + + # -- sync_cross_section_plane ------------------------------------------- + # + # The record half and the VTK half are separate tests: either can silently + # do nothing while the other succeeds, and a record that is never + # projected leaves the server's own clip plane where it was while + # ``get_state`` reports the new one. + + def test_sync_cross_section_plane_writes_the_record(self, renderer, widget): + """Store half: a reported plane becomes the server's record.""" + renderer.sync_cross_section_plane(REPORTED_PLANE_ORIGIN, REPORTED_PLANE_NORMAL) + + record = renderer.get_cross_section_plane() + assert record.origin == REPORTED_PLANE_ORIGIN + assert record.normal == REPORTED_PLANE_NORMAL + + def test_sync_cross_section_plane_writes_both_vtk_objects(self, renderer, widget): + """Apply half: the origin and the normal both reach the widget. + + Asserted as the whole call list, not with two ``assert_called_with``: + a body that wrote the origin twice, or that wrote the normal and not + the origin, is caught by the list and not by the pair. ``set_origin`` + and ``set_normal`` each write the plane *and* the representation, so + naming both here is what stops the clip and the handle diverging. + """ + renderer.sync_cross_section_plane(REPORTED_PLANE_ORIGIN, REPORTED_PLANE_NORMAL) + + assert widget.calls == [ + ("set_origin", REPORTED_PLANE_ORIGIN), + ("set_normal", REPORTED_PLANE_NORMAL), + ] + + # -- serialize_cross_section_state -------------------------------------- + + def test_serialize_cross_section_state_names_the_plane_and_the_representation_ids( + self, renderer, widget + ): + """The re-serialise names exactly two objects: the plane and the handle. + + This is the assertion that pins the increment. The id source is keyed + on object identity, so every object other than those two resolves to a + third, equally distinctive literal; asserting these two therefore + fails -- rather than coincides -- if production names the widget, the + render window or the renderer. + + Revert to a plane-only re-serialise and the call count is 1. That + revert leaves the server correct and the client saving a stale handle, + which no other assertion in this suite sees. + + No render is in this test at all, which is what pins the "never relies + on a render following" clause: the two calls are the whole mechanism. + """ + renderer._object_manager.GetId.side_effect = ( + lambda obj: CROSS_SECTION_PLANE_WASM_ID + if obj is widget.plane + else CROSS_SECTION_REPRESENTATION_WASM_ID + if obj is widget.plane_representation + else WRONG_OBJECT_WASM_ID + ) + + renderer.serialize_cross_section_state() + + calls = renderer._object_manager.UpdateStateFromObject.call_args_list + assert len(calls) == 2 + assert [call.args[0] for call in calls] == [ + CROSS_SECTION_PLANE_WASM_ID, + CROSS_SECTION_REPRESENTATION_WASM_ID, + ] + + # -- update_bounds: the seed -------------------------------------------- + + def test_update_bounds_seeds_the_record_when_there_is_none(self, renderer, widget): + """With no record, the record is seeded from the widget's own plane. + + This is what lets ``get_state`` assign the plane unconditionally, as + it does the camera. Reverted, the record stays ``None`` past the + first populate and a save of a scene nobody has dragged writes no + plane at all. + """ + assert renderer.get_cross_section_plane() is None + + renderer.update_bounds(BOUNDS) + + record = renderer.get_cross_section_plane() + assert record.origin == SEEDED_PLANE_ORIGIN + assert record.normal == SEEDED_PLANE_NORMAL + + def test_update_bounds_seeds_after_the_widgets_own_update_bounds( + self, renderer, widget + ): + """The seed reads the plane *after* the widget has rewritten it. + + Its own test, and not an extra assertion above, because the failure is + different in kind: a seed written before the widget call records the + *previous* plane, so the record is one populate behind and lags the + scene by one dataset load. The values test above still passes in that + arrangement whenever the two planes happen to agree, which is most of + the time. + + Asserted on the widget's own ordered call list, with the read-back + taken after the call returns: the widget call must have been made, and + the record must hold what the double reports *now*. + """ + renderer.update_bounds(BOUNDS) + + assert widget.calls == [("update_bounds", BOUNDS)] + assert renderer.get_cross_section_plane().origin == SEEDED_PLANE_ORIGIN + + # -- update_bounds: an existing record wins ----------------------------- + # + # ``set_part_visibility`` fans out to ``_update_widget_bounds``, so this + # method runs on every part toggle and every dataset add, not once per + # scene. The two tests below are what stop that fan-out discarding a + # plane the user dragged. They are separate because a body that wrote the + # record back to the widget *and* re-seeded it afterwards passes the first + # and fails the second. + + def test_update_bounds_with_a_record_writes_the_record_back_to_the_widget( + self, renderer, widget + ): + """An existing record is pushed back over the widget's defaults. + + The widget's own ``update_bounds`` unconditionally rewrites both VTK + objects to the default plane for the new bounds, so without this + write-back a part toggle moves the clip and the handle even though the + record still holds the dragged plane. + + Ordered after the widget call, and asserted as one list for that + reason: written before it, the widget's defaults win and the drag is + lost with every assertion on the record still passing. + """ + renderer.sync_cross_section_plane(RECORDED_PLANE_ORIGIN, RECORDED_PLANE_NORMAL) + widget.calls.clear() + + renderer.update_bounds(BOUNDS) + + assert widget.calls == [ + ("update_bounds", BOUNDS), + ("set_origin", RECORDED_PLANE_ORIGIN), + ("set_normal", RECORDED_PLANE_NORMAL), + ] + + def test_update_bounds_with_a_record_leaves_the_record_unchanged( + self, renderer, widget + ): + """The record is not re-seeded from the widget when it already exists. + + The negative twin of the test above. The widget double reports the + seed literals, which differ in every component from the record's, so a + body that wrote back *and* then re-seeded passes the write-back test + and fails this one -- and in the running application would still throw + the dragged plane away on the next save. + """ + renderer.sync_cross_section_plane(RECORDED_PLANE_ORIGIN, RECORDED_PLANE_NORMAL) + + renderer.update_bounds(BOUNDS) + + record = renderer.get_cross_section_plane() + assert record.origin == RECORDED_PLANE_ORIGIN + assert record.normal == RECORDED_PLANE_NORMAL + + diff --git a/tests/unit/vtk/scene/test_base.py b/tests/unit/vtk/scene/test_base.py index e51f98e8..57235c74 100644 --- a/tests/unit/vtk/scene/test_base.py +++ b/tests/unit/vtk/scene/test_base.py @@ -28,6 +28,7 @@ from ansys.visor.viewer.core.visor_colors import VisorColors from ansys.visor.viewer.core.visor_enums import VisorVtkVariableType from ansys.visor.viewer.models.common.visor_camera_state import VisorCameraState +from ansys.visor.viewer.models.common.visor_cross_section_state import VisorCrossSectionState from ansys.visor.viewer.models.common.visor_ui_state import VisorUIState from ansys.visor.viewer.models.common.visor_variable_state import VisorVariableState from ansys.visor.viewer.models.persist.persisted_viewer_state import PersistedViewerStateV1 @@ -1030,15 +1031,23 @@ class _CameraRecordRenderer: Records the scene's lock depth at the moment the record is read, so the read can be asserted to happen with the lock *held* rather than merely taken at some point. + + ``get_cross_section_plane`` is answered because ``get_state`` now reads + the plane record on every call, beside the camera's. It defaults to + ``None`` -- the camera tests above say nothing about the plane and must + keep saying nothing -- and the plane tests below pass one in. """ - def __init__(self, record, pipeline_camera, scene=None): + def __init__(self, record, pipeline_camera, scene=None, cross_section=None): self._record = record self._pipeline_camera = pipeline_camera self._scene = scene + self._cross_section = cross_section self.record_reads = 0 self.pipeline_reads = 0 + self.plane_reads = 0 self.depth_at_read = None + self.plane_depth_at_read = None def get_camera_state(self): self.record_reads += 1 @@ -1046,6 +1055,12 @@ def get_camera_state(self): self.depth_at_read = getattr(self._scene._vtk_lock, "depth", None) return self._record + def get_cross_section_plane(self): + self.plane_reads += 1 + if self._scene is not None: + self.plane_depth_at_read = getattr(self._scene._vtk_lock, "depth", None) + return self._cross_section + def _read_pipeline_camera(self): self.pipeline_reads += 1 return self._pipeline_camera @@ -2568,3 +2583,262 @@ def test_get_state_orthographic_enabled_is_none_when_the_record_is_empty(scene): assert persisted.scene.orthographic_enabled is None assert persisted.scene.camera is None + + +# =========================================================================== +# The cross-section plane -- trigger path, save path, load path +# +# ``VisorSceneBase.sync_cross_section_plane`` is the coordinator method the +# ``sync_cross_section_plane`` trigger routes through. As with the camera, +# the handler arrives on trame's daemon thread and must not touch the renderer +# directly, so what is asserted is the whole critical section: the write, the +# re-serialisation that follows it, and the lock held across both. +# +# The plane has exactly one delivery channel to a rebuilt or reconnecting +# client -- the wasm state fetch after the re-serialisation. It is not on the +# scene-details payload. That is why the re-serialisation is asserted here at +# all: dropping it leaves the server correct, every other gate green, and the +# user's drag snapping back on the next page reload. +# +# Own literals, distinct from the camera's above, so that a failure names the +# path that broke. +# =========================================================================== + +# What the renderer's record holds at save time. +RECORD_CROSS_SECTION_ORIGIN = [51.0, 52.0, 53.0] +RECORD_CROSS_SECTION_NORMAL = [0.0, 1.0, 0.0] + +# What the browser answers getState with. Never the right answer, and +# different in every component, so "read the record" and "read the reply" +# cannot both pass. +REPLY_CROSS_SECTION_ORIGIN = [61.0, 62.0, 63.0] +REPLY_CROSS_SECTION_NORMAL = [1.0, 0.0, 0.0] + +# What a settled drag reports, and what a save file carries on load. +GESTURE_CROSS_SECTION_ORIGIN = [71.0, 72.0, 73.0] +GESTURE_CROSS_SECTION_NORMAL = [0.0, 0.0, 1.0] + +LOADED_CROSS_SECTION_ORIGIN = [81.0, 82.0, 83.0] +LOADED_CROSS_SECTION_NORMAL = [0.0, 1.0, 0.0] + + +def _record_plane() -> VisorCrossSectionState: + """The renderer's plane record, from hand-written literals.""" + return VisorCrossSectionState( + origin=RECORD_CROSS_SECTION_ORIGIN, + normal=RECORD_CROSS_SECTION_NORMAL, + ) + + +def _reply_plane() -> VisorCrossSectionState: + """What the browser answers getState with. Never the right answer.""" + return VisorCrossSectionState( + origin=REPLY_CROSS_SECTION_ORIGIN, + normal=REPLY_CROSS_SECTION_NORMAL, + ) + + +def _plane_save_scene(scene, record_plane, reply_plane): + """Wire *scene* for a save: renderer record *record_plane*, reply *reply_plane*. + + The same shape as ``_save_scene`` above, but the reply carries a plane + rather than a camera, because that is the field whose source is under + test. The registry is emptied so the mapper's per-dataset loop + contributes nothing. + """ + double = _CameraRecordRenderer( + None, _pipeline_camera(), scene=scene, cross_section=record_plane + ) + 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={}, + cross_section=reply_plane, + ) + + scene._get_runtime_state_async = _get_runtime_state_async + return double + + +# --------------------------------------------------------------------------- +# The coordinator +# --------------------------------------------------------------------------- + +def test_sync_cross_section_plane_applies_to_the_renderer_and_then_serializes(scene): + """The re-serialisation follows the write, and there are two of them. + + Reverted -- the write kept and the re-serialisation dropped -- the record + is right, the server's own plane and representation are right, every other + test in this module still passes, and the client is served the pre-drag + plane on its next fetch. The user sees a page reload snap the plane back + to where it was before they dragged it. + + Two serialise calls, not one: ``serialize_cross_section_state`` names the + plane and the representation separately. Which object each names is + pinned in tests/unit/renderer/test_local_renderer.py, against a widget + double whose two objects have distinct ids; what is pinned here is that + the coordinator reaches that method at all, and reaches it after the + write. + """ + order = [] + real_sync = scene._renderer.sync_cross_section_plane + + def _sync(origin, normal): + order.append("plane") + return real_sync(origin, normal) + + scene._renderer.sync_cross_section_plane = _sync + scene._renderer._object_manager.UpdateStateFromObject = ( + lambda object_id: order.append("serialize") + ) + + scene.sync_cross_section_plane( + GESTURE_CROSS_SECTION_ORIGIN, GESTURE_CROSS_SECTION_NORMAL + ) + + assert order == ["plane", "serialize", "serialize"] + + +def test_sync_cross_section_plane_holds_the_lock_across_both_halves(scene): + """Both halves run inside one and the same critical section. + + Its own test rather than another assertion on the ordering test, on the + precedent of ``test_sync_camera_holds_the_lock_across_both_halves``: lock + depth and call order fail for different reasons and want to be readable + apart. The pair discriminates precisely -- a re-serialisation that was + dropped fails both, one that was merely moved below the ``with`` block + fails only this one. + + The two depths are asserted **equal** as well as non-zero. Non-zero alone + would pass a body that released and re-took the lock between the write and + the re-serialise, which is not one critical section: another thread can + mutate the VTK object graph in the gap, and the client is then served a + half-written scene. That failure is intermittent and never reproduces + under a gate. + """ + scene._vtk_lock = _LockSpy() + observed = {} + real_sync = scene._renderer.sync_cross_section_plane + + def _sync(origin, normal): + observed["write_depth"] = scene._vtk_lock.depth + return real_sync(origin, normal) + + scene._renderer.sync_cross_section_plane = _sync + scene._renderer._object_manager.UpdateStateFromObject = ( + lambda object_id: observed.update(serialize_depth=scene._vtk_lock.depth) + ) + + scene.sync_cross_section_plane( + GESTURE_CROSS_SECTION_ORIGIN, GESTURE_CROSS_SECTION_NORMAL + ) + + assert observed["write_depth"] >= 1 + assert observed["serialize_depth"] >= 1 + assert observed["serialize_depth"] == observed["write_depth"] + assert scene._vtk_lock.depth == 0 + assert scene._vtk_lock.enter_count == scene._vtk_lock.exit_count + + +# --------------------------------------------------------------------------- +# get_state -- the plane comes from the record, not from the reply +# --------------------------------------------------------------------------- + +def test_get_state_takes_the_cross_section_from_the_record(scene): + """The saved plane is the record's, with the browser saying otherwise. + + The record and the reply differ in every component, so "server + authoritative" and "round-trips the client's answer" cannot both pass. + Revert the assignment and both assertions below report the reply's + numbers. + + Asserted on what get_state RETURNS -- the object that reaches the writer + -- not on the runtime state it was built from. An assignment placed after + ``runtime_to_persisted`` passes every assertion made against the runtime + object and still writes the wrong file. + """ + _plane_save_scene(scene, _record_plane(), _reply_plane()) + + persisted = asyncio.run(scene.get_state(timeout=1.0)) + + assert persisted.scene.cross_section.origin == RECORD_CROSS_SECTION_ORIGIN + assert persisted.scene.cross_section.normal == RECORD_CROSS_SECTION_NORMAL + + +def test_get_state_writes_no_cross_section_when_the_record_is_empty(scene): + """An empty record writes ``None`` through, reply notwithstanding. + + The assignment is unconditional, exactly as the camera's, and this is the + only test that a guarded one -- one that skipped the write when the record + was ``None`` -- would fail. The reply carries a perfectly valid plane, so + the guarded version would save the browser's answer and look correct + everywhere else. + + ``None`` says "no plane was ever written". The guard for "absent says + nothing" belongs to the load path, not here. + """ + _plane_save_scene(scene, None, _reply_plane()) + + persisted = asyncio.run(scene.get_state(timeout=1.0)) + + assert persisted.scene.cross_section is None + + +# --------------------------------------------------------------------------- +# apply_state -- the load path writes the renderer and re-serialises +# --------------------------------------------------------------------------- + +def test_apply_state_serializes_the_loaded_plane_after_syncing_it(scene): + """A plane in the file reaches the renderer, and is then re-serialised. + + Driven with a real ``PersistedViewerStateV1`` through the real state + mapper, so this also exercises the mapper's plane pass-through; it is the + first test here that would notice if the mapper stopped handing + ``cross_section`` on verbatim. + + ``camera=None`` keeps the camera step out of the recording, so every + ``"serialize"`` below is the plane's. Reverted -- the sync kept and the + re-serialise dropped -- the server's plane is right and the client is + served the pre-load one, which is the defect the load path already had for + the camera and fixed for the same reason. + """ + order = [] + real_sync = scene._renderer.sync_cross_section_plane + + def _sync(origin, normal): + order.append(("plane", list(origin), list(normal))) + return real_sync(origin, normal) + + scene._renderer.sync_cross_section_plane = _sync + scene._renderer._object_manager.UpdateStateFromObject = ( + lambda object_id: order.append("serialize") + ) + + scene.apply_state( + PersistedViewerStateV1.from_components( + ui_state=VisorUIState(dark_theme=False), + unit="m", + orthographic_enabled=None, + cross_section_enabled=None, + edges_enabled=None, + bounding_box_enabled=None, + datasets={}, + camera=None, + cross_section=VisorCrossSectionState( + origin=LOADED_CROSS_SECTION_ORIGIN, + normal=LOADED_CROSS_SECTION_NORMAL, + ), + ) + ) + + assert order == [ + ("plane", LOADED_CROSS_SECTION_ORIGIN, LOADED_CROSS_SECTION_NORMAL), + "serialize", + "serialize", + ] + + diff --git a/tests/unit/vtk/widgets/test_visor_cross_section_widget.py b/tests/unit/vtk/widgets/test_visor_cross_section_widget.py index af754edf..8bf23e47 100644 --- a/tests/unit/vtk/widgets/test_visor_cross_section_widget.py +++ b/tests/unit/vtk/widgets/test_visor_cross_section_widget.py @@ -74,6 +74,18 @@ def test_plane_property(mocks): widget = VisorCrossSectionWidget(mocks['interactor']) assert widget.plane is widget._plane +def test_plane_representation_property(mocks): + """Verify that plane_representation returns the internal representation. + + Asserted by identity against ``_plane_representation`` and, separately, + as *not* the plane: the re-serialise names both objects one at a time and + a property that answered the plane would make it name the same object + twice, which is a plane-only re-serialise wearing two ids. + """ + widget = VisorCrossSectionWidget(mocks['interactor']) + assert widget.plane_representation is widget._plane_representation + assert widget.plane_representation is not widget.plane + def test_algorithm_filter_property(mocks): """Verify that the algorithm_filter property returns the internal algorithm filter object.""" widget = VisorCrossSectionWidget(mocks['interactor']) From 943996175c74f1d18373fbfe31a7984fe74f4dea Mon Sep 17 00:00:00 2001 From: Laura Kasian Date: Tue, 22 Sep 2026 11:07:16 -0700 Subject: [PATCH 2/8] feat: report the cross-section plane at the end of drag # Conflicts: # src/ansys/visor/visor-client/src/jest-tests/WasmRendererWidgetTriggers.test.tsx --- .../src/jest-tests/CrossSectionWidget.test.ts | 101 ++++++++++++++++++ .../WasmRendererWidgetTriggers.test.tsx | 54 +++++++++- .../visor-client/src/renderer/WasmRenderer.ts | 34 ++++++ .../src/widgets/crossSectionWidget.ts | 22 ++++ 4 files changed, 209 insertions(+), 2 deletions(-) create mode 100644 src/ansys/visor/visor-client/src/jest-tests/CrossSectionWidget.test.ts diff --git a/src/ansys/visor/visor-client/src/jest-tests/CrossSectionWidget.test.ts b/src/ansys/visor/visor-client/src/jest-tests/CrossSectionWidget.test.ts new file mode 100644 index 00000000..c65322fa --- /dev/null +++ b/src/ansys/visor/visor-client/src/jest-tests/CrossSectionWidget.test.ts @@ -0,0 +1,101 @@ +import { CrossSectionWidget } from '../widgets/crossSectionWidget'; +import type VtkScene from '../wasm/VtkScene'; + +/** + * The client cross-section widget's **set** path. + * + * The widget holds two wasm objects: the plane, which is the clip function + * every pipeline holds, and the representation, which is the draggable + * handle. The read path deliberately reads the representation, because that + * is what the handle moved. The write path used to write only the plane, and + * that asymmetry is the defect pinned here: a load set the clip and left the + * handle where it was, and the next end-of-drag report then carried the stale + * handle to the server, so the server-authoritative plane would have been + * *given* the wrong value rather than merely displaying one. + * + * Two tests, one per setter, because a fix applied to one of them only is a + * shippable defect in its own right -- the clip would follow a dragged origin + * and not a dragged normal -- and the two fail independently. + * + * Every value below is a hand-written literal, restated in the expectation + * rather than read back off the double. Nothing here is derived from VTK, and + * the doubles for the plane and the representation are separate objects, which + * is the whole point: a single shared double could not tell the two writes + * apart. + */ + +const PLANE_ID = 202; +const PLANE_WIDGET_ID = 203; +const PLANE_REPRESENTATION_ID = 204; + +function makeWidget() { + const plane = { + SetOrigin: jest.fn(async () => undefined), + SetNormal: jest.fn(async () => undefined), + GetOrigin: jest.fn(async () => [0, 0, 0]), + GetNormal: jest.fn(async () => [0, 0, 1]), + }; + const rep = { + SetOrigin: jest.fn(async () => undefined), + SetNormal: jest.fn(async () => undefined), + GetOrigin: jest.fn(async () => [0, 0, 0]), + GetNormal: jest.fn(async () => [0, 0, 1]), + }; + const planeWidget = { + On: jest.fn(async () => undefined), + Off: jest.fn(async () => undefined), + observe: jest.fn(), + }; + const scene = { + render: jest.fn(), + getVtkObject: (wasmId: number) => { + switch (wasmId) { + case PLANE_ID: + return plane; + case PLANE_REPRESENTATION_ID: + return rep; + default: + return planeWidget; + } + }, + }; + const widget = new CrossSectionWidget(scene as unknown as VtkScene, { + planeWasmId: PLANE_ID, + representationWasmId: PLANE_REPRESENTATION_ID, + widgetWasmId: PLANE_WIDGET_ID, + }); + return { widget, plane, rep }; +} + +describe('CrossSectionWidget writes both plane objects on the set path', () => { + test('setOriginAsync writes the representation and then the plane', async () => { + const { widget, plane, rep } = makeWidget(); + + await widget.setOriginAsync([1, 2, 3]); + + expect(rep.SetOrigin).toHaveBeenCalledTimes(1); + expect(rep.SetOrigin).toHaveBeenCalledWith([1, 2, 3]); + expect(plane.SetOrigin).toHaveBeenCalledTimes(1); + expect(plane.SetOrigin).toHaveBeenCalledWith([1, 2, 3]); + // Representation first, plane second, matching the order the server's + // own set_origin writes them in. + expect(rep.SetOrigin.mock.invocationCallOrder[0]).toBeLessThan( + plane.SetOrigin.mock.invocationCallOrder[0] + ); + }); + + test('setNormalAsync writes the representation and then the plane', async () => { + const { widget, plane, rep } = makeWidget(); + + await widget.setNormalAsync([0, 1, 0]); + + expect(rep.SetNormal).toHaveBeenCalledTimes(1); + expect(rep.SetNormal).toHaveBeenCalledWith([0, 1, 0]); + expect(plane.SetNormal).toHaveBeenCalledTimes(1); + expect(plane.SetNormal).toHaveBeenCalledWith([0, 1, 0]); + expect(rep.SetNormal.mock.invocationCallOrder[0]).toBeLessThan( + plane.SetNormal.mock.invocationCallOrder[0] + ); + }); +}); + 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 4c87d189..9a601d9e 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 @@ -94,8 +94,12 @@ function makeFakeWasmObjects() { 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]), + // Distinctive hand-written literals. They stand in for the + // *representation's* origin and normal, and they are deliberately + // nothing any default would produce, so a payload carrying them can + // only have come from reading the representation back. + GetOrigin: jest.fn(async () => [1.5, -2.5, 3.5]), + GetNormal: jest.fn(async () => [0, 1, 0]), SetOrigin: jest.fn(async () => undefined), SetNormal: jest.fn(async () => undefined), }; @@ -287,3 +291,49 @@ describe('WasmRenderer.createAsync seeds the orthographic flag from the wasm cam expect(renderer.isOrthographicEnabled()).toBe(false); }); }); + +describe('WasmRenderer reports the cross-section plane on the end-of-drag event', () => { + // The event itself does not exist under jsdom, so what is pinned here is + // the *registration*: which event the report is bound to, and what the + // callback bound to it sends. Whether that event ever fires is MC-5's + // subject and no gate reaches it. + test('the plane report is registered on EndInteractionEvent and sends the representation plane once', async () => { + const sender = makeSender(); + const { renderer, widget } = await makeRenderer(sender); + + const endCalls = widget.observe.mock.calls.filter( + (call) => call[0] === 'EndInteractionEvent' + ); + expect(endCalls).toHaveLength(1); + + await endCalls[0][1](); + + expect(sender).toHaveBeenCalledTimes(1); + expect(sender).toHaveBeenCalledWith('sync_cross_section_plane', { + origin: [1.5, -2.5, 3.5], + normal: [0, 1, 0], + }); + // `renderer` is held so the construction that registered the observer + // is not mistaken for dead code by a future reader. + expect(renderer.isCrossSectionVisible()).toBe(false); + }); + + test('no callback registered on InteractionEvent sends anything', async () => { + // InteractionEvent fires many times across one drag. A report bound + // to it would satisfy every other assertion in this module and flood + // the trigger channel, which is a fault no other gate can see. + const sender = makeSender(); + const { widget } = await makeRenderer(sender); + + const interactionCalls = widget.observe.mock.calls.filter( + (call) => call[0] === 'InteractionEvent' + ); + expect(interactionCalls.length).toBeGreaterThan(0); + for (const call of interactionCalls) { + await call[1](); + } + + expect(sender).not.toHaveBeenCalled(); + }); +}); + diff --git a/src/ansys/visor/visor-client/src/renderer/WasmRenderer.ts b/src/ansys/visor/visor-client/src/renderer/WasmRenderer.ts index 8998e36f..d61fff6d 100644 --- a/src/ansys/visor/visor-client/src/renderer/WasmRenderer.ts +++ b/src/ansys/visor/visor-client/src/renderer/WasmRenderer.ts @@ -62,6 +62,40 @@ export class WasmRenderer implements IRenderer { const planeWidget = vtkScene.getVtkObject(wasmPlaneWidgetId); planeWidget.observe('InteractionEvent', this.#crossSectionWidget.interactionHandler); + /** + * Report the settled plane to the server, once per handle release. + * + * `EndInteractionEvent`, not `InteractionEvent`: the per-motion event + * fires many times across one drag, and a report bound to it passes + * every other gate and floods the trigger channel. The end event + * fires once when the handle is released, and not on a camera orbit. + * + * A bare click on the handle with no drag also releases it, so it + * also reports -- carrying the plane the server already holds. That + * arrival is accepted as idempotent and is deliberately **not** + * suppressed: there is no last-sent cache and no debounce here, so a + * drag that produced no arrival means the observer is not wired, + * rather than meaning a filter swallowed it. + * + * The values come from `getOriginAsync` / `getNormalAsync`, which read + * the **representation** -- the object the handle actually moved, and + * the one whose values the client would otherwise save. The payload + * keys are `origin` and `normal`, snake_case and unaliased, three + * floats each, which is what the server's payload model requires. + * + * Deliberately not defensive: `#sendWidgetTriggerAsync` already + * swallows and logs a send rejection, and a `catch` around the two + * reads would turn a broken representation read into a silent + * no-report, which is the one outcome that reads as "the observer was + * never wired". + */ + planeWidget.observe('EndInteractionEvent', async () => { + await this.#sendWidgetTriggerAsync('sync_cross_section_plane', { + origin: await this.#crossSectionWidget.getOriginAsync(), + normal: await this.#crossSectionWidget.getNormalAsync(), + }); + }); + // Bounding-box ids are stashed for attachSceneGraph, which is the // point at which the live sceneGraph (needed by BoundingBoxWidget) // becomes available. diff --git a/src/ansys/visor/visor-client/src/widgets/crossSectionWidget.ts b/src/ansys/visor/visor-client/src/widgets/crossSectionWidget.ts index 5774aacd..041b1938 100644 --- a/src/ansys/visor/visor-client/src/widgets/crossSectionWidget.ts +++ b/src/ansys/visor/visor-client/src/widgets/crossSectionWidget.ts @@ -76,10 +76,32 @@ export class CrossSectionWidget { getNormalAsync: () => Promise = async () => { return await this.#rep.GetNormal(); }; + /** + * Write a plane to **both** objects: the representation first, then the + * plane. + * + * The two setters below and the two getters above are deliberately no + * longer symmetric in what they touch. The getters read the + * representation, because that is the object the draggable handle moves. + * The setters write the representation *and* the plane, because the + * representation is the handle and the plane is the clip function every + * pipeline holds, and a write that reached only one of them left the two + * disagreeing: a load set the clip while the handle stayed where it was, + * and the next end-of-drag report then carried that stale handle to the + * server. + * + * Representation first, plane second, matching the order the server's own + * `VisorCrossSectionWidget.set_origin` / `set_normal` write them in. The + * order is consistency with the server rather than a correctness + * requirement of its own -- these are two independent objects and neither + * write feeds the other. + */ setOriginAsync: (origin: number[]) => Promise = async (origin) => { + await this.#rep.SetOrigin(origin); await this.#plane.SetOrigin(origin); }; setNormalAsync: (normal: number[]) => Promise = async (normal) => { + await this.#rep.SetNormal(normal); await this.#plane.SetNormal(normal); }; setVisibilityAsync: (visible?: boolean) => Promise; From ef8595eb71d390b6a33dadb86f87e57a8cc649e6 Mon Sep 17 00:00:00 2001 From: pyansys-ci-bot <92810346+pyansys-ci-bot@users.noreply.github.com> Date: Tue, 22 Sep 2026 22:08:25 +0000 Subject: [PATCH 3/8] chore: adding changelog file 139.added.md [dependabot-skip] --- doc/changelog.d/139.added.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 doc/changelog.d/139.added.md diff --git a/doc/changelog.d/139.added.md b/doc/changelog.d/139.added.md new file mode 100644 index 00000000..72c6ffc1 --- /dev/null +++ b/doc/changelog.d/139.added.md @@ -0,0 +1 @@ +[Remote rendering 3.3c] server-authoritative cross-section plane From 157f3263ea89a11de7d1bd7a0f334b3a70565698 Mon Sep 17 00:00:00 2001 From: Laura Kasian Date: Tue, 22 Sep 2026 15:12:18 -0700 Subject: [PATCH 4/8] prettier fixes --- .../visor/visor-client/src/jest-tests/CrossSectionWidget.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/CrossSectionWidget.test.ts b/src/ansys/visor/visor-client/src/jest-tests/CrossSectionWidget.test.ts index c65322fa..5e74e078 100644 --- a/src/ansys/visor/visor-client/src/jest-tests/CrossSectionWidget.test.ts +++ b/src/ansys/visor/visor-client/src/jest-tests/CrossSectionWidget.test.ts @@ -98,4 +98,3 @@ describe('CrossSectionWidget writes both plane objects on the set path', () => { ); }); }); - 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 9a601d9e..0ce6a698 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 @@ -336,4 +336,3 @@ describe('WasmRenderer reports the cross-section plane on the end-of-drag event' expect(sender).not.toHaveBeenCalled(); }); }); - From 3c9e72d6d8cac197b350104581c7842bf3ec2682 Mon Sep 17 00:00:00 2001 From: Laura Kasian Date: Wed, 23 Sep 2026 14:26:44 -0700 Subject: [PATCH 5/8] move finalize_scene to apply_state instead of when adding datasets --- src/ansys/visor/viewer/app/visor_vtk.py | 3 -- src/ansys/visor/viewer/vtk/scene/base.py | 19 +++++++ .../visor/viewer/vtk/scene/local_scene.py | 4 +- tests/unit/app/test_visor_vtk_local.py | 35 ++++++++++--- tests/unit/vtk/scene/test_base.py | 52 +++++++++++++++++-- 5 files changed, 97 insertions(+), 16 deletions(-) diff --git a/src/ansys/visor/viewer/app/visor_vtk.py b/src/ansys/visor/viewer/app/visor_vtk.py index 8e155127..2d8442db 100644 --- a/src/ansys/visor/viewer/app/visor_vtk.py +++ b/src/ansys/visor/viewer/app/visor_vtk.py @@ -459,9 +459,6 @@ def _load_datasets_from_state(self, state: "PersistedViewerStateV1") -> None: if dataset is not None: dataset.mark_clean() - if self._scene.dataset_count > 0: - self._scene.finalize_scene(skip_reset_camera=True) - """ Protected Methods (async start and stop for FastAPI) """ @require_server_off @validate_input_metadata_types diff --git a/src/ansys/visor/viewer/vtk/scene/base.py b/src/ansys/visor/viewer/vtk/scene/base.py index b5c0bb05..eb80b5d3 100644 --- a/src/ansys/visor/viewer/vtk/scene/base.py +++ b/src/ansys/visor/viewer/vtk/scene/base.py @@ -247,6 +247,25 @@ def apply_state(self, state: PersistedViewerStateV1): self._restore_camera_state(runtime_app_state) # TODO: restore UI state and variable states when they are synced back to the server. + # Finalize here, not on the load path. On a cold load -- viewer started with no dataset, + # then a state loaded -- load_state adds the datasets and only then calls apply_state, so a + # finalize on the load path syncs wasm from a scene that predates every restore above: the + # client was served a scene with no cross-section plane, and neither the plane nor its + # handle ever rendered. Finalizing here syncs after the restores. + # + # Before _push_runtime_state, never after. finalize_scene -> render() ends in + # LocalView.update(), the same wasm flush flush_wasm_state performs; placed after the push + # it *is* the flush the note below refuses, racing the client's rebuild against the + # fire-and-forget set_state. See VisorLocalScene._push_runtime_state, which is written on + # the assumption that the full render and wasm sync have already happened by the time it runs. + # + # Correct only after _restore_widget_state: populate_scene reaches IRenderer.update_bounds, + # which writes an existing cross-section record back to the widget rather than seeding from + # the widget's defaults. With no record -- which is what the load path had on a cold load -- + # that branch seeds the defaults instead. + if self.dataset_count > 0: + self.finalize_scene(skip_reset_camera=True) + # 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. self._push_runtime_state(runtime_app_state) diff --git a/src/ansys/visor/viewer/vtk/scene/local_scene.py b/src/ansys/visor/viewer/vtk/scene/local_scene.py index 42faf6da..1970e332 100644 --- a/src/ansys/visor/viewer/vtk/scene/local_scene.py +++ b/src/ansys/visor/viewer/vtk/scene/local_scene.py @@ -59,8 +59,8 @@ def _push_runtime_state(self, runtime_app_state: "RuntimeAppState") -> None: the React frontend, which clears the setState event listener while it rebuilds the scene. If the set_state() JS call below arrives during that window the persisted state is silently lost. - finalize_scene() (called just before apply_state when loading from an - empty scene) has already done a full render + wasm sync; all we need + finalize_scene() (called from apply_state, in the statement immediately + before this one) has already done a full render + wasm sync; all we need here is a lightweight VTK flush before the JS payload is sent. """ self._renderer.render_window_only() diff --git a/tests/unit/app/test_visor_vtk_local.py b/tests/unit/app/test_visor_vtk_local.py index 3a71d91a..4ac73634 100644 --- a/tests/unit/app/test_visor_vtk_local.py +++ b/tests/unit/app/test_visor_vtk_local.py @@ -412,7 +412,19 @@ def _make_state_with_datasets(snapshot_path: str | None, name="model", unit="m") def test_load_state_restores_datasets_when_registry_empty(tmp_path, iface): - """Verify that datasets are restored when no datasets are currently loaded.""" + """Verify that datasets are restored when no datasets are currently loaded. + + ``finalize_scene`` is asserted *not* called: it belongs to ``apply_state``, + which is mocked here, so a call arriving at this level could only have come + from the load path. That is the pin against returning it to + ``_load_datasets_from_state``; its placement *within* ``apply_state`` is + pinned by + tests/unit/vtk/scene/test_base.py::test_apply_state_flushes_before_the_bridge_call. + + ``side_effect`` is a one-element list rather than a return value so that + the number of ``dataset_count`` reads stays pinned: ``load_state`` reads it + once, and a second read raises StopIteration. + """ snapshot = tmp_path / "model_snapshot.vtkhdf" snapshot.touch() state = _make_state_with_datasets(str(snapshot), name="model") @@ -421,7 +433,7 @@ def test_load_state_restores_datasets_when_registry_empty(tmp_path, iface): iface._file_io.read_state = MagicMock(return_value=state) iface._file_io.read_snapshot = MagicMock(return_value=mock_data) iface._file_io.build_metadata_for_load_state = MagicMock(return_value=MagicMock(spec=ExtendedMetadata)) - type(iface._scene).dataset_count = PropertyMock(side_effect=[0, 1]) + type(iface._scene).dataset_count = PropertyMock(side_effect=[0]) loaded_ds = MagicMock() iface._scene.add_dataset = MagicMock(return_value=42) iface._scene.datasets = {42: loaded_ds} @@ -433,7 +445,7 @@ def test_load_state_restores_datasets_when_registry_empty(tmp_path, iface): iface._file_io.read_snapshot.assert_called_once_with(str(snapshot)) iface._scene.add_dataset.assert_called_once_with(mock_data, iface._file_io.build_metadata_for_load_state.return_value) loaded_ds.mark_clean.assert_called_once() - iface._scene.finalize_scene.assert_called_once() + iface._scene.finalize_scene.assert_not_called() iface._scene.apply_state.assert_called_once_with(state) @@ -456,19 +468,28 @@ def test_load_state_skips_dataset_restore_when_registry_not_empty(tmp_path, ifac def test_load_state_skips_missing_snapshot(tmp_path, iface): - """Verify that missing dataset snapshots are ignored during state loading.""" + """Verify that missing dataset snapshots are ignored during state loading. + + This test pins the missing-snapshot skip and nothing else. It previously + also asserted ``finalize_scene`` was not called; that assertion no longer + pins anything, because ``finalize_scene`` is not reachable from + ``load_state`` at all -- it lives in ``apply_state``, which is mocked here + -- so it passed for a reason unrelated to the missing snapshot. It has + been removed rather than kept green. The load path is pinned by + ``test_load_state_restores_datasets_when_registry_empty`` and the placement + by + tests/unit/vtk/scene/test_base.py::test_apply_state_flushes_before_the_bridge_call. + """ state = _make_state_with_datasets("/nonexistent/snap.vtkhdf", name="model") iface._file_io.read_state = MagicMock(return_value=state) iface._file_io.read_snapshot = MagicMock() type(iface._scene).dataset_count = PropertyMock(return_value=0) iface._scene.apply_state = MagicMock() - iface._scene.finalize_scene = MagicMock() iface.load_state(str(tmp_path)) iface._file_io.read_snapshot.assert_not_called() - iface._scene.finalize_scene.assert_not_called() iface._scene.apply_state.assert_called_once_with(state) @@ -507,7 +528,7 @@ def test_load_state_passes_correct_metadata_to_add_dataset(tmp_path, iface): iface._file_io.read_snapshot = MagicMock(return_value=mock_data) iface._file_io.build_metadata_for_load_state = MagicMock(return_value=built_meta) iface._scene.add_dataset = MagicMock(return_value=1) - type(iface._scene).dataset_count = PropertyMock(side_effect=[0, 1]) + type(iface._scene).dataset_count = PropertyMock(side_effect=[0]) iface._scene.datasets = {1: MagicMock()} iface._scene.apply_state = MagicMock() iface._scene.finalize_scene = MagicMock() diff --git a/tests/unit/vtk/scene/test_base.py b/tests/unit/vtk/scene/test_base.py index 57235c74..3ac40a2a 100644 --- a/tests/unit/vtk/scene/test_base.py +++ b/tests/unit/vtk/scene/test_base.py @@ -1238,15 +1238,37 @@ def test_apply_state_applies_every_part_to_the_pipeline( assert second_pipeline.actor.GetProperty().GetOpacity() == pytest.approx(0.25) -def test_apply_state_does_not_flush_after_the_bridge_call(scene, registry): - """Exactly one flush, and it is ordered after the bridge call returns.""" +def test_apply_state_flushes_before_the_bridge_call(scene, registry): + """Exactly one flush, and it is ordered before the bridge call. + + This is the assertion that pins the placement of the ``finalize_scene`` + call in ``apply_state``. The flush is what carries the restored object + graph -- the cross-section plane above all -- to the client, so it has to + run *after* every restore and *before* the fire-and-forget ``set_state`` + the bridge call issues. A flush after the push races the client's rebuild + against that call, which is the loss ``VisorLocalScene._push_runtime_state`` + documents. + + The spy is on ``_local_view.update`` and not on ``flush_wasm_state``, + deliberately. ``flush_wasm_state`` *is* ``_local_view.update()``, and + ``render()`` ends in the same call, so only a spy at that boundary sees the + flush whichever route it arrives by. The previous version of this test + spied ``flush_wasm_state`` and was therefore blind to the flush + ``finalize_scene`` performs: it stayed green with the call before the push, + after the push, and absent entirely. + + Three wrong placements, one failing assertion: after the push gives + ``["bridge", "flush"]``, removed gives ``["bridge"]``, and returned to the + load path also gives ``["bridge"]``, since nothing in this module drives + ``load_state``. + """ order = [] scene._push_runtime_state = lambda state: order.append("bridge") - scene._renderer.flush_wasm_state = lambda: order.append("flush") + scene._renderer._local_view.update = lambda: order.append("flush") _apply(scene, _runtime_state({NODE_ID: RuntimePartProperties(id=NODE_ID, opacity=0.25)})) - assert order == ["bridge"] + assert order == ["flush", "bridge"] # --------------------------------------------------------------------------- @@ -1960,11 +1982,24 @@ class _ToggleSpyRenderer: 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. + + ``update_bounds``, ``update_actor_count`` and ``render`` are the scene + surface ``apply_state`` reaches through its ``finalize_scene`` call, and + are the only three names added for it: with a toggle-only state the part + loop makes no renderer call, the cross-section and camera restores are + both guarded off, and ``skip_reset_camera=True`` keeps ``reset_camera`` + out. Anything else is still an AttributeError, which is the point of the + double. They record into ``scene_calls`` rather than ``calls`` so that + the toggle assertions keep their exact meaning; nothing asserts + ``scene_calls``, which exists so a failure dump shows what was reached. + The placement of the finalize call is pinned in + ``test_apply_state_flushes_before_the_bridge_call``. """ def __init__(self, scene): self._scene = scene self.calls = [] + self.scene_calls = [] self.depths = {} def _record(self, name, visible): @@ -1980,6 +2015,15 @@ def set_edges_visible(self, visible): def set_bounding_box_visibility(self, visible): self._record("set_bounding_box_visibility", visible) + def update_bounds(self, bounds): + self.scene_calls.append("update_bounds") + + def update_actor_count(self, count): + self.scene_calls.append("update_actor_count") + + def render(self): + self.scene_calls.append("render") + class _SceneDetailsRenderer: """Renderer double for the scene-details delivery path. From 054a6eeee685d789ab08d50c6fad65ffcfb7cd2d Mon Sep 17 00:00:00 2001 From: Laura Kasian Date: Wed, 23 Sep 2026 15:11:42 -0700 Subject: [PATCH 6/8] rename ScenePartStateApi -> SceneMutationApi and clean up docs --- tests/unit/app/test_local_app_sync_cross_section_plane.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/app/test_local_app_sync_cross_section_plane.py b/tests/unit/app/test_local_app_sync_cross_section_plane.py index 193de09b..50ff8f66 100644 --- a/tests/unit/app/test_local_app_sync_cross_section_plane.py +++ b/tests/unit/app/test_local_app_sync_cross_section_plane.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, ) From 7ee86d1a1afb678da19efc8ebf36bb20b8c72628 Mon Sep 17 00:00:00 2001 From: Laura Kasian Date: Wed, 23 Sep 2026 20:38:25 -0700 Subject: [PATCH 7/8] clean up docstring --- src/ansys/visor/viewer/vtk/scene/base.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/ansys/visor/viewer/vtk/scene/base.py b/src/ansys/visor/viewer/vtk/scene/base.py index eb80b5d3..04c9ca91 100644 --- a/src/ansys/visor/viewer/vtk/scene/base.py +++ b/src/ansys/visor/viewer/vtk/scene/base.py @@ -191,14 +191,12 @@ async def get_state(self, timeout: float) -> PersistedViewerStateV1: separately, so it can't disagree with the camera. ``None`` means "nothing was ever written." - The cross-section plane is the camera's twin and is taken from the - renderer's record on exactly the same terms. The assignment is - unconditional: the renderer seeds the record from its own widget the - first time bounds are pushed, so in normal operation there is no - ``None`` case to guard, and a ``None`` record is written through as - ``None`` because that is what says "no plane was ever written". The - guard for "absent says nothing" belongs to the load path, as it does - for the camera. + The cross-section plane is the camera's twin, taken from the + renderer's record on exactly the same terms -- see the camera + paragraph above for why the assignment is unconditional and what + ``None`` means. In normal operation there is no ``None`` case to + guard because the renderer seeds the record from its own widget the + first time bounds are pushed. """ runtime_state = await self._get_runtime_state_async(timeout) From b87c85f3bc90370f754593701271afad0cf6c221 Mon Sep 17 00:00:00 2001 From: Laura Kasian Date: Thu, 24 Sep 2026 11:13:35 -0700 Subject: [PATCH 8/8] rename part_state_api to mutation_api --- src/ansys/visor/viewer/app/trame/local_app.py | 2 +- tests/unit/app/test_local_app_sync_cross_section_plane.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ansys/visor/viewer/app/trame/local_app.py b/src/ansys/visor/viewer/app/trame/local_app.py index ea637b35..f702b1ab 100644 --- a/src/ansys/visor/viewer/app/trame/local_app.py +++ b/src/ansys/visor/viewer/app/trame/local_app.py @@ -562,7 +562,7 @@ def sync_cross_section_plane(self, payload) -> None: re-serialises them in one critical section; nothing is pushed from here. """ - api = self._part_state_api("sync_cross_section_plane", payload) + api = self._mutation_api("sync_cross_section_plane", payload) if api is None: return api.sync_cross_section_plane(payload.origin, payload.normal) diff --git a/tests/unit/app/test_local_app_sync_cross_section_plane.py b/tests/unit/app/test_local_app_sync_cross_section_plane.py index 50ff8f66..9f9564ab 100644 --- a/tests/unit/app/test_local_app_sync_cross_section_plane.py +++ b/tests/unit/app/test_local_app_sync_cross_section_plane.py @@ -9,7 +9,7 @@ there would multiply the per-part tests by one more and rewrite them. Two tests, not six. The no-coordinator path and the not-a-mapping path run -through ``_part_state_api`` and ``parse_payload``, which three sibling trigger +through ``_mutation_api`` and ``parse_payload``, which three sibling trigger modules already pin against the same two functions; repeating them here would be a third and fourth copy of one failure mode rather than a new one. What is unique to this trigger is that it forwards *two* vectors in a fixed order, and