PlatformFontWin::DeriveFontWithHeight insensitive to base font's height

ui/gfx/platform_font_win.cc

Index: ui/gfx/platform_font_win.cc
diff --git a/ui/gfx/platform_font_win.cc b/ui/gfx/platform_font_win.cc
index 4acaf64b34abdb2ff9abb8b0755e588741f8f1e1..a47bbd777437bfcad1397a46dcdf4d4ce1c0cb39 100644
--- a/ui/gfx/platform_font_win.cc
+++ b/ui/gfx/platform_font_win.cc
@@ -14,6 +14,7 @@
 #include "base/strings/string_util.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/win/scoped_gdi_object.h"
 #include "base/win/scoped_hdc.h"
 #include "base/win/scoped_select_object.h"
 #include "base/win/win_util.h"
@@ -60,6 +61,11 @@ void SetLogFontStyle(int font_style, LOGFONT* font_info) {
   font_info->lfWeight = (font_style & gfx::Font::BOLD) ? FW_BOLD : FW_NORMAL;
 }
 
+void GetTextMetricsForFont(HDC hdc, HFONT font, TEXTMETRIC* text_metrics) {
+  base::win::ScopedSelectObject scoped_font(hdc, font);
+  GetTextMetrics(hdc, text_metrics);
+}
+
 }  // namespace
 
 namespace gfx {
@@ -116,7 +122,7 @@ Font PlatformFontWin::DeriveFontWithHeight(int height, int style) {
   SetLogFontStyle(style, &font_info);
 
   HFONT hfont = CreateFontIndirect(&font_info);
-  return Font(new PlatformFontWin(CreateHFontRef(hfont)));
+  return DeriveWithCorrectedSize(hfont);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -230,11 +236,16 @@ PlatformFontWin::HFontRef* PlatformFontWin::CreateHFontRef(HFONT font) {
 
   {
     base::win::ScopedGetDC screen_dc(NULL);
-    base::win::ScopedSelectObject scoped_font(screen_dc, font);
     gfx::ScopedSetMapMode mode(screen_dc, MM_TEXT);
-    GetTextMetrics(screen_dc, &font_metrics);
+    GetTextMetricsForFont(screen_dc, font, &font_metrics);
   }
 
+  return CreateHFontRef(font, font_metrics);
+}
+
+PlatformFontWin::HFontRef* PlatformFontWin::CreateHFontRef(
+    HFONT font,
+    const TEXTMETRIC& font_metrics) {
   const int height = std::max<int>(1, font_metrics.tmHeight);
   const int baseline = std::max<int>(1, font_metrics.tmAscent);
   const int cap_height =
@@ -254,6 +265,34 @@ PlatformFontWin::HFontRef* PlatformFontWin::CreateHFontRef(HFONT font) {
                       ave_char_width, style);
 }
 
+Font PlatformFontWin::DeriveWithCorrectedSize(HFONT base_font) {
+  base::win::ScopedGetDC screen_dc(NULL);
+  gfx::ScopedSetMapMode mode(screen_dc, MM_TEXT);
+
+  base::win::ScopedGDIObject<HFONT> best_font(base_font);
+  TEXTMETRIC best_font_metrics;
+  GetTextMetricsForFont(screen_dc, best_font, &best_font_metrics);
+
+  LOGFONT font_info;
+  GetObject(base_font, sizeof(LOGFONT), &font_info);
+
+  do {
+    // Increment font size. Prefer font with greater size if its height isn't
+    // greater than height of base font.
+    font_info.lfHeight =
+        -(best_font_metrics.tmHeight - best_font_metrics.tmInternalLeading + 1);

[Alexei Svitkine (slow), 2014/06/11 16:58:48]

Question: Is it necessary to do this calculation or can you just
increment/decrement the previous lfHeight value (since it has been filled in by
GetObject()). Maybe via AdjustFontSize()?

[Tomasz Moniuszko, 2014/06/12 08:04:24]

Good catch! In the worst case size might be not increased at all and we would
enter endless loop (I don't know if such font exists but I can imagine such
case).

lfHeight returned from GetObject is the same value that was passed to
CreateFontIndirect. It may be >= 0 (which means that we requested font by its
height). We need to ensure that lfHeight is < 0 (font's size) before entering
the loop. Then we can use AdjustFontSize inside the loop.

I made necessary changes.

+    base::win::ScopedGDIObject<HFONT> font(CreateFontIndirect(&font_info));
+    TEXTMETRIC font_metrics;
+    GetTextMetricsForFont(screen_dc, font, &font_metrics);
+    if (font_metrics.tmHeight > best_font_metrics.tmHeight)
+      break;
+    best_font.Set(font.release());
+    best_font_metrics = font_metrics;
+  } while (true);
+
+  return Font(new PlatformFontWin(CreateHFontRef(best_font.release())));
+}
+
 PlatformFontWin::PlatformFontWin(HFontRef* hfont_ref) : font_ref_(hfont_ref) {
 }