1258051 - Checkerboarding every scroll wheel event on a Hi-DPI screen

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Thomas Daede [:TD-Linux]]

Attachment: checkerboarding log

With GL layers on in Linux, scrolling a page longer than two viewport sizes with the scrollwheel causes a visible checkerboarding every frame.

2x HiDPI is in use.

No visible checkerboarding (and no checkerboarding logs) appear with basic layers.

Comment 1

[Botond Ballo [:botond]]

The first thing that jumps out from the checkerboard report is that we have a bug in how we calculate the severity for single-frame checkerboard events. The time duration used as the basis for the severity calculation is roughly one frame less than the number of frames of checkerboarding, so for single-frame checkerboard events we get very small severity numbers :) But that's unrelated to the actual bug.

The second thing that jumps out is that the displayports are very heavily weighted to areas *above* the viewport rather than below it. It's not clear why, but this is probably the cause of the constant checkerboarding.

Comment 2

[Botond Ballo [:botond]]

Btw., Thomas confirmed over IRC that the size_multiplier prefs are both 3.5, so it's not a case of being part of the checkerboarding telemetry experiment or anything.

Comment 3

[Botond Ballo [:botond]]

I tried reproducing this with GL layers enabled in Linux, but wasn't able to.

Thomas, do you know if this is a regression for you?

Comment 4

[Thomas Daede [:TD-Linux]]

Unfortunately no, I turned on GL layers because of a regression in basic layers: (bug 1127910)

BTW, it seems a bit odd that about:checkerboard displays coordinates in CSS pixels rather than screen pixels, is that normal?

Comment 5

[Botond Ballo [:botond]]

(In reply to Thomas Daede from comment #4)
> BTW, it seems a bit odd that about:checkerboard displays coordinates in CSS
> pixels rather than screen pixels, is that normal?

We could print it in screen pixels. I think we just used CSS pixels because that's what was most readily available.

Comment 6

[Thomas Daede [:TD-Linux]]

This bug seems to trigger only for a window size that is higher than 1320 screen pixels. The width of the window doesn't seem to affect it, other than reducing draw time and making the checkerboarding less severe.

Comment 7

[Thomas Daede [:TD-Linux]]

Attachment: Minimap when scrolling

Comment 8

[Botond Ballo [:botond]]

The minimap confirms the observation in comment 1 that the displayport is, for some reason, being positioned so that it's completely weighted above rather than below the viewport.

We also established through IRC discussion that the problem only seems to affect Hi-DPI monitors. Unfortunately, I couldn't reproduce it on a 4K monitor in the Toronto office.

Thomas kindly offered to reproduce the problem with some extra logging enabled, and I suggested enabling APZC logging [1]. Setting needinfo as a reminder.

[1] https://dxr.mozilla.org/mozilla-central/source/gfx/layers/apz/src/AsyncPanZoomController.cpp?from=AsyncPanZoomController.cpp#76

Comment 9

[Thomas Daede [:TD-Linux]]

Here's a snippet from the log:

APZC: Calculated displayport as (0.000000 -1175.000000 1917.000000 3290.000000) from velocity (0,0) paint time 50.000000 metrics:{ [cb=(x=0.000000, y=0.000000, w=3814.000000, h=1880.000000)] [sr=(x=0.000000, y=0.000000, w=1917.000000, h=4258.166504)] [s=(0,1482)] [dp=(x=0.000000, y=0.000000, w=1907.500000, h=940.500000)] [cdp=(x=0.000000, y=0.000000, w=0.000000, h=0.000000)] [color=rgba(255, 255, 255, 1.000000) [dpm=(l=-0.000000, t=2349.499023, r=20.000000, b=2350.500977)] um=0] [rcs=(w=1907.000000, h=940.000000)] [v=(x=0.000000, y=0.000000, w=1907.000000, h=940.000000)] [z=(ld=2.000 r=1.000 cr=1 z=2 er=1)] [u=(0 0 0)] [p=0] [i=(5 3 1)] }
APZC: 7fccc9fa3000 requesting content repaint:{ [cb=(x=0.000000, y=0.000000, w=3814.000000, h=1880.000000)] [sr=(x=0.000000, y=0.000000, w=1917.000000, h=4258.166504)] [s=(0,1482)] [dp=(x=0.000000, y=0.000000, w=1907.500000, h=940.500000)] [cdp=(x=0.000000, y=0.000000, w=0.000000, h=0.000000)] [color=rgba(255, 255, 255, 1.000000) [dpm=(l=-0.000000, t=2350.000000, r=20.000000, b=2350.000000)] um=1] [rcs=(w=1907.000000, h=940.000000)] [v=(x=0.000000, y=0.000000, w=1907.000000, h=940.000000)] [z=(ld=2.000 r=1.000 cr=1 z=2 er=1)] [u=(0 0 0)] [p=0] [i=(5 3 1)] }
APZC: 0x7fccbc4a3000 NotifyLayersUpdated short-circuit
APZC: 0x7fccc9fa3000 NotifyLayersUpdated short-circuit
APZC: 0x7fccbc4a3000 NotifyLayersUpdated short-circuit
APZC: 0x7fccc9fa3000 NotifyLayersUpdated short-circuit
APZC: 0x7fccbc4a3000 NotifyLayersUpdated short-circuit
APZC: 7fccc9fa3000 got a NotifyLayersUpdated with aIsFirstPaint=0, aThisLayerTreeUpdated=1:{ [cb=(x=0.000000, y=0.000000, w=3814.000000, h=1880.000000)] [sr=(x=0.000000, y=0.000000, w=1917.000000, h=4258.166504)] [s=(0,1482)] [dp=(x=0.000000, y=-970.000000, w=1917.000000, h=1920.000000)] [cdp=(x=0.000000, y=0.000000, w=0.000000, h=0.000000)] [color=rgba(255, 255, 255, 1.000000) [dpm=(l=-0.000000, t=2350.000000, r=20.000000, b=2350.000000)] um=0] [rcs=(w=1907.000000, h=940.000000)] [v=(x=0.000000, y=0.000000, w=1907.000000, h=940.000000)] [z=(ld=2.000 r=1.000 cr=1 z=2 er=1)] [u=(0 0 0)] [p=0] [i=(5 3 1)] }

If I'm reading that correctly, it's correctly requesting a paint of ~3x the viewport CSS size (3814), but only getting a 1920 CSS high paint region back.

Comment 10

[Kartikaya Gupta (email:[e-mail])]

I see some v=[...] output in comment 9, which indicates there is a CSS viewport being applied. Do you have any non-default prefs set, such as dom.meta-viewport.enabled or apz.allow_zooming?

Comment 11

[Thomas Daede [:TD-Linux]]

No, I checked about:support and there are no related non-default prefs.

Comment 12

[Kartikaya Gupta (email:[e-mail])]

Botond pointed out that the v=[...] is actually irrelevant because layout will populate that with the scrollport size if there isn't a CSS viewport set. Sorry for that distraction.

I agree with your assessment that the APZC code is requesting a tall displayport but is only getting a relatively short displayport back. Most likely this is being clamped by the code at [1]. The clamping just blindly applies as much of the "budget" as it can to the top margin and then uses the rest for the bottom margin. If your system's max texture size is coming back as 1920 then that would explain this behaviour.

We should probably update that code to distribute the budget more evenly between the top and the bottom margins.

[1] http://mxr.mozilla.org/mozilla-central/source/layout/base/nsLayoutUtils.cpp?rev=2d171d75b746&mark=996-1023#996

Comment 13

[Thomas Daede [:TD-Linux]]

On my system, GL_MAX_TEXTURE_SIZE is 8192, which should be sufficient for the entire area requested, so something in the calculation is going bad. Should I file a separate bug about this?

It would still be good for the viewport to be centered when the maximum texture size is the limitation.

Comment 14

[Kartikaya Gupta (email:[e-mail])]

Yeah, please file a separate bug about that. We can use this bug for centering the viewport. If you're comfortable with debugging either with a debugger or by adding printfs it would be good to find out what's going on in the GetMaxTextureSize call at [1] and why it's returning something unexpected. Note that in our OpenGL compositor backend we seem to use LOCAL_GL_MAX_TEXTURE_SIZE as the max texture size [2], rather than GL_MAX_TEXTURE_SIZE. I'm not sure if there's a difference between the two.

[1] http://mxr.mozilla.org/mozilla-central/source/layout/base/nsLayoutUtils.cpp?rev=2d171d75b746#879
[2] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/opengl/CompositorOGL.cpp?rev=e63011d83825#1627

Comment 15

[Markus Stange [:mstange]]

LOCAL_GL_* is just how we use GL_* enums in mozilla code. Something about GL headers and collisions and includes I think.

Comment 16

[Kartikaya Gupta (email:[e-mail])]

Attachment: Patch

Here's a patch that should keep the relative distribution of the displayport margins when constraining it by the max texture size. Although I think there's a pre-existing bug here because the max texture size is only clamping the margins, not the margins + base rect.

Comment 17

[Botond Ballo [:botond]]

I'll bet the max texture size being used in 4096. Since the hi-DPI screen has a device pixel ratio of 2, that means the CSS size limit of the displayport is 2048. 1920 is 2048 - 128, where 128 is the boundary to which we align the displayport size. We reduce the max size by three times the alignment to be safe, but during alignment we only end up expanding by twice the alignment, giving us a final value that's 128 less than the maximum.

Comment 18

[Botond Ballo [:botond]]

Note also that the default constructor of TextureFactoryIdentifier sets the max texture size to 4096 [1]. So, what could be happening is, we fail to properly initialize the TextureFactoryIdentifier from which we're reading the max texture size for some reason, and use this default value.

[1] https://dxr.mozilla.org/mozilla-central/rev/494289c72ba3997183e7b5beaca3e0447ecaf96d/gfx/layers/CompositorTypes.h#174

Comment 19

[Thomas Daede [:TD-Linux]]

The 4096 actually comes from another bug workaround in the Firefox OpenGL code. I've filed bug 1260922 about this.

Comment 20

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8736382 [details] [diff] [review]
Patch

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #16)
> Created attachment 8736382 [details] [diff] [review]
> Patch
> 
> Here's a patch that should keep the relative distribution of the displayport
> margins when constraining it by the max texture size. Although I think
> there's a pre-existing bug here because the max texture size is only
> clamping the margins, not the margins + base rect.

I moved this patch to bug 1261062 since it's a more specific problem than this bug - although it should help it may not fix this bug entirely. Also ignore the thing about the "pre-existing bug", I misread the code. The base rect is subtracted off as |screenRect| when computing the budget.

Comment 21

[Kartikaya Gupta (email:[e-mail])]

The checkerboarding situation should be better now, it shouldn't checkerboard immediately while scrolling in a particular direction, and should be about the same going up vs going down.

Comment 22

[Thomas Daede [:TD-Linux]]

Yup, the displayport is now nicely centered. There isn't much margin on my 4K monitor, but it's enough to make the experience much better. Bug 1260922 tracks allowing the displayport to be larger.

Comment 23

[Kartikaya Gupta (email:[e-mail])]

Thanks!