1451439 - Often some elements are jumping one pixel up and down while scrolling up/down. Snap async scroll offsets to pixels

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect, P2)

Description

[Darkspirit]

Attachment: 2018-04-04_20-10-29.mp4

Debian Testing, KDE, Radeon RX480, 2560x1440

mozregression --good 2018-03-14 --bad 2018-03-16 --pref gfx.webrender.all:true general.autoScroll:true browser.startup.blankWindow:false browser.tabs.drawInTitlebar:true startup.homepage_welcome_url:"https://www.mozilla.org/de/firefox/new/"
> 5:02.90 INFO: Last good revision: c9cb048dfd438c26e58e416338a6cca24226ae80
> 5:02.90 INFO: First bad revision: fc150f21f7fac1be6f31ca8d950570751ddad6aa
> 5:02.90 INFO: Pushlog:
> https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=c9cb048dfd438c26e58e416338a6cca24226ae80&tochange=fc150f21f7fac1be6f31ca8d950570751ddad6aa

> fc150f21f7fa	Nicolas Silva — Bug 1437032 - Rely on WebRender to pixel snap gradients instead of doing it when building the display list. r=mstange

Comment 1

[Darkspirit]

At first I thought this would be related to bug 1434503 and friends.
If you think the bug description is not appropriate I would have to search for another regression about this description then.

Comment 2

[Darkspirit]

Could you have a look, please? mozregression points to your change.

Comment 3

[Nicolas Silva [:nical]]

I think that this is caused by different types of display items not using the same strategy for pixel-snapping. I think that in the long run we should not do any pixel snapping while building the display list and only rely on WebRender's pixel snapping logic, bt the trickiness will probably be in moving over to this without upsetting loads of reftests (I guess we'll just have to bite the bullet).

Comment 4

[Darkspirit]

Attachment: 2018-04-06_18-52-09_bmo.mp4

Same regression range.
URL: https://bugzilla.mozilla.org/show_bug.cgi?id=1208646

Comment 5

[Darkspirit]

Win10, Radeon RX480
comment 0 looks the same. comment 4 is reproducible at 110% zoom.

Comment 6

[Markus Stange [:mstange]]

I think snapping scroll offsets to device pixels would fix this. Lee, you tried this before but ran into problems - do you remember what they were?

Comment 7

[Darkspirit]

GTX 1060, Debian Testing, KDE, 2560x1440
I still see this bug, bug 1434503, bug 1432046 (flickering line), bug 1434047. (And I think bug 1464371 might be the same.)

Comment 8

[Lee Salzman [:lsalzman]]

(In reply to Markus Stange [:mstange] from comment #6)
> I think snapping scroll offsets to device pixels would fix this. Lee, you
> tried this before but ran into problems - do you remember what they were?

Those problems were related to intricacies of text, in that the text run itself has an offset and individual glyphs have an offset. That wouldn't be relevant to other primitives.

Comment 9

[Markus Stange [:mstange]]

Oh, then I think I misunderstood your previous implementation idea.

In this bug, I think we should snap the scroll position on composition, so that all display items are drawn with only an integer offset compared to their main thread position, which will make all items snap consistently during scrolling.

Comment 10

[Mayank Bansal]

This is very easy to see on youtube. Just go to any video with comments, and then scroll sloooowly. The "up vote" and 'down vote" thumb buttons jiggle up and down

Comment 11

[Markus Stange [:mstange]]

I'm making this the authoritative bug for snapping async scroll offsets. We have a bunch of open bugs that should be fixed by doing that.

Comment 12

[Chris Pearce [:cpearce (Not reading bugmail)]]

darkspirit: Can you still repro this specific bug? There are a number of bugs about scrolling and snapping, but I can't repro this specific case on Win10.

Comment 13

[Markus Stange [:mstange]]

I can still reproduce this one. You might need to experiment with different zoom levels and different input mechanisms for scrolling. Autoscroll works best.

Comment 14

[Chris Pearce [:cpearce (Not reading bugmail)]]

As markus suggested in comment 9, snapping scroll offsets to device pixel offsets (I did it in APZCTreeManager::SampleForWebRender()) fixes the flickering on scrolling (for the cases I can repro at least).

Comment 15

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Bug 1451439 - Snap scroll offset to pixel boundaries before passing to WebRender. r?botond

When scrolling very slowly, sometimes some things can jump up and down, causing
flickering. This is because the scroll offset that the APZC passes to WebRender
isn't snapped to pixel boundaries; things drawn relative to that can move up or
down depending on how their boundaries round.

So round the scroll offset to device pixel boundaries before passing it to
WebRender. This eliminates the flicker in cases I've been able to reproduce.

Comment 16

[Chris Pearce [:cpearce (Not reading bugmail)]]

The patch here causes flickering on https://codepen.io/Aux-Lux/pen/mxdbOX (which was reported as in issue in bug 1444090, and subsequently resolved as WFM).

Comment 17

[Botond Ballo [:botond]]

Comment on attachment 9011972 [details]
Bug 1451439 - Snap scroll offset to pixel boundaries before passing to WebRender. r?botond

Botond Ballo [:botond] has approved the revision.

Comment 18

[Darkspirit]

This also fixes the flickering horizontal line on the green "back" box (in the vertical middle of the page):
https://3dtransforms.desandro.com/cube

Comment 19

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Chris Pearce (:cpearce) from comment #16)
> The patch here causes flickering on https://codepen.io/Aux-Lux/pen/mxdbOX
> (which was reported as in issue in bug 1444090, and subsequently resolved as
> WFM).

I actually see this in Nightly if I resize the browser window from the RHS, so I'm going to assume it's not a regression from my patch. I have a fix for that too, will put that in a new bug. I'll land this now.

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bdc90f28bd84
Snap scroll offset to pixel boundaries before passing to WebRender. r=botond

Comment 21

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/bdc90f28bd84