Chrome not considering Underline Position from Font

Source/core/rendering/InlineTextBox.cpp

Index: Source/core/rendering/InlineTextBox.cpp
diff --git a/Source/core/rendering/InlineTextBox.cpp b/Source/core/rendering/InlineTextBox.cpp
index 8a0b42f2ada185c1829de7d9359606eec36636fb..dcf3be4bc38d7aa8dd38904ae9081ddfe790f014 100644
--- a/Source/core/rendering/InlineTextBox.cpp
+++ b/Source/core/rendering/InlineTextBox.cpp
@@ -910,7 +910,14 @@ static int computeUnderlineOffset(const TextUnderlinePosition underlinePosition,
 {
     // Compute the gap between the font and the underline. Use at least one
     // pixel gap, if underline is thick then use a bigger gap.
-    const int gap = std::max<int>(1, ceilf(textDecorationThickness / 2.f));
+    int gap = std::max<int>(1, ceilf(textDecorationThickness / 2.f));
+
+    // Underline position of zero means draw underline on Baseline Position,
+    // in Blink we need at least 1-pixel gap to adding following check. Default convention
+    // is Positive underline Pos means draw above baseline while Negative means draw below
+    // baseline, as Blink has Y increasing downwards so negating the value.
+    if (fontMetrics.underlinePosition())
+        gap = -fontMetrics.underlinePosition();

[eae, 2014/02/28 17:37:59]

Computing the gap and then immediately overwriting the value seems wasteful.

How about

if (fontMetrics.underlinePosition())
  gap = -fontMetrics.underlinePosition();
else
  gap = std::max<int>(1, ceilf(textDecorationThickness / 2.f));

or

const int gap = fontMetrics.underlinePosition()
    ? -fontMetrics.underlinePosition();
    : std::max<int>(1, ceilf(textDecorationThickness / 2.f));

preserving the comments, obviously.

[h.joshi, 2014/03/01 03:36:12]

Will make changes in next patch.

On 2014/02/28 17:37:59, eae wrote:

 
     // FIXME: We support only horizontal text for now.
     switch (underlinePosition) {