Fix close-camera line rendering for drei-based lines under reversed depth

src/viser/client/src/CameraFrustumVariants.tsx

@@ -1,3 +1,6 @@
+// drei's Line constructs its own three-stdlib LineMaterial; make sure the
+// reversed-depth near-plane patch is installed before that happens.
+import "./patchLineMaterialReversedDepth";
 import { Line } from "@react-three/drei";
 import { useFrame } from "@react-three/fiber";
 import React from "react";

src/viser/client/src/Line.tsx

@@ -4,6 +4,7 @@
  */
 
 import "./r3f-extend";
+import "./patchLineMaterialReversedDepth";
 import * as React from "react";
 import * as THREE from "three";
 import { ColorRepresentation } from "three";
@@ -106,31 +107,13 @@ export const Line: ForwardRefComponent<LineProps, Line2 | LineSegments2> =
         [ref],
       );
 
-      // LineMaterial's trimSegment near-plane estimate assumes standard
-      // depth; under reversed depth (App.tsx) it explodes and lines smear
-      // when the camera is close. Switch the formula on sign(a). Three.js
-      // has an equivalent fix queued for r185
-      // (https://github.com/mrdoob/three.js/pull/33572); drop this patch
-      // once we upgrade to three@>=0.185.
-      const patchLineMaterialShader = React.useCallback(
-        (mat: LineMaterial | null) => {
-          (matRef as React.MutableRefObject<LineMaterial | null>).current = mat;
-          if (!mat) return;
-          mat.onBeforeCompile = (shader) => {
-            shader.vertexShader = shader.vertexShader.replace(
-              "float nearEstimate = - 0.5 * b / a;",
-              "float nearEstimate = ( a > 0.0 ) ? ( - b / ( 1.0 + a ) ) : ( - 0.5 * b / a );",
-            );
-          };
-          mat.needsUpdate = true;
-        },
-        [],
-      );
+      // Reversed-depth near-plane fix for LineMaterial is applied globally
+      // (all instances, including drei's) in patchLineMaterialReversedDepth.
 
       // R3F manages lifecycle for all declarative children -- no manual disposal.
       const materialJsx = (
         <lineMaterial
-          ref={patchLineMaterialShader}
+          ref={matRef}
           color={effectiveColor}
           vertexColors={Boolean(vertexColors)}
           resolution={[size.width, size.height]}

src/viser/client/src/patchLineMaterialReversedDepth.test.ts

@@ -0,0 +1,38 @@
+import { describe, expect, it } from "vitest";
+import { LineMaterial } from "three-stdlib";
+
+import {
+  BROKEN_NEAR_ESTIMATE,
+  FIXED_NEAR_ESTIMATE,
+} from "./patchLineMaterialReversedDepth";
+
+describe("patchLineMaterialReversedDepth", () => {
+  it("rewrites the near-plane estimate on every new LineMaterial", () => {
+    // Constructing directly mirrors what drei's <Line> does internally; the
+    // patch must apply without any per-instance setup.
+    const mat = new LineMaterial();
+    expect(mat.vertexShader).not.toContain(BROKEN_NEAR_ESTIMATE);
+    expect(mat.vertexShader).toContain(FIXED_NEAR_ESTIMATE);
+  });
+
+  it("survives clone() without double-applying", () => {
+    const mat = new LineMaterial();
+    const cloned = mat.clone();
+    expect(cloned.vertexShader).toBe(mat.vertexShader);
+    // Exactly one occurrence of the fixed expression.
+    expect(cloned.vertexShader.split(FIXED_NEAR_ESTIMATE).length - 1).toBe(1);
+  });
+
+  it("preserves three-stdlib's own onBeforeCompile hook", () => {
+    // three-stdlib's LineMaterial assigns an onBeforeCompile that toggles
+    // USE_LINE_COLOR_ALPHA for transparent materials. Our patch must not
+    // replace it (the old per-instance patch did).
+    const mat = new LineMaterial();
+    mat.transparent = true;
+    mat.onBeforeCompile(
+      { vertexShader: "", fragmentShader: "" } as never,
+      null as never,
+    );
+    expect(mat.defines.USE_LINE_COLOR_ALPHA).toBe("1");
+  });
+});

src/viser/client/src/patchLineMaterialReversedDepth.ts

@@ -0,0 +1,58 @@
+/**
+ * Reversed-depth fix for three-stdlib's LineMaterial, applied to EVERY
+ * instance via a prototype accessor.
+ *
+ * LineMaterial's vertex shader trims line segments that cross the camera
+ * plane back to a near-plane estimate derived from the projection matrix:
+ *
+ *   float nearEstimate = - 0.5 * b / a;
+ *
+ * That formula assumes a standard depth projection. Under the reversed
+ * depth buffer we enable in App.tsx, a = near / (far - near) and
+ * b = far * near / (far - near), so it evaluates to -far / 2 -- segments
+ * that cross the camera plane get extrapolated hundreds of units into the
+ * scene and smear across the screen whenever the camera gets close to a
+ * line. The reversed-depth-safe estimate is -b / (1 + a) = -near exactly;
+ * we switch on sign(a), which distinguishes the two projection forms.
+ *
+ * We can't fix this with a one-off material patch: drei's <Line> (used for
+ * camera frustums, Catmull-Rom / cubic Bezier splines, and internally by
+ * PivotControls) constructs its own LineMaterial instances that we never
+ * see. Instead we intercept `vertexShader` on LineMaterial.prototype: the
+ * ShaderMaterial constructor assigns the shader source with a plain
+ * `this.vertexShader = ...`, which invokes this inherited setter, so every
+ * instance is rewritten at construction time. The replacement is a no-op
+ * for shader sources that don't contain the broken line (idempotent for
+ * clone()/copy(), and safe if three-stdlib ships its own fix).
+ *
+ * three.js fixed its bundled copy of this shader for r185
+ * (https://github.com/mrdoob/three.js/pull/33572), but drei and our Line
+ * component use the fork in three-stdlib, which carries its own copy of
+ * the shader -- so this patch is needed until three-stdlib ships the
+ * equivalent fix, independent of the three version we build against.
+ */
+import { LineMaterial } from "three-stdlib";
+
+const BROKEN_NEAR_ESTIMATE = "float nearEstimate = - 0.5 * b / a;";
+const FIXED_NEAR_ESTIMATE =
+  "float nearEstimate = ( a > 0.0 ) ? ( - b / ( 1.0 + a ) ) : ( - 0.5 * b / a );";
+
+const STORAGE_KEY = "__viserPatchedVertexShader";
+
+type PatchedMaterial = LineMaterial & { [STORAGE_KEY]?: string };
+
+Object.defineProperty(LineMaterial.prototype, "vertexShader", {
+  configurable: true,
+  enumerable: true,
+  get(this: PatchedMaterial) {
+    return this[STORAGE_KEY];
+  },
+  set(this: PatchedMaterial, source: string) {
+    this[STORAGE_KEY] =
+      typeof source === "string"
+        ? source.replace(BROKEN_NEAR_ESTIMATE, FIXED_NEAR_ESTIMATE)
+        : source;
+  },
+});
+
+export { BROKEN_NEAR_ESTIMATE, FIXED_NEAR_ESTIMATE };

tests/e2e/test_get_render.py

@@ -42,7 +42,7 @@ def test_get_render_raises_promptly_on_disconnect(
     own_server: viser.ViserServer, browser: Browser
 ) -> None:
     captured: list[viser.ClientHandle] = []
-    own_server.on_client_connect(lambda client: captured.append(client))
+    own_server.on_client_connect(captured.append)
 
     context = browser.new_context()
     page = context.new_page()

tests/e2e/test_line_close_camera.py

@@ -0,0 +1,68 @@
+"""Regression test for fat-line rendering when the camera is very close.
+
+Under the reversed depth buffer (App.tsx), three-stdlib LineMaterial's
+``trimSegment`` near-plane estimate evaluates to -far/2 instead of -near, so
+any line segment crossing the camera plane gets extrapolated hundreds of
+units away from the camera: the part of the line sweeping past the camera
+vanishes. viser's own <Line> carried a local patch (#720), but drei's <Line>
+-- used for camera frustums, splines, and internally by PivotControls --
+constructs its own LineMaterial and was still broken. The patch now applies
+to every LineMaterial via patchLineMaterialReversedDepth.ts; this test
+exercises the drei code path through the built client.
+"""
+
+from __future__ import annotations
+
+from io import BytesIO
+
+import numpy as np
+from PIL import Image
+from playwright.sync_api import Page
+
+import viser
+
+from .utils import wait_for_scene_node
+
+
+def test_spline_passing_camera_renders(
+    viser_server: viser.ViserServer,
+    viser_page: Page,
+) -> None:
+    """A straight spline passing just beside the camera must sweep across the
+    screen toward the viewport edge, not vanish near the camera."""
+    # drei's CatmullRomLine (unlike add_line_segments) renders through drei's
+    # <Line>, whose LineMaterial viser code never touches directly.
+    viser_server.scene.add_spline_catmull_rom(
+        "/spline",
+        points=np.array(
+            [[-5.0, 0.0, 0.0], [-2.0, 0.0, 0.0], [2.0, 0.0, 0.0], [5.0, 0.0, 0.0]]
+        ),
+        color=(255, 0, 0),
+        line_width=6.0,
+    )
+    viser_server.scene.world_axes.visible = False
+    wait_for_scene_node(viser_page, "/spline")
+    viser_page.wait_for_timeout(300)
+
+    client = list(viser_server.get_clients().values())[0]
+    # Camera right next to the line (~0.02 to the side), looking along it, and
+    # offset along x so the curve chunk crossing the camera plane has its
+    # forward endpoint well in front of the camera (a chunk boundary exactly
+    # at the camera plane would mask the trimSegment bug).
+    client.camera.position = (0.25, -0.02, 0.007)
+    client.camera.look_at = (3.25, 0.0, 0.0)
+    viser_page.wait_for_timeout(400)
+
+    canvas = viser_page.locator("canvas").first
+    img = np.array(Image.open(BytesIO(canvas.screenshot())).convert("RGB")).astype(int)
+    red = (
+        (img[:, :, 0] > 150)
+        & (img[:, :, 0] > img[:, :, 1] + 60)
+        & (img[:, :, 0] > img[:, :, 2] + 60)
+    )
+    # The section of line sweeping past the camera projects into the
+    # mid-height left third of the canvas (a region free of the control
+    # panel, the software-WebGL notification, and the viser logo). With the
+    # trimSegment bug this region is empty (measured 0 vs ~2000 pixels).
+    left_mid = int(red[200:500, 0:300].sum())
+    assert left_mid > 500, f"line sweep toward viewport edge missing: {left_mid=}"