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.

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

WebXR: Add support for hand tracking #88411

Merged
merged 1 commit into from
Feb 23, 2024

Conversation

dsnopek
Copy link
Contributor

@dsnopek dsnopek commented Feb 16, 2024

This adds support for hand tracking in WebXR!

It only gives direct access to the raw hand-tracking data, in a way that's similar to OpenXRInterface. It doesn't add a node similar to OpenXRHand to make animating a skeleton from hand tracking data easier - I'll address that in a follow up PR.

UPDATE: This is now rebuilt on top of PR #88639, and so will be marked as a draft until that PR is merged first.

It works in my testing on the Meta Quest 3!

Closes https://github.com/godotengine/internal-team-priorities/issues/38

@BastiaanOlij
Copy link
Contributor

Cool! But I'm going to take my reaction into the XR contributors channel because I think we need to change tack here :)

modules/webxr/doc_classes/WebXRInterface.xml Outdated Show resolved Hide resolved
modules/webxr/doc_classes/WebXRInterface.xml Outdated Show resolved Hide resolved
modules/webxr/doc_classes/WebXRInterface.xml Outdated Show resolved Hide resolved
modules/webxr/doc_classes/WebXRInterface.xml Outdated Show resolved Hide resolved
modules/webxr/doc_classes/WebXRInterface.xml Outdated Show resolved Hide resolved
modules/webxr/doc_classes/WebXRInterface.xml Outdated Show resolved Hide resolved
@dsnopek dsnopek marked this pull request as draft February 21, 2024 20:41
@dsnopek dsnopek force-pushed the webxr-hand-tracking branch 2 times, most recently from d9a3f94 to cd22cd0 Compare February 21, 2024 20:47
@dsnopek
Copy link
Contributor Author

dsnopek commented Feb 21, 2024

This is now rebuilt on top of PR #88639, and so will be marked as a draft until that PR is merged first.

(BTW, XRHandModifier3D seems to be working in WebXR with these changes :-))

@BastiaanOlij
Copy link
Contributor

Not really an expert on webXR, code looks great and I trust you when you say it works :)

@dsnopek dsnopek marked this pull request as ready for review February 23, 2024 21:31
@dsnopek
Copy link
Contributor Author

dsnopek commented Feb 23, 2024

PR #88639 was just merged! 🚀 So, rebasing this one and taking out of draft again

@akien-mga akien-mga merged commit cf9de66 into godotengine:master Feb 23, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@akien-mga
Copy link
Member

akien-mga commented Feb 23, 2024

For some reason, this PR didn't fail the CI checks, but once merged we got a JS linter failure: https://github.com/godotengine/godot/actions/runs/8025645968/job/21926705818

/home/runner/work/godot/godot/modules/webxr/native/library_godot_webxr.js
  322:104  error  Strings must use singlequote         quotes
  570:42   error  Expected '!==' and instead saw '!='  eqeqeq
  570:63   error  Expected '!==' and instead saw '!='  eqeqeq

Edit: Oh, our logic doesn't expect any JS outside platform/web...

if grep -q "platform/web" changed.txt || [ -z "$(cat changed.txt)" ]; then
    cd platform/web
    npm ci
    npm run lint
    npm run docs -- -d dry-run
  else
    echo "Skipping JavaScript formatting as no Web/JS files were changed."
  fi

Should probably grep for .js extension instead.

@dsnopek
Copy link
Contributor Author

dsnopek commented Feb 23, 2024

Eep! I'll make a PR to fix the linting issues in a moment.

@akien-mga
Copy link
Member

If you can include this, I think it should fix running the linter on PRs:

diff --git a/.github/workflows/static_checks.yml b/.github/workflows/static_checks.yml
index e7ebd2fb64..cf8b0f4132 100644
--- a/.github/workflows/static_checks.yml
+++ b/.github/workflows/static_checks.yml
@@ -76,7 +76,7 @@ jobs:
 
       - name: JavaScript style and documentation checks via ESLint and JSDoc
         run: |
-          if grep -q "platform/web" changed.txt || [ -z "$(cat changed.txt)" ]; then
+          if grep -q "\.js" changed.txt || [ -z "$(cat changed.txt)" ]; then
             cd platform/web
             npm ci
             npm run lint

@dsnopek
Copy link
Contributor Author

dsnopek commented Feb 23, 2024

Sure! Here's the new PR: #88740

And it does appear to be running eslint on CI :-)

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

Successfully merging this pull request may close these issues.

4 participants