text-emphasis marks should not be rendered if there is no emphasized …

…character

https://bugs.webkit.org/show_bug.cgi?id=251201
rdar://problem/104688963

Reviewed by Myles C. Maxfield.

There were 2 different reasons for failing these tests:
1. When drawing emphasis mark (drawEmphasisMarks())
we weren't skipping the glyphs that were replaced
by the deleted glyph (0xFFFF).

2. When iterating through the characters to be drawn with
the text iterators, we weren't converting characters encoded
as surrogated pairs (UTF16). When one of these characters
would show up, we would pass them to canReceiveTextEmphasis(),
implicitly converting the UChar word to a UChar32 word,
without properly converting the surrogated pairs.

* LayoutTests/TestExpectations:
* Source/WebCore/platform/graphics/ComplexTextController.cpp:
(WebCore::ComplexTextController::adjustGlyphsAndAdvances):
* Source/WebCore/platform/graphics/FontCascade.cpp:
(WebCore::FontCascade::drawEmphasisMarks const):
* Source/WebCore/platform/graphics/GlyphBuffer.h:
(WebCore::GlyphBuffer::makeGlyphInvisible):
* Source/WebCore/platform/graphics/WidthIterator.cpp:
(WebCore::WidthIterator::advanceInternal):
(WebCore::WidthIterator::applyCSSVisibilityRules):
* Source/WebCore/platform/graphics/coretext/GlyphPageCoreText.cpp:

Canonical link: https://commits.webkit.org/262997@main

LayoutTests/TestExpectations

@@ -3605,8 +3605,6 @@ imported/w3c/web-platform-tests/css/css-text-decor/text-emphasis-style-open-001.
 imported/w3c/web-platform-tests/css/css-text-decor/text-emphasis-style-shape-001.xht [ ImageOnlyFailure ]
 imported/w3c/web-platform-tests/css/css-text-decor/text-emphasis-style-string-001.xht [ ImageOnlyFailure ]
 webkit.org/b/230083 imported/w3c/web-platform-tests/css/css-text-decor/text-underline-offset-001.html [ ImageOnlyFailure ]
-imported/w3c/web-platform-tests/css/css-text-decor/text-emphasis-style-property-010Cc.html [ ImageOnlyFailure ]
-imported/w3c/web-platform-tests/css/css-text-decor/text-emphasis-style-property-010Cf.html [ ImageOnlyFailure ]
 
 # wpt css-position failures
 webkit.org/b/203445 [ Debug ] imported/w3c/web-platform-tests/css/css-position/position-absolute-container-dynamic-002.html [ Skip ]
@@ -4325,6 +4323,7 @@ webkit.org/b/209080 imported/w3c/web-platform-tests/css/css-writing-modes/vertic
 webkit.org/b/209080 imported/w3c/web-platform-tests/css/css-writing-modes/vertical-alignment-srl-032.xht [ ImageOnlyFailure ]
 webkit.org/b/209080 imported/w3c/web-platform-tests/css/css-writing-modes/vertical-alignment-srl-034.xht [ ImageOnlyFailure ]
 webkit.org/b/209080 imported/w3c/web-platform-tests/css/css-writing-modes/vertical-alignment-srl-040.xht [ ImageOnlyFailure ]
+webkit.org/b/255298 imported/w3c/web-platform-tests/css/css-writing-modes/text-combine-upright-decorations-001.html [ ImageOnlyFailure ]
 
 webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-column-012.html [ ImageOnlyFailure ]
 webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/aspect-ratio-intrinsic-size-005.html [ ImageOnlyFailure ]

Source/WebCore/platform/graphics/ComplexTextController.cpp

@@ -32,6 +32,7 @@
 #include "RenderText.h"
 #include "TextRun.h"
 #include <unicode/ubrk.h>
+#include <unicode/utf16.h>
 #include <wtf/StdLibExtras.h>
 #include <wtf/text/TextBreakIterator.h>
 #include <wtf/unicode/CharacterNames.h>
@@ -778,9 +779,14 @@ void ComplexTextController::adjustGlyphsAndAdvances()
 
             m_totalAdvance += advance;
 
-            // FIXME: Combining marks should receive a text emphasis mark if they are combine with a space.
-            if (m_forTextEmphasis && (!FontCascade::canReceiveTextEmphasis(ch) || (U_GET_GC_MASK(ch) & U_GC_M_MASK)))
-                glyph = 0;
+            if (m_forTextEmphasis) {
+                UChar32 ch32 = ch;
+                if (U16_IS_SURROGATE(ch))
+                    U16_GET(cp, 0, characterIndex, complexTextRun.stringLength(), ch32);
+                // FIXME: Combining marks should receive a text emphasis mark if they are combine with a space.
+                if (!FontCascade::canReceiveTextEmphasis(ch32) || (U_GET_GC_MASK(ch) & U_GC_M_MASK))
+                    glyph = deletedGlyph;
+            }
 
             m_adjustedBaseAdvances.append(advance);
             if (auto* origins = complexTextRun.glyphOrigins()) {

Source/WebCore/platform/graphics/FontCascade.cpp

@@ -1377,13 +1377,18 @@ void FontCascade::drawEmphasisMarks(GraphicsContext& context, const GlyphBuffer&
     FloatPoint startPoint(point.x() + middleOfLastGlyph - offsetToMiddleOfGlyph(*markFontData, markGlyph), point.y());
 
     GlyphBuffer markBuffer;
+    auto glyphForMarker = [&](unsigned index) {
+        auto glyph = glyphBuffer.glyphAt(index);
+        return (glyph && glyph != deletedGlyph) ? markGlyph : spaceGlyph;
+    };
+
     for (unsigned i = 0; i + 1 < glyphBuffer.size(); ++i) {
         float middleOfNextGlyph = offsetToMiddleOfGlyphAtIndex(glyphBuffer, i + 1);
         float advance = WebCore::width(glyphBuffer.advanceAt(i)) - middleOfLastGlyph + middleOfNextGlyph;
-        markBuffer.add(glyphBuffer.glyphAt(i) ? markGlyph : spaceGlyph, *markFontData, advance);
+        markBuffer.add(glyphForMarker(i), *markFontData, advance);
         middleOfLastGlyph = middleOfNextGlyph;
     }
-    markBuffer.add(glyphBuffer.glyphAt(glyphBuffer.size() - 1) ? markGlyph : spaceGlyph, *markFontData, 0);
+    markBuffer.add(glyphForMarker(glyphBuffer.size() - 1), *markFontData, 0);
 
     drawGlyphBuffer(context, markBuffer, startPoint, CustomFontNotReadyAction::DoNotPaintIfFontNotReady);
 }

Source/WebCore/platform/graphics/GlyphBuffer.h

@@ -39,6 +39,8 @@
 
 namespace WebCore {
 
+static const constexpr GlyphBufferGlyph deletedGlyph = 0xFFFF;
+
 class Font;
 
 class GlyphBuffer {
@@ -126,7 +128,6 @@ class GlyphBuffer {
     void makeGlyphInvisible(unsigned index)
     {
         // GlyphID 0xFFFF is the "deleted glyph" and is supposed to be invisible when rendered.
-        static const constexpr GlyphBufferGlyph deletedGlyph = 0xFFFF;
         m_glyphs[index] = deletedGlyph;
     }
 

Source/WebCore/platform/graphics/WidthIterator.cpp

@@ -273,7 +273,6 @@ inline void WidthIterator::advanceInternal(TextIterator& textIterator, GlyphBuff
         if (!glyph && !characterMustDrawSomething) {
             commitCurrentFontRange(glyphBuffer, lastGlyphCount, currentCharacterIndex, lastFontData, primaryFont, primaryFont, character, widthOfCurrentFontRange, width, charactersTreatedAsSpace);
 
-            Glyph deletedGlyph = 0xFFFF;
             addToGlyphBuffer(glyphBuffer, deletedGlyph, primaryFont, 0, currentCharacterIndex, character);
 
             textIterator.advance(advanceLength);
@@ -300,7 +299,7 @@ inline void WidthIterator::advanceInternal(TextIterator& textIterator, GlyphBuff
         }
 
         if (m_forTextEmphasis && !FontCascade::canReceiveTextEmphasis(character))
-            glyph = 0;
+            glyph = deletedGlyph;
 
         addToGlyphBuffer(glyphBuffer, glyph, font, width, currentCharacterIndex, character);
 
@@ -534,7 +533,6 @@ void WidthIterator::applyCSSVisibilityRules(GlyphBuffer& glyphBuffer, unsigned g
 
     auto adjustForSyntheticBold = [&](auto index) {
         auto glyph = glyphBuffer.glyphAt(index);
-        static constexpr const GlyphBufferGlyph deletedGlyph = 0xFFFF;
         auto syntheticBoldOffset = glyph == deletedGlyph ? 0 : glyphBuffer.fontAt(index).syntheticBoldOffset();
         m_runWidthSoFar += syntheticBoldOffset;
         auto& advance = glyphBuffer.advances(index)[0];

Source/WebCore/platform/graphics/coretext/GlyphPageCoreText.cpp

@@ -47,7 +47,6 @@ static bool shouldFillWithVerticalGlyphs(const UChar* buffer, unsigned bufferLen
     return false;
 }
 
-static const constexpr CGGlyph deletedGlyph = 0xFFFF;
 
 bool GlyphPage::fill(UChar* buffer, unsigned bufferLength)
 {