Fix message culling bug for scene clicks

src/viser/_message_api.py

@@ -607,7 +607,7 @@ def _queue(self, message: _messages.Message) -> None:
             # This could be optimized!
             self._queued_messages.put(message)
 
-            def try_again():
+            def try_again() -> None:
                 with self._atomic_lock:
                     self._queue_unsafe(self._queued_messages.get())
 
@@ -695,10 +695,9 @@ def on_scene_click(
         """Add a callback for scene pointer events."""
         self._scene_pointer_cb.append(func)
 
-        # Notify client of a new listener. This can help the client determine whether
-        # or not click events should still be sent; note that we have no way of knowing
-        # here because both server and client handles manage their own callbacks.
-        self._queue(_messages.ScenePointerCallbackInfoMessage(count=1))
+        # If this is the first callback.
+        if len(self._scene_pointer_cb) == 1:
+            self._queue(_messages.SceneClickEnableMessage(enable=True))
         return func
 
     def remove_scene_click_callback(
@@ -710,7 +709,41 @@ def remove_scene_click_callback(
             self._scene_pointer_cb.remove(func)
 
         # Notify client that the listener has been removed.
-        self._queue(_messages.ScenePointerCallbackInfoMessage(count=-1))
+        if len(self._scene_pointer_cb) == 0:
+            from ._viser import ViserServer
+
+            if isinstance(self, ViserServer):
+                # Turn off server-level scene click events.
+                self._queue(_messages.SceneClickEnableMessage(enable=False))
+
+                # Catch an unlikely edge case: we need to re-enable click events for
+                # clients that still have callbacks.
+                clients = self.get_clients()
+                if len(clients) > 0:
+                    # TODO: putting this in a thread with an initial sleep is a hack for
+                    # giving us a soft guarantee on message ordering; the enable messages
+                    # need to arrive after the disable one above.
+                    #
+                    # Ideally we should implement a flush() method of some kind that
+                    # empties the message buffer.
+
+                    def reenable() -> None:
+                        time.sleep(1.0 / 60.0)
+                        for client in clients.values():
+                            if len(client._scene_pointer_cb) > 0:
+                                self._queue(
+                                    _messages.SceneClickEnableMessage(enable=True)
+                                )
+
+                    threading.Thread(target=reenable).start()
+
+            else:
+                assert isinstance(self, ClientHandle)
+
+                # Turn off scene click events for clients, but only if there's no
+                # server-level scene click events.
+                if len(self._state.viser_server._scene_pointer_cb) == 0:
+                    self._queue(_messages.SceneClickEnableMessage(enable=False))
 
     def add_3d_gui_container(
         self,

src/viser/_messages.py

@@ -64,10 +64,10 @@ class ScenePointerMessage(Message):
 
 
 @dataclasses.dataclass
-class ScenePointerCallbackInfoMessage(Message):
-    """Message to enable/disable scene pointer"""
+class SceneClickEnableMessage(Message):
+    """Message to enable/disable scene click events."""
 
-    count: int
+    enable: bool
 
 
 @dataclasses.dataclass

src/viser/_viser.py

@@ -223,6 +223,7 @@ def got_render_cb(
 
 @dataclasses.dataclass
 class _ClientHandleState:
+    viser_server: ViserServer
     server: infra.Server
     connection: infra.ClientConnection
 
@@ -344,7 +345,7 @@ def _(conn: infra.ClientConnection) -> None:
             client = ClientHandle(
                 conn.client_id,
                 camera,
-                _ClientHandleState(server, conn),
+                _ClientHandleState(self, server, conn),
             )
             camera._state.client = client
             first = True

src/viser/client/src/App.tsx

@@ -15,7 +15,6 @@ import {
 import { BlendFunction, KernelSize } from "postprocessing";
 
 import { SynchronizedCameraControls } from "./CameraControls";
-import { ScenePointerControls } from "./ScenePointerControls";
 import { Box, MantineProvider, MediaQuery } from "@mantine/core";
 import React from "react";
 import { SceneNodeThreeObject, UseSceneTree } from "./SceneTree";
@@ -38,6 +37,7 @@ import { Titlebar } from "./Titlebar";
 import { ViserModal } from "./Modal";
 import { useSceneTreeState } from "./SceneTreeState";
 import { GetRenderRequestMessage, Message } from "./WebsocketMessages";
+import { makeThrottledMessageSender } from "./WebsocketFunctions";
 
 export type ViewerContextContents = {
   // Zustand hooks.
@@ -68,7 +68,7 @@ export type ViewerContextContents = {
     "ready" | "triggered" | "pause" | "in_progress"
   >;
   getRenderRequest: React.MutableRefObject<null | GetRenderRequestMessage>;
-  scenePointerCallbackCount: React.MutableRefObject<number>;
+  sceneClickEnable: React.MutableRefObject<boolean>;
 };
 export const ViewerContext = React.createContext<null | ViewerContextContents>(
   null,
@@ -110,7 +110,7 @@ function ViewerRoot() {
     messageQueueRef: React.useRef([]),
     getRenderRequestState: React.useRef("ready"),
     getRenderRequest: React.useRef(null),
-    scenePointerCallbackCount: React.useRef(0),
+    sceneClickEnable: React.useRef(false),
   };
 
   return (
@@ -164,6 +164,10 @@ function ViewerContents() {
 
 function ViewerCanvas({ children }: { children: React.ReactNode }) {
   const viewer = React.useContext(ViewerContext)!;
+  const sendClickThrottled = makeThrottledMessageSender(
+    viewer.websocketRef,
+    20,
+  );
   return (
     <Canvas
       camera={{ position: [3.0, 3.0, -3.0] }}
@@ -176,14 +180,52 @@ function ViewerCanvas({ children }: { children: React.ReactNode }) {
       }}
       performance={{ min: 0.95 }}
       ref={viewer.canvasRef}
+      onClick={(e) => {
+        // Don't send click events if the scene pointer events are disabled.
+        if (!viewer.sceneClickEnable.current) return;
+
+        // clientX/Y are relative to the viewport, offsetX/Y are relative to the canvasRef.
+        // clientX==offsetX if there is no titlebar, but clientX>offsetX if there is a titlebar.
+        const mouseVector = new THREE.Vector2();
+        mouseVector.x =
+          2 * (e.nativeEvent.offsetX / viewer.canvasRef.current!.clientWidth) -
+          1;
+        mouseVector.y =
+          1 -
+          2 * (e.nativeEvent.offsetY / viewer.canvasRef.current!.clientHeight);
+        if (
+          mouseVector.x > 1 ||
+          mouseVector.x < -1 ||
+          mouseVector.y > 1 ||
+          mouseVector.y < -1
+        )
+          return;
+
+        const raycaster = new THREE.Raycaster();
+        raycaster.setFromCamera(mouseVector, viewer.cameraRef.current!);
+
+        sendClickThrottled({
+          type: "ScenePointerMessage",
+          event_type: "click",
+          ray_origin: [
+            raycaster.ray.origin.x,
+            -raycaster.ray.origin.z,
+            raycaster.ray.origin.y,
+          ],
+          ray_direction: [
+            raycaster.ray.direction.x,
+            -raycaster.ray.direction.z,
+            raycaster.ray.direction.y,
+          ],
+        });
+      }}
     >
       {children}
       <BackgroundImage />
       <AdaptiveDpr pixelated />
       <AdaptiveEvents />
       <SceneContextSetter />
       <SynchronizedCameraControls />
-      <ScenePointerControls />
       <Selection>
         <SceneNodeThreeObject name="" parent={null} />
         <EffectComposer enabled={true} autoClear={false}>

src/viser/client/src/WebsocketInterface.tsx

@@ -95,12 +95,13 @@ function useMessageHandler() {
       }
 
       // Enable/disable whether scene pointer events are sent.
-      case "ScenePointerCallbackInfoMessage": {
-        viewer.scenePointerCallbackCount.current += message.count;
+      case "SceneClickEnableMessage": {
+        viewer.sceneClickEnable.current = message.enable;
 
         // Update cursor to indicate whether the scene can be clicked.
-        viewer.canvasRef.current!.style.cursor =
-          viewer.scenePointerCallbackCount.current > 0 ? "pointer" : "auto";
+        viewer.canvasRef.current!.style.cursor = message.enable
+          ? "pointer"
+          : "auto";
         return;
       }
 
@@ -833,7 +834,7 @@ export function WebsocketMessageProducer() {
         console.log(`Disconnected! ${server} code=${event.code}`);
         clearTimeout(retryTimeout);
         viewer.websocketRef.current = null;
-        viewer.scenePointerCallbackCount.current = 0;
+        viewer.sceneClickEnable.current = 0;
         viewer.useGui.setState({ websocketConnected: false });
         resetGui();
 

src/viser/client/src/WebsocketMessages.tsx

@@ -27,13 +27,13 @@ export interface ScenePointerMessage {
   ray_origin: [number, number, number];
   ray_direction: [number, number, number];
 }
-/** Message to enable/disable scene pointer
+/** Message to enable/disable scene click events.
  *
  * (automatically generated)
  */
-export interface ScenePointerCallbackInfoMessage {
-  type: "ScenePointerCallbackInfoMessage";
-  count: number;
+export interface SceneClickEnableMessage {
+  type: "SceneClickEnableMessage";
+  enable: boolean;
 }
 /** Variant of CameraMessage used for visualizing camera frustums.
  *
@@ -653,7 +653,7 @@ export interface GetRenderResponseMessage {
 export type Message =
   | ViewerCameraMessage
   | ScenePointerMessage
-  | ScenePointerCallbackInfoMessage
+  | SceneClickEnableMessage
   | CameraFrustumMessage
   | GlbMessage
   | FrameMessage