linux: Use subpixel positioning for forced scale factor.

linux: Use subpixel positioning for forced scale factor.

Chrome OS currently tells Blink to use subpixel-positioned
text when running on images built with the highdpi USE flag.
Additionally use subpixel positioning when
--force-device-scale-factor is used to force high-DPI mode.

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

[oshima, 2014-04-09 20:11:12]

lgtm

[oshima, 2014-04-09 20:11:46]

one optional nit.

https://codereview.chromium.org/231583002/diff/1/ui/gfx/font_render_params_li...
File ui/gfx/font_render_params_linux.cc (right):

https://codereview.chromium.org/231583002/diff/1/ui/gfx/font_render_params_li...
ui/gfx/font_render_params_linux.cc:32: } else {
optional nit: you can omit else.

[Daniel Erat, 2014-04-09 20:14:41]

https://codereview.chromium.org/231583002/diff/1/ui/gfx/font_render_params_li...
File ui/gfx/font_render_params_linux.cc (right):

https://codereview.chromium.org/231583002/diff/1/ui/gfx/font_render_params_li...
ui/gfx/font_render_params_linux.cc:32: } else {
On 2014/04/09 20:11:46, oshima wrote:
> optional nit: you can omit else.

Done.

[Daniel Erat, 2014-04-09 20:15:17]

apply review feedback

[oshima, 2014-04-09 20:18:55]

On 2014/04/09 20:15:17, Daniel Erat wrote:
> apply review feedback

slgtm

[Alexei Svitkine (slow), 2014-04-09 20:43:23]

LGTM with nits

https://codereview.chromium.org/231583002/diff/20001/ui/gfx/font_render_param...
File ui/gfx/font_render_params_linux.cc (right):

https://codereview.chromium.org/231583002/diff/20001/ui/gfx/font_render_param...
ui/gfx/font_render_params_linux.cc:28: return
CommandLine::ForCurrentProcess()->HasSwitch(
Nit: Extract CommandLine::ForCurrentProcess() into a local var to make this part
more concise. It can be above the if since it's also used below.

https://codereview.chromium.org/231583002/diff/20001/ui/gfx/font_render_param...
ui/gfx/font_render_params_linux.cc:30: (Display::HasForceDeviceScaleFactor() &&
Nit: Could you add a comment explaining the motivation behind this logic here?

[Daniel Erat, 2014-04-09 21:03:10]

https://codereview.chromium.org/231583002/diff/20001/ui/gfx/font_render_param...
File ui/gfx/font_render_params_linux.cc (right):

https://codereview.chromium.org/231583002/diff/20001/ui/gfx/font_render_param...
ui/gfx/font_render_params_linux.cc:28: return
CommandLine::ForCurrentProcess()->HasSwitch(
On 2014/04/09 20:43:23, Alexei Svitkine wrote:
> Nit: Extract CommandLine::ForCurrentProcess() into a local var to make this
part
> more concise. It can be above the if since it's also used below.

Done.

https://codereview.chromium.org/231583002/diff/20001/ui/gfx/font_render_param...
ui/gfx/font_render_params_linux.cc:30: (Display::HasForceDeviceScaleFactor() &&
On 2014/04/09 20:43:23, Alexei Svitkine wrote:
> Nit: Could you add a comment explaining the motivation behind this logic here?

Done.

[Daniel Erat, 2014-04-09 21:03:20]

more review comments

[Daniel Erat, 2014-04-10 16:11:05]

The CQ bit was checked by derat@chromium.org

[commit-bot: I haz the power, 2014-04-10 16:11:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/derat@chromium.org/231583002/40001

[commit-bot: I haz the power, 2014-04-10 18:24:51]

Change committed as 263031