-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
3DOF support in tracked-controls, daydream-controls, head/arm model #2538
Conversation
src/components/gearvr-controls.js
Outdated
@@ -0,0 +1,201 @@ | |||
var registerComponent = require('../core/component').registerComponent; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should back the GearVR pieces until the GearVR controllers are released, then we can focus on Daydream for this PR.
is there a need to back it out? won't do anything unless you explicitly enable zero-DOF, and then you presumably want it to do something (with the current trackpad) |
We can't test the 3DOF GearVR until it's released, and it's code baggage until then. Would make PR quick to test/review. The 0DOF trackpad style of interaction will start to lose relevance and in the long-term completely disappear, I'd leave it as a community feature. |
if we think that 0.6.0 will be shipped before the GearVR controllers are available, I would agree. but the controller release is happening quite soon (3 weeks from now) |
We're shipping sooner than that. 3 weeks is forever, and then there's the time to wait in line for the controllers and wait for shipment. |
ok, let me see if i can split into this PR for controllers we can exercise now (Daydream; any other 3DOF gamepads?) and a separate one for GearVR once we can confirm with those. Ideally we would be able to confirm support to be available before or at launch, not afterward. (The existing code in this PR may just work.) |
Cool, thanks. GearVR's not too urgent, the controllers will take time to roll out amongst the GearVR + WebVR developer community and consumers as well. |
src/components/tracked-controls.js
Outdated
headElement: {type: 'selector'}, | ||
hand: {type: 'string', default: 'right'}, | ||
eyesToElbow: {default: {x: 0.175, y: -0.3, z: -0.03}}, // vector from eyes to elbow (divided by user height) | ||
forearm: {default: {x: 0, y: 0, z: -0.175}}, // vector from eyes to elbow (divided by user height) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to expose this on the component API? Cannot these values be derived from user height?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
those are scaled by user height, yes. but there need to be base values to utilize. they are in the schema in case one wants to override to better match something else being modeled e.g. robot with longer or shorter arms
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would not expose them at the beginning and make them just private contsts. If there's demand we can make them public later on. I prefer to start with a simple, minimal public API
src/components/tracked-controls.js
Outdated
hand: {type: 'string', default: 'right'}, | ||
eyesToElbow: {default: {x: 0.175, y: -0.3, z: -0.03}}, // vector from eyes to elbow (divided by user height) | ||
forearm: {default: {x: 0, y: 0, z: -0.175}}, // vector from eyes to elbow (divided by user height) | ||
defaultUserHeight: {type: 'number', default: 1.6} // default user height (for cameras with zero) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have 1.6 as default user height in other places in the code. Can we consolidate the value on a single global variable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it tries to read the value used by camera first before it touches this value. if you're willing to expose an accessor, can call it. may not be always appropriate as hardcoded constant, for example rift knows what height you entered regardless of roomscale...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can maybe do an AFRAME.defaultUserHeight
variable that components can use to be consistent?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was thinking maybe AFRAME.utils.defaultUserHeight() so that if it becomes an inherited VR preference and not a constant, the contract doesn't need to change
src/components/tracked-controls.js
Outdated
rotationOffset: {default: 0}, | ||
// Arm model parameters, to use when not 6DOF. (pose hasPosition false, no position) | ||
headElement: {type: 'selector'}, | ||
hand: {type: 'string', default: 'right'}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tracked controls should be hand agnostic. I'm not comfortable with having the concept of hand
around here. I start to think that tracked-controls
is trying to cover too much ground. What about a separate 3dof-controls
component?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we consolidate all these new properties to a single one 3dof: { default: false }
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is really only the arm-model that needs to know which hand to use (or none) - it is intended to be passthrough like rotationOffset for hand-controls. other ways to specify arm-model? not sure 3dof-controls is a good idea since you have to modify recorders, teleporters, etc. ... but maybe if the arm model can be a separate component (order of operations problem when updating pose though) and tracked-controls still holds the resultant position and rotation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use case : hand controls with teleporter; two hands if touch or vive, one hand if daydream (or gearvr) 3DOF, and you can enable zero DOF if you really want... to do the one hand, you need to know which hand you should be treating the single 3DOF controller to be, or else the arm model will be rather broken.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thiking that 3dof-controls
would underneath rely on tracked-controls
so teleport, motion capture would not be affected and for zero DOF I would leave it out for this PR and reevaluate later if it's something we really want as kevin mentioned
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In daydream then handedness is configured by the user. It should not be the application's job. Should the handedness be reported by the gamepad API instead of being configurable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't remember picking daydream handedness, but may be forgetting.
for vive and oculus, gamepad API does expose hand (although originally vive hand value was unreliable so wasn't used; might be able to change that now)
I don't think gamepad API provides hand for daydream, so maybe this would be defaultHand (to use when gamepad API does not specify) ?
I do think we need headElement, unless there is some better way to automatically select the head element/entity to use with arm model?
I don't think you can actually use 3dof since it starts with a number, but maybe three-dof-controls (and/or zero-dof-controls)... the trick there being that tracked-controls needs to avoid updating pose, because another component is going to do it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking that
3dof-controls
would underneath rely ontracked-controls
If tracked-controls
is a dependency, then three-dof-controls
needs to replicate all its data for passthrough (yuck) and it's not clear when it should be used.
If done as a related component, then higher layers like hand-controls
need to know when to specify three-dof-controls
, and three-dof-controls
needs to be able to configure tracked-controls
to call its updatePose
instead of doing its normal processing.
034cb13
to
32b9048
Compare
src/components/tracked-controls.js
Outdated
rotationOffset: {default: 0}, | ||
// Arm model parameters, to use when not 6DOF. (pose hasPosition false, no position) | ||
headElement: {type: 'selector'}, | ||
defaultHand: {type: 'string', default: 'right'} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can also hide this as a constant. I've talked to someone working with the daydream SDK and you can get the handedness configured by the user. The info should be passed to the browser via the hand
attribute on the gamepad object similarly to the Oculus controllers. It should not be an application configurable property.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the current version of PR will use gamepad hand if available first (thus property was renamed defaultHand)
you can't hide rotationOffset as it is passed in from the other higher level components such as hand-controls, and it varies based upon the model those higher level components used (NOT constant)
if you are saying you'd like to make defaultHand a constant (e.g. always default to right if not defined) that is possible, but maybe not best?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(the question is less whether handedness is available from daydream sdk, and more whether browser passes it on through gamepad API.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I meant is that if the gamepad does not give us the hand we use right
by default. The property is not exposed for the applications to change. The hand should be given by the gamepad API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so internalize defaultHand, ok makes sense (although maybe should be configurable somehow like on tracked-controls system?) as just discussed. am currently fighting with windows 10 update, if you guys want to do that before I can get to it, go right ahead
(got the change in between reboots.) |
@machenmusik the controller motions are pretty laggy/slow to update. Is the pose data coming through slow, or is this something else? Also noticed this with the Also regarding handedness. Shouldn't this be exposed in the controller API? The component should set that up for you. |
handedness: yeah we discussed just above, #2538 (review) laggy/slow - when I was using, there was a bit of delay after doing daydream enter-vr dance and then it would settle down to be fast. what were you using, got a URL / codepen / glitch / etc.? |
(if you are using more complicated models with daydream controller, daydream GPU is still way worse than any desktop so you may be running into scene complexity issues?) |
…ot the active camera e.g. spectator view
…ant; hand => defaultHand
14dbf82
to
9f756a2
Compare
src/components/daydream-controls.js
Outdated
*/ | ||
module.exports.Component = registerComponent('daydream-controls', { | ||
schema: { | ||
hand: {default: 'right'}, // This informs the degenerate arm model. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should hide the hand
property so we take whatever handness comes from the Gamepad API
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for oculus and vive, it's passed down from e.g. hand-controls to be used as a filter. not sure it is appropriate to remove, if the platform provides hand then we want to be able to filter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, the current code isn't applying the hand filter -- but really it should if gamepad provides a value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@machenmusik It looks like the handedness is settable from within the daydream settings menu. Unsure whether this is passed down into the controller API, but ideally it should be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@machenmusik According to Klaus, the handedness should be on gamepad.hand
as per https://w3c.github.io/gamepad/extensions.html
I did take a look, and I don't see the property on the gamepad object as of Canary 59.0.3062.0. Though, we should be looking for it and setting handedness from this. I would for now even make the default assumption of right-handedness and optionally setting left.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
navigator.getGamepads()[0].hand
works -- my bad (user error in dev tools)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok so then we do want hand on daydream-controls, to use as filter, and it should be placed on both hand-controls like oculus and vive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a single entity + controller component:
<a-entity daydream-controls></a-entity>
Should default to whatever getGamepad.hand
returns, unless the hand
property is specified, in which case it overrides. Sound about right? @dmarcos?
As far as hand-controls
. The arm model puts the hand too close into the body and leaves the other hand floating. The Daydream controller works sufficiently different enough from VIVE and Touch, I question trying to make it work with hand-controls at all. At the very least, we would want to tweak the arm model so that it is more useable as a hand.
The laser pointer type control would probably be the better primary control model for daydream-controller (rather than hand-controls):
<a-entity daydream-controller>
<a-cone id="ray" color="cyan" position="0 0 -2" rotation="-90 0 0" radius-bottom="0.005" radius-top="0.001" height="4"></a-cone>
</a-entity>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should behave as you describe with latest PR. does it not?
The arm model puts the hand too close into the body and leaves the other hand floating.
w.r.t. leaving the other hand floating - that is because the hands are left visible by default, rather than only making visible when controller is detected. easy exercise left to the reader...
w.r.t. arm model - it was configurable, @dmarcos wanted to hide the settings :-/
can you adjust the parameters to something that works better for you, that we can try as well?
I had a codepen that was doing laser pointer, but you definitely need an arm model or that feels very broken.
src/components/tracked-controls.js
Outdated
// The controller is not 6DOF, so apply arm model. | ||
// Use controllerPosition and deltaControllerPosition to avoid creating yet more variables... | ||
|
||
// Use camera position as head position. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we put all this code in its own method applyArmModel(controllerPosition)
?
src/constants/index.js
Outdated
@@ -1,6 +1,7 @@ | |||
module.exports = { | |||
AFRAME_INJECTED: 'aframe-injected', | |||
DEFAULT_CAMERA_HEIGHT: 1.6, | |||
DEFAULT_HAND: 'right', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is DEFAULT_HANDEDNESS
probably a better name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the property is called hand
, not sure it helps to make the constant have different name than the property for which it is the default value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but the hand
property is something gamepad
specific. This is a global variable at the library level. DEFAULT_HAND
does not give you much info when looking at the list of global variables with no context.
This is pretty close! :) Thanks for the patience |
@machenmusik I don't think it's the model. Though I will experiment by taking it out to see if that helps to improve performance. I'll whip up a a-frame example that works with the controller since the existing motion controls one doesn't work for 3dof. |
@machenmusik update: definetly not the model. removing it doesn't seem to help performance. I should also mention that in-headset, the performance is good and it seems to be just the controller that laggy/slow. |
@dmarcos mentioned yesterday that three.js does the same thing so doesn't seem like A-Frame has much to do with it (maybe browser) |
Could it be the browser not sending the pose frequent enough? |
OK, so some further testing. It seems like the pose updates are just not very frequent. I see this also in three.js examples as well. So it's not something specific to the work here. After some more testing, I definetly see some lag in native daydream apps as well, so it's definetly possible that it's a limitation with the controller itself. It could be that they are doing some kind of interpolation with the pose that makes it less noticeable. Edit: Guess what I'm saying is that the perf issues are not something that should hold this patch back. It seems that there is more of a general problem here to solve with the controller. |
yes that was the conclusion @dmarcos and I came to yesterday (perf not us, hand --> defaultHand) -- looks like some editorial nits but very close now |
…hand, so use it to filter, and attach to both hand-controls
ok? |
Thanks! I'm going to merge this but I think we need a better example to illustrate the capabilities of a 3DOF a controller. |
Woo, thanks @mchen! When you have time, can you throw in some unit tests too? |
This PR adds support to
tracked-controls
for non-6DOF controllers, for example the Google Daydream controller, by using camera position and a primitive arm model to emulate positional data. As they should now be supported, this PR also addsdaydream-controls
andgearvr-controls
alongsideoculus-touch-controls
andvive-controls
. Lastly, it includes support for allowing zero-DOF controllers, for example Gear VR Touchpad, by using camera orientation (disabled by default, but enabled forexamples/showcase/tracked-controls/
since otherwise that does nothing in zero-DOF cases).