1640060 - Please revert removal of apz.frame_delay.enabled pref, and perhaps change the default to false

Status: NEW

(Core :: Panning and Zooming, enhancement, P5)

Description

[Russell Haley]

I learned about apz.frame_delay.enabled from this Reddit thread, and found flipping that preference alone, with none of the other tweaks, significantly improved the responsiveness of scrolling in desktop Firefox. Wanting to see how it would feel on a touchscreen, I opened up about:config on Fenix Nightly, but unfortunately, the preference did not exist. After some digging, I found that it was recently removed by a patch associated with Bug 1630781.

I object, in general terms, to the views expressed by the reporter of Bug 1367770, which prompted the introduction of this extra frame of input lag. JS frame production should be synchronized with CSS only on a best-effort basis. Whenever the goal of providing stiff and responsive scrolling of web pages conflicts with some whizbang javascript, the whizbang javascript should lose. After all, what is even the point of async panning, if it is not async?

Although this extra input lag has been around for nearly 3 years and apparently no one has crawled out of the woodwork to complain until now, I only really stumbled upon the cause by coincidence. It would not be easy for a single frame step change of input lag to rise to the level of conscious awareness, and even someone who noticed it would need a lot of tech savvy to out where it came from. (Monitor? Graphics driver? Desktop compositor? There are lots of things on a modern computer that could have a bug that adds buffering.) At best, it would show up in the tide of complaints about Firefox's scrolling being worse than Chrome. At worst, using Firefox would just feel worse and users wouldn't realize why.

This preference should be, at least, kept around until a better solution for scroll-linked events can be found -- perhaps predicting future scrolled-view position and lying to the JS after the first frame. Ideally, the default should be changed to false so that all users of Firefox benefit from the same lowered input lag presently available to people who follow the right forum.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Technically this is not a regression as it's an intended outcome of bug 1630781. I understand your argument here, and I'm not opposed to putting back the pref, but it would have to be reimplemented on top of the current code, as we can't just back out bug 1630781. This is not too hard to do, and I'd happily accept a patch to do it. I'm marking this as dependent on bug 1632522 because we should definitely add a test for the current behaviour before we muck around with it further.

That being said, while individual users like yourself can flip the pref, I doubt we will disable the frame delay by default, unless we can come up with a better solution for scroll-linked effects (and we've tried, but so far have been unsuccessful). JS-animated elements being out of sync with CSS animated elements is very easily noticeable, whereas a frame delay for CSS animated elements is not (as you point out yourself). So in this case going with the latter seems like the better option for the general population of users.

Comment 2

[Russell Haley]

I'd argue that scrolling being out-of-sync with the user's finger is a bigger problem than scrolling being out-of-sync with scroll-linked javascript, which, after all, is not present on considerate websites designed to conserve battery life. See this excellent demonstration from Microsoft Research on how important input latency is to feeling of responsiveness. For now, apz.frame_delay.enabled is still present in Fenix beta, and scrolling is markedly more pleasant with it turned on. That is, with the artificial input lag turned off.

It's hard to tell from the outside, but maybe the resolution of 1367770 was stepping on a prior decision to prioritize responsiveness? The reproduction example for that bug throws a warning in the console, which links an MDN page about scroll-linked effects, where it is very clearly stated that scrolling is composited and visible to the user before events are fired, and that scroll-linked javascript will lag behind. The MDN page is also the featured result on Google and the top result on DuckDuckGo for "scroll-linked effects".

As for learning the Firefox codebase and making a patch, well... it's on the queue. Something that comes to mind is that the blast radius of the frame delay could perhaps be contained to pages that have non-passive (see bug 1266066) scroll listeners.

Comment 3

[Botond Ballo [:botond]]

(In reply to Russell Haley from comment #2)

It's hard to tell from the outside, but maybe the resolution of 1367770 was stepping on a prior decision to prioritize responsiveness? The reproduction example for that bug throws a warning in the console, which links an MDN page about scroll-linked effects, where it is very clearly stated that scrolling is composited and visible to the user before events are fired, and that scroll-linked javascript will lag behind. The MDN page is also the featured result on Google and the top result on DuckDuckGo for "scroll-linked effects".

To provide some context here:

Before Firefox supported asynchronous scrolling (2016 for desktop, a few years earlier for mobile), it used to wait for JS to run and the results of any changes made by JS to be rendered, before the result of scrolling would be visible. This had the potential to introduce not just one frame of delay, but an unbounded number of frames (since the page can potentially spend an unbounded amount of time running JS in a scroll event handler).

When we introduced asynchronous scrolling, we did it knowing that it could break correctness for scroll-linked effects (i.e. make them out of sync with scrolling), but we decided that was preferable to the alternative of having to wait for a potentially unbounded number of frames. That decision -- that a delay of an unbounded number of frames is unacceptable for responsiveness -- stands today.

However, we discovered that the way our asynchronous scrolling was implemented was such that even if you were on a sufficiently fast system and/or viewing a sufficiently simple page, that the JS in a scroll event handler and the paint to render changes made by that JS, could run within our 16 ms frame budget -- even then, the scroll-linked effect would be at least one frame out of sync. In other words, scroll-linked effects were now always at least one frame out of sync, even under the most optimal of conditions.

While we continued to believe that a delay of an unbounded number of frames was unacceptable for responsiveness, we also felt that having scroll-linked effects always be at least one frame out of sync was unacceptable for correctness. This led to the change in bug 1367770, which made it such that scroll-linked effects would be in sync if the JS + paint could happen within one frame budget, but continued to be out of sync if the JS + paint took longer. For responsiveness, this meant adding one frame of delay, while continuing to avoid delays of a potentially unbounded number of frames. We felt this was a reasonable compromise between correctness and responsiveness.

The warning about scroll-linked effects in the devtools and MDN was kept because there was still the potential for them to be out of sync depending on environmental conditions (e.g. a slow system, heavy resource utilization by other processes, etc.)

I will also note that during the code review of bug 1630781, I did run the removal of the frame delay pref by our performance team. The response I got was:

I'm fine with doing this. Touch latency is something that will need to be addressed eventually, but it should be done by looking at the entire pipeline and coming up with a holistic approach. And last time I checked, turning off the frame delay actually made scrolling more stuttery.
And if I recall correctly, during some conversations about whole-pipeline latency and scrolling uniformity, we actually concluded that the "frame delay" actually gives us a really useful property. Unfortunately I forgot why.

I've cc'ed Markus (the author of this response) on this bug in case he'd like to chime in on this discussion.

(In reply to Russell Haley from comment #2)

Something that comes to mind is that the blast radius of the frame delay could perhaps be contained to pages that have non-passive (see bug 1266066) scroll listeners.

Passive listeners are a red herring here. They are relevant not for scroll events (which are the effect of scrolling), but for events that cause scrolling, such as wheel and touch, for which the spec says that if a JS event listener calls preventDefault() on the event, then no scrolling by the browser should occur. The possibility of preventDefault() is much worse for responsiveness than a one-frame delay: having to wait to see whether page content will call preventDefault() on a touch event can once again add an unbounded number of frames of delay. (In practice, browsers have a timeout for this, which for Firefox on mobile is 600 ms; still, that's 37 frames of delay). This issue is effectively addressed by the mentioned passive option, which Firefox has supported for years (and more recently has made the default).

However, I do understand and agree with the general direction of your suggestion: we could limit the frame delay to pages that actually have scroll-linked effects. We actually have a mechanism in place to detect scroll-linked effects (which we use to issue the devtools warning), although it currently only detects a subset of scroll-linked effects. We could improve this mechanism to detect all scroll-linked effects, and then use it gate whether or not to use the frame delay. This is definitely something we'd accept a patch for (in fact, it's something I've been wanting to work on myself, but so far other priorities have gotten in the way).

Comment 4

[Botond Ballo [:botond]]

(In reply to Botond Ballo [:botond] from comment #3)

We could improve this mechanism to detect all scroll-linked effects, and then use it gate whether or not to use the frame delay. This is definitely something we'd accept a patch for (in fact, it's something I've been wanting to work on myself, but so far other priorities have gotten in the way).

Perhaps I shouldn't say "definitely", as this approach is not entirely without downside. Since the mechanism to detect scroll-linked effects detects them dynamically ("when they happen"), this would mean having to dynamically transition a page from "no frame delay" mode to "frame delay" mode, and have the scroll-linked effect be out of sync for at least one frame for the first scroll event on the page. We'd have to make a UX call on that.

Still, if this allows us to remove one frame of responsiveness delay for 95% of pages (which I recall is the proportion of pages that do not have scroll-linked effects), I would support this change.