PrePointerObservable pointerTap fired after drag when skipOnPointerObservable true

After this commit, prePointer pointerTap events are fired even after dragging, if a prePointer pointerMove observer has set skipOnPointerObservable to true. (PG)

To fix this I think these lines should be moved above the _checkPrePointerObservable call.

Another issue (though this is long standing) is that prePointer pointerUp observers aren’t called on pointerTap if skipOnPointerObservable has been set to true. The same is true for pointerDoubleTap. (PG)

These lines could be refactored to allow both up and tap/doubleTap events to be observed.


Finally, perhaps not an issue, but should pointerUp be called before pointerTap/pointerDoubleTap (for both prePointer and normal pointer observables)? For mouse events, the order is:

  • mousedown
  • mouseup
  • click
  • mousedown
  • mouseup
  • click
  • dblclick

For consistency, it might be nice to call the pointerEvent equivalent observers in a similar order (pointerDown → pointerUp → pointerTap).

I can do a pr for some or all of the above if required.

cc @PolygonalSun would be great to have a quick look

Lemme see what’s going on

1 Like

So I’ve been looking at what you’ve got here and here are my thoughts:

100% agree with your fix for this. I was initially worried that there may be side effects but there isn’t anything that stood out for me.

It does feel like this could be refactored to not have the POINTERTAP cancel/prevent the POINTERUP pre-check from occurring. I feel like this will also need to account for skipping the specific observable calls (TAP only skips TAP, UP only skips UP, etc)

TBH, I actually really like this, but this technically changes the expected behavior so I’d kinda wanna bring this to the team to see what their thoughts are on it.

I’ll try to get all of this addressed on Monday.

1 Like

Here’s an update:
So as far as I know, changing the order of the PointerEventTypes to more closely resemble the order of execution for PointerEvents isn’t going to be an issue. As far as the skipOnPointerObservable calls, the logic for this need to be that TAP can cancel TAP, and DOUBLETAP can cancel DOUBLETAP but UP cancels all three. This is due in part to backwards compatibility, at least as far as I understand. In any case, I’m currently testing the changes to make sure that there aren’t any side effects or issues that I’m not accounting for. I’ll keep you posted on the status of this.

Thanks for the update; sounds good.

By this do you just mean that the onPointerObservables that would be cancelled (i.e., _checkPrePointerObservable would be called for UP and TAP/DOUBLETAP even if one or both set skipOnPointerObservable to true)?

In the code found in the current release, what happens is that the _checkPrePointerObservable is call for TAP/DOUBLETAP, then UP. The first one to set skipOnPointerObservable to true will stop all up-input processing for both UP and TAP/DOUBLETAP.

These new changes would modify this by processing UP first, skipping everything if skipOnPointerObservable is true (similar to how it’s originally done). If TAP/DOUBLETAP set that flag then UP will still execute and the respective TAP will be skipped.

Here’s the PR: InputManager: Fix Order and Execution of onPrePointerObservable by PolygonalSun · Pull Request #13295 · BabylonJS/Babylon.js (github.com)