InputManager: Fix Picking on PointerUp and add bool to skip pointerup picking #12524
Conversation
PolygonalSun
requested review from
deltakosh,
CedricGuillemet,
sebavan and
bghgary
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
|
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://babylonsnapshots.z22.web.core.windows.net/refs/pull/12524/merge/index.html#WGZLGJ#4600 Links to test babylon tools with this snapshot: https://playground.babylonjs.com/?snapshot=refs/pull/12524/merge To test the snapshot in the playground with a playground ID add it after the snapshot query string: https://playground.babylonjs.com/?snapshot=refs/pull/12524/merge#BCU1XR#0 |
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
|
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://babylonsnapshots.z22.web.core.windows.net/refs/pull/12524/merge/index.html#WGZLGJ#4600 Links to test babylon tools with this snapshot: https://playground.babylonjs.com/?snapshot=refs/pull/12524/merge To test the snapshot in the playground with a playground ID add it after the snapshot query string: https://playground.babylonjs.com/?snapshot=refs/pull/12524/merge#BCU1XR#0 |
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
|
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://babylonsnapshots.z22.web.core.windows.net/refs/pull/12524/merge/index.html#WGZLGJ#4600 Links to test babylon tools with this snapshot: https://playground.babylonjs.com/?snapshot=refs/pull/12524/merge To test the snapshot in the playground with a playground ID add it after the snapshot query string: https://playground.babylonjs.com/?snapshot=refs/pull/12524/merge#BCU1XR#0 |
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
|
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://babylonsnapshots.z22.web.core.windows.net/refs/pull/12524/merge/index.html#WGZLGJ#4600 Links to test babylon tools with this snapshot: https://playground.babylonjs.com/?snapshot=refs/pull/12524/merge To test the snapshot in the playground with a playground ID add it after the snapshot query string: https://playground.babylonjs.com/?snapshot=refs/pull/12524/merge#BCU1XR#0 |
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
|
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://babylonsnapshots.z22.web.core.windows.net/refs/pull/12524/merge/index.html#WGZLGJ#4600 Links to test babylon tools with this snapshot: https://playground.babylonjs.com/?snapshot=refs/pull/12524/merge To test the snapshot in the playground with a playground ID add it after the snapshot query string: https://playground.babylonjs.com/?snapshot=refs/pull/12524/merge#BCU1XR#0 |
There was a problem hiding this comment.
Seems like pick is still occuring
Though, I tested (I believe) this commit and it worked
Test pg:
https://playground.babylonjs.com/?snapshot=refs/pull/12524/merge#CASE7T#3
|
@PolygonalSun could you double check ? I can repro the picking being set on pointer up |
Okay, so this pick isn't directly apart of the picking done by up, down, or move but is triggered as part of |
|
This is a different issue than the one you were addressing in here. I would suggest to make it another PR. I do not think the flag combination is the right approach for it. This would probably need a separate way to turn it off ? @deltakosh what do you think ? |
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
|
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://babylonsnapshots.z22.web.core.windows.net/refs/pull/12524/merge/index.html#WGZLGJ#4600 Links to test babylon tools with this snapshot: https://playground.babylonjs.com/?snapshot=refs/pull/12524/merge To test the snapshot in the playground with a playground ID add it after the snapshot query string: https://playground.babylonjs.com/?snapshot=refs/pull/12524/merge#BCU1XR#0 |
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
|
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://babylonsnapshots.z22.web.core.windows.net/refs/pull/12524/merge/index.html#WGZLGJ#4600 Links to test babylon tools with this snapshot: https://playground.babylonjs.com/?snapshot=refs/pull/12524/merge To test the snapshot in the playground with a playground ID add it after the snapshot query string: https://playground.babylonjs.com/?snapshot=refs/pull/12524/merge#BCU1XR#0 |
|
Current findings: All of the skips disable picking as they should and the pointerUpPredicate is now being used properly in _initActionManager. One thing that I've noticed is that the code for |
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
|
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://babylonsnapshots.z22.web.core.windows.net/refs/pull/12524/merge/index.html#WGZLGJ#4600 Links to test babylon tools with this snapshot: https://playground.babylonjs.com/?snapshot=refs/pull/12524/merge To test the snapshot in the playground with a playground ID add it after the snapshot query string: https://playground.babylonjs.com/?snapshot=refs/pull/12524/merge#BCU1XR#0 |
Recently on the forums (forum post), there was a user who found an issue with the pointerUpPredicate not being used by the InputManager properly. When a dummy predicate was used (
() => false), there was still a pickedmesh/point in the pickInfo object. This PR does a couple of things:pointerUpPredicateand actually callscene.pick