serialize framework's roots rather than render window alone

Author: LKasianAnsys

src/ansys/visor/viewer/renderer/local_renderer.py

@@ -324,19 +324,49 @@ def serialize_camera_state(self) -> None:
         content, and the client fetches the pre-write camera and applies it
         over the one just installed.
 
-        The render window's id is passed, not the camera's own.  It is the
-        form the framework itself reproduces -- ``LocalView.update`` resolves
-        ``[self._render_window, *registered]`` to ids and hands those to
-        ``UpdateStatesFromObjects`` -- and the camera sits inside the render
-        window's dependency closure, which is why ``get_status`` can name the
-        camera's id at all when building ``ignore_ids``.
+        The camera's own id is not passed; the render window's is, because
+        the camera sits inside the render window's dependency closure, which
+        is why ``get_status`` can name the camera's id at all when building
+        ``ignore_ids``.
+
+        **But the render window alone is not the root list the framework
+        uses.**  ``LocalView.update`` resolves ``[self._render_window,
+        *registered]`` -- the render window *plus* every object handed to
+        ``register_vtk_object`` -- and hands all of those ids to
+        ``UpdateStatesFromObjects``.  Here that is the interactor, the
+        picker, and the orientation / cross-section / bounding-box widget
+        objects.  They are registered separately precisely because they are
+        **not** reachable from the render window's dependency closure; that
+        is also why ``ObjectManagerAPI.get_status`` has to union
+        ``GetAllDependencies`` over ``_widgets[root_id]`` on top of the
+        render window's own closure before it can describe the scene to a
+        client.
+
+        Serialising from the render window alone therefore re-roots the
+        object manager's state store on a strictly narrower set and leaves
+        those widget subtrees unrefreshed.  A client that is already
+        connected does not notice -- it cached those states at its own first
+        load -- but a *freshly* connecting client builds its entire fetch
+        list from what ``get_status`` reports, so the ids it needs to resolve
+        (``WasmRendererAnnotation.widgets``) are simply absent and its scene
+        construction fails.  Passing the same root list ``LocalView.update``
+        does closes that hole.
+
+        ``api.get_all_ids(render_window_id)`` is the id-space spelling of
+        that list: ``[root_id, *self._widgets[root_id]]``, which is exactly
+        ``[self._render_window, *registered]`` resolved to ids.  It is the
+        same call ``LocalView.save`` uses, and it degrades to ``[root_id]``
+        if nothing was ever registered.
 
         No ``js_call``: that lives in ``LocalView.update``, not in the object
         manager, so this serialises without pushing and without re-opening
-        the rebuild race ``_apply_runtime_state_to_render`` refuses.
+        the rebuild race ``_apply_runtime_state_to_render`` refuses.  Widening
+        the root list changes *what* is serialised, never *whether* the
+        client is notified.
         """
+        render_window_id = self._object_manager.GetId(self._render_window)
         self._object_manager.UpdateStatesFromObjects(
-            [self._object_manager.GetId(self._render_window)]
+            self._local_view.api.get_all_ids(render_window_id)
         )
 
     # ------------------------------------------------------------------

tests/unit/renderer/test_local_renderer.py

@@ -191,13 +191,22 @@ def SetParallelScale(self, value): # noqa: N802
 # therefore fails if production names the renderer, the interactor, the picker
 # or the active camera, instead of coinciding with whatever it named.
 #
-# Both are far outside the small-integer range a real object manager hands out
-# in a freshly initialised scene, so a literal arriving from anywhere other
-# than here is visible on sight.
+# ``REGISTERED_WASM_IDS`` stands for the separately registered objects -- the
+# interactor, the picker and the widget objects -- that ``LocalView.update``
+# carries alongside the render window and that ``api.get_all_ids`` appends to
+# the root it is handed.  Kept as its own literal so that an implementation
+# passing the render window *alone* fails rather than coincides: that
+# narrowing is exactly what leaves a freshly connecting client without the
+# widget states it has to resolve.
+#
+# All three are far outside the small-integer range a real object manager
+# hands out in a freshly initialised scene, so a literal arriving from
+# anywhere other than here is visible on sight.
 # ---------------------------------------------------------------------------
 
 RENDER_WINDOW_WASM_ID = 8150001
 WRONG_OBJECT_WASM_ID = 8150999
+REGISTERED_WASM_IDS = [8150101, 8150102, 8150103]
 
 
 
@@ -224,6 +233,15 @@ def renderer():
     render_window = MagicMock(name="render_window")
     interactor = MagicMock(name="interactor")
     local_view = MagicMock(name="local_view")
+    # serialize_camera_state roots on api.get_all_ids(render_window_id).
+    # Stubbed as a faithful function of the root it is handed -- the real
+    # ObjectManagerAPI.get_all_ids returns [root_id, *widgets[root_id]] --
+    # so the identity keying of GetId still reaches the assertion: a
+    # production that rooted on the wrong object puts that object's id at
+    # the head of this list, not the render window's.
+    local_view.api.get_all_ids.side_effect = (
+        lambda root_id: [root_id, *REGISTERED_WASM_IDS]
+    )
     orientation_widget = MagicMock(name="orientation_widget")
     cross_section_widget = MagicMock(name="cross_section_widget")
     bounding_box_widget = MagicMock(name="bounding_box_widget")
@@ -741,13 +759,25 @@ def test_sync_camera_stores_the_object_without_copying(self, renderer):
     def test_serialize_camera_state_updates_states_from_the_render_window_id(
         self, renderer
     ):
-        """The re-serialise names the render window, and nothing else.
+        """The re-serialise names the render window and the registered objects.
+
+        That is the root list ``LocalView.update`` uses --
+        ``[self._render_window, *registered]`` -- of which
+        ``api.get_all_ids(root_id)`` is the id-space spelling,
+        ``[root_id, *widgets[root_id]]``.
 
         The id source is keyed on object identity, so every object other than
         the render window resolves to a different, equally distinctive
-        literal.  Asserting ``RENDER_WINDOW_WASM_ID`` therefore fails if the
-        implementation names ``_vtk_renderer``, the interactor, or the active
-        camera, rather than coinciding with them.  Asserting against
+        literal, and the fixture's ``get_all_ids`` stub is a faithful
+        function of the root it is handed.  Asserting
+        ``[RENDER_WINDOW_WASM_ID, *REGISTERED_WASM_IDS]`` therefore fails if
+        the implementation roots on ``_vtk_renderer``, the interactor, or the
+        active camera, rather than coinciding with them -- that object's id
+        arrives at the head of the list.  It fails equally if the
+        implementation drops the registered objects and passes the render
+        window alone: that narrowing re-roots the object manager's state
+        store on less than the scene, and starves a freshly connecting client
+        of the widget states it has to resolve.  Asserting against
         ``GetId(renderer._render_window)`` -- the attribute production reads
         -- would pass in all of those cases and pin nothing.
 
@@ -763,7 +793,7 @@ def test_serialize_camera_state_updates_states_from_the_render_window_id(
         renderer.serialize_camera_state()
 
         renderer._object_manager.UpdateStatesFromObjects.assert_called_with(
-            [RENDER_WINDOW_WASM_ID]
+            [RENDER_WINDOW_WASM_ID, *REGISTERED_WASM_IDS]
         )
 
     def test_serialize_camera_state_does_not_notify_the_client(self, renderer):

tests/unit/vtk/scene/test_base.py

@@ -162,12 +162,19 @@ def array_dataset() -> vtkPolyData:
 # rather than coincides -- if the re-serialisation names the renderer, the
 # interactor, the picker or the active camera instead of the render window.
 #
+# REGISTERED_WASM_IDS stands for the separately registered objects -- the
+# interactor, the picker and the widget objects -- that LocalView.update
+# carries alongside the render window and that api.get_all_ids appends to the
+# root it is handed.  Kept as its own literal so that a re-serialisation
+# passing the render window alone fails rather than coincides.
+#
 # Asserting against the id source's own answer for the attribute production
 # reads would pin nothing: it would pass whichever object production named.
 # ---------------------------------------------------------------------------
 
 RENDER_WINDOW_WASM_ID = 8150001
 WRONG_OBJECT_WASM_ID = 8150999
+REGISTERED_WASM_IDS = [8150101, 8150102, 8150103]
 
 
 @pytest.fixture
@@ -183,6 +190,12 @@ def renderer():
     identity, the render window resolves to RENDER_WINDOW_WASM_ID and every
     other object to WRONG_OBJECT_WASM_ID.  That is what lets the ordering test
     below assert which object the re-serialisation named.
+
+    ``api.get_all_ids`` is seeded alongside it, since the re-serialisation
+    roots on ``api.get_all_ids(render_window_id)``.  It is stubbed as a
+    faithful function of the root it is handed -- the real
+    ``ObjectManagerAPI.get_all_ids`` returns ``[root_id, *widgets[root_id]]``
+    -- so the identity keying above still reaches the assertion.
     """
     mock_server = MagicMock()
     mock_server.state = {}
@@ -209,6 +222,9 @@ def renderer():
         if obj is r._render_window
         else WRONG_OBJECT_WASM_ID
     )
+    r._local_view.api.get_all_ids.side_effect = (
+        lambda root_id: [root_id, *REGISTERED_WASM_IDS]
+    )
     return r
 
 
@@ -1134,7 +1150,9 @@ def test_apply_state_syncs_the_camera_under_the_lock_before_the_render_step(scen
     ``("serialize", <ids>)`` for the re-serialisation -- the tuple is the
     recording format, not the argument -- and ``"bridge"`` for the delegated
     render step.  ``<ids>`` is asserted as the list production passes, since
-    ``UpdateStatesFromObjects`` takes a sequence.
+    ``UpdateStatesFromObjects`` takes a sequence: the render window at the
+    head, keyed on identity, followed by the separately registered objects
+    that ``LocalView.update`` carries alongside it.
 
     Ordered between the two: after the write, because re-serialising before
     it would publish the pre-load camera; before the render step, because the
@@ -1161,7 +1179,7 @@ def _sync(camera_state):
 
     assert order == [
         "camera",
-        ("serialize", [RENDER_WINDOW_WASM_ID]),
+        ("serialize", [RENDER_WINDOW_WASM_ID, *REGISTERED_WASM_IDS]),
         "bridge",
     ]
     assert observed["depth"] >= 1
@@ -1202,8 +1220,8 @@ def test_apply_state_serializes_the_camera_under_the_lock(scene):
 # then ``self._renderer.serialize_camera_state()``, both inside
 # ``_vtk_lock``.  Same shape as the ``sync_camera`` trigger section above: the
 # re-serialisation runs after the reset, is spied on
-# ``_object_manager.UpdateStatesFromObjects`` with the render-window id, and
-# the lock is held (depth >= 1) at both points.
+# ``_object_manager.UpdateStatesFromObjects`` with the render-window id at the
+# head of the root list, and the lock is held (depth >= 1) at both points.
 #
 # Own literals are unnecessary here -- reset_camera takes no camera argument,
 # only the scene-graph bounds -- so what is pinned is order and lock depth,
@@ -1221,7 +1239,10 @@ def test_reset_camera_serializes_after_the_reset_with_the_render_window_id(scene
     The spy appends ``"reset"`` for the reset call and the two-tuple
     ``("serialize", <ids>)`` for the re-serialisation; the tuple is the
     recording format, not the argument.  ``<ids>`` is asserted as the list
-    production passes, since ``UpdateStatesFromObjects`` takes a sequence.
+    production passes, since ``UpdateStatesFromObjects`` takes a sequence:
+    the render window at the head, keyed on identity, followed by the
+    separately registered objects that ``LocalView.update`` carries
+    alongside it.
     """
     order = []
     real_reset = scene._renderer.reset_camera
@@ -1237,7 +1258,10 @@ def _reset(bounds):
 
     scene.reset_camera()
 
-    assert order == ["reset", ("serialize", [RENDER_WINDOW_WASM_ID])]
+    assert order == [
+        "reset",
+        ("serialize", [RENDER_WINDOW_WASM_ID, *REGISTERED_WASM_IDS]),
+    ]
 
 
 def test_reset_camera_holds_the_lock_across_both_halves(scene):