Closed Bug 728738 Opened 12 years ago Closed 1 year ago

glitch in the smooth scroll animation

Categories

(Core :: Web Painting, defect)

Product:
Core
Component:
Web Painting
Type:
defect

Tracking

()

Status:
RESOLVED WORKSFORME

People

(Reporter: tnikkel, Unassigned)

References

Details

(Whiteboard: [snappy:p1])

Attachments

(2 files)

Sample video (8s @60fps) which shows the smooth-scroll glitches
(deleted), application/octet-stream
Details
Simple HTML page to test animation using different loop methods
(deleted), text/html
Details 
In bug 206438 comment 23 (and later) Avi reported a jitter in the smooth scroll animation. We should investigate this.
Thanks for opening the bug.

I'm not sure my observations from bug 206438 comment 32 and 33 are reliable/valid enough. To reproduce this issue (visually see the jitter), I would suggest:

1. Enable smooth scroll.

2. Increase smooth scroll duration to 4000ms (kSmoothScrollAnimationDuration at layout/nsGfxScrollFrame.cpp).

3. To rule out refresh/sync accuracy issues, increase the refresh rate. I set it to 4ms (250Hz) both at layout/nsRefreshDriver.cpp using:
#define DEFAULT_FRAME_RATE 250

and at layout/nsGfxScrollFrame.cpp using:
mAsyncScroll->mScrollTimer->InitWithFuncCallback(
        AsyncScrollCallback, this, 1000 / 250,
        nsITimer::TYPE_REPEATING_SLACK);

4. Load a "light" page to scroll to rule out CPU getting choked. I used the page of bug 206438 .

5. Use Page-Down/Up to let it scroll a significant amount of pixels for relatively long duration. Try it few times.

Observe that Firefox 10 (and earlier) is, while not without glitches, much smoother than latest builds (which seem to have periodical jitters about once a second).

 - As an alternative to steps 2+3, one can use the SmoothWheel addon and set the speed to slow and distance to long (and use the wheel - SmoothWheel doesn't support page up/down).

As I noted at the other bug, I'm not sure Gecko was designed for (or if it's reasonable to expect it) to not skip a frame, which is something which is expected of game engines, but not necessarily from a browser. This issue is only noticeable in animations which are expected to be very smooth. It applies to smooth scrolling as in bug 206438, and possibly also to HTML5 games (though I haven't looked for one which exhibits it).

I wouldn't have reported this issue, but since it can be considered a regression, I wanted to make the issue visible.
In addition to the smooth scroll timer and the refresh driver timer we are also dependent on the OS to send us paint events in response to the invalidates that we flush to the OS from the refresh driver. At 250 fps I would be surprised if the OS sent us 250 paint events per second. So that is going to confound the analysis here. (http://mxr.mozilla.org/mozilla-central/source/view/src/nsViewManager.cpp#784 is where we receive paint events)
Might bug 702463 help with jitter?
Maybe. I don't think there is any reason not to do bug 702463, so we should do it anyway and see if it helps.
I'm not sure if this helps, but I think I can explain the cause of this small glitch: it happens when CC works out.

How did I notice this?
1. I opened error console (and selected to show "messages").
2. I checked that AutoScrolling is enabled ("general.autoScroll" is set to "true" by default, so I just did a check).
3. Middle click somewhere on the page and move cursor just A BIT lower/higher from where you've middle clicked, so autoscrolling starts working and the page is getting scrolled very slowly.
4. watch both windows: error console and the page you scroll.

The noticed result: every time you see the message in the error console that CC has just worked out - you'll also see the small glitch at scrolling.

I hope that it's a helpful observation.
Oh, maybe (just maybe, not sure) the CC messages are not shown by default, so you might also need to set "browser.dom.window.dump.enabled" to "true", not sure though.
That reminds me of bug 730113 - although the CC probably wasn't designed to be cut up into little chunks like IGC.
autoscroll isn't a smooth scroll, it doesn't use the smooth scroll infrastructure of nsGfxScrollFrame, it achieves animation of scrolling through other means, so that is definitely a different bug.
I've already added prints at some CC functions some time ago, but haven't noted it here since while there seemed to be some correlation (of prints to scroll jitters), I couldn't find functions for prints which correlate to 100% of the jitter. It still might be CC related though.

For reference: bug 712478 (and proper usage of it) might also solve this bug.
Only with regard to the new regular glitches of about once/sec (otherwise, most exhibit irregular ones with varying durations, averaging probably around once in few seconds but VERY variable - nothing new here):

1 0ee7513032c3 - OK
2 6fad3c48edf5 - BAD
3 93ccc3fd51c9 - OK
4 611c4ec849f2 - OK
5 802680fecc3c - BAD
6 bf9d75994453 - BAD

Could it be that you posted them not in correct commit order?
Sean Newman(In reply to Sean Newman from comment #5)
...
> 2. I checked that AutoScrolling is enabled ("general.autoScroll" is set to
> "true" by default, so I just did a check).
...

I tried it with autoscroll too. While it seem to have similar glitches (possibly slightly more frequent), I think it's not the same as this bug because, for example, all the builds which I tested on comment 12 (and nightlies of 2012-01 15 and 16) had the autoscroll glitches, but only some had the smoothscroll glitches.

Also, I couldn't reproduce the CC messages on the error console after following your instructions. Maybe something else is required. FYI.
(In reply to Avi Halachmi (:avih) from comment #12)
> Could it be that you posted them not in correct commit order?

I just didn't bother ordering them.

Anyway this is the result

m-c d7984d345c24 try 0ee7513032c3 ok
m-c 49afabda6701 try 611c4ec849f2 ok
m-c 59cb54c6dfe1 try 93ccc3fd51c9 ok
m-c d7dcde1032ed try 6fad3c48edf5 bad
m-c 5e6e63f3aed8 try bf9d75994453 bad
m-c 047c8ba7d2e4 try 802680fecc3c bad

And so that confirms that it is moving invalidation flushing to the refresh driver (bug 598482) like I had suspected.

Thanks for testing.
Blocks: 598482
For the timer thing there is bug 640796.
Depends on: 702463, 698547
Whiteboard: [snappy]
Blocks: 710372
Based on comment 1 this is a regression, so highly unlikely a CC issue. Comment 13 also hints that
this is not about CC.
Yeah, it's not about CC.
No longer depends on: 698547
If it's indeed the refresh driver invalidation, is there a quick and dirty fix just to confirm it? e.g. increase some timers resolution (have tried few per bug 206438 comment 31, to no avail), request explicit flushes at some places, etc? I'm willing to test if someone comes up with such dirty fix.
(In reply to Avi Halachmi (:avih) from comment #18)
> If it's indeed the refresh driver invalidation, is there a quick and dirty
> fix just to confirm it? e.g. increase some timers resolution (have tried few
> per bug 206438 comment 31, to no avail), request explicit flushes at some
> places, etc? I'm willing to test if someone comes up with such dirty fix.

Timothy, how should we proceed?
Whiteboard: [snappy] → [snappy:p1]
I think the next step is to do bug 702463 and see if it helps with this problem. If not we can investigate more, but we want to do that anyway (for at least bug 728153).
Maybe we shouldn't enable smoothscroll by default until this bug is resolved. Or we should backport the fix to version 13.
(In reply to Marco Castelluccio from comment #21)
> Maybe we shouldn't enable smoothscroll by default until this bug is
> resolved. Or we should backport the fix to version 13.

Maybe. Requesting tracking.
tracking-firefox13: --- → ?
Would somebody mind attaching a video of the issue or providing STR that our users would notice this glitch with?
tracking-firefox13: ? → +
I'll post a video today or tomorrow.
tracking-firefox13: + → ---
It appears that my previous post removed tracking-firefox13. It was not intentional.
It's as compressed as I could, so ignore the low quality (and the tearing) - the smoothness/glitches are well preserved.

To get good compression, I used H.264(yeh, shoot me ;) ), so if you can't play this, then mplayer or SMPlayer or VLC would work.
Also, for that video sample, to get a continuous and long scroll, I set general.smoothScroll.pages.duration*MS to 10000 (10s), pressed page-down few times quickly, and then sat back and let it scroll.

Generally speaking, the shorter the animation duration is, the less bothering are these glitches.
(In reply to Sean Newman from comment #5)
> I'm not sure if this helps, but I think I can explain the cause of this
> small glitch: it happens when CC works out.
> 
> How did I notice this?
> 1. I opened error console (and selected to show "messages").

I was wrong, that's not enough: you have to set "javascript.options.mem.log" to "true" first.
(In reply to Sean Newman from comment #28)
> (In reply to Sean Newman from comment #5)
> I was wrong, that's not enough: you have to set "javascript.options.mem.log"
> to "true" first.

After following this ^, I'm now seeing CC messages once every few seconds (6s-7s?) while scrolling, similar to:

CC(T+341.2) duration: 7ms, suspected: 663, visited: 5019 RCed and 609 GCed, collected: 0 RCed and 0 GCed (30 waiting for GC)
ForgetSkippable 8 times before CC, min: 0 ms, max: 1 ms, avg: 0 ms, total: 3 ms, removed: 642

Unless this is an aggregate message, then this is much less frequent than the glitches (tried both smooth and auto scroll).

Since ForgetSkippable was apparently invoked 8 times, I thought it might be the culprit, even if it appears to use very little CPU (max 1ms, avg 0ms at that message). So I added a print at nsCycleCollector::ForgetSkippable, and while it's triggered at a similar rate to the glitches, it doesn't coincide with neither the smooth scroll glitches, nor with the autoscroll ones.
Avi - I'm going to assign this to you since you appear to be leading the investigation at the moment. Please let me know if you need somebody else to take this bug up.
Assignee: nobody → chuku_regs
tracking-firefox13: ? → +
The GC/CC related pauses are not this bug.
I believe it's better that someone more knowledgeable than me handles it. It's just that I haven't seen someone else acknowledging it yet, so I'm offering to test possible approaches (preferably as patches).
Assignee: chuku_regs → nobody
(In reply to Avi Halachmi (:avih) from comment #32)
> I believe it's better that someone more knowledgeable than me handles it.
> It's just that I haven't seen someone else acknowledging it yet, so I'm
> offering to test possible approaches (preferably as patches).

Thanks Avi. tn - can you take point on this bug from the engineering side?
Assignee: nobody → tnikkel
After I replaced the smooth scroll timer with a refresh driver notification (pending patch at bug 702463), I found the cause for the repetitive glitch.

As tn suspected, it's caused by the fix to bug 598482. It's not necessarily a regression, but rather a side effect of that fix.

Since invalidation is now triggered by the refresh driver, it's limited to once in NSToIntRound(1000.0/rate) ms. This results in 17ms for a 60Hz rate, so we're "losing" 0.33ms on each iteration compared to the actual monitor refresh rate (the unrounded result is 16.67ms). This means that once every 16.67/0.33 = 50 frames (833ms on 60Hz), we have one monitor refresh without invalidation, and that's exactly the repetitive glitch which I saw.

The naive "fix" would be to use floor instead of round, such that we're at least as fast as the monitor refresh rate, however, this also doesn't fix it, because using a similar calculation, we "gain" 0.67ms per frame, so every 25 frames (~400ms) we have a single monitor refresh for which we invoke 2 smooth scroll iterations, so this results in a "jump forward". This was also observed when I changed the code to use floor.

SmoothWheel "solves" this issue by offering custom refresh rate selection for the timer (defaults to 240). The equivalent change in mozilla code would be to set layout.frame_rate to 240, and indeed it improves the smoothness (Setting it to 1000 improves it even more). However, this approach results in increased CPU usage while scrolling.

The best approach would be to hook the refresh driver to vsync (or equivalent) instead of using a timer, however, that's both platform specific, and not always an easy task.

A lesser approach (but still better than now) might be to improve the timer accuracy, either by using a sub-ms resolution timer, or by using one-shots with a compensation mechanism (such that the timer fires in ms resolution, but on average we fire at a custom high-resolution interval).
Clarification: Timoty alerted me to the fact that I may have been inaccurate in my bug/regression description.

My tests with older builds were done with the SmoothWheel addon only. These tests showed that the smoothwheel glitches started after landing bug 598482. SmoothWheel's default refresh rate timer is 240Hz.

My tests with recent builds (since I started compiling Firefox 6 weeks ago - after bug 598482 landed) were done using mozilla's own smooth scrolling (which now can be configured to longer duration, so it's noticeable), and the glitches do appear.

I may have wrongly assumed (or even said so explicitly) that mozilla's own smooth scroll was not glitchy prior to landing bug 598482 but it is glitchy now. This is NOT correct.

-------
The fact is that Mozilla's smooth scroll WAS glitchy before landing this bug, and also after it (=now), and on both cases it's because the timer wasn't 100% in sync with the monitor refresh.

So no regression for mozilla's smooth scroll, but still yes glitches.
-------

The real regression which landing bug 598482 caused, is that now NO animation (internal or on html pages, etc) can be 100% smooth. Prior to landing that bug, animations could use a 1000hz timer, and get it pretty smooth, but now they can't because the refresh driver limits the invalidations to once in 17ms, which results in glitches every 800ms (see comment 34 for the full explanation).

Sorry for any misunderstanding due to bad descriptions.
(In reply to Avi Halachmi (:avih) from comment #35)

> The real regression which landing bug 598482 caused, is that now NO
> animation (internal or on html pages, etc) can be 100% smooth. Prior to
> landing that bug, animations could use a 1000hz timer, and get it pretty
> smooth, but now they can't because the refresh driver limits the
> invalidations to once in 17ms, which results in glitches every 800ms (see
> comment 34 for the full explanation).

This sounds really bad. Tim, is this a known issue? Do we have a plan for hooking into the vsync?
(In reply to Taras Glek (:taras) from comment #36)
> This sounds really bad. Tim, is this a known issue? Do we have a plan for
> hooking into the vsync?

We should file a new bug for it and discuss with the relevant parties what we want to do about it.
By request from taras, here's a simple test case for HTML animation. Worth trying it on different browsers and/or browser versions.
(In reply to Timothy Nikkel (:tn) from comment #37)
> (In reply to Taras Glek (:taras) from comment #36)
> > This sounds really bad. Tim, is this a known issue? Do we have a plan for
> > hooking into the vsync?
> 
> We should file a new bug for it and discuss with the relevant parties what
> we want to do about it.

Bug 689418 is a tracking bug for updating stuff during VBLANK.
oh oh wasn't the Gif Animation improvements also based on the invalidation and many other things ? 
I also experience these glitches using my haptic smooth scroll preset for Yet Another Smooth Scrolling thx avih for finding out what the cause for these brake ups is i guess many had @ first CC timers in their eyes :)
Here is a Video comparing the current situation and how this works out Firefox D2D (non thebes) compared vs Chromium 20 GPU http://www.mediafire.com/?60yg21u967n2efb forgive the quality due to it it's hardly visible on the Text (also due to the window size it's much more annoying @ Fullscreen or a Big window but the more Graphic orientated Scroll case shows it very clear :(
Untracking as per email from tn, this isn't going to be something that can uplift.
Assignee: tnikkel → nobody
tracking-firefox13: + → ---
FYI, after Bug 702463 landed, it's now possible to increase the refresh frequency and thus, sort of "bypass" this bug, by changing layout.frame_rate (default is -1 for 60hz) to a high value (e.g. 240 or 500).
I am also having some short term glitches when scrolling (with smooth scrolling disabled), BUT after raising the layout.frame_rate preference to 120+ its mostly fluid as butter (with some aliasing). And with the more fluid scrolling, came the rise in GPU activity, which uplifted the clocks on the card.
Note: I am using a 60 HZ monitor.
Hi guys just to make sure I have this right, if I scroll using the middle mouse button method. ie. the scrolling is coninous without me touching mouse this is autoscroll right?

Basically I noticed if I set layout.frame_rate to default or 60, on autoscroll almost every second like on a timer it jerks, gpu and cpu are not saturated, if I smooth scroll by using the mousewheel it also seems jerky but not so noticeable.  If I set to 240 as mentioned earlier the problem goes away for both.  so for me it seems this bug affects both but its just more noticeable on autoscroll.  I hope this helps.
(In reply to Chris from comment #45)
> Hi guys just to make sure I have this right, if I scroll using the middle
> mouse button method. ie. the scrolling is coninous without me touching mouse
> this is autoscroll right?
> 
> Basically I noticed if I set layout.frame_rate to default or 60, on
> autoscroll almost every second like on a timer it jerks, gpu and cpu are not
> saturated, if I smooth scroll by using the mousewheel it also seems jerky
> but not so noticeable.  If I set to 240 as mentioned earlier the problem
> goes away for both.  so for me it seems this bug affects both but its just
> more noticeable on autoscroll.  I hope this helps.

Setting layout.frame_rate to 240 is likely to have negative performance implications for things other than this one scenario.
Hi again did this ever get fixed?

I came back here as I am dealing with high cpu usage issues with firefox, so decreased the framerate back to 60 to match my monitor refresh and get lower cpu usage, but jerky scrolling comes back so seems isnt fixed?
(In reply to Chris from comment #47)
> Hi again did this ever get fixed?
> 
> I came back here as I am dealing with high cpu usage issues with firefox, so
> decreased the framerate back to 60 to match my monitor refresh and get lower
> cpu usage, but jerky scrolling comes back so seems isnt fixed?

Yes, it was fixed, from 2 aspects:

The glitches at this specific bug were tracked back to "screen iterations" every 17ms (or 58.9 Hz) instead of every 16.667 ms, which got out of sync with the Monitor every 800ms (if the Monitor is 60Hz) which resulted in a missed frame.

Since then, the "screen update" timers were changed to use compensation such that on average the iteration rate is accurate. This defaults to 60 hz (if layout.frame_rate = -1 - which is the default) or any other value set to that pref.

On top of that, on Windows it now also uses vsync to match the monitor's rate even if it's not 60 hz (also when layout.frame_rate = -1). It's not bullet proof, but it does "soft sync" to the vsync signal.

On OS X it's using vsync implicitly with the timers set to 60 hz, so this bug shouldn't be noticeable there.

On Linux it's just accurate 60 hz (on average) without vsync AFAIK.

Hopefully at some stage we'll get proper vsync on all platforms (Project silk), though right now the first platform it's implemented for is B2G. Desktop platforms hopefully will follow soon afterwards.
well been doing some tests.

with the frame rate set to either 60 manually or left at -1 (which sets at 60 to match monitor) I get stutters like before (using esrv31)  When I set back to 240 the stutters improve but dont go away, suggesting still same issue, but the reason I think they dont go away is because v31 has higher cpu utilisation to scroll/show a page than v24 does, changes in the app seem to have regressed graphical performance.  I get no stutters at 30fps but it isnt smooth.
Component: Layout: View Rendering → Layout: Web Painting
Severity: normal → S3

The severity field for this bug is relatively low, S3. However, the bug has 18 votes.
:tnikkel, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Flags: needinfo?(tnikkel)
Flags: needinfo?(tnikkel)
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: