Don't check lfQuality in LOGFONT as it has no effect on rendering

Source/platform/fonts/win/FontPlatformDataWin.cpp

Index: Source/platform/fonts/win/FontPlatformDataWin.cpp
diff --git a/Source/platform/fonts/win/FontPlatformDataWin.cpp b/Source/platform/fonts/win/FontPlatformDataWin.cpp
index 0391291ac2aa9d2982abd7cd1ce796325541c514..40b8cc2cf0bc3e8ea671171ba3fe2554961cfda5 100644
--- a/Source/platform/fonts/win/FontPlatformDataWin.cpp
+++ b/Source/platform/fonts/win/FontPlatformDataWin.cpp
@@ -85,7 +85,7 @@ void FontPlatformData::setupPaint(SkPaint* paint, GraphicsContext* context) cons
 // Lookup the current system settings for font smoothing.
 // We cache these values for performance, but if the browser has a way to be
 // notified when these change, we could re-query them at that time.
-static uint32_t getDefaultGDITextFlags()
+static uint32_t getSystemTextFlags()

[bungeman-chromium, 2013/12/09 15:29:58]

I have no real issue with this renaming, but note that DirectWrite actually uses
a different set of settings (which can actually vary per monitor/display).

 {
     static bool gInited;
     static uint32_t gFlags;
@@ -106,52 +106,29 @@ static uint32_t getDefaultGDITextFlags()
     return gFlags;
 }
 
-static bool isWebFont(const LOGFONT& lf)
+static bool isWebFont(const String& familyName)
 {
-    // web-fonts have artifical names constructed to always be
+    // Web-fonts have artifical names constructed to always be:
     // 1. 24 characters, followed by a '\0'
     // 2. the last two characters are '=='
-    return '=' == lf.lfFaceName[22] && '=' == lf.lfFaceName[23] && '\0' == lf.lfFaceName[24];
+    return familyName.length() == 24
+        && '=' == familyName[22] && '=' == familyName[23];
 }
 
-static int computePaintTextFlags(const LOGFONT& lf)
+int FontPlatformData::paintTextFlags() const
 {
-    int textFlags = 0;
-    switch (lf.lfQuality) {
-    case NONANTIALIASED_QUALITY:
-        textFlags = 0;
-        break;
-    case ANTIALIASED_QUALITY:
-        textFlags = SkPaint::kAntiAlias_Flag;
-        break;
-    case CLEARTYPE_QUALITY:
-        textFlags = (SkPaint::kAntiAlias_Flag | SkPaint::kLCDRenderText_Flag);
-        break;
-    default:
-        textFlags = getDefaultGDITextFlags();
-        break;
-    }
+    int textFlags = getSystemTextFlags();
 
-    // only allow features that SystemParametersInfo allows
-    textFlags &= getDefaultGDITextFlags();
-
-    /*
-     *  FontPlatformData(...) will read our logfont, and try to honor the the lfQuality
-     *  setting (computing the corresponding SkPaint flags for AA and LCD). However, it
-     *  will limit the quality based on its query of SPI_GETFONTSMOOTHING. This could mean
-     *  we end up drawing the text in BW, even though our lfQuality requested antialiasing.
-     *
-     *  Many web-fonts are so poorly hinted that they are terrible to read when drawn in BW.
-     *  In these cases, we have decided to FORCE these fonts to be drawn with at least grayscale AA,
-     *  even when the System (getDefaultGDITextFlags) tells us to draw only in BW.
-     */
-    if (isWebFont(lf) && !isRunningLayoutTest())
+    // Many web-fonts are so poorly hinted that they are terrible to read when drawn in BW.
+    // In these cases, we have decided to FORCE these fonts to be drawn with at least grayscale AA,
+    // even when the System (getSystemTextFlags) tells us to draw only in BW.
+    if (isWebFont(fontFamilyName()) && !isRunningLayoutTest())

[bungeman-chromium, 2014/02/10 20:59:57]

Hopefully getting the family name here doesn't take too long. It seems like
before we were going off the logfont data which was already in hand as opposed
to going off and finding the name of the font.

[bungeman-skia, 2014/02/10 23:16:15]

Yep, this is it. Doing the work of actually digging out the name each time we
create the paint (basically every measure and draw for each bit of text) is
taking up all of the time. I ran

python tools/perf/run_benchmark -v --browser=release rasterize_and_record.top_25

locally at tip of tree with and without this 'if' (and the line below) commented
out. When commented out, the record_time is cut in half.

         textFlags |= SkPaint::kAntiAlias_Flag;
     return textFlags;
 }
 
 #if !USE(HARFBUZZ)
-PassRefPtr<SkTypeface> CreateTypefaceFromHFont(HFONT hfont, int* size, int* paintTextFlags)
+PassRefPtr<SkTypeface> CreateTypefaceFromHFont(HFONT hfont, int* size)
 {
     LOGFONT info;
     GetObject(hfont, sizeof(info), &info);
@@ -161,8 +138,6 @@ PassRefPtr<SkTypeface> CreateTypefaceFromHFont(HFONT hfont, int* size, int* pain
             height = -height;
         *size = height;
     }
-    if (paintTextFlags)
-        *paintTextFlags = computePaintTextFlags(info);
     return adoptRef(SkCreateTypefaceFromLOGFONT(info));
 }
 #endif
@@ -173,7 +148,6 @@ FontPlatformData::FontPlatformData(WTF::HashTableDeletedValueType)
     , m_fakeItalic(false)
     , m_orientation(Horizontal)
     , m_typeface(adoptRef(SkTypeface::RefDefault()))
-    , m_paintTextFlags(0)
     , m_isHashTableDeletedValue(true)
     , m_useSubpixelPositioning(false)
 {
@@ -189,7 +163,6 @@ FontPlatformData::FontPlatformData()
     , m_fakeItalic(false)
     , m_orientation(Horizontal)
     , m_typeface(adoptRef(SkTypeface::RefDefault()))
-    , m_paintTextFlags(0)

[bungeman-chromium, 2014/02/10 20:59:57]

It seems like the paintTextFlags were often 0 (monochrome) in this older code.
This isn't true in the new code, not sure if that affects anything.

     , m_isHashTableDeletedValue(false)
     , m_useSubpixelPositioning(false)
 {
@@ -207,7 +180,7 @@ FontPlatformData::FontPlatformData(HFONT font, float size, FontOrientation orien
     , m_fakeItalic(false)
     , m_orientation(orientation)
     , m_scriptCache(0)
-    , m_typeface(CreateTypefaceFromHFont(font, 0, &m_paintTextFlags))
+    , m_typeface(CreateTypefaceFromHFont(font, 0))
     , m_isHashTableDeletedValue(false)
     , m_useSubpixelPositioning(false)
 {
@@ -221,7 +194,6 @@ FontPlatformData::FontPlatformData(float size, bool bold, bool oblique)
     , m_fakeItalic(false)
     , m_orientation(Horizontal)
     , m_typeface(adoptRef(SkTypeface::RefDefault()))
-    , m_paintTextFlags(0)
     , m_isHashTableDeletedValue(false)
     , m_useSubpixelPositioning(false)
 {
@@ -237,7 +209,6 @@ FontPlatformData::FontPlatformData(const FontPlatformData& data)
     , m_fakeItalic(data.m_fakeItalic)
     , m_orientation(data.m_orientation)
     , m_typeface(data.m_typeface)
-    , m_paintTextFlags(data.m_paintTextFlags)
     , m_isHashTableDeletedValue(false)
     , m_useSubpixelPositioning(data.m_useSubpixelPositioning)
 {
@@ -253,7 +224,6 @@ FontPlatformData::FontPlatformData(const FontPlatformData& data, float textSize)
     , m_fakeItalic(data.m_fakeItalic)
     , m_orientation(data.m_orientation)
     , m_typeface(data.m_typeface)
-    , m_paintTextFlags(data.m_paintTextFlags)
     , m_isHashTableDeletedValue(false)
     , m_useSubpixelPositioning(data.m_useSubpixelPositioning)
 {
@@ -279,7 +249,6 @@ FontPlatformData::FontPlatformData(PassRefPtr<SkTypeface> tf, const char* family
     LOGFONT logFont;
     SkLOGFONTFromTypeface(m_typeface.get(), &logFont);
     logFont.lfHeight = -textSize;
-    m_paintTextFlags = computePaintTextFlags(logFont);

[bungeman-chromium, 2014/02/10 20:59:57]

Note that here 'logFont' would always be CLEARTYPE_QUALITY since that's the
canonical quality. The typeface itself doesn't care how it would be rendered,
that's up to a paint.

Note that after this line is removed, the previous three lines may be moved into
the #if below, since 'logFont' is no longer being used here. (I suppose that
means the FIXME can be fixed.)

 
 #if !USE(HARFBUZZ)
     HFONT hFont = CreateFontIndirect(&logFont);
@@ -296,7 +265,6 @@ FontPlatformData& FontPlatformData::operator=(const FontPlatformData& data)
         m_fakeItalic = data.m_fakeItalic;
         m_orientation = data.m_orientation;
         m_typeface = data.m_typeface;
-        m_paintTextFlags = data.m_paintTextFlags;
 
 #if !USE(HARFBUZZ)
         m_font = data.m_font;