PlatformFontWin::DeriveFontWithHeight insensitive to base font's height

ui/gfx/platform_font_win.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/gfx/platform_font_win.h"
 
 #include <windows.h>
 #include <math.h>
 
 #include <algorithm>
 #include <string>
 
 #include "base/logging.h"
 #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"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/font.h"
 #include "ui/gfx/win/scoped_set_map_mode.h"
 
 namespace {
 
 // If the tmWeight field of a TEXTMETRIC structure has a value >= this, the
@@ skipping 82 matching lines @@
     }
     return font;
   }
 
   LOGFONT font_info;
   GetObject(GetNativeFont(), sizeof(LOGFONT), &font_info);
   font_info.lfHeight = height;
   SetLogFontStyle(style, &font_info);
 
   HFONT hfont = CreateFontIndirect(&font_info);
-  return Font(new PlatformFontWin(CreateHFontRef(hfont)));
+  return DeriveWithCorrectedSize(hfont);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // PlatformFontWin, PlatformFont implementation:
 
 Font PlatformFontWin::DeriveFont(int size_delta, int style) const {
   LOGFONT font_info;
   GetObject(GetNativeFont(), sizeof(LOGFONT), &font_info);
   const int requested_font_size = font_ref_->requested_font_size();
   font_info.lfHeight = AdjustFontSize(-requested_font_size, size_delta);
@@ skipping 98 matching lines @@
 PlatformFontWin::HFontRef* PlatformFontWin::CreateHFontRef(HFONT font) {
   TEXTMETRIC font_metrics;
 
   {
     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);
   }
 
+  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 =
       std::max<int>(1, font_metrics.tmAscent - font_metrics.tmInternalLeading);
   const int ave_char_width = std::max<int>(1, font_metrics.tmAveCharWidth);
   const int font_size =
       std::max<int>(1, font_metrics.tmHeight - font_metrics.tmInternalLeading);
   int style = 0;
   if (font_metrics.tmItalic)
     style |= Font::ITALIC;
   if (font_metrics.tmUnderlined)
     style |= Font::UNDERLINE;
   if (font_metrics.tmWeight >= kTextMetricWeightBold)
     style |= Font::BOLD;
 
   return new HFontRef(font, font_size, height, baseline, cap_height,
                       ave_char_width, style);
 }
 
+Font PlatformFontWin::DeriveWithCorrectedSize(HFONT base_font) {

[Alexei Svitkine (slow), 2014/06/02 15:42:55]

Make this a free-standing function in the anon namespace at the top of the file
instead.

[Tomasz Moniuszko, 2014/06/11 13:11:56]

This method calls PlatformFontWin::CreateHFontRef which is a private method so
it cannot be called from anonymous namespace.

+  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;
+  {
+    base::win::ScopedSelectObject scoped_font(screen_dc, best_font);
+    GetTextMetrics(screen_dc, &best_font_metrics);

[Alexei Svitkine (slow), 2014/06/02 15:42:55]

Nit: Make a helper function in the anon namespace for getting the metrics given
a font and a dc, so these blocks can be much shorter, i.e.:

TEXTMETRIC best_font_metrics;
GetTextMetricsForFont(screen_dc, best_font, &best_font_metrics);

You can use it both here and in CreateHFontRef() above.

[Tomasz Moniuszko, 2014/06/11 13:11:55]

Done.

+  }
+
+  LOGFONT font_info;
+  GetObject(base_font, sizeof(LOGFONT), &font_info);
+
+  do {
+    font_info.lfHeight =
+        -(best_font_metrics.tmHeight - best_font_metrics.tmInternalLeading + 1);

[Alexei Svitkine (slow), 2014/06/02 15:42:55]

Nit: Add a comment explaining this logic.

[Tomasz Moniuszko, 2014/06/11 13:11:55]

Done.

+    base::win::ScopedGDIObject<HFONT> font(CreateFontIndirect(&font_info));
+    TEXTMETRIC font_metrics;
+    {
+      base::win::ScopedSelectObject scoped_font(screen_dc, font);
+      GetTextMetrics(screen_dc, &font_metrics);
+    }
+    if (font_metrics.tmHeight > best_font_metrics.tmHeight) {

[Alexei Svitkine (slow), 2014/06/02 15:42:55]

Nit: No {}'s.

[Tomasz Moniuszko, 2014/06/11 13:11:56]

Done.

+      break;
+    }
+    best_font.Set(font.release());
+    best_font_metrics = font_metrics;
+  } while (true);
+
+  return Font(new PlatformFontWin(
+      CreateHFontRef(best_font.release())));

[Alexei Svitkine (slow), 2014/06/02 15:42:55]

Nit: Does this fit on the previous line?

[Tomasz Moniuszko, 2014/06/11 13:11:56]

Done.

+}
+
 PlatformFontWin::PlatformFontWin(HFontRef* hfont_ref) : font_ref_(hfont_ref) {
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // PlatformFontWin::HFontRef:
 
 PlatformFontWin::HFontRef::HFontRef(HFONT hfont,
                                     int font_size,
                                     int height,
                                     int baseline,
@@ skipping 55 matching lines @@
   return new PlatformFontWin(native_font);
 }
 
 // static
 PlatformFont* PlatformFont::CreateFromNameAndSize(const std::string& font_name,
                                                   int font_size) {
   return new PlatformFontWin(font_name, font_size);
 }
 
 }  // namespace gfx