Open Bug 1254260 Opened 4 years ago Updated 29 days ago

Perspective-transformed elements sometimes not painted during scrolling

Categories

(Core :: Layout, defect)

defect
Not set
normal

Tracking

()

Tracking Status
firefox47 --- wontfix
firefox48 + wontfix
firefox49 + wontfix
firefox50 --- wontfix
firefox51 --- wontfix
firefox52 --- fix-optional
firefox53 --- fix-optional

People

(Reporter: mstange, Unassigned)

References

Details

(Keywords: regression)

Attachments

(2 files)

This is a tricky one.

STR (best chances with velocity bias 0 or with the patches from bug 1192910):
 1. Load the attached testcase.
 2. Scroll down until only the rightmost square is in the viewport.
 3. Scroll back up.

Now the other squares sometimes appear too late.


These elements have their z coordinates "in front" of the scrolled area, so during scrolling they move faster than the z=0 plane.

I think what's happening here is that these elements are considered outside of the display port, and then during scrolling they move into the display port before the display port actually changes, and they don't get painted.

And I think "outside of the displayport" currently means "not intersecting the 3d cone between the displayport rect and the camera".
Maybe the display port needs to be a cuboid instead, extending infinitely to the front and the back along the z axis?
Attached file testcase
Oh, this only reproduces properly if perspective scrolling actually works, i.e. after bug 1247854.
This blocks bug 1264297, which is tagged as a regression - does that mean this is a regression as well?  It may not, wanted to check.
Flags: needinfo?(mstange)
This is caused by APZ, so yes, it's a regression over non-APZ.
Flags: needinfo?(mstange)
Keywords: regression
Tracking since this is an apz related regression. If this is minor we may not need to track though.
Attached patch ReftestSplinter Review
I turned Markus' test case into a reftest in the hopes that I could land it with bug 1264297. However I think only the proper fix in this bug will make it pass (see bug 1264297 comment 10). I'm attaching the reftest here anyway as it will come in handy later.
Depends on: 1198135
I've been thinking about the best way to solve this:

I think when we enter BuildDisplayListForStackingContext for a transformed frame, that is the child of a scrolled frame that has both perspective and a display port, then we need to override the dirty rect.

This also includes frames that participate in a preserve-3d context, where the root frame of the preserve-3d context meets the above criteria.

The latest patch in bug 1198135 introduces a way to compute the 'scrolling rate' for these perspective transformed frames.

If we take the display port margins, scale them by the inverse of the scrolling rate for the given frame and then recompute the displayport using these new margins then we should get the correct result.
(In reply to Matt Woodrow (:mattwoodrow) from comment #7)
> If we take the display port margins, scale them by the inverse of the
> scrolling rate for the given frame and then recompute the displayport using
> these new margins then we should get the correct result.

I take back the inverse part, an element that scrolls at double normal speed will compute a scrolling rate of 2.0, and we want to double the displayport margins for this case.
(In reply to Matt Woodrow (:mattwoodrow) from comment #8)
> (In reply to Matt Woodrow (:mattwoodrow) from comment #7)
> > If we take the display port margins, scale them by the inverse of the
> > scrolling rate for the given frame and then recompute the displayport using
> > these new margins then we should get the correct result.
> 
> I take back the inverse part, an element that scrolls at double normal speed
> will compute a scrolling rate of 2.0, and we want to double the displayport
> margins for this case.

Hi :mattwoodrow,
Is there any updates for this bug?
Flags: needinfo?(matt.woodrow)
I'm still trying to get the dependent bug (bug 1198135) landed, then I'll move onto fixing this.
Flags: needinfo?(matt.woodrow)
I can't reproduce this in Nightly 51 now that bug 1198135 has landed.

Markus: if it works for you, can you close this bug? Thanks!
Flags: needinfo?(mstange)
I can't reproduce it either but that's probably because we disabled paint skipping for this case, in bug 1264297. We want to fix this properly and then back out bug 1264297 to enable paint skipping again.

Matt will know whether there's more work to do here.
Flags: needinfo?(mstange) → needinfo?(matt.woodrow)
I think we still want to fix this properly, and allow paint skipping again, since it'll improve battery life.

That said, it's relatively minor, and I don't think we need to track this bug.

Liz, can we remove the tracking flags from this bug?
Flags: needinfo?(matt.woodrow) → needinfo?(lhenry)
Sure. Thanks Matt!  It may get picked up by the platform triage meeting again for 51, though.
Flags: needinfo?(lhenry)
You need to log in before you can comment on or make changes to this bug.