Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

OrbitControls stays in incorrect state after pointercancel event #25505

Closed
molnarm opened this issue Feb 14, 2023 · 8 comments · Fixed by #25540 or pmndrs/three-stdlib#365
Closed

OrbitControls stays in incorrect state after pointercancel event #25505

molnarm opened this issue Feb 14, 2023 · 8 comments · Fixed by #25540 or pmndrs/three-stdlib#365
Labels
Bug

Comments

@molnarm
Copy link

molnarm commented Feb 14, 2023

Description

Opening a page with OrbitControls on an iPad can result in several errors when moving the scene with two fingers closely pinched together.

Probably it's the same in any case where pointercancel would be fired, but this is the scenario which we can consistently reproduce on multiple devices. Surface tables or Android phones don't fire this event (in this case at least).

Reproduction steps

  1. Open a page with OrbitControls (even the official example works) on an iPad (tested on multiple devices, the latest is iOS 16.3 Safari)
  2. Add a global error listener (window.addEventListener('error', e=>{...}))
  3. Start panning the scene with two fingers pinched closely together

The error handler will fire several times.

Code

The error triggered regardless of user code.

As far as I could tell, the problem is that in OrbitControls.js, in onPointerCancel, the pointer is removed, but state is not changed. So if we were in the middle of a panning or zooming gesture for example (state = TOUCH_DOLLY_PAN), the next call to onPointerMove will eventually end up in getSecondPointerPosition, which will throw when trying to read the position of the nonexistent second pointer.

Live example

Screenshots

No response

Version

r149

Device

Mobile

Browser

Safari

OS

iOS
@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 20, 2023

with two fingers closely pinched together.

So the pointercancel is fired on iOS when two fingers are moved closely together? Does that mean Webkit interprets this setup as a single pointer? Besides, pointerup isn't fired in this scenario, right?

I wonder if this issue can be fixed by simply adding state = STATE.NONE; in onPointerCancel()?

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 20, 2023 •
edited

I've added the line to a branch for testing? Can you still reproduce the issue with the following link?

https://rawcdn.githack.com/Mugen87/three.js/d0d953fe1c71ad38a1096b7cd25c00e264bd2f2a/examples/misc_controls_orbit.html

@molnarm
Copy link
Author

molnarm commented Feb 20, 2023

No, that change fixes this issue. Thanks for the quick response.

As for your questions: I didn't find any specifics on what exactly triggers pointercancel, but we can reproduce it this way consistently. It probably happens when the two touches move so close that they "merge" into one, but that's just my guess.

Besides, pointerup isn't fired in this scenario, right?

Yes. In fact, that is my current workaround: trigger a fake pointerup when getting pointercancel.

I wonder if this issue can be fixed by simply adding state = STATE.NONE; in onPointerCancel()?

I'm not very familiar with the details of OrbitControls, but probably the other steps taken in onPointerUp (removing event listeners, dispatching end event) should be done too, except maybe the pointer capture release?

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 20, 2023 •
edited

I'm not very familiar with the details of OrbitControls, but probably the other steps taken in onPointerUp (removing event listeners, dispatching end event) should be done too, except maybe the pointer capture release?

Yes, that makes sense. We probably can remove the onPointerCancel() function and just register the onPointerUp() listener for the event. Meaning:

scope.domElement.addEventListener( 'pointercancel', onPointerUp );

@Mugen87 Mugen87 mentioned this issue Feb 21, 2023
Merged
@optimus007
Copy link

is this related ?

On android when two fingers are active and i use the third finger to a system action (like pull down notification drawer) orbitcontrols gets stuck in a locked state

renn.mp4

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 24, 2023

@optimus007 Does this still happen with: https://rawcdn.githack.com/Mugen87/three.js/dev75/examples/misc_controls_orbit.html

@optimus007
Copy link

Yeah, .. still happens

Record_2023-02-24-15-25-18.mp4

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 24, 2023 •
edited

Then this seems to be a different problem. It's best to file a new issue.

esquevin pushed a commit to esquevin/three-stdlib that referenced this issue May 16, 2024
fix(OrbitControls): upstream fixes
66aa085 
Uses https://github.com/mrdoob/three.js/pull/25540/files to fix mrdoob/three.js#25505
@esquevin esquevin mentioned this issue May 16, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug
Projects
None yet
Milestone
No milestone
3 participants
@molnarm @optimus007 @Mugen87