1392876 - Enable coalescing mouse events to be once per refresh cycle (enable the pref)

Status: RESOLVED FIXED

(Core :: DOM: Events, enhancement, P3)

Comment 1

[(no longer active)]

Any idea what this is blocked on Stone?

Comment 2

[Ming-Chou Shih [:stone]]

(In reply to :Ehsan Akhgari (needinfo please, extremely long backlog, Away 8/23) from comment #1)
> Any idea what this is blocked on Stone?

Not sure if we should implement PointerEvent.getCoalescedEvents() before shipping this? Now we only enable PointerEvent on nightly. Also not sure if any other blockers.

Comment 3

[(no longer active)]

OK, makes sense.  Shipping pointer events is probably a bigger blocker at this point then!

Comment 4

[Chris Peterson [:cpeterson]]

(In reply to :Ehsan Akhgari (needinfo please, extremely long backlog) from comment #3)
> OK, makes sense.  Shipping pointer events is probably a bigger blocker at
> this point then!

Ehsan, are you recommending that we wait until after we ship PointerEvents before enabling dom.event.coalesce_mouse_move by default?

Comment 5

[(no longer active)]

I defer to smaug.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

I wonder if there is something else we could do before we have pointer events. Like enable unless the page (or iframes) has <canvas>.

Perhaps Jukka has an opinion from games point of view.

Comment 7

[Jukka Jylänki]

Oh my god, thanks for bringing this to my attention.

Agreed that some applications only care about the latest data that aligns with a rAF, so doing this sounds fine for many applications. The problem is that not all applications align with a rAF, especially now that Emscripten WebAssembly applications are in the process of migrating to run in a Web Worker. Such applications can run their own main loop, and use OffscreenCanvas to render (which has a .commit() API to push a canvas present). These application continue to use web events from the main browser thread to receive input, which are then routed to the WebAssembly heap, where a queue exists for these applications to read from.

An application that runs in a web worker, but does not run a synchronously blocking loop given by OffscreenCanvas, but wanted to keep using rAFs, will need bug 1203382 to land. I.e. currently an app running in a Web Worker cannot align accurately with rAF() (except by manually pinging postMessage()s over from main thread to the worker)

This kind of change would be a fine thing to do, as long as the API to opt out from coalescing is an appropriate one. Reading at https://w3c.github.io/pointerevents/extension.html#extensions-to-the-pointerevent-interface, is looks like the current API to opt out from coalescing seems to be that once a coalesced event finally arrives, one can then look at that event to ask from all the events that fell "in the middle" between the previously sent event and the current one.

This API means that applications that do want to opt out from coalescing still have to suffer from the added latency that coalescing introduced. That is, one does not get the uncoalesced input events until when the final coalesced event arrives.

In addition to the above "app renders in a web worker" case, there are VR applications that might have support for a mouse. Presenting VR content is not based on the usual rAFs (but a special version of it: https://developer.mozilla.org/en-US/docs/Web/API/VRDisplay/requestAnimationFrame), and for VR minimizing latency is a priority, so it would suffer from the added latency when the API to opt out from coalescing still takes a latency hit.

The feature to coalesce itself sounds fine, although I'd hope that the proposed method to opt out would not cause an added latency hit, and that the API to opt out from coalescing would be separate from the code that does the actual input handling. This way the decision of opting vs the code that does the actual mouse input could be treated orthogonally to each other - now the opting is tied to the specific locations in code that does the input handling.

Would it be possible to have an API where getting high refresh rate/low latency mouse and touch input would be some kind of option on .addEventListener(), or by calling a function somewhere to opt to it? Perhaps a foo.addEventListener('rapidmousemove') or similar would work better as a mechanism for applications that know they need the fast rate? That would be breaking, but equally breaking as a .getCoalescedEvents() API. The advantage of a foo.addEventListener('rapidmousemove') would be that the event object granularity would stay the same - in a .getCoalescedEvents() there is one Event object that is raised for N actual move events, and applications may need to refactor one Event callback to actually internally fire N event callbacks, one for each coalesced event. With a foo.addEventListener('rapidmousemove') vs foo.addEventListener('mousemove') apps would have an easy time testing out if it matters for them, and the code that actually processes the input events would not have to change?

Comment 8

[Jukka Jylänki]

All the above being said, OffscreenCanvas is not yet a spec that ships, Wasm multithreading is not yet here, and VR's main use case is not with mouse, so a big green stamp to go forward from that perspective - perhaps "rapidmousemove" and "rapidpointermove"  events might be good ideas to improve support for high refresh rate/low latency input devices for those use cases in the future(?)

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Some related discussion is happening in https://github.com/w3c/pointerevents/issues/214

Comment 10

[Daniel Holbert [:dholbert]]

Smaug, could you give this a [qf] rating?  Bas, emma, and I aren't clear on what the correct rating is here, given the possible-spec-work-dependence.

(I'm also tweaking the summary to make it clearer how this differs from bug 1361067.)

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Let's say qf:p3. It isn't clear whether we can enable this for FF57.

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

Based on some comments from rbyers that blink folks haven't seen regression bugs filed and what Jukka said on IRC, perhaps we could try to enable this on Nightly and see if this helps with any measurements. If not, which might be very possible, it might be still safest to switch this off for FF57.
Stone, does that sound reasonable to you?

Comment 13

[Ming-Chou Shih [:stone]]

(In reply to Olli Pettay [:smaug] from comment #12)
> Based on some comments from rbyers that blink folks haven't seen regression
> bugs filed and what Jukka said on IRC, perhaps we could try to enable this
> on Nightly and see if this helps with any measurements. If not, which might
> be very possible, it might be still safest to switch this off for FF57.
> Stone, does that sound reasonable to you?

Agree with you. I'll follow up.

Comment 14

[Ming-Chou Shih [:stone]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7dcb18b04f864ec7290599d7878587950229db8c

Comment 15

[Ming-Chou Shih [:stone]]

Attachment: enable the pref

Comment 16

[Olli Pettay [:smaug][[e-mail]]]

If this works well, need to remember to remove that ifdef!

Comment 17

[Ming-Chou Shih [:stone]]

(In reply to Olli Pettay [:smaug] from comment #16)
> If this works well, need to remember to remove that ifdef!

I'll keep an eye on this and monitor if any improvements or problems.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/79c73def3bdf
Enable coalescing mouse events to be once per refresh cycle (enable the pref). r=smaug.

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/79c73def3bdf

Comment 20

[Ben Kelly [:bkelly, not reviewing]]

Stone, can we remove the nightly ifdef?  Or is there a follow-up filed to do that I can follow?

Comment 21

[Ming-Chou Shih [:stone]]

(In reply to Ben Kelly [:bkelly] from comment #20)
> Stone, can we remove the nightly ifdef?  Or is there a follow-up filed to do
> that I can follow?

I think we can. Create bug 1403743 to remove it and ride the next train.