1238571 - APZ scrolling broken in the log preview on treeherder, which has a scrollbox inside a position:fixed body element

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[Markus Stange [:mstange]]

Attachment: testcase

STR:
 1. Load the attached testcase.
 2. In the scroll frame at the bottom, scroll down until you're roughly in the middle of the scrollable range (to work around bug 1192910).
 3. Slowly scroll up and down.

Instead of scrolling smoothly, the scrolled layer will only update its position when the display port changes or when something else triggers a main thread paint, e.g. when you hover over a scrollbar. Interestingly, the scrollbar thumbs are still moving the way you'd expect them to.

Comment 1

[Markus Stange [:mstange]]

This is due to body { position: fixed; }. Botond, do you want to take a look?

Comment 2

[Markus Stange [:mstange]]

Attachment: reduced testcase

Comment 3

[Botond Ballo [:botond]]

(In reply to Markus Stange [:mstange] from comment #1)
> This is due to body { position: fixed; }. Botond, do you want to take a look?

Sure.

Comment 4

[Botond Ballo [:botond]]

I can't reproduce the problem on Linux desktop using today's nightly build.

Comment 5

[Botond Ballo [:botond]]

(In reply to Botond Ballo [:botond] from comment #4)
> I can't reproduce the problem on Linux desktop using today's nightly build.

Nor on a Linux laptop using the trackpad.

Markus, assuming you were testing on OS X with a trackpad, is it possible the issue is specific to that input method? Could you test this by attaching a mouse and trying a wheel-scroll?

Alternatively, do you have any local patches applied that might be causing this?

Comment 6

[Botond Ballo [:botond]]

(In reply to Botond Ballo [:botond] from comment #4)
> I can't reproduce the problem on Linux desktop using today's nightly build.

Actually, that seems to be because we're painting after just about every composite, which I'm guessing is a bug unto its own.

I do see the scroll pause when painting gets backed up and we do a few composites without painting, indicating that there is indeed a problem in AsyncCompositionManager.

Comment 7

[Botond Ballo [:botond]]

(In reply to Botond Ballo [:botond] from comment #6)
> (In reply to Botond Ballo [:botond] from comment #4)
> > I can't reproduce the problem on Linux desktop using today's nightly build.
> 
> Actually, that seems to be because we're painting after just about every
> composite, which I'm guessing is a bug unto its own.

The optimization put in place in bug 1187432 (to avoid a paint when the displayport didn't move) is firing, but it looks like something else is scheduling a paint every composite.

Comment 8

[Botond Ballo [:botond]]

(In reply to Botond Ballo [:botond] from comment #7)
> (In reply to Botond Ballo [:botond] from comment #6)
> > (In reply to Botond Ballo [:botond] from comment #4)
> > > I can't reproduce the problem on Linux desktop using today's nightly build.
> > 
> > Actually, that seems to be because we're painting after just about every
> > composite, which I'm guessing is a bug unto its own.
> 
> The optimization put in place in bug 1187432 (to avoid a paint when the
> displayport didn't move) is firing, but it looks like something else is
> scheduling a paint every composite.

Filed bug 1238805 for this; will continue investigation there.

Comment 9

[Botond Ballo [:botond]]

So on this page, we have:

  - a non-scrollable RSF, which is also position:fixed
  - a scrollable subframe

Neither the scrollframes nor the position:fixed create a ContainerLayer, so we end up with a PaintedLayer which has the following annotations:

  - metrics for the RSF
  - metrics for the subframe
  - a "fixed" annotation that says it's fixed with respect to the RSF

It gets the fixed annotation because it's one of the layers that contains content inside the fixed-position frame.

The problem is, APZ can't distinguish this from a situation where the layer represents content inside the scrollable subframe with that content being marked fixed-position, which produces the same set of annotations. APZ interprets the annotations as indicating this latter scenario, and accordingly untransforms the layer by the async transform of the subframe to keep the content fixed with respect to the RSF.

Comment 10

[Botond Ballo [:botond]]

Matt Woodrow is working on having fixed-position frames create ContainerLayers in bug 1231538. I think that's going to fix this problem, so I'll wait for that to land.

Comment 11

[Botond Ballo [:botond]]

Attachment: Reftest

Here's the reduced testcase turned into a reftest. However we end up fixing this bug, we should land the reftest.

Comment 12

[Botond Ballo [:botond]]

(In reply to Botond Ballo [:botond] from comment #10)
> Matt Woodrow is working on having fixed-position frames create
> ContainerLayers in bug 1231538. I think that's going to fix this problem, so
> I'll wait for that to land.

Confirmed that the patch in bug 1231538 (more precisely, I pulled from [1]) fixes this bug.

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=79c557f064a4

Comment 13

[Botond Ballo [:botond]]

Bug 1231538 has now landed. Remaining step is to land the reftest here.

Comment 14

[Botond Ballo [:botond]]

Attachment: MozReview Request: Bug 1238571 - Reftest. r=mstange

Review commit: https://reviewboard.mozilla.org/r/32833/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/32833/

Comment 15

[Botond Ballo [:botond]]

Oh, Markus is away until Feb 15. I will ask Matt for review instead.

Comment 16

[Botond Ballo [:botond]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3817fd9116f3

Comment 17

[Markus Stange [:mstange]]

Comment on attachment 8713837 [details]
MozReview Request: Bug 1238571 - Reftest. r=mstange

https://reviewboard.mozilla.org/r/32833/#review29701

here's a last minute review

Comment 18

[Botond Ballo [:botond]]

Comment on attachment 8713837 [details]
MozReview Request: Bug 1238571 - Reftest. r=mstange

Thanks!

Comment 19

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f08d02ec2952

Comment 20

[Phil Ringnalda (:philor)]

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