Rename: background_is_transparent -> subpixel_rendering_enabled

Rename: background_is_transparent -> subpixel_rendering_enabled

This is derived from crrev.com/867003002.
RenderText::background_is_transparent() field does not directly
mean the background color is transparent. This is a flag to
control the subpixel rendering (see canvas_skia.cc) and can
be set for various reasons.

Considering this situation, it is clearer to rename the field
to subpixel_rendering_enabled().

R=msw@chromium.org
TEST=no functional changes, build succeeds
BUG=None

Committed: https://crrev.com/4b75bd7856f4838e43bbda59badd9c2031da2073
Cr-Commit-Position: refs/heads/master@{#317139}

[msw, 2015-02-19 00:56:25]

lgtm with nits

https://codereview.chromium.org/941523002/diff/1/ui/gfx/canvas_skia.cc
File ui/gfx/canvas_skia.cc (right):

https://codereview.chromium.org/941523002/diff/1/ui/gfx/canvas_skia.cc#newcod...
ui/gfx/canvas_skia.cc:146: render_text->set_subpixel_rendering_enabled(false);
nit: maybe this should be set unconditionally; I think it'd be:
  render_text->set_subpixel_rendering_enabled(
      (flags & Canvas::NO_SUBPIXEL_RENDERING) == 0);

https://codereview.chromium.org/941523002/diff/1/ui/gfx/render_text.h
File ui/gfx/render_text.h (right):

https://codereview.chromium.org/941523002/diff/1/ui/gfx/render_text.h#newcode737
ui/gfx/render_text.h:737: // Set to false to suppress subpixel rendering due to
non-font reasons (such
nit: "(eg. if the background is transparent)"

[Jun Mukai, 2015-02-19 02:04:52]

New patchsets have been uploaded after l-g-t-m from msw@chromium.org

[Jun Mukai, 2015-02-19 02:07:49]

https://codereview.chromium.org/941523002/diff/1/ui/gfx/canvas_skia.cc
File ui/gfx/canvas_skia.cc (right):

https://codereview.chromium.org/941523002/diff/1/ui/gfx/canvas_skia.cc#newcod...
ui/gfx/canvas_skia.cc:146: render_text->set_subpixel_rendering_enabled(false);
On 2015/02/19 00:56:24, msw wrote:
> nit: maybe this should be set unconditionally; I think it'd be:
>   render_text->set_subpixel_rendering_enabled(
>       (flags & Canvas::NO_SUBPIXEL_RENDERING) == 0);

Done.

https://codereview.chromium.org/941523002/diff/1/ui/gfx/render_text.h
File ui/gfx/render_text.h (right):

https://codereview.chromium.org/941523002/diff/1/ui/gfx/render_text.h#newcode737
ui/gfx/render_text.h:737: // Set to false to suppress subpixel rendering due to
non-font reasons (such
On 2015/02/19 00:56:25, msw wrote:
> nit: "(eg. if the background is transparent)"

Done.

[Jun Mukai, 2015-02-19 02:07:59]

mukai@chromium.org changed reviewers:
+ sky@chromium.org

[Jun Mukai, 2015-02-19 02:08:26]

sky, could you review this?  (especially for chrome/browser/ui/views and
ui/views)

[Daniel Erat, 2015-02-19 13:49:31]

derat@chromium.org changed reviewers:
+ derat@chromium.org

[Daniel Erat, 2015-02-19 13:49:31]

the new name is inaccurate. the actual meaning of this field is more like
"subpixel_rendering_not_forced_off". subpixel rendering still may or may not be
used depending on what's requested in FontRenderParams. what about renaming the
old field to something like "force_subpixel_rendering_off"?

[sky, 2015-02-19 17:51:04]

LGTM

https://codereview.chromium.org/941523002/diff/40001/ui/views/controls/textfi...
File ui/views/controls/textfield/textfield.cc (right):

https://codereview.chromium.org/941523002/diff/40001/ui/views/controls/textfi...
ui/views/controls/textfield/textfield.cc:1651: // Disable subpixel rendering
when the background color is transparent.
This comment documents what the code does. Useful comments tell you why the code
is like this. Why is it important that subpixel rendering is disabled when
transparent?

[Jun Mukai, 2015-02-19 18:24:52]

On 2015/02/19 13:49:31, Daniel Erat wrote:
> the new name is inaccurate. the actual meaning of this field is more like
> "subpixel_rendering_not_forced_off". subpixel rendering still may or may not
be
> used depending on what's requested in FontRenderParams. what about renaming
the
> old field to something like "force_subpixel_rendering_off"?

Agree, but saying 'not-' or 'force-off' looks a bit confusing to me for some
reasons.
what about 'suppress_subpixel_rendering'? (or 'prevent' or 'stop'?)

[Daniel Erat, 2015-02-19 18:39:41]

On 2015/02/19 18:24:52, Jun Mukai wrote:
> On 2015/02/19 13:49:31, Daniel Erat wrote:
> > the new name is inaccurate. the actual meaning of this field is more like
> > "subpixel_rendering_not_forced_off". subpixel rendering still may or may not
> be
> > used depending on what's requested in FontRenderParams. what about renaming
> the
> > old field to something like "force_subpixel_rendering_off"?
> 
> Agree, but saying 'not-' or 'force-off' looks a bit confusing to me for some
> reasons.
> what about 'suppress_subpixel_rendering'? (or 'prevent' or 'stop'?)

sure, "suppress" and "prevent" both sound good to me!

[Jun Mukai, 2015-02-19 18:52:27]

New patchsets have been uploaded after l-g-t-m from sky@chromium.org

[Jun Mukai, 2015-02-19 18:54:34]

https://codereview.chromium.org/941523002/diff/40001/ui/views/controls/textfi...
File ui/views/controls/textfield/textfield.cc (right):

https://codereview.chromium.org/941523002/diff/40001/ui/views/controls/textfi...
ui/views/controls/textfield/textfield.cc:1651: // Disable subpixel rendering
when the background color is transparent.
On 2015/02/19 17:51:03, sky wrote:
> This comment documents what the code does. Useful comments tell you why the
code
> is like this. Why is it important that subpixel rendering is disabled when
> transparent?

Done, also put the link to the reported bug.

[Jun Mukai, 2015-02-19 18:54:41]

The CQ bit was checked by mukai@chromium.org

[commit-bot: I haz the power, 2015-02-19 18:55:45]

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

[commit-bot: I haz the power, 2015-02-19 19:43:29]

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

[commit-bot: I haz the power, 2015-02-19 19:43:32]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[msw, 2015-02-19 19:59:28]

new naming lgtm

[Jun Mukai, 2015-02-19 20:32:39]

New patchsets have been uploaded after l-g-t-m from msw@chromium.org

[Jun Mukai, 2015-02-19 20:32:48]

The CQ bit was checked by mukai@chromium.org

[commit-bot: I haz the power, 2015-02-19 20:33:35]

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

[commit-bot: I haz the power, 2015-02-19 21:44:56]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-02-19 21:45:30]

Patchset 5 (id:??) landed as
https://crrev.com/4b75bd7856f4838e43bbda59badd9c2031da2073
Cr-Commit-Position: refs/heads/master@{#317139}