fix for cached textblobs look garbled

fix for cached textblobs look garbled

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/e4cee1f283c435618343f0c7b298207d5a9a3e1c

[joshualitt, 2015-05-11 18:27:11]

joshualitt@google.com changed reviewers:
+ bsalomon@google.com, joshualitt@google.com, robertphillips@google.com

[joshualitt, 2015-05-11 18:27:12]

ptal

[joshualitt, 2015-05-11 18:29:40]

joshualitt@google.com changed reviewers:
+ jvanverth@google.com

[bsalomon, 2015-05-11 18:33:13]

https://codereview.chromium.org/1135813007/diff/20001/gm/textblobgeometrychan...
File gm/textblobgeometrychange.cpp (right):

https://codereview.chromium.org/1135813007/diff/20001/gm/textblobgeometrychan...
gm/textblobgeometrychange.cpp:38: canvas->clear(0x00ffffff);
Do we really want the bad looking LCD text on transparent? Also, transparent GMs
cause headaches for baselining since the background of the baselining UI is
composited in.

https://codereview.chromium.org/1135813007/diff/20001/gm/textblobgeometrychan...
gm/textblobgeometrychange.cpp:52: const SkTextBlobBuilder::RunBuffer& run =
builder.allocRun(paint, glyphs.count(), 10, 10, NULL);
wrap?

https://codereview.chromium.org/1135813007/diff/20001/src/gpu/GrAtlasTextCont...
File src/gpu/GrAtlasTextContext.cpp (right):

https://codereview.chromium.org/1135813007/diff/20001/src/gpu/GrAtlasTextCont...
src/gpu/GrAtlasTextContext.cpp:371: SkPixelGeometry pixelGeometry = hasLCD ?
fDeviceProperties.pixelGeometry() :
Shouldn't we also check for df here?

[robertphillips, 2015-05-11 18:34:41]

https://codereview.chromium.org/1135813007/diff/20001/gm/textblobgeometrychan...
File gm/textblobgeometrychange.cpp (right):

https://codereview.chromium.org/1135813007/diff/20001/gm/textblobgeometrychan...
gm/textblobgeometrychange.cpp:1: /*
timewarp

https://codereview.chromium.org/1135813007/diff/20001/gm/textblobgeometrychan...
gm/textblobgeometrychange.cpp:11: #include "SkCanvas.h"
Do we need gradient shader ?

https://codereview.chromium.org/1135813007/diff/20001/gm/textblobgeometrychan...
gm/textblobgeometrychange.cpp:18: namespace skiagm {
// informative comment ?
// link to Chrome bug ?

https://codereview.chromium.org/1135813007/diff/20001/gm/textblobgeometrychan...
gm/textblobgeometrychange.cpp:23: protected:
rm this ?

https://codereview.chromium.org/1135813007/diff/20001/gm/textblobgeometrychan...
gm/textblobgeometrychange.cpp:39: 
Maybe a build_lcd_blob helper ?

https://codereview.chromium.org/1135813007/diff/20001/gm/textblobgeometrychan...
gm/textblobgeometrychange.cpp:51:
paint.setTextEncoding(SkPaint::kGlyphID_TextEncoding);
overlength

[jvanverth1, 2015-05-11 18:40:04]

Just checked, this does fix
https://code.google.com/p/chromium/issues/detail?id=486242.

[joshualitt, 2015-05-11 19:07:47]

feedback inc

https://codereview.chromium.org/1135813007/diff/20001/gm/textblobgeometrychan...
File gm/textblobgeometrychange.cpp (right):

https://codereview.chromium.org/1135813007/diff/20001/gm/textblobgeometrychan...
gm/textblobgeometrychange.cpp:1: /*
On 2015/05/11 18:34:41, robertphillips wrote:
> timewarp

Acknowledged.

https://codereview.chromium.org/1135813007/diff/20001/gm/textblobgeometrychan...
gm/textblobgeometrychange.cpp:11: #include "SkCanvas.h"
On 2015/05/11 18:34:41, robertphillips wrote:
> Do we need gradient shader ?

Acknowledged.

https://codereview.chromium.org/1135813007/diff/20001/gm/textblobgeometrychan...
gm/textblobgeometrychange.cpp:18: namespace skiagm {
On 2015/05/11 18:34:41, robertphillips wrote:
> // informative comment ?
> // link to Chrome bug ?

Acknowledged.

https://codereview.chromium.org/1135813007/diff/20001/gm/textblobgeometrychan...
gm/textblobgeometrychange.cpp:23: protected:
On 2015/05/11 18:34:41, robertphillips wrote:
> rm this ?

Acknowledged.

https://codereview.chromium.org/1135813007/diff/20001/gm/textblobgeometrychan...
gm/textblobgeometrychange.cpp:38: canvas->clear(0x00ffffff);
On 2015/05/11 18:33:13, bsalomon wrote:
> Do we really want the bad looking LCD text on transparent? Also, transparent
GMs
> cause headaches for baselining since the background of the baselining UI is
> composited in.

Acknowledged.

https://codereview.chromium.org/1135813007/diff/20001/gm/textblobgeometrychan...
gm/textblobgeometrychange.cpp:39: 
On 2015/05/11 18:34:41, robertphillips wrote:
> Maybe a build_lcd_blob helper ?

Acknowledged.

https://codereview.chromium.org/1135813007/diff/20001/gm/textblobgeometrychan...
gm/textblobgeometrychange.cpp:51:
paint.setTextEncoding(SkPaint::kGlyphID_TextEncoding);
On 2015/05/11 18:34:41, robertphillips wrote:
> overlength 

Acknowledged.

https://codereview.chromium.org/1135813007/diff/20001/gm/textblobgeometrychan...
gm/textblobgeometrychange.cpp:52: const SkTextBlobBuilder::RunBuffer& run =
builder.allocRun(paint, glyphs.count(), 10, 10, NULL);
On 2015/05/11 18:33:13, bsalomon wrote:
> wrap?

Acknowledged.

[robertphillips, 2015-05-11 19:15:26]

lgtm + nits

https://codereview.chromium.org/1135813007/diff/40001/gm/textblobgeometrychan...
File gm/textblobgeometrychange.cpp (right):

https://codereview.chromium.org/1135813007/diff/40001/gm/textblobgeometrychan...
gm/textblobgeometrychange.cpp:3: *
BD style ?

https://codereview.chromium.org/1135813007/diff/40001/gm/textblobgeometrychan...
gm/textblobgeometrychange.cpp:9: 
Don't think we need Resources.h either.

https://codereview.chromium.org/1135813007/diff/40001/gm/textblobgeometrychan...
gm/textblobgeometrychange.cpp:11: #include "SkCanvas.h"
SkStream.h ?

https://codereview.chromium.org/1135813007/diff/40001/gm/textblobgeometrychan...
gm/textblobgeometrychange.cpp:61: // This should not look garbled
since we should disable LCD text in this case (i.e., unknown pixel geometry) ?

[joshualitt, 2015-05-11 19:19:57]

The CQ bit was checked by joshualitt@google.com

[joshualitt, 2015-05-11 19:19:58]

The patchset sent to the CQ was uploaded after l-g-t-m from
robertphillips@google.com
Link to the patchset: https://codereview.chromium.org/1135813007/#ps60001
(title: "more")

[commit-bot: I haz the power, 2015-05-11 19:20:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1135813007/60001

[commit-bot: I haz the power, 2015-05-11 19:24:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-05-11 19:24:26]

Try jobs failed on following builders:
  Build-Win-MSVC-x86-Debug-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Win-MSVC-x86-D...)

[joshualitt, 2015-05-11 19:58:07]

The CQ bit was checked by joshualitt@google.com

[joshualitt, 2015-05-11 19:58:07]

The patchset sent to the CQ was uploaded after l-g-t-m from
robertphillips@google.com
Link to the patchset: https://codereview.chromium.org/1135813007/#ps80001
(title: "more")

[commit-bot: I haz the power, 2015-05-11 19:58:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1135813007/80001

[commit-bot: I haz the power, 2015-05-11 20:04:39]

Committed patchset #5 (id:80001) as
https://skia.googlesource.com/skia/+/e4cee1f283c435618343f0c7b298207d5a9a3e1c