Skip to content

Leverage XRViewGeometry instead of XRView #60

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Apr 17, 2025
Merged

Leverage XRViewGeometry instead of XRView #60

merged 5 commits into from
Apr 17, 2025

Conversation

alcooper91
Copy link
Contributor

@alcooper91 alcooper91 commented Apr 9, 2025 •
edited by pr-preview bot
Loading

Addresses #59

Preview | Diff

@alcooper91 alcooper91 requested a review from cabanier April 9, 2025 21:34
@alcooper91
Copy link
Contributor Author

@cabanier thoughts? I started to implement this in Chrome and encountered some of the circular references and began noticing other APIs that probably shouldn't be able to make use of this new XRView.

@alcooper91
Copy link
Contributor Author

/agenda

@probot-label probot-label bot added the agenda label Apr 11, 2025
@cabanier
Copy link
Member

@cabanier thoughts? I started to implement this in Chrome and encountered some of the circular references and began noticing other APIs that probably shouldn't be able to make use of this new XRView.

I noticed those APIs as well when I was implementing this. Some of the functions on xrview also don't make sense so +1 on this chang.

cabanier
cabanier reviewed Apr 11, 2025
View reviewed changes
bialpio
bialpio reviewed Apr 15, 2025
View reviewed changes
bialpio
bialpio reviewed Apr 15, 2025
View reviewed changes
@Yonet Yonet removed the agenda label Apr 15, 2025
@alcooper91 alcooper91 force-pushed the alcooper/depth_view branch from 585adb9 to 5f87b75 Compare April 16, 2025 21:32
@alcooper91
Copy link
Contributor Author

This is now "based on" immersive-web/webxr#1408 (name needs finalization), but is how I think the text should look once that lands. There may be a rename of XRViewCalibration/some other cleanups; but this should provide a good idea of the intended usage.

@alcooper91
Copy link
Contributor Author

@cabanier I'm going to hold off on merging the core spec change until you have a chance to look at this, just in case any concepts end up needing more/less definitions.

@alcooper91 alcooper91 requested a review from cabanier April 17, 2025 18:41
@alcooper91 alcooper91 changed the title Introduce XRDepthView instead of XRView Leverage XRViewGeometry instead of XRView Apr 17, 2025
@alcooper91 alcooper91 merged commit df9b71e into main Apr 17, 2025
2 checks passed
github-actions bot added a commit that referenced this pull request Apr 17, 2025
@alcooper91 @github-actions
Leverage XRViewGeometry instead of XRView (#60)
b5f4114 
SHA: df9b71e
Reason: push, by alcooper91

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@alcooper91 alcooper91 deleted the alcooper/depth_view branch May 1, 2025 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bialpio bialpio bialpio left review comments

@cabanier cabanier cabanier approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@alcooper91 @cabanier @bialpio @Yonet