Glyph advances from generateAdvance do not always match generateMetrics results.

http://codereview.appspot.com/5841071/


git-svn-id: http://skia.googlecode.com/svn/trunk@3480 2bbb7eff-a529-9590-31e7-b0007b416f81

2 files changed, 62 insertions, 6 deletions

diff --git a/src/ports/SkFontHost_FreeType.cpp b/src/ports/SkFontHost_FreeType.cpp
index b2e3b4245b..55845dcadd 100644
--- a/src/ports/SkFontHost_FreeType.cpp
+++ b/src/ports/SkFontHost_FreeType.cpp
@@ -767,7 +767,7 @@ SkScalerContext_FreeType::SkScalerContext_FreeType(const SkDescriptor* desc)
     // compute the flags we send to Load_Glyph
     {
         FT_Int32 loadFlags = FT_LOAD_DEFAULT;
-        bool linearMetrics = false;
+        bool linearMetrics = SkToBool(fRec.fFlags & SkScalerContext::kSubpixelPositioning_Flag);
 
         if (SkMask::kBW_Format == fRec.fMaskFormat) {
             // See http://code.google.com/p/chromium/issues/detail?id=43252#c24
@@ -784,7 +784,6 @@ SkScalerContext_FreeType::SkScalerContext_FreeType(const SkDescriptor* desc)
                 break;
             case SkPaint::kSlight_Hinting:
                 loadFlags = FT_LOAD_TARGET_LIGHT;  // This implies FORCE_AUTOHINT
-                linearMetrics = true;
                 break;
             case SkPaint::kNormal_Hinting:
                 if (fRec.fFlags & SkScalerContext::kAutohinting_Flag)
@@ -1076,16 +1075,17 @@ void SkScalerContext_FreeType::generateMetrics(SkGlyph* glyph) {
         goto ERROR;
     }
 
-    if ((fRec.fFlags & SkScalerContext::kSubpixelPositioning_Flag) == 0) {
+    if (fDoLinearMetrics) {
+        glyph->fAdvanceX = SkFixedMul(fMatrix22.xx, fFace->glyph->linearHoriAdvance);
+        glyph->fAdvanceY = -SkFixedMul(fMatrix22.yx, fFace->glyph->linearHoriAdvance);
+    } else {
         glyph->fAdvanceX = SkFDot6ToFixed(fFace->glyph->advance.x);
         glyph->fAdvanceY = -SkFDot6ToFixed(fFace->glyph->advance.y);
+
         if (fRec.fFlags & kDevKernText_Flag) {
             glyph->fRsbDelta = SkToS8(fFace->glyph->rsb_delta);
             glyph->fLsbDelta = SkToS8(fFace->glyph->lsb_delta);
         }
-    } else {
-        glyph->fAdvanceX = SkFixedMul(fMatrix22.xx, fFace->glyph->linearHoriAdvance);
-        glyph->fAdvanceY = -SkFixedMul(fMatrix22.yx, fFace->glyph->linearHoriAdvance);
     }
 
     if ((fRec.fFlags & SkScalerContext::kVertical_Flag)
diff --git a/tests/FontHostTest.cpp b/tests/FontHostTest.cpp
index 8ab7ad3bb7..eab7bc4246 100644
--- a/tests/FontHostTest.cpp
+++ b/tests/FontHostTest.cpp
@@ -85,8 +85,64 @@ static void test_tables(skiatest::Reporter* reporter) {
     }
 }
 
+/*
+ * Verifies that the advance values returned by generateAdvance and
+ * generateMetrics match.
+ */
+static void test_advances(skiatest::Reporter* reporter) {
+    static const char* const faces[] = {
+        NULL,   // default font
+        "Arial", "Times", "Times New Roman", "Helvetica", "Courier",
+        "Courier New", "Verdana", "monospace",
+    };
+
+    static const struct {
+        SkPaint::Hinting    hinting;
+        unsigned            flags;
+    } settings[] = {
+        { SkPaint::kNo_Hinting,     0                               },
+        { SkPaint::kNo_Hinting,     SkPaint::kLinearText_Flag       },
+        { SkPaint::kNo_Hinting,     SkPaint::kSubpixelText_Flag     },
+        { SkPaint::kSlight_Hinting, 0                               },
+        { SkPaint::kSlight_Hinting, SkPaint::kLinearText_Flag       },
+        { SkPaint::kSlight_Hinting, SkPaint::kSubpixelText_Flag     },
+        { SkPaint::kNormal_Hinting, 0                               },
+        { SkPaint::kNormal_Hinting, SkPaint::kLinearText_Flag       },
+        { SkPaint::kNormal_Hinting, SkPaint::kSubpixelText_Flag     },
+    };
+
+    SkPaint paint;
+    char txt[] = "long.text.with.lots.of.dots.";
+
+    for (size_t i = 0; i < SK_ARRAY_COUNT(faces); i++) {
+        SkTypeface* face = SkTypeface::CreateFromName(faces[i], SkTypeface::kNormal);
+        paint.setTypeface(face);
+
+        for (size_t j = 0; j  < SK_ARRAY_COUNT(settings); j++) {
+             paint.setHinting(settings[j].hinting);
+             paint.setLinearText((settings[j].flags & SkPaint::kLinearText_Flag) != 0);
+             paint.setSubpixelText((settings[j].flags & SkPaint::kSubpixelText_Flag) != 0);
+
+             SkRect bounds;
+
+             // For no hinting and light hinting this should take the
+             // optimized generateAdvance path.
+             SkScalar width1 = paint.measureText(txt, strlen(txt));
+
+             // Requesting the bounds forces a generateMetrics call.
+             SkScalar width2 = paint.measureText(txt, strlen(txt), &bounds);
+
+             // SkDebugf("Font: %s, generateAdvance: %f, generateMetrics: %f\n",
+             //    faces[i], SkScalarToFloat(width1), SkScalarToFloat(width2));
+
+             REPORTER_ASSERT(reporter, width1 == width2);
+        }
+    }
+}
+
 static void TestFontHost(skiatest::Reporter* reporter) {
     test_tables(reporter);
+    test_advances(reporter);
 }
 
 // need tests for SkStrSearch