Closed Bug 1386742 Opened 7 years ago Closed 7 years ago

APZ autoscrolling scrolls the page faster than main-thread autoscrolling

Categories

(Core :: Panning and Zooming, defect, P3)

defect

Tracking

()

VERIFIED FIXED
mozilla57
Tracking Status
firefox55 --- unaffected
firefox56 --- fixed
firefox57 --- verified

People

(Reporter: botond, Assigned: botond)

References

Details

(Whiteboard: [gfx-noted])

Attachments

(1 file)

STR:
  1. Open a scrollable page in latest Nightly
  2. Set apz.autoscroll.enabled to true
  3. Autoscroll the page

Expected results:

  Autoscrolling scrolls the page at the same speed as with
  apz.autoscroll.enabled set to false.

Actual results:

  Autoscrolling scrolls the page faster than with
  apz.autoscroll.enabled set to false.
Well don't I feel silly now. When transcribing the main-thread autoscroll calculation for APZ, I accidentally replaced a 'min' with a 'max'...
Comment on attachment 8893015 [details]
Bug 1386742 - Fix a transcription error in the APZ autoscroll calculation.

https://reviewboard.mozilla.org/r/164028/#review169336

If only DNA transcription errors made people go faster like this!
Attachment #8893015 - Flags: review?(bugmail) → review+
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #3)
> If only DNA transcription errors made people go faster like this!

:)
Pushed by bballo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4e2992293058
Fix a transcription error in the APZ autoscroll calculation. r=kats
https://hg.mozilla.org/mozilla-central/rev/4e2992293058
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
So now that we're past merge day this won't get into v56 beta?
That'd suck, because I wanted to test APZ autoscrolling on my main profile during my normal browsing.
We should probably uplift this (and other autoscrolling fixes) to 56.
Comment on attachment 8893015 [details]
Bug 1386742 - Fix a transcription error in the APZ autoscroll calculation.

autoscroll is behind a preference, so this patch is low risk, but it helps users that may want to test it sooner.
Attachment #8893015 - Flags: approval-mozilla-beta?
https://hg.mozilla.org/projects/date/rev/4e299229305882c2685e8f8286eedec7255e4855
Bug 1386742 - Fix a transcription error in the APZ autoscroll calculation. r=kats
Comment on attachment 8893015 [details]
Bug 1386742 - Fix a transcription error in the APZ autoscroll calculation.

APZ fix, for a feature not enabled by default in beta. This should land for beta 2.
Attachment #8893015 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Hi!

So I tested with 56b2 right now. And I can verify that the acceleration seems normal now.

However I notice a lot of checkerboarding (I believe it' called). When I autoscroll with high speed (rougly half the height of my 1920*1200 display) a long (few hundred comments) bugzilla page I only see the background color of bugzilla and no content, which is not useful at all. Is this expected?
Flags: needinfo?(botond)
(In reply to avada from comment #13)

Scratch that... I was assuming the update was b2, but it was a new build of b1. (Also confirmation bias and testing with  a long page confused me...)

Anyway now with actual b2 it's actually correct. And I can't get scrolling to be fast enough to produce artifacts anymore.
Flags: needinfo?(botond)
On Firefox 56.0b2 (Win 10 64-bit and Mac OS X 10.12) APZ autoscrolling still feels a little faster than main-thread scrolling - 5 seconds vs 4 seconds (measured on about:support page).

Is there a way to measure this more accurately?
(In reply to avada from comment #14)
> Anyway now with actual b2 it's actually correct. And I can't get scrolling
> to be fast enough to produce artifacts anymore.

That's great to hear - thanks for testing!
(In reply to Petruta Rasa [QA] [:petruta] from comment #15)
> On Firefox 56.0b2 (Win 10 64-bit and Mac OS X 10.12) APZ autoscrolling still
> feels a little faster than main-thread scrolling - 5 seconds vs 4 seconds
> (measured on about:support page).
> 
> Is there a way to measure this more accurately?

There isn't any built-in / automated mechanism to measure the speed. Keep in mind the speed depends on the distance from the mouse to the anchor, so two scrolls could have slightly different speeds due to that distance being slightly different.

Also, APZ autoscrolling (which is driven by the compositor) is expected to be faster than main-thread autoscrolling (which is driven by the refresh driver) on pages where we are slow to paint. On such pages, we are unable to tick the refresh driver at 60 fps, but we can still composite at 60 fps. So, even though on each sample of the animation we scroll by the same amount, we sample it more frequently with APZ autoscrolling. (The way to think about this scenario is not that APZ autoscrolling is going faster than intended, but that main-thread autoscrolling has been going slower than intended and we are fixing it.)
BTW, is there any reason that autoscroll shouldn't work on about:addons? (I just noticed that it doesn't)
(In reply to avada from comment #18)
> BTW, is there any reason that autoscroll shouldn't work on about:addons? (I
> just noticed that it doesn't)

It looks like autoscrolling is not enabled for scrollable XUL elements [1], only HTML.

Neil, do you perhaps know the reason for this restriction? Looking at the blame, it seems to have been added in bug 295977, but I can't find any explanation for it in the comments of that bug (it was also a long time ago).

[1] http://searchfox.org/mozilla-central/rev/4d8e389498a08668cce9ebf6232cc96be178c3e4/toolkit/content/browser-content.js#87
Flags: needinfo?(enndeakin)
It doesn't look like the code from bug 295977 intentionally disabled it for XUL elements, so since autoscroll generally only works inside content loaded into <browser>, I'd suggest adding if desired for other types of elements.
Flags: needinfo?(enndeakin)
Thanks. I filed bug 1397422 to track autoscrolling of scrollable XUL elements such as in the "Extensions" section of about:addons. (It already works in the "Get Add-ons" section of about:addons.)
Marking 57 as verified based on sign off tests (mid-Nightly and pre-Beta) performed for Autoscrolling in 57.0a1 Nightly.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.