Use GetEffectiveVisibleRegion instead of GetVisibleRegion consistently in LayerTreeInvalidation

RESOLVED FIXED in Firefox 47

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: botond, Assigned: botond)

Tracking

Trunk
mozilla47
Points:
---

Firefox Tracking Flags

(firefox47 fixed)

Details

(Whiteboard: [gfx-noted])

Attachments

(1 attachment)

In bug 1161978, Sotaro fixed a bug where LayerTreeInvalidation was calling GetVisibleRegion() instead of GetEffectiveVisibleRegion(), which gave wrong results on the compositor side (where the visible region can have changed by async transformations, and GetEffectiveVisibleRegion() accounts for these changes by GetVisibleRegion() does not).

I think that, for the same reason, all calls to GetVisibleRegion() in LayerTreeInvalidation.cpp should be calls to GetEffectiveVisibleRegion() instead.

Sotaro, please correct me if I'm mistaken.
(Note that GetEffectiveVisibleRegion() will be renamed to GetLocalVisibleRegion() in bug 1247445.)
Assignee: nobody → botond
A survey of callers of GetVisibleRegion() revealed a few other call sites in the compositor that should probably be using GetEffectiveVisibleRegion() as well. My patch updates all of these.
layout/reftests/async-scrolling/checkerboard-3.html is failing. I'm investigating.
Comment on attachment 8719005 [details]
MozReview Request: Bug 1247452 - Use the effective visible region rather than the visible region where appropriate in the compositor. r=mattwoodrow

Dropping review flag until I figure out what's going on with the test.
Attachment #8719005 - Flags: review?(matt.woodrow)
The short answer is: this patch makes compositor-side culling more effective - so effective, in fact, that we're breaking a scenario related to checkerboard background color drawing where we were relying on it *not* happening. Basically, more aggressive culling exposed a bug (much as in bug 1237827).
Specifically, the change to CalculateScissorRect() is causing the checkerboarding layer's clip rect, as calculated in ContainerPrepare(), to become empty. This leads to culling the layer, but even if we avoid that (by adding a guard the way we did to the "empty visible region" culling path in bug 1013385), an empty clip rect will lead to no checkerboard color being drawn (because the checkerboard color is restricted to the clip rect).
Comment on attachment 8719005 [details]
MozReview Request: Bug 1247452 - Use the effective visible region rather than the visible region where appropriate in the compositor. r=mattwoodrow

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/34839/diff/1-2/
Attachment #8719005 - Flags: review?(matt.woodrow)
The updated patch omits the change to CalculateScissorRect(), since it interferes with the desired checkerboard background drawing behaviour.
Comment on attachment 8719005 [details]
MozReview Request: Bug 1247452 - Use the effective visible region rather than the visible region where appropriate in the compositor. r=mattwoodrow

https://reviewboard.mozilla.org/r/34839/#review31493

Probably worth adding a comment to the scissor rect code about why we need to use client side visible region instead of the effective/shadow one.
Attachment #8719005 - Flags: review?(matt.woodrow) → review+
Rebased across bug 1247445, added comment as requested, and landed.
https://hg.mozilla.org/mozilla-central/rev/afeaae6740e5
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.