Smart layer invalidation for LCD text.

Smart layer invalidation for LCD text.

When LCD text needs to be disabled for a layer, we can invalidate just the tiles containing text.
This only works with impl-side painting where it is very cheap to mark tiles that have text.

BUG=181245
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=192936

[alokp, 2013-04-05 18:48:16]

I thought it was a smart idea until I wrote the unittest. It seems
AnalysisCanvas does not clip text. So if text is present in SkPicture, all tiles
will be marked as having text.

Vlad - Am I missing something? How is solid-color detection working if the layer
has text?

[vmpstr, 2013-04-05 20:04:30]

> Vlad - Am I missing something? How is solid-color detection working if the
layer
> has text?

I thought SkCanvas would clip away text before passing it to the device, but it
seems that's only happening for a few cases (rect, bitmap, etc). This is
probably contributing to some false negatives in the solid color prediction
(tile is solid, but we predict that it's not)

There's more clipping that happens in picture_pile_impl Raster call. That is, if
a picture's content bounds is outside of the tile clip, then the whole picture
is not rasterized.

[enne (OOO), 2013-04-05 21:39:36]

https://codereview.chromium.org/13726013/diff/1/skia/ext/analysis_canvas.cc
File skia/ext/analysis_canvas.cc (right):

https://codereview.chromium.org/13726013/diff/1/skia/ext/analysis_canvas.cc#n...
skia/ext/analysis_canvas.cc:191: color_ = color;
Do you need to set hasText_ here?  Can you add a test for this?

Also, the same thing anywhere else we set isSolidColor = true.

https://codereview.chromium.org/13726013/diff/1/skia/ext/analysis_canvas.h
File skia/ext/analysis_canvas.h (right):

https://codereview.chromium.org/13726013/diff/1/skia/ext/analysis_canvas.h#ne...
skia/ext/analysis_canvas.h:38: bool hasText() const;
Is this file not in Chromium style?

[alokp, 2013-04-05 22:50:11]

Longer term I would like to track text region in the same way we track opaque
regions. Once we do that Tile::hasText() would just need to check if the text
region is empty.

https://codereview.chromium.org/13726013/diff/1/skia/ext/analysis_canvas.cc
File skia/ext/analysis_canvas.cc (right):

https://codereview.chromium.org/13726013/diff/1/skia/ext/analysis_canvas.cc#n...
skia/ext/analysis_canvas.cc:191: color_ = color;
Good catch. Done. Added Tests.

https://codereview.chromium.org/13726013/diff/1/skia/ext/analysis_canvas.h
File skia/ext/analysis_canvas.h (right):

https://codereview.chromium.org/13726013/diff/1/skia/ext/analysis_canvas.h#ne...
skia/ext/analysis_canvas.h:38: bool hasText() const;
We seem to be following skia style in this file.

[enne (OOO), 2013-04-05 22:58:12]

lgtm

https://codereview.chromium.org/13726013/diff/1/skia/ext/analysis_canvas.h
File skia/ext/analysis_canvas.h (right):

https://codereview.chromium.org/13726013/diff/1/skia/ext/analysis_canvas.h#ne...
skia/ext/analysis_canvas.h:38: bool hasText() const;
On 2013/04/05 22:50:11, Alok Priyadarshi wrote:
> We seem to be following skia style in this file.

Except hasText_ is not Skia style?  At any rate, this whole file's style isn't
really in the scope of this patch to fix, but it's pretty confusing to review.

[commit-bot: I haz the power, 2013-04-08 18:06:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alokp@chromium.org/13726013/7001

[commit-bot: I haz the power, 2013-04-08 18:17:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alokp@chromium.org/13726013/21002

[commit-bot: I haz the power, 2013-04-08 20:10:32]

Retried try job too often on mac_rel for step(s) unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2013-04-08 21:42:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alokp@chromium.org/13726013/21002

[commit-bot: I haz the power, 2013-04-08 22:45:19]

Change committed as 192936