- Revision
- 229273
- Author
- [email protected]
- Date
- 2018-03-05 05:17:26 -0800 (Mon, 05 Mar 2018)
Log Message
Merge r229165 - REGRESSION(r222843): [HarfBuzz] Combining enclosed keycap not correctly handled
https://bugs.webkit.org/show_bug.cgi?id=183246
Reviewed by Michael Catanzaro.
Source/WebCore:
We are not correctly handling the combining enclosed keycap since we switched to use
ComplexTextController. This is because fontForCombiningCharacterSequence() always returns the font of the first
character, without checking if that font can render the whole sequence or not. Before 222843, the shaper did
that check when creating the text runs. In this case the sequence was split and a different font was used for the
text and the mark. This patch makes fontForCombiningCharacterSequence() try to find a suitable font for the
whole sequence, first looking at the CSS fallbacks and finally at system ones. The result is much better than
the old one, because we use the same font for both the text and the mark. If there isn't any font to render the
mark, then we fallback to use the first character font, since we will end up rendering the missing glyph
character, it's better to use the same font than the first character one.
Test: fast/text/combining-enclosing-keycap.html
* platform/graphics/cairo/FontCairoHarfbuzzNG.cpp:
(WebCore::FontCascade::fontForCombiningCharacterSequence const): Check if the first charatcer font can render
the whole sequence, trying with fallbacks otherwise.
* platform/graphics/freetype/SimpleFontDataFreeType.cpp:
(WebCore::Font::canRenderCombiningCharacterSequence const): Check if the font face has glyphs for the whole
sequence not just the first character.
LayoutTests:
* fast/text/combining-enclosing-keycap-expected.txt: Added.
* platform/gtk/fast/text/combining-enclosing-keycap.html: Added.
* platform/gtk/TestExpectations:
Modified Paths
Added Paths
Diff
Modified: releases/WebKitGTK/webkit-2.20/LayoutTests/ChangeLog (229272 => 229273)
--- releases/WebKitGTK/webkit-2.20/LayoutTests/ChangeLog 2018-03-05 13:17:11 UTC (rev 229272)
+++ releases/WebKitGTK/webkit-2.20/LayoutTests/ChangeLog 2018-03-05 13:17:26 UTC (rev 229273)
@@ -1,5 +1,16 @@
2018-03-01 Carlos Garcia Campos <[email protected]>
+ REGRESSION(r222843): [HarfBuzz] Combining enclosed keycap not correctly handled
+ https://bugs.webkit.org/show_bug.cgi?id=183246
+
+ Reviewed by Michael Catanzaro.
+
+ * fast/text/combining-enclosing-keycap-expected.txt: Added.
+ * platform/gtk/fast/text/combining-enclosing-keycap.html: Added.
+ * platform/gtk/TestExpectations:
+
+2018-03-01 Carlos Garcia Campos <[email protected]>
+
[FreeType] Remove FontPlatformData fallbacks
https://bugs.webkit.org/show_bug.cgi?id=183210
Added: releases/WebKitGTK/webkit-2.20/LayoutTests/fast/text/combining-enclosing-keycap.html (0 => 229273)
--- releases/WebKitGTK/webkit-2.20/LayoutTests/fast/text/combining-enclosing-keycap.html (rev 0)
+++ releases/WebKitGTK/webkit-2.20/LayoutTests/fast/text/combining-enclosing-keycap.html 2018-03-05 13:17:26 UTC (rev 229273)
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html>
+<head>
+</head>
+<body>
+<span>#⃣</span>
+<span>0⃣</span>
+<span>1⃣</span>
+<span>2⃣</span>
+<span>3⃣</span>
+<span>4⃣</span>
+<span>5⃣</span>
+<span>6⃣</span>
+<span>7⃣</span>
+<span>8⃣</span>
+<span>9⃣</span>
+</body>
+</html>
Modified: releases/WebKitGTK/webkit-2.20/LayoutTests/platform/gtk/TestExpectations (229272 => 229273)
--- releases/WebKitGTK/webkit-2.20/LayoutTests/platform/gtk/TestExpectations 2018-03-05 13:17:11 UTC (rev 229272)
+++ releases/WebKitGTK/webkit-2.20/LayoutTests/platform/gtk/TestExpectations 2018-03-05 13:17:26 UTC (rev 229273)
@@ -3390,22 +3390,6 @@
fast/scrolling/rtl-scrollbars-alternate-iframe-body-dir-attr-does-not-update-scrollbar-placement.html [ Pass ]
webkit.org/b/156437 fast/scrolling/rtl-scrollbars-listbox-scroll.html [ ImageOnlyFailure Pass ]
-# We don't support emoji genders, but half the reftests pass anyway.
-fast/text/emoji-gender-2-3.html [ Pass ]
-fast/text/emoji-gender-2-4.html [ Pass ]
-fast/text/emoji-gender-2-5.html [ Pass ]
-fast/text/emoji-gender-2-6.html [ Pass ]
-fast/text/emoji-gender-2-7.html [ Pass ]
-fast/text/emoji-gender-2-8.html [ Pass ]
-fast/text/emoji-gender-2-9.html [ Pass ]
-fast/text/emoji-gender-fe0f-3.html [ Pass ]
-fast/text/emoji-gender-fe0f-4.html [ Pass ]
-fast/text/emoji-gender-fe0f-5.html [ Pass ]
-fast/text/emoji-gender-fe0f-6.html [ Pass ]
-fast/text/emoji-gender-fe0f-7.html [ Pass ]
-fast/text/emoji-gender-fe0f-8.html [ Pass ]
-fast/text/emoji-gender-fe0f-9.html [ Pass ]
-
transitions/svg-text-shadow-transition.html [ Pass ]
webkit.org/b/155132 http/tests/security/contentSecurityPolicy/video-with-https-url-allowed-by-csp-media-src-star.html [ Pass ]
Added: releases/WebKitGTK/webkit-2.20/LayoutTests/platform/gtk/fast/text/combining-enclosing-keycap-expected.txt (0 => 229273)
--- releases/WebKitGTK/webkit-2.20/LayoutTests/platform/gtk/fast/text/combining-enclosing-keycap-expected.txt (rev 0)
+++ releases/WebKitGTK/webkit-2.20/LayoutTests/platform/gtk/fast/text/combining-enclosing-keycap-expected.txt 2018-03-05 13:17:26 UTC (rev 229273)
@@ -0,0 +1,59 @@
+layer at (0,0) size 800x600
+ RenderView at (0,0) size 800x600
+layer at (0,0) size 800x34
+ RenderBlock {HTML} at (0,0) size 800x34
+ RenderBody {BODY} at (8,8) size 784x18
+ RenderInline {SPAN} at (0,0) size 10x17
+ RenderText {#text} at (0,0) size 10x17
+ text run at (0,0) width 10: "#\x{20E3}"
+ RenderText {#text} at (10,0) size 4x17
+ text run at (10,0) width 4: " "
+ RenderInline {SPAN} at (0,0) size 10x17
+ RenderText {#text} at (14,0) size 10x17
+ text run at (14,0) width 10: "0\x{20E3}"
+ RenderText {#text} at (24,0) size 4x17
+ text run at (24,0) width 4: " "
+ RenderInline {SPAN} at (0,0) size 10x17
+ RenderText {#text} at (28,0) size 10x17
+ text run at (28,0) width 10: "1\x{20E3}"
+ RenderText {#text} at (38,0) size 4x17
+ text run at (38,0) width 4: " "
+ RenderInline {SPAN} at (0,0) size 10x17
+ RenderText {#text} at (42,0) size 10x17
+ text run at (42,0) width 10: "2\x{20E3}"
+ RenderText {#text} at (52,0) size 4x17
+ text run at (52,0) width 4: " "
+ RenderInline {SPAN} at (0,0) size 10x17
+ RenderText {#text} at (56,0) size 10x17
+ text run at (56,0) width 10: "3\x{20E3}"
+ RenderText {#text} at (66,0) size 4x17
+ text run at (66,0) width 4: " "
+ RenderInline {SPAN} at (0,0) size 10x17
+ RenderText {#text} at (70,0) size 10x17
+ text run at (70,0) width 10: "4\x{20E3}"
+ RenderText {#text} at (80,0) size 4x17
+ text run at (80,0) width 4: " "
+ RenderInline {SPAN} at (0,0) size 10x17
+ RenderText {#text} at (84,0) size 10x17
+ text run at (84,0) width 10: "5\x{20E3}"
+ RenderText {#text} at (94,0) size 4x17
+ text run at (94,0) width 4: " "
+ RenderInline {SPAN} at (0,0) size 10x17
+ RenderText {#text} at (98,0) size 10x17
+ text run at (98,0) width 10: "6\x{20E3}"
+ RenderText {#text} at (108,0) size 4x17
+ text run at (108,0) width 4: " "
+ RenderInline {SPAN} at (0,0) size 10x17
+ RenderText {#text} at (112,0) size 10x17
+ text run at (112,0) width 10: "7\x{20E3}"
+ RenderText {#text} at (122,0) size 4x17
+ text run at (122,0) width 4: " "
+ RenderInline {SPAN} at (0,0) size 10x17
+ RenderText {#text} at (126,0) size 10x17
+ text run at (126,0) width 10: "8\x{20E3}"
+ RenderText {#text} at (136,0) size 4x17
+ text run at (136,0) width 4: " "
+ RenderInline {SPAN} at (0,0) size 10x17
+ RenderText {#text} at (140,0) size 10x17
+ text run at (140,0) width 10: "9\x{20E3}"
+ RenderText {#text} at (0,0) size 0x0
Modified: releases/WebKitGTK/webkit-2.20/Source/WebCore/ChangeLog (229272 => 229273)
--- releases/WebKitGTK/webkit-2.20/Source/WebCore/ChangeLog 2018-03-05 13:17:11 UTC (rev 229272)
+++ releases/WebKitGTK/webkit-2.20/Source/WebCore/ChangeLog 2018-03-05 13:17:26 UTC (rev 229273)
@@ -1,5 +1,31 @@
2018-03-01 Carlos Garcia Campos <[email protected]>
+ REGRESSION(r222843): [HarfBuzz] Combining enclosed keycap not correctly handled
+ https://bugs.webkit.org/show_bug.cgi?id=183246
+
+ Reviewed by Michael Catanzaro.
+
+ We are not correctly handling the combining enclosed keycap since we switched to use
+ ComplexTextController. This is because fontForCombiningCharacterSequence() always returns the font of the first
+ character, without checking if that font can render the whole sequence or not. Before 222843, the shaper did
+ that check when creating the text runs. In this case the sequence was split and a different font was used for the
+ text and the mark. This patch makes fontForCombiningCharacterSequence() try to find a suitable font for the
+ whole sequence, first looking at the CSS fallbacks and finally at system ones. The result is much better than
+ the old one, because we use the same font for both the text and the mark. If there isn't any font to render the
+ mark, then we fallback to use the first character font, since we will end up rendering the missing glyph
+ character, it's better to use the same font than the first character one.
+
+ Test: fast/text/combining-enclosing-keycap.html
+
+ * platform/graphics/cairo/FontCairoHarfbuzzNG.cpp:
+ (WebCore::FontCascade::fontForCombiningCharacterSequence const): Check if the first charatcer font can render
+ the whole sequence, trying with fallbacks otherwise.
+ * platform/graphics/freetype/SimpleFontDataFreeType.cpp:
+ (WebCore::Font::canRenderCombiningCharacterSequence const): Check if the font face has glyphs for the whole
+ sequence not just the first character.
+
+2018-03-01 Carlos Garcia Campos <[email protected]>
+
[FreeType] Remove FontPlatformData fallbacks
https://bugs.webkit.org/show_bug.cgi?id=183210
Modified: releases/WebKitGTK/webkit-2.20/Source/WebCore/platform/graphics/cairo/FontCairoHarfbuzzNG.cpp (229272 => 229273)
--- releases/WebKitGTK/webkit-2.20/Source/WebCore/platform/graphics/cairo/FontCairoHarfbuzzNG.cpp 2018-03-05 13:17:11 UTC (rev 229272)
+++ releases/WebKitGTK/webkit-2.20/Source/WebCore/platform/graphics/cairo/FontCairoHarfbuzzNG.cpp 2018-03-05 13:17:26 UTC (rev 229273)
@@ -29,6 +29,7 @@
#if USE(CAIRO)
+#include "FontCache.h"
#include "SurrogatePairAwareTextIterator.h"
#include <unicode/normlzr.h>
@@ -58,7 +59,25 @@
if (!iterator.consume(character, clusterLength))
return nullptr;
- return glyphDataForCharacter(character, false, NormalVariant).font;
+ const Font* baseFont = glyphDataForCharacter(character, false, NormalVariant).font;
+ if (baseFont && (static_cast<int32_t>(clusterLength) == normalizedLength || baseFont->canRenderCombiningCharacterSequence(characters, length)))
+ return baseFont;
+
+ for (unsigned i = 0; !fallbackRangesAt(i).isNull(); ++i) {
+ const Font* fallbackFont = fallbackRangesAt(i).fontForCharacter(character);
+ if (!fallbackFont || fallbackFont == baseFont)
+ continue;
+
+ if (fallbackFont->canRenderCombiningCharacterSequence(characters, length))
+ return fallbackFont;
+ }
+
+ if (auto systemFallback = FontCache::singleton().systemFallbackForCharacters(m_fontDescription, baseFont, false, characters, length)) {
+ if (systemFallback->canRenderCombiningCharacterSequence(characters, length))
+ return systemFallback.get();
+ }
+
+ return baseFont;
}
} // namespace WebCore
Modified: releases/WebKitGTK/webkit-2.20/Source/WebCore/platform/graphics/freetype/SimpleFontDataFreeType.cpp (229272 => 229273)
--- releases/WebKitGTK/webkit-2.20/Source/WebCore/platform/graphics/freetype/SimpleFontDataFreeType.cpp 2018-03-05 13:17:11 UTC (rev 229272)
+++ releases/WebKitGTK/webkit-2.20/Source/WebCore/platform/graphics/freetype/SimpleFontDataFreeType.cpp 2018-03-05 13:17:26 UTC (rev 229273)
@@ -42,6 +42,7 @@
#include "GlyphBuffer.h"
#include "OpenTypeTypes.h"
#include "RefPtrCairo.h"
+#include "SurrogatePairAwareTextIterator.h"
#include "UTF16UChar32Iterator.h"
#include <cairo-ft.h>
#include <cairo.h>
@@ -199,8 +200,7 @@
UErrorCode error = U_ZERO_ERROR;
Vector<UChar, 4> normalizedCharacters(length);
int32_t normalizedLength = unorm_normalize(characters, length, UNORM_NFC, UNORM_UNICODE_3_2, &normalizedCharacters[0], length, &error);
- // Can't render if we have an error or no composition occurred.
- if (U_FAILURE(error) || (static_cast<size_t>(normalizedLength) == length))
+ if (U_FAILURE(error))
return false;
CairoFtFaceLocker cairoFtFaceLocker(m_platformData.scaledFont());
@@ -208,10 +208,16 @@
if (!face)
return false;
- if (FcFreeTypeCharIndex(face, normalizedCharacters[0]))
- addResult.iterator->value = true;
+ UChar32 character;
+ unsigned clusterLength = 0;
+ SurrogatePairAwareTextIterator iterator(normalizedCharacters.data(), 0, normalizedLength, normalizedLength);
+ for (iterator.advance(clusterLength); iterator.consume(character, clusterLength); iterator.advance(clusterLength)) {
+ if (!FcFreeTypeCharIndex(face, character))
+ return false;
+ }
- return addResult.iterator->value;
+ addResult.iterator->value = true;
+ return true;
}
#endif
