Commit eeb7bfd6 authored by Dana Fried's avatar Dana Fried Committed by Commit Bot

Unscale text by Windows Text Scaling factor to avoid double-scaling.

Eliminates text rendering artifacts resulting from launching Chrome
browser with Windows Text Zoom enabled.

----------
Background
----------

The new Windows Text Zoom accessibility feature scales small text on
the screen by an amount in addition to normal DPI scaling. In
consultation with MS accessibility engineers, we have decided that
rather than doing complex and extensive scaling math in Chrome, we are
just going to have the browser scale additionally by that factor.
Chrome now scales dynamically with the zoom factor.

When we read default system fonts (such as those used by the
views::Label class - used by tabs, menu items, bookmarks, etc.), Windows
reports the logical size of the font. This does not take DPI into
account, but it *does* take the Windows Text Zoom amount into account
(for backwards-compatibility with apps that don't read it directly).

When the UI renders the text, it scales by the overall browser scale
factor (which includes both DPI and Windows Text Zoom). So effectively,
if the Text Zoom is set before Chrome starts, all system-default text
ends up scaled *twice*.

Correct text size (current behavior only if Text Zoom is changed *after*
launching Chrome):
 Text Scale 100% 150% 200%
DPI 100%     12   18   24
DPI 150%     18   27   36
DPI 200%     24   36   48

Current text size if Text Zoom is enabled prior to launching Chrome:
 Text Scale 100% 150% 200%
DPI 100%     12   27   48
DPI 150%     18   39   72
DPI 200%     24   54   96

-------------
Patch Details
-------------

This change captures the situations in which we ask Windows for a system
font (for menus and captions), and removes the additional scaling done by
the OS, since we are already taking this into account when rendering
text.

We already apply scaling for localization in most of these places for
similar reasons, so there is a precedent.

Chrome still scales appropriately when Text Scale Factor or DPI is set
prior to Chrome launch or changed dynamically while Chrome is running.

----------
Follow-Ups
----------

We call GetNonClientMetrics() in a number of plces, and use LOGFONT*
structs as well. These are Windows implementation details and should
ideally be hidden away from browser code. We should consolidate to a
single cache of system fonts, which will internally ensure that the fonts
are scaled appropriately (preferably in gfx::PlatformFontWin or similar).

Bug: 866513
Change-Id: Ideb248e2f196bb93d46a3f60780c68cf31446097
Reviewed-on: https://chromium-review.googlesource.com/1235313
Commit-Queue: Dana Fried <dfried@chromium.org>
Reviewed-by: default avatarRobert Liao <robliao@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593732}
parent abf6981f
...@@ -82,6 +82,7 @@ ...@@ -82,6 +82,7 @@
#include "ui/base/ui_base_switches.h" #include "ui/base/ui_base_switches.h"
#include "ui/base/win/hidden_window.h" #include "ui/base/win/hidden_window.h"
#include "ui/base/win/message_box_win.h" #include "ui/base/win/message_box_win.h"
#include "ui/display/win/dpi.h"
#include "ui/gfx/platform_font_win.h" #include "ui/gfx/platform_font_win.h"
#include "ui/gfx/switches.h" #include "ui/gfx/switches.h"
#include "ui/strings/grit/app_locale_settings.h" #include "ui/strings/grit/app_locale_settings.h"
...@@ -110,6 +111,7 @@ void DumpHungRendererProcessImpl(const base::Process& renderer) { ...@@ -110,6 +111,7 @@ void DumpHungRendererProcessImpl(const base::Process& renderer) {
// gfx::Font callbacks // gfx::Font callbacks
void AdjustUIFont(LOGFONT* logfont) { void AdjustUIFont(LOGFONT* logfont) {
display::win::AdjustFontForAccessibility(logfont);
l10n_util::AdjustUIFont(logfont); l10n_util::AdjustUIFont(logfont);
} }
......
...@@ -95,6 +95,7 @@ ...@@ -95,6 +95,7 @@
#if defined(OS_WIN) #if defined(OS_WIN)
#include "base/win/win_client_metrics.h" #include "base/win/win_client_metrics.h"
#include "ui/display/win/dpi.h"
#include "ui/display/win/screen_win.h" #include "ui/display/win/screen_win.h"
#include "ui/gfx/geometry/dip_util.h" #include "ui/gfx/geometry/dip_util.h"
#include "ui/gfx/platform_font_win.h" #include "ui/gfx/platform_font_win.h"
...@@ -124,6 +125,18 @@ void GetPlatformSpecificPrefs(RendererPreferences* prefs) { ...@@ -124,6 +125,18 @@ void GetPlatformSpecificPrefs(RendererPreferences* prefs) {
NONCLIENTMETRICS_XP metrics = {0}; NONCLIENTMETRICS_XP metrics = {0};
base::win::GetNonClientMetrics(&metrics); base::win::GetNonClientMetrics(&metrics);
// Render process has the same problem with Windows applying text scaling
// before Chrome DPI + accessibility scaling is applied, resulting in double
// scaling. These fonts are only used for very specific CSS styles, but to
// remain consistent with the rest of chrome we will make sure they aren't
// incorrectly scaled:
display::win::AdjustFontForAccessibility(&metrics.lfCaptionFont);
display::win::AdjustFontForAccessibility(&metrics.lfSmCaptionFont);
display::win::AdjustFontForAccessibility(&metrics.lfMenuFont);
display::win::AdjustFontForAccessibility(&metrics.lfStatusFont);
display::win::AdjustFontForAccessibility(&metrics.lfMessageFont);
// Store the preferred font faces and sizes:
prefs->caption_font_family_name = metrics.lfCaptionFont.lfFaceName; prefs->caption_font_family_name = metrics.lfCaptionFont.lfFaceName;
prefs->caption_font_height = gfx::PlatformFontWin::GetFontSize( prefs->caption_font_height = gfx::PlatformFontWin::GetFontSize(
metrics.lfCaptionFont); metrics.lfCaptionFont);
......
...@@ -4,10 +4,9 @@ ...@@ -4,10 +4,9 @@
#include "ui/display/win/dpi.h" #include "ui/display/win/dpi.h"
#include <windows.h>
#include "base/win/scoped_hdc.h" #include "base/win/scoped_hdc.h"
#include "ui/display/display.h" #include "ui/display/display.h"
#include "ui/display/win/uwp_text_scale_factor.h"
namespace display { namespace display {
namespace win { namespace win {
...@@ -18,6 +17,19 @@ const float kDefaultDPI = 96.f; ...@@ -18,6 +17,19 @@ const float kDefaultDPI = 96.f;
float g_device_scale_factor = 0.f; float g_device_scale_factor = 0.f;
LONG AdjustForScaleFactor(LONG value, float scale) {
// This should never happen, but make sure we never divide by zero.
DCHECK_GT(scale, 0.0f);
if (value != 0) {
LONG new_value = LONG{std::roundf(value / scale)};
if (new_value == 0) {
new_value = value > 0 ? 1 : -1;
}
value = new_value;
}
return value;
}
} // namespace } // namespace
void SetDefaultDeviceScaleFactor(float scale) { void SetDefaultDeviceScaleFactor(float scale) {
...@@ -35,6 +47,15 @@ int GetDPIFromScalingFactor(float device_scaling_factor) { ...@@ -35,6 +47,15 @@ int GetDPIFromScalingFactor(float device_scaling_factor) {
return kDefaultDPI * device_scaling_factor; return kDefaultDPI * device_scaling_factor;
} }
void AdjustFontForAccessibility(LOGFONT* font) {
const float accessibility_scale_factor =
UwpTextScaleFactor::Instance()->GetTextScaleFactor();
font->lfHeight =
AdjustForScaleFactor(font->lfHeight, accessibility_scale_factor);
font->lfWidth =
AdjustForScaleFactor(font->lfWidth, accessibility_scale_factor);
}
namespace internal { namespace internal {
int GetDefaultSystemDPI() { int GetDefaultSystemDPI() {
......
...@@ -5,6 +5,8 @@ ...@@ -5,6 +5,8 @@
#ifndef UI_DISPLAY_WIN_DPI_H_ #ifndef UI_DISPLAY_WIN_DPI_H_
#define UI_DISPLAY_WIN_DPI_H_ #define UI_DISPLAY_WIN_DPI_H_
#include <windows.h>
#include "ui/display/display_export.h" #include "ui/display/display_export.h"
namespace display { namespace display {
...@@ -27,6 +29,16 @@ DISPLAY_EXPORT float GetDPIScale(); ...@@ -27,6 +29,16 @@ DISPLAY_EXPORT float GetDPIScale();
// Returns the equivalent DPI for |device_scaling_factor|. // Returns the equivalent DPI for |device_scaling_factor|.
DISPLAY_EXPORT int GetDPIFromScalingFactor(float device_scaling_factor); DISPLAY_EXPORT int GetDPIFromScalingFactor(float device_scaling_factor);
// Adjusts |font|'s height for accessibility measures already built into the
// font in order to prevent applying the same factor twice.
//
// Windows will add text scaling factor into the logical size of its default
// system fonts (which it does *not* do for DPI scaling). Since we're scaling
// the entire UI by a combination of text scale and DPI scale, this results in
// double scaling. Call this function to unscale the font before using it in
// any of our rendering code.
DISPLAY_EXPORT void AdjustFontForAccessibility(LOGFONT* font);
namespace internal { namespace internal {
// Note: These methods do not take accessibility adjustments into account. // Note: These methods do not take accessibility adjustments into account.
......
...@@ -158,6 +158,10 @@ class UwpTextScaleFactorImpl : public UwpTextScaleFactor { ...@@ -158,6 +158,10 @@ class UwpTextScaleFactorImpl : public UwpTextScaleFactor {
} }
} }
// Windows documents this property to always have a value greater than or
// equal to 1. Let's make sure that's the case - if we don't, we could get
// bizarre behavior and divide-by-zeros later on.
DCHECK_GE(result, 1.0);
return float{result}; return float{result};
} }
......
...@@ -37,6 +37,8 @@ class DISPLAY_EXPORT UwpTextScaleFactor { ...@@ -37,6 +37,8 @@ class DISPLAY_EXPORT UwpTextScaleFactor {
static UwpTextScaleFactor* Instance(); static UwpTextScaleFactor* Instance();
virtual ~UwpTextScaleFactor(); virtual ~UwpTextScaleFactor();
// Retrieves the Windows Text Zoom scale factor. Guaranteed to be >= 1.
virtual float GetTextScaleFactor() const; virtual float GetTextScaleFactor() const;
// Registers and observer that will be notified of any changes to UWP screen // Registers and observer that will be notified of any changes to UWP screen
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "base/win/scoped_gdi_object.h" #include "base/win/scoped_gdi_object.h"
#include "base/win/win_client_metrics.h" #include "base/win/win_client_metrics.h"
#include "ui/base/l10n/l10n_util_win.h" #include "ui/base/l10n/l10n_util_win.h"
#include "ui/display/win/dpi.h"
#include "ui/gfx/color_utils.h" #include "ui/gfx/color_utils.h"
#include "ui/native_theme/native_theme_win.h" #include "ui/native_theme/native_theme_win.h"
...@@ -24,6 +25,7 @@ void MenuConfig::Init() { ...@@ -24,6 +25,7 @@ void MenuConfig::Init() {
NONCLIENTMETRICS_XP metrics; NONCLIENTMETRICS_XP metrics;
base::win::GetNonClientMetrics(&metrics); base::win::GetNonClientMetrics(&metrics);
display::win::AdjustFontForAccessibility(&metrics.lfMenuFont);
l10n_util::AdjustUIFont(&(metrics.lfMenuFont)); l10n_util::AdjustUIFont(&(metrics.lfMenuFont));
{ {
base::win::ScopedHFONT new_font(CreateFontIndirect(&metrics.lfMenuFont)); base::win::ScopedHFONT new_font(CreateFontIndirect(&metrics.lfMenuFont));
......
...@@ -59,6 +59,7 @@ ...@@ -59,6 +59,7 @@
#include "base/win/scoped_gdi_object.h" #include "base/win/scoped_gdi_object.h"
#include "base/win/win_client_metrics.h" #include "base/win/win_client_metrics.h"
#include "ui/base/l10n/l10n_util_win.h" #include "ui/base/l10n/l10n_util_win.h"
#include "ui/display/win/dpi.h"
#include "ui/views/widget/desktop_aura/desktop_window_tree_host_win.h" #include "ui/views/widget/desktop_aura/desktop_window_tree_host_win.h"
#endif #endif
...@@ -1228,6 +1229,7 @@ gfx::FontList NativeWidgetPrivate::GetWindowTitleFontList() { ...@@ -1228,6 +1229,7 @@ gfx::FontList NativeWidgetPrivate::GetWindowTitleFontList() {
#if defined(OS_WIN) #if defined(OS_WIN)
NONCLIENTMETRICS_XP ncm; NONCLIENTMETRICS_XP ncm;
base::win::GetNonClientMetrics(&ncm); base::win::GetNonClientMetrics(&ncm);
display::win::AdjustFontForAccessibility(&(ncm.lfCaptionFont));
l10n_util::AdjustUIFont(&(ncm.lfCaptionFont)); l10n_util::AdjustUIFont(&(ncm.lfCaptionFont));
base::win::ScopedHFONT caption_font(CreateFontIndirect(&(ncm.lfCaptionFont))); base::win::ScopedHFONT caption_font(CreateFontIndirect(&(ncm.lfCaptionFont)));
return gfx::FontList(gfx::Font(caption_font.get())); return gfx::FontList(gfx::Font(caption_font.get()));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment