Mark layers whether they can use LCD text depending on content-opacity, draw-opacity, and draw-tran…

Mark layers whether they can use LCD text depending on content-opacity, draw-opacity, and draw-transform.

BUG=100666

[alokp, 2012-10-04 22:27:26]

Uploading to make sure we are on the same page. Not ready to check-in. Things
left to do:
- Plumb use-lcd flag to a place that is accessible during paint time
- Add unit tests

[danakj, 2012-10-04 22:33:24]

http://codereview.chromium.org/11027045/diff/1/cc/CCLayerTreeHostCommon.cpp
File cc/CCLayerTreeHostCommon.cpp (right):

http://codereview.chromium.org/11027045/diff/1/cc/CCLayerTreeHostCommon.cpp#n...
cc/CCLayerTreeHostCommon.cpp:619: layer->setUseLCDText(canUseLCDText &&
The layer's opacity and transform affects its subtree too.

I think you might want to consider setting canUseLCDText up at the top before
the if(subtreeRendersToSeparateSurface) thing.

Then here && it with contentsOpaque().

[enne (OOO), 2012-10-04 22:39:24]

Yeah, this is exactly the sort of thing I had in mind.

I'm still not 100% sure on the drawOpacity being required or not.  It's
something that needs investigation.

[danakj, 2012-10-04 22:43:01]

On 2012/10/04 22:39:24, enne wrote:
> Yeah, this is exactly the sort of thing I had in mind.
> 
> I'm still not 100% sure on the drawOpacity being required or not.  It's
> something that needs investigation.

drawOpacity is computed from all of its ancestors' opacity values right?
drawOpacity == 1 <==> all ancestors opacity == 1

[enne (OOO), 2012-10-04 22:45:56]

On 2012/10/04 22:43:01, danakj wrote:
> On 2012/10/04 22:39:24, enne wrote:
> > Yeah, this is exactly the sort of thing I had in mind.
> > 
> > I'm still not 100% sure on the drawOpacity being required or not.  It's
> > something that needs investigation.
> 
> drawOpacity is computed from all of its ancestors' opacity values right?
> drawOpacity == 1 <==> all ancestors opacity == 1

I meant that I'm not sure that having opaque contents but alpha blending a layer
is incorrect for lcd text.

Also, drawOpacity could be 1 but an ancestor could create a surface with a non-1
opacity.

[danakj, 2012-10-04 22:46:48]

On 2012/10/04 22:45:56, enne wrote:
> On 2012/10/04 22:43:01, danakj wrote:
> > On 2012/10/04 22:39:24, enne wrote:
> > > Yeah, this is exactly the sort of thing I had in mind.
> > > 
> > > I'm still not 100% sure on the drawOpacity being required or not.  It's
> > > something that needs investigation.
> > 
> > drawOpacity is computed from all of its ancestors' opacity values right?
> > drawOpacity == 1 <==> all ancestors opacity == 1
> 
> I meant that I'm not sure that having opaque contents but alpha blending a
layer
> is incorrect for lcd text.

Ah ok!

> Also, drawOpacity could be 1 but an ancestor could create a surface with a
non-1
> opacity.

Right you are :)

[jamesr, 2012-10-04 22:54:25]

You also have to consider if the layer is being filtered in a way that distorts
pixels (you don't want fringing if you end up blurring the layer, for instance)

[enne (OOO), 2012-10-05 16:21:59]

Other wrinkles:

* layers whose screen space positions are being animated

* pinch zoom

I'm not sure what to do about pinch zoom.  I wish we could say that we just
won't use LCD text on any platform that can pinch, but Win8 seems problematic
there.  I'm not sure whether we should responsively allow pinches (but with
bad-looking color fringing on text) or instead jank on the first (or every?)
pinch.

http://codereview.chromium.org/11027045/diff/1/cc/LayerChromium.cpp
File cc/LayerChromium.cpp (right):

http://codereview.chromium.org/11027045/diff/1/cc/LayerChromium.cpp#newcode94
cc/LayerChromium.cpp:94: //setNeedsDisplay();
I would rename this function setCanUseLCDText() and defer the decision about
whether to use LCD text to the derived layer type.  That would allow the derived
layer type to manage the invalidation more intelligently.  It would allow the
derived layer type to also override this setting (it flips back and forth, so
just leave it off).

[alokp, 2012-10-05 17:54:20]

This is how I was planning on handling animations - do not adjust LCD setting
while opacity or transform is animating. This may show look in some cases, but
would avoid the initial jank.

Is there a flag somewhere for pinch-zoom?

http://codereview.chromium.org/11027045/diff/1/cc/CCLayerTreeHostCommon.cpp
File cc/CCLayerTreeHostCommon.cpp (right):

http://codereview.chromium.org/11027045/diff/1/cc/CCLayerTreeHostCommon.cpp#n...
cc/CCLayerTreeHostCommon.cpp:619: layer->setUseLCDText(canUseLCDText &&
On 2012/10/04 22:33:24, danakj wrote:
> The layer's opacity and transform affects its subtree too.
> 
> I think you might want to consider setting canUseLCDText up at the top before
> the if(subtreeRendersToSeparateSurface) thing.
> 
> Then here && it with contentsOpaque().

It does but if the layer does not create a render-surface, we can defer the
decision to the child layer. It may happen that later down in the hierarchy, the
transform becomes integral translation. I disable LCD text for subtree only when
the render-target cannot support it.

Anyways I removed duplicated code in the new patch.

[alokp, 2012-10-11 21:12:11]

http://codereview.chromium.org/11027045/diff/1/cc/LayerChromium.cpp
File cc/LayerChromium.cpp (right):

http://codereview.chromium.org/11027045/diff/1/cc/LayerChromium.cpp#newcode94
cc/LayerChromium.cpp:94: //setNeedsDisplay();
On 2012/10/05 16:21:59, enne wrote:
> I would rename this function setCanUseLCDText() and defer the decision about
> whether to use LCD text to the derived layer type.  That would allow the
derived
> layer type to manage the invalidation more intelligently.  It would allow the
> derived layer type to also override this setting (it flips back and forth, so
> just leave it off).

Done.

[jamesr, 2012-10-16 04:17:55]

On 2012/10/05 17:54:20, Alok Priyadarshi wrote:
> This is how I was planning on handling animations - do not adjust LCD setting
> while opacity or transform is animating. This may show look in some cases, but
> would avoid the initial jank.

I don't think this is going to work - could you produce some test pages (use a
png with rasterized text to emulate LCD) and try it out, but based on how Safari
behaves with pinch-zoom even on retina displays it looks horrible.

Most of the time, when we start a CSS animation on a layer the animation
triggers compositing so we have to repaint the content anyway.

> 
> Is there a flag somewhere for pinch-zoom?
> 

No, but that's because currently pinch zoom is only enabled for Android AFAIK.