OrbitControls: Set certain event listeners to non-passive #19782

WestLangley · 2020-07-03T02:09:18Z

... similar to the suggestion by @unphased in #10373 (comment).

It does seem to prevent the following warning in Chrome:

[Violation] Added non-passive event listener to a scroll-blocking 'wheel' event. Consider marking event handler as 'passive' to make the page more responsive.

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

unphased · 2020-07-03T02:14:49Z

Nice :)

would apply to the other controls as well
Not sure if it would make sense to revert Examples: Make domElement to a mandatory parameter for all controls. #17612 but it seems like marking them as explicitly non-passive makes it safe to attach these listeners to documentElement again.

Mugen87 · 2020-07-03T08:20:44Z

It does seem to prevent the following warning in Chrome:

How can I trigger this warning? I don't see it in the production link right now.

unphased · 2020-07-03T15:07:02Z

It does seem to prevent the following warning in Chrome:

How can I trigger this warning? I don't see it in the production link right now.

What's your chrome version? (Ellipsis menu, Help, About) Mine is Version 83.0.4103.116 (Official Build) (64-bit).

Also you must refresh the page after opening it. console does not show these messages if you open it after pageload.

Mugen87 · 2020-07-03T15:23:00Z

Um, I still don't see it 🤔 . I'm using Chrome 83.0.4103.116 on macOS.

unphased · 2020-07-03T15:28:38Z

I observe it on my macOS as well. Perhaps check if you have console filters activated.

Mugen87 · 2020-07-03T15:30:52Z

I have not changed the default settings. It looks like so:

unphased · 2020-07-03T15:33:18Z

Yeah... looks like it is a debug-level warning and only shows if you set all levels... just to the right of your screenshot i guess itll say "3 hidden". Hey thanks for the screenshot, I'm gonna go check out the three.js dev tools!

Mugen87 · 2020-07-03T16:01:38Z

Okay, they only appear if you ensure Verbose is checked.

trusktr

It'd be nice to allow event options to be passed in, which would override (merge with) the default options.

trusktr · 2020-07-03T22:09:13Z

Also, how does orbit controls work if rendering more than one scene or camera within the same WebGL context? For this reason, it may be also good to allow the DOM element to be optionally passed in, to give more optional control (f.e. cover the canvas with elements to handle events for different portions of the canvas).

mrdoob · 2020-07-04T02:13:57Z

examples/js/controls/OrbitControls.js

@@ -1125,11 +1125,11 @@ THREE.OrbitControls = function ( object, domElement ) {
 	scope.domElement.addEventListener( 'contextmenu', onContextMenu, false );

 	scope.domElement.addEventListener( 'mousedown', onMouseDown, false );
-	scope.domElement.addEventListener( 'wheel', onMouseWheel, false );
+	scope.domElement.addEventListener( 'wheel', onMouseWheel, { capture: false, passive: false } );


I can't find on the internets if capture is false by default...

For context, useCapture (the third parameter) is supposed to be false by default, but Firefox used to error a decade ago if it was not specified.

See this spec, but the relevant section is non-normative.

Thanks!

capture (a boolean, initially false)

I guess I can remove it from the options object then?

Since non-normative means the specifications in that section are recommendations, not requirements, I think it would be best if we specify the value we want.

I was reading a different section than you were.

When set to true, options’s capture prevents callback from being invoked when the event’s eventPhase attribute value is BUBBLING_PHASE. When false (or not present), ...

So I guess it should just be this?

scope.domElement.addEventListener( 'wheel', onMouseWheel, { passive: false } );

PS: I realised I didn't explain the useCapture issue property. Firefox used to crash because the third parameter was not passed.

unphased · 2020-07-04T08:15:37Z

Also, how does orbit controls work if rendering more than one scene or camera within the same WebGL context? For this reason, it may be also good to allow the DOM element to be optionally passed in

I think this is already the case. That’s what scope.domElement is in the controls code.

WestLangley · 2020-07-07T14:02:08Z

This is easily reverted. Let's try it early in the dev cycle so we can get feedback.

WestLangley · 2020-09-02T23:53:14Z

This PR eliminates the Chrome warnings if there is no DAT.GUI -- but not if there is one...

mrdoob · 2020-09-03T08:38:07Z

Hmmm... I can't see any warnings on Chrome 84.0.4147.135...

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

Took a screenshot after scrolling on the view:

WestLangley · 2020-09-03T15:28:27Z

Try #19782 (comment).

mrdoob · 2020-09-03T16:29:56Z

@WestLangley Yeah! As you can see in the screenshot I have all levels selected. Including verbose.

mrdoob · 2020-09-03T16:31:41Z

Oh wait, I can reproduce it from the examples page:

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

WestLangley · 2020-09-28T22:06:15Z

When the console sidebar is visible in Chrome 85, 'verbose' appears to be hardwired, and the warnings appear.

My current workaround to avoid the warnings is to hide the console sidebar and toggle-off 'verbose'.

WestLangley · 2021-02-01T01:30:08Z

It doesn't appear there is interest in this, so closing...

bananu7 · 2021-03-03T21:08:13Z

Um, this is pretty weird. I found this page looking for PRs that would fix the [Violation] warnings that are pretty annoying, only to find that this has been... dropped? I mean, it's a trivial change, what's preventing this from being merged in?

Set certain event listeners to non-passive

b32f7c4

trusktr reviewed Jul 3, 2020

View reviewed changes

mrdoob reviewed Jul 4, 2020

View reviewed changes

WestLangley added this to the r119 milestone Jul 7, 2020

mrdoob modified the milestones: r119, r120 Jul 29, 2020

mrdoob modified the milestones: r120, r121 Aug 25, 2020

WestLangley added 2 commits September 2, 2020 19:30

Merge branch 'dev' into dev_orbit_controls_passive

e89fc38

set wheel listener to non-passive

ec9d2bc

mrdoob modified the milestones: r121, r122 Sep 29, 2020

mrdoob removed this from the r122 milestone Oct 28, 2020

mrdoob added this to the r123 milestone Oct 28, 2020

mrdoob mentioned this pull request Nov 22, 2020

Support passive event listeners in controls #10373

Closed

mrdoob modified the milestones: r123, r124 Nov 25, 2020

mrdoob modified the milestones: r124, r125 Dec 24, 2020

mrdoob modified the milestones: r125, r126 Jan 27, 2021

WestLangley closed this Feb 1, 2021

WestLangley removed this from the r126 milestone Feb 2, 2021

WestLangley deleted the dev_orbit_controls_passive branch February 3, 2021 20:56

Mugen87 mentioned this pull request Apr 13, 2021

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

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OrbitControls: Set certain event listeners to non-passive #19782

OrbitControls: Set certain event listeners to non-passive #19782

WestLangley commented Jul 3, 2020 •

edited

Loading

unphased commented Jul 3, 2020

Mugen87 commented Jul 3, 2020

unphased commented Jul 3, 2020 •

edited

Loading

Mugen87 commented Jul 3, 2020

unphased commented Jul 3, 2020

Mugen87 commented Jul 3, 2020

unphased commented Jul 3, 2020 •

edited

Loading

Mugen87 commented Jul 3, 2020

trusktr left a comment

trusktr commented Jul 3, 2020

mrdoob Jul 4, 2020

mrdoob Jul 4, 2020

WestLangley Jul 4, 2020

mrdoob Jul 4, 2020

WestLangley Jul 4, 2020

WestLangley Jul 4, 2020

mrdoob Sep 3, 2020 •

edited

Loading

unphased commented Jul 4, 2020

WestLangley commented Jul 7, 2020

WestLangley commented Sep 2, 2020

mrdoob commented Sep 3, 2020

WestLangley commented Sep 3, 2020

mrdoob commented Sep 3, 2020

mrdoob commented Sep 3, 2020

WestLangley commented Sep 28, 2020

WestLangley commented Feb 1, 2021

bananu7 commented Mar 3, 2021

OrbitControls: Set certain event listeners to non-passive #19782

OrbitControls: Set certain event listeners to non-passive #19782

Conversation

WestLangley commented Jul 3, 2020 • edited Loading

unphased commented Jul 3, 2020

Mugen87 commented Jul 3, 2020

unphased commented Jul 3, 2020 • edited Loading

Mugen87 commented Jul 3, 2020

unphased commented Jul 3, 2020

Mugen87 commented Jul 3, 2020

unphased commented Jul 3, 2020 • edited Loading

Mugen87 commented Jul 3, 2020

trusktr left a comment

Choose a reason for hiding this comment

trusktr commented Jul 3, 2020

mrdoob Jul 4, 2020

Choose a reason for hiding this comment

mrdoob Jul 4, 2020

Choose a reason for hiding this comment

WestLangley Jul 4, 2020

Choose a reason for hiding this comment

mrdoob Jul 4, 2020

Choose a reason for hiding this comment

WestLangley Jul 4, 2020

Choose a reason for hiding this comment

WestLangley Jul 4, 2020

Choose a reason for hiding this comment

mrdoob Sep 3, 2020 • edited Loading

Choose a reason for hiding this comment

unphased commented Jul 4, 2020

WestLangley commented Jul 7, 2020

WestLangley commented Sep 2, 2020

mrdoob commented Sep 3, 2020

WestLangley commented Sep 3, 2020

mrdoob commented Sep 3, 2020

mrdoob commented Sep 3, 2020

WestLangley commented Sep 28, 2020

WestLangley commented Feb 1, 2021

bananu7 commented Mar 3, 2021

WestLangley commented Jul 3, 2020 •

edited

Loading

unphased commented Jul 3, 2020 •

edited

Loading

unphased commented Jul 3, 2020 •

edited

Loading

mrdoob Sep 3, 2020 •

edited

Loading