Skip to content

Commit

Permalink
Select primary font for missing glyph for PUA characters
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=273233
rdar://124036223

Reviewed by Antti Koivisto.

We used to render the missing glyph for PUA characters (see [1])
with the lastResortFontFallback, instead of the primary font.
This is done by FontCascadeFonts::glyphDataForVariant(...)
producing a GlyphData object with such font for the missing glyph (glyph 0).

The ComplexTextController, which handles complex text, would always
use the primary font for getting metrics for the missing glyph. When rendering
PUA characters we would use the simple path (WidthIterator is used instead
of ComplexTextController), selecting the last resort font both for layout and painting.

However, when computing the caret position, we still use the ComplexTextController
because WidthIterator can't handle partial runs with ligatures (for example).

This would result in WidthIterator being used for layout/painting and selecting the
lastResortFont, and ComplexTextController being used for calculating the caret position,
selecting the primary font.

We are fixing it by updating the special case for PUA character and selecting
the primary font for such case, instead of the lastResortFont. This also has
the benefit of enforcing interoperability with other browser, as Blink
and Gecko seem to also use the primary font selected by the author.

[1] https://commits.webkit.org/269524@main

* LayoutTests/platform/mac/fast/text/softbank-emoji-expected.txt:
* LayoutTests/platform/wpe/fast/text/softbank-emoji-expected.txt:
- Rebasing this test since they contain PUA characters and font will change.

* LayoutTests/TestExpectations:
- We should pass this test now.

* Source/WebCore/platform/graphics/FontCascadeFonts.cpp:
(WebCore::FontCascadeFonts::glyphDataForVariant):
- Using primary font instead of last resort font for glyph 0 when PUA characters can't be mapped by selected fonts.

Canonical link: https://commits.webkit.org/278221@main
  • Loading branch information
vitorroriz committed May 1, 2024
1 parent cd68eda commit df0f175
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 16 deletions.
2 changes: 0 additions & 2 deletions LayoutTests/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -7454,5 +7454,3 @@ webkit.org/b/272417 imported/w3c/web-platform-tests/svg/path/property/priority.s

# re-import css/css-align WPT failure
webkit.org/b/271692 imported/w3c/web-platform-tests/css/css-align/blocks/align-content-block-break-overflow-020.html [ ImageOnlyFailure ]

webkit.org/b/273233 imported/w3c/web-platform-tests/css/css-fonts/matching/font-unicode-PUA-primary-font.html [ Skip ]
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ layer at (0,0) size 800x246
RenderBody {BODY} at (8,8) size 784x230
RenderBlock {DIV} at (0,0) size 784x230
RenderBlock {DIV} at (0,0) size 784x115
RenderText {#text} at (0,0) size 256x115
text run at (0,0) width 256: "\x{E001} a \x{E001}"
RenderText {#text} at (0,0) size 238x115
text run at (0,0) width 238: "\x{E001} a \x{E001}"
RenderBlock {DIV} at (0,115) size 784x115
RenderText {#text} at (0,0) size 73x115
text run at (0,0) width 73: "\x{E001}"
RenderText {#text} at (0,0) size 64x115
text run at (0,0) width 64: "\x{E001}"
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ layer at (0,0) size 800x246
RenderBody {BODY} at (8,8) size 784x230
RenderBlock {DIV} at (0,0) size 784x230
RenderBlock {DIV} at (0,0) size 784x115
RenderText {#text} at (0,1) size 268x112
text run at (0,1) width 268: "\x{E001} a \x{E001}"
RenderText {#text} at (0,1) size 262x112
text run at (0,1) width 262: "\x{E001} a \x{E001}"
RenderBlock {DIV} at (0,115) size 784x115
RenderText {#text} at (0,1) size 78x112
text run at (0,1) width 78: "\x{E001}"
RenderText {#text} at (0,1) size 75x112
text run at (0,1) width 75: "\x{E001}"
8 changes: 2 additions & 6 deletions Source/WebCore/platform/graphics/FontCascadeFonts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -485,12 +485,8 @@ GlyphData FontCascadeFonts::glyphDataForVariant(char32_t character, const FontCa
// http://www.unicode.org/Public/MAPPINGS/VENDORS/APPLE/CORPCHAR.TXT
shouldCheckForPrivateUseAreaCharacters = character != 0xF8FF;
#endif
if (shouldCheckForPrivateUseAreaCharacters && isPrivateUseAreaCharacter(character)) {
auto font = FontCache::forCurrentThread().lastResortFallbackFont(description);
GlyphData glyphData(0, font.ptr());
m_systemFallbackFontSet.add(WTFMove(font));
return glyphData; // 0 is the font's reserved .notdef glyph
}
if (shouldCheckForPrivateUseAreaCharacters && isPrivateUseAreaCharacter(character))
return { 0, &primaryFont(description) }; // 0 is the font's reserved .notdef glyph

return glyphDataForSystemFallback(character, description, variant, resolvedEmojiPolicy, fallbackVisibility == FallbackVisibility::Invisible);
}
Expand Down

0 comments on commit df0f175

Please sign in to comment.