Changeset 76445 in webkit


Ignore:
Timestamp:
Jan 22, 2011 5:46:31 AM (13 years ago)
Author:
Nikolas Zimmermann
Message:

2011-01-22 Nikolas Zimmermann <​nzimmermann@rim.com>

Reviewed by Dirk Schulze.

REGRESSION: Vertical line metrics incorrect
​https://bugs.webkit.org/show_bug.cgi?id=52960

SimpleFontDataMac.mm contains a hack to modifiy lineGap/descent for the 'Hiragino' font.
That didn't influence the lineSpacing so far, but does now, causing regressions.

Restore old line spacing behaviour to fix the regression.
Covered by existing fast/blockflow, fast/repaint and fast/text/international test cases.

  • platform/graphics/FontMetrics.h: (WebCore::FontMetrics::FontMetrics): (WebCore::FontMetrics::floatLineSpacing): (WebCore::FontMetrics::setLineSpacing): (WebCore::FontMetrics::lineSpacing): (WebCore::FontMetrics::reset):
  • platform/graphics/SimpleFontData.cpp: (WebCore::SimpleFontData::SimpleFontData):
  • platform/graphics/chromium/SimpleFontDataChromiumWin.cpp: (WebCore::SimpleFontData::platformInit):
  • platform/graphics/chromium/SimpleFontDataLinux.cpp: (WebCore::SimpleFontData::platformInit):
  • platform/graphics/freetype/SimpleFontDataFreeType.cpp: (WebCore::SimpleFontData::platformInit):
  • platform/graphics/haiku/SimpleFontDataHaiku.cpp: (WebCore::SimpleFontData::platformInit):
  • platform/graphics/mac/SimpleFontDataMac.mm: (WebCore::SimpleFontData::platformInit):
  • platform/graphics/pango/SimpleFontDataPango.cpp: (WebCore::SimpleFontData::platformInit):
  • platform/graphics/qt/SimpleFontDataQt.cpp: (WebCore::SimpleFontData::platformInit):
  • platform/graphics/win/SimpleFontDataCGWin.cpp: (WebCore::SimpleFontData::platformInit):
  • platform/graphics/win/SimpleFontDataCairoWin.cpp: (WebCore::SimpleFontData::platformInit):
  • platform/graphics/win/SimpleFontDataWin.cpp: (WebCore::SimpleFontData::initGDIFont):
  • platform/graphics/wince/SimpleFontDataWinCE.cpp: (WebCore::SimpleFontData::platformInit):
  • platform/graphics/wx/SimpleFontDataWx.cpp: (WebCore::SimpleFontData::platformInit):
Location:
trunk/Source/WebCore
Files:
15 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r76444 r76445  
     12011-01-22  Nikolas Zimmermann  <nzimmermann@rim.com>
     2
     3        Reviewed by Dirk Schulze.
     4
     5        REGRESSION: Vertical line metrics incorrect
     6        https://bugs.webkit.org/show_bug.cgi?id=52960
     7
     8        SimpleFontDataMac.mm contains a hack to modifiy lineGap/descent for the 'Hiragino' font.
     9        That didn't influence the lineSpacing so far, but does now, causing regressions.
     10
     11        Restore old line spacing behaviour to fix the regression.
     12        Covered by existing fast/blockflow, fast/repaint and fast/text/international test cases.
     13
     14        * platform/graphics/FontMetrics.h:
     15        (WebCore::FontMetrics::FontMetrics):
     16        (WebCore::FontMetrics::floatLineSpacing):
     17        (WebCore::FontMetrics::setLineSpacing):
     18        (WebCore::FontMetrics::lineSpacing):
     19        (WebCore::FontMetrics::reset):
     20        * platform/graphics/SimpleFontData.cpp:
     21        (WebCore::SimpleFontData::SimpleFontData):
     22        * platform/graphics/chromium/SimpleFontDataChromiumWin.cpp:
     23        (WebCore::SimpleFontData::platformInit):
     24        * platform/graphics/chromium/SimpleFontDataLinux.cpp:
     25        (WebCore::SimpleFontData::platformInit):
     26        * platform/graphics/freetype/SimpleFontDataFreeType.cpp:
     27        (WebCore::SimpleFontData::platformInit):
     28        * platform/graphics/haiku/SimpleFontDataHaiku.cpp:
     29        (WebCore::SimpleFontData::platformInit):
     30        * platform/graphics/mac/SimpleFontDataMac.mm:
     31        (WebCore::SimpleFontData::platformInit):
     32        * platform/graphics/pango/SimpleFontDataPango.cpp:
     33        (WebCore::SimpleFontData::platformInit):
     34        * platform/graphics/qt/SimpleFontDataQt.cpp:
     35        (WebCore::SimpleFontData::platformInit):
     36        * platform/graphics/win/SimpleFontDataCGWin.cpp:
     37        (WebCore::SimpleFontData::platformInit):
     38        * platform/graphics/win/SimpleFontDataCairoWin.cpp:
     39        (WebCore::SimpleFontData::platformInit):
     40        * platform/graphics/win/SimpleFontDataWin.cpp:
     41        (WebCore::SimpleFontData::initGDIFont):
     42        * platform/graphics/wince/SimpleFontDataWinCE.cpp:
     43        (WebCore::SimpleFontData::platformInit):
     44        * platform/graphics/wx/SimpleFontDataWx.cpp:
     45        (WebCore::SimpleFontData::platformInit):
     46
    1472011-01-22  Andreas Kling  <kling@webkit.org>
    248

  • trunk/Source/WebCore/platform/graphics/FontMetrics.h

    r76442 r76445  
    3434        , m_descent(0)
    3535        , m_lineGap(0)
     36        , m_lineSpacing(0)
    3637        , m_xHeight(0)
    3738    {
    … …  
    6768    void setLineGap(float lineGap) { m_lineGap = lineGap; }
    6869
    69     float floatLineSpacing() const { return m_ascent + m_descent + m_lineGap; }
     70    float floatLineSpacing() const { return m_lineSpacing; }
     71    void setLineSpacing(float lineSpacing) { m_lineSpacing = lineSpacing; }
    7072
    7173    float xHeight() const { return m_xHeight; }
    … …  
    9395
    9496    int lineGap() const { return lroundf(m_lineGap); }
    95 
    96     int lineSpacing() const
    97     {
    98         // This mimics the old WebCore definition of lineSpacing. Changing to lroundf(m_ascent + m_descent + m_lineGap) causes lots of 1px height differences in the DRT dumps
    99         return lroundf(m_ascent) + lroundf(m_descent) + lroundf(m_lineGap);
    100     }
     97    int lineSpacing() const { return lroundf(m_lineSpacing); }
    10198
    10299private:
    … …  
    109106        m_descent = 0;
    110107        m_lineGap = 0;
     108        m_lineSpacing = 0;
    111109        m_xHeight = 0;
    112110    }
    … …  
    116114    float m_descent;
    117115    float m_lineGap;
     116    float m_lineSpacing;
    118117    float m_xHeight;
    119118};

  • trunk/Source/WebCore/platform/graphics/SimpleFontData.cpp

    r76442 r76445  
    8989    m_fontMetrics.setDescent(descent);
    9090    m_fontMetrics.setLineGap(lineGap);
     91    m_fontMetrics.setLineSpacing(ascent + descent + lineGap);
    9192    m_fontMetrics.setXHeight(xHeight);
    9293

  • trunk/Source/WebCore/platform/graphics/chromium/SimpleFontDataChromiumWin.cpp

    r76442 r76445  
    9898    m_fontMetrics.setLineGap(lineGap);
    9999    m_fontMetrics.setXHeight(xHeight);
     100    m_fontMetrics.setLineSpacing(ascent + descent + lineGap);
    100101
    101102    SelectObject(dc, oldFont);

  • trunk/Source/WebCore/platform/graphics/chromium/SimpleFontDataLinux.cpp

    r76442 r76445  
    110110    }
    111111
    112     m_fontMetrics.setLineGap(SkScalarToFloat(metrics.fLeading));
     112    float lineGap = SkScalarToFloat(metrics.fLeading);
     113    m_fontMetrics.setLineGap(lineGap);
    113114    m_fontMetrics.setXHeight(xHeight);
     115    m_fontMetrics.setLineSpacing(lroundf(ascent) + lroundf(descent) + lroundf(lineGap));
    114116
    115117    if (m_orientation == Vertical) {

  • trunk/Source/WebCore/platform/graphics/freetype/SimpleFontDataFreeType.cpp

    r76442 r76445  
    6363    if (lineSpacing < font_extents.ascent + font_extents.descent)
    6464        lineSpacing = font_extents.ascent + font_extents.descent;
     65
     66    m_fontMetrics.setLineSpacing(lroundf(lineSpacing));
    6567    m_fontMetrics.setLineGap(lineSpacing - font_extents.ascent - font_extents.descent);
    6668

  • trunk/Source/WebCore/platform/graphics/haiku/SimpleFontDataHaiku.cpp

    r76442 r76445  
    5353    m_fontMetrics.setXHeight(height.ascent * 0.56f); // Hack taken from the win port.
    5454    m_fontMetrics.setLineGap(height.leading);
     55    m_fontMetrics.setLineSpacing(lroundf(height.ascent) + lroundf(height.descent) + lroundf(height.leading));
    5556}
    5657

  • trunk/Source/WebCore/platform/graphics/mac/SimpleFontDataMac.mm

    r76442 r76445  
    250250    }
    251251
     252    // Compute and store line spacing, before the line metrics hacks are applied.
     253    m_fontMetrics.setLineSpacing(lroundf(ascent) + lroundf(descent) + lroundf(lineGap));
     254
    252255    // Hack Hiragino line metrics to allow room for marked text underlines.
    253256    // <rdar://problem/5386183>

  • trunk/Source/WebCore/platform/graphics/pango/SimpleFontDataPango.cpp

    r76442 r76445  
    6262    if (lineSpacing < font_extents.ascent + font_extents.descent)
    6363        lineSpacing = font_extents.ascent + font_extents.descent;
     64    m_fontMetrics.setLineSpacing(lroundf(lineSpacing));
    6465    m_fontMetrics.setLineGap(lineSpacing - font_extents.ascent - font_extents.descent);
    6566

  • trunk/Source/WebCore/platform/graphics/qt/SimpleFontDataQt.cpp

    r76442 r76445  
    5353    m_fontMetrics.setXHeight(fm.xHeight());
    5454    m_fontMetrics.setLineGap(fm.leading());
     55    m_fontMetrics.setLineSpacing(fm.lineSpacing());
    5556    m_spaceWidth = fm.width(QLatin1Char(' '));
    5657}

  • trunk/Source/WebCore/platform/graphics/win/SimpleFontDataCGWin.cpp

    r76442 r76445  
    9797    m_fontMetrics.setDescent(fDescent);
    9898    m_fontMetrics.setLineGap(fLineGap);
     99    m_fontMetrics.setLineSpacing(lroundf(fAscent) + lroundf(fDescent) + lroundf(fLineGap));
    99100
    100101    // Measure the actual character "x", because AppKit synthesizes X height rather than getting it from the font.

  • trunk/Source/WebCore/platform/graphics/win/SimpleFontDataCairoWin.cpp

    r76442 r76445  
    7171    m_fontMetrics.setDescent(descent);
    7272    m_fontMetrics.setLineGap(lineGap);
     73    m_fontMetrics.setLineSpacing(lroundf(ascent) + lroundf(descent) + lroundf(lineGap));
    7374    m_avgCharWidth = textMetrics.tmAveCharWidth * metricsMultiplier;
    7475    m_maxCharWidth = textMetrics.tmMaxCharWidth * metricsMultiplier;

  • trunk/Source/WebCore/platform/graphics/win/SimpleFontDataWin.cpp

    r76442 r76445  
    8484     m_fontMetrics.setDescent(descent);
    8585     m_fontMetrics.setLineGap(lineGap);
     86     m_fontMetrics.setLineSpacing(lroundf(ascent) + lroundf(descent) + lroundf(lineGap));
    8687     m_avgCharWidth = textMetrics.tmAveCharWidth;
    8788     m_maxCharWidth = textMetrics.tmMaxCharWidth;

  • trunk/Source/WebCore/platform/graphics/wince/SimpleFontDataWinCE.cpp

    r76442 r76445  
    5858    m_fontMetrics.setDescent(descent);
    5959    m_fontMetrics.setLineGap(lineGap);
     60    m_fontMetrics.setLineSpacing(lroundf(ascent) + lroundf(descent) + lroundf(lineGap));
    6061    m_fontMetrics.setXHeight(ascent * 0.56f);
    6162}

  • trunk/Source/WebCore/platform/graphics/wx/SimpleFontDataWx.cpp

    r76442 r76445  
    5959        m_fontMetrics.setUnitsPerEm(1); // FIXME!
    6060        m_fontMetrics.setLineGap(props.GetLineGap());
     61        m_fontMetrics.setLineSpacing(props.GetLineSpacing());
    6162    }
    6263
Note: See TracChangeset for help on using the changeset viewer.