Fix clipping RenderTextHarfBuzz text on Windows 8.

ui/gfx/render_text.cc

Index: ui/gfx/render_text.cc
diff --git a/ui/gfx/render_text.cc b/ui/gfx/render_text.cc
index 1ca874a0e461f2014c5568c4fcfeb0802bba7fcc..1d8375a3177e7c164a76765c5b843ed9b5bd5e9b 100644
--- a/ui/gfx/render_text.cc
+++ b/ui/gfx/render_text.cc
@@ -28,6 +28,10 @@
 #include "ui/gfx/text_utils.h"
 #include "ui/gfx/utf16_indexing.h"
 
+#if defined(OS_WIN)
+#include "base/win/windows_version.h"
+#endif
+
 namespace gfx {
 
 namespace {
@@ -169,6 +173,24 @@ SkPaint::Hinting FontRenderParamsHintingToSkPaintHinting(
   return SkPaint::kNo_Hinting;
 }
 
+// Returns true if the RenderTextHarfBuzz implementation should be used.
+bool UseHarfBuzz() {
+  static const bool kUseHarfBuzz =
+      !CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kDisableHarfBuzzRenderText);
+  return kUseHarfBuzz;
+}
+
+// Returns the padding left of centered text used to match legacy behavior.
+// TODO(msw|ckocagil): This may not be needed at all for RenderTextHarfBuzz.
+int GetLeftOfCenterPadding() {
+#if defined(OS_WIN)
+   if (UseHarfBuzz() && base::win::GetVersion() > base::win::VERSION_WIN7)
+     return 0;
+#endif
+   return 1;
+}
+
 }  // namespace
 
 namespace internal {
@@ -398,9 +420,7 @@ RenderText::~RenderText() {
 }
 
 RenderText* RenderText::CreateInstance() {
-  return CommandLine::ForCurrentProcess()->HasSwitch(
-      switches::kDisableHarfBuzzRenderText) ? CreateNativeInstance() :
-      new RenderTextHarfBuzz;
+  return UseHarfBuzz() ? new RenderTextHarfBuzz() : CreateNativeInstance();
 }
 
 void RenderText::SetText(const base::string16& text) {
@@ -1072,9 +1092,10 @@ Vector2d RenderText::GetAlignmentOffset(size_t line_number) {
     const int width = GetContentWidth();
 #endif
     offset.set_x(display_rect().width() - width);
-    // Put any extra margin pixel on the left to match legacy behavior.
+    // Put any extra margin pixel on the left to match some legacy behaviors.
+    static const int kLeftOfCenterPadding = GetLeftOfCenterPadding();
     if (horizontal_alignment == ALIGN_CENTER)
-      offset.set_x((offset.x() + 1) / 2);
+      offset.set_x((offset.x() + kLeftOfCenterPadding) / 2);

[Peter Kasting, 2014/11/06 23:14:47]

I don't understand why the previous code was causing problems assuming |width|
is at least as large at display_rect().width() and display_rect().width()
actually contains the full extent of the text.

The only difference between the old code and the new is that, if |width| has an
odd number of extra pixels beyond what's necessary for diaply_rect(), the old
code puts the extra pixel on the left, and the new on the right.  But in neither
case should we actually be truncating something visible.  That should only
happen if one of the two conditions I mention above is violated.  And if those
are violated, it seems like the right fix is elsewhere rather than here.

Maybe I just don't understand what the code is doing.  Where is the logic flaw
above?

[msw, 2014/11/06 23:48:44]

You're right, width+1 in RenderTextHarfBuzz::GetStringSizeF works.
That's a better temporary hack to use until we can dig deeper.

   }
 
   // Vertically center the text.