Started noticing this as of this week. Mouse wheel scroll gets really slow. If I right click to show tab bar and scroll again it's fast for the first scroll, then gets back to being slow for next scrolls.

This regressed on Aurora between the 01-16 nightly and the 01-17 nightly: http://hg.mozilla.org/releases/mozilla-aurora/pushloghtml?fromchange=1dd80f8faa2f&tochange=2c033140eff4 I'm working on bisecting further.

The first bad revision is: changeset: 178259:b3b07adaf7cd user: Kartikaya Gupta <kgupta@mozilla.com> date: Tue Jan 14 16:43:43 2014 -0500 summary: Bug 951113 - Trigger a repaint request when getting a scroll offset update Testing notes: The bug can't be reproduced on the Firefox Start page. Sometimes I have to use the mouse wheel several times to trigger the bug, but I can always trigger it by scrolling back and forth for 10 seconds or so.

Hm. The second patch on bug 951113 is likely the culprit. It assumes that scrolling is mostly being "driven" via touch. When you're scrolling with the mouse wheel this assumption is broken and we'll end up painting twice as much. :(

Turns out it's not extra painting that's the problem, it's that when we request the paint, we interrupt the "smooth scroll" that the mouse wheel triggers. The attached patch helps significantly on my Surface but doesn't fix the problem entirely because there's a race with the APZ's repaint and the content's incremental update of the scroll offset as it does the smooth scrolling. A better long-term (but much more involved) is to add a smooth scroll API to APZ and use that. I recall discussion about this on some bug somewhere but can't find it now. Rodrigo (and Matt) - could you test with this patch to see if the result is acceptable?

Comment on attachment 8362084 [details] [diff] [review] Patch Scrolling is improved with this patch but still very visibly regressed. Instead of moving smoothly, it goes in fits and starts, repeatedly speeding up and then slowing to a crawl.

I'm getting the same behavior Matt described, plus it sometime jumps as a page up/down.

After giving this some thought I think the only reasonable solution here is to put in the right fix for the long term. This involves routing the WidgetWheelEvent to the APZ rather than gecko, and having the APZ code implement smooth scrolling in the compositor.

Incidentally this would also fix the problem where if you scroll with the wheel while zoomed in, you scroll farther than if you are zoomed out. (Same amount of content, but it encompasses more pixels on the screen).

The above two patches route mouse wheel events through the APZ. I haven't tested this extensively but the basic feature works. The actual scroll amounts need to be tuned some more, and we might need to still dispatch the wheel events to content - I don't know how hard it will be to do that without also triggering scrolling code in nsEventStateManager. But presumably it can be done. If anybody else wants to take these patches and run with them feel free to do so.

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #4) > Created attachment 8362084 [details] [diff] [review] > Patch > > Turns out it's not extra painting that's the problem, it's that when we > request the paint, we interrupt the "smooth scroll" that the mouse wheel > triggers. The attached patch helps significantly on my Surface but doesn't > fix the problem entirely because there's a race with the APZ's repaint and > the content's incremental update of the scroll offset as it does the smooth > scrolling. A better long-term (but much more involved) is to add a smooth > scroll API to APZ and use that. I recall discussion about this on some bug > somewhere but can't find it now. That was in bug 907100.

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #11) > The above two patches route mouse wheel events through the APZ. I haven't > tested this extensively but the basic feature works. The actual scroll > amounts need to be tuned some more, and we might need to still dispatch the > wheel events to content - I don't know how hard it will be to do that > without also triggering scrolling code in nsEventStateManager. But > presumably it can be done. > > If anybody else wants to take these patches and run with them feel free to > do so. Note bug 23456, comment 16 and bug bug 23456, comment 17. There's a lot to support here including event stream cancellation and legacy desktop wheel prefs. Any chance we could punt on this and instead just fix the dom smooth scroll issue?

(In reply to Jim Mathies [:jimm] from comment #13) > (In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #11) > > The above two patches route mouse wheel events through the APZ. I haven't > > tested this extensively but the basic feature works. The actual scroll > > amounts need to be tuned some more, and we might need to still dispatch the > > wheel events to content - I don't know how hard it will be to do that > > without also triggering scrolling code in nsEventStateManager. But > > presumably it can be done. > > > > If anybody else wants to take these patches and run with them feel free to > > do so. > > Note bug 23456, comment 16 and bug bug 23456, comment 17. There's a lot to > support here including event stream cancellation and legacy desktop wheel > prefs. Any chance we could punt on this and instead just fix the dom smooth > scroll issue? Sorry, I meant bug 907100, comment 16 and bug 907100, comment 17.

Also, we need a wheel scroll metro mochitest test to catch further regressions here. Someone on the metro team can help put that together if you like.

So then I can think of a couple of options. One is to create a new scroll type (APZC) that just returns early somewhere around [1] after processing all the prefs and computing the scroll amount, but before actually doing the scroll. Then the widget code can take that and trigger an APZ scroll. The other option is to expose via nsIScrollableFrame whether or not a smooth scroll is happening, and then check that at [2] and not do the scroll if it is. This would be the easier approach but is also more brittle and probably not the best solution for the long-term. It also means that legitimate interruptions of smooth scrolling via touch input won't happen (i.e. you do a fling with the wheel and then attempt to stop it with your finger). But for a short term solution it's low effort to implement and debug. [1] http://mxr.mozilla.org/mozilla-central/source/dom/events/nsEventStateManager.cpp?rev=9961f372f396#2969 [2] http://mxr.mozilla.org/mozilla-central/source/widget/xpwidgets/APZCCallbackHelper.cpp?rev=3f13d3dd021d#104

I'm testing out that second option now.

Seems to work ok for me. What do you guys think?

Comment on attachment 8363045 [details] [diff] [review] Alternate patch (short-term workaround) This reverts back to the original behavior afaict.

Btw I filed bug 962218 for the long-term plan of smooth scrolling in APZ code.

Restoring the beta28 flag. This breaks core functionality and has become our top feedback issue, so we need to track this for Metro Firefox 28.

Trees are closed :(

Comment on attachment 8363045 [details] [diff] [review] Alternate patch (short-term workaround) [Approval Request Comment] Bug caused by (feature/regressing bug #): 951113 User impact if declined: scrolling with a mouse wheel is really slow Testing completed (on m-c, etc.): locally by a few people, on m-c now Risk to taking this patch (and alternatives if risky): low risk; should only affect metrofox String or IDL/UUID changes made by this patch: none Note: this needs to be uplifted to 28. Since we are close to the next merge this might need beta approval instead of aurora approval if we go past the merge.

Comment on attachment 8363045 [details] [diff] [review] Alternate patch (short-term workaround) Given low risk and impacts only approving for uplift. Adding verify for QA verification

Verified as fixed with Firefox 28 beta 7 on a Dell XPS 12 ultrabook with Windows 8 64-bit.

Verified as fixed on latest Aurora (build ID: 20140304004003) using a Microsoft Surface Pro 2 device running Windows 8.1 64bit.