Skip to content
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

Rewrite OpenXR hand tracking to reflect new architecture #9499

Merged
merged 1 commit into from
Jul 13, 2024
Merged

Rewrite OpenXR hand tracking to reflect new architecture #9499

merged 1 commit into from
Jul 13, 2024

Conversation

BastiaanOlij
Copy link
Contributor

Work in progress, I'm rewriting the hand tracking manual page to reflect the new way things are structured in Godot 4.3.
About half way done.

Note that this manual page references godotengine/godot-demo-projects#973 and that PR needs to be merged alongside this.

@BastiaanOlij BastiaanOlij added topic:xr Related to XR documentation area:manual Issues and PRs related to the Manual/Tutorials section of the documentation labels Jun 17, 2024
@BastiaanOlij BastiaanOlij self-assigned this Jun 17, 2024
@skyace65 skyace65 added this to the 4.3 milestone Jun 17, 2024
@BastiaanOlij BastiaanOlij force-pushed the openxr_update_hand_tracking branch 2 times, most recently from a2fff80 to 81cf5e7 Compare June 18, 2024 10:39
@BastiaanOlij BastiaanOlij marked this pull request as ready for review June 18, 2024 10:39
AThousandShips
AThousandShips reviewed Jun 18, 2024
View reviewed changes
tutorials/xr/openxr_hand_tracking.rst Outdated Show resolved Hide resolved
tutorials/xr/openxr_hand_tracking.rst Outdated Show resolved Hide resolved
tutorials/xr/openxr_hand_tracking.rst Outdated Show resolved Hide resolved
tutorials/xr/openxr_body_tracking.rst Outdated Show resolved Hide resolved
tutorials/xr/openxr_hand_tracking.rst Outdated Show resolved Hide resolved
tutorials/xr/openxr_hand_tracking.rst Outdated Show resolved Hide resolved
tutorials/xr/openxr_hand_tracking.rst Outdated Show resolved Hide resolved
tutorials/xr/openxr_hand_tracking.rst Outdated Show resolved Hide resolved
tutorials/xr/openxr_hand_tracking.rst Outdated Show resolved Hide resolved
tutorials/xr/openxr_hand_tracking.rst Outdated Show resolved Hide resolved
dsnopek
dsnopek reviewed Jun 18, 2024
View reviewed changes
Copy link
Contributor

@dsnopek dsnopek left a comment

Choose a reason for hiding this comment

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

Thanks! This looks great!

I only have a couple language nit-picks.

tutorials/xr/openxr_hand_tracking.rst Outdated Show resolved Hide resolved
tutorials/xr/openxr_hand_tracking.rst Outdated Show resolved Hide resolved
tutorials/xr/openxr_hand_tracking.rst Outdated Show resolved Hide resolved
tutorials/xr/openxr_hand_tracking.rst Outdated Show resolved Hide resolved
tutorials/xr/openxr_hand_tracking.rst Outdated Show resolved Hide resolved
tutorials/xr/openxr_hand_tracking.rst Outdated Show resolved Hide resolved
tutorials/xr/openxr_hand_tracking.rst Outdated Show resolved Hide resolved
@BastiaanOlij BastiaanOlij force-pushed the openxr_update_hand_tracking branch 2 times, most recently from 98d0d28 to 110220b Compare June 19, 2024 02:02
@BastiaanOlij
Copy link
Contributor Author

I think I got all the reported grammar issue, thanks for the proof reading @AThousandShips and @dsnopek

skyace65
skyace65 requested changes Jun 19, 2024
View reviewed changes
tutorials/xr/openxr_hand_tracking.rst Outdated Show resolved Hide resolved
tutorials/xr/openxr_hand_tracking.rst Outdated Show resolved Hide resolved
tutorials/xr/openxr_hand_tracking.rst Outdated Show resolved Hide resolved
@BastiaanOlij BastiaanOlij force-pushed the openxr_update_hand_tracking branch from 110220b to 74e7338 Compare June 20, 2024 12:48
@BastiaanOlij BastiaanOlij force-pushed the openxr_update_hand_tracking branch from 74e7338 to f513020 Compare June 20, 2024 12:49
@BastiaanOlij BastiaanOlij requested a review from skyace65 June 20, 2024 12:50
@BastiaanOlij
Copy link
Contributor Author

Thanks for the approvals everyone, please also check out the demo project that accompanies this.

@skyace65
Copy link
Contributor

I can check that the demo runs on my computer later this week. Unfortunately though my headset doesn't have hand tracking.

@BastiaanOlij
Copy link
Contributor Author

I can check that the demo runs on my computer later this week. Unfortunately though my headset doesn't have hand tracking.

If you have steamVR it'll work on inferred hand tracking, all should still work :)

@mhilbrunner
Copy link
Member

This PR is good to go and just waiting on the demo to be merged. Unfortunately, I currently lack the bandwith to check it out myself. Please poke me once that PR is merged so we can merge this one :)

@skyace65 skyace65 merged commit 6842f41 into godotengine:master Jul 13, 2024
1 check passed
@skyace65
Copy link
Contributor

XR Demo is merged as well. Thanks for your work on both of these!

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

@AThousandShips AThousandShips AThousandShips approved these changes

@skyace65 skyace65 skyace65 approved these changes

@mhilbrunner mhilbrunner mhilbrunner approved these changes

@m4gr3d m4gr3d m4gr3d approved these changes

@dsnopek dsnopek dsnopek approved these changes

@Malcolmnixon Malcolmnixon Awaiting requested review from Malcolmnixon

Assignees

@BastiaanOlij BastiaanOlij

Labels
area:manual Issues and PRs related to the Manual/Tutorials section of the documentation topic:xr Related to XR documentation
Projects
None yet
Milestone
4.3
Development

Successfully merging this pull request may close these issues.
6 participants
@BastiaanOlij @skyace65 @mhilbrunner @dsnopek @m4gr3d @AThousandShips