Opt-in rotation of portrait video onto the process canvas - #345
Open
kalwalt wants to merge 1 commit into
Open
Conversation
Letterboxing a portrait video into the fixed 4:3 process canvas leaves the NFT tracker with far less usable width, which pushes markers below the 3-point tracking floor sooner. When videoSettings.rotatePortrait is enabled and the stream is portrait, the frame is now rotated 90 degrees onto the process canvas instead, and the projection matrix is rotated back to compensate so the AR content keeps the correct orientation. Landscape streams and the default (flag off) behavior are unaffected. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
Summary
Closes/relates to #344.
CameraViewRenderer.prepareImage()fits the camera stream into a fixed 320x240 (4:3) process canvas. For a portrait stream this letterboxes it heavily, leaving the NFT tracker much less usable width and pushing markers below thetracking.c3-point floor sooner than in landscape. This was measured and reported in #344 (~2.3x more tracking points when rotating the frame instead of letterboxing it).This adds an opt-in
videoSettings.rotatePortraitflag (defaultfalse, no behavior change unless set). When enabled and the stream is portrait (vh > vw):CameraViewRendererrotates the frame 90 degrees onto the process canvas instead of letterboxing it (destination footprint is identical to the non-rotated case, only the source draw is rotated).NFTWorkerrotates the projection matrix back so the AR content isn't rendered rotated 90 degrees relative to the (correctly oriented, untouched) visible video.NFTWorkernow receives the actual runtime video dimensions fromCameraViewRenderer(previously it received the requested config width/height, which isn't guaranteed to match what the device actually negotiated).The shared canvas-fitting math (previously duplicated between
CameraViewRenderer.prepareImage()andARnftUtils.getWindowSize()) is now a singlegetProcessGeometry()helper, so both stay in sync going forward.Caveat — please verify on-device
The rotation-direction math for the projection matrix compensation was derived from the canvas transform used here (not copy-pasted from the reporter's own patch, which has a different pipeline), and verified with
tsc/prettier, but not verified end-to-end on a real portrait camera stream with an NFT marker — I don't have a device to test with. If the AR content comes out rotated the wrong way on-device, the fix is a one-line sign flip inNFTWorker.ts/NFTWorker.simd.ts, in theif (rot) { ... }block:Test plan
tsc --noEmitpassesprettier --checkpasses on changed files🤖 Generated with Claude Code