889953 - transformed color layers seem to cause intermediate surfaces

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Comment 1

[Matt Woodrow (:mattwoodrow)]

When we optimize a ThebesLayer into a ColorLayer we always set a clip rect onto the ColorLayer.

If the parent of the ColorLayer is a transformed ContainerLayer, then this forces an intermediate surface.

We can't skip the intermediate surface, because folding the ContainerLayer transform onto the child would require us to clip to a non-axis-aligned rectangle, which we don't support.

Above URL has a large number of layers that hit this.

A few possible options:

* Don't optimize ThebesLayers to ImageLayers if it's going to force a temporary
  - This would be a huge memory regression for the above page. Sadness

* Set the visible region on the colour layers and change the layer implementations to restrict drawing to, and drop the clip rect.

* Add a pre-transform clip property to layers and use this.

* Detect that the ColorLayer has no transform (or a simple translation only), and treat the clip as being pre-transform. Adjust the layer implementations to apply this by adjusting the quad rect to draw.

Comment 2

[Matt Woodrow (:mattwoodrow)]

Attachment: Don't clip ColorLayers

We should just require that ColorLayers don't render outside their visible region.

All the accelerated and OMTC backends do this already, it's only BasicLayers.

I don't think it costs us anything on BasicLayers either, we won't clip to the clip rect, but will clip to the visible region (which is always a rect) instead.

This is a pretty huge perf win for this page at least.

Comment 3

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

(In reply to Matt Woodrow (:mattwoodrow) from comment #1)
> Above URL has a large number of layers that hit this.

There is no URL.

(In reply to Matt Woodrow (:mattwoodrow) from comment #2)
> We should just require that ColorLayers don't render outside their visible
> region.

I am not a fan of this. Up till now, visible regions have just been advisory; using a bigger one would not be a bug. Now it will be.

How about just adding a rectangle property to ColorLayer and say that ColorLayer fills that rectangle, instead of the whole plane?

Comment 4

[Sotaro Ikeda [:sotaro]]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #3)
> (In reply to Matt Woodrow (:mattwoodrow) from comment #1)
> > Above URL has a large number of layers that hit this.
> 
> There is no URL.

http://threejs.org/examples/css3d_molecules.html

"Graphite" create a lot of layers.

Comment 5

[Matt Woodrow (:mattwoodrow)]

Attachment: Don't clip ColorLayers v2

Ok, fine with me.

Comment 6

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

Comment on attachment 772238 [details] [diff] [review]
Don't clip ColorLayers v2

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

::: gfx/layers/basic/BasicColorLayer.cpp
@@ +42,1 @@
>      PaintColorTo(mColor, GetEffectiveOpacity(), aContext, aMaskLayer);

Wouldn't it be faster to do a fillrect than to clip+paint?

Comment 7

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a17b6115cdd7

Comment 8

[Matt Woodrow (:mattwoodrow)]

Backed out for causing reftest failures: https://hg.mozilla.org/integration/mozilla-inbound/rev/27c7258febfe

Comment 9

[Matt Woodrow (:mattwoodrow)]

Attachment: Don't clip ColorLayers v3

The clips used by ColorLayers are clipped to device space with BasicLayers. This makes the ColorLayer bounds do the same.

It also adds a new gfxContext::Rectangle overload to make this more obvious, the current bool param isn't clear.

Comment 10

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a38f49d05074

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a38f49d05074