diff --git a/docs/design/scene_node_identity.md b/docs/design/scene_node_identity.md new file mode 100644 index 000000000..9b2e93f70 --- /dev/null +++ b/docs/design/scene_node_identity.md @@ -0,0 +1,180 @@ +# Scene node identity: per-name variant slots with client-wins display + +Status: **implemented** (same branch, after an interim rejection-based +design; the "where we are" section below describes the state this replaced). +Code map: owner/virtual fields in `_messages.py`; owner stamping via +`SceneApi._queue_scene_message` and virtual anchors via +`_ensure_ancestors_exist` in `_scene_api.py` / `_scene_handles.py`; variant +slots, display rule, and frozen-pose inheritance in the frontend's +`SceneTreeState.ts` with owner routing in `MessageHandler.tsx`. Tests: +`tests/test_scene_scopes.py`, `SceneTreeState.test.ts`, +`tests/e2e/test_cross_scope_handles.py`. Not yet implemented: a scene-tree +panel badge for shadowing/local variants (cosmetic follow-up). + +## Where this started + +Scene nodes were identified by name alone, everywhere: the frontend scene +tree was a single store keyed by node name with no record of which scope +created an entry, and every name-keyed message (updates, removes, clicks, +drags) resolved against that one namespace. Because `server.scene` +(broadcast) and each `client.scene` (per-client) both feed the same tree, a +name claimed by two scopes visible to the same viewer silently corrupted +state. + +An interim fix (a `SceneNameIndex`, since removed) kept name-only identity +and **rejected** overlapping-scope claims at the add site (`ValueError`), +with an audience-subset rule for cross-scope parenting and cross-scope +cascade on broadcast removals. That was sound, but it made the collision +class *forbidden* rather than *unrepresentable*, and it forced server and +client code to coordinate names. + +## The model + +Each scene-tree **name** becomes a slot holding up to two **variants**: a +broadcast variant and a client variant. Both variants keep independent state +(props, pose, visibility, interaction bindings), fed independently by their +scopes' messages. Exactly one variant is **effective** (rendered, +interactive) per name, chosen by a local display rule: + +> Pick the variant maximizing ``(is_real, is_client)``: +> real client > real broadcast > virtual client > virtual broadcast. + +"Virtual" marks auto-created intermediate ancestors (see below). The rule +gives client-over-server supersede semantics -- a client-scoped add of a name +the server owns *shadows* the broadcast node for that one viewer -- without +any of the machinery that made shadowing expensive under name-only identity: + +- **No per-client filtering of broadcast sends.** Updates to a shadowed + broadcast node land in the broadcast variant's state; they're simply not + displayed while shadowed. Nothing clobbers the client variant. +- **No resurrection machinery.** Removing the client variant un-hides the + broadcast variant, which has been accumulating state all along -- the + display rule is recomputed locally from data already in the store. +- **Deterministic removal in both directions.** Server removes its `/x`: + the broadcast variant leaves the slot; a client variant is unaffected. + Client removes its `/x`: the broadcast variant (if any) shows again. +- **Late joiners are trivially correct**: broadcast replay populates only + broadcast variants. + +**Hierarchy stays name-based.** One tree edge structure per name; children +attach to their parent *name*, not to a specific variant, and pose composes +through whichever variant is effective. This is what keeps the frontend +change small: no per-variant tree, no parent-edge resolution rules. + +### Virtual intermediates + +Ancestor auto-creation (`_ensure_ancestors_exist`) becomes **unconditional +per scope**: every add creates anchors for all missing *same-scope* +ancestors, even when another scope's variant of that name exists. These +anchors are flagged **virtual** -- a field on the create message -- and: + +- Virtual variants yield to real ones in the display rule, so a client + auto-ancestor for `/a` never shadows the server's real `/a` (its axes, its + pose visuals). A later explicit add of the same name from the same scope + supersedes the virtual variant with a real one (ordinary within-scope + supersede). +- Virtual variants render nothing and are never interactive; while a real + variant of the name exists, the anchor is pure lifecycle bookkeeping. + +Unconditional anchors give every node a complete same-scope ancestor chain, +which is what makes scope-local cascade (below) orphan-free: the two scopes +are two complete overlaid trees, merged per name by the display rule. The +cost is a handful of tiny anchor messages per deep add. + +Virtual intermediates also dissolve the audience-subset rule: a broadcast add +of `/a/b` where `/a` exists only in some client's scope auto-creates a +*virtual broadcast* `/a`. Other clients see the child under an invisible +anchor; the owning client's real `/a` shadows the anchor. No error needed in +either direction, so **both `ValueError`s from the name index are relaxed** +(non-breaking: code that raised starts working, with defined semantics). + +## Wire protocol + +`owner` is an **opaque string** stamped on scene-node messages, not a +boolean: today it takes two values ("broadcast" and a per-connection +identifier), but under the audience-set endgame (elements carry an audience; +`client.scene.add_*` becomes sugar) a client may see nodes from several +owners, and an opaque id avoids a second identity migration. + +**Per-message owner field, not per-batch origin tagging.** Batch tagging +(each of the two producer tasks stamping the windows it drains) is cheaper +today, but it identifies owners with *buffers* -- and the endgame is a single +persistent buffer whose per-client window generator filters messages by +audience (precedent: `excluded_self_client` is already filtered per-client in +`AsyncMessageBuffer.window_generator`). In that world one batch carries mixed +owners. Pay the schema sweep once: + +- Server→client: scene-node messages (`_CreateSceneNodeMessage` subclasses, + `SceneNodeUpdateMessage`, `Set{Orientation,Position,...}`, + `SetSceneNodeVisibilityMessage`, `RemoveSceneNodeMessage`, binding + messages) gain `owner: str`, stamped by the queueing `SceneApi`. Create + messages additionally gain the `virtual` flag. Changes go through + `_messages.py` + `sync_client_server.py --sync-messages`. +- Client→server: interaction messages (`SceneNodeClickMessage`, + `SceneNodeDragMessage`, transform-controls updates and drag start/end) + echo the effective variant's owner, so dispatch resolves to exactly one + scope's registry. Only the effective variant is interactive; a shadowed + broadcast node's bindings lie dormant until it is unshadowed. +- Entity identity for redundancy keys and GC becomes (owner, name) -- a + no-op while buffers are split per owner, load-bearing once merged. + +## Frontend + +- Store: per-name slot with `broadcast?` / `client?` variant entries; pose + data and bindings move into the variant. Effective-variant selection is + one pure function; only the effective variant is mounted (a shadow toggle + remounts, which is acceptable churn -- same cost as today's same-name + re-add). +- `nodeRefFromName` stays name-keyed (only the effective variant mounts). +- Scene-tree panel: one row per name (the effective variant), with a badge + for local/shadowing variants. +- `.viser` serialization: unchanged (recordings already filter to the + broadcast scope, which is collision-free on its own). + +## Python side + +- The `SceneNameIndex` keeps its bookkeeping roles (ancestor-existence + checks, disconnect cleanup) and loses both claim-time rejections -- and, + with scope-local cascade, its cross-scope cascade lookup. +- **Cascade is scope-local**: a server remove cascades through broadcast + descendants only; a client remove cascades through that client's + descendants only. Neither scope can destroy the other's state -- both + scopes are driven by the same Python program, so when coupled teardown is + wanted (a per-client annotation that should die with the mesh it + annotates), the author removes it explicitly rather than the design doing + it behind their back. Unconditional same-scope virtual anchors (above) + guarantee no orphans: the surviving scope's subtree keeps a complete + ancestor chain. This also deletes the cross-scope handle-invalidation + machinery from the current branch -- the zombie problem is solved from + the other side, by keeping the frontend node alive so handle and frontend + agree by construction. +- **Frozen-pose inheritance**: children compose pose through the parent + name's *effective* variant; virtual anchors contribute nothing while a + real variant exists. When the effective variant is removed and a virtual + anchor becomes effective, the anchor inherits the departing variant's + last pose (a frontend-local copy) -- surviving children stay where they + were instead of teleporting to identity. Accepted caveat: a client child + can outlive the broadcast object it annotated, frozen in place, until its + author removes it. +- `client.scene.add_frame("/WorldAxes")` becomes the sanctioned per-client + world-axes override (shadowing the server's node), replacing the removed + `client.scene.world_axes` handle. + +## Migration and sequencing + +1. (Done, current branch) Name-only identity + `SceneNameIndex` rejection. + Errors are forward-compatible: relaxing them later breaks nobody. +2. Merged single producer per connection (planned): one ordered stream, + cross-scope `atomic()`. Independent of this design but shares the + filtered-window machinery. +3. This design: owner + virtual fields, variant slots + display rule on the + frontend, both `ValueError`s relaxed, cascade rewired to scope-local + semantics (the interim cross-scope cascade + handle invalidation from + step 1 is deleted; handles that used to be invalidated stay valid, so the + change is again a relaxation). The branch's rejection tests flip to + coexistence/shadowing assertions. Client/server version gating already + forces matched deploys; no wire compatibility shims needed. +4. Audience sets: `audience=` on add, per-client filtering in the window + generator, audience mutation on live elements. Owner ids from step 3 are + the identity substrate; the display rule generalizes by ranking owner + specificity (more-specific audience wins). diff --git a/docs/source/conventions.rst b/docs/source/conventions.rst index 8dfb93f93..c77840562 100644 --- a/docs/source/conventions.rst +++ b/docs/source/conventions.rst @@ -75,6 +75,52 @@ In ``viser``, all camera parameters use the **COLMAP/OpenCV convention**: **Conversion**: A simple **180° rotation around the local X-axis** converts between the two conventions. +Server and Client Scopes +------------------------ + +Scene and GUI elements can be created through two kinds of handles: + +- ``server.scene`` / ``server.gui``: **shared** elements, visible to every + connected client and replayed to clients that connect later. +- ``client.scene`` / ``client.gui`` (via :class:`~viser.ClientHandle`): + **per-client** elements, visible to one client only. Client state is + ephemeral -- it disappears when the connection closes, and a reconnecting + browser is a new client -- so per-client state should be (re)built in + :meth:`~viser.ViserServer.on_client_connect`. + +Each scene-tree name can hold one node from each scope. When both exist, +the client-scoped node **shadows** the shared one for that client: it is +the one rendered and the one that receives clicks and drags, while other +clients keep seeing the shared node. Updates to a shadowed shared node keep +accumulating invisibly; removing the client-scoped node reveals the shared +node again with its latest state. This makes per-client overrides of shared +elements a one-liner:: + + # Everyone sees this... + server.scene.add_box("/box", color=(255, 0, 0)) + # ...except this client, who now sees their own version instead: + client.scene.add_box("/box", color=(0, 255, 0)) + +Removal is **scope-local**: removing a node (or a whole subtree) through +one scope's handle never touches the other scope's nodes, even per-client +children named under a shared parent -- those stay, anchored at the +parent's last pose, until their own scope removes them. In the scene-tree +panel, per-client nodes are marked with a ``local`` badge. + +GUI container nesting across scopes is **directional**: a ``client.gui`` +element may be added inside a ``server.gui`` container context (its +audience is a subset of the container's), rendering inside the shared +folder for that client only:: + + with server.gui.add_folder("Shared folder"): + client.gui.add_button("Only I see this") + +The reverse -- a ``server.gui`` element inside a ``client.gui`` container +-- raises, since no other client could see the container. Cross-nested +elements are the one exception to scope-local removal: removing the +server container also removes the client elements nested inside it (an +orphaned widget, unlike a scene node, has nowhere coherent to go). + ---- .. seealso:: @@ -82,5 +128,6 @@ In ``viser``, all camera parameters use the **COLMAP/OpenCV convention**: **Related Documentation** - :class:`~viser.ViserServer` for scene management + - :class:`~viser.ClientHandle` for per-client state - :func:`~viser.SceneApi.set_up_direction` for coordinate system configuration - :mod:`~viser.transforms` for transformation utilities diff --git a/examples/03_interaction/08_per_client_scenes.py b/examples/03_interaction/08_per_client_scenes.py new file mode 100644 index 000000000..973a8ad40 --- /dev/null +++ b/examples/03_interaction/08_per_client_scenes.py @@ -0,0 +1,77 @@ +"""Per-client scene state + +Mix shared (broadcast) scene elements with per-client ones, including +per-client overrides of shared nodes. + +**Features:** + +* :attr:`viser.ClientHandle.scene` for elements only one client sees +* Shadowing: a client-scoped node with a shared node's name replaces it for + that client only, and the shared node (with its latest state) returns when + the client-scoped variant is removed +* Scope-local removal: per-client annotations survive shared-node removal + until their own scope removes them +""" + +from __future__ import annotations + +import time + +import viser + +server = viser.ViserServer() + +# A shared box that every client sees, animated by the server. +shared_box = server.scene.add_box( + "/box", dimensions=(0.5, 0.5, 0.5), color=(200, 60, 60) +) + + +@server.on_client_connect +def _(client: viser.ClientHandle) -> None: + # Per-client GUI + scene state. Everything created through `client.` is + # visible to this client alone and is rebuilt here on reconnect (client + # state is ephemeral -- see the ClientHandle docs). + highlight = client.gui.add_checkbox("Highlight box (only me)", False) + annotate = client.gui.add_checkbox("Annotate box (only me)", False) + + highlight_handle: viser.BoxHandle | None = None + annotation_handle = None + + @highlight.on_update + def _(_) -> None: + nonlocal highlight_handle + if highlight.value: + # Same name as the shared box: this client-scoped variant + # SHADOWS the shared one for this client only. Other clients + # keep seeing the server's red box, and server updates keep + # accumulating in the hidden variant. + highlight_handle = client.scene.add_box( + "/box", dimensions=(0.55, 0.55, 0.55), color=(60, 200, 80) + ) + elif highlight_handle is not None: + # Un-shadow: the shared box reappears with its LATEST state. + highlight_handle.remove() + highlight_handle = None + + @annotate.on_update + def _(_) -> None: + nonlocal annotation_handle + if annotate.value: + # A per-client child under the shared node. If the server ever + # removes /box, this annotation survives (anchored where the box + # was) until this client removes it -- removal never reaches + # across scopes. + annotation_handle = client.scene.add_label( + "/box/note", text=f"client {client.client_id}'s note" + ) + elif annotation_handle is not None: + annotation_handle.remove() + annotation_handle = None + + +while True: + # Server-side animation of the shared box; shadowing clients don't see + # these updates until they un-shadow. + shared_box.position = (0.0, 0.0, 0.4 + 0.2 * (time.time() % 1.0)) + time.sleep(0.05) diff --git a/src/viser/_backwards_compat_shims.py b/src/viser/_backwards_compat_shims.py index 8547005f8..4d2542d19 100644 --- a/src/viser/_backwards_compat_shims.py +++ b/src/viser/_backwards_compat_shims.py @@ -103,6 +103,19 @@ class DeprecatedAttributeShim: `<=0.1.30`.""" def __getattr__(self, name: str) -> Any: + # During partial construction (or an attribute miss on a + # partially-torn-down object), `self.scene` / `self.gui` don't exist + # yet -- and looking them up would land back in THIS __getattr__, + # recursing until RecursionError. Bail out to a plain AttributeError + # instead, so init-ordering mistakes fail legibly. (Both attributes + # are plain instance attributes on ViserServer and ClientHandle, so + # __dict__ is the right place to check.) + if "scene" not in self.__dict__ or "gui" not in self.__dict__: + raise AttributeError( + f"'{type(self).__name__}' object has no attribute '{name}' " + "(object is partially constructed: scene/gui APIs not set up " + "yet)" + ) fixed_name = { # Map from old method names (viser v0.1.*) to new methods names. "reset_scene": "reset", diff --git a/src/viser/_gui_api.py b/src/viser/_gui_api.py index 3af75eac9..1301addc2 100644 --- a/src/viser/_gui_api.py +++ b/src/viser/_gui_api.py @@ -12,6 +12,7 @@ from asyncio import AbstractEventLoop from collections.abc import Mapping from concurrent.futures import ThreadPoolExecutor +from contextvars import ContextVar from pathlib import Path from typing import ( TYPE_CHECKING, @@ -76,6 +77,7 @@ _colors_to_int_tuple, _CommandHandleState, _GuiButtonHandleState, + _GuiHandle, _GuiHandleState, _GuiInputHandle, _make_uuid, @@ -195,6 +197,20 @@ class _RootGuiContainer: _children: dict[str, SupportsRemoveProtocol] +_context_owner_by_server: ContextVar[dict[int, GuiApi]] = ContextVar( + "viser_gui_context_owner_by_server", default={} +) +"""Which GuiApi owns the active (non-root) container context, keyed by +``id()`` of the owning server's GuiApi. A ContextVar rather than +thread-keyed state: asyncio callbacks interleave on one event-loop thread, +and a ``with`` block suspended at an ``await`` must not leak its container +context into unrelated callbacks -- each asyncio task runs in a copied +Context, so only code inside the block sees the marker. Sync code keeps +working like before (a thread has its own implicit Context). Values are +treated as IMMUTABLE -- every update installs a fresh dict -- because +copied Contexts share the mapping object itself.""" + + _global_order_counter = 0 @@ -236,9 +252,11 @@ class GuiApi: _target_container_from_thread_id: dict[int, str] """ID of container to put GUI elements into. Per-instance (NOT a shared - class attribute) -- otherwise a thread inside a ``with some_gui.add_folder()`` - block would leak that container target into a *different* GuiApi instance - (e.g. server.gui vs a client.gui) and raise KeyError on the foreign uuid.""" + class attribute). Cross-instance nesting is DIRECTIONAL, resolved by + _get_container_uuid's thread-context check: a ``client.gui`` add inside + a ``with server.gui.add_folder()`` block nests in the server container + (the element's audience is a subset of the container's), while the + reverse raises instead of silently landing at the other scope's root.""" def __init__( self, @@ -266,6 +284,12 @@ def __init__( "root": _RootGuiContainer({}) } self._modal_handle_from_uuid: dict[str, GuiModalHandle] = {} + # Elements of THIS scope that were nested inside another scope's + # container (client elements inside server containers -- the only + # allowed direction). They are parented in the server GuiApi's + # container tree, so this scope's reset() and disconnect teardown + # can't reach them through the root container walk; track them here. + self._handles_in_foreign_containers: dict[str, _GuiHandle[Any]] = {} self._panel_handle_from_uuid: dict[str, PanelHandle] = {} # Layout-update counter, bumped on every placement command (any panel) # and stamped onto the placement message (see @@ -648,12 +672,169 @@ async def _handle_command_trigger( ).add_done_callback(print_threadpool_errors) def _get_container_uuid(self) -> str: - """Get container ID associated with the current thread.""" + """Get container ID associated with the current thread. + + When a container context from a DIFFERENT GuiApi of the same server + is active in the current Context, nesting is directional: a + client-scope add inside a server-scope container targets the + server's container, while the reverse raises -- silently placing the + element at this scope's root (the historical behavior) hid the + mistake.""" + owner_api = self._context_owner() + if owner_api is not None and owner_api is not self: + from ._viser import ViserServer + + if isinstance(owner_api._owner, ViserServer) and not isinstance( + self._owner, ViserServer + ): + # Client element into a server container: allowed (the + # element's audience is a subset of the container's). The + # element nests under the SERVER scope's active container. + # This is the one deliberate exception to scope-local + # removal: the server container's teardown cascades into + # cross-nested client elements, because an orphaned widget + # -- unlike a scene node, which keeps its pose -- has + # nowhere coherent to go. + return owner_api._target_container_from_thread_id.get( + threading.get_ident(), "root" + ) + # Server (or other-client) element into a client container: + # every viewer outside that client scope couldn't see the + # container, so the element would dangle. Fail loudly. + raise RuntimeError( + "A GUI container context from a client scope is active on " + "this thread, and elements with a broader audience cannot " + "nest inside it (they would dangle for every other client). " + "Client elements may nest inside server containers, but not " + "vice versa." + ) return self._target_container_from_thread_id.get(threading.get_ident(), "root") + def _resolve_container_handle(self, container_uuid: str) -> GuiContainerProtocol: + """Resolve a container uuid to its handle, tolerating client + elements nested inside SERVER containers: the parent then lives in + the server GuiApi's registry rather than this one's.""" + handle = self._container_handle_from_uuid.get(container_uuid) + if handle is not None: + return handle + server_gui = self._root_server().gui + if server_gui is not self: + handle = server_gui._container_handle_from_uuid.get(container_uuid) + if handle is not None: + return handle + raise KeyError(container_uuid) + + def _release_cross_scope_nesting(self) -> None: + """Bookkeeping-only detach of this (client-scoped) GuiApi's elements + from the server containers they were nested in. Called on + disconnect: the connection's buffer is already closed, so no removal + messages are sent -- we just unhook the handles from the server's + container tree so a later server-side container removal doesn't + cascade a remove into a dead connection.""" + while self._handles_in_foreign_containers: + uuid, handle = self._handles_in_foreign_containers.popitem() + # Tombstone BEFORE detaching from the server parent: a user + # thread racing us with handle.remove() then bails at the + # already-removed check instead of finding a half-detached + # tree (its registry pops are tolerant of ours regardless). + self._tombstone_subtree(handle) + parent = self._root_server().gui._container_handle_from_uuid.get( + handle._impl.parent_container_id + ) + if parent is not None: + parent._children.pop(uuid, None) + + def _tombstone_subtree(self, handle: Any) -> None: + """Recursively mark a cross-nested subtree removed and purge it from + this scope's registries WITHOUT queuing messages (the connection is + closed). Descendants must be tombstoned too: a surviving user + reference calling ``.remove()`` on one should get the ordinary + already-removed warning, not a KeyError from a parent that no + longer resolves. Mirrors the per-type drains in the remove() + implementations.""" + from ._gui_handles import GuiTabHandle + + # Tabs keep their tombstone/uuid on the handle itself; everything + # else keeps them on `_impl`. + is_tab = isinstance(handle, GuiTabHandle) + state = handle if is_tab else handle._impl + if state.removed: + return + state.removed = True + uuid = handle._id if is_tab else handle._impl.uuid + self._gui_input_handle_from_uuid.pop(uuid, None) + self._container_handle_from_uuid.pop(uuid, None) + for child in ( + *getattr(handle, "_tab_handles", ()), + *tuple(getattr(handle, "_children", {}).values()), + ): + self._tombstone_subtree(child) + + def _root_server(self): + """The ViserServer this GuiApi ultimately belongs to (itself for the + broadcast scope, the owning server for a client scope).""" + from ._viser import ViserServer + + return ( + self._owner + if isinstance(self._owner, ViserServer) + else self._owner._viser_server + ) + + def _context_owner(self) -> GuiApi | None: + """The GuiApi owning the current Context's active (non-root) + container context on this API's server, if any.""" + return _context_owner_by_server.get().get(id(self._root_server().gui)) + + def _set_context_owner(self, owner: GuiApi | None) -> None: + """Install (or clear, with None) this server's context-owner marker + in the current Context. Copy-on-write: copied Contexts (sibling + asyncio tasks) share the mapping object, so it is never mutated.""" + key = id(self._root_server().gui) + current = _context_owner_by_server.get() + if owner is None: + if key in current: + updated = dict(current) + del updated[key] + _context_owner_by_server.set(updated) + else: + _context_owner_by_server.set({**current, key: owner}) + def _set_container_uuid(self, container_uuid: str) -> None: - """Set container ID associated with the current thread.""" - self._target_container_from_thread_id[threading.get_ident()] = container_uuid + """Set container ID associated with the current thread, tracking + which GuiApi currently owns an active (non-root) container context + so cross-scope nesting stays directional (see _get_container_uuid).""" + thread_id = threading.get_ident() + self._target_container_from_thread_id[thread_id] = container_uuid + if container_uuid == "root": + if self._context_owner() is self: + self._set_context_owner(None) + else: + self._set_context_owner(self) + + def _snapshot_container_context(self) -> tuple[GuiApi, str]: + """Snapshot the active container context for a `with` block to + restore on exit: the OWNING GuiApi and its current target uuid. + Carrying the owner explicitly -- instead of re-deriving it from a + bare uuid at exit time -- keeps the restore correct even when the + snapshot container has been removed while the block was open (a + dangling uuid restores into the scope that owned it, exactly like a + removed-while-open container always has within a single scope). + Raises for disallowed nesting directions, via _get_container_uuid.""" + container_uuid = self._get_container_uuid() + owner_api = self._context_owner() + return (owner_api if owner_api is not None else self, container_uuid) + + def _restore_container_context(self, snapshot: tuple[GuiApi, str]) -> None: + """Restore an `__enter__`-time snapshot on `__exit__`. When the + snapshot belongs to another scope (a client container context nested + inside a server one), our own thread target -- set for the block + that is now exiting -- is dropped so later adds can't resolve + against it once the owning scope's context also exits.""" + owner_api, container_uuid = snapshot + if owner_api is not self: + self._target_container_from_thread_id.pop(threading.get_ident(), None) + owner_api._set_container_uuid(container_uuid) def _next_layout_counter(self) -> int: """Bump and return the layout-update counter. THE single home of the @@ -672,6 +853,11 @@ def reset(self) -> None: root_container = self._container_handle_from_uuid["root"] while root_container._children: next(iter(root_container._children.values())).remove() + # This scope's elements nested inside ANOTHER scope's containers + # (client elements in server containers) aren't reachable from this + # root; drain them explicitly. + while self._handles_in_foreign_containers: + next(iter(self._handles_in_foreign_containers.values())).remove() while self._modal_handle_from_uuid: next(iter(self._modal_handle_from_uuid.values())).close() # Panels are top-level entities (not under `root`), so drain them @@ -1003,7 +1189,12 @@ def add_form( #
is well-formed. container = self._get_container_uuid() while container != "root": - parent = self._container_handle_from_uuid.get(container) + # Resolve across scopes: a client-scope add_form() inside a + # server-scope form is just as invalid as a same-scope nesting. + try: + parent = self._resolve_container_handle(container) + except KeyError: + break if isinstance(parent, GuiFormHandle): raise ValueError( "Nested forms are not supported: add_form() was called " diff --git a/src/viser/_gui_handles.py b/src/viser/_gui_handles.py index da243989c..9667b5a4f 100644 --- a/src/viser/_gui_handles.py +++ b/src/viser/_gui_handles.py @@ -103,6 +103,22 @@ class SupportsRemoveProtocol(Protocol): def remove(self) -> None: ... +def _cascade_remove(child: SupportsRemoveProtocol) -> None: + """Remove a child as part of a parent's removal cascade, silently + skipping children that a concurrent disconnect teardown already + tombstoned -- remove() would warn "already removed" for a purely + internal race the user did not cause. + + Best-effort: the unlocked check narrows the race window rather than + closing it (a teardown landing between this check and the removed-guard + inside remove() still warns, harmlessly). Closing it would need a lock + shared across all GUI removal paths, which the design avoids.""" + impl = getattr(child, "_impl", child) # Tabs carry `removed` directly. + if getattr(impl, "removed", False): + return + child.remove() + + class GuiPropsProtocol(Protocol): order: float @@ -149,13 +165,18 @@ class _GuiButtonHandleState(_GuiHandleState[bool]): class _GuiHandle(Generic[T], AssignablePropsBase[_GuiHandleState]): def __init__(self, impl: _GuiHandleState[T]) -> None: super().__init__(impl=impl) - parent = self._impl.gui_api._container_handle_from_uuid[ - self._impl.parent_container_id - ] + gui_api = self._impl.gui_api + parent = gui_api._resolve_container_handle(self._impl.parent_container_id) parent._children[self._impl.uuid] = self + if self._impl.parent_container_id not in gui_api._container_handle_from_uuid: + # Client element nested inside a server container: the parent + # lives in the server GuiApi's registry, so this scope's reset() + # and disconnect teardown can't find it through the root + # container walk. Track it for those paths. + gui_api._handles_in_foreign_containers[self._impl.uuid] = self if isinstance(self, _GuiInputHandle): - self._impl.gui_api._gui_input_handle_from_uuid[self._impl.uuid] = self + gui_api._gui_input_handle_from_uuid[self._impl.uuid] = self @override def _queue_update(self, name: str, value: Any) -> None: @@ -177,11 +198,18 @@ def remove(self) -> None: gui_api = self._impl.gui_api gui_api._websock_interface.queue_message(GuiRemoveMessage(self._impl.uuid)) - parent = gui_api._container_handle_from_uuid[self._impl.parent_container_id] - parent._children.pop(self._impl.uuid) + # Tolerant detach: a disconnect teardown racing this remove() may + # have already purged the parent and these registry entries + # (bookkeeping-only, no lock). + try: + parent = gui_api._resolve_container_handle(self._impl.parent_container_id) + parent._children.pop(self._impl.uuid, None) + except KeyError: + pass + gui_api._handles_in_foreign_containers.pop(self._impl.uuid, None) if isinstance(self, _GuiInputHandle): - gui_api._gui_input_handle_from_uuid.pop(self._impl.uuid) + gui_api._gui_input_handle_from_uuid.pop(self._impl.uuid, None) class _GuiInputHandle( @@ -796,9 +824,9 @@ def __init__(self, _impl: _GuiHandleState[None]) -> None: self._tab_handles: list[GuiTabHandle] = [] def __post_init__(self) -> None: - parent = self._impl.gui_api._container_handle_from_uuid[ + parent = self._impl.gui_api._resolve_container_handle( self._impl.parent_container_id - ] + ) parent._children[self._impl.uuid] = self def remove(self) -> None: @@ -825,9 +853,13 @@ def remove(self) -> None: # skips for a removed group (props_setattr would reject the write; the # client drops the whole entity via the remove message anyway). for tab in tuple(self._tab_handles): - tab.remove() - parent = gui_api._container_handle_from_uuid[self._impl.parent_container_id] - parent._children.pop(self._impl.uuid) + _cascade_remove(tab) + try: + parent = gui_api._resolve_container_handle(self._impl.parent_container_id) + parent._children.pop(self._impl.uuid, None) + except KeyError: + pass # Parent already torn down by a disconnect race. + gui_api._handles_in_foreign_containers.pop(self._impl.uuid, None) @dataclasses.dataclass @@ -838,7 +870,7 @@ class GuiTabHandle: _id: str # Used as container ID of children. _label: str _icon: IconName | None - _container_id_restore: str | None = None + _container_id_restore: tuple[GuiApi, str] | None = None _children: dict[str, SupportsRemoveProtocol] = dataclasses.field( default_factory=dict ) @@ -864,14 +896,18 @@ def __enter__(self) -> GuiTabHandle: "This GuiTabHandle is already active as a context; it cannot " "be re-entered inside itself." ) - self._container_id_restore = self._parent._impl.gui_api._get_container_uuid() + self._container_id_restore = ( + self._parent._impl.gui_api._snapshot_container_context() + ) self._parent._impl.gui_api._set_container_uuid(self._id) return self def __exit__(self, *args) -> None: del args assert self._container_id_restore is not None - self._parent._impl.gui_api._set_container_uuid(self._container_id_restore) + self._parent._impl.gui_api._restore_container_context( + self._container_id_restore + ) self._container_id_restore = None def __post_init__(self) -> None: @@ -899,8 +935,8 @@ def remove(self) -> None: self._parent._rebuild_tab_props() for child in tuple(self._children.values()): - child.remove() - self._parent._impl.gui_api._container_handle_from_uuid.pop(self._id) + _cascade_remove(child) + self._parent._impl.gui_api._container_handle_from_uuid.pop(self._id, None) # The control panel's fixed uuid, shared with the client (CONTROL_PANEL_ID in @@ -1312,7 +1348,7 @@ def remove(self) -> None: # message anyway). gui_api._panel_handle_from_uuid.pop(self._impl.uuid) for tab in tuple(self._tab_handles): - tab.remove() + _cascade_remove(tab) class MainPanelHandle(_PlacementMixin): @@ -1351,12 +1387,12 @@ def __init__(self, _impl: _GuiHandleState[None]) -> None: super().__init__(impl=_impl) self._impl.gui_api._container_handle_from_uuid[self._impl.uuid] = self self._children = {} - parent = self._impl.gui_api._container_handle_from_uuid[ + parent = self._impl.gui_api._resolve_container_handle( self._impl.parent_container_id - ] + ) parent._children[self._impl.uuid] = self - _container_id_restore: str | None = None + _container_id_restore: tuple[GuiApi, str] | None = None def __enter__(self) -> Self: if self._container_id_restore is not None: @@ -1368,14 +1404,14 @@ def __enter__(self) -> Self: "This GuiFolderHandle is already active as a context; it " "cannot be re-entered inside itself." ) - self._container_id_restore = self._impl.gui_api._get_container_uuid() + self._container_id_restore = self._impl.gui_api._snapshot_container_context() self._impl.gui_api._set_container_uuid(self._impl.uuid) return self def __exit__(self, *args) -> None: del args assert self._container_id_restore is not None - self._impl.gui_api._set_container_uuid(self._container_id_restore) + self._impl.gui_api._restore_container_context(self._container_id_restore) self._container_id_restore = None def remove(self) -> None: @@ -1394,10 +1430,14 @@ def remove(self) -> None: gui_api = self._impl.gui_api gui_api._websock_interface.queue_message(GuiRemoveMessage(self._impl.uuid)) for child in tuple(self._children.values()): - child.remove() - parent = gui_api._container_handle_from_uuid[self._impl.parent_container_id] - parent._children.pop(self._impl.uuid) - gui_api._container_handle_from_uuid.pop(self._impl.uuid) + _cascade_remove(child) + try: + parent = gui_api._resolve_container_handle(self._impl.parent_container_id) + parent._children.pop(self._impl.uuid, None) + except KeyError: + pass # Parent already torn down by a disconnect race. + gui_api._container_handle_from_uuid.pop(self._impl.uuid, None) + gui_api._handles_in_foreign_containers.pop(self._impl.uuid, None) class GuiFormHandle(GuiFolderHandle): @@ -1502,21 +1542,21 @@ class GuiModalHandle: _gui_api: GuiApi _uuid: str # Used as container ID of children. - _container_uuid_restore: str | None = None + _container_uuid_restore: tuple[GuiApi, str] | None = None _children: dict[str, SupportsRemoveProtocol] = dataclasses.field( default_factory=dict ) closed: bool = False def __enter__(self) -> GuiModalHandle: - self._container_uuid_restore = self._gui_api._get_container_uuid() + self._container_uuid_restore = self._gui_api._snapshot_container_context() self._gui_api._set_container_uuid(self._uuid) return self def __exit__(self, *args) -> None: del args assert self._container_uuid_restore is not None - self._gui_api._set_container_uuid(self._container_uuid_restore) + self._gui_api._restore_container_context(self._container_uuid_restore) self._container_uuid_restore = None def __post_init__(self) -> None: @@ -1536,7 +1576,7 @@ def close(self) -> None: GuiCloseModalMessage(self._uuid), ) for child in tuple(self._children.values()): - child.remove() + _cascade_remove(child) self._gui_api._container_handle_from_uuid.pop(self._uuid) self._gui_api._modal_handle_from_uuid.pop(self._uuid) diff --git a/src/viser/_messages.py b/src/viser/_messages.py index f112a5138..bbf5e06c6 100644 --- a/src/viser/_messages.py +++ b/src/viser/_messages.py @@ -306,6 +306,19 @@ class _CreateSceneNodeMessage( include_in_scene_serialization=True, ): name: str + owner: str = dataclasses.field(default="", init=False) + """Scope that owns this node: "" for the broadcast scope + (``server.scene``), otherwise an opaque per-client identifier. Each + scene-tree name holds at most one variant per owner; the client renders + the effective variant chosen by the display rule (real client > real + broadcast > virtual client > virtual broadcast). ``init=False`` so + defaulted fields don't precede subclasses' non-default props on + Python < 3.10; stamped by the queueing SceneApi.""" + virtual: bool = dataclasses.field(default=False, init=False) + """True for auto-created intermediate ancestor frames. Virtual variants + yield to real ones in the display rule and exist so every node has a + complete same-scope ancestor chain (which is what makes scope-local + cascade removal orphan-free).""" @dataclasses.dataclass @@ -314,9 +327,13 @@ class RemoveSceneNodeMessage( entity=EntityLifecycle("scene", "remove", "name"), include_in_scene_serialization=True, ): - """Remove a particular node from the scene.""" + """Remove a particular node's variant, for the scope stamped in + ``owner``, from the scene. Removal is scope-local: it never touches the + other scope's variant of the same name (the server enumerates one such + message per same-scope descendant; the client does not cascade).""" name: str + owner: str = dataclasses.field(default="", init=False) @dataclasses.dataclass @@ -453,13 +470,20 @@ class ScenePointerEnableMessage(Message, include_in_scene_serialization=False): """Set the modifier-filter set for a scene pointer ``event_type``. An empty ``modifiers`` tuple disables all callbacks for that - ``event_type``. A non-empty tuple enables them, and the client uses - the filter list to gate gesture engagement: a pointerdown whose - held-modifier state doesn't match any filter is treated as if no - callback were registered (no rectangle drawn, no message sent).""" + ``event_type`` in the sending scope. A non-empty tuple enables them, + and the client uses the filter list to gate gesture engagement: a + pointerdown whose held-modifier state doesn't match any filter is + treated as if no callback were registered (no rectangle drawn, no + message sent). + + Filters are kept per ``owner`` on the client and engagement uses the + union across owners, so the broadcast scope and a client scope can + register pointer callbacks independently -- one scope clearing its + filters never deactivates the other's.""" event_type: ScenePointerEventType modifiers: Tuple[Optional[KeyModifier], ...] + owner: str = dataclasses.field(default="", init=False) @override def redundancy_key(self) -> str: @@ -1178,6 +1202,7 @@ class SetBoneOrientationMessage( name: str bone_index: int wxyz: Tuple[float, float, float, float] + owner: str = dataclasses.field(default="", init=False) @override def redundancy_key(self) -> str: @@ -1197,6 +1222,7 @@ class SetBonePositionMessage( name: str bone_index: int position: Tuple[float, float, float] + owner: str = dataclasses.field(default="", init=False) @override def redundancy_key(self) -> str: @@ -1332,6 +1358,7 @@ class SetOrientationMessage( name: str wxyz: Tuple[float, float, float, float] + owner: str = dataclasses.field(default="", init=False) @dataclasses.dataclass @@ -1346,6 +1373,7 @@ class SetPositionMessage( name: str position: Tuple[float, float, float] + owner: str = dataclasses.field(default="", init=False) @dataclasses.dataclass @@ -1357,6 +1385,10 @@ class TransformControlsUpdateMessage(Message, include_in_scene_serialization=Fal name: str wxyz: Tuple[float, float, float, float] position: Tuple[float, float, float] + owner: str = "" + """Echo of the effective variant's owner, so the server dispatches to + exactly one scope's registry (regular init field: this message is + deserialized).""" @dataclasses.dataclass @@ -1364,6 +1396,7 @@ class TransformControlsDragStartMessage(Message, include_in_scene_serialization= """Client -> server message when a transform control drag starts.""" name: str + owner: str = "" @dataclasses.dataclass @@ -1371,6 +1404,7 @@ class TransformControlsDragEndMessage(Message, include_in_scene_serialization=Fa """Client -> server message when a transform control drag ends.""" name: str + owner: str = "" @dataclasses.dataclass @@ -1420,6 +1454,7 @@ class SetSceneNodeVisibilityMessage( name: str visible: bool + owner: str = dataclasses.field(default="", init=False) @dataclasses.dataclass(frozen=True) @@ -1449,6 +1484,7 @@ class SetSceneNodeDragBindingsMessage(Message, include_in_scene_serialization=Fa name: str bindings: Tuple[DragBinding, ...] + owner: str = dataclasses.field(default="", init=False) @dataclasses.dataclass @@ -1466,6 +1502,7 @@ class SetSceneNodeClickBindingsMessage(Message, include_in_scene_serialization=F name: str bindings: Tuple[DragBinding, ...] + owner: str = dataclasses.field(default="", init=False) @dataclasses.dataclass @@ -1479,6 +1516,9 @@ class SceneNodeClickMessage(Message, include_in_scene_serialization=False): ray_direction: Tuple[float, float, float] screen_pos: Tuple[float, float] modifier: Optional[KeyModifier] + owner: str = "" + """Echo of the clicked variant's owner, so the server dispatches to + exactly one scope's registry.""" _DragPhase: TypeAlias = Literal["start", "update", "end"] @@ -1509,6 +1549,9 @@ class SceneNodeDragMessage(Message, include_in_scene_serialization=False): """Current pointer in OpenCV screen-space coordinates.""" button: Literal["left", "middle", "right"] modifier: Optional[KeyModifier] + owner: str = "" + """Echo of the dragged variant's owner, so the server dispatches to + exactly one scope's registry.""" @dataclasses.dataclass @@ -2196,6 +2239,10 @@ class SceneNodeUpdateMessage( name: str updates: Dict[str, Any] """Mapping from property name to new value.""" + owner: str = "" + """Owning scope of the targeted variant. A regular init field (unlike + the other server->client owner stamps) so the message stays + deserializable in both directions.""" @dataclasses.dataclass diff --git a/src/viser/_scene_api.py b/src/viser/_scene_api.py index e5fc985c7..d697383b2 100644 --- a/src/viser/_scene_api.py +++ b/src/viser/_scene_api.py @@ -4,7 +4,6 @@ import dataclasses import io import math -import threading import time import warnings from collections.abc import Coroutine @@ -276,18 +275,40 @@ def __init__( str, TransformControlsHandle ] = {} self._handle_from_node_name: dict[str, SceneNodeHandle] = {} - self._node_lifecycle_lock = threading.RLock() + self._creating_virtual_anchors = False + """True only while _ensure_ancestors_exist creates intermediate + frames; _make stamps `virtual=True` on create messages queued while + set. An api-level flag (rather than a parameter) keeps the marker + out of the public add_frame signature; the lifecycle lock makes it + race-free.""" + if isinstance(owner, ViserServer): + self._owner_id = "" + """Opaque owner id stamped on every outgoing scene message: "" + for the broadcast scope, otherwise a per-client identifier. Each + scene-tree name holds at most one variant per owner on the + frontend; the effective (rendered, interactive) variant follows + the display rule: real client > real broadcast > virtual client + > virtual broadcast. Scene state is fully scope-local -- adds, + updates, and removals from one scope never touch the other + scope's variant of the same name.""" + server_owner = owner + else: + self._owner_id = str(owner.client_id) + server_owner = owner._viser_server + self._node_lifecycle_lock = server_owner._scene_lifecycle_lock """Serializes scene-node lifecycle transitions (remove, same-name supersede) against interaction-callback (de)registration. All critical sections are short and synchronous (no awaits inside). - Reentrant defensively: no current teardown path re-enters (a 3D GUI - container's _on_remove removes GUI children only), but subclass - _on_remove hooks run under the lock and must stay safe to extend. - Without this lock, a registration racing a remove/supersede from - another thread could publish a name-keyed binding into the - persistent buffer AFTER the teardown's empty-bindings emit -- a - ghost a same-name successor would inherit on late-joining - clients.""" + Shared server-wide by every SceneApi: scene lifecycle is scope-local, + so per-scope locks would also be correct, but one lock keeps the + invariants easy to reason about and costs nothing (lifecycle ops are + rare and short). Reentrant: ancestor auto-creation re-enters by + design, and subclass _on_remove hooks run under the lock and must + stay safe to extend. Without this lock, a registration racing a + remove/supersede from another thread could publish a name-keyed + binding into the persistent buffer AFTER the teardown's + empty-bindings emit -- a ghost a same-name successor would inherit + on late-joining clients.""" self._children_from_node_name: dict[str, set[str]] = {} # Tracks handles with an in-flight drag gesture, plus the last # message we processed for that drag. Populated on @@ -322,39 +343,111 @@ def __init__( self._scene_pointer_cb: list[_PointerCallbackEntry] = [] self._scene_pointer_done_cb: list[Callable[[], None | Coroutine]] = [] - # Set up world axes handle. - self.world_axes: FrameHandle = self.add_frame( - "/WorldAxes", - axes_radius=0.0125, - ) - """Handle for the world axes, which are created by default.""" - - self.world_axes.visible = False - - self._websock_interface.register_handler( + # Set up world axes handle. Only the SERVER scope creates one by + # default (each ClientHandle used to re-add /WorldAxes over its own + # connection, racing the broadcast replay). Client-scoped SceneApis + # expose no world_axes handle -- see the property below; a client + # that wants different axes adds its own "/WorldAxes" frame, which + # shadows the server's variant for that one client. + self._world_axes: FrameHandle | None = None + if self._owner_id == "": + self._world_axes = self.add_frame( + "/WorldAxes", + axes_radius=0.0125, + ) + self._world_axes.visible = False + + # Node-keyed interaction messages echo the effective variant's owner, + # and every incoming message fans out to BOTH the server's and the + # connection's handler lists -- so these handlers are registered + # through the owner-scoping wrapper, which makes exactly one scope's + # SceneApi act on each message. Registering one of these directly + # would not fail; it would silently double-dispatch callbacks in + # both scopes. + self._register_owner_scoped_handler( _messages.TransformControlsUpdateMessage, self._handle_transform_controls_updates, ) - self._websock_interface.register_handler( + self._register_owner_scoped_handler( _messages.TransformControlsDragStartMessage, self._handle_transform_controls_drag_start, ) - self._websock_interface.register_handler( + self._register_owner_scoped_handler( _messages.TransformControlsDragEndMessage, self._handle_transform_controls_drag_end, ) - self._websock_interface.register_handler( + self._register_owner_scoped_handler( _messages.SceneNodeClickMessage, self._handle_node_click_updates, ) - self._websock_interface.register_handler( + self._register_owner_scoped_handler( _messages.SceneNodeDragMessage, self._handle_node_drag ) + # Deliberately NOT owner-scoped: scene pointer events are scene-level + # (no target node). The client engages a gesture when the held + # modifiers match the UNION of both scopes' filters and sends ONE + # message; every scope's handler then dispatches its own matching + # registrations -- coexistence, with per-owner filter state on the + # client (ScenePointerEnableMessage.owner) so one scope's disable + # never deactivates the other's callbacks. self._websock_interface.register_handler( _messages.ScenePointerMessage, self._handle_scene_pointer_updates, ) + @property + def world_axes(self) -> FrameHandle: + """Handle for the world axes, which are created by default. Hidden + until made visible via ``server.scene.world_axes.visible = True``. + + Only available on the server's scene API; accessing this on a client + handle's ``client.scene`` raises ``AttributeError``. To show + different axes for one client, add a client-scoped frame named + ``"/WorldAxes"`` -- the client's variant shadows the server's for + that one viewer.""" + if self._world_axes is None: + raise AttributeError( + "world_axes is only available on the server's scene API " + "(server.scene.world_axes). To show or hide the shared axes " + "for every client, assign server.scene.world_axes.visible; " + "to override them for one client, add a client-scoped frame " + 'named "/WorldAxes" (it shadows the server\'s node for that ' + "client)." + ) + return self._world_axes + + def _queue_scene_message(self, message: _messages.Message) -> None: + """Queue a name-keyed scene message, stamped with this scope's owner + id. Every scene message that targets a node by name MUST go through + here (or stamp ``owner`` itself): an unstamped message defaults to + the broadcast owner and would be routed to the wrong variant on the + client.""" + # A message class without a declared `owner` field would accept the + # assignment below but silently DROP it at serialization (only + # declared fields go over the wire) -- the client would then route + # the message to the wrong variant. Catch that at the first test + # that exercises the new message instead. + assert hasattr(message, "owner"), ( + f"{type(message).__name__} is queued as a scene message but " + "declares no `owner` field." + ) + message.owner = self._owner_id # type: ignore[attr-defined] + self._websock_interface.queue_message(message) + + def _register_owner_scoped_handler(self, message_cls, handler) -> None: + """Register an incoming-message handler that only fires when the + message's echoed ``owner`` matches this scope. This is the dispatch + rule that makes the fan-out registration model safe: node-keyed + interaction messages reach both the server's and the connection's + handler lists, and exactly one scope may act on each.""" + + async def owner_scoped(client_id: ClientId, message) -> None: + if message.owner != self._owner_id: + return + await handler(client_id, message) + + self._websock_interface.register_handler(message_cls, owner_scoped) + def _is_drag_active_for(self, name: str) -> bool: """Whether the named scene node currently has any in-flight drag gesture (from any connected client). Used by ``remove()`` to @@ -416,12 +509,43 @@ async def _drop_active_drags_for_client( print_awaited_callback_error(exc) def _ensure_ancestors_exist(self, name: str) -> None: - """Create intermediate frame nodes for any missing ancestors of `name`.""" + """Create VIRTUAL intermediate frames for any ancestors of ``name`` + missing from THIS scope's registry. + + Unconditional per scope: an anchor is created even when another + scope has a (real) variant of the ancestor name. Virtual variants + yield to real ones in the client's display rule, so the anchor never + shadows anything -- it exists so every node has a complete + same-scope ancestor chain, which is what makes scope-local cascade + removal orphan-free (a client child survives a broadcast parent's + removal by hanging from its own scope's anchor, which inherits the + departing variant's pose client-side). + + Caller (``SceneNodeHandle._make``) holds the lifecycle lock, so the + existence checks and creates are atomic against concurrent + adds/removes.""" + # Fast path: a registered parent implies a complete ancestor chain + # (every add ensures its own chain; cascade removes whole same-scope + # subtrees), so per-add cost is one rsplit + dict lookup. + parent = name.rsplit("/", 1)[0] + if parent == "" or parent in self._handle_from_node_name: + return parts = name.split("/") - for i in range(2, len(parts)): # skip root ("") and the node itself - ancestor = "/".join(parts[:i]) - if ancestor not in self._handle_from_node_name: - self.add_frame(ancestor, show_axes=False) + # The anchors are ordinary add_frame() calls; the flag below makes + # _make stamp their create messages virtual, keeping the marker out + # of add_frame's public signature. Safe without save/restore + # subtleties: the lifecycle lock serializes adds, and the nested + # _ensure_ancestors_exist calls that add_frame triggers all hit the + # fast path above (ancestors are created parent-first). + self._creating_virtual_anchors = True + try: + for i in range(2, len(parts)): # skip root ("") and the node itself + ancestor = "/".join(parts[:i]) + if ancestor not in self._handle_from_node_name: + # Recurses into _make under the reentrant lifecycle lock. + self.add_frame(ancestor, show_axes=False) + finally: + self._creating_virtual_anchors = False def set_up_direction( self, @@ -491,8 +615,10 @@ def rotate_between(before: np.ndarray, after: np.ndarray) -> tf.SO3: ) if not np.any(np.isnan(R_threeworld_world.wxyz)): - # Set the orientation of the root node. - self._websock_interface.queue_message( + # Set the orientation of the root node. The root ("") is a + # singleton on the client -- name-empty messages apply to it + # regardless of the stamped owner. + self._queue_scene_message( _messages.SetOrientationMessage( "", cast_vector(R_threeworld_world.wxyz, 4) ) @@ -509,9 +635,7 @@ def set_global_visibility(self, visible: bool) -> None: Args: visible: Whether or not all scene nodes should be visible. """ - self._websock_interface.queue_message( - _messages.SetSceneNodeVisibilityMessage("", visible) - ) + self._queue_scene_message(_messages.SetSceneNodeVisibilityMessage("", visible)) @deprecated_positional_shim def add_light_directional( @@ -2942,14 +3066,14 @@ def sync_cb(client_id: ClientId, state: TransformControlsHandle) -> None: wxyz=tuple(map(float, state._impl.wxyz)), # type: ignore ) message_orientation.excluded_self_client = client_id - self._websock_interface.queue_message(message_orientation) + self._queue_scene_message(message_orientation) message_position = _messages.SetPositionMessage( name=name, position=tuple(map(float, state._impl.position)), # type: ignore ) message_position.excluded_self_client = client_id - self._websock_interface.queue_message(message_position) + self._queue_scene_message(message_position) node_handle = SceneNodeHandle._make( self, message, name, wxyz, position, visible @@ -2979,7 +3103,10 @@ def reset(self) -> None: # Remove all scene nodes. handles = list(self._handle_from_node_name.values()) for handle in handles: - if handle.name == "/WorldAxes": + # The broadcast scope keeps its default world-axes handle; a + # client-scoped "/WorldAxes" is an ordinary per-client override + # and resets away like everything else. + if handle.name == "/WorldAxes" and self._owner_id == "": continue # Skip handles already removed by cascading. if handle._impl.removed: @@ -2989,28 +3116,28 @@ def reset(self) -> None: # Clear the background image. self.set_background_image(image=None) - def _get_client_handle(self, client_id: ClientId) -> ClientHandle: - """Private helper for getting a client handle from its ID.""" + def _get_client_handle(self, client_id: ClientId) -> ClientHandle | None: + """Resolve the ClientHandle for a given client_id. Returns ``None`` + when the client disconnected between queueing and dispatch -- + callers early-return, dropping the event. Mirrors + ``GuiApi._resolve_client`` so the two APIs treat the same race the + same way.""" # Avoid circular imports. from ._viser import ViserServer - # Implementation-wise, note that MessageApi is never directly instantiated. - # Instead, it serves as a mixin/base class for either ViserServer, which - # maintains a registry of connected clients, or ClientHandle, which should - # only ever be dealing with its own client_id. if isinstance(self._owner, ViserServer): - handle = self._owner._connected_clients.get(client_id) - if handle is None: - raise KeyError(f"No connected client with id {client_id}.") - return handle - else: - assert client_id == self._owner.client_id - return self._owner + return self._owner._connected_clients.get(client_id) + assert client_id == self._owner.client_id + return self._owner async def _handle_transform_controls_updates( self, client_id: ClientId, message: _messages.TransformControlsUpdateMessage ) -> None: - """Apply pose update and fire `update_cb` with phase="update".""" + """Apply pose update and fire `update_cb` with phase="update". + + Registered via _register_owner_scoped_handler, like every node-keyed + handler below: only the scope whose owner the message echoes runs it. + """ # Prefer the active-drag map so a late update still resolves after the # gizmo was removed mid-drag (which pops it from the live registry). handle = self._active_transform_drag_handles.get( @@ -3059,6 +3186,10 @@ async def _fire_transform_controls_callbacks( phase: DragPhase, event_client: ClientHandle | None = None, ) -> None: + # Unlike the click/drag/pointer events (whose `client` field is + # non-Optional, so an unresolvable client drops the event), + # TransformControlsEvent.client is Optional by contract: gizmo + # lifecycle callbacks still fire when the client can't be resolved. event = TransformControlsEvent( client=event_client if event_client is not None @@ -3097,8 +3228,12 @@ async def _handle_node_click_updates( handle = self._handle_from_node_name.get(message.name, None) if handle is None or handle._impl.click_cb is None: return + client = self._get_client_handle(client_id) + if client is None: + # Client disconnected between queueing and dispatch; drop. + return event = SceneNodePointerEvent( - client=self._get_client_handle(client_id), + client=client, client_id=client_id, event="click", target=cast(_RaycastSupportedSceneNodeHandle, handle), @@ -3177,10 +3312,16 @@ async def _dispatch_drag_callbacks( if not matching: return - event = SceneNodeDragEvent( - client=event_client + client = ( + event_client if event_client is not None - else self._get_client_handle(client_id), + else self._get_client_handle(client_id) + ) + if client is None: + # Client disconnected between queueing and dispatch; drop. + return + event = SceneNodeDragEvent( + client=client, client_id=client_id, target=cast(_RaycastSupportedSceneNodeHandle, handle), phase=message.phase, @@ -3203,6 +3344,9 @@ async def _handle_scene_pointer_updates( if not self._scene_pointer_cb: return client = self._get_client_handle(client_id) + if client is None: + # Client disconnected between queueing and dispatch; drop. + return modifier = message.modifier # Build the typed event once for the actual gesture; the legacy @@ -3345,19 +3489,7 @@ def _register_scene_pointer_callback( ) -> Any: normalized_modifier = _messages._normalize_key_modifier(modifier) - from ._viser import ClientHandle, ViserServer - def decorator(func: Callable[[Any], None]) -> Callable[[Any], None]: - # Server-scope and client-scope share the same client-side - # enable toggle. Coexistence would let one scope's - # disable silently deactivate the other's callbacks; - # enforce exclusivity instead. - if isinstance(self._owner, ViserServer): - for client in self._owner.get_clients().values(): - client.scene._remove_all_pointer_callbacks() - elif isinstance(self._owner, ClientHandle): - self._owner._viser_server.scene._remove_all_pointer_callbacks() - self._scene_pointer_cb.append( _PointerCallbackEntry( callback=func, @@ -3388,7 +3520,7 @@ def _sync_scene_pointer_filters( } ), ) - self._websock_interface.queue_message( + self._queue_scene_message( _messages.ScenePointerEnableMessage( event_type=event_type, modifiers=modifiers ) diff --git a/src/viser/_scene_handles.py b/src/viser/_scene_handles.py index 2c79065dc..e017883a8 100644 --- a/src/viser/_scene_handles.py +++ b/src/viser/_scene_handles.py @@ -29,7 +29,6 @@ from ._assignable_props_api import AssignablePropsBase from .infra._infra import ( WebsockClientConnection, - WebsockMessageHandler, WebsockServer, ) @@ -47,15 +46,17 @@ def _set_pose_vector( current: np.ndarray, value: _PoseTupleT | np.ndarray, length: int, - websock: WebsockMessageHandler, + queue: Callable[[_messages.Message], None], make_message: Callable[[_PoseTupleT], _messages.Message], ) -> None: """Shared write path for the scene-node and skinned-bone pose setters. Casts and validates ``value``, no-ops if it is numerically unchanged from ``current``, and otherwise writes it into ``current`` in place and queues the - message built from the cast value. Keeping this in one place stops the four - near-identical wxyz/position setters from drifting apart. + message built from the cast value (via ``queue``, typically the owning + SceneApi's owner-stamping ``_queue_scene_message``). Keeping this in one + place stops the four near-identical wxyz/position setters from drifting + apart. """ from ._scene_api import cast_vector @@ -64,7 +65,7 @@ def _set_pose_vector( if np.allclose(value_arr, current): return current[:] = value_arr - websock.queue_message(make_message(value_cast)) + queue(make_message(value_cast)) def _queue_empty_interaction_bindings( @@ -77,13 +78,9 @@ def _queue_empty_interaction_bindings( ``_make`` so the two can't drift; emits only -- callers own any ``drag_cb`` bookkeeping.""" if had_click: - api._websock_interface.queue_message( - _messages.SetSceneNodeClickBindingsMessage(name, ()) - ) + api._queue_scene_message(_messages.SetSceneNodeClickBindingsMessage(name, ())) if had_drag: - api._websock_interface.queue_message( - _messages.SetSceneNodeDragBindingsMessage(name, ()) - ) + api._queue_scene_message(_messages.SetSceneNodeDragBindingsMessage(name, ())) @dataclasses.dataclass(frozen=True) @@ -250,7 +247,7 @@ class SceneNodeHandle(AssignablePropsBase[_SceneNodeHandleState]): @override def _queue_update(self, name: str, value: Any) -> None: - self._impl.api._websock_interface.queue_message( + self._impl.api._queue_scene_message( _messages.SceneNodeUpdateMessage(self._impl.name, {name: value}) ) @@ -275,9 +272,6 @@ def _make( name = _normalize_node_name(name) message.name = name - # Ensure all ancestor nodes exist (creates intermediate frames as needed). - api._ensure_ancestors_exist(name) - # Snapshot array props before the message is queued and persisted for # replay. The add_* methods use np.asarray casts that may alias the # caller's array; without a copy here, a caller mutating that array @@ -296,6 +290,13 @@ def _make( # is marked removed but still registered, where its remove() would # tear down the replacement's fresh state. with api._node_lifecycle_lock: + # Ensure all SAME-SCOPE ancestors exist (creates virtual anchor + # frames as needed; re-enters _make under the reentrant lifecycle + # lock). Scene state is scope-local: another scope's variant of + # an ancestor name neither satisfies nor blocks this scope's + # chain. + api._ensure_ancestors_exist(name) + old_handle = api._handle_from_node_name.get(name) if old_handle is not None and not old_handle._impl.removed: # 1. The old Python handle goes inert. Removal resolves by NAME, @@ -328,9 +329,13 @@ def _make( had_drag=bool(old_handle._impl.drag_cb), ) - # Send message. + # Send message, stamped with this scope's owner id -- and marked + # virtual when this create is an auto-generated ancestor anchor + # (see SceneApi._creating_virtual_anchors). assert isinstance(message, _messages.Message) - api._websock_interface.queue_message(message) + if api._creating_virtual_anchors: + message.virtual = True # type: ignore[attr-defined] + api._queue_scene_message(message) # Shallow copy is enough to decouple the handle from the queued # message: AssignablePropsBase.__init__ copies each top-level @@ -356,10 +361,10 @@ def _make( # entry in the buffer via its redundancy key. from ._scene_api import cast_vector - api._websock_interface.queue_message( + api._queue_scene_message( _messages.SetOrientationMessage(name, cast_vector(out._impl.wxyz, 4)) ) - api._websock_interface.queue_message( + api._queue_scene_message( _messages.SetPositionMessage(name, cast_vector(out._impl.position, 3)) ) @@ -384,7 +389,7 @@ def wxyz(self, wxyz: tuple[float, float, float, float] | np.ndarray) -> None: self._impl.wxyz, wxyz, 4, - self._impl.api._websock_interface, + self._impl.api._queue_scene_message, lambda v: _messages.SetOrientationMessage(self._impl.name, v), ) @@ -401,7 +406,7 @@ def position(self, position: tuple[float, float, float] | np.ndarray) -> None: self._impl.position, position, 3, - self._impl.api._websock_interface, + self._impl.api._queue_scene_message, lambda v: _messages.SetPositionMessage(self._impl.name, v), ) @@ -414,7 +419,7 @@ def visible(self) -> bool: def visible(self, visible: bool) -> None: if visible == self._impl.visible: return - self._impl.api._websock_interface.queue_message( + self._impl.api._queue_scene_message( _messages.SetSceneNodeVisibilityMessage(self._impl.name, visible) ) self._impl.visible = visible @@ -471,12 +476,16 @@ def _remove_locked(self) -> None: # dispatch is about to snapshot -- losing the user's required # on_drag_end. drag_active = api._is_drag_active_for(node_name) - _queue_empty_interaction_bindings( - api, - node_name, - had_click=len(impl.click_cb) > 0, - had_drag=had_drag, - ) + # These emits are part of the removal: on a dead per-client + # buffer (remove() from on_client_disconnect) they are benign + # no-ops and must not trip the dead-connection write warning. + with api._websock_interface.get_message_buffer().sanctioned_dead_writes(): + _queue_empty_interaction_bindings( + api, + node_name, + had_click=len(impl.click_cb) > 0, + had_drag=had_drag, + ) # Clear AFTER both emits (the snapshots above key them): if an # emit raises mid-remove, the handle keeps its callback state, so # a RETRY re-emits everything -- clearing first left a retry @@ -506,12 +515,14 @@ def _remove_locked(self) -> None: if parent_children is not None: parent_children.discard(self._impl.name) - # Send a RemoveSceneNodeMessage per descendant so redundancy keys - # clean up their creation messages from the broadcast buffer. + # Send a RemoveSceneNodeMessage per SAME-SCOPE descendant so + # redundancy keys clean up their creation messages from the buffer. + # Cascade is scope-local by design: the client does not recurse on + # removes (this enumeration is the complete removal set), and the + # other scope's variants of these names -- including any children + # hanging from their own scope's virtual anchors -- are untouched. for node_name in to_remove: - api._websock_interface.queue_message( - _messages.RemoveSceneNodeMessage(node_name) - ) + api._queue_scene_message(_messages.RemoveSceneNodeMessage(node_name)) def _on_remove(self) -> None: """Release any subclass-specific registries for this node. @@ -651,7 +662,7 @@ def _sync_drag_bindings(self) -> None: bindings.append( _messages.DragBinding(button=entry.button, modifier=entry.modifier) ) - self._impl.api._websock_interface.queue_message( + self._impl.api._queue_scene_message( _messages.SetSceneNodeDragBindingsMessage(self._impl.name, tuple(bindings)) ) @@ -936,7 +947,7 @@ def _publish_click_state(self) -> None: # Queue the message BEFORE committing the cache. If # ``queue_message`` raises, the cache stays at its previous # value so the next state change retries the publish. - self._impl.api._websock_interface.queue_message( + self._impl.api._queue_scene_message( _messages.SetSceneNodeClickBindingsMessage(self._impl.name, bindings) ) self._impl._last_published_click_bindings = bindings @@ -1367,7 +1378,7 @@ def wxyz(self, wxyz: tuple[float, float, float, float] | np.ndarray) -> None: self._impl.wxyz, wxyz, 4, - self._impl.websock_interface, + self._impl.mesh_impl.api._queue_scene_message, lambda v: _messages.SetBoneOrientationMessage( self._impl.name, self._impl.bone_index, v ), @@ -1387,7 +1398,7 @@ def position(self, position: tuple[float, float, float] | np.ndarray) -> None: self._impl.position, position, 3, - self._impl.websock_interface, + self._impl.mesh_impl.api._queue_scene_message, lambda v: _messages.SetBonePositionMessage( self._impl.name, self._impl.bone_index, v ), @@ -1784,14 +1795,14 @@ def __init__(self, impl: _SceneNodeHandleState, gui_api: GuiApi, container_id: s self._gui_api._container_handle_from_uuid[self._container_id] = self def __enter__(self) -> Gui3dContainerHandle: - self._container_id_restore = self._gui_api._get_container_uuid() + self._container_id_restore = self._gui_api._snapshot_container_context() self._gui_api._set_container_uuid(self._container_id) return self def __exit__(self, *args) -> None: del args assert self._container_id_restore is not None - self._gui_api._set_container_uuid(self._container_id_restore) + self._gui_api._restore_container_context(self._container_id_restore) self._container_id_restore = None @override diff --git a/src/viser/_viser.py b/src/viser/_viser.py index e2d777ffb..ecc3ad4fd 100644 --- a/src/viser/_viser.py +++ b/src/viser/_viser.py @@ -568,6 +568,35 @@ class ClientHandle(DeprecatedAttributeShim if not TYPE_CHECKING else object): these are used, for example via a client's :meth:`SceneApi.add_point_cloud()` method, created elements are local to only one specific client. + + **Client state is ephemeral.** A client handle corresponds to a single + websocket connection: when the browser disconnects or reloads, elements + created through the handle are gone, and the reconnected browser is a new + client (new handle, new ``client_id``). Per-client state should therefore + be (re)built in :meth:`ViserServer.on_client_connect`, which fires again + on reconnect. State that must outlive a connection belongs client-side + (browser storage) or in application code keyed however the application + identifies its users; the server never retains per-client element state. + + **Scene names shadow, not collide.** Each scene-tree name holds at most + one node per scope: adding a client-scoped node under a name the server + also uses creates an independent per-client variant that *shadows* the + server's node for this one client (the server's node, with its latest + state, shows again when the client-scoped variant is removed). State is + fully scope-local -- updates and removals from one scope never touch the + other scope's variant, and removing a node cascades only through its own + scope's descendants. A client-scoped node may be named under a + server-scoped parent (e.g. per-client annotations under a shared frame); + it survives the parent's removal, anchored at the parent's last pose, + until this handle removes it. + + **GUI containers nest one way.** A client-scoped GUI element may be + added inside a server-scoped container context (``with + server.gui.add_folder(...): client.gui.add_button(...)``); it renders + inside the shared folder for this client only, and is removed along + with the folder. The reverse -- a server-scoped element inside a + client-scoped container -- raises, since no other client could see the + container. """ def __init__( @@ -578,6 +607,13 @@ def __init__( self._viser_server = server # Public attributes. + # client_id is assigned BEFORE the scene/gui APIs: SceneApi.__init__ + # reads it (the owner id stamped on this scope's scene messages), and + # an attribute miss during construction would recurse through + # DeprecatedAttributeShim.__getattr__ (whose `self.scene` lookup is + # also unset at that point). + self.client_id: int = conn.client_id + """Unique ID for this client.""" self.scene: SceneApi = SceneApi( self, thread_executor=server._thread_executor, event_loop=server._event_loop ) @@ -586,8 +622,6 @@ def __init__( self, thread_executor=server._thread_executor, event_loop=server._event_loop ) """Handle for interacting with the GUI.""" - self.client_id: int = conn.client_id - """Unique ID for this client.""" self.camera: CameraHandle = CameraHandle(self) """Handle for reading from and manipulating the client's viewport camera.""" @@ -1006,6 +1040,11 @@ def __init__( self._connection = server self._connected_clients: dict[int, ClientHandle] = {} self._client_lock = threading.Lock() + # Lifecycle lock shared by every SceneApi (server- and + # client-scoped). Created BEFORE server.start(): a client can + # connect (and build its SceneApi) as soon as the server thread + # runs, which may be before `self.scene` exists below. + self._scene_lifecycle_lock = threading.RLock() self._client_connect_cb: list[Callable[[ClientHandle], None | Coroutine]] = [] self._client_disconnect_cb: list[ Callable[[ClientHandle], None | Coroutine] @@ -1107,16 +1146,27 @@ async def _(conn: infra.WebsockClientConnection) -> None: self.gui._drop_uploads_from_client(cast(infra.ClientId, conn.client_id)) handle.gui._drop_uploads_from_client(cast(infra.ClientId, conn.client_id)) + # Unhook this client's GUI elements from any SERVER containers + # they were nested in (bookkeeping only; the connection's buffer + # is closed). Otherwise a later server-side container removal + # would cascade removes into a dead connection. + handle.gui._release_cross_scope_nesting() + # Drop any in-flight drag entries for this client; the # corresponding ``phase="end"`` will never arrive, so without # this the active-drag map leaks an entry per dropped drag and - # ``on_drag_end`` is silently skipped. The popped handle is - # passed in explicitly so the synthesized end events can still - # resolve ``event.client`` without the client being publicly - # listed. + # ``on_drag_end`` is silently skipped. BOTH scopes: owner-scoped + # dispatch routes drags on client-scoped nodes to the client's + # own SceneApi, so its map needs the same drain as the server's. + # The popped handle is passed in explicitly so the synthesized + # end events can still resolve ``event.client`` without the + # client being publicly listed. await self.scene._drop_active_drags_for_client( cast(infra.ClientId, conn.client_id), event_client=handle ) + await handle.scene._drop_active_drags_for_client( + cast(infra.ClientId, conn.client_id), event_client=handle + ) await self._dispatch_client_callbacks(disconnect_cbs, handle) # Start the server. @@ -1509,6 +1559,10 @@ def on_client_connect( ) -> Callable[[ClientHandle], NoneOrCoroutine]: """Attach a callback to run for newly connected clients. + This is also where per-client state should be (re)built: client state + is ephemeral (see :class:`ClientHandle`), and a browser that + reconnects or reloads arrives here as a brand-new client. + The callback can be either a standard function or an async function: - Standard functions (def) will be executed in a threadpool. - Async functions (async def) will be executed in the event loop. diff --git a/src/viser/client/src/ControlPanel/SceneTreeTable.tsx b/src/viser/client/src/ControlPanel/SceneTreeTable.tsx index 554fbe1df..d32a36be3 100644 --- a/src/viser/client/src/ControlPanel/SceneTreeTable.tsx +++ b/src/viser/client/src/ControlPanel/SceneTreeTable.tsx @@ -19,7 +19,7 @@ import { import { useDisclosure } from "@mantine/hooks"; import { useForm } from "@mantine/form"; import { ViewerContext } from "../ViewerContext"; -import { SceneNode } from "../SceneTreeState"; +import { ownerOf, SceneNode } from "../SceneTreeState"; import { shallowArrayEqual } from "../utils/shallowArrayEqual"; import { ScenePropDescriptor, @@ -27,6 +27,7 @@ import { } from "../WebsocketMessages"; import { parseToRgb, toMantineColor } from "../components/colorUtils"; import { + Badge, Box, Checkbox, Flex, @@ -608,9 +609,21 @@ const SceneTreeTableRow = React.memo(function SceneTreeTableRow(props: { (node) => node?.children, shallowArrayEqual, ); - const messageType = viewer.useSceneTree( + // One subscription for the row's message-derived facts (scenes can hold + // thousands of rows; every extra per-row selector multiplies the + // subscriber walk on each store write). Variant provenance: client-local + // variants get a badge (they exist only on THIS client, possibly + // shadowing a server node of the same name), and virtual anchors -- + // auto-created ancestor frames that render nothing -- are de-emphasized. + const [messageType, nodeIsClientLocal, nodeIsVirtual] = viewer.useSceneTree( props.nodeName, - (node) => node?.message.type, + (node) => + [ + node?.message.type, + node !== undefined && ownerOf(node.message) !== "", + (node?.message as { virtual?: boolean } | undefined)?.virtual ?? false, + ] as const, + shallowArrayEqual, ); const expandable = (childrenName?.length ?? 0) > 0; const [expanded, { toggle: toggleExpanded }] = useDisclosure(false); @@ -735,11 +748,29 @@ const SceneTreeTableRow = React.memo(function SceneTreeTableRow(props: { whiteSpace: "nowrap", overflow: "hidden", textOverflow: "ellipsis", + ...(nodeIsVirtual && { opacity: 0.45, fontStyle: "italic" }), }} > / {props.nodeName.split("/").at(-1)} + {nodeIsClientLocal ? ( + + + local + + + ) : null} {overrideVisibility !== undefined ? ( } = {}; - const propsUpdates: { [name: string]: { [key: string]: any } } = {}; + const parked = createParkedSceneUpdates( + viewer.useSceneTree, + viewer.sceneTreeActions, + ); const guiUpdates: { uuid: string; updates: { [key: string]: any } }[] = []; for (const msg of processBatch) { + // An add or variant remove flips the name's variant topology at + // receive time; parked updates for it must land first to keep + // wire order (see the drainFor contract). + if ( + isSceneNodeMessage(msg) || + msg.type === "RemoveSceneNodeMessage" + ) { + parked.drainFor(msg.name); + } const result = handleMessage(msg); if (result === undefined) continue; switch (result.kind) { case "sceneNodeAttrUpdate": { - const existing = attrUpdates[result.targetNode]; - if (existing) { - Object.assign(existing, result.updates); - } else { - attrUpdates[result.targetNode] = { ...result.updates }; - } + parked.parkAttr( + ownerOf(msg as { owner?: string }), + result.targetNode, + result.updates, + ); break; } case "sceneNodePropsUpdate": { - const existing = propsUpdates[result.targetNode]; - if (existing) { - Object.assign(existing, result.propsUpdates); - } else { - propsUpdates[result.targetNode] = { ...result.propsUpdates }; - } + parked.parkProps( + ownerOf(msg as { owner?: string }), + result.targetNode, + result.propsUpdates, + ); break; } case "guiUpdate": @@ -1026,38 +1094,7 @@ export function FrameSynchronizedMessageHandler() { } // Apply all accumulated scene tree updates in a single set(). - const mergedUpdates: { [name: string]: SceneNode } = {}; - - // Merge attribute-level updates (wxyz, position, visibility, etc.). - for (const [k, v] of Object.entries(attrUpdates)) { - const currentNode = viewer.useSceneTree.get(k); - if (currentNode === undefined) { - console.log(`(OK) Tried to update non-existent scene node ${k}`); - continue; - } - mergedUpdates[k] = { ...currentNode, ...v }; - } - - // Merge props-level updates (batched_wxyzs, colors, etc.). - for (const [k, v] of Object.entries(propsUpdates)) { - const currentNode = viewer.useSceneTree.get(k); - if (currentNode === undefined) { - console.log(`(OK) Tried to update non-existent scene node ${k}`); - continue; - } - const node = mergedUpdates[k] || currentNode; - mergedUpdates[k] = { - ...node, - message: { - ...node.message, - props: { - ...node.message.props, - ...v, - }, - } as SceneNodeMessage, - }; - } - + const { mergedUpdates, visibilityNames } = parked.flush(); if (Object.keys(mergedUpdates).length > 0) { viewer.useSceneTree.set(mergedUpdates); } @@ -1114,12 +1151,12 @@ export function FrameSynchronizedMessageHandler() { }); } - // Recompute effective visibility for nodes whose visibility changed. - // This needs to be done after updates are applied. - for (const [nodeName, nodeState] of Object.entries(attrUpdates)) { - if ("visibility" in nodeState) { - viewer.sceneTreeActions.computeEffectiveVisibility(nodeName); - } + // Recompute effective visibility for nodes whose visibility change + // actually merged into an effective variant (updates consumed into + // a shadow slot don't affect what renders). This needs to be done + // after updates are applied. + for (const name of visibilityNames) { + viewer.sceneTreeActions.computeEffectiveVisibility(name); } // A render request that arrived with no messages in front of it has diff --git a/src/viser/client/src/SceneTree.tsx b/src/viser/client/src/SceneTree.tsx index 2ffb38c2a..0d6523b41 100644 --- a/src/viser/client/src/SceneTree.tsx +++ b/src/viser/client/src/SceneTree.tsx @@ -11,7 +11,7 @@ import { useThrottledMessageSender, } from "./WebsocketUtils"; import { Html } from "@react-three/drei"; -import { useSceneTreeState } from "./SceneTreeState"; +import { ownerOf, useSceneTreeState } from "./SceneTreeState"; import { rayToViserCoords } from "./WorldTransformUtils"; import { HoverableContext, HoverState } from "./HoverContext"; import { shallowArrayEqual } from "./utils/shallowArrayEqual"; @@ -445,6 +445,7 @@ function createObjectFactory( viewer.mutable.current.sendMessage({ type: "TransformControlsDragStartMessage", name: message.name, + owner: ownerOf(message), }); }} onDrag={(l) => { @@ -485,6 +486,7 @@ function createObjectFactory( name: message.name, wxyz: wxyzArray, position: positionArray, + owner: ownerOf(message), }); }} onDragEnd={() => { @@ -494,6 +496,7 @@ function createObjectFactory( viewer.mutable.current.sendMessage({ type: "TransformControlsDragEndMessage", name: message.name, + owner: ownerOf(message), }); } }} @@ -1207,6 +1210,12 @@ export function SceneNodeThreeObject(props: { name: string }) { ], screen_pos: [mouseVectorOpenCV.x, mouseVectorOpenCV.y], modifier: keyModifierFromEvent(e), + // Echo the EFFECTIVE variant's owner: only the mounted + // variant is interactive, and the server routes the + // event to that scope's registry alone. + owner: ownerOf( + viewer.useSceneTree.get(props.name)?.message, + ), }); } } diff --git a/src/viser/client/src/SceneTreeState.test.ts b/src/viser/client/src/SceneTreeState.test.ts index b757b4785..1fea51b45 100644 --- a/src/viser/client/src/SceneTreeState.test.ts +++ b/src/viser/client/src/SceneTreeState.test.ts @@ -5,10 +5,16 @@ import { createKeyedStore } from "./store"; import { FrameMessage } from "./WebsocketMessages"; import { NodePoseDataMap } from "./ViewerContext"; -function makeFrameMessage(name: string): FrameMessage { +function makeFrameMessage( + name: string, + owner: string = "", + virtual: boolean = false, +): FrameMessage { return { type: "FrameMessage", name, + owner, + virtual, props: { show_axes: true, axes_length: 0.5, @@ -32,7 +38,7 @@ function setup() { const nodeRefFromName: { [name: string]: undefined | THREE.Object3D } = {}; const nodePoseData: NodePoseDataMap = {}; const actions = createSceneTreeActions(store, nodeRefFromName, nodePoseData); - return { store, nodeRefFromName, actions }; + return { store, nodeRefFromName, nodePoseData, actions }; } describe("addSceneNode ref handling", () => { @@ -77,3 +83,401 @@ describe("addSceneNode ref handling", () => { expect(nodeRefFromName["/node"]).toBe(obj); }); }); + +describe("variant slots and the display rule", () => { + it("client variant shadows a broadcast variant, preserving its state", () => { + const { store, nodePoseData, actions } = setup(); + + const broadcastMsg = makeFrameMessage("/x", ""); + expect(actions.addSceneNode(broadcastMsg)).toBe("effective"); + nodePoseData["/x"] = { + wxyz: [0, 0, 0, 1], + position: [1, 2, 3], + poseUpdateState: "updated", + }; + + const clientMsg = makeFrameMessage("/x", "7"); + expect(actions.addSceneNode(clientMsg)).toBe("effective"); + + const node = store.get("/x")!; + expect(node.message).toBe(clientMsg); + // The broadcast variant is parked with its pose snapshot. + expect(node.shadowed?.message).toBe(broadcastMsg); + expect(node.shadowed?.position).toEqual([1, 2, 3]); + // The fresh client variant starts from identity pose. + expect(nodePoseData["/x"]!.position).toEqual([0, 0, 0]); + }); + + it("a virtual client anchor does not shadow a real broadcast node", () => { + const { store, actions } = setup(); + + const broadcastMsg = makeFrameMessage("/x", ""); + actions.addSceneNode(broadcastMsg); + const anchorMsg = makeFrameMessage("/x", "7", true); + expect(actions.addSceneNode(anchorMsg)).toBe("shadowed"); + + const node = store.get("/x")!; + expect(node.message).toBe(broadcastMsg); // Still effective. + expect(node.shadowed?.message).toBe(anchorMsg); // Parked. + }); + + it("a real broadcast node arriving late does not displace a real client node", () => { + const { store, actions } = setup(); + + const clientMsg = makeFrameMessage("/x", "7"); + actions.addSceneNode(clientMsg); + const broadcastMsg = makeFrameMessage("/x", ""); + actions.addSceneNode(broadcastMsg); + + const node = store.get("/x")!; + expect(node.message).toBe(clientMsg); + expect(node.shadowed?.message).toBe(broadcastMsg); + }); + + it("removing the effective variant promotes the shadowed one with accumulated state", () => { + const { store, nodePoseData, actions } = setup(); + + actions.addSceneNode(makeFrameMessage("/x", "")); + actions.addSceneNode(makeFrameMessage("/x", "7")); // Shadows broadcast. + + // Broadcast keeps updating while shadowed; the router consumes the + // update (returns true) instead of letting it hit the effective path. + expect(actions.routeShadowedUpdate("/x", "", { position: [4, 5, 6] })).toBe( + true, + ); + // An update for the EFFECTIVE variant is not consumed. + expect( + actions.routeShadowedUpdate("/x", "7", { position: [0, 0, 9] }), + ).toBe(false); + + expect(actions.removeSceneNodeVariant("/x", "7")).toBe("promoted"); + const node = store.get("/x")!; + expect(node.message.owner).toBe(""); + expect(node.shadowed).toBeUndefined(); + // Promotion restores the broadcast variant's LATEST pose. + expect(nodePoseData["/x"]!.position).toEqual([4, 5, 6]); + }); + + it("a promoted virtual anchor inherits the departing variant's pose", () => { + const { store, nodePoseData, actions } = setup(); + + actions.addSceneNode(makeFrameMessage("/a", "")); // Real broadcast parent. + actions.addSceneNode(makeFrameMessage("/a", "7", true)); // Client anchor, parked. + nodePoseData["/a"] = { + wxyz: [1, 0, 0, 0], + position: [9, 9, 9], + poseUpdateState: "updated", + }; + + actions.removeSceneNodeVariant("/a", ""); + const node = store.get("/a")!; + expect(node.message.virtual).toBe(true); + // Frozen-pose inheritance: children of /a stay where they were. + expect(nodePoseData["/a"]!.position).toEqual([9, 9, 9]); + }); + + it("variant removal is scope-local and non-recursive", () => { + const { store, actions } = setup(); + + actions.addSceneNode(makeFrameMessage("/a", "")); + actions.addSceneNode(makeFrameMessage("/a/child", "7")); + + // Broadcast /a removed; the client child's entry must survive (its own + // scope's anchor/removals are the only things that may touch it). + actions.removeSceneNodeVariant("/a", ""); + expect(store.get("/a")).toBeUndefined(); + expect(store.get("/a/child")).toBeDefined(); + }); + + it("removing a shadowed variant leaves the effective one untouched", () => { + const { store, actions } = setup(); + + const clientMsg = makeFrameMessage("/x", "7"); + actions.addSceneNode(makeFrameMessage("/x", "")); + actions.addSceneNode(clientMsg); + + actions.removeSceneNodeVariant("/x", ""); // Drop the parked broadcast copy. + const node = store.get("/x")!; + expect(node.message).toBe(clientMsg); + expect(node.shadowed).toBeUndefined(); + }); + + it("same-scope supersede preserves the shadow slot", () => { + const { store, actions } = setup(); + + actions.addSceneNode(makeFrameMessage("/x", "7")); // Client, effective. + actions.addSceneNode(makeFrameMessage("/x", "")); // Broadcast, parked. + const newClientMsg = makeFrameMessage("/x", "7"); + actions.addSceneNode(newClientMsg); // Client supersede. + + const node = store.get("/x")!; + expect(node.message).toBe(newClientMsg); + expect(node.shadowed?.message.owner).toBe(""); + }); +}); + +describe("removeSceneNodeVariantSubtree", () => { + it("removes same-scope descendants of a single non-enumerated remove (old recordings)", () => { + // Recordings from servers predating per-descendant remove enumeration + // contain ONE RemoveSceneNodeMessage per subtree; the store must sweep + // descendants (and their side state) itself. + const { store, nodeRefFromName, nodePoseData, actions } = setup(); + for (const name of ["/p", "/p/a", "/p/a/b"]) { + actions.addSceneNode(makeFrameMessage(name, "")); + nodeRefFromName[name] = new THREE.Object3D(); + nodePoseData[name] = { + wxyz: [1, 0, 0, 0], + position: [0, 0, 0], + poseUpdateState: "updated", + }; + } + + const removedNames = actions.removeSceneNodeVariantSubtree("/p", ""); + + for (const name of ["/p", "/p/a", "/p/a/b"]) { + expect(store.get(name)).toBeUndefined(); + expect(nodeRefFromName[name]).toBeUndefined(); + expect(nodePoseData[name]).toBeUndefined(); + } + expect(removedNames).toEqual(["/p", "/p/a", "/p/a/b"]); + // Later per-descendant remove messages (current servers enumerate them) + // no-op silently. + expect(actions.removeSceneNodeVariantSubtree("/p/a", "")).toEqual([]); + }); + + it("stays scope-local: other-owner descendants survive and shadowed ones promote", () => { + const { store, actions } = setup(); + actions.addSceneNode(makeFrameMessage("/p", "")); + actions.addSceneNode(makeFrameMessage("/p/shared", "")); + const clientVariant = makeFrameMessage("/p/shared", "7"); + actions.addSceneNode(clientVariant); // Shadows the broadcast one. + actions.addSceneNode(makeFrameMessage("/p/mine", "7")); + + const removedNames = actions.removeSceneNodeVariantSubtree("/p", ""); + + // Both broadcast variants went away: /p entirely, /p/shared's PARKED + // copy (the client variant shadows it and stays effective). + expect(removedNames).toEqual(["/p", "/p/shared"]); + expect(store.get("/p/shared")!.message).toBe(clientVariant); + // The client-only descendant is untouched by the broadcast sweep. + expect(store.get("/p/mine")).toBeDefined(); + }); +}); + +describe("routeShadowedUpdate", () => { + it("drops a stale other-scope update even when no shadow slot exists", () => { + // Server adds /a, a client shadows it, then the server variant is + // removed (no shadow slot anywhere anymore). A late broadcast-owned + // update -- e.g. a write through a stale server handle -- must be + // consumed (dropped), NOT applied to the client's surviving variant. + // Regression: a global shadow-count fast path skipped the per-name + // owner check when the count was zero. + const { store, actions } = setup(); + actions.addSceneNode(makeFrameMessage("/a", "")); + const clientMsg = makeFrameMessage("/a", "7"); + actions.addSceneNode(clientMsg); + actions.removeSceneNodeVariant("/a", ""); // Parked broadcast copy dies. + + const consumed = actions.routeShadowedUpdate("/a", "", { + position: [9, 9, 9], + }); + expect(consumed).toBe(true); + expect(store.get("/a")!.message).toBe(clientMsg); + }); +}); + +describe("parked batch updates (batchedSceneUpdates)", () => { + it("keeps wire order across a mid-batch variant flip", async () => { + // Batch: [visibility(false, server), client add (flip), visibility(true, + // server)]. Msg 1 parks (server variant effective at receive time); msg 3 + // is consumed into the shadow slot at receive time. Without draining the + // parked entries at the flip, the STALE parked false would overwrite the + // newer true at flush. + const { createParkedSceneUpdates } = await import("./batchedSceneUpdates"); + const { store, actions } = setup(); + actions.addSceneNode(makeFrameMessage("/x", "")); + const parked = createParkedSceneUpdates(store, actions); + + // Msg 1: server visibility=false. Effective at receive time -> parks. + expect(actions.routeShadowedUpdate("/x", "", { visibility: false })).toBe( + false, + ); + parked.parkAttr("", "/x", { visibility: false }); + + // Msg 2: client add flips the effective variant. MessageHandler drains + // parked entries for the name BEFORE the add. + parked.drainFor("/x"); + actions.addSceneNode(makeFrameMessage("/x", "7")); + + // Msg 3: server visibility=true. Server variant now shadowed -> consumed + // into the shadow slot at receive time. + expect(actions.routeShadowedUpdate("/x", "", { visibility: true })).toBe( + true, + ); + + const { mergedUpdates, visibilityNames } = parked.flush(); + store.set(mergedUpdates); + + // The server variant's accumulated state must reflect the LAST wire + // value (true), and no effective-visibility recompute is owed (nothing + // merged into the effective variant). + expect(store.get("/x")!.shadowed!.visibility).toBe(true); + expect(visibilityNames).toEqual([]); + + // Promotion materializes that state: remove the client variant and the + // server node comes back visible. + actions.removeSceneNodeVariant("/x", "7"); + expect(store.get("/x")!.visibility).toBe(true); + }); + + it("reports merged visibility changes for effective-variant recompute", async () => { + const { createParkedSceneUpdates } = await import("./batchedSceneUpdates"); + const { store, actions } = setup(); + actions.addSceneNode(makeFrameMessage("/y", "")); + const parked = createParkedSceneUpdates(store, actions); + + parked.parkAttr("", "/y", { visibility: false }); + const { mergedUpdates, visibilityNames } = parked.flush(); + store.set(mergedUpdates); + + expect(store.get("/y")!.visibility).toBe(false); + expect(visibilityNames).toEqual(["/y"]); + }); +}); + +describe("randomized display-rule oracle", () => { + // Deterministic PRNG so failures reproduce from the logged round index. + function mulberry32(seed: number) { + return () => { + seed |= 0; + seed = (seed + 0x6d2b79f5) | 0; + let t = Math.imul(seed ^ (seed >>> 15), 1 | seed); + t = (t + Math.imul(t ^ (t >>> 7), 61 | t)) ^ t; + return ((t ^ (t >>> 14)) >>> 0) / 4294967296; + }; + } + + const rank = (owner: string, virtual: boolean) => + (virtual ? 0 : 2) + (owner !== "" ? 1 : 0); + + type ModelVariant = { virtual: boolean; visibility: boolean }; + + it("random op sequences match the documented semantics", async () => { + const { createParkedSceneUpdates } = await import("./batchedSceneUpdates"); + const NAMES = ["/n1", "/n2"]; + const OWNERS = ["", "7"]; + + for (let round = 0; round < 300; round++) { + const rand = mulberry32(round + 1); + const { store, actions } = setup(); + const parked = createParkedSceneUpdates(store, actions); + // Oracle: per name, per owner, the variant's state; plus which owner + // is effective. Mirrors ONLY the documented rules (display rule, + // fresh-on-add, snapshot-on-shadow, promotion, virtual force-true). + const model: { + [name: string]: { + variants: { [owner: string]: ModelVariant }; + effective: string | undefined; + }; + } = { + "/n1": { variants: {}, effective: undefined }, + "/n2": { variants: {}, effective: undefined }, + }; + const opLog: string[] = []; + + for (let i = 0; i < 12; i++) { + const name = NAMES[Math.floor(rand() * NAMES.length)]; + const owner = OWNERS[Math.floor(rand() * OWNERS.length)]; + const m = model[name]; + const r = rand(); + if (r < 0.45) { + // Add (sometimes virtual, as anchors are on the wire). + const virtual = rand() < 0.25; + opLog.push(`add(${name}, ${owner || "server"}, virtual=${virtual})`); + parked.drainFor(name); + actions.addSceneNode(makeFrameMessage(name, owner, virtual)); + if (m.effective !== undefined && m.effective !== owner) { + const eff = m.variants[m.effective]; + if (rank(owner, virtual) >= rank(m.effective, eff.virtual)) { + m.variants[owner] = { virtual, visibility: true }; + m.effective = owner; // Old effective keeps its state, parked. + } else { + m.variants[owner] = { virtual, visibility: true }; // Parked fresh. + } + } else { + const prev = m.variants[owner]; + m.variants[owner] = { + virtual, + // Same-owner supersede preserves visibility; new name is true. + visibility: m.effective === owner ? prev.visibility : true, + }; + // Stays effective WITHOUT re-ranking against the shadow slot, + // matching addSceneNode: the wire never downgrades an existing + // in-scope name to a virtual anchor, so the only sequence where + // this would matter (real -> virtual same-owner supersede while + // shadowing a real other-scope variant) is unreachable live. + m.effective = owner; + } + } else if (r < 0.7) { + opLog.push(`removeVariant(${name}, ${owner || "server"})`); + parked.drainFor(name); + actions.removeSceneNodeVariant(name, owner); + if (m.variants[owner] !== undefined) { + delete m.variants[owner]; + if (m.effective === owner) { + const other = Object.keys(m.variants)[0]; + m.effective = other; + if (other !== undefined && m.variants[other].virtual) { + m.variants[other].visibility = true; // Promotion force-true. + } + } + } + } else { + // Visibility update, exactly as MessageHandler routes it. Skipped + // for virtual variants (no wire path sends these). + if (m.variants[owner]?.virtual) continue; + const visible = rand() < 0.5; + opLog.push(`visibility(${name}, ${owner || "server"}, ${visible})`); + if ( + !actions.routeShadowedUpdate(name, owner, { visibility: visible }) + ) { + parked.parkAttr(owner, name, { visibility: visible }); + } + if (m.variants[owner] !== undefined) { + m.variants[owner].visibility = visible; + } + } + } + + const { mergedUpdates } = parked.flush(); + store.set(mergedUpdates); + + for (const name of NAMES) { + const m = model[name]; + const node = store.get(name); + const ctx = `round ${round}: ${opLog.join(" ; ")} -- ${name}`; + if (m.effective === undefined) { + expect(node, ctx).toBeUndefined(); + continue; + } + expect(node, ctx).toBeDefined(); + const eff = m.variants[m.effective]; + expect(node!.message.owner ?? "", ctx).toBe(m.effective); + expect(!!node!.message.virtual, ctx).toBe(eff.virtual); + expect(node!.visibility ?? true, ctx).toBe(eff.visibility); + const shadowOwner = Object.keys(m.variants).find( + (o) => o !== m.effective, + ); + if (shadowOwner === undefined) { + expect(node!.shadowed, ctx).toBeUndefined(); + } else { + const sh = m.variants[shadowOwner]; + expect(node!.shadowed, ctx).toBeDefined(); + expect(node!.shadowed!.message.owner ?? "", ctx).toBe(shadowOwner); + expect(!!node!.shadowed!.message.virtual, ctx).toBe(sh.virtual); + expect(node!.shadowed!.visibility, ctx).toBe(sh.visibility); + } + } + } + }); +}); diff --git a/src/viser/client/src/SceneTreeState.ts b/src/viser/client/src/SceneTreeState.ts index 13815ad7a..95bdd2287 100644 --- a/src/viser/client/src/SceneTreeState.ts +++ b/src/viser/client/src/SceneTreeState.ts @@ -20,8 +20,51 @@ export type SceneNode = { visibility?: boolean; // Visibility state from the server. overrideVisibility?: boolean; // Override from the GUI. effectiveVisibility?: boolean; // Computed visibility including parent chain. + /** The lower-ranked variant of this name, when both scopes (broadcast + + * this client) have one. Each scene-tree name holds at most one variant + * per scope; only the effective (higher-ranked) variant is mounted and + * interactive, but the shadowed variant keeps accumulating state from its + * scope's messages so that removing the effective variant promotes it + * with up-to-date state -- no resurrection round-trip needed. */ + shadowed?: ShadowedVariant; }; +export type ShadowedVariant = { + message: SceneNodeMessage; + clickBindings: DragBinding[]; + dragBindings: DragBinding[]; + wxyz: [number, number, number, number]; + position: [number, number, number]; + visibility: boolean; +}; + +/** Owner id stamped on scene messages: "" is the broadcast scope + * (server.scene), anything else is a per-client scope. Old recordings + * predate the field; a missing field -- or a missing MESSAGE, e.g. an + * interaction racing a node removal -- means broadcast. */ +/** Composite key for one scope's variant of a scene node, used wherever + * per-variant side state lives in a flat map (skinnedMeshState, the message + * batcher's parked updates). Owners are "" (broadcast) or a client id, so + * NUL can't collide with a real owner. */ +export function variantKey(owner: string | undefined, name: string): string { + return `${owner ?? ""}\u0000${name}`; +} + +export function ownerOf(message: { owner?: string } | undefined): string { + return message?.owner ?? ""; +} + +function isVirtual(message: SceneNodeMessage): boolean { + return (message as { virtual?: boolean }).virtual ?? false; +} + +/** Display-rule rank: real client > real broadcast > virtual client > + * virtual broadcast. The higher-ranked variant of a name is effective + * (rendered, interactive); the other is shadowed. */ +function variantRank(message: SceneNodeMessage): number { + return (isVirtual(message) ? 0 : 2) + (ownerOf(message) !== "" ? 1 : 0); +} + function makeRootNodeTemplate(): SceneNode { // Default quaternion: 90 deg around X, 180 deg around Y, -90 deg around Z. // This matches the coordinate system transformation. @@ -33,6 +76,8 @@ function makeRootNodeTemplate(): SceneNode { message: { type: "FrameMessage", name: "", + owner: "", + virtual: false, props: { show_axes: false, axes_length: 0.5, @@ -57,6 +102,8 @@ function makeWorldAxesNodeTemplate(): SceneNode { message: { type: "FrameMessage", name: "/WorldAxes", + owner: "", + virtual: false, props: { show_axes: true, axes_length: 0.5, @@ -90,9 +137,109 @@ export function createSceneTreeActions( nodeRefFromName: { [name: string]: undefined | THREE.Object3D }, nodePoseData: NodePoseDataMap, ) { + /** Pre-order names of `name`'s subtree, collected BEFORE any removal: + * children lists die with their nodes. Shared by variant-subtree and + * whole-node removal. */ + function collectSubtreeNames(name: string): string[] { + const names: string[] = []; + function collect(nodeName: string) { + names.push(nodeName); + store.get(nodeName)?.children.forEach(collect); + } + collect(name); + return names; + } + + /** Remove `name` from its parent's `children` list, recording the change + * in `updates`. Shared by variant removal and recursive removal. */ + function dropFromParentChildren( + name: string, + updates: Record, + ) { + const parentName = name.split("/").slice(0, -1).join("/"); + const parentNode = store.get(parentName); + if (parentNode) { + updates[parentName] = { + ...parentNode, + children: parentNode.children.filter( + (child_name) => child_name !== name, + ), + }; + } + } + const actions = { - addSceneNode: (message: SceneNodeMessage) => { + /** Returns whether the added variant became the effective one for its + * name, or was parked in the shadow slot. */ + addSceneNode: (message: SceneNodeMessage): "effective" | "shadowed" => { const existingNode = store.get(message.name); + + // Cross-scope add: the name already has a variant from the OTHER + // scope. The display rule decides which becomes effective; the loser + // is parked in the shadow slot, where its scope's messages keep + // updating it. + if ( + existingNode !== undefined && + ownerOf(existingNode.message) !== ownerOf(message) + ) { + if (variantRank(message) >= variantRank(existingNode.message)) { + // Incoming variant shadows the current effective one. Snapshot the + // effective variant's state (including its live pose) into the + // shadow slot, then install the incoming variant fresh: a new + // variant starts at default pose/visibility, and its own Set* + // messages follow its create in the same buffer. + const pose = nodePoseData[message.name]; + const shadowed: ShadowedVariant = { + message: existingNode.message, + clickBindings: existingNode.clickBindings, + dragBindings: existingNode.dragBindings, + wxyz: pose?.wxyz ?? [1, 0, 0, 0], + position: pose?.position ?? [0, 0, 0], + visibility: existingNode.visibility ?? true, + }; + delete nodeRefFromName[message.name]; + nodePoseData[message.name] = { + wxyz: [1, 0, 0, 0], + position: [0, 0, 0], + poseUpdateState: "needsUpdate", + }; + store.set({ + [message.name]: { + ...existingNode, + message, + shadowed, + clickBindings: [], + dragBindings: [], + visibility: true, + }, + }); + actions.computeEffectiveVisibility(message.name); + return "effective"; + } + // Incoming variant is lower-ranked (e.g. a virtual anchor next to + // a real node): park it in the shadow slot, effective untouched. + store.set({ + [message.name]: { + ...existingNode, + shadowed: { + message, + clickBindings: [], + dragBindings: [], + wxyz: [1, 0, 0, 0], + position: [0, 0, 0], + visibility: true, + }, + }, + }); + return "shadowed"; + } + + // Same-owner add (within-scope create or supersede), or a brand-new + // name. `...existingNode` carries any shadow slot across a supersede. + // Deliberately NOT re-ranked against the shadow slot: the wire never + // downgrades an existing in-scope name to a virtual anchor (anchors + // are only sent for names missing in that scope), so a supersede + // keeps this variant effective unconditionally. const parentName = message.name.split("/").slice(0, -1).join("/"); const parentNode = store.get(parentName); @@ -123,19 +270,137 @@ export function createSceneTreeActions( delete nodeRefFromName[message.name]; } store.set(updates); + return "effective"; }, - removeSceneNode: (name: string) => { - // Remove this scene node and all children. - const removeNames: string[] = []; - function findChildrenRecursive(nodeName: string) { - removeNames.push(nodeName); - const node = store.get(nodeName); - if (node) { - node.children.forEach(findChildrenRecursive); + /** Remove ONE scope's variant of a name. Scope-local by design: the + * server enumerates a message per same-scope descendant, and the other + * scope's variants (including children hanging from their own scope's + * virtual anchors) are untouched -- so unlike `removeSceneNode`, this + * does not recurse. Removing the effective variant promotes the + * shadowed one; a promoted VIRTUAL anchor inherits the departing + * variant's pose (frozen-pose inheritance), so surviving children stay + * where they were instead of teleporting to identity. + * + * The return value reports which disposition happened, so callers can + * act on it (e.g. clean per-node side state only when the mounted + * variant went away) without re-deriving the display-rule decision. */ + removeSceneNodeVariant: ( + name: string, + owner: string, + ): "removed-effective" | "promoted" | "removed-shadow" | "noop" => { + // Absent names are routine, not an anomaly: servers enumerate one + // remove per descendant, and the subtree recursion (see + // removeSceneNodeVariantSubtree) has usually removed them already. + const node = store.get(name); + if (node === undefined) return "noop"; + if (ownerOf(node.message) === owner) { + const shadowed = node.shadowed; + if (shadowed !== undefined) { + // Promote the shadowed variant, with the state its scope's + // messages have been accumulating while it was hidden. + delete nodeRefFromName[name]; + if (!isVirtual(shadowed.message)) { + nodePoseData[name] = { + wxyz: shadowed.wxyz, + position: shadowed.position, + poseUpdateState: "needsUpdate", + }; + } + store.set({ + [name]: { + ...node, + message: shadowed.message, + clickBindings: shadowed.clickBindings, + dragBindings: shadowed.dragBindings, + visibility: isVirtual(shadowed.message) + ? true + : shadowed.visibility, + shadowed: undefined, + }, + }); + actions.computeEffectiveVisibility(name); + return "promoted"; + } + // Last variant: drop the entry (no recursion -- see docstring). + const updates: Record = { + [name]: undefined, + }; + delete nodeRefFromName[name]; + delete nodePoseData[name]; + dropFromParentChildren(name, updates); + store.set(updates); + return "removed-effective"; + } + if (node.shadowed && ownerOf(node.shadowed.message) === owner) { + store.set({ [name]: { ...node, shadowed: undefined } }); + return "removed-shadow"; + } + return "noop"; + }, + + /** Remove one scope's variants of `name` AND its same-scope descendants. + * Current servers enumerate one RemoveSceneNodeMessage per descendant + * (the recursion then no-ops on the later messages), but recordings from + * older servers contain a single non-enumerated remove per subtree -- + * without the recursion their descendants would linger forever, along + * with their pose/ref side state. Still scope-local: other-owner + * variants of descendant names are untouched. Returns the names whose + * variant actually went away, so callers can clean per-variant side + * state. */ + removeSceneNodeVariantSubtree: (name: string, owner: string): string[] => { + if (store.get(name) === undefined) return []; + return collectSubtreeNames(name).filter( + (n) => actions.removeSceneNodeVariant(n, owner) !== "noop", + ); + }, + + /** Route a node-keyed state update by owner: returns false when it + * targets the EFFECTIVE variant of `name` (or the scope-less root + * singleton), in which case the caller applies it through the normal + * effective-variant path; returns true when it was consumed here -- + * applied to the shadowed variant, or dropped because no variant of + * that owner exists (e.g. the update raced a removal). + * + * Shadowed state is mutated IN PLACE, with no store write: nothing + * subscribes to the shadow slot (only promotion reads it, via + * non-reactive store.get), and a server animating a shadowed node can + * stream pose updates at 60Hz -- reactive writes would notify every + * subscriber of the effective node per message for state that nothing + * renders. Promotion materializes the accumulated state into fresh + * store objects. */ + routeShadowedUpdate: ( + name: string, + owner: string | undefined, + updates: Partial> & { + propsUpdates?: { [key: string]: any }; + }, + ): boolean => { + // The root ("") is a singleton across scopes; owner is ignored for it. + if (name === "") return false; + const node = store.get(name); + if (node === undefined) return false; + if (ownerOf(node.message) === (owner ?? "")) return false; + const shadowed = node.shadowed; + if ( + shadowed !== undefined && + ownerOf(shadowed.message) === (owner ?? "") + ) { + const { propsUpdates, ...rest } = updates; + Object.assign(shadowed, rest); + if (propsUpdates !== undefined) { + Object.assign( + shadowed.message.props as Record, + propsUpdates, + ); } } - findChildrenRecursive(name); + return true; + }, + + removeSceneNode: (name: string) => { + // Remove this scene node and all children. + const removeNames = collectSubtreeNames(name); const updates: Record = {}; removeNames.forEach((removeName) => { @@ -145,16 +410,7 @@ export function createSceneTreeActions( }); // Remove node from parent's children list. - const parentName = name.split("/").slice(0, -1).join("/"); - const parentNode = store.get(parentName); - if (parentNode) { - updates[parentName] = { - ...parentNode, - children: parentNode.children.filter( - (child_name) => child_name !== name, - ), - }; - } + dropFromParentChildren(name, updates); store.set(updates); }, @@ -217,7 +473,6 @@ export function createSceneTreeActions( actions.removeSceneNode(child); } } - // Reset root and /WorldAxes to default state. const defaultState = makeDefaultSceneTreeState(); store.set({ diff --git a/src/viser/client/src/ViewerContext.ts b/src/viser/client/src/ViewerContext.ts index 84607b872..8b6147e3f 100644 --- a/src/viser/client/src/ViewerContext.ts +++ b/src/viser/client/src/ViewerContext.ts @@ -78,9 +78,12 @@ export type ViewerMutable = { rootWxyzAtCapture: [number, number, number, number]; } | null; - // Skinned mesh state. + // Skinned mesh state, keyed PER VARIANT via variantKey(owner, + // name): each scope's variant of a name owns independent bone state, so + // bone updates for a shadowed variant accumulate without corrupting the + // effective one, and promotion finds the promoted variant's state intact. skinnedMeshState: { - [name: string]: { + [ownerAndName: string]: { initialized: boolean; // True once a mounted SkinnedMesh instance has claimed this entry. // Entries can be recreated without a remount (FilePlayback's loop and @@ -102,6 +105,8 @@ export type ViewerMutable = { nodePoseData: NodePoseDataMap; }; +export { variantKey } from "./SceneTreeState"; + export type ViewerContextContents = { // Non-mutable state. messageSource: "websocket" | "file_playback" | "embed"; diff --git a/src/viser/client/src/WebsocketInterface.tsx b/src/viser/client/src/WebsocketInterface.tsx index 4cfc4ec69..01fbe60b3 100644 --- a/src/viser/client/src/WebsocketInterface.tsx +++ b/src/viser/client/src/WebsocketInterface.tsx @@ -62,12 +62,17 @@ export function WebsocketMessageProducer() { // so this transient state isn't reset for us. viewerMutable.messageQueue.length = 0; viewerMutable.firstMessageBatch = true; - // Skinned-mesh pose buffers are keyed by node name on the mutable ref, + // Skinned-mesh pose buffers are keyed by variant on the mutable ref, // which persists across reconnects; drop them so they don't leak (and // so stale bone state doesn't apply to the fresh scene). for (const key of Object.keys(viewerMutable.skinnedMeshState)) { delete viewerMutable.skinnedMeshState[key]; } + // Scene-pointer filters are keyed per owner, and owner ids are + // connection-scoped: entries from the previous connection can never + // be disabled again, so drop them all before the replay re-enables + // the live ones. + viewer.interaction.scenePointer.clearFilters(); // Clear any render request left in flight from the previous connection. // Message handling is gated on this being "ready", and a stale request // would otherwise render once against the fresh scene (its response is diff --git a/src/viser/client/src/WebsocketMessages.ts b/src/viser/client/src/WebsocketMessages.ts index 3ce7acd43..a7303ec73 100644 --- a/src/viser/client/src/WebsocketMessages.ts +++ b/src/viser/client/src/WebsocketMessages.ts @@ -9,6 +9,8 @@ export interface CameraFrustumMessage { type: "CameraFrustumMessage"; name: string; + owner: string; + virtual: boolean; props: { fov: number; aspect: number; @@ -30,6 +32,8 @@ export interface CameraFrustumMessage { export interface GlbMessage { type: "GlbMessage"; name: string; + owner: string; + virtual: boolean; props: { glb_data: Uint8Array; cast_shadow: boolean; @@ -44,6 +48,8 @@ export interface GlbMessage { export interface FrameMessage { type: "FrameMessage"; name: string; + owner: string; + virtual: boolean; props: { show_axes: boolean; axes_length: number; @@ -63,6 +69,8 @@ export interface FrameMessage { export interface BatchedAxesMessage { type: "BatchedAxesMessage"; name: string; + owner: string; + virtual: boolean; props: { batched_wxyzs: Float32Array; batched_positions: Float32Array; @@ -79,6 +87,8 @@ export interface BatchedAxesMessage { export interface GridMessage { type: "GridMessage"; name: string; + owner: string; + virtual: boolean; props: { width: number; height: number; @@ -106,6 +116,8 @@ export interface GridMessage { export interface LabelMessage { type: "LabelMessage"; name: string; + owner: string; + virtual: boolean; props: { text: string; font_size_mode: "screen" | "scene"; @@ -131,6 +143,8 @@ export interface LabelMessage { export interface Gui3DMessage { type: "Gui3DMessage"; name: string; + owner: string; + virtual: boolean; props: { order: number; container_uuid: string }; } /** Point cloud message. @@ -145,6 +159,8 @@ export interface Gui3DMessage { export interface PointCloudMessage { type: "PointCloudMessage"; name: string; + owner: string; + virtual: boolean; props: { points: Uint16Array | Float32Array; colors: Uint8Array; @@ -162,6 +178,8 @@ export interface PointCloudMessage { export interface DirectionalLightMessage { type: "DirectionalLightMessage"; name: string; + owner: string; + virtual: boolean; props: { color: [number, number, number]; intensity: number; @@ -175,6 +193,8 @@ export interface DirectionalLightMessage { export interface AmbientLightMessage { type: "AmbientLightMessage"; name: string; + owner: string; + virtual: boolean; props: { color: [number, number, number]; intensity: number }; } /** Hemisphere light message. @@ -184,6 +204,8 @@ export interface AmbientLightMessage { export interface HemisphereLightMessage { type: "HemisphereLightMessage"; name: string; + owner: string; + virtual: boolean; props: { sky_color: [number, number, number]; ground_color: [number, number, number]; @@ -197,6 +219,8 @@ export interface HemisphereLightMessage { export interface PointLightMessage { type: "PointLightMessage"; name: string; + owner: string; + virtual: boolean; props: { color: [number, number, number]; intensity: number; @@ -212,6 +236,8 @@ export interface PointLightMessage { export interface RectAreaLightMessage { type: "RectAreaLightMessage"; name: string; + owner: string; + virtual: boolean; props: { color: [number, number, number]; intensity: number; @@ -226,6 +252,8 @@ export interface RectAreaLightMessage { export interface SpotLightMessage { type: "SpotLightMessage"; name: string; + owner: string; + virtual: boolean; props: { color: [number, number, number]; intensity: number; @@ -246,6 +274,8 @@ export interface SpotLightMessage { export interface MeshMessage { type: "MeshMessage"; name: string; + owner: string; + virtual: boolean; props: { vertices: Float32Array; faces: Uint32Array; @@ -267,6 +297,8 @@ export interface MeshMessage { export interface BoxMessage { type: "BoxMessage"; name: string; + owner: string; + virtual: boolean; props: { dimensions: [number, number, number]; color: [number, number, number]; @@ -287,6 +319,8 @@ export interface BoxMessage { export interface IcosphereMessage { type: "IcosphereMessage"; name: string; + owner: string; + virtual: boolean; props: { radius: number; subdivisions: number; @@ -308,6 +342,8 @@ export interface IcosphereMessage { export interface CylinderMessage { type: "CylinderMessage"; name: string; + owner: string; + virtual: boolean; props: { radius: number; height: number; @@ -330,6 +366,8 @@ export interface CylinderMessage { export interface SkinnedMeshMessage { type: "SkinnedMeshMessage"; name: string; + owner: string; + virtual: boolean; props: { vertices: Float32Array; faces: Uint32Array; @@ -355,6 +393,8 @@ export interface SkinnedMeshMessage { export interface BatchedMeshesMessage { type: "BatchedMeshesMessage"; name: string; + owner: string; + virtual: boolean; props: { batched_wxyzs: Float32Array; batched_positions: Float32Array; @@ -381,6 +421,8 @@ export interface BatchedMeshesMessage { export interface BatchedGlbMessage { type: "BatchedGlbMessage"; name: string; + owner: string; + virtual: boolean; props: { batched_wxyzs: Float32Array; batched_positions: Float32Array; @@ -399,6 +441,8 @@ export interface BatchedGlbMessage { export interface TransformControlsMessage { type: "TransformControlsMessage"; name: string; + owner: string; + virtual: boolean; props: { scale: number; line_width: number; @@ -420,6 +464,8 @@ export interface TransformControlsMessage { export interface ImageMessage { type: "ImageMessage"; name: string; + owner: string; + virtual: boolean; props: { _format: "jpeg" | "png"; _data: Uint8Array; @@ -437,6 +483,8 @@ export interface ImageMessage { export interface LineSegmentsMessage { type: "LineSegmentsMessage"; name: string; + owner: string; + virtual: boolean; props: { points: Float32Array; thickness: number; @@ -452,6 +500,8 @@ export interface LineSegmentsMessage { export interface ArrowMessage { type: "ArrowMessage"; name: string; + owner: string; + virtual: boolean; props: { points: Float32Array; colors: Uint8Array; @@ -468,6 +518,8 @@ export interface ArrowMessage { export interface CatmullRomSplineMessage { type: "CatmullRomSplineMessage"; name: string; + owner: string; + virtual: boolean; props: { points: Float32Array; curve_type: "centripetal" | "chordal" | "catmullrom"; @@ -487,6 +539,8 @@ export interface CatmullRomSplineMessage { export interface CubicBezierSplineMessage { type: "CubicBezierSplineMessage"; name: string; + owner: string; + virtual: boolean; props: { points: Float32Array; control_points: Float32Array; @@ -504,15 +558,21 @@ export interface CubicBezierSplineMessage { export interface GaussianSplatsMessage { type: "GaussianSplatsMessage"; name: string; + owner: string; + virtual: boolean; props: { buffer: Uint32Array; scale: number | [number, number, number] }; } -/** Remove a particular node from the scene. +/** Remove a particular node's variant, for the scope stamped in + * ``owner``, from the scene. Removal is scope-local: it never touches the + * other scope's variant of the same name (the server enumerates one such + * message per same-scope descendant; the client does not cascade). * * (automatically generated) */ export interface RemoveSceneNodeMessage { type: "RemoveSceneNodeMessage"; name: string; + owner: string; } /** GuiFolderMessage(uuid: 'str', container_uuid: 'str', props: 'GuiFolderProps') * @@ -1273,10 +1333,16 @@ export interface ScenePointerMessage { /** Set the modifier-filter set for a scene pointer ``event_type``. * * An empty ``modifiers`` tuple disables all callbacks for that - * ``event_type``. A non-empty tuple enables them, and the client uses - * the filter list to gate gesture engagement: a pointerdown whose - * held-modifier state doesn't match any filter is treated as if no - * callback were registered (no rectangle drawn, no message sent). + * ``event_type`` in the sending scope. A non-empty tuple enables them, + * and the client uses the filter list to gate gesture engagement: a + * pointerdown whose held-modifier state doesn't match any filter is + * treated as if no callback were registered (no rectangle drawn, no + * message sent). + * + * Filters are kept per ``owner`` on the client and engagement uses the + * union across owners, so the broadcast scope and a client scope can + * register pointer callbacks independently -- one scope clearing its + * filters never deactivates the other's. * * (automatically generated) */ @@ -1293,6 +1359,7 @@ export interface ScenePointerEnableMessage { | "cmd/ctrl+alt+shift" | null )[]; + owner: string; } /** Fog message. * @@ -1350,6 +1417,7 @@ export interface SetBoneOrientationMessage { name: string; bone_index: number; wxyz: [number, number, number, number]; + owner: string; } /** Server -> client message to set a skinned mesh bone's position. * @@ -1362,6 +1430,7 @@ export interface SetBonePositionMessage { name: string; bone_index: number; position: [number, number, number]; + owner: string; } /** Server -> client message to set the camera's position. * @@ -1450,6 +1519,7 @@ export interface SetOrientationMessage { type: "SetOrientationMessage"; name: string; wxyz: [number, number, number, number]; + owner: string; } /** Server -> client message to set a scene node's position. * @@ -1461,6 +1531,7 @@ export interface SetPositionMessage { type: "SetPositionMessage"; name: string; position: [number, number, number]; + owner: string; } /** Client -> server message when a transform control is updated. * @@ -1473,6 +1544,7 @@ export interface TransformControlsUpdateMessage { name: string; wxyz: [number, number, number, number]; position: [number, number, number]; + owner: string; } /** Client -> server message when a transform control drag starts. * @@ -1481,6 +1553,7 @@ export interface TransformControlsUpdateMessage { export interface TransformControlsDragStartMessage { type: "TransformControlsDragStartMessage"; name: string; + owner: string; } /** Client -> server message when a transform control drag ends. * @@ -1489,6 +1562,7 @@ export interface TransformControlsDragStartMessage { export interface TransformControlsDragEndMessage { type: "TransformControlsDragEndMessage"; name: string; + owner: string; } /** Message for rendering a background image. * @@ -1508,6 +1582,7 @@ export interface SetSceneNodeVisibilityMessage { type: "SetSceneNodeVisibilityMessage"; name: string; visible: boolean; + owner: string; } /** Declare the drag-input combinations a scene node listens for. * @@ -1537,6 +1612,7 @@ export interface SetSceneNodeDragBindingsMessage { | "cmd/ctrl+alt+shift" | null; }[]; + owner: string; } /** Declare the click-input combinations a scene node listens for. * @@ -1566,6 +1642,7 @@ export interface SetSceneNodeClickBindingsMessage { | "cmd/ctrl+alt+shift" | null; }[]; + owner: string; } /** Message for clicked objects. * @@ -1587,6 +1664,7 @@ export interface SceneNodeClickMessage { | "alt+shift" | "cmd/ctrl+alt+shift" | null; + owner: string; } /** Client -> server message for a scene-node drag (start/update/end). * @@ -1616,6 +1694,7 @@ export interface SceneNodeDragMessage { | "alt+shift" | "cmd/ctrl+alt+shift" | null; + owner: string; } /** Reset GUI. * @@ -1779,6 +1858,7 @@ export interface SceneNodeUpdateMessage { type: "SceneNodeUpdateMessage"; name: string; updates: { [key: string]: any }; + owner: string; } /** Message from server->client to configure parts of the GUI. * diff --git a/src/viser/client/src/batchedSceneUpdates.ts b/src/viser/client/src/batchedSceneUpdates.ts new file mode 100644 index 000000000..62d1a721d --- /dev/null +++ b/src/viser/client/src/batchedSceneUpdates.ts @@ -0,0 +1,164 @@ +import { + createSceneTreeActions, + SceneNode, + variantKey, +} from "./SceneTreeState"; +import { KeyedStore } from "./store"; +import { SceneNodeMessage } from "./WebsocketMessages"; + +type SceneTreeActions = ReturnType; + +/** Per-batch parking tables for scene-node updates. + * + * Attribute and props updates are parked per (owner, name) during a message + * batch and applied in merged form at flush, so a 60Hz stream costs one + * store write per batch instead of one per message. Parked entries are + * RE-ROUTED through `routeShadowedUpdate` when they land: a cross-scope add + * later in the SAME batch can flip a name's effective variant after an + * update was parked, and flushing by name alone would then write one + * scope's update onto the other scope's variant. + * + * Wire-order invariant: an add/remove is applied at receive time while + * updates park until flush, so the caller MUST `drainFor(name)` before + * applying any message that can flip `name`'s variant topology. Without the + * drain, a pre-flip parked update would flush AFTER post-flip updates that + * were consumed into the shadow slot at receive time, overwriting newer + * state with older state. + */ +export interface ParkedSceneUpdates { + parkAttr(owner: string, name: string, updates: Partial): void; + parkProps(owner: string, name: string, updates: { [key: string]: any }): void; + /** Apply and clear all parked entries for `name` immediately, before a + * topology-flipping message (add / variant remove) for it is handled. */ + drainFor(name: string): void; + /** Apply all remaining parked entries. Returns the merged store updates + * (for a single set()) and the names whose visibility actually merged + * into an effective variant (whose effective visibility must be + * recomputed after the set()). */ + flush(): { + mergedUpdates: { [name: string]: SceneNode }; + visibilityNames: string[]; + }; +} + +export function createParkedSceneUpdates( + store: KeyedStore, + actions: SceneTreeActions, +): ParkedSceneUpdates { + const attrUpdates: { + [ownerAndName: string]: { + name: string; + owner: string; + updates: Partial; + }; + } = {}; + const propsUpdates: { + [ownerAndName: string]: { + name: string; + owner: string; + updates: { [key: string]: any }; + }; + } = {}; + // Names with at least one parked entry, so the per-add/remove drainFor + // call is O(1) in the common case (nothing parked for that name) instead + // of scanning both tables. Never pruned on drain; a stale member only + // costs one extra scan. + const parkedNames = new Set(); + + /** Route one attr entry to the shadow slot, or merge it into `out`. + * Returns true when a visibility change merged into the effective + * variant. */ + function applyAttr( + entry: { name: string; owner: string; updates: Partial }, + out: { [name: string]: SceneNode }, + ): boolean { + if (actions.routeShadowedUpdate(entry.name, entry.owner, entry.updates)) + return false; + const currentNode = out[entry.name] ?? store.get(entry.name); + if (currentNode === undefined) { + console.log(`(OK) Tried to update non-existent scene node ${entry.name}`); + return false; + } + out[entry.name] = { ...currentNode, ...entry.updates }; + return "visibility" in entry.updates; + } + + function applyProps( + entry: { name: string; owner: string; updates: { [key: string]: any } }, + out: { [name: string]: SceneNode }, + ): void { + if ( + actions.routeShadowedUpdate(entry.name, entry.owner, { + propsUpdates: entry.updates, + }) + ) + return; + const currentNode = out[entry.name] ?? store.get(entry.name); + if (currentNode === undefined) { + console.log(`(OK) Tried to update non-existent scene node ${entry.name}`); + return; + } + out[entry.name] = { + ...currentNode, + message: { + ...currentNode.message, + props: { + ...currentNode.message.props, + ...entry.updates, + }, + } as SceneNodeMessage, + }; + } + + return { + parkAttr: (owner, name, updates) => { + parkedNames.add(name); + const entry = (attrUpdates[variantKey(owner, name)] ??= { + name, + owner, + updates: {}, + }); + Object.assign(entry.updates, updates); + }, + parkProps: (owner, name, updates) => { + parkedNames.add(name); + const entry = (propsUpdates[variantKey(owner, name)] ??= { + name, + owner, + updates: {}, + }); + Object.assign(entry.updates, updates); + }, + drainFor: (name) => { + if (!parkedNames.has(name)) return; + // Rare path (a cross-scope flip mid-batch): immediate store writes + // are fine, and required -- the topology change lands right after. + for (const [key, entry] of Object.entries(attrUpdates)) { + if (entry.name !== name) continue; + delete attrUpdates[key]; + const out: { [n: string]: SceneNode } = {}; + const recompute = applyAttr(entry, out); + if (Object.keys(out).length > 0) store.set(out); + if (recompute) actions.computeEffectiveVisibility(name); + } + for (const [key, entry] of Object.entries(propsUpdates)) { + if (entry.name !== name) continue; + delete propsUpdates[key]; + const out: { [n: string]: SceneNode } = {}; + applyProps(entry, out); + if (Object.keys(out).length > 0) store.set(out); + } + }, + flush: () => { + const mergedUpdates: { [name: string]: SceneNode } = {}; + const visibilityNames: string[] = []; + for (const entry of Object.values(attrUpdates)) { + if (applyAttr(entry, mergedUpdates)) visibilityNames.push(entry.name); + } + for (const entry of Object.values(propsUpdates)) { + applyProps(entry, mergedUpdates); + } + return { mergedUpdates, visibilityNames }; + }, + }; +} diff --git a/src/viser/client/src/dragUtils.ts b/src/viser/client/src/dragUtils.ts index a88c15e4d..3f218a377 100644 --- a/src/viser/client/src/dragUtils.ts +++ b/src/viser/client/src/dragUtils.ts @@ -216,6 +216,13 @@ export function motionExceedsThreshold( * are updated in place on every pointermove. */ export type ActiveDragState = { nodeName: string; + /** Owner of the effective variant this drag started on, frozen at + * drag-start. Every phase of the drag echoes THIS owner: re-deriving it + * from the live store would misroute the final ``end`` when the node is + * removed (ownerOf(undefined) is the broadcast "") or when the effective + * variant flips mid-drag -- the scope that received ``start`` must be + * the one that receives ``end``. */ + owner: string; /** Frozen at drag-start. Non-null for batched scene nodes (meshes, * GLBs, axes); ``null`` otherwise. */ instanceIndex: number | null; diff --git a/src/viser/client/src/mesh/SkinnedMesh.tsx b/src/viser/client/src/mesh/SkinnedMesh.tsx index d909ac769..a59adfcf3 100644 --- a/src/viser/client/src/mesh/SkinnedMesh.tsx +++ b/src/viser/client/src/mesh/SkinnedMesh.tsx @@ -3,7 +3,7 @@ import * as THREE from "three"; import { ViserStandardMeshMaterial, ShadowSkinnedMesh } from "./MeshUtils"; import { SkinnedMeshMessage } from "../WebsocketMessages"; import { OutlinesIfHovered } from "../OutlinesIfHovered"; -import { ViewerContext, ViewerMutable } from "../ViewerContext"; +import { ViewerContext, ViewerMutable, variantKey } from "../ViewerContext"; import { useFrame } from "@react-three/fiber"; import { normalizeScale } from "../utils/normalizeScale"; @@ -112,12 +112,17 @@ export const SkinnedMesh = React.forwardRef< // Get mutable once. const viewerMutable = viewer.mutable.current; + // Bone state is keyed per VARIANT: this mounted instance renders exactly + // one scope's variant, and must never read/claim the entry of a same-name + // variant from the other scope. + const stateKey = variantKey(message.owner, message.name); + // Clean up geometry and skeleton when they change (they're created together). React.useEffect(() => { // The state entry can be deleted while this component is still mounted // (subtree-prefix removal in MessageHandler, reconnect clearing in // WebsocketInterface), so guard reads like the bone-message handlers do. - const state = viewerMutable.skinnedMeshState[message.name]; + const state = viewerMutable.skinnedMeshState[stateKey]; if (state !== undefined) { state.initialized = false; state.claimed = true; @@ -137,14 +142,14 @@ export const SkinnedMesh = React.forwardRef< // run, or a new mount) can take the entry over. if ( ownedEntryRef.current !== null && - viewerMutable.skinnedMeshState[message.name] === ownedEntryRef.current + viewerMutable.skinnedMeshState[stateKey] === ownedEntryRef.current ) { ownedEntryRef.current.claimed = false; } if (skeleton) skeleton.dispose(); if (geometry) geometry.dispose(); }; - }, [skeleton, geometry, message.name, viewerMutable.skinnedMeshState]); + }, [skeleton, geometry, stateKey, viewerMutable.skinnedMeshState]); // Check if we should render a shadow mesh. const shadowOpacity = @@ -159,7 +164,7 @@ export const SkinnedMesh = React.forwardRef< // useFrame (priority -100000) earlier in the same rAF tick, while this // subscriber is still registered. R3F's subscriber loop has no try/catch, // so throwing here would skip the remaining subscribers and gl.render. - const state = viewerMutable.skinnedMeshState[message.name]; + const state = viewerMutable.skinnedMeshState[stateKey]; if (state === undefined) return; // Only one live instance may drive an entry. Normally the init effect // claims it; but FilePlayback can recreate the entry WITHOUT a remount diff --git a/src/viser/client/src/pointer/gestures.ts b/src/viser/client/src/pointer/gestures.ts index 4064aabb0..6b6338348 100644 --- a/src/viser/client/src/pointer/gestures.ts +++ b/src/viser/client/src/pointer/gestures.ts @@ -50,10 +50,26 @@ const IDLE: CanvasGesture = { kind: "idle" }; export class ScenePointerController { private gesture: CanvasGesture = IDLE; + /** Modifier filters per event type, kept PER OWNER (broadcast scope "" + * plus this connection's client scope): both scopes may register scene + * pointer callbacks independently, and one scope disabling its filters + * must not deactivate the other's. Gesture engagement uses the union + * across owners; the server dispatches one ScenePointerMessage to every + * scope, each of which matches against its own registrations. */ private readonly filters = new Map< ScenePointerEventType, - (KeyModifier | null)[] + Map >(); + + private unionFilter( + eventType: ScenePointerEventType, + ): (KeyModifier | null)[] | undefined { + const byOwner = this.filters.get(eventType); + if (byOwner === undefined) return undefined; + const out: (KeyModifier | null)[] = []; + for (const modifiers of byOwner.values()) out.push(...modifiers); + return out; + } /** Cleanup for window-level pointerup/pointercancel listeners * installed while a gesture is engaged. Null when idle. The * listeners catch releases that happen off the canvas -- a @@ -106,23 +122,35 @@ export class ScenePointerController { applyFiltersDelta( eventType: ScenePointerEventType, + owner: string, modifiers: readonly (KeyModifier | null)[], ): void { - if (modifiers.length === 0) this.filters.delete(eventType); - else this.filters.set(eventType, [...modifiers]); + const byOwner = this.filters.get(eventType); + if (modifiers.length === 0) { + if (byOwner !== undefined) { + byOwner.delete(owner); + if (byOwner.size === 0) this.filters.delete(eventType); + } + } else if (byOwner === undefined) { + this.filters.set(eventType, new Map([[owner, [...modifiers]]])); + } else { + byOwner.set(owner, [...modifiers]); + } this.hover.refresh(); } getFilter( eventType: ScenePointerEventType, ): readonly (KeyModifier | null)[] | undefined { - return this.filters.get(eventType); + return this.unionFilter(eventType); } anyFilterMatches(modifier: KeyModifier | null): boolean { - for (const list of this.filters.values()) { - for (const f of list) { - if (matchesModifierFilter(modifier, f)) return true; + for (const byOwner of this.filters.values()) { + for (const list of byOwner.values()) { + for (const f of list) { + if (matchesModifierFilter(modifier, f)) return true; + } } } return false; @@ -143,7 +171,8 @@ export class ScenePointerController { const input: DragInput = { button, modifier: args.modifier }; const eligible = new Set(); - for (const [eventType, modifiers] of this.filters) { + for (const eventType of this.filters.keys()) { + const modifiers = this.unionFilter(eventType)!; if ( button === "left" && modifiers.some((m) => matchesModifierFilter(args.modifier, m)) @@ -262,11 +291,21 @@ export class ScenePointerController { this.removeWindowListeners(); } - resetForTest(): void { - this.cancelAny(); + /** Drop every scope's filters. Called on (re)connect: owner ids are + * connection-scoped (a reconnected browser is a NEW client id), so any + * surviving per-owner entry is unreachable garbage that would keep its + * event type permanently engaged -- no disable for that owner can ever + * arrive. Broadcast-scope enables replay from the persistent buffer + * right after. */ + clearFilters(): void { this.filters.clear(); this.hover.refresh(); } + + resetForTest(): void { + this.cancelAny(); + this.clearFilters(); + } } type NodeCandidate = { diff --git a/src/viser/infra/_async_message_buffer.py b/src/viser/infra/_async_message_buffer.py index bc29c4c84..851cb8091 100644 --- a/src/viser/infra/_async_message_buffer.py +++ b/src/viser/infra/_async_message_buffer.py @@ -1,10 +1,11 @@ from __future__ import annotations import asyncio +import contextlib import dataclasses import threading from asyncio.events import AbstractEventLoop -from typing import AsyncGenerator, Callable, Dict, List, Sequence +from typing import AsyncGenerator, Callable, Dict, Generator, List, Sequence from ._messages import Message @@ -31,6 +32,12 @@ class AsyncMessageBuffer: window_duration_sec: float = 1.0 / 60.0 done: bool = False atomic_counter: int = 0 + _warned_push_after_done: bool = False + """One-shot latch for the dead-connection write warning in push().""" + + _sanctioned_dead_writes: int = 0 + """Nesting depth of ``sanctioned_dead_writes()`` scopes; nonzero + suppresses the dead-connection write warning.""" generator_cursors: Dict[int, int] = dataclasses.field(default_factory=dict) """Per-active-connection consumption cursors (client id -> last message id @@ -41,6 +48,24 @@ class AsyncMessageBuffer: pending?" event is not a consumption watermark, since a backpressured client's cursor can sit arbitrarily far behind it.""" + @contextlib.contextmanager + def sanctioned_dead_writes(self) -> Generator[None, None, None]: + """Suppress the dead-connection write warning for pushes inside this + scope. For cleanup emits that removal paths perform on behalf of the + user (e.g. the empty interaction-bindings broadcasts in a scene + node's ``remove()``): on a dead buffer they are benign no-ops, same + as the removal messages themselves. The depth is adjusted under + ``buffer_lock`` so concurrent scopes can't lose an update and wedge + the counter; a concurrent unsanctioned push slipping through + unwarned is still acceptable for a best-effort diagnostic.""" + with self.buffer_lock: + self._sanctioned_dead_writes += 1 + try: + yield + finally: + with self.buffer_lock: + self._sanctioned_dead_writes -= 1 + def remove_from_buffer(self, match_fn: Callable[[Message], bool]) -> None: """Remove messages that match some condition.""" @@ -58,6 +83,31 @@ def push(self, message: Message) -> None: assert isinstance(message, Message) + # A done buffer has no producer left to drain it: for a per-client + # buffer this means the client disconnected, and anything pushed via + # a stale handle silently accumulates forever. Warn ONCE per buffer + # so the dead-handle write is diagnosable without spamming loops + # that keep animating a departed client's elements. Removal messages + # are exempt: releasing elements of a departed client (e.g. inside + # on_client_disconnect, which runs after the buffer is closed) is + # ordinary cleanup, and a remove on a dead connection is a benign + # no-op rather than a leak in the making. + if ( + self.done + and not self._warned_push_after_done + and message.lifecycle_phase != "remove" + and self._sanctioned_dead_writes == 0 + ): + self._warned_push_after_done = True + import warnings + + warnings.warn( + f"Queued a {type(message).__name__} on a closed connection " + "(e.g. via a handle owned by a disconnected client, or after " + "the server stopped); it will never be delivered.", + stacklevel=4, + ) + # Add message to buffer. redundancy_key = message.redundancy_key() diff --git a/src/viser/infra/_messages.py b/src/viser/infra/_messages.py index 45a40aabc..e25b03f10 100644 --- a/src/viser/infra/_messages.py +++ b/src/viser/infra/_messages.py @@ -14,6 +14,7 @@ Dict, List, Optional, + Tuple, Type, TypeVar, Union, @@ -170,6 +171,25 @@ def get_type_hints_cached(cls: Type[Any]) -> Dict[str, Any]: return get_type_hints(cls) # type: ignore +@functools.lru_cache(maxsize=None) +def wire_field_names(cls: Type[Any]) -> Tuple[str, ...]: + """Names of the fields a message type puts on the wire: declared, + type-hinted dataclass fields, in declaration order. THE single + definition of the wire field set -- the serializer and the TypeScript + interface generator must never disagree on it. Declared fields (not + ``vars()``) so that non-init defaulted fields (e.g. the scene messages' + owner/virtual stamps) are included even before first assignment.""" + hints = get_type_hints_cached(cls) + return tuple(f.name for f in dataclasses.fields(cls) if f.name in hints) + + +@functools.lru_cache(maxsize=None) +def _non_init_field_names(cls: Type[Any]) -> Tuple[str, ...]: + """Dataclass fields excluded from ``__init__`` (assigned post-construction + on deserialization). Empty for most message types.""" + return tuple(f.name for f in dataclasses.fields(cls) if not f.init) + + class Message(abc.ABC): """Base message type for server/client communication.""" @@ -222,12 +242,11 @@ def as_serializable_dict( Otherwise, arrays are inlined as memoryviews in the returned dict.""" message_type = type(self) hints = get_type_hints_cached(message_type) - # Filter to type-hinted fields only -- excludes dynamic attributes - # like cached values that shouldn't be serialized. out = { - k: _prepare_for_serialization(v, hints[k], binary_buffers) - for k, v in vars(self).items() - if k in hints + name: _prepare_for_serialization( + getattr(self, name), hints[name], binary_buffers + ) + for name in wire_field_names(message_type) } out["type"] = message_type.__name__ return out @@ -254,7 +273,22 @@ def deserialize(cls, message: bytes) -> Message: # a blanket recursive traversal of the entire message tree. message_type = cls._subclass_from_type_string()[cast(str, mapping.pop("type"))] message_kwargs = message_type._from_serializable_dict(mapping) - return message_type(**message_kwargs) + # Non-init fields (e.g. the scene messages' owner/virtual stamps, + # declared init=False so defaulted fields can follow subclasses' + # positional ones on Python 3.8) can't be passed to __init__; strip + # them out and assign after construction so the serialize -> + # deserialize round trip stays lossless. The per-class name tuple is + # cached: most message types have none, and this runs per inbound + # message. + non_init = { + k: message_kwargs.pop(k) + for k in _non_init_field_names(message_type) + if k in message_kwargs + } + decoded = message_type(**message_kwargs) + for k, v in non_init.items(): + setattr(decoded, k, v) + return decoded @classmethod @functools.lru_cache(maxsize=100) diff --git a/src/viser/infra/_typescript_interface_gen.py b/src/viser/infra/_typescript_interface_gen.py index 9723eb5bb..f020dbe9f 100644 --- a/src/viser/infra/_typescript_interface_gen.py +++ b/src/viser/infra/_typescript_interface_gen.py @@ -21,7 +21,7 @@ except ImportError: LiteralAlt = Literal # type: ignore -from ._messages import Message +from ._messages import Message, wire_field_names _raw_type_mapping = { bool: "boolean", @@ -252,13 +252,10 @@ def generate_typescript_interfaces(message_cls: Type[Message]) -> str: out_lines.append(f"export interface {cls.__name__} " + "{") out_lines.append(f' type: "{cls.__name__}";') - field_names = set([f.name for f in dataclasses.fields(cls)]) # type: ignore - for name, typ in get_type_hints(cls, include_extras=True).items(): - if name in field_names: - typ = _get_ts_type(typ) - else: - continue - out_lines.append(f" {name}: {typ};") + # Same field set (and order) the serializer puts on the wire. + hints = get_type_hints(cls, include_extras=True) + for name in wire_field_names(cls): + out_lines.append(f" {name}: {_get_ts_type(hints[name])};") out_lines.append("}") out_lines.append("") diff --git a/tests/e2e/assets/pre_scope_recording.viser b/tests/e2e/assets/pre_scope_recording.viser new file mode 100644 index 000000000..26188255c Binary files /dev/null and b/tests/e2e/assets/pre_scope_recording.viser differ diff --git a/tests/e2e/test_cross_scope_handles.py b/tests/e2e/test_cross_scope_handles.py new file mode 100644 index 000000000..845b8def1 --- /dev/null +++ b/tests/e2e/test_cross_scope_handles.py @@ -0,0 +1,778 @@ +"""E2E tests for cross-scope (server vs. client handle) scene/GUI semantics. + +Scene and GUI elements can be created through two scopes: ``server.scene`` / +``server.gui`` (broadcast, persistent) and ``client.scene`` / ``client.gui`` +(one connection, ephemeral). Scene nodes are identified by (owner, name): +each scene-tree name holds at most one variant per scope, fed independently +by its scope's messages, and the client renders the effective variant chosen +by the display rule (real client > real broadcast > virtual client > virtual +broadcast). + +This suite pins that seam from both directions: + +- **Contract tests** lock in cross-scope behavior that any future redesign + must preserve: per-client namespace isolation, the ephemeral lifecycle of + client-scoped elements across reconnects, and scene pointer callback + coexistence (both scopes may register; a gesture matching both scopes' + filters fires both scopes' callbacks; one scope's disable never + deactivates the other's). + +- **Variant/shadowing tests** cover the display rule end to end: a client + variant shadows the server's and un-shadows with the server's LATEST + state, clicks dispatch to exactly the effective variant's scope, removal + cascades are scope-local (client children survive broadcast parent + removal, anchored at the parent's frozen pose), and virtual anchors never + shadow real nodes. Fast Python-side coverage of the same rules lives in + ``tests/test_scene_scopes.py``; the frontend store logic is unit-tested in + ``src/viser/client/src/SceneTreeState.test.ts``. +""" + +from __future__ import annotations + +import threading +import time +from typing import Generator + +import pytest +from playwright.sync_api import Browser, Page + +import viser +import viser._client_autobuild + +from .utils import ( + canvas_center, + find_free_port, + get_client_handle, + wait_for_connection, + wait_for_scene_node, + wait_for_scene_node_hidden, + wait_for_scene_node_removed, + wait_for_scene_node_visible, + wait_for_server_ready, +) + +JS_GET_EFFECTIVE_OWNER = """ +(nodeName) => { + const tree = window.__viserSceneTree; + if (!tree) return null; + const node = tree.getState()[nodeName]; + if (!node) return null; + return node.message.owner ?? ""; +} +""" + + +def wait_for_node_position( + page: Page, + node_name: str, + position: tuple[float, float, float], + timeout: int = 10_000, +) -> None: + """Wait until a node's latest wire pose matches ``position``. + + Observes ``nodePoseData`` (written synchronously when the message is + handled), NOT the mounted three.js object: the object's position is + applied by a ``useFrame`` hook and needs requestAnimationFrame ticks, + which stall for seconds under CI's software-GL + xdist contention (the + only repeated CI failures in this suite were exactly that stall). The + applier path is covered by the visual/pixel tests.""" + wait_for_scene_node(page, node_name, timeout=timeout) # Delivery proof. + page.wait_for_function( + """([nodeName, expected]) => { + const pose = window.__viserMutable?.nodePoseData?.[nodeName]; + if (!pose) return false; + const p = pose.position; + return ( + Math.abs(p[0] - expected[0]) < 1e-4 && + Math.abs(p[1] - expected[1]) < 1e-4 && + Math.abs(p[2] - expected[2]) < 1e-4 + ); + }""", + arg=[node_name, list(position)], + timeout=timeout, + ) + + +@pytest.fixture() +def two_client_setup(browser: Browser) -> Generator[dict, None, None]: + """A viser server with two connected pages and their client handles.""" + viser._client_autobuild.ensure_client_is_built = lambda: None + + max_retries = 3 + server: viser.ViserServer | None = None + for attempt in range(max_retries): + port = find_free_port() + try: + server = viser.ViserServer(port=port, verbose=False) + break + except OSError: + if attempt == max_retries - 1: + raise + assert server is not None + wait_for_server_ready(server.get_port()) + + context1 = browser.new_context() + page1 = context1.new_page() + wait_for_connection(page1, server.get_port()) + client1 = get_client_handle(server, expected_count=1) + + context2 = browser.new_context() + page2 = context2.new_page() + wait_for_connection(page2, server.get_port()) + client2 = get_client_handle(server, expected_count=2) + assert client2.client_id != client1.client_id + + yield { + "server": server, + "page1": page1, + "page2": page2, + "client1": client1, + "client2": client2, + } + + for context in (context1, context2): + try: + context.close() + except Exception: + pass # A test may have closed it already (disconnect tests). + server.stop() + + +# --------------------------------------------------------------------------- +# Contract tests: cross-scope behavior that must be preserved. +# --------------------------------------------------------------------------- + + +def test_per_client_namespaces_are_isolated(two_client_setup: dict) -> None: + """Elements added via one client handle must not appear for other clients.""" + page1: Page = two_client_setup["page1"] + page2: Page = two_client_setup["page2"] + client1: viser.ClientHandle = two_client_setup["client1"] + client2: viser.ClientHandle = two_client_setup["client2"] + + client1.scene.add_icosphere("/only_c1", radius=0.3) + client2.scene.add_icosphere("/only_c2", radius=0.3) + + wait_for_scene_node(page1, "/only_c1") + wait_for_scene_node(page2, "/only_c2") + + # Poll a little to let any (incorrect) cross-delivery land, then assert + # isolation. + time.sleep(0.5) + assert page1.evaluate( + "() => window.__viserSceneTree.getState()['/only_c2'] === undefined" + ) + assert page2.evaluate( + "() => window.__viserSceneTree.getState()['/only_c1'] === undefined" + ) + + +def test_same_name_coexists_across_different_clients(two_client_setup: dict) -> None: + """Two clients may each own a node with the SAME name, with independent + state.""" + page1: Page = two_client_setup["page1"] + page2: Page = two_client_setup["page2"] + client1: viser.ClientHandle = two_client_setup["client1"] + client2: viser.ClientHandle = two_client_setup["client2"] + + client1.scene.add_icosphere("/own", radius=0.3, position=(1.0, 0.0, 0.0)) + client2.scene.add_icosphere("/own", radius=0.3, position=(0.0, 2.0, 0.0)) + + wait_for_node_position(page1, "/own", (1.0, 0.0, 0.0)) + wait_for_node_position(page2, "/own", (0.0, 2.0, 0.0)) + + +def test_client_scope_elements_do_not_survive_reconnect( + viser_server: viser.ViserServer, viser_page: Page +) -> None: + """Server-scoped elements are replayed after a reconnect; client-scoped + elements are not (the per-client buffer is ephemeral and the reconnected + browser is a brand-new ClientHandle). This includes shadowing variants: + after a reconnect, the server's variant of a previously-shadowed name is + the one shown.""" + client = get_client_handle(viser_server) + + viser_server.scene.add_box( + "/shared_box", dimensions=(1.0, 1.0, 1.0), position=(2.0, 0.0, 0.0) + ) + client.scene.add_icosphere("/client_sphere", radius=0.3) + # A client variant shadowing a server name. Wait on the EFFECTIVE OWNER + # flipping to the client -- a (0, 0, 0) pose wait would pass vacuously on + # the store's freshly-initialized default pose before the shadowing add + # has even been processed. + client.scene.add_box("/shared_box", dimensions=(0.5, 0.5, 0.5)) + wait_for_scene_node(viser_page, "/client_sphere") + viser_page.wait_for_function( + f"() => ({JS_GET_EFFECTIVE_OWNER})('/shared_box') === '{client.client_id}'", + timeout=10_000, + ) + + viser_server._websock_server.disconnect_all_clients() + + # The frontend reconnects automatically, resets its stores, and replays + # the broadcast backlog: the client sphere is gone, and the server's + # variant of /shared_box (at ITS position) is effective again. + wait_for_scene_node_removed(viser_page, "/client_sphere") + wait_for_node_position(viser_page, "/shared_box", (2.0, 0.0, 0.0), timeout=15_000) + assert viser_page.evaluate(JS_GET_EFFECTIVE_OWNER, "/shared_box") == "" + + +def test_scene_pointer_callbacks_coexist_across_scopes( + viser_server: viser.ViserServer, viser_page: Page +) -> None: + """Scene pointer callbacks coexist across scopes: filters are kept per + owner on the frontend and gesture engagement uses the union, so both + scopes' registrations fire on one physical click and one scope's + disable never deactivates the other's. (This replaced the interim + cross-scope exclusivity rule.)""" + client = get_client_handle(viser_server) + + server_clicked = threading.Event() + client_clicked = threading.Event() + + @client.scene.on_click() + def _(_event: viser.SceneClickEvent) -> None: + client_clicked.set() + + @viser_server.scene.on_click() + def _(_event: viser.SceneClickEvent) -> None: + server_clicked.set() + + # Registration in one scope leaves the other's callbacks alone. + assert len(viser_server.scene._scene_pointer_cb) == 1 + assert len(client.scene._scene_pointer_cb) == 1 + + time.sleep(0.5) # Let both enable messages reach the frontend. + cx, cy = canvas_center(viser_page) + viser_page.mouse.move(cx, cy) + viser_page.mouse.down() + viser_page.mouse.up() + + # One physical click, both scopes' callbacks. + assert server_clicked.wait(5.0), "server-scope scene click did not fire" + assert client_clicked.wait(5.0), "client-scope scene click did not fire" + + # One scope disabling its filters must not deactivate the other's: after + # the client scope clears, a click still reaches the server callback. + client.scene.remove_click_callback() + server_clicked.clear() + client_clicked.clear() + time.sleep(0.5) # Let the disable reach the frontend. + viser_page.mouse.move(cx, cy) + viser_page.mouse.down() + viser_page.mouse.up() + assert server_clicked.wait(5.0), ( + "server-scope scene click stopped firing after the client scope " + "cleared its own filters" + ) + time.sleep(0.3) + assert not client_clicked.is_set() + + +def test_gui_container_nesting_is_directional(two_client_setup: dict) -> None: + """A ``with server.gui.add_folder(...)`` block CAN capture elements added + through a client handle's GuiApi: the client element renders inside the + shared folder for that client only, and dies with the folder. The + reverse (server element into a client container) raises instead of + silently landing the element at the other scope's root (the historical + behavior).""" + server = two_client_setup["server"] + page1: Page = two_client_setup["page1"] + page2: Page = two_client_setup["page2"] + client1 = two_client_setup["client1"] + + with server.gui.add_folder("SrvFolder") as folder: + client1.gui.add_button("MineBtn") + + # Client 1 sees its button inside the shared folder; client 2 sees the + # folder but no button. + button1 = page1.get_by_role("button", name="MineBtn") + button1.wait_for(state="visible", timeout=5_000) + page2.get_by_text("SrvFolder").wait_for(state="visible", timeout=5_000) + assert page2.get_by_role("button", name="MineBtn").count() == 0 + + # DOM containment, not just coexistence: collapsing the folder must hide + # the client's button; expanding brings it back. + page1.get_by_text("SrvFolder").click() + button1.wait_for(state="hidden", timeout=5_000) + page1.get_by_text("SrvFolder").click() + button1.wait_for(state="visible", timeout=5_000) + + # Removing the server folder cascades into the cross-nested client + # element (the one deliberate exception to scope-local removal). + folder.remove() + button1.wait_for(state="hidden", timeout=5_000) + + # Outside any server container context, client adds work normally... + client1.gui.add_button("OkBtn") + page1.get_by_role("button", name="OkBtn").wait_for(state="visible", timeout=5_000) + + # ...and the reverse nesting direction raises. + with client1.gui.add_folder("CliFolder"): + with pytest.raises(RuntimeError, match="not vice versa"): + server.gui.add_button("StrayBtn") + + +def test_disconnect_detaches_cross_nested_gui_elements( + two_client_setup: dict, +) -> None: + """A real websocket disconnect runs the cross-scope release hook: the + departed client's elements are detached from the server's container + tree, so removing the server folder afterwards is warning-free and + still propagates to the remaining client.""" + import warnings as warnings_module + + server = two_client_setup["server"] + page1: Page = two_client_setup["page1"] + page2: Page = two_client_setup["page2"] + client1 = two_client_setup["client1"] + + with server.gui.add_folder("SrvFolder") as folder: + client1.gui.add_button("MineBtn") + page1.get_by_role("button", name="MineBtn").wait_for(state="visible", timeout=5_000) + page2.get_by_text("SrvFolder").wait_for(state="visible", timeout=5_000) + + # Disconnect client 1 for real and wait for the server-side teardown. + page1.context.close() + deadline = time.time() + 5.0 + while client1.client_id in server._connected_clients: + assert time.time() < deadline, "client 1 never disconnected" + time.sleep(0.05) + + # The folder's cross-nested child was detached bookkeeping-only, so this + # removal must not try to message the dead connection. + with warnings_module.catch_warnings(record=True) as caught: + warnings_module.simplefilter("always") + folder.remove() + assert not any("closed connection" in str(w.message) for w in caught) + page2.get_by_text("SrvFolder").wait_for(state="hidden", timeout=5_000) + + +# --------------------------------------------------------------------------- +# Variant slots + display rule (shadowing). +# --------------------------------------------------------------------------- + + +def test_client_variant_shadows_and_unshadows_with_latest_state( + viser_server: viser.ViserServer, viser_page: Page +) -> None: + """The core shadowing round trip: a client-scoped add of a server-owned + name shadows it for that client; server updates keep accumulating in the + hidden variant; removing the client variant promotes the server's node + with its LATEST state -- no resurrection round trip.""" + client = get_client_handle(viser_server) + + viser_server.scene.add_icosphere("/dup", radius=0.3, position=(1.0, 2.0, 0.0)) + wait_for_node_position(viser_page, "/dup", (1.0, 2.0, 0.0)) + + # Client-scoped variant at the origin: shadows the server's node. + client_variant = client.scene.add_icosphere( + "/dup", radius=0.3, position=(0.0, 0.0, 0.0) + ) + wait_for_node_position(viser_page, "/dup", (0.0, 0.0, 0.0)) + assert viser_page.evaluate(JS_GET_EFFECTIVE_OWNER, "/dup") == str(client.client_id) + + # Server keeps writing while shadowed; the display doesn't budge. + viser_server.scene._handle_from_node_name["/dup"].position = (3.0, 3.0, 0.0) + time.sleep(0.5) + wait_for_node_position(viser_page, "/dup", (0.0, 0.0, 0.0)) + + # Un-shadow: the server's variant shows again, at its LATEST position. + client_variant.remove() + wait_for_node_position(viser_page, "/dup", (3.0, 3.0, 0.0)) + assert viser_page.evaluate(JS_GET_EFFECTIVE_OWNER, "/dup") == "" + + +def test_click_dispatches_to_effective_variant_only( + viser_server: viser.ViserServer, viser_page: Page +) -> None: + """A click on a shadowing client variant reaches the client scope's + callback and never the shadowed server variant's.""" + viser_server.initial_camera.position = (0.0, 0.0, 4.0) + viser_server.initial_camera.look_at = (0.0, 0.0, 0.0) + client = get_client_handle(viser_server) + + server_clicked = threading.Event() + client_clicks: list[int] = [] + client_clicked = threading.Event() + + server_box = viser_server.scene.add_box( + "/dup_click", dimensions=(4.0, 4.0, 0.2), color=(200, 60, 60) + ) + + @server_box.on_click + def _(_event: viser.SceneNodePointerEvent[viser.BoxHandle]) -> None: + server_clicked.set() + + client_box = client.scene.add_box( + "/dup_click", dimensions=(4.0, 4.0, 0.2), color=(60, 200, 60) + ) + + @client_box.on_click + def _(_event: viser.SceneNodePointerEvent[viser.BoxHandle]) -> None: + client_clicks.append(1) + client_clicked.set() + + wait_for_scene_node(viser_page, "/dup_click") + # Wait until the client-scoped variant is the effective one (non-empty + # owner) before clicking. + viser_page.wait_for_function( + """(nodeName) => { + const tree = window.__viserSceneTree; + const node = tree && tree.getState()[nodeName]; + return node !== undefined && (node.message.owner ?? "") !== ""; + }""", + arg="/dup_click", + timeout=5_000, + ) + time.sleep(0.5) # Let click bindings reach the frontend. + + cx, cy = canvas_center(viser_page) + viser_page.mouse.move(cx, cy) + viser_page.mouse.down() + viser_page.mouse.up() + + assert client_clicked.wait(5.0), "click did not reach the client-scoped handle" + time.sleep(0.5) + assert not server_clicked.is_set(), ( + "click dispatched to the shadowed server-scoped handle too" + ) + assert len(client_clicks) == 1 + + +def test_scope_local_cascade_client_child_survives( + viser_server: viser.ViserServer, viser_page: Page +) -> None: + """Removing a broadcast parent removes only broadcast descendants: a + client-scoped child survives, hanging from its own scope's virtual + anchor, which inherits the departing parent's pose (children don't + teleport).""" + client = get_client_handle(viser_server) + + parent = viser_server.scene.add_frame( + "/parent", show_axes=False, position=(1.0, 0.0, 1.0) + ) + child = client.scene.add_icosphere("/parent/child", radius=0.3) + wait_for_scene_node(viser_page, "/parent/child") + + parent.remove() + + # The child's entry survives on the frontend... + time.sleep(0.5) + wait_for_scene_node(viser_page, "/parent/child") + # ...hanging from the client's promoted virtual anchor... + assert viser_page.evaluate(JS_GET_EFFECTIVE_OWNER, "/parent") == str( + client.client_id + ) + # ...which inherited the departed parent's pose (frozen-pose + # inheritance -- the child stays where it was). + viser_page.wait_for_function( + """(expected) => { + const m = window.__viserMutable; + const pose = m && m.nodePoseData && m.nodePoseData["/parent"]; + if (!pose) return false; + return ( + Math.abs(pose.position[0] - expected[0]) < 1e-4 && + Math.abs(pose.position[1] - expected[1]) < 1e-4 && + Math.abs(pose.position[2] - expected[2]) < 1e-4 + ); + }""", + arg=[1.0, 0.0, 1.0], + timeout=5_000, + ) + # The Python handle is still live; the author removes it explicitly. + assert not child._impl.removed + child.remove() + wait_for_scene_node_removed(viser_page, "/parent/child") + + +def test_virtual_anchor_does_not_shadow_real_broadcast_node( + viser_server: viser.ViserServer, viser_page: Page +) -> None: + """A client's auto-created ancestor anchor must not hide the server's + real node of the same name.""" + client = get_client_handle(viser_server) + + viser_server.scene.add_frame("/anchor_parent", show_axes=True) + wait_for_scene_node(viser_page, "/anchor_parent") + + # Deep client add auto-creates client-scoped anchors for /anchor_parent. + client.scene.add_icosphere("/anchor_parent/mine", radius=0.3) + wait_for_scene_node(viser_page, "/anchor_parent/mine") + + # The server's real frame is still the effective variant. + assert viser_page.evaluate(JS_GET_EFFECTIVE_OWNER, "/anchor_parent") == "" + + +def test_broadcast_child_under_client_parent_coexists( + viser_server: viser.ViserServer, viser_page: Page +) -> None: + """A server add under a client-owned name is legal: the server's chain + hangs from its own virtual anchor, which is shadowed by the client's + real node on this client.""" + client = get_client_handle(viser_server) + + client.scene.add_frame("/cp", show_axes=False) + wait_for_scene_node(viser_page, "/cp") + + viser_server.scene.add_icosphere("/cp/child", radius=0.3) + wait_for_scene_node(viser_page, "/cp/child") + + # The client's real frame stays effective over the server's virtual + # anchor for /cp. + assert viser_page.evaluate(JS_GET_EFFECTIVE_OWNER, "/cp") == str(client.client_id) + assert viser_page.evaluate(JS_GET_EFFECTIVE_OWNER, "/cp/child") == "" + + +# --------------------------------------------------------------------------- +# World axes. +# --------------------------------------------------------------------------- + + +def test_world_axes_client_shadow_override( + viser_server: viser.ViserServer, viser_page: Page +) -> None: + """The sanctioned per-client world-axes override: a client-scoped + "/WorldAxes" frame shadows the server's, and removing it restores the + server's state. (client.scene.world_axes itself raises AttributeError.)""" + client = get_client_handle(viser_server) + with pytest.raises(AttributeError, match="server.scene.world_axes"): + _ = client.scene.world_axes + + viser_server.scene.world_axes.visible = True + wait_for_scene_node_visible(viser_page, "/WorldAxes") + + override = client.scene.add_frame("/WorldAxes", show_axes=True, visible=False) + wait_for_scene_node_hidden(viser_page, "/WorldAxes") + + override.remove() + wait_for_scene_node_visible(viser_page, "/WorldAxes") + + +def test_world_axes_server_state_deterministic_for_new_client( + browser: Browser, +) -> None: + """A client connecting after the server showed the world axes sees them + visible. (Previously each ClientHandle re-added /WorldAxes with + visible=False over the per-client connection, racing the broadcast + replay -- the axes' state at connect was nondeterministic.)""" + viser._client_autobuild.ensure_client_is_built = lambda: None + + max_retries = 3 + server: viser.ViserServer | None = None + for attempt in range(max_retries): + port = find_free_port() + try: + server = viser.ViserServer(port=port, verbose=False) + break + except OSError: + if attempt == max_retries - 1: + raise + assert server is not None + wait_for_server_ready(server.get_port()) + + server.scene.world_axes.visible = True + + context = browser.new_context() + page = context.new_page() + wait_for_connection(page, server.get_port()) + wait_for_scene_node_visible(page, "/WorldAxes") + + context.close() + server.stop() + + +# --------------------------------------------------------------------------- +# Regression tests: owner routing + per-connection frontend state. +# --------------------------------------------------------------------------- + + +def _wait_drag_ready(page: Page) -> None: + """The drag raycasts against the RENDERED scene: wait for the mounted + mesh (store presence precedes the object existing for the raycaster), + then for two animation frames (proof the render loop is producing + frames, so pointer events will be processed). Both lag by seconds under + CI's software-GL contention.""" + page.wait_for_function( + "() => window.__viserMutable?.nodeRefFromName?.['/dragme'] != null", + timeout=15_000, + ) + page.evaluate( + """() => new Promise((resolve) => + requestAnimationFrame(() => requestAnimationFrame(resolve)))""" + ) + + +def test_drag_end_routes_to_owner_after_mid_drag_removal( + viser_server: viser.ViserServer, viser_page: Page +) -> None: + """The final drag ``end`` echoes the owner captured at drag START. + Regression: it was re-derived from the live store per phase, so removing + the node mid-drag stamped the end with the broadcast owner ("") and the + client scope's ``on_drag`` end callback never fired.""" + client = get_client_handle(viser_server) + # Up must be set explicitly: the (0, 0, 4) -> origin view is parallel to + # the server-default +Z up, and whether the browser's camera sync has + # already replaced that default is a race. + client.camera.up_direction = (0.0, 1.0, 0.0) + client.camera.position = (0.0, 0.0, 4.0) + client.camera.look_at = (0.0, 0.0, 0.0) + + box = client.scene.add_box("/dragme", dimensions=(4.0, 4.0, 0.2)) + started = threading.Event() + ended = threading.Event() + + @box.on_drag("left") + def _(event: viser.SceneNodeDragEvent) -> None: + if event.phase == "start": + started.set() + elif event.phase == "end": + ended.set() + + _wait_drag_ready(viser_page) + cx, cy = canvas_center(viser_page) + viser_page.mouse.move(cx, cy) + viser_page.mouse.down() + viser_page.mouse.move(cx + 80, cy + 40, steps=10) + assert started.wait(timeout=5.0), "drag start never reached the client scope" + + # Remove the node MID-DRAG: the frontend synthesizes the end phase, which + # must still route to the client scope that received the start. + box.remove() + assert ended.wait(timeout=5.0), "drag end was misrouted after removal" + viser_page.mouse.up() + + +def test_pointer_filters_cleared_on_reconnect( + viser_server: viser.ViserServer, viser_page: Page +) -> None: + """Per-owner scene-pointer filters are connection-scoped. Regression: an + IN-PLACE reconnect (worker retry, no page reload) left the previous + client's filter entry behind forever -- its owner id can never send a + disable -- keeping click gestures engaged.""" + client = get_client_handle(viser_server) + + @client.scene.on_pointer_event(event_type="click") + def _(event) -> None: + pass + + viser_page.wait_for_function( + "() => window.__viserPointer?.hasSceneClickFilter() === true", + timeout=5_000, + ) + + # Kick the connection; the page's worker auto-reconnects WITHOUT a + # reload, so all frontend state survives except what the reconnect + # path deliberately resets. Nothing re-registers a pointer callback, so + # no filter may survive -- and since only the reconnect path clears + # filters, this wait doubles as the reconnect wait. + viser_server._websock_server.disconnect_all_clients() + viser_page.wait_for_function( + "() => window.__viserPointer?.hasSceneClickFilter() === false", + timeout=10_000, + ) + + +def test_skinned_mesh_bone_state_is_variant_scoped( + viser_server: viser.ViserServer, viser_page: Page +) -> None: + """Bone state is keyed per (owner, name) variant. Regression: a single + name-keyed entry let a shadowed server's bone stream corrupt the + client's variant, and promotion deleted the promoted variant's state.""" + import numpy as np + + client = get_client_handle(viser_server) + + def add_skinned(scene_api): + return scene_api.add_mesh_skinned( + "/skin", + vertices=np.array( + [[0.0, 0.0, 0.0], [1.0, 0.0, 0.0], [0.0, 1.0, 0.0]], + dtype=np.float32, + ), + faces=np.array([[0, 1, 2]], dtype=np.uint32), + bone_wxyzs=((1.0, 0.0, 0.0, 0.0), (1.0, 0.0, 0.0, 0.0)), + bone_positions=((0.0, 0.0, 0.0), (1.0, 0.0, 0.0)), + skin_weights=np.array( + [[1.0, 0.0], [0.5, 0.5], [0.0, 1.0]], dtype=np.float32 + ), + ) + + server_mesh = add_skinned(viser_server.scene) + wait_for_scene_node(viser_page, "/skin") + + # Client-scoped variant shadows the server's. + add_skinned(client.scene) + js_entry = """ + (owner) => { + const state = window.__viserMutable?.skinnedMeshState; + if (!state) return null; + const entry = state[`${owner}\\u0000/skin`]; + return entry ? entry.poses[1].position : null; + } + """ + client_owner = str(client.client_id) + viser_page.wait_for_function( + f"() => ({js_entry})('') !== null && ({js_entry})('{client_owner}') !== null", + timeout=10_000, + ) + + # A bone update from the (shadowed) server scope lands in the SERVER + # variant's entry; the client's stays at its initial pose. + server_mesh.bones[1].position = (5.0, 6.0, 7.0) + viser_page.wait_for_function( + f"() => String(({js_entry})('')) === '5,6,7'", + timeout=10_000, + ) + assert viser_page.evaluate(js_entry, client_owner) == [1, 0, 0] + + # Promotion: removing the client variant keeps the server variant's + # accumulated bone state (entry survives; client entry is dropped). + client.scene._handle_from_node_name["/skin"].remove() + viser_page.wait_for_function( + f"() => ({js_entry})('{client_owner}') === null", + timeout=10_000, + ) + assert viser_page.evaluate(js_entry, "") == [5, 6, 7] + + +def test_disconnect_mid_drag_fires_end_for_client_scope( + two_client_setup: dict, +) -> None: + """Regression: the disconnect teardown drained in-flight drags only from + the server SceneApi, but owner-scoped dispatch routes drags on + client-scoped nodes to the CLIENT's own scope -- so their synthesized + ``phase="end"`` (which lets apps release per-drag state) never fired.""" + server = two_client_setup["server"] + page1: Page = two_client_setup["page1"] + client1 = two_client_setup["client1"] + + client1.camera.up_direction = (0.0, 1.0, 0.0) + client1.camera.position = (0.0, 0.0, 4.0) + client1.camera.look_at = (0.0, 0.0, 0.0) + box = client1.scene.add_box("/dragme", dimensions=(4.0, 4.0, 0.2)) + started = threading.Event() + ended = threading.Event() + + @box.on_drag("left") + def _(event: viser.SceneNodeDragEvent) -> None: + if event.phase == "start": + started.set() + elif event.phase == "end": + ended.set() + + _wait_drag_ready(page1) + cx, cy = canvas_center(page1) + page1.mouse.move(cx, cy) + page1.mouse.down() + page1.mouse.move(cx + 80, cy + 40, steps=10) + assert started.wait(timeout=5.0), "drag start never reached the client scope" + + # Disconnect mid-drag: the wire "end" will never arrive, so the + # teardown must synthesize it for the CLIENT scope's registry. + page1.context.close() + deadline = time.time() + 5.0 + while client1.client_id in server._connected_clients: + assert time.time() < deadline, "client 1 never disconnected" + time.sleep(0.05) + assert ended.wait(timeout=5.0), "synthesized drag end never fired" diff --git a/tests/e2e/test_cross_scope_visual.py b/tests/e2e/test_cross_scope_visual.py new file mode 100644 index 000000000..8f09a0ec0 --- /dev/null +++ b/tests/e2e/test_cross_scope_visual.py @@ -0,0 +1,162 @@ +"""Pixel-level verification of cross-scope shadowing, via get_render(). + +The store-level halves of these behaviors are covered by +``test_cross_scope_handles.py`` (frontend state) and +``tests/test_scene_scopes.py`` (server state); here we verify the actually +rendered pixels: a shadowing client variant is what that client SEES, other +clients keep seeing the broadcast variant, un-shadowing reveals the +broadcast variant's LATEST state, and scope-local cascade leaves a client +child visibly on screen after its broadcast parent is removed. + +Conventions follow ``test_get_render_capture.py``: a box big enough to fill +the view center from the default camera pose, pure-channel colors, and +dominant-channel assertions on the center patch. +""" + +from __future__ import annotations + +import time +from typing import Generator + +import numpy as np +import pytest +from playwright.sync_api import Browser + +import viser +import viser._client_autobuild + +from .utils import center_mean, connect_client, find_free_port, wait_for_server_ready + +RED = (255, 0, 0) +GREEN = (0, 255, 0) +BLUE = (0, 0, 255) + + +@pytest.fixture() +def own_server() -> Generator[viser.ViserServer, None, None]: + viser._client_autobuild.ensure_client_is_built = lambda: None + server: viser.ViserServer | None = None + for attempt in range(3): + try: + server = viser.ViserServer(port=find_free_port(), verbose=False) + break + except OSError: + if attempt == 2: + raise + assert server is not None + wait_for_server_ready(server.get_port()) + yield server + server.stop() + + +def _capture_center(client: viser.ClientHandle) -> np.ndarray: + img = client.get_render(height=96, width=128, timeout=30.0) + return center_mean(img) + + +def _assert_dominant( + client: viser.ClientHandle, color: tuple[int, int, int], label: str +) -> None: + """The center patch's dominant channel must match `color`. Captures can + race the ~1-frame shadow remount, so retry briefly before failing. Every + capture is evaluated BEFORE the deadline check: a single software-WebGL + capture can exceed the whole retry budget under load, and a passing + frame must not be discarded just because it arrived late.""" + deadline = time.monotonic() + 10.0 + while True: + center = _capture_center(client) + if int(np.argmax(center)) == int(np.argmax(color)) and center.max() > 60: + return + if time.monotonic() > deadline: + raise AssertionError( + f"{label}: expected dominant {color}, captured {center}" + ) + time.sleep(0.2) + + +def test_shadowing_pixels_round_trip( + own_server: viser.ViserServer, browser: Browser +) -> None: + """RED server box -> GREEN client variant shadows it -> server recolors + its hidden variant BLUE (display unchanged) -> un-shadow reveals BLUE.""" + client, page, context = connect_client(own_server, browser) + try: + server_box = own_server.scene.add_box( + "/box", color=RED, dimensions=(2.0, 2.0, 2.0) + ) + _assert_dominant(client, RED, "baseline server box") + + client_box = client.scene.add_box( + "/box", color=GREEN, dimensions=(2.0, 2.0, 2.0) + ) + _assert_dominant(client, GREEN, "client variant shadows") + + # Server updates its shadowed variant; this client's view must not + # change. + server_box.color = BLUE + time.sleep(0.5) + _assert_dominant(client, GREEN, "shadowed server update hidden") + + # Un-shadow: the server variant reappears with its LATEST color. + client_box.remove() + _assert_dominant(client, BLUE, "un-shadow reveals latest state") + finally: + page.close() + context.close() + + +def test_two_clients_see_their_own_variant( + own_server: viser.ViserServer, browser: Browser +) -> None: + """A shadowing variant is per-client: the shadowing client sees GREEN + while a second client keeps seeing the server's RED.""" + client1, page1, context1 = connect_client(own_server, browser) + client2, page2, context2 = connect_client(own_server, browser) + assert client1.client_id != client2.client_id + try: + own_server.scene.add_box("/box", color=RED, dimensions=(2.0, 2.0, 2.0)) + _assert_dominant(client1, RED, "client1 baseline") + _assert_dominant(client2, RED, "client2 baseline") + + client1.scene.add_box("/box", color=GREEN, dimensions=(2.0, 2.0, 2.0)) + _assert_dominant(client1, GREEN, "client1 sees own variant") + _assert_dominant(client2, RED, "client2 unaffected") + finally: + page1.close() + context1.close() + page2.close() + context2.close() + + +def test_scope_local_cascade_child_stays_on_screen( + own_server: viser.ViserServer, browser: Browser +) -> None: + """A client child under a broadcast parent remains VISIBLE (not just in + the store) after the parent is removed, then disappears when its own + scope removes it.""" + client, page, context = connect_client(own_server, browser) + try: + parent = own_server.scene.add_frame("/parent", show_axes=False) + child = client.scene.add_box( + "/parent/child", color=GREEN, dimensions=(2.0, 2.0, 2.0) + ) + _assert_dominant(client, GREEN, "child visible under broadcast parent") + + parent.remove() + time.sleep(0.5) + _assert_dominant(client, GREEN, "child survives broadcast cascade") + + child.remove() + # Background: all channels bright (white-ish), nothing green-dominant + # at high saturation. + deadline = time.monotonic() + 5.0 + center = _capture_center(client) + while time.monotonic() < deadline and not np.all(center > 200): + time.sleep(0.2) + center = _capture_center(client) + assert np.all(center > 200), ( + f"child still visible after its own removal: center {center}" + ) + finally: + page.close() + context.close() diff --git a/tests/e2e/test_floating_panel.py b/tests/e2e/test_floating_panel.py index dd7f14d81..75db0a1df 100644 --- a/tests/e2e/test_floating_panel.py +++ b/tests/e2e/test_floating_panel.py @@ -12,6 +12,8 @@ import viser +from .utils import get_client_handle + # Wide enough to stay above the mobile breakpoint (xs = 36em = 576px), so the # floating layout -- not the bottom sheet -- is used. _VIEWPORT: ViewportSize = {"width": 1280, "height": 720} @@ -187,19 +189,6 @@ def test_resize_left_grip_keeps_right_edge_pinned(viser_page: Page) -> None: assert abs((after["x"] + after["width"]) - right_before) < 12 -def _wait_for_client(server: viser.ViserServer) -> viser.ClientHandle: - """Return the first connected client, polling briefly for it to register.""" - import time - - deadline = time.monotonic() + 5.0 - while time.monotonic() < deadline: - clients = server.get_clients() - if clients: - return next(iter(clients.values())) - time.sleep(0.05) - raise RuntimeError("no client connected within timeout") - - def test_notification_offset_clear_of_left_dock( viser_page: Page, viser_server: viser.ViserServer ) -> None: @@ -215,7 +204,7 @@ def test_notification_offset_clear_of_left_dock( panel = _panel_box(viser_page) # Raise a (non-auto-closing) notification from the server. - client = _wait_for_client(viser_server) + client = get_client_handle(viser_server) client.add_notification( "Docked test", "Should clear the GUI", auto_close_seconds=None ) diff --git a/tests/e2e/test_get_render_capture.py b/tests/e2e/test_get_render_capture.py index 301052fcc..eed2ae127 100644 --- a/tests/e2e/test_get_render_capture.py +++ b/tests/e2e/test_get_render_capture.py @@ -22,7 +22,12 @@ import viser import viser._client_autobuild -from .utils import find_free_port, wait_for_connection, wait_for_server_ready +from .utils import ( + center_mean, + connect_client, + find_free_port, + wait_for_server_ready, +) @pytest.fixture() @@ -42,37 +47,6 @@ def own_server() -> Generator[viser.ViserServer, None, None]: server.stop() -def _connect_client( - own_server: viser.ViserServer, browser: Browser -) -> tuple[viser.ClientHandle, object, object]: - captured: list[viser.ClientHandle] = [] - own_server.on_client_connect(lambda client: captured.append(client)) - context = browser.new_context() - page = context.new_page() - wait_for_connection(page, own_server.get_port()) - deadline = time.monotonic() + 10 - while not captured and time.monotonic() < deadline: - time.sleep(0.05) - assert captured, "client never connected" - client = captured[0] - # Wait for the first camera update so get_render() can read camera state. - while client.camera._state.update_timestamp == 0.0 and ( - time.monotonic() < deadline - ): - time.sleep(0.05) - assert client.camera._state.update_timestamp != 0.0, "camera never synced" - return client, page, context - - -def _center_mean(img: np.ndarray) -> np.ndarray: - h, w = img.shape[:2] - return img[ - h // 2 - h // 8 : h // 2 + h // 8, - w // 2 - w // 8 : w // 2 + w // 8, - :3, - ].mean(axis=(0, 1)) - - def test_get_render_reflects_prior_scene_updates( own_server: viser.ViserServer, browser: Browser ) -> None: @@ -81,7 +55,7 @@ def test_get_render_reflects_prior_scene_updates( down two different server buffers and the client needs a React commit before capturing) repeatedly, alternating colors so ANY stale frame fails the dominant-channel check.""" - client, page, context = _connect_client(own_server, browser) + client, page, context = connect_client(own_server, browser) try: # A box that fills the view center from the default camera pose. box = own_server.scene.add_box( @@ -93,7 +67,7 @@ def test_get_render_reflects_prior_scene_updates( img = client.get_render( height=96, width=128, transport_format="png", timeout=30.0 ) - center = _center_mean(img) + center = center_mean(img) expected_channel = int(np.argmax(color)) assert int(np.argmax(center)) == expected_channel, ( f"iteration {i}: set color {color} but captured center " @@ -115,7 +89,7 @@ def test_get_render_reflects_fresh_node_poses( The capture hook's defensive pose sweep plus the one-frame commit wait must cover it; a capture of the box at the default pose (origin) or the previous box's position fails the centroid side check.""" - client, page, context = _connect_client(own_server, browser) + client, page, context = connect_client(own_server, browser) try: h, w = 96, 128 @@ -174,7 +148,7 @@ def test_get_render_transport_formats_agree( on the center pixels, with corner pixels showing each format's background convention, and repeated solo captures (the same-frame fast path) staying stable. The default is JPEG.""" - client, page, context = _connect_client(own_server, browser) + client, page, context = connect_client(own_server, browser) try: own_server.scene.add_box( "/box", color=(0, 120, 255), dimensions=(2.0, 2.0, 2.0) @@ -186,7 +160,7 @@ def test_get_render_transport_formats_agree( # Stabilize first: capture until two consecutive frames agree. prev = None for _ in range(40): - cur = _center_mean( + cur = center_mean( client.get_render(height=h, width=w, transport_format="png", timeout=30) ) if prev is not None and np.allclose(cur, prev, atol=2.0): @@ -219,7 +193,7 @@ def test_get_render_transport_formats_agree( # Back-to-back solo captures (no interleaved scene updates) take the # same-frame capture path; they must stay correct and identical-ish. again = client.get_render(height=h, width=w, transport_format="png", timeout=30) - assert np.allclose(_center_mean(again), _center_mean(png), atol=3.0) + assert np.allclose(center_mean(again), center_mean(png), atol=3.0) finally: page.close() # type: ignore[attr-defined] context.close() # type: ignore[attr-defined] @@ -238,7 +212,7 @@ def test_get_render_does_not_leak_capture_state_into_splat_view( here as the capture's viewport size leaking across a frame boundary into the splat material (an in-page rAF probe can only ever see it if the same-frame repair did not happen).""" - client, page, context = _connect_client(own_server, browser) + client, page, context = connect_client(own_server, browser) try: rng = np.random.default_rng(0) n = 2000 @@ -303,7 +277,7 @@ def test_get_render_survives_hidden_unmounted_node_pose_update( too -- an undefined-only guard let the null through to a TypeError, failing the whole capture with a spurious "could not capture a frame" RuntimeError on a healthy client.""" - client, page, context = _connect_client(own_server, browser) + client, page, context = connect_client(own_server, browser) try: tc = own_server.scene.add_transform_controls("/gizmo") time.sleep(1.0) # Mount. diff --git a/tests/e2e/test_old_recording_playback.py b/tests/e2e/test_old_recording_playback.py new file mode 100644 index 000000000..b93cec678 --- /dev/null +++ b/tests/e2e/test_old_recording_playback.py @@ -0,0 +1,89 @@ +"""E2E: recordings serialized BEFORE the (owner, name) identity refactor +still play back correctly. + +``assets/pre_scope_recording.viser`` was generated by actual pre-refactor +code (the branch's merge-base) -- its messages carry no ``owner`` / +``virtual`` stamps, so this pins the deserialization defaults and the +client-side parent-anchor fallback for the old format end to end. +""" + +from __future__ import annotations + +import functools +import http.server +import threading +from pathlib import Path + +from playwright.sync_api import Page + +import viser + +from .utils import find_free_port, wait_for_scene_node, wait_for_scene_node_hidden + +_ASSETS = Path(__file__).parent / "assets" + + +class _CORSHandler(http.server.SimpleHTTPRequestHandler): + """The recording is served from a different origin than the viewer.""" + + def end_headers(self) -> None: + self.send_header("Access-Control-Allow-Origin", "*") + super().end_headers() + + def log_message(self, *args) -> None: + pass # Keep pytest output clean. + + +def test_pre_scope_recording_plays_back( + viser_server: viser.ViserServer, page: Page +) -> None: + file_port = find_free_port() + httpd = http.server.ThreadingHTTPServer( + ("127.0.0.1", file_port), + functools.partial(_CORSHandler, directory=str(_ASSETS)), + ) + threading.Thread(target=httpd.serve_forever, daemon=True).start() + try: + page.goto( + f"http://localhost:{viser_server.get_port()}/" + f"?playbackPath=http://127.0.0.1:{file_port}/pre_scope_recording.viser" + ) + + # Every node from the old recording, including the auto-created + # ancestors of the deep add (the old format has no server-sent + # virtual anchors; the client-side makeParents fallback covers it). + for name in ( + "/root_frame", + "/root_frame/box", + "/root_frame/box/label", + "/lines", + "/hidden_box", + "/deep", + "/deep/nested", + "/deep/nested/sphere", + ): + wait_for_scene_node(page, name, timeout=15_000) + + # Un-stamped messages must resolve to the broadcast scope. + owners = page.evaluate( + """() => { + const state = window.__viserSceneTree.getState(); + return ["/root_frame", "/lines", "/deep/nested/sphere"].map( + (n) => state[n].message.owner ?? "", + ); + }""" + ) + assert owners == ["", "", ""] + + # Recorded poses and visibility land. + page.wait_for_function( + """() => { + const pose = + window.__viserMutable?.nodePoseData?.["/root_frame"]; + return pose && Math.abs(pose.position[0] - 0.5) < 1e-4; + }""", + timeout=15_000, + ) + wait_for_scene_node_hidden(page, "/hidden_box", timeout=15_000) + finally: + httpd.shutdown() diff --git a/tests/e2e/utils.py b/tests/e2e/utils.py index 039006445..fc5973dc0 100644 --- a/tests/e2e/utils.py +++ b/tests/e2e/utils.py @@ -4,8 +4,13 @@ import socket import time +from typing import TYPE_CHECKING -from playwright.sync_api import Locator, Page +import numpy as np +from playwright.sync_api import Browser, BrowserContext, Locator, Page + +if TYPE_CHECKING: + import viser # --------------------------------------------------------------------------- # Network utilities @@ -44,6 +49,66 @@ def find_gui_input(page: Page, label_text: str) -> Locator: return gui_row.locator("input:not([type='hidden'])") +def get_client_handle( + server: viser.ViserServer, expected_count: int = 1, timeout: float = 10.0 +) -> viser.ClientHandle: + """Wait until ``expected_count`` clients are registered, return the newest. + + Client handles register on the first camera message, which can trail the + websocket handshake that ``wait_for_connection`` observes. + """ + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + clients = server.get_clients() + if len(clients) >= expected_count: + return clients[max(clients.keys())] + time.sleep(0.05) + raise TimeoutError( + f"Expected {expected_count} connected client(s) within {timeout}s." + ) + + +def connect_client( + server: viser.ViserServer, browser: Browser +) -> tuple[viser.ClientHandle, Page, BrowserContext]: + """Open a fresh browser context on ``server`` and return the NEW client's + handle once its camera has synced (required before ``get_render``). + Safe to call repeatedly on one server: only a client that was not + already connected is returned.""" + captured: list = [] + seen_ids = {c.client_id for c in server.get_clients().values()} + server.on_client_connect( + lambda client: ( + captured.append(client) if client.client_id not in seen_ids else None + ) + ) + context = browser.new_context() + page = context.new_page() + wait_for_connection(page, server.get_port()) + deadline = time.monotonic() + 10 + while not captured and time.monotonic() < deadline: + time.sleep(0.05) + assert captured, "client never connected" + client = captured[-1] + while client.camera._state.update_timestamp == 0.0 and ( + time.monotonic() < deadline + ): + time.sleep(0.05) + assert client.camera._state.update_timestamp != 0.0, "camera never synced" + return client, page, context + + +def center_mean(img: np.ndarray) -> np.ndarray: + """Mean RGB of the center patch of a rendered frame (pixel-assertion + helper for get_render()-based tests).""" + h, w = img.shape[:2] + return img[ + h // 2 - h // 8 : h // 2 + h // 8, + w // 2 - w // 8 : w // 2 + w // 8, + :3, + ].mean(axis=(0, 1)) + + def wait_for_connection(page: Page, port: int) -> None: """Navigate to the viser server and wait for WebSocket connection. diff --git a/tests/infra_utils.py b/tests/infra_utils.py new file mode 100644 index 000000000..8d359216d --- /dev/null +++ b/tests/infra_utils.py @@ -0,0 +1,45 @@ +"""Shared helpers for headless server-side tests (no browser). + +The synthetic-client construction here is the third home this pattern +needed (after inline copies in ``test_panel.py`` and +``test_get_render_latency.py``); new tests should import it from here. +""" + +from __future__ import annotations + +import asyncio + +import viser +from viser._viser import ClientHandle +from viser.infra._async_message_buffer import AsyncMessageBuffer +from viser.infra._infra import WebsockClientConnection, _ClientHandleState + + +def client_buffer_messages(client: ClientHandle) -> list: + """Messages currently in a client's per-connection buffer (post-GC).""" + return list( + client._websock_connection._state.message_buffer.message_from_id.values() + ) + + +def broadcast_messages(server: viser.ViserServer) -> list: + """Messages currently in the server's broadcast buffer (post-GC).""" + return list(server._websock_server._broadcast_buffer.message_from_id.values()) + + +def make_synthetic_client(server: viser.ViserServer, client_id: int) -> ClientHandle: + """In-process ClientHandle with a real per-client message buffer but no + websocket (mirrors how WebsockServer builds per-client state). Buffer + construction hops to the server's loop thread because + AsyncMessageBuffer's asyncio primitives bind to the running loop.""" + + async def _make_buffer() -> AsyncMessageBuffer: + return AsyncMessageBuffer(server._event_loop, persistent_messages=False) + + buffer = asyncio.run_coroutine_threadsafe( + _make_buffer(), server._event_loop + ).result(timeout=5.0) + conn = WebsockClientConnection( + client_id, _ClientHandleState(buffer, server._event_loop) + ) + return ClientHandle(conn, server) diff --git a/tests/test_gui_cross_scope.py b/tests/test_gui_cross_scope.py new file mode 100644 index 000000000..f200c0ce7 --- /dev/null +++ b/tests/test_gui_cross_scope.py @@ -0,0 +1,453 @@ +"""Tests for cross-scope GUI container nesting. + +GUI container nesting across scopes is DIRECTIONAL: a ``client.gui`` element +added inside a ``with server.gui.`` context nests in the server +container (its audience is a subset of the container's), while the reverse +raises. Cross-nested elements are the one deliberate exception to +scope-local removal: server-container teardown cascades into them, and +``client.gui.reset()`` / disconnect teardown reach them through per-scope +foreign-nesting bookkeeping. + +Browser-facing behavior (rendering inside the folder, per-client visibility, +DOM containment) is covered by ``tests/e2e/test_cross_scope_handles.py``; +this file pins the Python-side wiring on a headless server with synthetic +in-process clients. +""" + +from __future__ import annotations + +import warnings as warnings_module +from typing import Generator + +import pytest + +import viser +import viser._client_autobuild +from viser import _messages as m + +from .infra_utils import ( + broadcast_messages, + client_buffer_messages, + make_synthetic_client, +) + + +@pytest.fixture() +def server() -> Generator[viser.ViserServer, None, None]: + viser._client_autobuild.ensure_client_is_built = lambda: None + server = viser.ViserServer(port=0, verbose=False) + yield server + server.stop() + + +def _add_uuids(messages: list) -> set[str]: + """uuids of GUI element-creation messages (they carry container_uuid).""" + return {msg.uuid for msg in messages if hasattr(msg, "container_uuid")} + + +def _remove_uuids(messages: list) -> set[str]: + return {msg.uuid for msg in messages if isinstance(msg, m.GuiRemoveMessage)} + + +# --------------------------------------------------------------------------- +# Where elements land: the allowed direction. +# --------------------------------------------------------------------------- + + +def test_client_element_nests_in_server_folder(server: viser.ViserServer) -> None: + """The core wiring: a client add inside a server folder context parents + into the server folder's subtree, is tracked as foreign by the client + scope, and its add message targets the folder but travels on the + CLIENT's own connection (never the broadcast buffer).""" + client = make_synthetic_client(server, 0) + + with server.gui.add_folder("SrvFolder") as folder: + button = client.gui.add_button("mine") + + assert button._impl.parent_container_id == folder._impl.uuid + assert folder._children[button._impl.uuid] is button + assert button._impl.uuid in client.gui._handles_in_foreign_containers + + client_adds = [ + msg + for msg in client_buffer_messages(client) + if getattr(msg, "uuid", None) == button._impl.uuid + and hasattr(msg, "container_uuid") + ] + assert len(client_adds) == 1 + assert client_adds[0].container_uuid == folder._impl.uuid + assert button._impl.uuid not in _add_uuids(broadcast_messages(server)) + + +def test_other_server_container_types_nest_and_teardown( + server: viser.ViserServer, +) -> None: + """The directional rule holds for the other server container context + types -- tabs, modals, and scene-node-backed 3D GUI containers -- and + each type's (distinct) teardown path also cascades into cross-nested + client elements.""" + client = make_synthetic_client(server, 0) + + tab_group = server.gui.add_tab_group() + tab = tab_group.add_tab("Tab") + with tab: + in_tab = client.gui.add_button("in tab") + assert in_tab._impl.parent_container_id == tab._id + assert tab._children[in_tab._impl.uuid] is in_tab + + modal = server.gui.add_modal("Modal") + with modal: + in_modal = client.gui.add_button("in modal") + assert in_modal._impl.parent_container_id == modal._uuid + assert modal._children[in_modal._impl.uuid] is in_modal + + container_3d = server.scene.add_3d_gui_container("/gui3d") + with container_3d: + in_3d = client.gui.add_button("in 3d container") + assert in_3d._impl.parent_container_id == container_3d._container_id + assert container_3d._children[in_3d._impl.uuid] is in_3d + + for handle in (in_tab, in_modal, in_3d): + assert handle._impl.uuid in client.gui._handles_in_foreign_containers + + tab_group.remove() + assert in_tab._impl.removed + modal.close() + assert in_modal._impl.removed + container_3d.remove() # Scene-node removal path (_on_remove). + assert in_3d._impl.removed + assert not client.gui._handles_in_foreign_containers + + +# --------------------------------------------------------------------------- +# The rejected directions. +# --------------------------------------------------------------------------- + + +def test_server_element_into_client_container_raises( + server: viser.ViserServer, +) -> None: + client = make_synthetic_client(server, 0) + + with client.gui.add_folder("CliFolder"): + with pytest.raises(RuntimeError, match="not vice versa"): + server.gui.add_button("stray") + # Outside the context, both scopes add at their own roots again. + assert server.gui.add_button("ok")._impl.parent_container_id == "root" + assert client.gui.add_button("ok")._impl.parent_container_id == "root" + + +def test_cross_client_nesting_raises(server: viser.ViserServer) -> None: + """One client's element cannot nest in another client's container: the + audiences are disjoint, not nested.""" + client_a = make_synthetic_client(server, 0) + client_b = make_synthetic_client(server, 1) + + with client_a.gui.add_folder("A's folder"): + with pytest.raises(RuntimeError): + client_b.gui.add_button("stray") + + +# --------------------------------------------------------------------------- +# Removal cascades. +# --------------------------------------------------------------------------- + + +def test_server_folder_removal_cascades_through_client_subtree( + server: viser.ViserServer, +) -> None: + """Removing a server folder removes its server children AND cross-nested + client subtrees (the one exception to scope-local removal). Each remove + message travels on its owner's connection.""" + client = make_synthetic_client(server, 0) + + with server.gui.add_folder("SrvFolder") as folder: + srv_child = server.gui.add_button("shared") + with client.gui.add_folder("CliFolder") as cli_folder: + leaf = client.gui.add_button("leaf") + + # Only the subtree ROOT is foreign-tracked: the leaf is an ordinary + # same-scope element parented under the client folder. + assert cli_folder._impl.parent_container_id == folder._impl.uuid + assert leaf._impl.parent_container_id == cli_folder._impl.uuid + assert cli_folder._impl.uuid in client.gui._handles_in_foreign_containers + assert leaf._impl.uuid not in client.gui._handles_in_foreign_containers + + folder.remove() + + assert srv_child._impl.removed + assert cli_folder._impl.removed + assert leaf._impl.removed + assert not client.gui._handles_in_foreign_containers + + client_removes = _remove_uuids(client_buffer_messages(client)) + broadcast_removes = _remove_uuids(broadcast_messages(server)) + assert {cli_folder._impl.uuid, leaf._impl.uuid} <= client_removes + assert {folder._impl.uuid, srv_child._impl.uuid} <= broadcast_removes + assert not {cli_folder._impl.uuid, leaf._impl.uuid} & broadcast_removes + + +def test_client_remove_detaches_without_touching_others( + server: viser.ViserServer, +) -> None: + """A client removing its own cross-nested element detaches it from the + server folder; the folder and a second client's element are untouched.""" + client_a = make_synthetic_client(server, 0) + client_b = make_synthetic_client(server, 1) + + with server.gui.add_folder("SrvFolder") as folder: + a_button = client_a.gui.add_button("a") + b_button = client_b.gui.add_button("b") + + a_button.remove() + + assert a_button._impl.removed + assert a_button._impl.uuid not in folder._children + assert a_button._impl.uuid not in client_a.gui._handles_in_foreign_containers + assert not folder._impl.removed + assert not b_button._impl.removed + assert folder._children[b_button._impl.uuid] is b_button + + +# --------------------------------------------------------------------------- +# reset(). +# --------------------------------------------------------------------------- + + +def test_client_reset_drains_cross_nested_subtree(server: viser.ViserServer) -> None: + """client.gui.reset() reaches cross-nested subtrees, which the root + container walk alone can't see; the server folder and other clients' + elements are untouched.""" + client_a = make_synthetic_client(server, 0) + client_b = make_synthetic_client(server, 1) + + with server.gui.add_folder("SrvFolder") as folder: + with client_a.gui.add_folder("CliFolder") as cli_folder: + leaf = client_a.gui.add_button("leaf") + b_button = client_b.gui.add_button("b") + + client_a.gui.reset() + + assert cli_folder._impl.removed + assert leaf._impl.removed + assert cli_folder._impl.uuid not in folder._children + assert not client_a.gui._handles_in_foreign_containers + assert not folder._impl.removed + assert not b_button._impl.removed + + +# --------------------------------------------------------------------------- +# Thread-context restore across nested cross-scope `with` blocks. +# --------------------------------------------------------------------------- + + +def test_server_adds_ok_after_nested_client_context_exits( + server: viser.ViserServer, +) -> None: + """Regression: exiting a client container context that was nested inside + a server container context must hand the thread marker back to the + server scope -- a server add afterwards used to be misread as nesting + inside a client container and raised.""" + client = make_synthetic_client(server, 0) + + with server.gui.add_folder("SrvFolder") as srv_folder: + with client.gui.add_folder("CliFolder"): + client.gui.add_button("leaf") + # Back in the server context: both scopes target the server folder. + srv_button = server.gui.add_button("shared") + cli_button = client.gui.add_button("mine") + + assert srv_button._impl.parent_container_id == srv_folder._impl.uuid + assert cli_button._impl.parent_container_id == srv_folder._impl.uuid + + +def test_container_context_is_task_local(server: viser.ViserServer) -> None: + """Async callbacks interleave on ONE event-loop thread, each running in + a copied Context. Regression: a thread-keyed context marker made a + `with` block suspended at an await leak into unrelated callbacks -- a + sibling task's server add spuriously raised the cross-scope error. + Contexts copied before the block entered must see no marker.""" + import contextvars + + client_a = make_synthetic_client(server, 0) + client_b = make_synthetic_client(server, 1) + sibling = contextvars.copy_context() # Stands in for another asyncio task. + + folder = client_a.gui.add_folder("F") + folder.__enter__() + try: + # Inside the block, cross-scope nesting is live... + inside = client_a.gui.add_button("in") + assert inside._impl.parent_container_id == folder._impl.uuid + # ...but the sibling context is unaffected: no spurious raise, no + # cross-scope capture. + srv = sibling.run(server.gui.add_button, "s") + assert srv._impl.parent_container_id == "root" + other = sibling.run(client_b.gui.add_button, "b") + assert other._impl.parent_container_id == "root" + finally: + folder.__exit__(None, None, None) + + +def test_no_stale_target_after_server_context_exits( + server: viser.ViserServer, +) -> None: + """Regression: the cross-scope restore must not record the server + folder's uuid in the CLIENT's own target map -- after the server's + `with` block exits, adds from both scopes land at their own roots.""" + client = make_synthetic_client(server, 0) + + with server.gui.add_folder("SrvFolder"): + with client.gui.add_folder("CliFolder"): + pass + + assert server.gui.add_button("s")._impl.parent_container_id == "root" + assert client.gui.add_button("c")._impl.parent_container_id == "root" + + +# --------------------------------------------------------------------------- +# Forms. +# --------------------------------------------------------------------------- + + +def test_form_nesting_rules_apply_across_scopes(server: viser.ViserServer) -> None: + """The no-nested-forms rule sees through scope boundaries: a client form + inside a server form is invalid, while a client form inside a plain + server folder is fine.""" + client = make_synthetic_client(server, 0) + + with server.gui.add_form("SrvForm"): + with pytest.raises(ValueError, match="Nested forms"): + client.gui.add_form("CliForm") + + with server.gui.add_folder("SrvFolder") as folder: + form = client.gui.add_form("CliForm") + assert form._impl.parent_container_id == folder._impl.uuid + + +# --------------------------------------------------------------------------- +# Disconnect teardown. +# --------------------------------------------------------------------------- + + +def test_disconnect_releases_cross_nested_elements( + server: viser.ViserServer, +) -> None: + """The disconnect teardown detaches cross-nested client subtrees from the + server's container tree without sending messages -- including + DESCENDANTS of the cross-nested root, so surviving user references get + the ordinary already-removed warning instead of a KeyError.""" + client = make_synthetic_client(server, 0) + with server.gui.add_folder("SrvFolder") as folder: + with client.gui.add_folder("CliFolder") as cli_folder: + leaf = client.gui.add_button("leaf") + + # Simulate the disconnect teardown (buffer shutdown + release call). + client._websock_connection._state.message_buffer.set_done() + client.gui._release_cross_scope_nesting() + + assert cli_folder._impl.removed + assert leaf._impl.removed + assert cli_folder._impl.uuid not in folder._children + + # A surviving reference to a DESCENDANT degrades gracefully. + with pytest.warns(UserWarning, match="already removed"): + leaf.remove() + + # Removing the server folder afterwards is clean: no cascade into the + # dead connection, so no "closed connection" warning. + with warnings_module.catch_warnings(record=True) as caught: + warnings_module.simplefilter("always") + folder.remove() + assert not any("closed connection" in str(w.message) for w in caught) + + +def test_second_server_context_does_not_break_first( + server: viser.ViserServer, +) -> None: + """Regression: container-context markers are per-server, so an inner + `with` block on an unrelated ViserServer must not clobber the first + server's still-open context.""" + server_b = viser.ViserServer(port=0, verbose=False) + try: + client = make_synthetic_client(server, 0) + with server.gui.add_folder("FA") as fa: + with server_b.gui.add_folder("FB"): + pass + # Server A's context is still active for both of its scopes. + cli_button = client.gui.add_button("x") + srv_button = server.gui.add_button("y") + assert cli_button._impl.parent_container_id == fa._impl.uuid + assert srv_button._impl.parent_container_id == fa._impl.uuid + finally: + server_b.stop() + + +def test_client_scope_dangling_restore_stays_client_local( + server: viser.ViserServer, +) -> None: + """Regression: a CLIENT container removed while an inner client context + is open is a same-scope affair -- the dangling uuid must not be handed + to the server's target map (which would poison the thread for both + scopes permanently). After the outer context exits, both scopes add at + their roots again.""" + client = make_synthetic_client(server, 0) + + with client.gui.add_folder("Outer") as outer: + with client.gui.add_folder("Inner"): + outer.remove() + + assert server.gui.add_button("s")._impl.parent_container_id == "root" + assert client.gui.add_button("c")._impl.parent_container_id == "root" + + +def test_removed_server_container_restore_does_not_poison_thread( + server: viser.ViserServer, +) -> None: + """Regression: if the server container is removed while a nested client + context is open, the client context's exit must still hand the thread + marker back cleanly -- afterwards neither scope spuriously raises and + adds land at their own roots.""" + client = make_synthetic_client(server, 0) + + with server.gui.add_folder("F") as f: + with client.gui.add_folder("C"): + f.remove() + + assert server.gui.add_button("s")._impl.parent_container_id == "root" + assert client.gui.add_button("c")._impl.parent_container_id == "root" + + +def test_disconnect_cycles_leave_no_server_residue( + server: viser.ViserServer, +) -> None: + """Repeated connect / cross-nest / disconnect cycles must not accumulate + entries in the server-side GUI registries or the host container's + children (leak check for the release path).""" + folder = server.gui.add_folder("Host") + gui = server.gui + + def registry_sizes() -> tuple[int, int, int]: + return ( + len(gui._container_handle_from_uuid), + len(gui._gui_input_handle_from_uuid), + len(folder._children), + ) + + baseline = registry_sizes() + for i in range(20): + client = make_synthetic_client(server, i) + with folder: + with client.gui.add_folder(f"F{i}"): + client.gui.add_button(f"b{i}") + client.gui.add_slider( + f"s{i}", min=0.0, max=1.0, step=0.1, initial_value=0.5 + ) + client.scene.add_frame(f"/c{i}") + + # Disconnect teardown. + client._websock_connection._state.message_buffer.set_done() + client.gui._release_cross_scope_nesting() + + assert registry_sizes() == baseline, f"registry residue after cycle {i}" + assert len(client.gui._handles_in_foreign_containers) == 0 diff --git a/tests/test_modifier_filtering.py b/tests/test_modifier_filtering.py index 36e650e34..5cf5cf032 100644 --- a/tests/test_modifier_filtering.py +++ b/tests/test_modifier_filtering.py @@ -307,48 +307,59 @@ def _() -> None: @patch.object(viser._client_autobuild, "ensure_client_is_built", lambda: None) -def test_pointer_event_server_scope_clears_client_scope_and_vice_versa() -> None: - """Server-scope and per-client-scope ``on_pointer_event`` share the - same wire (the ``ScenePointerEnableMessage`` toggle on the client - side). Allowing both to register simultaneously would let one - scope's ``enable=False`` deactivate the other's callbacks. The API - enforces exclusivity: registering on one scope clears the other. - - This is asserted indirectly -- we don't fully spin up a client - handle here; we only verify the cross-scope cleanup hook is wired - by checking that the server-scope list is cleared via a fake - client whose ``scene._scene_pointer_cb`` we observe.""" - server = viser.ViserServer() - - # Stand up a minimal stub client that satisfies the cross-scope - # cleanup branch -- it just needs ``.scene._scene_pointer_cb`` and - # ``.scene._remove_all_pointer_callbacks`` to be present. - fake_client = MagicMock(name="fake-client") - fake_client.client_id = ClientId(0) - fake_client._viser_server = server - fake_client.scene._scene_pointer_cb = [] - - def _stub_remove_all(**_kwargs: object) -> None: - fake_client.scene._scene_pointer_cb.clear() - - fake_client.scene._remove_all_pointer_callbacks = MagicMock( - side_effect=_stub_remove_all +def test_pointer_event_scopes_coexist() -> None: + """Server-scope and per-client-scope pointer callbacks coexist: the + ``ScenePointerEnableMessage`` filters are kept per owner on the client + (gesture engagement uses the union), so registrations in one scope + never clear the other's, and each scope's enable messages ride its own + connection stamped with its own owner.""" + from .infra_utils import ( + broadcast_messages, + client_buffer_messages, + make_synthetic_client, ) - server._connected_clients[ClientId(0)] = fake_client - # Fake client populates its own list as if a client-scope - # registration had happened. - fake_client.scene._scene_pointer_cb.append(object()) - assert len(fake_client.scene._scene_pointer_cb) == 1 + server = viser.ViserServer() + client = make_synthetic_client(server, 5) + + @client.scene.on_click() + def _client_cb(event: viser.SceneClickEvent) -> None: + del event @server.scene.on_click() def _server_cb(event: viser.SceneClickEvent) -> None: del event - # Server-scope registration should have cleared the client-scope list. + # Both scopes keep their registrations. + assert len(server.scene._scene_pointer_cb) == 1 + assert len(client.scene._scene_pointer_cb) == 1 + + # Each scope's enable message is stamped with its own owner. + server_enables = [ + msg + for msg in broadcast_messages(server) + if isinstance(msg, _messages.ScenePointerEnableMessage) + ] + client_enables = [ + msg + for msg in client_buffer_messages(client) + if isinstance(msg, _messages.ScenePointerEnableMessage) + ] + assert server_enables and all(msg.owner == "" for msg in server_enables) + assert client_enables and all(msg.owner == "5" for msg in client_enables) + + # One scope clearing its callbacks leaves the other's untouched; the + # disable rides that scope's connection only (an empty-modifiers enable + # coalesced into the same redundancy slot). + client.scene.remove_click_callback() + assert len(client.scene._scene_pointer_cb) == 0 assert len(server.scene._scene_pointer_cb) == 1 - fake_client.scene._remove_all_pointer_callbacks.assert_called_once() - assert len(fake_client.scene._scene_pointer_cb) == 0 + latest_client_enable = [ + msg + for msg in client_buffer_messages(client) + if isinstance(msg, _messages.ScenePointerEnableMessage) + ][-1] + assert latest_client_enable.modifiers == () @patch.object(viser._client_autobuild, "ensure_client_is_built", lambda: None) diff --git a/tests/test_panel.py b/tests/test_panel.py index a82ce8d87..ac74f5ba5 100644 --- a/tests/test_panel.py +++ b/tests/test_panel.py @@ -9,7 +9,6 @@ from __future__ import annotations -import asyncio import threading import warnings from typing import Any @@ -384,31 +383,16 @@ def test_client_scoped_reset_does_not_touch_main_panel_placement() -> None: which the client's placement gate treats as a fresh deliberate command -- so the default CONTROL_PANEL_ID messages would clobber server-authored placement (e.g. undock a dock_left control panel) for that one client.""" - from viser._viser import ClientHandle - from viser.infra._async_message_buffer import AsyncMessageBuffer - from viser.infra._infra import WebsockClientConnection, _ClientHandleState + from .infra_utils import make_synthetic_client server = _make_server() try: server.gui.main_panel.dock_left() - # Synthetic in-process client connection: no websocket needed, we only - # inspect the outgoing per-client message buffer (mirrors how - # WebsockServer constructs the per-client state). Constructed ON the - # server's loop thread: AsyncMessageBuffer's asyncio.Event fields bind - # to the current event loop at construction on Python <= 3.9, and this - # test thread has none (production buffers are always built inside the - # loop, so only the test needs the hop). - async def _make_buffer() -> AsyncMessageBuffer: - return AsyncMessageBuffer(server._event_loop, persistent_messages=False) - - buffer = asyncio.run_coroutine_threadsafe( - _make_buffer(), server._event_loop - ).result(timeout=5.0) - conn = WebsockClientConnection( - 0, _ClientHandleState(buffer, server._event_loop) - ) - client = ClientHandle(conn, server) + # Synthetic in-process client connection: no websocket needed, we + # only inspect the outgoing per-client message buffer. + client = make_synthetic_client(server, client_id=0) + buffer = client._websock_connection._state.message_buffer client.gui.add_button("local") client.gui.reset() diff --git a/tests/test_scene_scopes.py b/tests/test_scene_scopes.py new file mode 100644 index 000000000..bb3ed6df3 --- /dev/null +++ b/tests/test_scene_scopes.py @@ -0,0 +1,326 @@ +"""Tests for cross-scope scene semantics: per-name variant slots. + +Scene nodes are identified by (owner, name): the broadcast scope +(``server.scene``) and each per-client scope (``client.scene``) may each hold +one variant of a name, fed independently by their scopes' messages. The +client renders the effective variant per the display rule (real client > +real broadcast > virtual client > virtual broadcast); shadowing and +promotion are covered by the frontend's ``SceneTreeState.test.ts`` and the +e2e suite (``tests/e2e/test_cross_scope_handles.py``). + +This file covers the Python/server half on a headless server with synthetic +in-process clients: owner stamping, unconditional same-scope virtual +anchors, scope-local cascade, and coexistence in the registries. +""" + +from __future__ import annotations + +from typing import Generator + +import pytest + +import viser +import viser._client_autobuild +from viser import _messages as m + +from .infra_utils import ( + broadcast_messages, + client_buffer_messages, + make_synthetic_client, +) + + +@pytest.fixture() +def server() -> Generator[viser.ViserServer, None, None]: + viser._client_autobuild.ensure_client_is_built = lambda: None + server = viser.ViserServer(port=0, verbose=False) + yield server + server.stop() + + +# --------------------------------------------------------------------------- +# Owner stamping. +# --------------------------------------------------------------------------- + + +def test_owner_stamped_onbroadcast_messages(server: viser.ViserServer) -> None: + handle = server.scene.add_icosphere("/ball", radius=0.1, position=(1.0, 0.0, 0.0)) + handle.visible = False + handle.color = (10, 20, 30) + + for msg in broadcast_messages(server): + if hasattr(msg, "owner"): + assert msg.owner == "", f"{type(msg).__name__} not broadcast-stamped" + + +def test_owner_stamped_on_client_messages(server: viser.ViserServer) -> None: + client = make_synthetic_client(server, 3) + handle = client.scene.add_icosphere("/ball", radius=0.1, position=(1.0, 0.0, 0.0)) + handle.visible = False + handle.color = (10, 20, 30) + handle.remove() + + scene_messages = [ + msg for msg in client_buffer_messages(client) if hasattr(msg, "owner") + ] + assert len(scene_messages) > 0 + for msg in scene_messages: + assert msg.owner == "3", f"{type(msg).__name__} not client-stamped" + + +# --------------------------------------------------------------------------- +# Coexistence: same name in both scopes is legal, state is scope-local. +# --------------------------------------------------------------------------- + + +def test_same_name_coexists_across_scopes(server: viser.ViserServer) -> None: + client = make_synthetic_client(server, 0) + + server_handle = server.scene.add_icosphere("/dup", radius=0.1) + client_handle = client.scene.add_icosphere("/dup", radius=0.2) + + # Both registries hold their own live handle; neither superseded. + assert not server_handle._impl.removed + assert not client_handle._impl.removed + assert server.scene._handle_from_node_name["/dup"] is server_handle + assert client.scene._handle_from_node_name["/dup"] is client_handle + + # Removing one scope's variant does not touch the other's. + client_handle.remove() + assert client_handle._impl.removed + assert not server_handle._impl.removed + + +def test_same_name_across_clients_coexists(server: viser.ViserServer) -> None: + client0 = make_synthetic_client(server, 0) + client1 = make_synthetic_client(server, 1) + + h0 = client0.scene.add_icosphere("/own", radius=0.1) + h1 = client1.scene.add_icosphere("/own", radius=0.2) + assert not h0._impl.removed + assert not h1._impl.removed + + +def test_same_scope_supersede_still_works(server: viser.ViserServer) -> None: + old = server.scene.add_box("/re", dimensions=(1.0, 1.0, 1.0)) + new = server.scene.add_box("/re", dimensions=(2.0, 2.0, 2.0)) + assert old._impl.removed + assert not new._impl.removed + assert server.scene._handle_from_node_name["/re"] is new + + +# --------------------------------------------------------------------------- +# Unconditional same-scope virtual anchors. +# --------------------------------------------------------------------------- + + +def test_virtual_anchors_created_per_scope(server: viser.ViserServer) -> None: + client = make_synthetic_client(server, 0) + + # Broadcast parent exists; the client's deep add still creates a + # same-scope anchor chain (which does not shadow: anchors are virtual). + server.scene.add_frame("/parent", show_axes=False) + client.scene.add_icosphere("/parent/child/deep", radius=0.1) + + assert "/parent" in client.scene._handle_from_node_name + assert "/parent/child" in client.scene._handle_from_node_name + + creates = { + msg.name: msg + for msg in client_buffer_messages(client) + if isinstance(msg, m._CreateSceneNodeMessage) + } + assert creates["/parent"].virtual is True + assert creates["/parent/child"].virtual is True + assert creates["/parent/child/deep"].virtual is False + for msg in creates.values(): + assert msg.owner == "0" + + +def test_real_add_supersedes_virtual_anchor(server: viser.ViserServer) -> None: + server.scene.add_icosphere("/a/b", radius=0.1) + anchor = server.scene._handle_from_node_name["/a"] + + real = server.scene.add_frame("/a", show_axes=True) + assert anchor._impl.removed + assert server.scene._handle_from_node_name["/a"] is real + # The real frame's create message is not virtual. + creates = [ + msg + for msg in broadcast_messages(server) + if isinstance(msg, m._CreateSceneNodeMessage) and msg.name == "/a" + ] + assert len(creates) == 1 # Redundancy key replaced the anchor's create. + assert creates[0].virtual is False + + +# --------------------------------------------------------------------------- +# Scope-local cascade. +# --------------------------------------------------------------------------- + + +def test_cascade_is_scope_local(server: viser.ViserServer) -> None: + client = make_synthetic_client(server, 0) + + parent = server.scene.add_frame("/parent", show_axes=False) + server_child = server.scene.add_icosphere("/parent/shared", radius=0.1) + client_child = client.scene.add_icosphere("/parent/mine", radius=0.1) + + parent.remove() + + # Broadcast descendants die with the parent; the client's subtree + # survives, anchored by its own scope's virtual /parent. + assert server_child._impl.removed + assert not client_child._impl.removed + assert "/parent/mine" in client.scene._handle_from_node_name + assert "/parent" in client.scene._handle_from_node_name # The anchor. + + # The client can still update and remove its node normally. + client_child.position = (1.0, 2.0, 3.0) + client_child.remove() + assert client_child._impl.removed + + +def test_client_cascade_does_not_touch_broadcast(server: viser.ViserServer) -> None: + client = make_synthetic_client(server, 0) + + server.scene.add_frame("/parent", show_axes=False) + server_child = server.scene.add_icosphere("/parent/shared", radius=0.1) + client_parent_variant = client.scene.add_frame("/parent", show_axes=True) + client_child = client.scene.add_icosphere("/parent/mine", radius=0.1) + + client_parent_variant.remove() + + assert client_child._impl.removed # Same-scope cascade. + assert not server_child._impl.removed # Other scope untouched. + assert "/parent" in server.scene._handle_from_node_name + + +def test_remove_messages_enumerate_descendants(server: viser.ViserServer) -> None: + """The frontend does not recurse on removes; the server must enumerate + one RemoveSceneNodeMessage per same-scope descendant.""" + parent = server.scene.add_frame("/p", show_axes=False) + server.scene.add_icosphere("/p/a", radius=0.1) + server.scene.add_icosphere("/p/a/b", radius=0.1) + + parent.remove() + + # After GC/redundancy, creates are gone; a remove tombstone per name. + removed_names = { + msg.name + for msg in broadcast_messages(server) + if isinstance(msg, m.RemoveSceneNodeMessage) + } + assert {"/p", "/p/a", "/p/a/b"} <= removed_names + + +# --------------------------------------------------------------------------- +# World axes. +# --------------------------------------------------------------------------- + + +def test_client_scene_construction_sends_nothing(server: viser.ViserServer) -> None: + client = make_synthetic_client(server, 0) + assert len(client_buffer_messages(client)) == 0, ( + "client SceneApi construction queued messages; it must not re-add " + "/WorldAxes (or anything else) over the per-client connection" + ) + assert "/WorldAxes" not in client.scene._handle_from_node_name + + +def test_client_world_axes_property_raises(server: viser.ViserServer) -> None: + client = make_synthetic_client(server, 0) + with pytest.raises(AttributeError, match="server.scene.world_axes"): + _ = client.scene.world_axes + # The sanctioned per-client override: a client-scoped "/WorldAxes" frame + # (shadows the server's variant on that client's frontend). + override = client.scene.add_frame("/WorldAxes", show_axes=True) + assert not override._impl.removed + assert not server.scene.world_axes._impl.removed + + +def test_reset_world_axes_skip_is_broadcast_only(server: viser.ViserServer) -> None: + """Regression: reset()'s "/WorldAxes" carve-out protects the broadcast + scope's default handle only -- a client-scoped override is an ordinary + per-client node and must reset away with everything else.""" + client = make_synthetic_client(server, 0) + override = client.scene.add_frame("/WorldAxes", show_axes=True) + + client.scene.reset() + assert override._impl.removed + assert "/WorldAxes" not in client.scene._handle_from_node_name + + server.scene.reset() + assert not server.scene.world_axes._impl.removed + + +def test_stamped_messages_roundtrip_through_serialization( + server: viser.ViserServer, +) -> None: + """Regression: the owner/virtual stamps are non-init dataclass fields, + which (a) vars()-based serialization silently omitted until first + assignment and (b) ``Message.deserialize`` choked on (they aren't + ``__init__`` parameters). The public serialize -> deserialize round trip + must stay lossless, stamps included.""" + import msgspec + + from viser.infra import Message + + client = make_synthetic_client(server, 3) + client.scene.add_frame("/rt/leaf", show_axes=False) # Anchor + real node. + + messages = client_buffer_messages(client) + assert len(messages) > 0 + for msg in messages: + serialized = msg.as_serializable_dict() + # The stamps are always on the wire (the generated TypeScript + # declares them as required fields). + assert serialized["owner"] == "3" + if hasattr(msg, "virtual"): + assert "virtual" in serialized + + decoded = Message.deserialize(msgspec.msgpack.encode(serialized)) + assert type(decoded) is type(msg) + assert decoded.owner == msg.owner # type: ignore[attr-defined] + if hasattr(msg, "virtual"): + assert decoded.virtual == msg.virtual # type: ignore[attr-defined] + + +# --------------------------------------------------------------------------- +# Dead-client writes. (Cross-scope GUI container tests live in +# tests/test_gui_cross_scope.py.) +# --------------------------------------------------------------------------- + + +def test_dead_connection_write_warns_once(server: viser.ViserServer) -> None: + """Writes through handles owned by a disconnected client warn exactly + once per connection, instead of accumulating silently forever.""" + import warnings as warnings_module + + client = make_synthetic_client(server, 0) + handle = client.scene.add_icosphere("/mine", radius=0.1) + + # Simulate the disconnect teardown's buffer shutdown. + client._websock_connection._state.message_buffer.set_done() + + with warnings_module.catch_warnings(record=True) as caught: + warnings_module.simplefilter("always") + handle.position = (1.0, 0.0, 0.0) + handle.position = (2.0, 0.0, 0.0) # Second write: no second warning. + dead_warnings = [w for w in caught if "closed connection" in str(w.message)] + assert len(dead_warnings) == 1 + + # Removal messages are exempt: releasing a departed client's elements + # (e.g. inside on_client_disconnect, which runs after the buffer is + # closed) is ordinary cleanup, not a leak in the making. The click + # callback matters: remove() then also emits empty interaction-bindings + # messages (not lifecycle_phase="remove"), which must be covered by the + # same exemption. + client2 = make_synthetic_client(server, 1) + handle2 = client2.scene.add_icosphere("/theirs", radius=0.1) + handle2.on_click(lambda _: None) + client2._websock_connection._state.message_buffer.set_done() + with warnings_module.catch_warnings(record=True) as caught: + warnings_module.simplefilter("always") + handle2.remove() + assert not any("closed connection" in str(w.message) for w in caught)