From e9396b24cb8243e160f35384f326ad0ed8b8760d Mon Sep 17 00:00:00 2001 From: Andrii Shylenko <14119286+w1ne@users.noreply.github.com> Date: Wed, 26 Aug 2026 00:51:52 +0200 Subject: [PATCH] fix(viewer): land the first camera fit instead of tweening it away MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An embed opened at app.kernelcad.com showed a blank viewport until the viewer clicked the home control. The auto-fit was running and computing the right pose — a Chrome trace of WebGLRenderer.render on the published embed shows the camera reaching (67.03, 62.04, 45.31), exactly buildFitCameraPose for that model — but it got there over a ~600ms lerp, and the lerp is cancelled by OrbitControls' 'start' event so the user can grab the camera. `lastFitBounds` is stamped when the fit is REQUESTED, not when the camera arrives, so a cancelled first fit is never re-issued: `boundsStable` suppresses every later run of the effect. One pointer-down on the canvas while the model loads therefore strands the camera permanently. Measured on the live embed: a click 120ms after the mesh appeared left the camera at (56.32, 53.31, 43.21) and it never moved again. A first framing has nothing to animate from, so deliver it in one frame and make it uncancellable. Later re-fits keep the tween and keep yielding to the user. Also hold a pending pose while `controls` is still null — retiring one before OrbitControls exists lets its origin-initialised target re-aim the camera at (0,0,0). Separately, the framing distance was wrong: `radius * 2.8` frames a flat disc (tan), not a sphere (sin, 2.924 at fov 40), and has no aspect term at all, while camera.fov is the VERTICAL angle. A 420x780 embed needs 5.2 * radius; the model overflowed every edge. fitDistance() now derives the distance from the camera's own fov and aspect, using the narrower axis. --- .../controllers/CameraHandler.fit.test.tsx | 203 ++++++++++++++++++ .../viewer/controllers/CameraHandler.tsx | 62 ++++-- .../viewer/controllers/cameraPose.ts | 41 ++++ 3 files changed, 289 insertions(+), 17 deletions(-) create mode 100644 src/studio/components/viewer/controllers/CameraHandler.fit.test.tsx diff --git a/src/studio/components/viewer/controllers/CameraHandler.fit.test.tsx b/src/studio/components/viewer/controllers/CameraHandler.fit.test.tsx new file mode 100644 index 000000000..34dccf6ed --- /dev/null +++ b/src/studio/components/viewer/controllers/CameraHandler.fit.test.tsx @@ -0,0 +1,203 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2026 Andrii Shylenko and kernelCAD contributors +// @vitest-environment happy-dom +// +// Regression cover for the two ways the viewer used to hand a visitor an +// unusable first frame: +// +// 1. The initial framing was a ~600ms tween that ANY pointer-down on the +// canvas cancelled, and `lastFitBounds` had already been stamped so no +// later run re-issued it. The camera stayed wherever the tween had got to +// — for a model straddling the default (40,40,40) camera, that is inside +// the part, i.e. a blank embed until the viewer finds the home button. +// 2. `distance = radius * 2.8` framed the bounding sphere with a +// tangent-derived constant and no aspect term, so the model overflowed the +// frame — mildly at 16:9, badly on anything portrait. +// +// Both are asserted on the camera the controller actually produces: does the +// scene's bounding sphere sit inside the resulting frustum? + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { act, cleanup, render } from '@testing-library/react'; +import * as THREE from 'three'; +import { OrbitControls } from 'three-stdlib'; +import type { GeometryResult } from '../../../../shared/worker/geometryEngine'; + +interface FiberState { camera: THREE.PerspectiveCamera; controls: OrbitControls | null } + +let fiberState: FiberState; +let frameCallback: ((state: unknown, delta: number) => void) | null = null; +let sketchMode: { active: boolean; plane?: unknown } = { active: false }; + +vi.mock('@react-three/fiber', () => ({ + useThree: () => fiberState, + // R3F keeps the LATEST callback in a mutable ref; mirror that. + useFrame: (cb: (state: unknown, delta: number) => void) => { frameCallback = cb; }, +})); + +vi.mock('../../../context/WorkbenchContext', () => ({ + useWorkbench: () => ({ selectedFace: null, sketchMode }), +})); + +const { CameraHandler } = await import('./CameraHandler'); + +/** One geometry whose vertex cloud spans `min`..`max`. */ +function boxGeometry(min: [number, number, number], max: [number, number, number]): GeometryResult { + const v = [...min, ...max]; + return { + faces: [{ + vertices: new Float32Array(v), + indices: new Uint32Array(), + normals: new Float32Array(v.length), + faceId: 0, + }], + } as GeometryResult; +} + +function boundingSphere(min: [number, number, number], max: [number, number, number]) { + const lo = new THREE.Vector3(...min); + const hi = new THREE.Vector3(...max); + return { + center: lo.clone().add(hi).multiplyScalar(0.5), + radius: Math.max(lo.distanceTo(hi) / 2, 1), + }; +} + +/** Signed slack, in world units, of the tightest frustum plane. Negative = clipped. */ +function frustumSlack(camera: THREE.PerspectiveCamera, center: THREE.Vector3, radius: number): number { + camera.updateMatrixWorld(true); + camera.updateProjectionMatrix(); + const frustum = new THREE.Frustum().setFromProjectionMatrix( + new THREE.Matrix4().multiplyMatrices(camera.projectionMatrix, camera.matrixWorldInverse), + ); + return Math.min(...frustum.planes.map((p) => p.distanceToPoint(center) - radius)); +} + +function mountViewer(width: number, height: number) { + const camera = new THREE.PerspectiveCamera(40, width / height, 0.1, 1000); + camera.position.set(40, 40, 40); + camera.lookAt(0, 0, 0); + const canvas = document.createElement('canvas'); + const controls = new OrbitControls(camera, canvas as unknown as HTMLElement); + fiberState = { camera, controls }; + return { camera, controls }; +} + +function runFrames(count: number) { + act(() => { + for (let i = 0; i < count; i += 1) frameCallback?.({}, 1 / 60); + }); +} + +beforeEach(() => { + frameCallback = null; + sketchMode = { active: false }; +}); + +afterEach(() => { + cleanup(); + fiberState?.controls?.dispose(); +}); + +describe('CameraHandler initial framing', () => { + // A model that straddles the default camera: the corner (40,30,20) sits + // right beside the default eye at (40,40,40), so a framing that never + // lands leaves the viewer staring at (or inside) the part. + const MIN: [number, number, number] = [0, 0, 0]; + const MAX: [number, number, number] = [40, 30, 20]; + + it('frames the model even when the viewer touches the canvas as it loads', () => { + const { camera, controls } = mountViewer(1280, 720); + const { rerender } = render(); + + rerender(); + // The visitor's pointer lands on the canvas the instant geometry shows + // up. OrbitControls fires 'start'; the old code dropped the framing. + act(() => { controls.dispatchEvent({ type: 'start' }); }); + runFrames(240); + + const sphere = boundingSphere(MIN, MAX); + expect(frustumSlack(camera, sphere.center, sphere.radius)).toBeGreaterThan(0); + }); + + it('frames the model inside the frustum on a 16:9 viewport', () => { + const { camera } = mountViewer(1280, 720); + const { rerender } = render(); + rerender(); + runFrames(240); + + const sphere = boundingSphere(MIN, MAX); + expect(frustumSlack(camera, sphere.center, sphere.radius)).toBeGreaterThan(0); + }); + + it('frames the model inside the frustum on a portrait viewport', () => { + // camera.fov is the VERTICAL angle, so a tall viewport has a much + // narrower horizontal one — the axis that actually crops. + const { camera } = mountViewer(420, 780); + const { rerender } = render(); + rerender(); + runFrames(240); + + const sphere = boundingSphere(MIN, MAX); + expect(frustumSlack(camera, sphere.center, sphere.radius)).toBeGreaterThan(0); + }); + + it('re-frames on the home/fit request', () => { + const { camera } = mountViewer(1280, 720); + const geometries = [boxGeometry(MIN, MAX)]; + const { rerender } = render(); + rerender(); + runFrames(240); + + camera.position.set(400, 400, 400); + rerender( + , + ); + runFrames(240); + + const sphere = boundingSphere(MIN, MAX); + expect(frustumSlack(camera, sphere.center, sphere.radius)).toBeGreaterThan(0); + }); +}); + +describe('CameraHandler anti-twitch behaviour', () => { + const MIN: [number, number, number] = [0, 0, 0]; + const MAX: [number, number, number] = [40, 30, 20]; + + it('leaves the orbit alone when a param nudge wobbles the bounds', () => { + const { camera } = mountViewer(1280, 720); + const { rerender } = render(); + rerender(); + runFrames(240); + + // The user orbits away from the fitted pose. + const orbited = new THREE.Vector3(-90, 20, 130); + camera.position.copy(orbited); + + // A slider moves a joint: the tessellated AABB shifts a hair. + rerender(); + runFrames(240); + + expect(camera.position.distanceTo(orbited)).toBeLessThan(0.001); + }); + + it('still yields to the user when a real shape change re-fits', () => { + const { camera, controls } = mountViewer(1280, 720); + const { rerender } = render(); + rerender(); + runFrames(240); + + const orbited = new THREE.Vector3(-90, 20, 130); + camera.position.copy(orbited); + + // A genuinely different model — this one is allowed to re-fit, but the + // re-fit is a tween the user can grab out of at any time. + rerender(); + runFrames(2); + act(() => { controls.dispatchEvent({ type: 'start' }); }); + const grabbed = camera.position.clone(); + runFrames(240); + + expect(camera.position.distanceTo(grabbed)).toBeLessThan(0.001); + }); +}); diff --git a/src/studio/components/viewer/controllers/CameraHandler.tsx b/src/studio/components/viewer/controllers/CameraHandler.tsx index f7ff18e57..516e7bcd4 100644 --- a/src/studio/components/viewer/controllers/CameraHandler.tsx +++ b/src/studio/components/viewer/controllers/CameraHandler.tsx @@ -7,7 +7,7 @@ import { useWorkbench } from "../../../context/WorkbenchContext"; import type { GeometryResult } from "../../../../shared/worker/geometryEngine"; import type { ViewportFocusTarget } from "../../../store/shellStore"; import { computeGeometryBounds } from "./cameraBounds"; -import { buildCameraPose, buildFitCameraPose, type ViewTarget } from "./cameraPose"; +import { buildCameraPose, buildFitCameraPose, fitDistanceForCamera, type ViewTarget } from "./cameraPose"; import { filterGeometriesForFocusTarget } from "./focusTarget"; // Using the exported constants if needed, but they are defined in Viewer.tsx conventionally. @@ -26,7 +26,14 @@ export function CameraHandler({ const { selectedFace, sketchMode } = useWorkbench(); const { camera, controls } = useThree(); const cameraRef = useRef(camera); - const targetState = useRef<{ position: THREE.Vector3; lookAt: THREE.Vector3; } | null>(null); + // `immediate` marks the FIRST framing of a scene: the camera has never + // pointed at this model, so there is nothing to animate from and nothing + // for a tween to fight. It is applied in one frame and cannot be cancelled. + const targetState = useRef<{ + position: THREE.Vector3; + lookAt: THREE.Vector3; + immediate?: boolean; + } | null>(null); const prevSketchActive = useRef(false); const savedCameraState = useRef<{ position: THREE.Vector3; target: THREE.Vector3; } | null>(null); const lastFitBounds = useRef<{ center: THREE.Vector3; radius: number } | null>(null); @@ -55,12 +62,20 @@ export function CameraHandler({ && Math.abs(bounds.radius - last.radius) <= tolerance, ); if (boundsStable) return; + // The first framing of a scene has to LAND. `lastFitBounds` is stamped + // when the fit is requested, not when the camera arrives, so a fit that + // is abandoned mid-flight is never retried — `boundsStable` suppresses + // every later run. That is how a published embed ended up showing a + // blank viewport until the viewer found the home button: one pointer- + // down on the canvas during the ~600ms tween cancelled the only fit + // this scene would ever get. So deliver it in a single frame instead. + const isFirstFit = lastFitBounds.current === null; lastFitBounds.current = { center: bounds.center.clone(), radius: bounds.radius }; - const distance = Math.max(bounds.radius * 2.8, SKETCH_DISTANCE); + const distance = Math.max(fitDistanceForCamera(bounds.radius, cameraRef.current), SKETCH_DISTANCE); const pose = buildFitCameraPose(bounds.center, distance); cameraRef.current.up.copy(pose.up); - targetState.current = { position: pose.position, lookAt: pose.lookAt }; + targetState.current = { position: pose.position, lookAt: pose.lookAt, immediate: isFirstFit }; cameraRef.current.near = Math.max(distance / 500, 0.01); cameraRef.current.far = Math.max(distance * 20, 1000); cameraRef.current.updateProjectionMatrix(); @@ -72,7 +87,7 @@ export function CameraHandler({ const bounds = computeGeometryBounds(geometries); if (!bounds) return; - const distance = Math.max(bounds.radius * 2.8, SKETCH_DISTANCE); + const distance = Math.max(fitDistanceForCamera(bounds.radius, cameraRef.current), SKETCH_DISTANCE); const pose = navigationRequest.target === 'fit' ? buildFitCameraPose(bounds.center, distance) : buildCameraPose(navigationRequest.target, bounds.center, distance); @@ -93,7 +108,7 @@ export function CameraHandler({ const bounds = computeGeometryBounds(focusedGeometries); if (!bounds) return; - const distance = Math.max(bounds.radius * 2.8, SKETCH_DISTANCE); + const distance = Math.max(fitDistanceForCamera(bounds.radius, cameraRef.current), SKETCH_DISTANCE); const pose = buildFitCameraPose(bounds.center, distance); cameraRef.current.up.copy(pose.up); cameraRef.current.near = Math.max(distance / 500, 0.01); @@ -158,24 +173,37 @@ export function CameraHandler({ removeEventListener?: (type: string, fn: () => void) => void; } | null; if (!ctrl?.addEventListener) return; - const onUserInteractStart = () => { targetState.current = null; }; + const onUserInteractStart = () => { + // ...but an in-flight FIRST framing is not the user's orbit being + // fought — it is the only thing that has ever pointed the camera at + // the model, and it is gone for good if dropped (the fit is already + // recorded in `lastFitBounds`, so no later run will re-issue it). + if (targetState.current?.immediate) return; + targetState.current = null; + }; ctrl.addEventListener('start', onUserInteractStart); return () => ctrl.removeEventListener?.('start', onUserInteractStart); }, [controls]); useFrame((_state, delta) => { - if (!targetState.current) return; - const dampFactor = 5.0 * delta; - camera.position.lerp(targetState.current.position, dampFactor); + const target = targetState.current; + if (!target) return; + // A first framing is not animated: lerping at 1 sets the pose exactly. + const dampFactor = target.immediate ? 1 : 5.0 * delta; + camera.position.lerp(target.position, dampFactor); const ctrl = controls as unknown as { target: THREE.Vector3, update: () => void }; - if (ctrl && ctrl.target) { - ctrl.target.lerp(targetState.current.lookAt, dampFactor); - ctrl.update(); - } else { - camera.lookAt(targetState.current.lookAt); + if (!ctrl?.target) { + // OrbitControls has not published itself to the store yet. Point the + // camera, but KEEP the pose pending: controls initialise their orbit + // target to the origin, so a framing retired before they exist gets + // silently re-aimed at (0,0,0) on their first update. + camera.lookAt(target.lookAt); + return; } - if (camera.position.distanceTo(targetState.current.position) < 0.1 && - (ctrl?.target?.distanceTo(targetState.current.lookAt) || 0) < 0.1) { + ctrl.target.lerp(target.lookAt, dampFactor); + ctrl.update(); + if (camera.position.distanceTo(target.position) < 0.1 && + ctrl.target.distanceTo(target.lookAt) < 0.1) { targetState.current = null; } }); diff --git a/src/studio/components/viewer/controllers/cameraPose.ts b/src/studio/components/viewer/controllers/cameraPose.ts index 0b9efd410..f93917a43 100644 --- a/src/studio/components/viewer/controllers/cameraPose.ts +++ b/src/studio/components/viewer/controllers/cameraPose.ts @@ -36,6 +36,47 @@ export function buildCameraPose( }; } +/** + * Distance at which a bounding sphere of `radius` sits fully inside a + * perspective frustum. + * + * The constant this replaces (`radius * 2.8`) was wrong on two counts. + * + * 1. It under-shoots even the vertical limit. Framing a *sphere* needs + * `radius / sin(fov/2)`; only a flat disc facing the camera needs + * `radius / tan(fov/2)`. At the viewer's 40° fov that is 2.924 vs 2.747 — + * 2.8 sits between them, so the model always overflowed slightly. + * 2. It ignores the aspect ratio. `camera.fov` is the VERTICAL angle, so the + * horizontal one is narrower on any viewport taller than it is wide, and + * the narrow axis is the one that crops. A 420x780 embed needs 5.2 * radius; + * 2.8 put the camera less than half far enough away. + * + * `margin` leaves a little air around the model instead of framing it + * edge-to-edge. + */ +export function fitDistance( + radius: number, + fovDegrees: number, + aspect: number, + margin = 1.08, +): number { + const safeFov = Number.isFinite(fovDegrees) && fovDegrees > 0 && fovDegrees < 180 + ? fovDegrees + : 40; + // Aspect is 0/NaN until the canvas has been measured once; a square + // viewport is the neutral assumption (it never under-shoots a wide one). + const safeAspect = Number.isFinite(aspect) && aspect > 0 ? aspect : 1; + const halfVertical = (safeFov * Math.PI) / 360; + const halfHorizontal = Math.atan(Math.tan(halfVertical) * safeAspect); + return (radius * margin) / Math.sin(Math.min(halfVertical, halfHorizontal)); +} + +/** Same, reading the fov/aspect off a camera that may not be perspective. */ +export function fitDistanceForCamera(radius: number, camera: THREE.Camera): number { + const perspective = camera as THREE.PerspectiveCamera; + return fitDistance(radius, perspective.fov, perspective.aspect); +} + export function buildFitCameraPose(center: THREE.Vector3, distance: number): CameraPose { const direction = new THREE.Vector3(1, 1, 0.75).normalize(); return {