Repository navigation
Keep the 3D preview camera outside the model and frame both projections alike - #145
Open
officialasishkumar wants to merge 2 commits into
Open
officialasishkumar wants to merge 2 commits into
officialasishkumar wants to merge 2 commits into
Conversation
…ns alike The 3D preview camera could move inside the model, where FrontSide materials leave an empty or clipped view: - Perspective: OrbitControls has no minDistance, so scrolling moves the camera all the way to the orbit target at the model's center. - Orthographic -> Perspective: zooming far in while orthographic and switching back places the perspective camera at the matching distance, which can be inside the mesh. The camera now stays outside the model's bounding box plus a margin of twice the framing near plane, so the near plane cannot clip the surface it stops at. A bounding-sphere minDistance would be too coarse for flat slabs (a 140 x 140 x 3 mm print has a ~99 mm radius), so the box itself is the limit: - A perspective camera that ends up inside the box moves back along its direction from the orbit target to the box surface. This covers dollying, panning (which also moves the target), and mode switches. - An orthographic camera's zoom is capped where the equivalent perspective camera from syncPerspectiveFromOrthographic would reach the box, so switching back never lands inside the model either. useThreeScene applies the limit after every OrbitControls update, and the limit is refreshed whenever a rebuilt model is framed. Auto-framing also used two hand-written branches that disagreed: the orthographic branch used a 2.5r view height and a fixed 0.01 near plane, about 15% tighter than perspective framing. Framing now lives in src/lib/cameraProjection.ts: framePerspectiveCamera frames the model in perspective terms, and an active orthographic camera is converted with matchOrthographicToPerspective, the in-place form of orthographicFromPerspective. Both projections now frame a rebuilt model identically, and the hook owns the cameras through a new frameModel callback instead of ThreeDView editing them directly. Fixes vycdev#127 Fixes vycdev#128
Codex Review Summary: ❌ FailedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…ra-inside-model # Conflicts: # CHANGELOG.md
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The 3D preview camera could move inside the model, where the
FrontSidematerials leave an empty or clipped view (#127). Auto-framing also used two hand-written branches that framed a rebuilt model differently in Perspective and Orthographic (#128). As #128 suggests, both are done together because they touch the same auto-frame code.Keeping the camera outside the model (#127)
The camera now stays outside the model's bounding box plus a margin of twice the framing near plane, so the near plane can't clip the surface it stops at. As the issue notes, a bounding-sphere
minDistancewould block close-ups of flat slabs, so the limit is the box itself:syncPerspectiveFromOrthographicwould reach the box. This is the matchingmaxZoomlimit, recomputed for the current view direction, so switching back never puts the camera inside the model.useThreeSceneapplies the limit after every OrbitControls update. The limit is refreshed whenever a rebuilt model is framed and only applies while a model is shown.One framing path for both projections (#128)
framePerspectiveCamerainsrc/lib/cameraProjection.tsframes the model in perspective terms, with the same angle, padding, and near and far planes as before. If Orthographic is active, the camera is converted withmatchOrthographicToPerspective, the in-place form oforthographicFromPerspectivethat also resetszoom. The separate Orthographic branch, with its 2.5r view height and fixed 0.01 near plane, is gone. ThreeDView's auto-frame block becomes a singleframeModel(box)call to the hook, which owns the cameras.Added an Unreleased changelog entry under Fixed.
Fixes #127
Fixes #128
Validation:
tests/cameraProjection.test.tshas 11 new tests that use real three.js cameras. They cover: framing fits the padded bounding sphere; the near-plane minimum for tiny models; Orthographic framing matches Perspective framing even when an old zoom is left over; the keep-out margin; dollying and panning into a 140 × 140 × 3 mm slab returns the camera to the surface along its view direction; close-up inspection 10 mm above the slab is still allowed; a camera sitting on its target backs out along its view axis; the Orthographic zoom cap; and the mode-switch regression. That last test asserts that the unconstrained switch enters the model, then checks that the limit puts the camera on the box surface.develop: scrolling in Perspective put the camera on the orbit target inside the slab (distance 0). Orthographic zoom grew without limit (about 1.3e7), and switching back left the camera inside. Rebuilding in Orthographic framed at distance 113 instead of 160, and switching to Perspective then gave 137.npm run lint,npx tsc -b,npm run check:i18n: passed.npm test: 1116 passed, 1 skipped, 0 failed.@smokee2e specssmooth-meshing-strengthandrelief-layer-preview(both build 3D models): passed.