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

DragControls/OrbitControls/TrackballControls: wheel/touchmove/touchstart add { passive: false } #21642

Merged
merged 3 commits into from Apr 13, 2021

Conversation

puxiao
Copy link
Contributor

@puxiao puxiao commented Apr 13, 2021

Related issue: null

Description

What?

https://threejs.org/examples/#misc_controls_orbit

violation_messages


Why?

https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener

passive

A Boolean that, if true, indicates that the function specified by listener will never call preventDefault(). If a passive listener does call preventDefault(), the user agent will do nothing other than generate a console warning.

https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#improving_scrolling_performance_with_passive_listeners

Improving scrolling performance with passive listeners

To prevent this problem, some browsers (specifically, Chrome and Firefox) have changed the default value of the passive option to true for the touchstart and touchmove events on the document-level nodes Window, Document, and Document.body. This prevents the event listener from being called, so it can't block page rendering while the user is scrolling.

Browser compatibility

passive_default


So...

DragControls.js

-	_domElement.addEventListener( 'touchmove', onTouchMove );
-	_domElement.addEventListener( 'touchstart', onTouchStart );

+	_domElement.addEventListener( 'touchmove', onTouchMove, { passive: false } );
+	_domElement.addEventListener( 'touchstart', onTouchStart, { passive: false } );

OrbitControls.js

-	scope.domElement.removeEventListener( 'wheel', onMouseWheel );
-	scope.domElement.removeEventListener( 'touchstart', onTouchStart );
-	scope.domElement.removeEventListener( 'touchmove', onTouchMove );

+	scope.domElement.addEventListener( 'wheel', onMouseWheel, { passive: false } );
+	scope.domElement.addEventListener( 'touchstart', onTouchStart, { passive: false } );
+	scope.domElement.addEventListener( 'touchmove', onTouchMove, { passive: false } );

TrackballControls.js

-	this.domElement.addEventListener( 'wheel', mousewheel );
-	this.domElement.addEventListener( 'touchstart', touchstart );
-	this.domElement.addEventListener( 'touchmove', touchmove );

+	this.domElement.addEventListener( 'wheel', mousewheel, { passive: false } );
+	this.domElement.addEventListener( 'touchstart', touchstart, { passive: false } );
+	this.domElement.addEventListener( 'touchmove', touchmove, { passive: false } );

@Mugen87
Copy link
Collaborator

Mugen87 commented Apr 13, 2021

Related #19782.

I guess the last PR was not merged because of some confusion about capture. I guess this looks more what @mrdoob had in mind.

@mrdoob mrdoob added this to the r128 milestone Apr 13, 2021
@mrdoob mrdoob merged commit 4036a22 into mrdoob:dev Apr 13, 2021
@mrdoob
Copy link
Owner

mrdoob commented Apr 13, 2021

Thanks!

@mrdoob
Copy link
Owner

mrdoob commented Apr 13, 2021

/cc @WestLangley

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.

None yet

3 participants