search

aframevr / aframe

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

Duplicate pinch events on the Quest Pro #5505

Open zakaton opened 3 months ago

zakaton commented 3 months ago

Performing a single pinch emits multiple "pinchstarted" and "pinchended" events on the Quest Pro (I don't have any other headsets to test this on). Here's a video of the issue comparing A-Frame 1.5.0 and 1.4.0

<html>
  <head>
    <title>A-Frame pinch issue on Quest Pro</title>
    <script src="https://aframe.io/releases/1.5.0/aframe.min.js"></script>
  </head>

  <body>
    <a-scene>
      <a-entity id="rightHand" hand-tracking-controls="hand: right;"></a-entity>

      <a-camera>
        <a-entity position="0 0 -1">
          <a-text
          id="numberOfPinches"
          color="black"
          align="center"
          value="0 pinches"
        ></a-text>
        <a-plane color="white" position="0 0 -0.01" height="0.3" width="1.1"></a-plane>
        </a-entity>

        <a-sky hide-on-enter-ar color="grey"></a-sky>
      </a-camera>
    </a-scene>

    <script>
      const rightHand = document.getElementById("rightHand");
      let numberOfPinches = 0;
      const numberOfPinchesEntity = document.getElementById("numberOfPinches");
      rightHand.addEventListener("pinchstarted", (event) => {
        numberOfPinches++;
        console.log({ numberOfPinches });
        numberOfPinchesEntity.setAttribute(
          "value",
          `${numberOfPinches} pinches`
        );
      });
    </script>
  </body>
</html>
dmarcos commented 3 months ago

Thanks. Super helpful if someone can also verify on Quest 2 / 3.

mrxz commented 3 months ago

Very useful test, thanks! I can reproduce the behaviour on a Quest 3 and managed to bisect it to the following commit (8341329d7112116d127c62b8aa4ebefba0539124):

8341329d7112116d127c62b8aa4ebefba0539124 is the first bad commit
commit 8341329d7112116d127c62b8aa4ebefba0539124
Author: Diego Marcos Segura <diego.marcos@gmail.com>
Date:   Wed Nov 8 19:27:19 2023 -0800

    Refine hand tracking controls pinch logic. passes wrist position as event detail. pinchended triggers when pinch distance is more than 10% of pinchstarted one

 src/components/hand-tracking-controls.js | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

Looking at the commit in question it seems to drastically lower the pinchend threshold. From a fixed 0.03 to anywhere ranging from 0.0-0.0165. This means that it could be below the fixed pinchstart threshold of 0.015, which would explain the multiple start/end events being fired. Not sure what the motivation behind the change was, but at the very least the end threshold should never dip below the start threshold.

dmarcos commented 2 months ago

Thanks. That change addressed stickiness when manipulating objects directly with hand. With old logic the objects remained attached / grabbed for quite a bit as the pinch gesture released and before the event pinchended fired, causing objects to move unintentionally and making impossible to accurately place them.

We need to address the double events.