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.

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

OrbitControls: Fully migrate to pointer events. #21972

Merged
merged 3 commits into from
Jun 17, 2021
Merged

Conversation

Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Jun 10, 2021

Related issue: -

Description

This change removes all touch* listeners from OrbitControls and migrates the logic to pointer event listeners.

Handling multi-touch with pointer events is quite different compared to touch events since a single pointer event has no access to other pointers.

This PR also removes the calls of preventDefault() and introduces the usage of touchAction instead.

@Mugen87 Mugen87 marked this pull request as draft June 10, 2021 09:30
@Mugen87 Mugen87 marked this pull request as ready for review June 11, 2021 18:01
@Mugen87 Mugen87 marked this pull request as draft June 11, 2021 18:03
@Mugen87 Mugen87 marked this pull request as ready for review June 11, 2021 19:13
@@ -24,7 +24,8 @@
if ( domElement === undefined ) console.warn( 'THREE.OrbitControls: The second parameter "domElement" is now mandatory.' );
if ( domElement === document ) console.error( 'THREE.OrbitControls: "document" should not be used as the target "domElement". Please use "renderer.domElement" instead.' );
this.object = object;
this.domElement = domElement; // Set to false to disable this control
this.domElement = domElement;
this.domElement.style.touchAction = 'none'; // Set to false to disable this control
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is problematic.

Ideally controls set touchAction to none in pointerdown and undo the change in pointerup event listeners. Defining touchAction is necessary to make pointermove work on mobile.

DragControls and TransformControls do it this way. However, this approach does not work with OrbitControls since the class adds an event listener to contextmenu. It seems this listener prevents the usage of the above pattern since the pointermove event listener still aborts after a short period of time. Removing the contextmenu listener fixes the issue. This is however no appropriate solution since preventing the context menu is important for desktop.

Adding this.domElement.style.touchAction = 'none'; in the ctor makes OrbitControls incompatible with TransformControls though since the latter one resets the touchAction value in its pointerup event listener. One possible solution is to set touchAction to none in the ctor of OrbitControls, DragControls and TransformControls.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One possible solution is to set touchAction to none in the ctor of OrbitControls, DragControls and TransformControls.

Added this change with the last commit 9a4099f

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the best solution to me 👍

@mrdoob mrdoob added this to the r130 milestone Jun 17, 2021
@mrdoob mrdoob merged commit e82542f into mrdoob:dev Jun 17, 2021
@mrdoob
Copy link
Owner

mrdoob commented Jun 17, 2021

Thanks!

haywirez added a commit to haywirez/three-stdlib that referenced this pull request Jun 23, 2021
haywirez added a commit to haywirez/three-stdlib that referenced this pull request Jun 23, 2021
joshuaellis pushed a commit to pmndrs/three-stdlib that referenced this pull request Jun 24, 2021
Mercury21th000 pushed a commit to Mercury21th000/three-stdlib that referenced this pull request Nov 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants