Changeset 85013 in webkit


Ignore:
Timestamp:
Apr 26, 2011 9:49:56 PM (13 years ago)
Author:
commit-queue@webkit.org
Message:

2011-04-26 Kenichi Ishibashi <​bashi@chromium.org>

Reviewed by Tony Chang.

[Chromium] Vertical positions are off for some Arabic glyphs on Linux
​https://bugs.webkit.org/show_bug.cgi?id=59182

Add a test for checking vertical offsets of Arabic script shaping.
The font which is used in the test was created from scratch.

  • platform/chromium-linux/fast/text/international/arabic-vertical-offset-expected.png: Added.
  • platform/chromium-linux/fast/text/international/arabic-vertical-offset-expected.txt: Added.
  • platform/chromium-linux/fast/text/international/arabic-vertical-offset.html: Added.
  • platform/chromium-linux/fast/text/international/resources/font-for-arabic-offset-test.ttf: Added.

2011-04-26 Kenichi Ishibashi <​bashi@chromium.org>

Reviewed by Tony Chang.

[Chromium] Vertical positions are off for some Arabic glyphs on Linux
​https://bugs.webkit.org/show_bug.cgi?id=59182

Use vertical offsets of the shaping results.

Test: platform/chromium-linux/fast/text/international/arabic-vertical-offset.html

  • platform/graphics/chromium/ComplexTextControllerLinux.cpp: (WebCore::ComplexTextController::ComplexTextController): Added initialization of m_startingY. (WebCore::ComplexTextController::nextScriptRun): Followed the change in handling positions. (WebCore::ComplexTextController::deleteGlyphArrays): Ditto. (WebCore::ComplexTextController::createGlyphArrays): Ditto. (WebCore::ComplexTextController::resetGlyphArrays): Ditto. (WebCore::ComplexTextController::setGlyphPositions): Changed to use vertical offsets as same as horizontal offsets.
  • platform/graphics/chromium/ComplexTextControllerLinux.h: Removed m_xPositions and Added m_positions and m_startingY. (WebCore::ComplexTextController::positions): Added.
  • platform/graphics/chromium/FontLinux.cpp: Followed the change in ComplexTextController. (WebCore::Font::drawComplexText): Ditto. (WebCore::Font::floatWidthForComplexText): Ditto. (WebCore::glyphIndexForXPositionInScriptRun): Ditto. (WebCore::Font::offsetForPositionForComplexText): Ditto. (WebCore::Font::selectionRectForComplexText): Ditto.
Location:
trunk
Files:
5 added
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r85012 r85013  
     12011-04-26  Kenichi Ishibashi  <bashi@chromium.org>
     2
     3        Reviewed by Tony Chang.
     4
     5        [Chromium] Vertical positions are off for some Arabic glyphs on Linux
     6        https://bugs.webkit.org/show_bug.cgi?id=59182
     7
     8        Add a test for checking vertical offsets of Arabic script shaping.
     9        The font which is used in the test was created from scratch.
     10
     11        * platform/chromium-linux/fast/text/international/arabic-vertical-offset-expected.png: Added.
     12        * platform/chromium-linux/fast/text/international/arabic-vertical-offset-expected.txt: Added.
     13        * platform/chromium-linux/fast/text/international/arabic-vertical-offset.html: Added.
     14        * platform/chromium-linux/fast/text/international/resources/font-for-arabic-offset-test.ttf: Added.
     15
    1162011-04-26  Tony Chang  <tony@chromium.org>
    217

  • trunk/Source/WebCore/ChangeLog

    r85011 r85013  
     12011-04-26  Kenichi Ishibashi  <bashi@chromium.org>
     2
     3        Reviewed by Tony Chang.
     4
     5        [Chromium] Vertical positions are off for some Arabic glyphs on Linux
     6        https://bugs.webkit.org/show_bug.cgi?id=59182
     7
     8        Use vertical offsets of the shaping results.
     9
     10        Test: platform/chromium-linux/fast/text/international/arabic-vertical-offset.html
     11
     12        * platform/graphics/chromium/ComplexTextControllerLinux.cpp:
     13        (WebCore::ComplexTextController::ComplexTextController):
     14        Added initialization of m_startingY.
     15        (WebCore::ComplexTextController::nextScriptRun):
     16        Followed the change in handling positions.
     17        (WebCore::ComplexTextController::deleteGlyphArrays): Ditto.
     18        (WebCore::ComplexTextController::createGlyphArrays): Ditto.
     19        (WebCore::ComplexTextController::resetGlyphArrays): Ditto.
     20        (WebCore::ComplexTextController::setGlyphPositions):
     21        Changed to use vertical offsets as same as horizontal offsets.
     22        * platform/graphics/chromium/ComplexTextControllerLinux.h:
     23        Removed m_xPositions and Added m_positions and m_startingY.
     24        (WebCore::ComplexTextController::positions): Added.
     25        * platform/graphics/chromium/FontLinux.cpp:
     26        Followed the change in ComplexTextController.
     27        (WebCore::Font::drawComplexText): Ditto.
     28        (WebCore::Font::floatWidthForComplexText): Ditto.
     29        (WebCore::glyphIndexForXPositionInScriptRun): Ditto.
     30        (WebCore::Font::offsetForPositionForComplexText): Ditto.
     31        (WebCore::Font::selectionRectForComplexText): Ditto.
     32
    1332011-04-26  Levi Weintraub  <leviw@chromium.org>
    234

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

    r77785 r85013  
    4747}
    4848
    49 ComplexTextController::ComplexTextController(const TextRun& run, unsigned startingX, const Font* font)
     49ComplexTextController::ComplexTextController(const TextRun& run, unsigned startingX, unsigned startingY, const Font* font)
    5050    : m_font(font)
    5151    , m_run(getNormalizedTextRun(run, m_normalizedRun, m_normalizedBuffer))
    … …  
    7676
    7777    reset(startingX);
     78    m_startingY = startingY;
    7879}
    7980
    … …  
    172173    setupFontForScriptRun();
    173174    shapeGlyphs();
    174     setGlyphXPositions(rtl());
     175    setGlyphPositions(rtl());
    175176
    176177    return true;
    … …  
    234235    delete[] m_item.offsets;
    235236    delete[] m_glyphs16;
    236     delete[] m_xPositions;
     237    delete[] m_positions;
    237238}
    238239
    … …  
    245246
    246247    m_glyphs16 = new uint16_t[size];
    247     m_xPositions = new SkScalar[size];
     248    m_positions = new SkPoint[size];
    248249
    249250    m_item.num_glyphs = size;
    … …  
    262263    memset(m_item.offsets, 0, size * sizeof(HB_FixedPoint));
    263264    memset(m_glyphs16, 0, size * sizeof(uint16_t));
    264     memset(m_xPositions, 0, size * sizeof(SkScalar));
     265    memset(m_positions, 0, size * sizeof(SkPoint));
    265266}
    266267
    … …  
    285286}
    286287
    287 void ComplexTextController::setGlyphXPositions(bool isRTL)
     288void ComplexTextController::setGlyphPositions(bool isRTL)
    288289{
    289290    const double rtlFlip = isRTL ? -1 : 1;
    … …  
    305306        m_glyphs16[i] = m_item.glyphs[i];
    306307        double offsetX = truncateFixedPointToInteger(m_item.offsets[i].x);
     308        double offsetY = truncateFixedPointToInteger(m_item.offsets[i].y);
    307309        double advance = truncateFixedPointToInteger(m_item.advances[i]);
    308310        if (isRTL)
    309311            offsetX -= advance;
    310312
    311         m_xPositions[i] = m_offsetX + (position * rtlFlip) + offsetX;
     313        m_positions[i].set(m_offsetX + (position * rtlFlip) + offsetX,
     314                           m_startingY + offsetY);
    312315
    313316        if (m_currentFontData->isZeroWidthSpaceGlyph(m_glyphs16[i]))

  • trunk/Source/WebCore/platform/graphics/chromium/ComplexTextControllerLinux.h

    r76732 r85013  
    3333
    3434#include "HarfbuzzSkia.h"
     35#include "SkPoint.h"
    3536#include "SkScalar.h"
    3637#include "TextRun.h"
    … …  
    6162class ComplexTextController {
    6263public:
    63     ComplexTextController(const TextRun&, unsigned, const Font*);
     64    ComplexTextController(const TextRun&, unsigned, unsigned, const Font*);
    6465    ~ComplexTextController();
    6566
    … …  
    9394    const unsigned length() const { return m_item.num_glyphs; }
    9495
    95     // Return the x offset for each of the glyphs. Note that this is translated
     96    // Return the offset for each of the glyphs. Note that this is translated
    9697    // by the current x offset and that the x offset is updated for each script
    9798    // run.
    98     const SkScalar* xPositions() const { return m_xPositions; }
     99    const SkPoint* positions() const { return m_positions; }
    99100
    100101    // Get the advances (widths) for each glyph.
    … …  
    126127    void resetGlyphArrays();
    127128    void shapeGlyphs();
    128     void setGlyphXPositions(bool);
     129    void setGlyphPositions(bool);
    129130
    130131    static void normalizeSpacesAndMirrorChars(const UChar* source, bool rtl, UChar* destination, int length);
    … …  
    138139    HB_ShaperItem m_item;
    139140    uint16_t* m_glyphs16; // A vector of 16-bit glyph ids.
    140     SkScalar* m_xPositions; // A vector of x positions for each glyph.
     141    SkPoint* m_positions; // A vector of positions for each glyph.
    141142    ssize_t m_indexOfNextScriptRun; // Indexes the script run in |m_run|.
    142143    unsigned m_offsetX; // Offset in pixels to the start of the next script run.
     144    unsigned m_startingY; // The Y starting point of the script run.
    143145    unsigned m_pixelWidth; // Width (in px) of the current script run.
    144146    unsigned m_glyphsArrayCapacity; // Current size of all the Harfbuzz arrays.

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

    r80610 r85013  
    207207    }
    208208
    209     ComplexTextController controller(run, point.x(), this);
     209    ComplexTextController controller(run, point.x(), point.y(), this);
    210210    controller.setWordSpacingAdjustment(wordSpacing());
    211211    controller.setLetterSpacingAdjustment(letterSpacing());
    … …  
    226226            controller.fontPlatformDataForScriptRun()->setupPaint(&fillPaint);
    227227            adjustTextRenderMode(&fillPaint, gc->platformContext());
    228             canvas->drawPosTextH(controller.glyphs(), controller.length() << 1, controller.xPositions(), point.y(), fillPaint);
     228            canvas->drawPosText(controller.glyphs(), controller.length() << 1, controller.positions(), fillPaint);
    229229        }
    230230
    … …  
    232232            controller.fontPlatformDataForScriptRun()->setupPaint(&strokePaint);
    233233            adjustTextRenderMode(&strokePaint, gc->platformContext());
    234             canvas->drawPosTextH(controller.glyphs(), controller.length() << 1, controller.xPositions(), point.y(), strokePaint);
     234            canvas->drawPosText(controller.glyphs(), controller.length() << 1, controller.positions(), strokePaint);
    235235        }
    236236    }
    … …  
    244244float Font::floatWidthForComplexText(const TextRun& run, HashSet<const SimpleFontData*>* /* fallbackFonts */, GlyphOverflow* /* glyphOverflow */) const
    245245{
    246     ComplexTextController controller(run, 0, this);
     246    ComplexTextController controller(run, 0, 0, this);
    247247    controller.setWordSpacingAdjustment(wordSpacing());
    248248    controller.setLetterSpacingAdjustment(letterSpacing());
    … …  
    259259    for (int glyphIndex = 0; static_cast<unsigned>(glyphIndex) < controller.length(); ++glyphIndex) {
    260260        int advance = truncateFixedPointToInteger(controller.advances()[glyphIndex]);
    261         int nextX = static_cast<int>(controller.xPositions()[glyphIndex]) + advance / 2;
     261        int nextX = static_cast<int>(controller.positions()[glyphIndex].x()) + advance / 2;
    262262        if (std::min(nextX, lastX) <= targetX && targetX <= std::max(nextX, lastX))
    263263            return glyphIndex;
    … …  
    278278    // (Mac code ignores includePartialGlyphs, and they don't know what it's
    279279    // supposed to do, so we just ignore it as well.)
    280     ComplexTextController controller(run, 0, this);
     280    ComplexTextController controller(run, 0, 0, this);
    281281    controller.setWordSpacingAdjustment(wordSpacing());
    282282    controller.setLetterSpacingAdjustment(letterSpacing());
    … …  
    329329{
    330330    int fromX = -1, toX = -1;
    331     ComplexTextController controller(run, 0, this);
     331    ComplexTextController controller(run, 0, 0, this);
    332332    controller.setWordSpacingAdjustment(wordSpacing());
    333333    controller.setLetterSpacingAdjustment(letterSpacing());
    … …  
    346346            // position.
    347347            int glyph = controller.logClusters()[from];
    348             fromX = controller.xPositions()[glyph];
     348            fromX = controller.positions()[glyph].x();
    349349            if (controller.rtl())
    350350                fromX += truncateFixedPointToInteger(controller.advances()[glyph]);
    … …  
    354354        if (toX == -1 && to >= 0 && static_cast<unsigned>(to) < controller.numCodePoints()) {
    355355            int glyph = controller.logClusters()[to];
    356             toX = controller.xPositions()[glyph];
     356            toX = controller.positions()[glyph].x();
    357357            if (controller.rtl())
    358358                toX += truncateFixedPointToInteger(controller.advances()[glyph]);
Note: See TracChangeset for help on using the changeset viewer.