immersive-web / webxr-samples

Samples to demonstrate use of the WebXR Device API
https://immersive-web.github.io/webxr-samples/
MIT License
987 stars 474 forks source link

Create hand-input.html #120

Closed fordacious closed 3 years ago

fordacious commented 3 years ago

Added hand input sample.

The sample tracks individual joints and the tips of index fingers to demonstrate interaction.

@cabanier

fordacious commented 3 years ago

Yup, it works well. The input profile based hand models appeared to be at the wrong orientation, however I do not believe this is an issue with this sample.

cabanier commented 3 years ago

What do you mean by "wrong orientation"? Does the scene not match your actual hands on quest?

fordacious commented 3 years ago

The model that is added to the scene by the input profile rendering library is oriented incorrectly and does not match the hands orientation. It is also not articulated.

https://lafordgithubtesting.github.io/webxr-samples/hands.html?usePolyfill=0

cabanier commented 3 years ago

I see. Yes, I think nobody is treating hands like a regular controller. Maybe it's best to disable the hand model

fordacious commented 3 years ago

Do you mean just disabling the input profile models entirely? Or is there a way to specifically disable them for hands?

cabanier commented 3 years ago

just for hands. I don't know if it's possible with the input profile library.

fordacious commented 3 years ago

There is no code in this sample that specifically deals with the hand models so we'll probably just have to not use the input profile library in that case

fordacious commented 3 years ago

Done

cabanier commented 3 years ago

I didn't realize this is for the webxr-samples repo. Do you want to replace immersive-hands.html with this file? Otherwise people won't be able to get to it from the landing page.

fordacious commented 3 years ago

Ah good point. I was going to add a new link as it is no longer a "proposal"

cabanier commented 3 years ago

We can that as a separate PR since we might want to discuss it first.

fordacious commented 3 years ago

SGTM

fordacious commented 3 years ago

@toji

toji commented 3 years ago

Comparing immersive-hands.html to this one, they both accomplish pretty much the same thing but I find the presentation of immersive-hands.html a little bit nicer (since it provides a scene rather than just a void) and I'd prefer to continue to support it since it's been part of the repo longer. (I was mildly surprised to see that the existing sample even supports AR sessions!) The only thing it really lacks is the interaction demonstration, which is pretty minimal.

So what I'd suggest is this:

fordacious commented 3 years ago

A Hegelian synthesis SGTM. I'll make the change here.

fordacious commented 3 years ago

Updated. @cabanier I just ran it on Quest and I noticed the local space origin is on the floor, which I believe is incorrect.

cabanier commented 3 years ago

Can I see it in immersive-hands.html?

fordacious commented 3 years ago

yup

fordacious commented 3 years ago

the old immersive-hands.html had an artificial offset of 1.5m. Removing that offset in this version makes it apparent.

cabanier commented 3 years ago

This is indeed a bug in the Oculus browser. I can't believe that this wasn't found before :-\ I will investigate this.

fordacious commented 3 years ago

@toji how is it looking?

cabanier commented 3 years ago

@fordacious feel free to pull this in. That way I can use it as a test case. I fixed the bug on our end but it will need a thorough test cycle.

toji commented 3 years ago

Did some additional digging on the local space bug, because it was unfathomable to me that the local space was completely broken all this time.

Turns out that local is fine unless you have either local-floor or bounded-floor in the session creation options optionalFeatures array. Most samples that use local space don't request those, which is why this bug doesn't usually show up. I imagine that when those features are requested the browser is passing something to the underlying API to say that the "native" space is floor-relative, and it's just not offsetting the local space properly.

For what it's worth I would typically say that those features should be removed from the session request because they're not used by this sample and I don't want developers to copy/paste unnecessary feature requests into their own code. I'm fine leaving it here as a testing case for Rik and co.

toji commented 3 years ago

Forgot to say in the previous comment: Thanks for merging this with the existing sample, and pushing it to the main page! And hey, finding bugs in the process is always a plus! :)

cabanier commented 3 years ago

Indeed. The code checked if those spaces were requested and then moved the origin to the floor. I don't know why that was done as requesting support for the floor shouldn't shift the origin.

I have a fix but it won't be in the upcoming browser (=next week). It will likely ship 2-3 weeks from now.

cabanier commented 3 years ago

@fordacious we'll send you a free Quest if you find 2 more bugs :-)

toji commented 3 years ago

Hey, if free hardware is up for grabs do I get credit for discovering that WebXR is incompatible with desks? 😆

cabanier commented 3 years ago

Hey, if free hardware is up for grabs do I get credit for discovering that WebXR is incompatible with desks? 😆

3 more bugs! :-)