Right-size ContainerLayers during LayerTreeInvalidation

RESOLVED FIXED in Firefox 56

Status

()

Core
Graphics: Layers
RESOLVED FIXED
10 months ago
10 months ago

People

(Reporter: dvander, Assigned: dvander)

Tracking

40 Branch
mozilla56
Points:
---

Firefox Tracking Flags

(firefox56 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Layout sometimes computes the visible region of container layers inaccurately. In the past this hasn't mattered because the region is effectively ignored, and recomputed by the compositor. The function that does this is PostProcessLayers  - it performs occlusion culling, and also happens to reset each container's visible region to be the sum of the visible regions of its children. This ends up masking bugs where the visible region computed by layout is too big or too small.

Also, PostProcessLayers runs *before* LayerTreeInvalidation, so invalidation is using the result of occlusion culling. (It should really be called "PreProcessLayers".)

Advanced Layers has the opposite behavior: it invalidates first, then performs occlusion culling on the invalid region. Since the second step happens in a single pass, there is no opportunity to compute the true visible region of containers. Thus when layout has computed too small a region, we render incorrectly (like bug 1359709). When the region is too big, we're allocating excess texture memory and drawing extra pixels.

I'm not sure how big a deal it really is, it's not a common occurrence. But it's worth fixing. Two possibilities for fixing it:
 (1) Delay intermediate surface creation until we finish processing a container's children. Then we can sum up the bounds of children. However this would occur after invalidation, so we'd have to be sure that any size changes we compute were also computed by LTI.
 (2) Do the calculation in LTI, since it already looks at layer bounds.
Created attachment 8881018 [details] [diff] [review]
patch

Turns out option (1) is difficult because AL bakes the render target offset into its draw calls, and we'd have to extend the surface size to include it. Option (2) doesn't make sense because LTI only visits layers from the previous draw, not the current one. Maybe we just have to calculate it the hard way.

This patch recursively adds up the visible bounds of leaf layers if we (1) don't yet have an intermediate surface or (2) LTI computes that a child of the container has changed.
Attachment #8881018 - Flags: review?(matt.woodrow)
Created attachment 8881856 [details] [diff] [review]
v2

This now takes clips into account. It's not pretty but it looks pretty similar to PostProcessLayers.
Attachment #8881018 - Attachment is obsolete: true
Attachment #8881018 - Flags: review?(matt.woodrow)
Attachment #8881856 - Flags: review?(matt.woodrow)
Attachment #8881856 - Flags: review?(matt.woodrow) → review+

Comment 3

10 months ago
Pushed by danderson@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4ef2bbae969a
Compute minimal intermediate surface sizes in Advanced Layers. (bug 1375785, r=mattwoodrow)

Comment 4

10 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/4ef2bbae969a
Status: ASSIGNED → RESOLVED
Last Resolved: 10 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.