Skip to content

fix: align iOS frame coordinates with camera sensor - #4175

Closed
huytdps13400 wants to merge 1 commit into
margelo:mainfrom
huytdps13400:fix/4114-ios-coordinate-space
Closed

fix: align iOS frame coordinates with camera sensor#4175
huytdps13400 wants to merge 1 commit into
margelo:mainfrom
huytdps13400:fix/4114-ios-coordinate-space

Conversation

@huytdps13400

Copy link
Copy Markdown

Summary\n\n- align iOS Frame and Depth coordinate conversion with the unrotated AVFoundation sensor space used by PreviewView\n- retain absolute buffer orientation and mirroring separately from the existing public output-relative metadata\n- preserve the legacy transform for Photo depth data, where no AVCaptureConnection is available\n\nThe regression test was already merged in #4113 and is red on main for iOS while Android passes.\n\n## Verification\n\n- bun camera typecheck\n- swift format lint on all six changed Swift files (exit 0; two pre-existing warnings)\n- xcodebuild -project Pods/Pods.xcodeproj -scheme VisionCamera -sdk iphonesimulator -destination generic/platform=iOS Simulator (arm64 and x86_64)\n- git diff --check\n- affine proof for all four buffer orientations, mirrored and unmirrored, including inverse round-trips\n\nThe full SimpleCamera build is not available locally because the installed Xcode lacks the optional Metal Toolchain required by the unrelated VisionCameraResizer shader target. The existing AWS Device Farm coordinate harness remains the end-to-end oracle for this PR.\n\nFixes #4114

Keep the physical buffer orientation separate from public output-relative metadata so Frame and Depth conversions use the same unrotated AVFoundation sensor space as PreviewView.\n\nFixes margelo#4114.
@vercel

vercel Bot commented Aug 26, 2026

Copy link
Copy Markdown

@huytdps13400 is attempting to deploy a commit to the Margelo Team on Vercel.

A member of the Team first needs to authorize it.

@mrousavy mrousavy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your PR, but this cannot be accepted as it contains a bunch of unnecessary refactors (naming is better that way, that's correct, but making it non-breaking causes lots of extra code), and one hardcoded orientation modification which is simply not true in production. This will break on tons of devices.

@huytdps13400

Copy link
Copy Markdown
Author

Reworked in #4177.

The fixed .left sensor-orientation assumption and the unnecessary converter rewrite from this PR are removed. The replacement reads each delivered buffer's actual AVCaptureConnection.videoRotationAngle on iOS 17+, keeps the existing fallback on older targets, and leaves public Frame.orientation / isMirrored semantics unchanged.

The replacement is limited to six iOS files. The existing converter is reused without modification. Local verification covers the old 3.16049 anisotropy versus the corrected 1.0 composition, the device-specific 180° default, typecheck, Swift formatting, and the VisionCamera simulator pod build for arm64/x86_64.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants