try adding flag to allow lcd text even in a layer

try adding flag to allow lcd text even in a layer

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/70ee31b2fa127eee6c0cea61cf05508e9d3ca7b1

[reed1, 2015-12-09 19:54:07]

reed@google.com changed reviewers:
+ bungeman@google.com

[reed1, 2015-12-09 19:54:08]

ptal

[reed1, 2015-12-09 19:54:44]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-12-09 19:54:53]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1513513002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1513513002/20001

[reed1, 2015-12-09 19:56:55]

reed@google.com changed reviewers:
+ pkasting@google.com

[reed1, 2015-12-09 19:56:55]

peter, does this api seem to meet your need?

[commit-bot: I haz the power, 2015-12-09 20:07:35]

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

[commit-bot: I haz the power, 2015-12-09 20:07:35]

Dry run: This issue passed the CQ dry run.

[Peter Kasting, 2015-12-10 01:45:39]

This seems to work.  Here's a CL making use of this, if it helps inform how you
name/structure the API:

https://codereview.chromium.org/1506203008

[reed1, 2015-12-10 14:13:59]

On 2015/12/10 01:45:39, Peter Kasting wrote:
> This seems to work.  Here's a CL making use of this, if it helps inform how
you
> name/structure the API:
> 
> https://codereview.chromium.org/1506203008

I know the api is a bit one-off-ish, but I don't expect this to be the long-term
name. Likely the "real" version will take flags for this, and perhaps other
attributes (opacity, etc.). For now I just wanted something clear and
conservative to unblock you.

[bungeman-skia, 2015-12-10 14:52:37]

https://codereview.chromium.org/1513513002/diff/20001/src/core/SkDevice.cpp
File src/core/SkDevice.cpp (right):

https://codereview.chromium.org/1513513002/diff/20001/src/core/SkDevice.cpp#n...
src/core/SkDevice.cpp:67: if (!allowLCDText) {
This CL does seem to expose one feature which we've been hiding (or trying to
hide) -- when Skia draws antialiased like subpixel anti-aliased if it can't
actually do subpixel anti-aliased due to geometry constraints.

As an example, the user saves the layer in such a way that allowLCDText is true,
stating that they will be responsible for properly setting the paint flags (just
always respect them). The user just drew opaque into a region, so they
paint.setLCDRenderText(true) and draw and get what they expected. The issue I'm
concerned about is when the user wants to draw text in a non-opaque region of
the layer they'll need to set paint.setLCDRenderText(false), but to get the same
result as they would have had (without saving the layer this way), they'll also
need to set the kGenA8FromLCD_Flag on the paint flags. Part of the reason for
the geometry was to get rid of kGenA8FromLCD_Flag, so it would be nice to set
this up so we don't need it. The issue here is just that subpixel anti-aliased
text is often just drawn from different outlines by the system due to
differences in hinting or auto-hinting. [As a result maybe that setting should
be on the SkTypeface instead?]

[reed1, 2015-12-10 19:52:03]

ptal

[reed1, 2015-12-10 19:52:11]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-12-10 19:52:17]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1513513002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1513513002/40001

[commit-bot: I haz the power, 2015-12-10 20:06:03]

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

[commit-bot: I haz the power, 2015-12-10 20:06:04]

Dry run: This issue passed the CQ dry run.

[bungeman-skia, 2015-12-10 20:35:10]

Let's normalize all the names to 'preserveLCDText'. After that lgtm or pwnslt
(persuaded we need something like this).

https://codereview.chromium.org/1513513002/diff/40001/include/core/SkDevice.h
File include/core/SkDevice.h (right):

https://codereview.chromium.org/1513513002/diff/40001/include/core/SkDevice.h...
include/core/SkDevice.h:337: bool allowLCDText);
Rename to preserveLCDText

https://codereview.chromium.org/1513513002/diff/40001/include/core/SkDevice.h...
include/core/SkDevice.h:351: bool allowLCDText,
preserveLCDText

https://codereview.chromium.org/1513513002/diff/40001/src/core/SkCanvas.cpp
File src/core/SkCanvas.cpp (right):

https://codereview.chromium.org/1513513002/diff/40001/src/core/SkCanvas.cpp#n...
src/core/SkCanvas.cpp:1144: unsigned flags = kARGB_ClipLayer_SaveFlag |
kAllowLDCText_PrivateSaveFlag;
kPreserveLDCText_PrivateSaveFlag

https://codereview.chromium.org/1513513002/diff/40001/src/core/SkCanvas.cpp#n...
src/core/SkCanvas.cpp:1193: SkToBool(flags & kAllowLDCText_PrivateSaveFlag);
preserveLCDText , kPreserveLDCText_PrivateSaveFlag

https://codereview.chromium.org/1513513002/diff/40001/src/core/SkDevice.cpp
File src/core/SkDevice.cpp (right):

https://codereview.chromium.org/1513513002/diff/40001/src/core/SkDevice.cpp#n...
src/core/SkDevice.cpp:59: bool allowLCDText) {
preserveLCDText

https://codereview.chromium.org/1513513002/diff/40001/src/core/SkImageFilter.cpp
File src/core/SkImageFilter.cpp (right):

https://codereview.chromium.org/1513513002/diff/40001/src/core/SkImageFilter....
src/core/SkImageFilter.cpp:661: false,   /* allowLCDText */
preserveLCDText

[reed1, 2015-12-10 21:32:23]

https://codereview.chromium.org/1513513002/diff/40001/include/core/SkDevice.h
File include/core/SkDevice.h (right):

https://codereview.chromium.org/1513513002/diff/40001/include/core/SkDevice.h...
include/core/SkDevice.h:337: bool allowLCDText);
On 2015/12/10 20:35:10, bungeman1 wrote:
> Rename to preserveLCDText

Done.

https://codereview.chromium.org/1513513002/diff/40001/include/core/SkDevice.h...
include/core/SkDevice.h:351: bool allowLCDText,
On 2015/12/10 20:35:10, bungeman1 wrote:
> preserveLCDText

Done.

https://codereview.chromium.org/1513513002/diff/40001/src/core/SkCanvas.cpp
File src/core/SkCanvas.cpp (right):

https://codereview.chromium.org/1513513002/diff/40001/src/core/SkCanvas.cpp#n...
src/core/SkCanvas.cpp:1144: unsigned flags = kARGB_ClipLayer_SaveFlag |
kAllowLDCText_PrivateSaveFlag;
On 2015/12/10 20:35:10, bungeman1 wrote:
> kPreserveLDCText_PrivateSaveFlag

Done.

https://codereview.chromium.org/1513513002/diff/40001/src/core/SkCanvas.cpp#n...
src/core/SkCanvas.cpp:1193: SkToBool(flags & kAllowLDCText_PrivateSaveFlag);
On 2015/12/10 20:35:10, bungeman1 wrote:
> preserveLCDText , kPreserveLDCText_PrivateSaveFlag

Done.

https://codereview.chromium.org/1513513002/diff/40001/src/core/SkDevice.cpp
File src/core/SkDevice.cpp (right):

https://codereview.chromium.org/1513513002/diff/40001/src/core/SkDevice.cpp#n...
src/core/SkDevice.cpp:59: bool allowLCDText) {
On 2015/12/10 20:35:10, bungeman1 wrote:
> preserveLCDText

Done.

https://codereview.chromium.org/1513513002/diff/40001/src/core/SkImageFilter.cpp
File src/core/SkImageFilter.cpp (right):

https://codereview.chromium.org/1513513002/diff/40001/src/core/SkImageFilter....
src/core/SkImageFilter.cpp:661: false,   /* allowLCDText */
On 2015/12/10 20:35:10, bungeman1 wrote:
> preserveLCDText

Done.

[reed1, 2015-12-10 21:32:30]

The CQ bit was checked by reed@google.com

[reed1, 2015-12-10 21:32:30]

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

[commit-bot: I haz the power, 2015-12-10 21:32:40]

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

[commit-bot: I haz the power, 2015-12-10 21:44:47]

Description was changed from

==========
try adding flag to allow lcd text even in a layer

BUG=skia:
==========

to

==========
try adding flag to allow lcd text even in a layer

BUG=skia:

Committed:
https://skia.googlesource.com/skia/+/70ee31b2fa127eee6c0cea61cf05508e9d3ca7b1
==========

[commit-bot: I haz the power, 2015-12-10 21:44:48]

Committed patchset #4 (id:60001) as
https://skia.googlesource.com/skia/+/70ee31b2fa127eee6c0cea61cf05508e9d3ca7b1