Bug 228097

Summary: Characters with no fonts that support them are drawn as two .notdef glyphs in the fast text codepath
Product: WebKit Reporter: Myles C. Maxfield <mmaxfield>
Component: New BugsAssignee: Myles C. Maxfield <mmaxfield>
Status: RESOLVED FIXED    
Severity: Normal CC: dino, jonlee, simon.fraser, thorton, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch zalan: review+

Description Myles C. Maxfield 2021-07-19 19:19:20 PDT
Characters with no fonts that support them are drawn as two .notdef glyphs in the fast text codepath
Comment 1 Myles C. Maxfield 2021-07-19 19:20:07 PDT
Created attachment 433844 [details]
Patch
Comment 2 Myles C. Maxfield 2021-07-19 19:20:10 PDT
<rdar://problem/80798113>
Comment 3 Myles C. Maxfield 2021-07-19 19:21:44 PDT
Comment on attachment 433844 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=433844&action=review

> Source/WebCore/platform/graphics/WidthIterator.cpp:274
> +        if (!U_IS_BMP(character) && glyph)

I'm not sure this is correct. Shaping expects the number of glyphs to equal the length of the string.
Comment 4 Myles C. Maxfield 2021-07-19 21:05:56 PDT
Created attachment 433847 [details]
Patch
Comment 5 Myles C. Maxfield 2021-07-19 21:07:33 PDT
Created attachment 433848 [details]
Patch
Comment 6 Myles C. Maxfield 2021-07-19 21:45:27 PDT
Comment on attachment 433848 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=433848&action=review

> Source/WebCore/ChangeLog:3
> +        Characters with no fonts that support them are drawn as two .notdef glyphs in the fast text codepath

REGRESSION(r272379):

> LayoutTests/ChangeLog:3
> +        Characters with no fonts that support them are drawn as two .notdef glyphs in the fast text codepath

REGRESSION(r272379):
Comment 7 Myles C. Maxfield 2021-07-19 22:16:23 PDT
Created attachment 433849 [details]
Patch
Comment 8 zalan 2021-07-20 07:00:02 PDT
Comment on attachment 433849 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=433849&action=review

> Source/WebCore/platform/graphics/WidthIterator.cpp:115
> +            if (!glyphBuffer.glyphAt(i) && !glyphBuffer.glyphAt(i + 1)) {

I assume these surrogates come in pairs and the i + 1 derefs are safe.
Comment 9 Simon Fraser (smfr) 2021-07-20 09:34:29 PDT
Comment on attachment 433849 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=433849&action=review

> Source/WebCore/platform/graphics/WidthIterator.cpp:121
> +                            continue;

This continue does nothing.
Comment 10 Myles C. Maxfield 2021-07-20 13:47:46 PDT
Committed r280103 (239820@main): <https://commits.webkit.org/239820@main>
Comment 11 Myles C. Maxfield 2021-07-20 13:48:48 PDT
Comment on attachment 433849 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=433849&action=review

>> Source/WebCore/platform/graphics/WidthIterator.cpp:115
>> +            if (!glyphBuffer.glyphAt(i) && !glyphBuffer.glyphAt(i + 1)) {
> 
> I assume these surrogates come in pairs and the i + 1 derefs are safe.

The for loop has "i < glyphBuffer.size() - 1"

They're not guaranteed to come in pairs, but when they do, that's the case where we get the double .notdef glyphs. If they don't come in pairs, we don't need to strip them out.
Comment 12 Myles C. Maxfield 2021-07-22 10:59:07 PDT
*** Bug 223308 has been marked as a duplicate of this bug. ***