Add tab characters support in complex path

Source/platform/fonts/shaping/HarfBuzzShaper.cpp

 /*
  * Copyright (c) 2012 Google Inc. All rights reserved.
  * Copyright (C) 2013 BlackBerry Limited. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
@@ skipping 534 matching lines @@
         stringFor8BitRun = String::make16BitFrom8BitSource(run.characters8(), run.length());
         source = stringFor8BitRun.characters16();
     } else {
         source = run.characters16();
     }
 
     *destinationLength = 0;
     while (position < length) {
         UChar32 character;
         U16_NEXT(source, position, length, character);
-        // Don't normalize tabs as they are not treated as spaces for word-end.
-        if (run.normalizeSpace() && Character::isNormalizedCanvasSpaceCharacter(character))
-            character = spaceCharacter;
-        else if (Character::treatAsSpace(character) && character != tabulationCharacter)
-            character = spaceCharacter;
-        else if (Character::treatAsZeroWidthSpaceInComplexScript(character))
-            character = zeroWidthSpaceCharacter;
+        if (!(character == tabulationCharacter && run.allowTabs())) {
+            // Don't normalize tabs as they are not treated as spaces for word-end.
+            if (run.normalizeSpace() && Character::isNormalizedCanvasSpaceCharacter(character))
+                character = spaceCharacter;
+            else if (Character::treatAsSpace(character) && character != tabulationCharacter)
+                character = spaceCharacter;
+            else if (Character::treatAsZeroWidthSpaceInComplexScript(character))
+                character = zeroWidthSpaceCharacter;
+        }
 
         U16_APPEND(destination, *destinationLength, length, character, error);
         ASSERT_UNUSED(error, !error);
     }
 }
 
 HarfBuzzShaper::HarfBuzzShaper(const Font* font, const TextRun& run,
     HashSet<const SimpleFontData*>* fallbackFonts)
     : Shaper(font, run, nullptr, fallbackFonts)
     , m_normalizedBufferLength(0)
@@ skipping 153 matching lines @@
 }
 
 struct CandidateRun {
     UChar32 character;
     unsigned start;
     unsigned end;
     const SimpleFontData* fontData;
     UScriptCode script;
 };
 
+static void collectCandidateRunsForTabulationCharacters(UTF16TextIterator& iterator,
+    Vector<CandidateRun>* runs, unsigned& startIndexOfCurrentRun,
+    UChar32& character, const SimpleFontData* currentFontData)
+{
+    ASSERT(character == tabulationCharacter);
+    for (iterator.advance(); iterator.consume(character); iterator.advance()) {
+        if (character != tabulationCharacter)
+            break;
+    }
+    CandidateRun run = { tabulationCharacter, startIndexOfCurrentRun, static_cast<unsigned>(iterator.offset()), currentFontData, USCRIPT_COMMON };
+    runs->append(run);
+    startIndexOfCurrentRun = iterator.offset();
+}
+
+static inline bool updateCharacterInfo(const Font* font, UChar32 character, const TextRun& textRun,

[eae, 2015/07/20 17:39:24]

Update makes it sound like it has state, how about something like this:

struct RunGlypInfo {
  const SimpleFontData* fontData;
  UScriptCode script;
}

RunGlypInfo glyphInfoForChar(cont char* font, UChar32 character, TextRun&
textRun)
{
...
}

[kojii, 2015/07/21 01:11:47]

Glad you suggested this! I once did this but it made the patch larger, and my
mentor told me to make patch smaller. I'll prioritize cleaner code than smaller
patch in future ;-)

+    const SimpleFontData*& fontData, UScriptCode& script)
+{
+    fontData = font->glyphDataForCharacter(character, false, textRun.normalizeSpace()).fontData;
+    UErrorCode errorCode = U_ZERO_ERROR;
+    script = uscript_getScript(character, &errorCode);
+    if (U_FAILURE(errorCode))
+        return false;
+    return true;
+}
+
 static inline bool collectCandidateRuns(const UChar* normalizedBuffer,
-    size_t bufferLength, const Font* font, Vector<CandidateRun>* runs, bool isSpaceNormalize)
+    size_t bufferLength, const Font* font, Vector<CandidateRun>* runs,
+    const TextRun& textRun)
 {
     UTF16TextIterator iterator(normalizedBuffer, bufferLength);
     UChar32 character;
     unsigned startIndexOfCurrentRun = 0;
 
     if (!iterator.consume(character))
         return false;
 
-    const SimpleFontData* nextFontData = font->glyphDataForCharacter(character, false, isSpaceNormalize).fontData;
-    UErrorCode errorCode = U_ZERO_ERROR;
-    UScriptCode nextScript = uscript_getScript(character, &errorCode);
-    if (U_FAILURE(errorCode))
+    const SimpleFontData* nextFontData;
+    UScriptCode nextScript;
+    if (!updateCharacterInfo(font, character, textRun, nextFontData, nextScript))
         return false;
 
     do {
+        // Tab characters need to go to its own run because the run doesn't use Harfbuzz.
+        if (character == tabulationCharacter && textRun.allowTabs()) {

[eae, 2015/07/20 17:39:24]

How about adding UNLIKELY macros around the tab checks to prime to branch
predictor?

if (UNLIKELY(character == tabulationCharacter && textRun.allowTabs())) {

[kojii, 2015/07/21 01:11:47]

Thought UNLIKELY() is the past. Thanks, learned one more thing.

+            collectCandidateRunsForTabulationCharacters(iterator, runs,
+                startIndexOfCurrentRun, character, nextFontData);
+            if (iterator.atEnd())
+                break;
+            if (!updateCharacterInfo(font, character, textRun, nextFontData, nextScript))
+                return false;
+        }
+
         const UChar* currentCharacterPosition = iterator.characters();
         const SimpleFontData* currentFontData = nextFontData;
         UScriptCode currentScript = nextScript;
 
         UChar32 lastCharacter = character;
         for (iterator.advance(); iterator.consume(character); iterator.advance()) {
-            if (Character::treatAsZeroWidthSpace(character))
+            if (Character::treatAsZeroWidthSpace(character)) {
+                if (character == tabulationCharacter && textRun.allowTabs())

[eae, 2015/07/20 17:39:24]

I'd move this check out of the treatAsZeroWidthSpace block,

if (character == tabulationCharacter && textRun.allowTabs()) {
...
} else if (Character::treatAsZeroWidthSpace(character)) {
...
}

[kojii, 2015/07/21 01:11:47]

That's better to read. treatAsZeroWidthSpace() checks tab, so we check tab twice
then. It's rare code so I tried less CPU cycles but I'm fine either way.

+                    break;
                 continue;
+            }
             if ((U_GET_GC_MASK(character) & U_GC_M_MASK)
                 && (Character::isUnicodeVariationSelector(character)
                     || currentFontData->canRenderCombiningCharacterSequence(
                         currentCharacterPosition,
                         iterator.glyphEnd() - currentCharacterPosition)))
                 continue;
 
-            nextFontData = font->glyphDataForCharacter(character, false, isSpaceNormalize).fontData;
-            nextScript = uscript_getScript(character, &errorCode);
-            if (U_FAILURE(errorCode))
+            if (!updateCharacterInfo(font, character, textRun, nextFontData, nextScript))
                 return false;
             if (lastCharacter == zeroWidthJoinerCharacter)
                 currentFontData = nextFontData;
             if ((nextFontData != currentFontData) || ((currentScript != nextScript) && (nextScript != USCRIPT_INHERITED) && (!uscript_hasScript(character, currentScript))))
                 break;
             currentCharacterPosition = iterator.characters();
             lastCharacter = character;
         }
 
         CandidateRun run = { lastCharacter, startIndexOfCurrentRun, static_cast<unsigned>(iterator.offset()), currentFontData, currentScript };
         runs->append(run);
 
         startIndexOfCurrentRun = iterator.offset();
-    } while (iterator.consume(character));
+    } while (!iterator.atEnd());
 
     return true;
 }
 
 static inline bool matchesAdjacentRun(UScriptCode* scriptExtensions, int length,
     CandidateRun& adjacentRun)
 {
     for (int i = 0; i < length; i++) {
         if (scriptExtensions[i] == adjacentRun.script)
             return true;
@@ skipping 92 matching lines @@
     return true;
 }
 
 bool HarfBuzzShaper::createHarfBuzzRuns()
 {
     if (m_normalizedBufferLength == 1)
         return createHarfBuzzRunsForSingleCharacter();
 
     Vector<CandidateRun> candidateRuns;
     if (!collectCandidateRuns(m_normalizedBuffer.get(),
-        m_normalizedBufferLength, m_font, &candidateRuns, m_textRun.normalizeSpace()))
+        m_normalizedBufferLength, m_font, &candidateRuns, m_textRun))
         return false;
 
     if (!resolveCandidateRuns(candidateRuns))
         return false;
 
     size_t length = candidateRuns.size();
     for (size_t i = 0; i < length; ) {
         CandidateRun& run = candidateRuns[i];
+        if (run.character == tabulationCharacter && m_textRun.allowTabs()) {

[eae, 2015/07/20 17:39:24]

Is this really needed? Isn't the extra condition in the loop below enough to
keep the tab runs separate?

[kojii, 2015/07/21 01:11:47]

Yeah, this check is needed if the first run is tab, and the second is not, and
the second has the same font and USCRIPT_COMMON; e.g., "\t//".

Maybe I could use USCRIPT_INVALID or something some sentry-like value for tab
runs.

+            addHarfBuzzRun(run.start, run.end, run.fontData, run.script);
+            ++i;
+            continue;
+        }
         CandidateRun lastMatchingRun = run;
         for (i++; i < length; i++) {
             if (candidateRuns[i].script != run.script
-                || candidateRuns[i].fontData != run.fontData)
+                || candidateRuns[i].fontData != run.fontData
+                || candidateRuns[i].character == tabulationCharacter)
                 break;
             lastMatchingRun = candidateRuns[i];
         }
         addHarfBuzzRun(run.start, lastMatchingRun.end, run.fontData, run.script);
     }
     return !m_harfBuzzRuns.isEmpty();
 }
 
 // A port of hb_icu_script_to_script because harfbuzz on CrOS is built
 // without hb-icu. See http://crbug.com/356929
@@ skipping 32 matching lines @@
 static const uint16_t* toUint16(const UChar* src)
 {
     // FIXME: This relies on undefined behavior however it works on the
     // current versions of all compilers we care about and avoids making
     // a copy of the string.
     static_assert(sizeof(UChar) == sizeof(uint16_t), "UChar should be the same size as uint16_t");
     return reinterpret_cast<const uint16_t*>(src);
 }
 
 static inline void addToHarfBuzzBufferInternal(hb_buffer_t* buffer,
-    const FontDescription& fontDescription, const UChar* normalizedBuffer,
-    unsigned startIndex, unsigned numCharacters)
+    const FontDescription& fontDescription, const UChar* characters,
+    unsigned numCharacters)
 {
     if (fontDescription.variant() == FontVariantSmallCaps
-        && u_islower(normalizedBuffer[startIndex])) {
-        String upperText = String(normalizedBuffer + startIndex, numCharacters)
+        && u_islower(characters[0])) {
+        String upperText = String(characters, numCharacters)
             .upper();
         // TextRun is 16 bit, therefore upperText is 16 bit, even after we call
         // makeUpper().
         ASSERT(!upperText.is8Bit());
         hb_buffer_add_utf16(buffer, toUint16(upperText.characters16()),
             numCharacters, 0, numCharacters);
     } else {
-        hb_buffer_add_utf16(buffer, toUint16(normalizedBuffer + startIndex),
+        hb_buffer_add_utf16(buffer, toUint16(characters),
             numCharacters, 0, numCharacters);
     }
 }
 
 PassRefPtr<ShapeResult> HarfBuzzShaper::shapeHarfBuzzRuns()
 {
     RefPtr<ShapeResult> result = ShapeResult::create(
         m_normalizedBufferLength, m_textRun.direction());
     HarfBuzzScopedPtr<hb_buffer_t> harfBuzzBuffer(hb_buffer_create(), hb_buffer_destroy);
 
     const FontDescription& fontDescription = m_font->fontDescription();
     const String& localeString = fontDescription.locale();
     CString locale = localeString.latin1();
     const hb_language_t language = hb_language_from_string(locale.data(), locale.length());
 
     result->m_runs.resize(m_harfBuzzRuns.size());
     for (unsigned i = 0; i < m_harfBuzzRuns.size(); ++i) {
         unsigned runIndex = m_textRun.rtl() ? m_harfBuzzRuns.size() - i - 1 : i;
         const HarfBuzzRun* currentRun = &m_harfBuzzRuns[runIndex];
+        ASSERT(m_normalizedBufferLength >= currentRun->m_startIndex + currentRun->m_numCharacters);
+        const UChar* characters = m_normalizedBuffer.get() + currentRun->m_startIndex;
+
+        if (characters[0] == tabulationCharacter && m_textRun.allowTabs()) {
+            shapeResultForTabulationCharacters(result.get(), i, currentRun);
+            continue;
+        }
 
         const SimpleFontData* currentFontData = currentRun->m_fontData;
         FontPlatformData* platformData = const_cast<FontPlatformData*>(&currentFontData->platformData());
         HarfBuzzFace* face = platformData->harfBuzzFace();
         if (!face)
             return nullptr;
 
         hb_buffer_set_language(harfBuzzBuffer.get(), language);
         hb_buffer_set_script(harfBuzzBuffer.get(), currentRun->m_script);
         hb_buffer_set_direction(harfBuzzBuffer.get(), currentRun->m_direction);
 
         // Add a space as pre-context to the buffer. This prevents showing dotted-circle
         // for combining marks at the beginning of runs.
         static const uint16_t preContext = spaceCharacter;
         hb_buffer_add_utf16(harfBuzzBuffer.get(), &preContext, 1, 1, 0);
 
         addToHarfBuzzBufferInternal(harfBuzzBuffer.get(),
-            fontDescription, m_normalizedBuffer.get(), currentRun->m_startIndex,
-            currentRun->m_numCharacters);
+            fontDescription, characters, currentRun->m_numCharacters);
 
         if (fontDescription.isVerticalAnyUpright())
             face->setScriptForVerticalGlyphSubstitution(harfBuzzBuffer.get());
 
         HarfBuzzScopedPtr<hb_font_t> harfBuzzFont(face->createFont(), hb_font_destroy);
         hb_shape(harfBuzzFont.get(), harfBuzzBuffer.get(), m_features.isEmpty() ? 0 : m_features.data(), m_features.size());
         shapeResult(result.get(), i, currentRun, harfBuzzBuffer.get());
 
         hb_buffer_reset(harfBuzzBuffer.get());
     }
@@ skipping 37 matching lines @@
         bool runEnd = i + 1 == numGlyphs;
         uint16_t glyph = glyphInfos[i].codepoint;
         offsetX = harfBuzzPositionToFloat(glyphPositions[i].x_offset);
         offsetY = -harfBuzzPositionToFloat(glyphPositions[i].y_offset);
 
         // One out of x_advance and y_advance is zero, depending on
         // whether the buffer direction is horizontal or vertical.
         float advance = harfBuzzPositionToFloat(glyphPositions[i].x_advance - glyphPositions[i].y_advance);
         unsigned currentCharacterIndex = currentRun->m_startIndex + glyphInfos[i].cluster;
         RELEASE_ASSERT(m_normalizedBufferLength > currentCharacterIndex);
+        ASSERT(m_normalizedBuffer[currentCharacterIndex] != tabulationCharacter || !m_textRun.allowTabs());
         bool isClusterEnd = runEnd || glyphInfos[i].cluster != glyphInfos[i + 1].cluster;
         float spacing = 0;
 
         run->m_glyphData[i].characterIndex = glyphInfos[i].cluster;
 
         if (isClusterEnd)
             spacing += adjustSpacing(run, i, currentCharacterIndex, *directionOffset, totalAdvance);
 
         if (currentFontData->isZeroWidthSpaceGlyph(glyph)) {
             run->setGlyphAndPositions(i, glyph, 0, 0, 0);
@@ skipping 16 matching lines @@
         result->m_glyphBoundingBox.unite(glyphBounds);
         glyphOrigin += FloatSize(advance + offsetX, offsetY);
     }
 
     run->m_width = totalAdvance > 0.0 ? totalAdvance : 0.0;
     result->m_width += run->m_width;
     for (auto& fallbackFont : *m_fallbackFonts)
         result->m_fallbackFonts.add(const_cast<SimpleFontData*>(fallbackFont));
 }
 
+void HarfBuzzShaper::shapeResultForTabulationCharacters(ShapeResult* result, unsigned index, const HarfBuzzRun* currentRun)
+{
+    const SimpleFontData& fontData = *currentRun->m_fontData;
+    ShapeResult::RunInfo* run = new ShapeResult::RunInfo(currentRun->m_fontData,
+        currentRun->m_direction, currentRun->m_script, currentRun->m_startIndex,
+        currentRun->m_numCharacters, currentRun->m_numCharacters);
+    result->m_runs[index] = run;
+    result->m_numGlyphs += currentRun->m_numCharacters;
+    float position = m_textRun.xPos() + result->m_width;
+    float positionStart = position;
+    for (size_t i = 0; i < currentRun->m_numCharacters; ++i) {
+        ASSERT(m_normalizedBuffer[currentRun->m_startIndex + i] == tabulationCharacter);
+        float advance = m_font->tabWidth(fontData, m_textRun.tabSize(), position);
+        run->m_glyphData[i].characterIndex = i;
+        run->setGlyphAndPositions(i, fontData.spaceGlyph(), advance, 0, 0);
+        position += advance;
+    }
+    run->m_width = position - positionStart;
+    result->m_width += run->m_width;
+}
+
 float HarfBuzzShaper::adjustSpacing(ShapeResult::RunInfo* run, size_t glyphIndex, unsigned currentCharacterIndex, float& offset, float& totalAdvance)
 {
     float spacing = 0;
     UChar32 character = m_normalizedBuffer[currentCharacterIndex];
     if (m_letterSpacing && !Character::treatAsZeroWidthSpace(character))
         spacing += m_letterSpacing;
 
     bool treatAsSpace = Character::treatAsSpace(character);
     if (treatAsSpace && currentCharacterIndex && (character != '\t' || !m_textRun.allowTabs()))
         spacing += m_wordSpacingAdjustment;
@@ skipping 37 matching lines @@
             return spacing;
     }
 
     // Don't need to check m_textRun.allowsTrailingExpansion() since it's covered by !m_expansionOpportunityCount above
     spacing += nextExpansionPerOpportunity();
     m_isAfterExpansion = true;
     return spacing;
 }
 
 } // namespace blink