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)
Core
Panning and Zooming
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)
59 bytes,
text/x-review-board-request
|
kats
:
review+
lizzard
:
approval-mozilla-beta+
|
Details |
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.
Assignee | ||
Comment 1•7 years ago
|
||
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 hidden (mozreview-request) |
Comment 3•7 years ago
|
||
mozreview-review |
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+
Assignee | ||
Comment 4•7 years ago
|
||
(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
Comment 6•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/4e2992293058
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
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.
Comment 8•7 years ago
|
||
We should probably uplift this (and other autoscrolling fixes) to 56.
status-firefox55:
--- → unaffected
status-firefox56:
--- → affected
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?
Comment 10•7 years ago
|
||
https://hg.mozilla.org/projects/date/rev/4e299229305882c2685e8f8286eedec7255e4855 Bug 1386742 - Fix a transcription error in the APZ autoscroll calculation. r=kats
Comment 11•7 years ago
|
||
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+
Comment 12•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/6f5f957aa6dd
Comment 13•7 years ago
|
||
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)
Comment 14•7 years ago
|
||
(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)
Comment 15•7 years ago
|
||
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?
Assignee | ||
Comment 16•7 years ago
|
||
(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!
Assignee | ||
Comment 17•7 years ago
|
||
(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.)
Comment 18•7 years ago
|
||
BTW, is there any reason that autoscroll shouldn't work on about:addons? (I just noticed that it doesn't)
Assignee | ||
Comment 19•7 years ago
|
||
(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)
Comment 20•7 years ago
|
||
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)
Assignee | ||
Comment 21•7 years ago
|
||
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.)
Comment 22•7 years ago
|
||
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.
Description
•