Bug 221356 - Supplementary code points (U+10000 - U+10FFFF) are not shaped correctly in the fast text codepath
Summary: Supplementary code points (U+10000 - U+10FFFF) are not shaped correctly in th...
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
Depends on:
Blocks:
 
Reported: 2021-02-03 13:27 PST by Myles C. Maxfield
Modified: 2021-03-04 11:11 PST (History)
3 users (show)

See Also:


Attachments
Needs test (1.87 KB, patch)
2021-02-03 13:31 PST, Myles C. Maxfield
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (3.63 KB, patch)
2021-02-03 15:42 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (3.63 KB, patch)
2021-02-03 15:46 PST, Myles C. Maxfield
zalan: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch for committing (4.07 KB, patch)
2021-02-03 20:19 PST, Myles C. Maxfield
no flags 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-02-03 13:27:43 PST
Supplementary code points (U+10000 - U+10FFFF) are not shaped correctly in the fast text codepath
Comment 1 Myles C. Maxfield 2021-02-03 13:31:06 PST
Created attachment 419182 [details]
Needs test
Comment 2 Myles C. Maxfield 2021-02-03 13:31:41 PST
<rdar://problem/72555297>
Comment 3 Myles C. Maxfield 2021-02-03 15:42:39 PST
Created attachment 419196 [details]
Patch
Comment 4 Myles C. Maxfield 2021-02-03 15:46:05 PST
Created attachment 419198 [details]
Patch
Comment 5 EWS 2021-02-03 17:53:06 PST
Found 18 new test failures: fast/text/complex-small-caps-non-bmp-capitalize.html, fast/text/multiple-codeunit-vertical-upright-2.html, fast/text/multiple-codeunit-vertical-upright.html, imported/w3c/web-platform-tests/mathml/relations/css-styling/mathvariant-bold-fraktur.html, imported/w3c/web-platform-tests/mathml/relations/css-styling/mathvariant-bold-italic.html, imported/w3c/web-platform-tests/mathml/relations/css-styling/mathvariant-bold-sans-serif.html, imported/w3c/web-platform-tests/mathml/relations/css-styling/mathvariant-bold-script.html, imported/w3c/web-platform-tests/mathml/relations/css-styling/mathvariant-fraktur.html, imported/w3c/web-platform-tests/mathml/relations/css-styling/mathvariant-initial.html, imported/w3c/web-platform-tests/mathml/relations/css-styling/mathvariant-looped.html ...
Comment 6 Myles C. Maxfield 2021-02-03 20:19:57 PST
Created attachment 419223 [details]
Patch for committing
Comment 7 Darin Adler 2021-02-04 12:15:47 PST
Comment on attachment 419198 [details]
Patch

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

> Source/WebCore/platform/graphics/WidthIterator.cpp:265
> +        if (character >= 0x10000)

I think it would be more elegant if this used U_IS_SUPPLEMENTARY or !U_IS_BMP. Not really sure how much we want to lean into using the semantically more clear macros over the mechanically more clear numeric comparisons.
Comment 8 Darin Adler 2021-02-04 12:17:36 PST
Comment on attachment 419198 [details]
Patch

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

>> Source/WebCore/platform/graphics/WidthIterator.cpp:265
>> +        if (character >= 0x10000)
> 
> I think it would be more elegant if this used U_IS_SUPPLEMENTARY or !U_IS_BMP. Not really sure how much we want to lean into using the semantically more clear macros over the mechanically more clear numeric comparisons.

Or maybe this should be checking advanceLength or clusterLength (they are the same, why do we have both?) instead of character?
Comment 9 Myles C. Maxfield 2021-02-04 12:20:28 PST
(In reply to Darin Adler from comment #7)
> Comment on attachment 419198 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=419198&action=review
> 
> > Source/WebCore/platform/graphics/WidthIterator.cpp:265
> > +        if (character >= 0x10000)
> 
> I think it would be more elegant if this used U_IS_SUPPLEMENTARY or
> !U_IS_BMP. Not really sure how much we want to lean into using the
> semantically more clear macros over the mechanically more clear numeric
> comparisons.

I actually already did this in the patch for committing :)
Comment 10 Darin Adler 2021-02-04 13:05:01 PST
(In reply to Myles C. Maxfield from comment #9)
> I actually already did this in the patch for committing :)

Excellent!
Comment 11 EWS 2021-02-04 13:28:34 PST
Committed r272379: <https://trac.webkit.org/changeset/272379>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 419223 [details].