Support passive event listeners in controls

[winstliu]

Description of the feature

Since Chrome 51 and Firefox 49, passive event listeners have been supported. Marking the listeners in the controls as passive should greatly help perceived performance. A polyfill is available in the link if backwards-compatibility is important.

EDIT: I should note that this disables the ability to call preventDefault(). So listeners that call preventDefault() cannot be passive.

Three.js version

All

[mrdoob]

Usually the reason we use event.preventDefault() is because we want to avoid mousewheel to scroll the page when we're zooming in the scene.

[brianchirls]

I'd like to bump this one, as I'm seeing a bunch of warnings in the Chrome (Canary) console. Passive event listeners could/should be applied to most mousewheel and touchstart events in many of the examples as well as:

• OrbitControls
• OrthographicTrackballControls
• TrackballControls
• EditorControls

I'm not sure how you would want to structure this, exactly, since it requires a bit of feature detection. You might want to place the feature detection code in a common utility file that you can reference from all the different control scripts.

Here's the code I use in my own private repo. Feel free to adapt or use this:

let eventOpts = false;
function nop() {}
try {
	const opts = {};
	Object.defineProperty(opts, 'passive', {
		get () {
			eventOpts = {
				passive: true
			};
		}
	});
	window.addEventListener('test', nop, opts);
	window.removeEventListener('test', nop, opts);
} catch (e) {}

module.exports = eventOpts;

Then use it like this:

element.addEventListener('touchstart', touchStart, eventOpts);

[mrdoob]

I don't think those warnings apply to our case though...

[brianchirls]

Why not?

If I understand it correctly (which, admittedly, I may not), you don't need to set the option if you're calling event.preventDefault(). You alluded to the fact in your previous comment. But we're not calling that in many of the touch event handlers. So let's do one or the other.

[mrdoob]

I'm confused...

[brianchirls]

Me too. It's gonna be okay. Let's all have some coffee, give it a think, read up on it and reconsider.

I'm sure somewhere there's some data about the number of milliseconds of delay this is causing that will help us justify a change.

[arodic]

This is a very confusing spec. If I'm getting it right, passive is used to improve the performance of browser's native scrolling which can be done in a separate thread. However, all controls in three.js use listeners to extract event data and apply it in javascript context. Moreover, if you have three.js canvas in a scrollable page, you probably want the preventDefault() on scroll anyway.

[Kaiido]

https://www.chromestatus.com/features/6662647093133312

Wheel events attached to the document or body are now automatically treated as being passive.
You might actually want to implement this issue's proposal but to force non-passive listeners, otherwise, you'll be unable to call preventDefault().

Ps: (This was reported at https://stackoverflow.com/questions/58282573/ , I didn't check where you do attach such events, but it seems at least TrackballControls is affected).

[Mugen87]

but it seems at least TrackballControls is affected).

Yes, and OrbitControls/MapControls, too. We can hopefully resolve this issue with #17612

[unphased]

Hi @Mugen87. I think that #17612 addresses the
Error: Unable to preventDefault inside passive event listener due to target being treated as passive,
however the
[Violation] Added non-passive event listener to a scroll-blocking 'touchmove' event. Consider marking event handler as 'passive' to make the page more responsive
warning in Chrome is very much still happening.

I believe @Kaiido's comment is correct.

• A passive event listener is an optimization the browser encourages you to apply, in the case of when you know the listener will never preventDefault to affect scroll.
• For Three.js we want to use a non-passive listener (a disruptive listener?) because usually these listeners' whole point is to facilitate scene interaction.

By specifying them as passive: false in the controls source code, that will appease Chrome and remove this violation.

The change to binding them to the canvas instead of to the body is also fine to have, but I wonder if anyone knows whether Chrome will honor it if we mark it passive: false. If that is the case and prevents Error: Unable to preventDefault, maybe we should still allow assigning the listener to body, because that way you can interact with the scene with your mouse over other parts of the page, which some use cases may call for.

[unphased]

I have done a test consisting of this commit: unphased@729ea92, and this cleans up all violation related messages in the console (aside from one related to a wheel event for the iframe, which i'm ignoring for the test). based on this, I think it shows that the api does not need to make specifying the domelement mandatory...

[mrdoob]

@unphased can you share a jsfiddle that shows the violation?

[unphased]

You can observe it on chrome on any of the controls examples, such as this: https://threejs.org/examples/misc_controls_orbit.html Just open console and reload.

[mrdoob]

Ah I see...

[duhaime]

Should this issue be reopened? The trackball controls in 0.121.1 are still throwing this warning

[mrdoob]

@duhaime Not sure.