Improve pinching accuracy (fixes #4689) #4691
Conversation
The default start pinch distance is updated to 0.015 (from 0.01)
| @@ -25,7 +25,8 @@ Use `hand-tracking-controls` to integrate [hand tracked input][webxrhandinput] i | |||
| | hand | The hand that will be tracked (i.e., right, left). | left | | |||
| | modelColor | Color of hand material. | white | | |||
| | modelStyle | Mesh representing the hand or dots matching the joints | mesh | |||
|
|
|||
| | startPinchDistance | Maximum distance between tip of thumb and index finger before a pinch gesture is started. | 0.015 | | |||
There was a problem hiding this comment.
Should we make it configurable? Is not a value that clearly works better for most cases?
There was a problem hiding this comment.
This is the best value I've found from my personal tests--happy to hardcode for that. The even better approach would use the radius property of the joint poses, but maybe best to wait until we have more than one example piece of hardware to determine the best number using that. If you confirm, I'll remove both configurable properties.
There was a problem hiding this comment.
We can remove the properties, use 0.015 and adjust later if needed. Just keep the API simpler. Still a lot to learn about hand tracking 😃
There was a problem hiding this comment.
The latest commit removes the properties.
| @@ -8,6 +8,8 @@ suite('tracked-controls-webxr', function () { | |||
| var standingMatrix = new THREE.Matrix4(); | |||
| var index = {transform: {position: {x: 0, y: 0, z: 0}}}; | |||
| var thumb = {transform: {position: {x: 0, y: 0, z: 0}}}; | |||
| var indexPosition = new THREE.Vector3(); | |||
| var thumbPosition = new THREE.Vector3(); | |||
There was a problem hiding this comment.
Thanks for taking care of the tests 🏅
| @@ -217,22 +217,22 @@ module.exports.Component = registerComponent('hand-tracking-controls', { | |||
|
|
|||
| var distance = indexTipPosition.distanceTo(thumbTipPosition); | |||
|
|
|||
| if (distance < 0.01 && this.isPinched === false) { | |||
| if (distance < 0.015 && this.isPinched === false) { | |||
There was a problem hiding this comment.
We can maybe move this to a variable at the top var PINCH_DISTANCE = 0.015 . We use similar pattern in other components
There was a problem hiding this comment.
Addressed in the latest commit
| this.isPinched = true; | ||
| this.pinchEventDetail.position.copy(indexTipPose.transform.position); | ||
| this.pinchEventDetail.position.copy(indexTipPosition).lerp(thumbTipPosition, 0.5); |
There was a problem hiding this comment.
What is the 0.5 value for? Maybe define another variable on top?
|
Thanks! Congrats on your first PR 🥳 |
Description: As discussed in #4689, this PR improves pinching accuracy by 1) adjusting the distance between thumb and index finger needed to start the gesture, and by 2) moving the position pinch events report to the halfway point between the thumb and index finger.
Changes proposed:
0.015.hand-tracking-controls:startPinchDistance, which defaults to0.015andendPinchDistance, which defaults to0.03.positionproperty forpinchstarted,pinchmoved, andpinchendedevents is moved to the halfway point between thumb and index fingertips.