Closed Bug 1238571 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: Panning and Zooming, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox46 --- wontfix
firefox47 --- fixed

People

(Reporter: mstange, Assigned: botond)

References

Details

Attachments

(4 files)

Attached file 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.
This is due to body { position: fixed; }. Botond, do you want to take a look?
Flags: needinfo?(botond)
Summary: APZ scrolling broken in the log preview on treeherder → APZ scrolling broken in the log preview on treeherder, which has a scrollbox inside a position:fixed body element
Attached file reduced testcase
(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.
Assignee: nobody → botond
Flags: needinfo?(botond)
I can't reproduce the problem on Linux desktop using today's nightly build.
(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?
Flags: needinfo?(mstange)
(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.
Flags: needinfo?(mstange)
(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.
(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.
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.
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.
Depends on: 1231538
Attached patch ReftestSplinter Review
Here's the reduced testcase turned into a reftest. However we end up fixing this bug, we should land the reftest.
See Also: → 1239943
(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
Bug 1231538 has now landed. Remaining step is to land the reftest here.
Oh, Markus is away until Feb 15. I will ask Matt for review instead.
Attachment #8713837 - Flags: review?(mstange) → review?(matt.woodrow)
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
Attachment #8713837 - Flags: review+
Comment on attachment 8713837 [details]
MozReview Request: Bug 1238571 - Reftest. r=mstange

Thanks!
Attachment #8713837 - Flags: review?(matt.woodrow)
https://hg.mozilla.org/mozilla-central/rev/f08d02ec2952
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.