[Chromium] Sometimes bottom of text is truncated when page has a fractional scale
authorwangxianzhu@chromium.org <wangxianzhu@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 14 Jul 2012 00:52:56 +0000 (00:52 +0000)
committerwangxianzhu@chromium.org <wangxianzhu@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 14 Jul 2012 00:52:56 +0000 (00:52 +0000)
https://bugs.webkit.org/show_bug.cgi?id=88684

Reviewed by Tony Chang.

Source/WebCore:

When the page has a fractional scale, the ascent and descent part of the fonts might be fractional.
If the descent part is rounded down, the bottom of the text might be truncated when displayed
when subpixel text positioning is enabled.
To avoid that, borrow one unit from the ascent when possible.

Test: fast/text/descent-clip-in-scaled-page.html

* platform/graphics/harfbuzz/FontPlatformDataHarfBuzz.cpp:
(WebCore::FontPlatformData::setupPaint): Moved NoPreference handling into querySystemForRenderStyle so that fontRenderStyle() can have actual styles without NoPreference.
(WebCore::FontPlatformData::querySystemForRenderStyle): Added NoPreference handling (moved from setupPaint)
* platform/graphics/harfbuzz/FontPlatformDataHarfBuzz.h:
(FontPlatformData):
(WebCore::FontPlatformData::fontRenderStyle): Added to let SimpleFontDataSkia access the font render styles.
* platform/graphics/skia/SimpleFontDataSkia.cpp:
(WebCore::SimpleFontData::platformInit):

LayoutTests:

New test case.

* fast/text/descent-clip-in-scaled-page-expected.html: Added.
* fast/text/descent-clip-in-scaled-page.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@122651 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/text/descent-clip-in-scaled-page-expected.html [new file with mode: 0644]
LayoutTests/fast/text/descent-clip-in-scaled-page.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/harfbuzz/FontPlatformDataHarfBuzz.cpp
Source/WebCore/platform/graphics/harfbuzz/FontPlatformDataHarfBuzz.h
Source/WebCore/platform/graphics/skia/SimpleFontDataSkia.cpp

index da6ee53..ecfebc8 100644 (file)
@@ -1,3 +1,15 @@
+2012-07-13  Xianzhu Wang  <wangxianzhu@chromium.org>
+
+        [Chromium] Sometimes bottom of text is truncated when page has a fractional scale
+        https://bugs.webkit.org/show_bug.cgi?id=88684
+
+        Reviewed by Tony Chang.
+
+        New test case.
+
+        * fast/text/descent-clip-in-scaled-page-expected.html: Added.
+        * fast/text/descent-clip-in-scaled-page.html: Added.
+
 2012-07-13  Filip Pizlo  <fpizlo@apple.com>
 
         ASSERTION FAILED: use.useKind() != DoubleUse
diff --git a/LayoutTests/fast/text/descent-clip-in-scaled-page-expected.html b/LayoutTests/fast/text/descent-clip-in-scaled-page-expected.html
new file mode 100644 (file)
index 0000000..75d3a34
--- /dev/null
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+    div {
+        font-family: ahem;
+        font-size: 16px;
+    }
+</style>
+<script>
+    if (window.layoutTestController && window.layoutTestController.setTextSubpixelPositioning)
+        window.layoutTestController.setTextSubpixelPositioning(true);
+    if (window.internals)
+        window.internals.settings.setPageScaleFactor(1.7, 0, 0);
+</script>
+</head>
+<body>
+    Tests if the bottom of the text is truncated when the page is scaled by a fractional factor.
+    If success, the text in the "overflow: hidden" div (the test case) should be displayed
+    the same as in a normal div (the ref html).
+    'p' is the character in ahem font with only the descent part.
+    <div>&nbsp;pppp&nbsp;</div>
+</body>
+</html>
diff --git a/LayoutTests/fast/text/descent-clip-in-scaled-page.html b/LayoutTests/fast/text/descent-clip-in-scaled-page.html
new file mode 100644 (file)
index 0000000..49a21b9
--- /dev/null
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+    div {
+        font-family: ahem;
+        font-size: 16px;
+        overflow: hidden; /* the only difference between the test the the ref html */
+    }
+</style>
+<script>
+    if (window.layoutTestController && window.layoutTestController.setTextSubpixelPositioning)
+        window.layoutTestController.setTextSubpixelPositioning(true);
+    if (window.internals)
+        window.internals.settings.setPageScaleFactor(1.7, 0, 0);
+</script>
+</head>
+<body>
+    Tests if the bottom of the text is truncated when the page is scaled by a fractional factor.
+    If success, the text in the "overflow: hidden" div (the test case) should be displayed
+    the same as in a normal div (the ref html).
+    'p' is the character in ahem font with only the descent part.
+    <div>&nbsp;pppp&nbsp;</div>
+</body>
+</html>
index bcaedce..aa9bf79 100644 (file)
@@ -1,3 +1,26 @@
+2012-07-13  Xianzhu Wang  <wangxianzhu@chromium.org>
+
+        [Chromium] Sometimes bottom of text is truncated when page has a fractional scale
+        https://bugs.webkit.org/show_bug.cgi?id=88684
+
+        Reviewed by Tony Chang.
+
+        When the page has a fractional scale, the ascent and descent part of the fonts might be fractional.
+        If the descent part is rounded down, the bottom of the text might be truncated when displayed
+        when subpixel text positioning is enabled.
+        To avoid that, borrow one unit from the ascent when possible.
+
+        Test: fast/text/descent-clip-in-scaled-page.html
+
+        * platform/graphics/harfbuzz/FontPlatformDataHarfBuzz.cpp:
+        (WebCore::FontPlatformData::setupPaint): Moved NoPreference handling into querySystemForRenderStyle so that fontRenderStyle() can have actual styles without NoPreference.
+        (WebCore::FontPlatformData::querySystemForRenderStyle): Added NoPreference handling (moved from setupPaint)
+        * platform/graphics/harfbuzz/FontPlatformDataHarfBuzz.h:
+        (FontPlatformData):
+        (WebCore::FontPlatformData::fontRenderStyle): Added to let SimpleFontDataSkia access the font render styles.
+        * platform/graphics/skia/SimpleFontDataSkia.cpp:
+        (WebCore::SimpleFontData::platformInit):
+
 2012-07-13  Ryosuke Niwa  <rniwa@webkit.org>
 
         Remove an assertion after r122637.
index 695531c..48b1624 100644 (file)
@@ -175,31 +175,19 @@ String FontPlatformData::description() const
 
 void FontPlatformData::setupPaint(SkPaint* paint) const
 {
-    const float ts = m_textSize >= 0 ? m_textSize : 12;
+    paint->setAntiAlias(m_style.useAntiAlias);
+    paint->setHinting(static_cast<SkPaint::Hinting>(m_style.hintStyle));
+    paint->setEmbeddedBitmapText(m_style.useBitmaps);
+    paint->setAutohinted(m_style.useAutoHint);
+    paint->setSubpixelText(m_style.useSubpixelPositioning);
+    if (m_style.useAntiAlias)
+        paint->setLCDRenderText(m_style.useSubpixelRendering);
 
-    paint->setAntiAlias(m_style.useAntiAlias == FontRenderStyle::NoPreference ? useSkiaAntiAlias : m_style.useAntiAlias);
-    switch (m_style.useHinting) {
-    case FontRenderStyle::NoPreference:
-        paint->setHinting(skiaHinting);
-        break;
-    case 0:
-        paint->setHinting(SkPaint::kNo_Hinting);
-        break;
-    default:
-        paint->setHinting(static_cast<SkPaint::Hinting>(m_style.hintStyle));
-        break;
-    }
-
-    paint->setEmbeddedBitmapText(m_style.useBitmaps == FontRenderStyle::NoPreference ? useSkiaBitmaps : m_style.useBitmaps);
+    const float ts = m_textSize >= 0 ? m_textSize : 12;
     paint->setTextSize(SkFloatToScalar(ts));
     paint->setTypeface(m_typeface);
     paint->setFakeBoldText(m_fakeBold);
     paint->setTextSkewX(m_fakeItalic ? -SK_Scalar1 / 4 : 0);
-    paint->setAutohinted(m_style.useAutoHint == FontRenderStyle::NoPreference ? useSkiaAutoHint : m_style.useAutoHint);
-    paint->setSubpixelText(m_style.useSubpixelPositioning == FontRenderStyle::NoPreference ? useSkiaSubpixelPositioning : m_style.useSubpixelPositioning);
-
-    if (m_style.useAntiAlias == 1 || (m_style.useAntiAlias == FontRenderStyle::NoPreference && useSkiaAntiAlias))
-        paint->setLCDRenderText(m_style.useSubpixelRendering == FontRenderStyle::NoPreference ? useSkiaSubpixelRendering : m_style.useSubpixelRendering);
 }
 
 SkFontID FontPlatformData::uniqueID() const
@@ -272,6 +260,26 @@ HarfbuzzFace* FontPlatformData::harfbuzzFace() const
 void FontPlatformData::querySystemForRenderStyle()
 {
     PlatformSupport::getRenderStyleForStrike(m_family.data(), (((int)m_textSize) << 2) | (m_typeface->style() & 3), &m_style);
+
+    // Fix FontRenderStyle::NoPreference to actual styles.
+    if (m_style.useAntiAlias == FontRenderStyle::NoPreference)
+         m_style.useAntiAlias = useSkiaAntiAlias;
+
+    if (!m_style.useHinting)
+        m_style.hintStyle = SkPaint::kNo_Hinting;
+    else if (m_style.useHinting == FontRenderStyle::NoPreference)
+        m_style.hintStyle = skiaHinting;
+
+    if (m_style.useBitmaps == FontRenderStyle::NoPreference)
+        m_style.useBitmaps = useSkiaBitmaps;
+    if (m_style.useAutoHint == FontRenderStyle::NoPreference)
+        m_style.useAutoHint = useSkiaAutoHint;
+    if (m_style.useSubpixelPositioning == FontRenderStyle::NoPreference)
+        m_style.useSubpixelPositioning = useSkiaSubpixelPositioning;
+    if (m_style.useAntiAlias == FontRenderStyle::NoPreference)
+        m_style.useAntiAlias = useSkiaAntiAlias;
+    if (m_style.useSubpixelRendering == FontRenderStyle::NoPreference)
+        m_style.useSubpixelRendering = useSkiaSubpixelRendering;
 }
 
 } // namespace WebCore
index b4423f4..6f4eca3 100644 (file)
@@ -139,6 +139,9 @@ public:
     HarfbuzzFace* harfbuzzFace() const;
 #endif
 
+    // The returned styles are all actual styles without FontRenderStyle::NoPreference.
+    const FontRenderStyle& fontRenderStyle() const { return m_style; }
+
     // -------------------------------------------------------------------------
     // Global font preferences...
 
index b4d20b0..54486c3 100644 (file)
@@ -86,7 +86,7 @@ void SimpleFontData::platformInit()
     float descent;
 
     // Beware those who step here: This code is designed to match Win32 font
-    // metrics *exactly*.
+    // metrics *exactly* (except the adjustment of ascent/descent on Linux/Android).
     if (isVDMXValid) {
         ascent = vdmxAscent;
         descent = -vdmxDescent;
@@ -94,6 +94,16 @@ void SimpleFontData::platformInit()
         SkScalar height = -metrics.fAscent + metrics.fDescent + metrics.fLeading;
         ascent = SkScalarRound(-metrics.fAscent);
         descent = SkScalarRound(height) - ascent;
+#if OS(LINUX) || OS(ANDROID)
+        // When subpixel positioning is enabled, if the descent is rounded down, the descent part
+        // of the glyph may be truncated when displayed in a 'overflow: hidden' container.
+        // To avoid that, borrow 1 unit from the ascent when possible.
+        // FIXME: This can be removed if sub-pixel ascent/descent is supported.
+        if (platformData().fontRenderStyle().useSubpixelPositioning && descent < SkScalarToFloat(metrics.fDescent) && ascent >= 1) {
+            ++descent;
+            --ascent;
+        }
+#endif
     }
 
     m_fontMetrics.setAscent(ascent);