911471 - reduce composition of occluded regions

Status: RESOLVED DUPLICATE of bug 1085223

(Core :: Graphics: Layers, defect)

Description

[Andreas Gal :gal]

When compositing, we often end up rendering pixels that will be occluded by layers we render on top of them. This patch goes through the layers of each container in reverse Z order and tries to shrink the scissor rect to avoid painting pixels that are guaranteed to not be visible anyway. This approach is not complete since we only have a scissor rect but we might have to paint a complex region of the layer since several parts of it are occluded. A complete approach would require breaking such layers into multiple quads.

Comment 1

[Andreas Gal :gal]

Bug 833897 should use the new method introduced by this bug.

Comment 2

[Andreas Gal :gal]

Attachment: patch

Comment 3

[Andreas Gal :gal]

Here is an example of this patch in action, starting my mac desktop FF with a page:

original scissor rect: 0 0 2880 52
original scissor rect: 0 0 2880 1704
original scissor rect: 0 130 2880 1574
original scissor rect: 0 0 2880 1704
new scissor rect: 0 0 2880 130

The last rect is the UI which gets composited underneath the content. Its height is shrunk to 130, the actual height of the UI on my Retina mac, because opaque content is known to sit on top of the rest. The rect 0 130 2880 1574 is not composited, which saves some 18MB VRAM traffic every time we composite. On desktop this doesn't matter that much for performance, but might affect battery life.

Comment 4

[Andreas Gal :gal]

Attachment: patch

Updated patch.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Do we need to worry about this region getting very complicated, giving O(N^2) behavior?

Comment 6

[Andreas Gal :gal]

We have seen problems like that before. Since I am simplifying out to the bounds of the scissor rect anyway, I could as well put a limit of 5 rects or so on the region and then start simplifying on the spot. Wdyt?

Comment 7

[Boris Zbarsky [:bzbarsky]]

You can probably go higher than 5; in the past we've used things like 100 to prevent the _really_ bad edge cases.  We don't want to go too low, because the region-simplification in question is a SimplifyInward on "occluded", right?  So we might just miss whole chunks of opaque things once we hit the limit we set.

(On a separate note, I wonder whether a smarter algorithm for SimplifyInward than what we have now is worth thinking about.)

Comment 8

[Avi Halachmi (:avih)]

I wonder if wins from this optimization come up in any of our tests...

Comment 9

[Andreas Gal :gal]

Attachment: patch

Simplify inward once we hit 100 rects to prevent the occluded region from exploding quadratically.

Comment 10

[Andreas Gal :gal]

Bas, I think at least D3D10 supports multiple scissor rects. Do you think its worth changing the signatures to hand down a region and GL will use the bounds and Windows the actual region?

Comment 11

[Andreas Gal :gal]

glScissorArrayv is not available until GL 4.1. It would be pretty straight forward to emulate it in GLContext if we ever have to by allocating an VBO and splitting the supplied rect into sub-rects applying the scissor rects.

Comment 12

[Andreas Gal :gal]

A couple follow-up items:
- for this to work well we want as many regions to be opaque as possible, it might be even worth it to analyze RGBA content and on the fly infer that its actually opaque even when we didn't know when drawing it
- as bz suggests, its probably time to measure our region code a bit

Comment 13

[Andreas Gal :gal]

Filed profiling and optimizing the region code as bug 911590.

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Boris Zbarsky [:bz] from comment #7)
> You can probably go higher than 5; in the past we've used things like 100 to
> prevent the _really_ bad edge cases.  We don't want to go too low, because
> the region-simplification in question is a SimplifyInward on "occluded",
> right?  So we might just miss whole chunks of opaque things once we hit the
> limit we set.
> 
> (On a separate note, I wonder whether a smarter algorithm for SimplifyInward
> than what we have now is worth thinking about.)

Jeff is switching our region implementation to use pixman regions which behave better and rely less on Simplify*.

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 798306 [details] [diff] [review]
patch

Review of attachment 798306 [details] [diff] [review]:
-----------------------------------------------------------------

For most cases (not involving compositor-driven animations) we've already performed occlusion detection in the display list and the visible regions of the layers should reflect that. So, I wonder if we could just modify Layer::CalculateScissorRect to restrict the scissor rect to the bounds of the visible region (after applying the layer transform to the visible region)?

::: gfx/layers/Layers.cpp
@@ +788,5 @@
> +        aScissorRects[i] = scissorRect;
> +      }
> +      // Updated the occluded region only after we determined the clipping rect
> +      // for the current layer since the layer doesn't occlude itself.
> +      if (layer->GetContentFlags() & CONTENT_OPAQUE) {

There's a problem here: layer->GetContentFlags() & CONTENT_OPAQUE only guarantees that the layer's visible region is opaque, not necessarily the entire area of the layer inside its computed scissor rect.

::: gfx/layers/Layers.h
@@ +1476,5 @@
>    void SortChildrenBy3DZOrder(nsTArray<Layer*>& aArray);
> +  void GetScissorRectsForSortedChildren(const nsIntRect& aCurrentScissorRect,
> +                                        const gfxMatrix* aWorldTransform,
> +                                        const nsTArray<Layer*>& aLayers,
> +                                        nsTArray<nsIntRect>& aScissorRects);

This needs to be documented to make clear that we compute one scissor rect for each element of aLayers.

Comment 16

[Andreas Gal :gal]

roc, we are definitely not doing this right whenever we cross document boundaries (XUL->content) and also process boundaries. So I don't think CalculateScissorRect will suffice, but I will redo the patch to take the visible region into account.

Comment 17

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Although I suppose those visible regions are being used to construct the quads already, so if this scissor-rect stuff helps, the visible regions set on the layers must not be accurate enough.

Maybe that's because nsDisplayTransform::ComputeVisibility always gives up on updating aVisibleRegion to occlude the display items under it?

Peter, a full display list dump and layer tree dump for the homescreen would be really useful.

Comment 18

[Andreas Gal :gal]

roc, apply the patch and start up a browser. I already see this help by reducing the size of the scissor rect for the XUL window under the content window (since content covers the bottom, minus the UI on top). I think thats very similar to what happens with the home screen where we have a color layer thats fully covered by the background image in a different process (maybe we don't see that thats rgb?).

Comment 19

[Peter Chang[:pchang]]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #17)
> Although I suppose those visible regions are being used to construct the
> quads already, so if this scissor-rect stuff helps, the visible regions set
> on the layers must not be accurate enough.
> 
> Maybe that's because nsDisplayTransform::ComputeVisibility always gives up
> on updating aVisibleRegion to occlude the display items under it?
> 
> Peter, a full display list dump and layer tree dump for the homescreen would
> be really useful.


I created bug 907048 for one over painting case during video playback.
And bug 907109 for layer tree dump for homescreen.

Comment 20

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Andreas Gal :gal from comment #18)
> roc, apply the patch and start up a browser. I already see this help by
> reducing the size of the scissor rect for the XUL window under the content
> window (since content covers the bottom, minus the UI on top).

My layer tree for a browser showing about:blank looks like this:
ClientLayerManager (0x7f7c58009800)
  ClientContainerLayer (0x7f7c065c9800) [visible=< (x=0, y=0, w=1717, h=996); >] [opaqueContent] [metrics={ viewport=(x=0.000000, y=0.000000, w=1717.000000, h=996.000000) viewportScroll=(x=0.000000, y=0.000000) displayport=(x=0.000000, y=0.000000, w=0.000000, h=0.000000) scrollId=0 }]
    ClientThebesLayer (0x7f7c065c9c00) [visible=< (x=0, y=0, w=1717, h=68); >] [opaqueContent] [isFixedPosition anchor=0.000000,0.000000] [valid=< (x=0, y=0, w=1717, h=68); >]
    ClientContainerLayer (0x7f7c065ca800) [clip=(x=0, y=68, w=1717, h=928)] [visible=< (x=0, y=68, w=1717, h=928); >] [opaqueContent] [isFixedPosition anchor=0.000000,0.000000]
      ClientThebesLayer (0x7f7c1691d400) [clip=(x=0, y=0, w=0, h=0)] [transform=[ 1 0; 0 1; 0 68; ]] [not visible] [isFixedPosition anchor=0.000000,0.000000]
      ClientColorLayer (0x7f7c1691d800) [transform=[ 1 0; 0 1; 0 68; ]] [visible=< (x=0, y=0, w=1717, h=928); >] [opaqueContent] [isFixedPosition anchor=0.000000,0.000000] [color=rgba(255, 255, 255, 1)]

The content of the XUL window is drawn into layer 0x7f7c065c9c00. Your patch gives it a scissor rect of 0,0,1717,68. But we already knew that its visible region is x=0, y=0, w=1717, h=68 (that comes from the display list visibility calculations, which take into account occlusion by the content window), and that determines the quad that gets drawn, so AFAIK improving the scissor rect has no effect on GPU resource usage in this case.

> I think thats
> very similar to what happens with the home screen where we have a color
> layer thats fully covered by the background image in a different process
> (maybe we don't see that thats rgb?).

Maybe. Maybe not. We need to look at the full layer tree and display list dumps to understand at what stage occlusion information is being lost, and why.

Comment 21

[Andreas Gal :gal]

We have the tree somewhere. I will find the bug.

Comment 22

[Peter Chang[:pchang]]

Attachment: 0001-avoid-overcomposition.patch

I made a patch to detect the over-composition condition and it could skip the redundant colorlayer during video playback. 

But it had problem (skip two layers)during the animation when I switch video to background by homekey.

Comment 23

[Peter Chang[:pchang]]

Attachment: avoid-overcomposition v2

fix wrong detection of opacity layer by checking the content flag.

Now it could skip the redundant colorlayer composition correctly during video playback.

Comment 24

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 798306 [details] [diff] [review]
patch

roc is a way better reviewer here.

Comment 25

[Robert O'Callahan (:roc) (email my personal email if necessary)]

One reason that layer visible regions can be set to include areas that are covered by other opaque layers is for layers with animated transforms; for those layers we prerender the layer contents, which we do by treating the entire contents of the layer as visible.

Other than that, I hope that display list visibility computation would avoid overdrawing.

I'd like to focus on specific test cases that exhibit compositor overdrawing and find out what the underlying problem is (e.g., whether it's due to layer prerendering). We may be able to fix the problem without doing occlusion computation again in the compositor.

Comment 26

[Peter Chang[:pchang]]

Comment on attachment 798795 [details] [diff] [review]
avoid-overcomposition v2

I put my patch back to bug 907048.

Comment 27

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 798306 [details] [diff] [review]
patch

Review of attachment 798306 [details] [diff] [review]:
-----------------------------------------------------------------

Generally display-list visibility computation should be reducing the visible regions of layers to account for occlusion. Bug 907048 fixed a specific case where that doesn't work: when we're using RefLayers to composite together layer trees from different processes, so display list construction for one process doesn't know which areas are opaque due to the rendering of other processes. If there are other cases that bug 907048 didn't fix, I think we should track them down and try to fix them with targeted fixes rather than doing general occlusion culling in the layer tree.

Comment 28

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.