Use pango underline metrics in RenderTextLinux.

ui/gfx/render_text.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.h"
 
 #include <algorithm>
 
 #include "base/i18n/break_iterator.h"
 #include "base/logging.h"
@@ skipping 161 matching lines @@
   return SkGradientShader::CreateLinear(&points[0], &colors[0], &positions[0],
       colors.size(), SkShader::kClamp_TileMode);
 }
 
 }  // namespace
 
 namespace gfx {
 
 namespace internal {
 
+// Value of |underline_thickness_| that indicates that underline metrics have
+// not been set explicitly.
+const SkScalar kUnderlineMetricsNotSet = -1.0f;
+
 SkiaTextRenderer::SkiaTextRenderer(Canvas* canvas)
     : canvas_skia_(canvas->sk_canvas()),
-      started_drawing_(false) {
+      started_drawing_(false),
+      underline_thickness_(kUnderlineMetricsNotSet),
+      underline_position_(0.0f) {

[msw, 2012/06/12 20:44:39]

You should also init this to kUnderlineMetricsNotSet.
It's better to be explicit with its value.

   DCHECK(canvas_skia_);
   paint_.setTextEncoding(SkPaint::kGlyphID_TextEncoding);
   paint_.setStyle(SkPaint::kFill_Style);
   paint_.setAntiAlias(true);
   paint_.setSubpixelText(true);
   paint_.setLCDRenderText(true);
   bounds_.setEmpty();
 }
 
 SkiaTextRenderer::~SkiaTextRenderer() {
@@ skipping 19 matching lines @@
                                                 bool enable_lcd_text) {
   paint_.setAntiAlias(enable_smoothing);
   paint_.setSubpixelText(enable_smoothing);
   paint_.setLCDRenderText(enable_lcd_text);
 }
 
 void SkiaTextRenderer::SetTypeface(SkTypeface* typeface) {
   paint_.setTypeface(typeface);
 }
 
-void SkiaTextRenderer::SetTextSize(int size) {
+void SkiaTextRenderer::SetTextSize(SkScalar size) {
   paint_.setTextSize(size);
 }
 
 void SkiaTextRenderer::SetFontFamilyWithStyle(const std::string& family,
                                               int style) {
   DCHECK(!family.empty());
 
   SkTypeface::Style skia_style = ConvertFontStyleToSkiaTypefaceStyle(style);
   SkTypeface* typeface = SkTypeface::CreateFromName(family.c_str(), skia_style);
   SkAutoUnref auto_unref(typeface);
@@ skipping 10 matching lines @@
 
 void SkiaTextRenderer::SetForegroundColor(SkColor foreground) {
   paint_.setColor(foreground);
 }
 
 void SkiaTextRenderer::SetShader(SkShader* shader, const Rect& bounds) {
   bounds_ = RectToSkRect(bounds);
   paint_.setShader(shader);
 }
 
+void SkiaTextRenderer::SetUnderlineMetrics(SkScalar thickness,
+                                           SkScalar position) {
+  underline_thickness_ = thickness;
+  underline_position_ = position;
+}
+
 void SkiaTextRenderer::DrawPosText(const SkPoint* pos,
                                    const uint16* glyphs,
                                    size_t glyph_count) {
   if (!started_drawing_) {
     started_drawing_ = true;
     // Work-around for http://crbug.com/122743, where non-ClearType text is
     // rendered with incorrect gamma when using the fade shader. Draw the text
     // to a layer and restore it faded by drawing a rect in kDstIn_Mode mode.
     //
     // Skip this when there is a looper which seems not working well with
@@ skipping 32 matching lines @@
   const SkScalar kDiagonalStrikeThroughMarginOffset = (SK_Scalar1 / 4);
 
   SkScalar text_size = paint_.getTextSize();
   SkScalar height = SkScalarMul(text_size, kLineThickness);
   SkRect r;
 
   r.fLeft = x;
   r.fRight = x + width;
 
   if (style.underline) {
-    SkScalar offset = SkScalarMulAdd(text_size, kUnderlineOffset, y);
-    r.fTop = offset;
-    r.fBottom = offset + height;
+    if (underline_thickness_ == kUnderlineMetricsNotSet) {
+      r.fTop = SkScalarMulAdd(text_size, kUnderlineOffset, y);
+      r.fBottom = r.fTop + height;
+    } else {
+      r.fTop = y + underline_position_;

[msw, 2012/06/12 20:44:39]

You should probably DCHECK that underline_position_ is not
kUnderlineMetricsNotSet. I know they're set together now, but that may not
always be the case...

[Alexei Svitkine (slow), 2012/06/12 20:47:43]

-1.0 may be a valid position offset, but definitely never a valid thickness.

[msw, 2012/06/12 21:28:12]

Fair enough, then it might be worthwhile to add a comment at the declaration /
setter that the two must be set together, etc.

+      r.fBottom = r.fTop + underline_thickness_;
+    }
     canvas_skia_->drawRect(r, paint_);
   }
   if (style.strike) {
     SkScalar offset = SkScalarMulAdd(text_size, kStrikeThroughOffset, y);
     r.fTop = offset;
     r.fBottom = offset + height;
     canvas_skia_->drawRect(r, paint_);
   }
   if (style.diagonal_strike) {
     SkScalar offset =
@@ skipping 625 matching lines @@
   if (cursor_enabled() && cursor_visible() && focused()) {
     const Rect& bounds = GetUpdatedCursorBounds();
     if (bounds.width() != 0)
       canvas->FillRect(bounds, cursor_color_);
     else
       canvas->DrawRect(bounds, cursor_color_);
   }
 }
 
 }  // namespace gfx