linux: Make RenderTextPango avoid clipping underlines.

ui/gfx/render_text_pango.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/render_text_pango.h"
 
 #include <pango/pangocairo.h>
 #include <algorithm>
 #include <string>
 #include <vector>
@@ skipping 37 matching lines @@
 }
 
 // Sets underline metrics on |renderer| according to Pango font |desc|.
 void SetPangoUnderlineMetrics(PangoFontDescription *desc,
                               internal::SkiaTextRenderer* renderer) {
   PangoFontMetrics* metrics = GetPangoFontMetrics(desc);
   int thickness = pango_font_metrics_get_underline_thickness(metrics);
   // Pango returns the position "above the baseline". Change its sign to convert
   // it to a vertical offset from the baseline.
   int position = -pango_font_metrics_get_underline_position(metrics);
-  pango_quantize_line_geometry(&thickness, &position);
+
   // Note: pango_quantize_line_geometry() guarantees pixel boundaries, so
   //       PANGO_PIXELS() is safe to use.
-  renderer->SetUnderlineMetrics(PANGO_PIXELS(thickness),
-                                PANGO_PIXELS(position));
+  pango_quantize_line_geometry(&thickness, &position);
+  int thickness_pixels = PANGO_PIXELS(thickness);
+  int position_pixels = PANGO_PIXELS(position);
+
+  // Ugly hack: make sure that underlines don't get clipped. See
+  // http://crbug.com/393117.
+  int descent_pixels = PANGO_PIXELS(pango_font_metrics_get_descent(metrics));

[Daniel Erat, 2014/08/28 20:37:23]

it's also possible that this isn't the right fix and it just happens to work. is
the underline even supposed to fit within the descent? am i misinterpreting the
units here and treating a point that's in the middle of a pixel as being at the
edge of a pixel instead?

i mainly just want to fix this for the default ubuntu config in a way that can
be merged back to 38 so we can limp to 39 and delete RenderTextPango in favor of
RenderTextHarfBuzz... :-/

[msw, 2014/08/29 00:41:55]

I honestly don't know the Pango underline code that well. Could there be an
issue in our ordering of quantizing/PANGO_PIXEL'ing versus actually using
|underline_position_| in SkiaTextRenderer::DrawUnderline? If this works for now,
it's probably okay, given the impending switch to HarfBuzz. Feel free to ping
Alexei, Dave, or other relevant authors for more knowledgeable review.

+  position_pixels = std::min(position_pixels,
+                             descent_pixels - thickness_pixels);
+
+  renderer->SetUnderlineMetrics(thickness_pixels, position_pixels);
 }
 
 }  // namespace
 
 // TODO(xji): index saved in upper layer is utf16 index. Pango uses utf8 index.
 // Since caret_pos is used internally, we could save utf8 index for caret_pos
 // to avoid conversion.
 
 RenderTextPango::RenderTextPango()
     : layout_(NULL),
@@ skipping 430 matching lines @@
                                           int glyph_index) const {
   return LayoutIndexToTextIndex(run->item->offset +
                                 run->glyphs->log_clusters[glyph_index]);
 }
 
 RenderText* RenderText::CreateNativeInstance() {
   return new RenderTextPango;
 }
 
 }  // namespace gfx