Changeset 211382 in webkit


Ignore:
Timestamp:
Jan 30, 2017, 12:43:14 PM (8 years ago)
Author:
mmaxfield@apple.com
Message:

Correct spacing regression on inter-element complex path shaping on some fonts
​https://bugs.webkit.org/show_bug.cgi?id=166013

Reviewed by Simon Fraser.

Source/WebCore:

This patch brings the implementation of ComplexTextController in-line with the
design at ​https://trac.webkit.org/wiki/ComplexTextController. Previously,
ComplexTextController had a few problems:

you iterated over the entire string and added up the advances

  • FontCascade::getGlyphsAndAdvancesForComplexText() tried to compensate for

the above by construing the concepts of paint advances as distinct from layout
advances

  • Initial advances were considered part of layout sometimes and part of painting

other times, depending on which function reports the information

  • For RTL runs, the wrong origin was added to the initial advance, and the origin

should have been subtracted instead of added

This patch exhaustively updates every function in ComplexTextController to be
consistent with the design linked to above. This design solves all of these
problems.

Tests: ComplexTextControllerTest.InitialAdvanceWithLeftRunInRTL

ComplexTextControllerTest.InitialAdvanceInRTL
ComplexTextControllerTest.InitialAdvanceWithLeftRunInLTR
ComplexTextControllerTest.InitialAdvanceInLTR
ComplexTextControllerTest.InitialAdvanceInRTLNoOrigins
ComplexTextControllerTest.LeadingExpansion
ComplexTextControllerTest.VerticalAdvances

  • platform/graphics/GlyphBuffer.h:

(WebCore::GlyphBuffer::setLeadingExpansion): Deleted. No longer necessary.
(WebCore::GlyphBuffer::leadingExpansion): Deleted. Ditto.

  • platform/graphics/cocoa/FontCascadeCocoa.mm:

(WebCore::FontCascade::adjustSelectionRectForComplexText): Removed use of
unnecessary leadingExpansion().
(WebCore::FontCascade::getGlyphsAndAdvancesForComplexText): This function needs
to compute paint advances, which means that it can't base this information off
of layout advances. This function uses the trick mentioned at the end of the
above link to compute the paint offset of an arbitrary glyph in the middle of
an RTL run.

  • platform/graphics/mac/ComplexTextController.cpp:

(WebCore::ComplexTextController::computeExpansionOpportunity): Refactored for
testing.
(WebCore::ComplexTextController::ComplexTextController): Ditto.
(WebCore::ComplexTextController::finishConstruction): Ditto.
(WebCore::ComplexTextController::offsetForPosition): This function operates on
layout advances, and the initial layout advance is already added into the
m_adjustedBaseAdvances vector by adjustGlyphsAndAdvances(). Therefore, there is
no need to add it again here.
(WebCore::ComplexTextController::advance): This function had completely busted
logic about the relationship between initial advances and the first origin in
each run. Because of the fortunate choice of only representing layout advances
in m_adjustedBaseAdvances, this entire block can be removed and the raw paint
initial advance can be reported to the GlyphBuffer. Later in the function, we
have to update the logic about how to compute a paint advance given a layout
advance and some origins. In particular, there are two tricky pieces here: 1.
The layout advance for the first glyph is equal to (initial advance - first
origin + first Core Text advance, so computing the paint offset must cancel out
the initial layout offset, and 2. the last paint advance in a run must actually
end up at the position of the first glyph in the next run, so the next run's
initial advance must be queried.
(WebCore::ComplexTextController::adjustGlyphsAndAdvances): Previously, we
represented an initial advance of a successive run by just adding it to the
previous run's last advance. However, this is incompatible with the new model
presented in the link above, so we remove this section. We also have to add in
the logic that the layout advance for the first glyph is equal to the formula
presented above.

  • platform/graphics/mac/ComplexTextController.h:

(WebCore::ComplexTextController::ComplexTextRun::initialAdvance): Adjust comment
to reflect reality.
(WebCore::ComplexTextController::leadingExpansion): Deleted.

Tools:

Unskip existing tests and make some new tests:

  • Testing complex text with no origins
  • Testing initial expansions
  • Testing the sign of vertical advances
  • TestWebKitAPI/Tests/WebCore/ComplexTextController.cpp:

(TestWebKitAPI::TEST_F):

Location:
trunk
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r211379 r211382  
     12017-01-30  Myles C. Maxfield  <mmaxfield@apple.com>
     2
     3        Correct spacing regression on inter-element complex path shaping on some fonts
     4        https://bugs.webkit.org/show_bug.cgi?id=166013
     5
     6        Reviewed by Simon Fraser.
     7
     8        This patch brings the implementation of ComplexTextController in-line with the
     9        design at https://trac.webkit.org/wiki/ComplexTextController. Previously,
     10        ComplexTextController had a few problems:
     11        - The total width computed by ComplexTextController didn't match the width if
     12        you iterated over the entire string and added up the advances
     13        - FontCascade::getGlyphsAndAdvancesForComplexText() tried to compensate for
     14        the above by construing the concepts of paint advances as distinct from layout
     15        advances
     16        - Initial advances were considered part of layout sometimes and part of painting
     17        other times, depending on which function reports the information
     18        - For RTL runs, the wrong origin was added to the initial advance, and the origin
     19        should have been subtracted instead of added
     20
     21        This patch exhaustively updates every function in ComplexTextController to be
     22        consistent with the design linked to above. This design solves all of these
     23        problems.
     24
     25        Tests: ComplexTextControllerTest.InitialAdvanceWithLeftRunInRTL
     26               ComplexTextControllerTest.InitialAdvanceInRTL
     27               ComplexTextControllerTest.InitialAdvanceWithLeftRunInLTR
     28               ComplexTextControllerTest.InitialAdvanceInLTR
     29               ComplexTextControllerTest.InitialAdvanceInRTLNoOrigins
     30               ComplexTextControllerTest.LeadingExpansion
     31               ComplexTextControllerTest.VerticalAdvances
     32
     33        * platform/graphics/GlyphBuffer.h:
     34        (WebCore::GlyphBuffer::setLeadingExpansion): Deleted. No longer necessary.
     35        (WebCore::GlyphBuffer::leadingExpansion): Deleted. Ditto.
     36        * platform/graphics/cocoa/FontCascadeCocoa.mm:
     37        (WebCore::FontCascade::adjustSelectionRectForComplexText): Removed use of
     38        unnecessary leadingExpansion().
     39        (WebCore::FontCascade::getGlyphsAndAdvancesForComplexText): This function needs
     40        to compute paint advances, which means that it can't base this information off
     41        of layout advances. This function uses the trick mentioned at the end of the
     42        above link to compute the paint offset of an arbitrary glyph in the middle of
     43        an RTL run.
     44        * platform/graphics/mac/ComplexTextController.cpp:
     45        (WebCore::ComplexTextController::computeExpansionOpportunity): Refactored for
     46        testing.
     47        (WebCore::ComplexTextController::ComplexTextController): Ditto.
     48        (WebCore::ComplexTextController::finishConstruction): Ditto.
     49        (WebCore::ComplexTextController::offsetForPosition): This function operates on
     50        layout advances, and the initial layout advance is already added into the
     51        m_adjustedBaseAdvances vector by adjustGlyphsAndAdvances(). Therefore, there is
     52        no need to add it again here.
     53        (WebCore::ComplexTextController::advance): This function had completely busted
     54        logic about the relationship between initial advances and the first origin in
     55        each run. Because of the fortunate choice of only representing layout advances
     56        in m_adjustedBaseAdvances, this entire block can be removed and the raw paint
     57        initial advance can be reported to the GlyphBuffer. Later in the function, we
     58        have to update the logic about how to compute a paint advance given a layout
     59        advance and some origins. In particular, there are two tricky pieces here: 1.
     60        The layout advance for the first glyph is equal to (initial advance - first
     61        origin + first Core Text advance, so computing the paint offset must cancel out
     62        the initial layout offset, and 2. the last paint advance in a run must actually
     63        end up at the position of the first glyph in the next run, so the next run's
     64        initial advance must be queried.
     65        (WebCore::ComplexTextController::adjustGlyphsAndAdvances): Previously, we
     66        represented an initial advance of a successive run by just adding it to the
     67        previous run's last advance. However, this is incompatible with the new model
     68        presented in the link above, so we remove this section. We also have to add in
     69        the logic that the layout advance for the first glyph is equal to the formula
     70        presented above.
     71        * platform/graphics/mac/ComplexTextController.h:
     72        (WebCore::ComplexTextController::ComplexTextRun::initialAdvance): Adjust comment
     73        to reflect reality.
     74        (WebCore::ComplexTextController::leadingExpansion): Deleted.
     75
    1762017-01-30  Simon Fraser  <simon.fraser@apple.com>
    277

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

    r206597 r211382  
    103103    void setInitialAdvance(GlyphBufferAdvance initialAdvance) { m_initialAdvance = initialAdvance; }
    104104    const GlyphBufferAdvance& initialAdvance() const { return m_initialAdvance; }
    105 
    106     void setLeadingExpansion(float leadingExpansion) { m_leadingExpansion = leadingExpansion; }
    107     float leadingExpansion() const { return m_leadingExpansion; }
    108105   
    109106    Glyph glyphAt(unsigned index) const
    … …  
    249246    Vector<FloatSize, 2048> m_offsets;
    250247#endif
    251     float m_leadingExpansion;
    252248};
    253249

  • trunk/Source/WebCore/platform/graphics/cocoa/FontCascadeCocoa.mm

    r208460 r211382  
    498498
    499499    if (run.rtl())
    500         selectionRect.move(controller.totalWidth() - afterWidth + controller.leadingExpansion(), 0);
     500        selectionRect.move(controller.totalWidth() - afterWidth, 0);
    501501    else
    502502        selectionRect.move(beforeWidth, 0);
    … …  
    509509
    510510    ComplexTextController controller(*this, run, false, 0, forTextEmphasis);
    511     controller.advance(from);
    512     float beforeWidth = controller.runWidthSoFar();
     511    GlyphBuffer dummyGlyphBuffer;
     512    controller.advance(from, &dummyGlyphBuffer);
    513513    controller.advance(to, &glyphBuffer);
    514514
    … …  
    516516        return 0;
    517517
    518     float afterWidth = controller.runWidthSoFar();
    519 
    520518    if (run.rtl()) {
    521         initialAdvance = controller.totalWidth() - afterWidth + controller.leadingExpansion();
     519        // Exploit the fact that the sum of the paint advances is equal to
     520        // the sum of the layout advances.
     521        initialAdvance = controller.totalWidth();
     522        for (unsigned i = 0; i < glyphBuffer.size(); ++i)
     523            initialAdvance -= glyphBuffer.advanceAt(i).width();
    522524        glyphBuffer.reverse(0, glyphBuffer.size());
    523     } else
    524         initialAdvance = beforeWidth;
     525    } else {
     526        initialAdvance = dummyGlyphBuffer.initialAdvance().width();
     527        for (unsigned i = 0; i < dummyGlyphBuffer.size(); ++i)
     528            initialAdvance += dummyGlyphBuffer.advanceAt(i).width();
     529    }
    525530
    526531    return initialAdvance;

  • trunk/Source/WebCore/platform/graphics/mac/ComplexTextController.cpp

    r211294 r211382  
    108108}
    109109
     110void ComplexTextController::computeExpansionOpportunity()
     111{
     112    if (!m_expansion)
     113        m_expansionPerOpportunity = 0;
     114    else {
     115        unsigned expansionOpportunityCount = FontCascade::expansionOpportunityCount(m_run.text(), m_run.ltr() ? LTR : RTL, m_run.expansionBehavior()).first;
     116
     117        if (!expansionOpportunityCount)
     118            m_expansionPerOpportunity = 0;
     119        else
     120            m_expansionPerOpportunity = m_expansion / expansionOpportunityCount;
     121    }
     122}
     123
    110124ComplexTextController::ComplexTextController(const FontCascade& font, const TextRun& run, bool mayUseNaturalWritingDirection, HashSet<const Font*>* fallbackFonts, bool forTextEmphasis)
    111125    : m_fallbackFonts(fallbackFonts)
    … …  
    117131    , m_forTextEmphasis(forTextEmphasis)
    118132{
    119     if (!m_expansion)
    120         m_expansionPerOpportunity = 0;
    121     else {
    122         unsigned expansionOpportunityCount = FontCascade::expansionOpportunityCount(m_run.text(), m_run.ltr() ? LTR : RTL, run.expansionBehavior()).first;
    123 
    124         if (!expansionOpportunityCount)
    125             m_expansionPerOpportunity = 0;
    126         else
    127             m_expansionPerOpportunity = m_expansion / expansionOpportunityCount;
    128     }
     133    computeExpansionOpportunity();
    129134
    130135    collectComplexTextRuns();
    … …  
    137142    , m_run(run)
    138143    , m_end(run.length())
    139 {
     144    , m_expansion(run.expansion())
     145{
     146    computeExpansionOpportunity();
     147
    140148    for (auto& run : runs)
    141149        m_complexTextRuns.append(run.ptr());
    … …  
    158166        }
    159167    }
    160 
    161     m_runWidthSoFar = m_leadingExpansion;
    162168}
    163169
    … …  
    167173        return m_run.ltr() ? m_end : 0;
    168174
    169     h -= m_leadingExpansion;
    170175    if (h < 0)
    171176        return m_run.ltr() ? 0 : m_end;
    … …  
    181186            size_t index = offsetIntoAdjustedGlyphs + j;
    182187            CGFloat adjustedAdvance = m_adjustedBaseAdvances[index].width;
    183             if (!index)
    184                 adjustedAdvance += complexTextRun.initialAdvance().width;
    185188            if (x < adjustedAdvance) {
    186189                CFIndex hitGlyphStart = complexTextRun.indexAt(j);
    … …  
    562565
    563566    if (offset <= m_currentCharacter) {
    564         m_runWidthSoFar = m_leadingExpansion;
     567        m_runWidthSoFar = 0;
    565568        m_numGlyphsSoFar = 0;
    566569        m_currentRun = 0;
    … …  
    573576    size_t runCount = m_complexTextRuns.size();
    574577
    575     unsigned leftmostGlyph = 0;
    576     unsigned currentRunIndex = indexOfCurrentRun(leftmostGlyph);
     578    unsigned indexOfLeftmostGlyphInCurrentRun = 0; // Relative to the beginning of ComplexTextController.
     579    unsigned currentRunIndex = indexOfCurrentRun(indexOfLeftmostGlyphInCurrentRun);
    577580    while (m_currentRun < runCount) {
    578581        const ComplexTextRun& complexTextRun = *m_complexTextRuns[currentRunIndex];
    579582        bool ltr = complexTextRun.isLTR();
    580583        size_t glyphCount = complexTextRun.glyphCount();
    581         unsigned g = ltr ? m_glyphInCurrentRun : glyphCount - 1 - m_glyphInCurrentRun;
    582         unsigned k = leftmostGlyph + g;
     584        unsigned glyphIndexIntoCurrentRun = ltr ? m_glyphInCurrentRun : glyphCount - 1 - m_glyphInCurrentRun;
     585        unsigned glyphIndexIntoComplexTextController = indexOfLeftmostGlyphInCurrentRun + glyphIndexIntoCurrentRun;
    583586        if (fallbackFonts && &complexTextRun.font() != &m_font.primaryFont())
    584587            fallbackFonts->add(&complexTextRun.font());
    … …  
    587590        // When leftmostGlyph is 0, it represents the first glyph to draw, taking into
    588591        // account the text direction.
    589         if (glyphBuffer && !leftmostGlyph) {
    590             CGSize initialAdvance = complexTextRun.initialAdvance();
    591 #if USE_LAYOUT_SPECIFIC_ADVANCES
    592             unsigned index = ltr ? 0 : m_adjustedBaseAdvances.size() - 1;
    593             initialAdvance.width += glyphOrigin(index).x;
    594             initialAdvance.height += glyphOrigin(index).y;
    595 #endif
    596             glyphBuffer->setInitialAdvance(initialAdvance);
    597 
    598             glyphBuffer->setLeadingExpansion(m_leadingExpansion);
    599         }
     592        if (!indexOfLeftmostGlyphInCurrentRun && glyphBuffer)
     593            glyphBuffer->setInitialAdvance(complexTextRun.initialAdvance());
    600594
    601595        while (m_glyphInCurrentRun < glyphCount) {
    602             unsigned glyphStartOffset = complexTextRun.indexAt(g);
     596            unsigned glyphStartOffset = complexTextRun.indexAt(glyphIndexIntoCurrentRun);
    603597            unsigned glyphEndOffset;
    604598            if (complexTextRun.isMonotonic()) {
    605599                if (ltr)
    606                     glyphEndOffset = std::max<unsigned>(glyphStartOffset, static_cast<unsigned>(g + 1 < glyphCount ? complexTextRun.indexAt(g + 1) : complexTextRun.indexEnd()));
     600                    glyphEndOffset = std::max<unsigned>(glyphStartOffset, static_cast<unsigned>(glyphIndexIntoCurrentRun + 1 < glyphCount ? complexTextRun.indexAt(glyphIndexIntoCurrentRun + 1) : complexTextRun.indexEnd()));
    607601                else
    608                     glyphEndOffset = std::max<unsigned>(glyphStartOffset, static_cast<unsigned>(g > 0 ? complexTextRun.indexAt(g - 1) : complexTextRun.indexEnd()));
     602                    glyphEndOffset = std::max<unsigned>(glyphStartOffset, static_cast<unsigned>(glyphIndexIntoCurrentRun > 0 ? complexTextRun.indexAt(glyphIndexIntoCurrentRun - 1) : complexTextRun.indexEnd()));
    609603            } else
    610                 glyphEndOffset = complexTextRun.endOffsetAt(g);
    611 
    612             CGSize adjustedBaseAdvance = m_adjustedBaseAdvances[k];
     604                glyphEndOffset = complexTextRun.endOffsetAt(glyphIndexIntoCurrentRun);
     605
     606            CGSize adjustedBaseAdvance = m_adjustedBaseAdvances[glyphIndexIntoComplexTextController];
    613607
    614608            if (glyphStartOffset + complexTextRun.stringLocation() >= m_currentCharacter)
    … …  
    616610
    617611            if (glyphBuffer && !m_characterInCurrentGlyph) {
     612                auto currentGlyphOrigin = glyphOrigin(glyphIndexIntoComplexTextController);
    618613                GlyphBufferAdvance paintAdvance = adjustedBaseAdvance;
    619 #if USE_LAYOUT_SPECIFIC_ADVANCES
    620                 if (k + 1 < m_adjustedBaseAdvances.size()) {
    621                     paintAdvance.setWidth(paintAdvance.width() + glyphOrigin(k + 1).x - glyphOrigin(k).x);
    622                     paintAdvance.setHeight(paintAdvance.height() - glyphOrigin(k + 1).y + glyphOrigin(k).y);
     614                if (!glyphIndexIntoCurrentRun) {
     615                    // The first layout advance of every run includes the "initial layout advance." However, here, we need
     616                    // paint advances, so subtract it out before transforming the layout advance into a paint advance.
     617                    paintAdvance.setWidth(paintAdvance.width() - (complexTextRun.initialAdvance().width - currentGlyphOrigin.x));
     618                    paintAdvance.setHeight(paintAdvance.height() - (complexTextRun.initialAdvance().height - currentGlyphOrigin.y));
    623619                }
    624 #endif
    625                 glyphBuffer->add(m_adjustedGlyphs[k], &complexTextRun.font(), paintAdvance, complexTextRun.indexAt(m_glyphInCurrentRun));
     620                paintAdvance.setWidth(paintAdvance.width() + glyphOrigin(glyphIndexIntoComplexTextController + 1).x - currentGlyphOrigin.x);
     621                paintAdvance.setHeight(paintAdvance.height() + glyphOrigin(glyphIndexIntoComplexTextController + 1).y - currentGlyphOrigin.y);
     622                if (glyphIndexIntoCurrentRun == glyphCount - 1 && currentRunIndex + 1 < runCount) {
     623                    // Our paint advance points to the end of the run. However, the next run may have an
     624                    // initial advance, and our paint advance needs to point to the location of the next
     625                    // glyph. So, we need to add in the next run's initial advance.
     626                    paintAdvance.setWidth(paintAdvance.width() - glyphOrigin(glyphIndexIntoComplexTextController + 1).x + m_complexTextRuns[currentRunIndex + 1]->initialAdvance().width);
     627                    paintAdvance.setHeight(paintAdvance.height() - glyphOrigin(glyphIndexIntoComplexTextController + 1).y + m_complexTextRuns[currentRunIndex + 1]->initialAdvance().height);
     628                }
     629                paintAdvance.setHeight(-paintAdvance.height()); // Increasing y points down
     630                glyphBuffer->add(m_adjustedGlyphs[glyphIndexIntoComplexTextController], &complexTextRun.font(), paintAdvance, complexTextRun.indexAt(m_glyphInCurrentRun));
    626631            }
    627632
    … …  
    637642            m_characterInCurrentGlyph = 0;
    638643            if (ltr) {
    639                 g++;
    640                 k++;
     644                glyphIndexIntoCurrentRun++;
     645                glyphIndexIntoComplexTextController++;
    641646            } else {
    642                 g--;
    643                 k--;
    644             }
    645         }
    646         currentRunIndex = incrementCurrentRun(leftmostGlyph);
     647                glyphIndexIntoCurrentRun--;
     648                glyphIndexIntoComplexTextController--;
     649            }
     650        }
     651        currentRunIndex = incrementCurrentRun(indexOfLeftmostGlyphInCurrentRun);
    647652        m_glyphInCurrentRun = 0;
    648653    }
    … …  
    691696        const Font& font = complexTextRun.font();
    692697
    693         // Represent the initial advance for a text run by adjusting the advance
    694         // of the last glyph of the previous text run in the glyph buffer.
    695         if (!complexTextRun.glyphOrigins() && runIndex && m_adjustedBaseAdvances.size()) {
    696             CGSize previousAdvance = m_adjustedBaseAdvances.last();
    697             previousAdvance.width += complexTextRun.initialAdvance().width;
    698             previousAdvance.height -= complexTextRun.initialAdvance().height;
    699             m_adjustedBaseAdvances[m_adjustedBaseAdvances.size() - 1] = previousAdvance;
    700         }
    701         widthSinceLastCommit += complexTextRun.initialAdvance().width;
    702 
    703698        if (!complexTextRun.isLTR())
    704699            m_isLTROnly = false;
    … …  
    742737                advance.width = 0;
    743738                glyph = font.spaceGlyph();
     739            }
     740
     741            if (!i) {
     742                advance.width += complexTextRun.initialAdvance().width;
     743                advance.height += complexTextRun.initialAdvance().height;
     744                if (auto* origins = complexTextRun.glyphOrigins()) {
     745                    advance.width -= origins[0].x;
     746                    advance.height -= origins[0].y;
     747                }
    744748            }
    745749
    … …  
    775779                        bool expandLeft, expandRight;
    776780                        std::tie(expandLeft, expandRight) = expansionLocation(ideograph, treatAsSpace, m_run.ltr(), afterExpansion, forbidLeadingExpansion, forbidTrailingExpansion, forceLeadingExpansion, forceTrailingExpansion);
     781                        m_expansion -= m_expansionPerOpportunity;
     782                        advance.width += m_expansionPerOpportunity;
    777783                        if (expandLeft) {
    778784                            // Increase previous width
    779                             m_expansion -= m_expansionPerOpportunity;
    780                             m_totalWidth += m_expansionPerOpportunity;
    781785                            if (m_adjustedBaseAdvances.isEmpty())
    782                                 m_leadingExpansion = m_expansionPerOpportunity;
     786                                complexTextRun.growInitialAdvanceHorizontally(m_expansionPerOpportunity);
    783787                            else
    784788                                m_adjustedBaseAdvances.last().width += m_expansionPerOpportunity;
    785789                        }
    786                         if (expandRight) {
    787                             m_expansion -= m_expansionPerOpportunity;
    788                             advance.width += m_expansionPerOpportunity;
     790                        if (expandRight)
    789791                            afterExpansion = true;
    790                         }
    791792                    } else
    792793                        afterExpansion = false;
    … …  
    805806                glyph = 0;
    806807
    807             advance.height *= -1;
    808808            m_adjustedBaseAdvances.append(advance);
    809 #if USE_LAYOUT_SPECIFIC_ADVANCES
    810809            if (auto* origins = complexTextRun.glyphOrigins()) {
    811810                ASSERT(m_glyphOrigins.size() < m_adjustedBaseAdvances.size());
    … …  
    814813                ASSERT(m_glyphOrigins.size() == m_adjustedBaseAdvances.size());
    815814            }
    816 #endif
    817815            m_adjustedGlyphs.append(glyph);
    818816           

  • trunk/Source/WebCore/platform/graphics/mac/ComplexTextController.h

    r211308 r211382  
    5252enum GlyphIterationStyle { IncludePartialGlyphs, ByWholeGlyphs };
    5353
    54 // ComplexTextController is responsible for rendering and measuring glyphs for
    55 // complex scripts on macOS and iOS.
     54// See https://trac.webkit.org/wiki/ComplexTextController for more information about ComplexTextController.
    5655class ComplexTextController {
    5756    WTF_MAKE_FAST_ALLOCATED;
    … …  
    7776    float minGlyphBoundingBoxY() const { return m_minGlyphBoundingBoxY; }
    7877    float maxGlyphBoundingBoxY() const { return m_maxGlyphBoundingBoxY; }
    79 
    80     float leadingExpansion() const { return m_leadingExpansion; }
    8178
    8279    class ComplexTextRun : public RefCounted<ComplexTextRun> {
    … …  
    109106
    110107        /*
    111          *                                              X (Paint glyph position)   X (Paint glyph position)   X (Paint glyph position)
    112          *                                             7                          7                          7
    113          *                                            /                          /                          /
    114          *                                           / (Glyph origin)           / (Glyph origin)           / (Glyph origin)
    115          *                                          /                          /                          /
    116          *                                         /                          /                          /
    117          *                X-----------------------X--------------------------X--------------------------X---->...
    118          * (text position ^)  (initial advance)          (base advance)             (base advance)
     108         * This is the format of the information CoreText gives us about each run:
     109         *
     110         *                                        ----->X (Paint glyph position)   X (Paint glyph position)   X (Paint glyph position)
     111         *                                       /     7                          7                          7
     112         *                                      /     /                          /                          /
     113         *                   (Initial advance) /     / (Glyph origin)           / (Glyph origin)           / (Glyph origin)
     114         *                  -------------------     /                          /                          /
     115         *                 /                       /                          /                          /
     116         *                X                       X--------------------------X--------------------------X--------------------------X
     117         * (text position ^)                             (base advance)             (base advance)             (base advance)
     118         *
     119         *
     120         *
     121         *
     122         *
     123         * And here is the output we transform this into (for each run):
     124         *
     125         *                                        ----->X------------------------->X------------------------->X
     126         *                                       /            (Paint advance)            (Paint advance)       \
     127         *                                      /                                                               \
     128         *                   (Initial advance) /                                                                 \ (Paint advance)
     129         *                  -------------------                                                                   ----------------
     130         *                 /                                                                                                      \
     131         *                X--------------------------------------------------X--------------------------X--------------------------X
     132         * (text position ^)                (layout advance)                       (layout advance)           (layout advance)
    119133         */
     134        void growInitialAdvanceHorizontally(CGFloat delta) { m_initialAdvance.width += delta; }
    120135        CGSize initialAdvance() const { return m_initialAdvance; }
    121136        const CGSize* baseAdvances() const { return m_baseAdvances; }
    … …  
    150165    };
    151166private:
     167    void computeExpansionOpportunity();
    152168    void finishConstruction();
    153169   
    … …  
    207223    float m_expansion { 0 };
    208224    float m_expansionPerOpportunity { 0 };
    209     float m_leadingExpansion { 0 };
    210225
    211226    float m_minGlyphBoundingBoxX { std::numeric_limits<float>::max() };

  • trunk/Tools/ChangeLog

    r211380 r211382  
     12017-01-30  Myles C. Maxfield  <mmaxfield@apple.com>
     2
     3        Correct spacing regression on inter-element complex path shaping on some fonts
     4        https://bugs.webkit.org/show_bug.cgi?id=166013
     5
     6        Reviewed by Simon Fraser.
     7
     8        Unskip existing tests and make some new tests:
     9        - Testing complex text with no origins
     10        - Testing initial expansions
     11        - Testing the sign of vertical advances
     12
     13        * TestWebKitAPI/Tests/WebCore/ComplexTextController.cpp:
     14        (TestWebKitAPI::TEST_F):
     15
    1162017-01-30  Carlos Alberto Lopez Perez  <clopez@igalia.com>
    217

  • trunk/Tools/TestWebKitAPI/Tests/WebCore/ComplexTextController.cpp

    r211308 r211382  
    4646};
    4747
    48 TEST_F(ComplexTextControllerTest, DISABLED_InitialAdvanceWithLeftRunInRTL)
     48TEST_F(ComplexTextControllerTest, InitialAdvanceWithLeftRunInRTL)
    4949{
    5050    FontCascadeDescription description;
    … …  
    8080    EXPECT_NEAR(controller.totalWidth(), spaceWidth + totalWidth, 0.0001);
    8181    GlyphBuffer glyphBuffer;
     82    EXPECT_NEAR(controller.runWidthSoFar(), 0, 0.0001);
    8283    controller.advance(0, &glyphBuffer);
    8384    EXPECT_NEAR(controller.runWidthSoFar(), 0, 0.0001);
    … …  
    9798}
    9899
    99 TEST_F(ComplexTextControllerTest, DISABLED_InitialAdvanceInRTL)
     100TEST_F(ComplexTextControllerTest, InitialAdvanceInRTL)
    100101{
    101102    FontCascadeDescription description;
    … …  
    128129    EXPECT_NEAR(controller.totalWidth(), totalWidth, 0.0001);
    129130    GlyphBuffer glyphBuffer;
     131    EXPECT_NEAR(controller.runWidthSoFar(), 0, 0.0001);
    130132    controller.advance(0, &glyphBuffer);
    131133    EXPECT_NEAR(controller.runWidthSoFar(), 0, 0.0001);
    … …  
    142144    EXPECT_NEAR(glyphBuffer.advanceAt(3).width(), advances[1].width, 0.0001);
    143145    EXPECT_NEAR(glyphBuffer.advanceAt(4).width(), -initialAdvance.width, 0.0001);
    144 }
    145 
    146 TEST_F(ComplexTextControllerTest, DISABLED_InitialAdvanceWithLeftRunInLTR)
     146    EXPECT_NEAR(glyphBuffer.advanceAt(4).height(), initialAdvance.height, 0.0001);
     147}
     148
     149TEST_F(ComplexTextControllerTest, InitialAdvanceWithLeftRunInLTR)
    147150{
    148151    FontCascadeDescription description;
    … …  
    175178    EXPECT_NEAR(controller.totalWidth(), spaceWidth + 76.347656 + initialAdvance.width, 0.0001);
    176179    GlyphBuffer glyphBuffer;
     180    EXPECT_NEAR(controller.runWidthSoFar(), 0, 0.0001);
    177181    controller.advance(0, &glyphBuffer);
    178182    EXPECT_NEAR(controller.runWidthSoFar(), 0, 0.0001);
    … …  
    191195}
    192196
    193 TEST_F(ComplexTextControllerTest, DISABLED_InitialAdvanceInLTR)
     197TEST_F(ComplexTextControllerTest, InitialAdvanceInLTR)
    194198{
    195199    FontCascadeDescription description;
    … …  
    219223    EXPECT_NEAR(controller.totalWidth(), 76.347656 + initialAdvance.width, 0.0001);
    220224    GlyphBuffer glyphBuffer;
     225    EXPECT_NEAR(controller.runWidthSoFar(), 0, 0.0001);
    221226    controller.advance(0, &glyphBuffer);
    222227    EXPECT_NEAR(controller.runWidthSoFar(), 0, 0.0001);
    … …  
    232237}
    233238
    234 }
     239TEST_F(ComplexTextControllerTest, InitialAdvanceInRTLNoOrigins)
     240{
     241    FontCascadeDescription description;
     242    description.setOneFamily("Times");
     243    description.setComputedSize(48);
     244    FontCascade font(description);
     245    font.update();
     246
     247    CGSize initialAdvance = CGSizeMake(4.33996383363472, 12.368896925859);
     248
     249    UChar characters[] = { 0x633, 0x20, 0x627, 0x650 };
     250    size_t charactersLength = WTF_ARRAY_LENGTH(characters);
     251    TextRun textRun(StringView(characters, charactersLength));
     252    auto run1 = ComplexTextController::ComplexTextRun::createForTesting({ CGSizeMake(-4.33996383363472, -12.368896925859), CGSizeMake(14.0397830018083, 0) }, { }, { 884, 240 }, { 3, 2 }, initialAdvance, font.primaryFont(), characters, 0, charactersLength, CFRangeMake(2, 2), false);
     253    auto run2 = ComplexTextController::ComplexTextRun::createForTesting({ CGSizeMake(12.0, 0) }, { }, { 3 }, { 1 }, CGSizeZero, font.primaryFont(), characters, 0, charactersLength, CFRangeMake(1, 1), false);
     254    auto run3 = ComplexTextController::ComplexTextRun::createForTesting({ CGSizeMake(43.8119349005425, 0) }, { }, { 276 }, { 0 }, CGSizeZero, font.primaryFont(), characters, 0, charactersLength, CFRangeMake(0, 1), false);
     255    Vector<Ref<ComplexTextController::ComplexTextRun>> runs;
     256    runs.append(WTFMove(run1));
     257    runs.append(WTFMove(run2));
     258    runs.append(WTFMove(run3));
     259    ComplexTextController controller(font, textRun, runs);
     260
     261    CGFloat totalWidth = 14.0397830018083 + 12.0 + 43.8119349005425;
     262    EXPECT_NEAR(controller.totalWidth(), totalWidth, 0.0001);
     263    GlyphBuffer glyphBuffer;
     264    EXPECT_NEAR(controller.runWidthSoFar(), 0, 0.0001);
     265    controller.advance(0, &glyphBuffer);
     266    EXPECT_NEAR(controller.runWidthSoFar(), 0, 0.0001);
     267    controller.advance(1, &glyphBuffer);
     268    EXPECT_NEAR(controller.runWidthSoFar(), 43.8119349005425, 0.0001);
     269    controller.advance(2, &glyphBuffer);
     270    EXPECT_NEAR(controller.runWidthSoFar(), 43.8119349005425 + 12.0, 0.0001);
     271    controller.advance(3, &glyphBuffer);
     272    EXPECT_NEAR(controller.runWidthSoFar(), totalWidth, 0.0001);
     273    controller.advance(4, &glyphBuffer);
     274    EXPECT_NEAR(controller.runWidthSoFar(), totalWidth, 0.0001);
     275    EXPECT_NEAR(glyphBuffer.initialAdvance().width(), initialAdvance.width, 0.0001);
     276    EXPECT_NEAR(glyphBuffer.initialAdvance().height(), initialAdvance.height, 0.0001);
     277    EXPECT_EQ(glyphBuffer.size(), 4U);
     278    EXPECT_NEAR(glyphBuffer.advanceAt(0).width(), 43.8119349005425, 0.0001);
     279    EXPECT_NEAR(glyphBuffer.advanceAt(1).width(), 12.0, 0.0001);
     280    EXPECT_NEAR(glyphBuffer.advanceAt(2).width(), 14.0397830018083, 0.0001);
     281    EXPECT_NEAR(glyphBuffer.advanceAt(3).width(), -4.33996383363472, 0.0001);
     282    EXPECT_NEAR(glyphBuffer.advanceAt(3).height(), 12.368896925859, 0.0001);
     283}
     284
     285TEST_F(ComplexTextControllerTest, LeadingExpansion)
     286{
     287    FontCascadeDescription description;
     288    description.setOneFamily("Times");
     289    description.setComputedSize(48);
     290    FontCascade font(description);
     291    font.update();
     292
     293    UChar characters[] = { 'a' };
     294    size_t charactersLength = WTF_ARRAY_LENGTH(characters);
     295    TextRun textRun(StringView(characters, charactersLength), 0, 100, ForceLeadingExpansion);
     296    auto run = ComplexTextController::ComplexTextRun::createForTesting({ CGSizeMake(24, 0) }, { }, { 16 }, { 0 }, CGSizeZero, font.primaryFont(), characters, 0, charactersLength, CFRangeMake(0, 1), true);
     297    Vector<Ref<ComplexTextController::ComplexTextRun>> runs;
     298    runs.append(WTFMove(run));
     299    ComplexTextController controller(font, textRun, runs);
     300
     301    CGFloat totalWidth = 100 + 24;
     302    EXPECT_NEAR(controller.totalWidth(), totalWidth, 0.0001);
     303    GlyphBuffer glyphBuffer;
     304    EXPECT_NEAR(controller.runWidthSoFar(), 0, 0.0001);
     305    controller.advance(0, &glyphBuffer);
     306    EXPECT_NEAR(controller.runWidthSoFar(), 0, 0.0001);
     307    controller.advance(1, &glyphBuffer);
     308    EXPECT_NEAR(controller.runWidthSoFar(), totalWidth, 0.0001);
     309    EXPECT_NEAR(glyphBuffer.initialAdvance().width(), 100, 0.0001);
     310    EXPECT_NEAR(glyphBuffer.initialAdvance().height(), 0, 0.0001);
     311    EXPECT_EQ(glyphBuffer.size(), 1U);
     312    EXPECT_NEAR(glyphBuffer.advanceAt(0).width(), 24, 0.0001);
     313}
     314
     315TEST_F(ComplexTextControllerTest, VerticalAdvances)
     316{
     317    FontCascadeDescription description;
     318    description.setOneFamily("Times");
     319    description.setComputedSize(48);
     320    FontCascade font(description);
     321    font.update();
     322
     323    UChar characters[] = { 'a', 'b', 'c', 'd' };
     324    size_t charactersLength = WTF_ARRAY_LENGTH(characters);
     325    TextRun textRun(StringView(characters, charactersLength));
     326    auto run1 = ComplexTextController::ComplexTextRun::createForTesting({ CGSizeMake(0, 1), CGSizeMake(0, 2) }, { CGPointMake(0, 4), CGPointMake(0, 8) }, { 16, 17 }, { 0, 1 }, CGSizeMake(0, 16), font.primaryFont(), characters, 0, charactersLength, CFRangeMake(0, 2), true);
     327    auto run2 = ComplexTextController::ComplexTextRun::createForTesting({ CGSizeMake(0, 32), CGSizeMake(0, 64) }, { CGPointMake(0, 128), CGPointMake(0, 256) }, { 18, 19 }, { 2, 3 }, CGSizeMake(0, 512), font.primaryFont(), characters, 0, charactersLength, CFRangeMake(2, 2), true);
     328    Vector<Ref<ComplexTextController::ComplexTextRun>> runs;
     329    runs.append(WTFMove(run1));
     330    runs.append(WTFMove(run2));
     331    ComplexTextController controller(font, textRun, runs);
     332
     333    EXPECT_NEAR(controller.totalWidth(), 0, 0.0001);
     334    GlyphBuffer glyphBuffer;
     335    EXPECT_NEAR(controller.runWidthSoFar(), 0, 0.0001);
     336    controller.advance(0, &glyphBuffer);
     337    EXPECT_NEAR(controller.runWidthSoFar(), 0, 0.0001);
     338    controller.advance(1, &glyphBuffer);
     339    EXPECT_NEAR(controller.runWidthSoFar(), 0, 0.0001);
     340    controller.advance(2, &glyphBuffer);
     341    EXPECT_NEAR(controller.runWidthSoFar(), 0, 0.0001);
     342    controller.advance(3, &glyphBuffer);
     343    EXPECT_NEAR(controller.runWidthSoFar(), 0, 0.0001);
     344    controller.advance(4, &glyphBuffer);
     345    EXPECT_NEAR(controller.runWidthSoFar(), 0, 0.0001);
     346    EXPECT_NEAR(glyphBuffer.initialAdvance().width(), 0, 0.0001);
     347    EXPECT_NEAR(glyphBuffer.initialAdvance().height(), 16, 0.0001);
     348    EXPECT_EQ(glyphBuffer.size(), 4U);
     349    EXPECT_NEAR(glyphBuffer.advanceAt(0).width(), 0, 0.0001);
     350    EXPECT_NEAR(glyphBuffer.advanceAt(0).height(), 4 - 1 -8, 0.0001);
     351    EXPECT_NEAR(glyphBuffer.advanceAt(1).width(), 0, 0.0001);
     352    EXPECT_NEAR(glyphBuffer.advanceAt(1).height(), 8 - 2 - 512, 0.0001);
     353    EXPECT_NEAR(glyphBuffer.advanceAt(2).width(), 0, 0.0001);
     354    EXPECT_NEAR(glyphBuffer.advanceAt(2).height(), 128 - 32 - 256, 0.0001);
     355    EXPECT_NEAR(glyphBuffer.advanceAt(3).width(), 0, 0.0001);
     356    EXPECT_NEAR(glyphBuffer.advanceAt(3).height(), 256 - 64, 0.0001);
     357}
     358
     359}
Note: See TracChangeset for help on using the changeset viewer.