Feature/orthographic camera toggle by nishantkluhera · Pull Request #560 · tscircuit/3d-viewer

nishantkluhera · 2025-11-09T20:27:34Z

Feature: Orthographic Camera Toggle
Overview
Adds orthographic camera support to the 3D viewer with seamless switching between perspective and orthographic projection modes. Users can toggle between camera types while maintaining their current view position and orientation.

Functionality

Camera Support: Full implementation of orthographic camera
Seamless Switchin: Toggle between camera types without losing view context
Smart State Preservation: Automatically saves and restores camera position, orientation, and zoom when switching modes
Zoom Conversion: Converts perspective FOV to equivalent orthographic zoom for consistent visual scale
Dynamic Frustum Sizing: Automatically calculates optimal orthographic frustum based on board dimensions

Testing Scenarios

Camera Type Switching:

Perspective → Orthographic: Preserves view, adjusts zoom
Orthographic → Perspective: Preserves position and orientation
Camera positioning presets work on both camera

- Fix camera state leaking across circuit changes by only restoring state when explicitly switching camera types - Remove expensive JSON.stringify for viewerKey that caused UI lag - Optimize Vector3 cloning in useFrame to reduce GC pressure - Improve orthographic frustum size calculation using boardDimensions - Ensure stable defaultTarget reference to prevent unnecessary remounts - Combine duplicate useFrame hooks for better performance

vercel · 2025-11-09T20:27:41Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
3d-viewer	Ready	Preview	Comment	Nov 11, 2025 9:49pm

src/react-three/Canvas.tsx

seveibar · 2025-11-09T20:42:18Z

src/hooks/useCameraController.ts

 }

 export const useCameraController = ({
+  key,


Can you remove key and make this more meaningful? Key is a bad arg

i could rename it to something like isOrthographic to make it clear what its tracking

seveibar

This seems too complex, can we simplify this

seveibar · 2025-11-09T20:43:37Z

In particular, avoid adding LoC to components like CadViewer or CadViewerContainer

…the camera perset position preview not updating

graphite-app · 2025-11-09T21:29:43Z

src/react-three/Canvas.tsx

+            const toOrigin = new THREE.Vector3()
+              .sub(camera.position)
+              .normalize()


The vector calculation for toOrigin contains a logic error. When initializing an empty vector with new THREE.Vector3() (which creates a zero vector) and then subtracting camera.position from it, the result will be the negative of the camera position. To properly calculate a vector from the origin to the camera, use:

const toOrigin = new THREE.Vector3().subVectors(new THREE.Vector3(), camera.position).normalize()

Or more simply:

const toOrigin = camera.position.clone().negate().normalize()

This ensures the vector points from the origin toward the camera position.

Spotted by Graphite Agent

Is this helpful? React 👍 or 👎 to let us know.

nailoo · 2025-11-10T14:52:13Z

src/react-three/Canvas.tsx

+    }, [
+      scene,
+      addFrameListener,
+      removeFrameListener,
+      cameraProps?.type,
+      cameraProps?.frustumSize,
+    ])


}, [ scene, addFrameListener, removeFrameListener, cameraProps?.type, cameraProps?.frustumSize, cameraProps?.position, cameraProps?.up, ])

seveibar · 2025-11-10T20:42:43Z

src/hooks/useCameraController.ts

      onReady: handleControllerReady,
    }),
-    [defaultTarget, handleControllerReady],
+    // eslint-disable-next-line react-hooks/exhaustive-deps


we don't use eslint, so you shouldn't need these (make sure you're not using eslint on this project, because the default rules are not compatible with biome)

my bad!! will keep that in mind and also remove those

seveibar · 2025-11-10T20:43:30Z

src/react-three/Canvas.tsx

-      )
+      const width = mountRef.current.clientWidth || 1
+      const height = mountRef.current.clientHeight || 1
+      renderer.setSize(width, height)


why is stuff like this changing?

from what i remember, those specific lines aren't functionally changing, they're appearing in the diff because the entire useEffect was modified to add camera state preservation logic (refs on lines 45-58, state saving on lines 89-114, restoration on lines 142-227). The renderer sizing code itself is unchanged, just showing as context in the larger diff

seveibar · 2025-11-10T20:46:33Z

i'm mostly ready to merge this BUT the sideviews are wrong for the camera position

https://share.cleanshot.com/7kZbKfxZ

also we need to fix the merge conflicts

nishantkluhera · 2025-11-11T21:25:32Z

fixing the eslint thing after this

graphite-app · 2025-11-11T21:26:40Z

src/react-three/Canvas.tsx

+          let target: THREE.Vector3
+          if (stateToRestore.target) {
+            target = stateToRestore.target.clone()
+          } else {
+            const forward = new THREE.Vector3(0, 0, -1).applyQuaternion(
+              camera.quaternion,
+            )
+            const estimatedDistance = camera.position.length()
+            const toOrigin = new THREE.Vector3()
+              .sub(camera.position)
+              .normalize()
+            const dotWithForward = forward.dot(toOrigin)
+            if (dotWithForward > 0.5) {
+              target = new THREE.Vector3(0, 0, 0)
+            } else {
+              target = camera.position
+                .clone()
+                .add(forward.multiplyScalar(estimatedDistance))
+            }
+          }


Attempts to access stateToRestore.target but the saved state objects (lines 109-121) never include a target property. This check will always be false, causing the else branch to always execute and potentially calculate incorrect target positions.

// In the state saving logic (lines 109-121), add target: if (wasOrthographic) { lastOrthographicStateRef.current = { position: existingCamera.position.clone(), quaternion: existingCamera.quaternion.clone(), up: existingCamera.up.clone(), zoom, target: contextState?.controls?.target?.clone() ?? new THREE.Vector3() } }

Spotted by Graphite Agent

Is this helpful? React 👍 or 👎 to let us know.

Removed eslint-disable comments for exhaustive-deps.

Ayushjhawar8

Hey @nishantkluhera All the camera positions are not working / broken, and the view cube also seems to be broken.

nishantkluhera · 2025-11-12T09:38:36Z

Yeah I saw that, working on, the conflicts resolutions with the 3 files earlier caused these>

Hey @nishantkluhera
All the camera positions are not working / broken, and the view cube also seems to be broken.

seveibar and others added 7 commits October 31, 2025 10:12

Rename orthographic camera flag

af4da53

Fix orientation gizmo sync after orthographic toggle

d7e13b6

Ensure camera preset selection animates

44b9abc

Fix camera preset selection triggering main view animation

6f2f49b

Fix camera preset animation on main view

68ad057

fix: camera position animations work correctly in orthographic mode

ce342e6

nishantkluhera requested a review from ShiboSoftwareDev as a code owner November 9, 2025 20:27

vercel bot deployed to Preview November 9, 2025 20:28 View deployment

graphite-app bot reviewed Nov 9, 2025

View reviewed changes

src/react-three/Canvas.tsx Show resolved Hide resolved

seveibar reviewed Nov 9, 2025

View reviewed changes

refactor: simplify camera preset handling and remove complexity, fix …

5de6b54

…the camera perset position preview not updating

vercel bot deployed to Preview November 9, 2025 21:27 View deployment

graphite-app bot reviewed Nov 9, 2025

View reviewed changes

nailoo reviewed Nov 10, 2025

View reviewed changes

seveibar requested a review from Ayushjhawar8 November 10, 2025 17:14

Ayushjhawar8 added the conflict label Nov 10, 2025

seveibar reviewed Nov 10, 2025

View reviewed changes

Merge branch 'main' into feature/orthographic-camera-toggle

abdc552

vercel bot deployed to Preview November 11, 2025 21:23 View deployment

graphite-app bot reviewed Nov 11, 2025

View reviewed changes

Clean up eslint-disable comments in useCameraController

00d8fbe

Removed eslint-disable comments for exhaustive-deps.

vercel bot deployed to Preview November 11, 2025 21:42 View deployment

Clone target from contextState in camera state

59bd501

vercel bot deployed to Preview November 11, 2025 21:49 View deployment

Ayushjhawar8 removed the conflict label Nov 12, 2025

Ayushjhawar8 requested changes Nov 12, 2025

View reviewed changes

Conversation

nishantkluhera commented Nov 9, 2025

Uh oh!

vercel bot commented Nov 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

seveibar Nov 9, 2025

Choose a reason for hiding this comment

Uh oh!

nishantkluhera Nov 9, 2025

Choose a reason for hiding this comment

Uh oh!

seveibar left a comment

Choose a reason for hiding this comment

Uh oh!

seveibar commented Nov 9, 2025

Uh oh!

graphite-app bot Nov 9, 2025

Choose a reason for hiding this comment

Uh oh!

nishantkluhera Nov 9, 2025

Choose a reason for hiding this comment

Uh oh!

nailoo Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seveibar Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

nishantkluhera Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

seveibar Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

nishantkluhera Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

seveibar commented Nov 10, 2025

Uh oh!

nishantkluhera commented Nov 11, 2025

Uh oh!

graphite-app bot Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Ayushjhawar8 left a comment

Choose a reason for hiding this comment

Uh oh!

nishantkluhera commented Nov 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

vercel bot commented Nov 9, 2025 •

edited

Loading

nailoo Nov 10, 2025 •

edited

Loading