Closed Bug 1247452 Opened 7 years ago Closed 7 years ago

Use GetEffectiveVisibleRegion instead of GetVisibleRegion consistently in LayerTreeInvalidation

Categories

(Core :: Graphics: Layers, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: botond, Assigned: botond)

References

Details

(Whiteboard: [gfx-noted])

Attachments

(1 file)

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.
See Also: → 1248822
https://hg.mozilla.org/mozilla-central/rev/afeaae6740e5
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.