Provide generic interface for XR hand tracking #88639
Conversation
dsnopek
force-pushed
the
xrserver-hand-tracker
branch
from
e781eec to
059f519
Compare
| @@ -0,0 +1,223 @@ | |||
| <?xml version="1.0" encoding="UTF-8" ?> | |||
| <class name="XRHandTracker" inherits="RefCounted" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="../class.xsd"> | |||
There was a problem hiding this comment.
How about classifying it as experimental just in case?
| <class name="XRHandTracker" inherits="RefCounted" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="../class.xsd"> | |
| <class name="XRHandTracker" inherits="RefCounted" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="../class.xsd" experimental=""> |
There was a problem hiding this comment.
Perhaps. But I don't know if it makes sense to mark this as experimental if we're also deprecated the older APIs. I feel like we should do one or the other: Mark the old APIs as deprecated OR mark the new APIs as experimental, but not both.
There was a problem hiding this comment.
Hmm, I think it's overkill, probably the original OpenXR implementation should have been called experimental, but since this is all based on the experience we gathered with this, I think we have a pretty solid grasp on it.
dsnopek
force-pushed
the
xrserver-hand-tracker
branch
3 times, most recently
from
5950618 to
6d9d109
Compare
dsnopek
force-pushed
the
xrserver-hand-tracker
branch
from
6d9d109 to
101d0e0
Compare
dsnopek
force-pushed
the
xrserver-hand-tracker
branch
from
101d0e0 to
5f945e9
Compare
dsnopek
force-pushed
the
xrserver-hand-tracker
branch
from
5f945e9 to
d4fef4c
Compare
|
@Malcolmnixon @AThousandShips Thanks for the review! It should be addressed in my latest push |
There was a problem hiding this comment.
Perfect! Only two nitpicking things. This looks real good, thanks @dsnopek !
| </members> | ||
| <constants> | ||
| <constant name="HAND_LEFT" value="0" enum="Hand"> | ||
| A left hand. |
There was a problem hiding this comment.
I've wondered about this for awhile seeing this comes back in a number of APIs. Should we add this constant to [XRServer] itself?
There was a problem hiding this comment.
Hm. Well, before this PR we have two left/right hand enums:
XRPositionalTracker.TrackerHandOpenXRInterface.Hand
The 2nd of those is going to be deprecated after this PR, so I think we can mostly ignore it.
The 1st of those has different values: unknown (0), left (1) and right (2). That's a little inconvenient for hand tracking because their is no unknown and we can use left (0) and right (1) as array indexes, and the max value (2) for loops.
Given that, I feel like it's OK for XRHandTracker to have a hand enum that fits its purposes. And, even if we decided to consolidate to a single enum on XRServer, we can't change XRPositionalTracker to use it for backwards compatibility reasons. Maybe when we can break compatibility (like, Godot 5) we could consolidate on a single enum in XRServer? Although, given the different needs of the two enums, I personally still think I'd prefer to have the two of them for their different purposes.
dsnopek
force-pushed
the
xrserver-hand-tracker
branch
from
d4fef4c to
0cf3278
Compare
|
@BastiaanOlij Thanks for the review! |
dsnopek
force-pushed
the
xrserver-hand-tracker
branch
from
0cf3278 to
2184fa9
Compare
|
Thanks! |
This adds a generic interface for hand tracking similar to what PR #88312 added for face tracking, based on notes from @BastiaanOlij and @Malcolmnixon on RocketChat and Discord.
It deprecates
OpenXRHandand the methods for accessing raw hand tracking data onOpenXRInterface.While this does seem to work, it could use some testing - I plan to experiment with it some more myself, but testing from others would also be much appreciated as well!