aframevr / aframe

:a: Web framework for building virtual reality experiences.
https://aframe.io/
MIT License
16.61k stars 3.94k forks source link

Add properties to hand-tracking-controls pinch events to be able to use pinch gesture to move objects #4690

Open cesmoak opened 4 years ago

cesmoak commented 4 years ago

Description: When using pinching to move objects, knowing which hand is pinching and what the orientation of the hand is in is useful so that the grabbed object can track the appropriate hand's position and rotation changes.

In my custom version of hand-tracking-controls, I have added in two additional properties to the existing event details for each pinch event:

I could see an argument for using the strings 'left' or 'right' instead of the hand element for the hand parameter. I could see an argument for using the orientation of the index finger or thumb (or some combination of the two) as an orientation for the hand (I haven't tried any but the wrist). What's useful for moving an object is a consistent relative reference frame over time, though, so the specific choice of orientation matters less for this use-case.

Question: If the wrist pose is null, should an event be fired? I currently don't fire if the pose is null.

I can provide a PR once finalized.

dmarcos commented 4 years ago

A reference to the element that emitted the event can be found in evt.target

 handEl.addEventListener('pinchstarted', function (evt) {
     var handEl = evt.target;
     ....
 }
cesmoak commented 4 years ago

True! What do you think about including an orientation in the event detail along with position?

dmarcos commented 4 years ago

One idea. We could forgo pinchmoved and use an entity state instead. Events that fire on a per frame basis add too much performance overhead. Instead we could have properties in the component wristWorldPosition, wristOrientation. Another component would consume the data in the following way:

tick: function () {
 if (handEl.is('pinched')) {
   var handPosition = handEl.components['hand-tracking-controls'].wristWorldPosition;
    .
    .
    .
 }
}

Would that work?

cesmoak commented 4 years ago

Removing pinchmoved in favor of tick and a pinched state works fine for my use-case.

Adding wristWorldPosition and wristOrientation as properties would work for moving objects. However, I can see cases where people will want access to other finger positions/poses. For example, in this same project, I also need to detect where the pad of a finger is and this requires the position and/or rotation of the phalanx distal and phalanx tip of a finger. In order to track that, I've created a component I call hand-joints that replicates some of the logic in hand-tracking-controls to track the other finger distal/tip poses I need. Assuming my situation will not be uncommon, here are a few potential options:

  1. Build for what is likely the most common case of picking up objects and add wrist pose to hand-tracking-controls; Leave other finger pose information as an exercise for the developer,
  2. Leave hands-tracking-controls as is; Create a hand-joints component that collects all poses (or a filtered list based on a component property) on each frame (or generates them on the fly as needed) and provides an accessor for them, or
  3. Add the common case wrist pose to hand-tracking-controls from 1; Also create the hand-joints component for other use-cases from 2

Separately, I figured out why I was led astray by event.target before: in the hand-tracking-controls example here: https://bristle-curly-primula.glitch.me/ the pinchable component re-fires the pinch* events but in doing so loses the original target element.

dmarcos commented 4 years ago

@cesmoak Thanks for all the ideas. I would go for option 1 to start with and expand as we see demand. I would probably add a property fingerTracking: true / false flag in hand-tracking-controls that is false by default to skip calculating the positions of the fingers if the developer doesn't need them.

cesmoak commented 4 years ago

Option 1 sounds good.

To clarify, your intention with the fingerTracking property is to:

  1. enable/disable finger tracking calculations used for pinching events, or
  2. enable/disable finger positions/orientations for all fingers, accessible as properties on the component, or
  3. something else?

A benefit of providing a single source for all joint data would be easy creation of additional gestures (and potentially pinching could be moved to its own component). Exposing the native pose object (i.e., for the radius property) along with the THREE version of the matrix/position/orientation, would allow other components to consume them. I've ended up creating a few gesture recognizer components and they benefit from having a single source of joint data. This also seems like it could be a whole new discussion as it is more than just pinching-related.

dmarcos commented 4 years ago

The name fingerTracking might not be the best. Maybe fingerPositionCalculation or jointPositionCalculation Calculating the positions is extra work done once per frame that it's better to avoid if dev doesn't need it.

Right now I'm only concerned about pinching events and wrist pose. The same flag could be reused to expose more joint poses in the future.