Bug 228097 - Characters with no fonts that support them are drawn as two .notdef glyphs in the fast text codepath
Summary: Characters with no fonts that support them are drawn as two .notdef glyphs in...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords: InRadar
: 223308 (view as bug list)
Depends on:
Blocks:
 
Reported: 2021-07-19 19:19 PDT by Myles C. Maxfield
Modified: 2021-07-22 10:59 PDT (History)
6 users (show)

See Also:


Attachments
Patch (4.22 KB, patch)
2021-07-19 19:20 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (5.54 KB, patch)
2021-07-19 21:05 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (5.49 KB, patch)
2021-07-19 21:07 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (5.56 KB, patch)
2021-07-19 22:16 PDT, Myles C. Maxfield
zalan: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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. ***