[linux] Allow font to use below 1.0f scale

Allow font to use below 1.0f scale

* If the raw device scale factor is lower than 1.0, shrink the font size further.

BUG=646254
TEST=manual

Committed: https://crrev.com/a98afff3156eb88b39f82cfae23d99bed436bb3c
Cr-Commit-Position: refs/heads/master@{#430311}

[oshima, 2016-10-21 17:05:51]

The CQ bit was checked by oshima@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-21 17:06:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-21 21:16:22]

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

[commit-bot: I haz the power, 2016-10-21 21:16:23]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[oshima, 2016-10-24 15:53:47]

The CQ bit was checked by oshima@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-24 15:54:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[oshima, 2016-10-24 16:02:44]

Patchset #2 (id:20001) has been deleted

[oshima, 2016-10-24 16:02:48]

Patchset #2 (id:40001) has been deleted

[oshima, 2016-10-24 16:02:57]

Patchset #1 (id:1) has been deleted

[oshima, 2016-10-24 16:03:03]

Patchset #1 (id:60001) has been deleted

[oshima, 2016-10-24 16:03:07]

The CQ bit was checked by oshima@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-24 16:03:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-24 16:46:02]

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

[commit-bot: I haz the power, 2016-10-24 16:46:03]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[oshima, 2016-10-24 20:23:32]

The CQ bit was checked by oshima@chromium.org to run a CQ dry run

[oshima, 2016-10-24 20:23:36]

Patchset #1 (id:80001) has been deleted

[commit-bot: I haz the power, 2016-10-24 20:24:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[oshima, 2016-10-25 00:46:13]

Patchset #2 (id:120001) has been deleted

[oshima, 2016-10-25 00:46:17]

Patchset #1 (id:100001) has been deleted

[oshima, 2016-10-25 00:46:23]

The CQ bit was checked by oshima@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-25 00:46:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-25 01:35:56]

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

[commit-bot: I haz the power, 2016-10-25 01:35:57]

Dry run: Try jobs failed on following builders:
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[oshima, 2016-10-25 15:09:57]

Patchset #1 (id:140001) has been deleted

[oshima, 2016-10-25 15:11:57]

Description was changed from

==========
Allow font to use below 1.0f scale

BUG=646254
==========

to

==========
Allow font to use below 1.0f scale

* add a flag --enable-xpi-for-dsf for users who want to control scale factor by
X's DPI.
* If the raw device scale factor is lower than 1.0, shrink the font size
further.

BUG=646254
TEST=manual
==========

[oshima, 2016-10-25 15:11:57]

oshima@chromium.org changed reviewers:
+ estade@chromium.org

[oshima, 2016-10-25 15:12:18]

The CQ bit was checked by oshima@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-25 15:12:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Evan Stade, 2016-10-25 15:21:14]

I'm a little wary of adding this to about:flags as that's not really supposed to
be a place for permanent options that users might want to toggle. Is it not
possible to keep using GTK dpi settings, disallow <1 dsf, but allow the dpi to
affect base font size?

https://codereview.chromium.org/2441043002/diff/160001/chrome/browser/ui/libg...
File chrome/browser/ui/libgtk2ui/gtk2_ui.cc (right):

https://codereview.chromium.org/2441043002/diff/160001/chrome/browser/ui/libg...
chrome/browser/ui/libgtk2ui/gtk2_ui.cc:388: // The |device_scale_factor| is used
to canel the scale to be applyed by UI
spelling nits: cancel, applied

https://codereview.chromium.org/2441043002/diff/160001/chrome/common/chrome_s...
File chrome/common/chrome_switches.h (right):

https://codereview.chromium.org/2441043002/diff/160001/chrome/common/chrome_s...
chrome/common/chrome_switches.h:395: extern const char kEnableXDPIForDSF[];
nit: kEnableXDpiForDsf

[commit-bot: I haz the power, 2016-10-25 15:59:01]

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

[commit-bot: I haz the power, 2016-10-25 15:59:02]

Dry run: Try jobs failed on following builders:
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[oshima, 2016-10-25 16:36:13]

> I'm a little wary of adding this to about:flags as that's not really supposed
to
> be a place for permanent options that users might want to toggle.

The command line isn't for permanent thing either. We're just doing this for
very minor use case,
so I'm fine with either way. Let me know if you want to remove it.

> Is it not
> possible to keep using GTK dpi settings, disallow <1 dsf, but allow the dpi to
> affect base font size?

Most people uses a configuration tool to adjust the UI scale, which uses GTK's
settings.
If we use X'dpi to change the font, people who uses these config tool will see
tiny text
because X's dpi stays the same.

We've also seen that some people have weird DPI values (not 96), so we want to
enable this
only for people who explicitly enabled.

https://codereview.chromium.org/2441043002/diff/160001/chrome/browser/ui/libg...
File chrome/browser/ui/libgtk2ui/gtk2_ui.cc (right):

https://codereview.chromium.org/2441043002/diff/160001/chrome/browser/ui/libg...
chrome/browser/ui/libgtk2ui/gtk2_ui.cc:388: // The |device_scale_factor| is used
to canel the scale to be applyed by UI
On 2016/10/25 15:21:14, Evan Stade wrote:
> spelling nits: cancel, applied

Done.

https://codereview.chromium.org/2441043002/diff/160001/chrome/common/chrome_s...
File chrome/common/chrome_switches.h (right):

https://codereview.chromium.org/2441043002/diff/160001/chrome/common/chrome_s...
chrome/common/chrome_switches.h:395: extern const char kEnableXDPIForDSF[];
On 2016/10/25 15:21:14, Evan Stade wrote:
> nit: kEnableXDpiForDsf

Done.

[Evan Stade, 2016-10-25 17:27:02]

I guess my point is that of the two bullet points in your CL description, it
seems like the latter should be sufficient for allowing users to get smaller
fonts but still having a minimum DSF of 1x. I don't think we need to support a
different way of setting DPI that's outside of the GTK system (users can change
how they're setting DPI).

https://codereview.chromium.org/2441043002/diff/180001/chrome/browser/ui/libg...
File chrome/browser/ui/libgtk2ui/gtk2_ui.cc (right):

https://codereview.chromium.org/2441043002/diff/180001/chrome/browser/ui/libg...
chrome/browser/ui/libgtk2ui/gtk2_ui.cc:360: double GetDPI() {
nit: GetDpi

[oshima, 2016-10-25 18:40:59]

On 2016/10/25 17:27:02, Evan Stade wrote:
> I guess my point is that of the two bullet points in your CL description, it
> seems like the latter should be sufficient for allowing users to get smaller
> fonts but still having a minimum DSF of 1x. I don't think we need to support a
> different way of setting DPI that's outside of the GTK system (users can
change
> how they're setting DPI).

If you set the GTK's scale lower, it will make entire UI smaller.
Let me double check with the reporter if that's okay with him.

> 
>
https://codereview.chromium.org/2441043002/diff/180001/chrome/browser/ui/libg...
> File chrome/browser/ui/libgtk2ui/gtk2_ui.cc (right):
> 
>
https://codereview.chromium.org/2441043002/diff/180001/chrome/browser/ui/libg...
> chrome/browser/ui/libgtk2ui/gtk2_ui.cc:360: double GetDPI() {
> nit: GetDpi

[oshima, 2016-11-02 22:35:56]

Patchset #3 (id:190001) has been deleted

[oshima, 2016-11-02 22:36:00]

The CQ bit was checked by oshima@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-02 22:36:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[oshima, 2016-11-02 22:37:32]

Patchset #4 (id:230001) has been deleted

[oshima, 2016-11-03 00:44:21]

Description was changed from

==========
Allow font to use below 1.0f scale

* add a flag --enable-xpi-for-dsf for users who want to control scale factor by
X's DPI.
* If the raw device scale factor is lower than 1.0, shrink the font size
further.

BUG=646254
TEST=manual
==========

to

==========
Allow font to use below 1.0f scale

* If the raw device scale factor is lower than 1.0, shrink the font size
further.

BUG=646254
TEST=manual
==========

[oshima, 2016-11-03 00:50:35]

The CQ bit was checked by oshima@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-03 00:50:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-03 01:20:35]

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

[commit-bot: I haz the power, 2016-11-03 01:20:36]

Dry run: Try jobs failed on following builders:
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[oshima, 2016-11-03 12:49:38]

Patchset #3 (id:210001) has been deleted

[oshima, 2016-11-03 12:49:41]

The CQ bit was checked by oshima@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-03 12:49:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[oshima, 2016-11-03 12:49:57]

I removed Xdpi and flags. PTAL.

[commit-bot: I haz the power, 2016-11-03 13:58:45]

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

[commit-bot: I haz the power, 2016-11-03 13:58:46]

Dry run: This issue passed the CQ dry run.

[Evan Stade, 2016-11-03 16:38:03]

lgtm

https://codereview.chromium.org/2441043002/diff/250001/chrome/browser/ui/libg...
File chrome/browser/ui/libgtkui/gtk_ui.cc (right):

https://codereview.chromium.org/2441043002/diff/250001/chrome/browser/ui/libg...
chrome/browser/ui/libgtkui/gtk_ui.cc:379: double GetFontSizePixelsInPoint(float
device_scale_factor) {
perhaps this param could be called ui_scale_factor or ui_dsf to be clearer about
its role?

https://codereview.chromium.org/2441043002/diff/250001/chrome/browser/ui/libg...
chrome/browser/ui/libgtkui/gtk_ui.cc:388: // the raw value however, beacuse the
1.0~1.3 is rounded to 1.0.
why not allow users to bump font size slightly? The drawing issues in the 1-1.3
range don't apply to fonts, do they?

also nit: s/beacuse/because

https://codereview.chromium.org/2441043002/diff/250001/chrome/browser/ui/libg...
File chrome/browser/ui/libgtkui/gtk_ui.h (right):

https://codereview.chromium.org/2441043002/diff/250001/chrome/browser/ui/libg...
chrome/browser/ui/libgtkui/gtk_ui.h:203: float device_scale_factor_ = 1.0f;
seems like we should init things in the same place (at least those that are
init'd with const expressions), either all in the ctor init list or all in this
file

I am ambivalent about which one and I dunno what the style guide has to say
about it, if anything.

[oshima, 2016-11-05 15:15:05]

https://codereview.chromium.org/2441043002/diff/250001/chrome/browser/ui/libg...
File chrome/browser/ui/libgtkui/gtk_ui.cc (right):

https://codereview.chromium.org/2441043002/diff/250001/chrome/browser/ui/libg...
chrome/browser/ui/libgtkui/gtk_ui.cc:379: double GetFontSizePixelsInPoint(float
device_scale_factor) {
On 2016/11/03 16:38:03, Evan Stade wrote:
> perhaps this param could be called ui_scale_factor or ui_dsf to be clearer
about
> its role?

changed to ui_device_scale_factor.

https://codereview.chromium.org/2441043002/diff/250001/chrome/browser/ui/libg...
chrome/browser/ui/libgtkui/gtk_ui.cc:388: // the raw value however, beacuse the
1.0~1.3 is rounded to 1.0.
On 2016/11/03 16:38:03, Evan Stade wrote:
> why not allow users to bump font size slightly? The drawing issues in the
1-1.3
> range don't apply to fonts, do they?

I want to keep the current behavior for dsf>1 case, at least for now.

With new scaling scheme in blink, I think we can relax this but I'll
do that in a separate CL.



> 
> also nit: s/beacuse/because
>

https://codereview.chromium.org/2441043002/diff/250001/chrome/browser/ui/libg...
File chrome/browser/ui/libgtkui/gtk_ui.h (right):

https://codereview.chromium.org/2441043002/diff/250001/chrome/browser/ui/libg...
chrome/browser/ui/libgtkui/gtk_ui.h:203: float device_scale_factor_ = 1.0f;
On 2016/11/03 16:38:03, Evan Stade wrote:
> seems like we should init things in the same place (at least those that are
> init'd with const expressions), either all in the ctor init list or all in
this
> file
> 
> I am ambivalent about which one and I dunno what the style guide has to say
> about it, if anything.

I moved default_font_xxx to the header.

[oshima, 2016-11-05 15:15:31]

The CQ bit was checked by oshima@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-05 15:15:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[oshima, 2016-11-05 15:17:36]

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

[oshima, 2016-11-05 15:17:37]

sky -> ui/views, c/b/ui/views

[commit-bot: I haz the power, 2016-11-05 15:59:56]

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

[commit-bot: I haz the power, 2016-11-05 15:59:57]

Dry run: This issue passed the CQ dry run.

[sky, 2016-11-07 16:35:07]

LGTM

[oshima, 2016-11-07 17:06:12]

The CQ bit was checked by oshima@chromium.org

[oshima, 2016-11-07 17:06:13]

The patchset sent to the CQ was uploaded after l-g-t-m from estade@chromium.org
Link to the patchset: https://codereview.chromium.org/2441043002/#ps270001
(title: "Allow font to use below 1.0f scale")

[commit-bot: I haz the power, 2016-11-07 17:06:48]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-07 17:55:36]

Description was changed from

==========
Allow font to use below 1.0f scale

* If the raw device scale factor is lower than 1.0, shrink the font size
further.

BUG=646254
TEST=manual
==========

to

==========
Allow font to use below 1.0f scale

* If the raw device scale factor is lower than 1.0, shrink the font size
further.

BUG=646254
TEST=manual
==========

[commit-bot: I haz the power, 2016-11-07 17:55:38]

Committed patchset #4 (id:270001)

[commit-bot: I haz the power, 2016-11-07 18:14:28]

Description was changed from

==========
Allow font to use below 1.0f scale

* If the raw device scale factor is lower than 1.0, shrink the font size
further.

BUG=646254
TEST=manual
==========

to

==========
Allow font to use below 1.0f scale

* If the raw device scale factor is lower than 1.0, shrink the font size
further.

BUG=646254
TEST=manual

Committed: https://crrev.com/a98afff3156eb88b39f82cfae23d99bed436bb3c
Cr-Commit-Position: refs/heads/master@{#430311}
==========

[commit-bot: I haz the power, 2016-11-07 18:14:30]

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