Bug 189448 – Synthetic bold additional advances need to be applied after shaping

• Component: Text
• Status: RESOLVED FIXED

[Myles C. Maxfield]

Osaka is totally busted

[Myles C. Maxfield]

Created attachment 349240 [details]
WIP

[Myles C. Maxfield]

<rdar://problem/40994395>

[EWS Watchlist]

Attachment 349240 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:9:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
ERROR: Source/WebCore/platform/graphics/WidthIterator.cpp:98:  Should have a space between // and comment  [whitespace/comments] [4]
Total errors found: 2 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Myles C. Maxfield]

There are two problems with Osaka:

1) Osaka changes the advance of every character significantly during shaping. We will slice and dice the text multiple ways during layout, and if we encounter a chunk of text of length 1, we don't run shaping. This is because Japanese text uses the simple code path, and, because shaping was intended only for kerning/ligatures, which don't need to apply kerning/ligatures when there is only a single character being measured. This is a problem because we often measure text "word" by "word" where "word" means "the text between adjacent line breaking opportunities." For Japanese, this usually happens every character, but sometimes it doesn't because of things like punctuation (U+3010).

The uploaded patch "fixes" this by simply running shaping even on 1-length TextRuns, which surely is a massive performance regression.

2) Synthetic bold affects layout in the complex text path, but not the simple one. In the complex text path, the width of every character gets inflated by the size of the synthetic bold offset. This is a problem because selecting text with the mouse causes us to use the complex text path, but the regular rendering causes us to use the simple text rendering (for Japanese) so they have to agree.

The uploaded patch fixes this by applying the offset even in the simple text codepath. This needs to be applied after shaping but before the charactersTreatedAsSpace stuff happens (since spaces don't get the inflated advance since they don't draw anything).

[EWS Watchlist]

Comment on attachment 349240 [details]
WIP

Attachment 349240 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/9137797

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 349246 [details]
Archive of layout-test-results from ews103 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 349240 [details]
WIP

Attachment 349240 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/9137810

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 349248 [details]
Archive of layout-test-results from ews106 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 349240 [details]
WIP

Attachment 349240 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/9137767

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 349249 [details]
Archive of layout-test-results from ews112 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 349240 [details]
WIP

Attachment 349240 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/9137894

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 349251 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.4

[Myles C. Maxfield]

Two other possibilities:

1) Don't use Osaka ever
2) Use Osaka-Mono instead of Osaka

[Myles C. Maxfield]

We can use https://bugs.webkit.org/show_bug.cgi?id=186804 for the single-character shaping, and this bug for the synthetic bold problem.

[Myles C. Maxfield]

Oh, I see. I wasn't quite right about Osaka & synthetic bold.

For our simple text codepath, we add the synthetic bold offset to each glyph's advance before shaping. Then, we run shaping on this previously-inflated glyphs and assume that the inflation will survive the transformation. However, Osaka disregards the input advances and just overwrites them totally. (We don't then re-apply the synthetic advance because we assume it survived.)

In the complex path, we create CTRuns from the original font, so we have to apply synthetic bold after shaping. Therefore, the simple & complex path disagree about the advances of Osaka.

The best solution is to apply the synthetic offset after shaping in both paths.

[Myles C. Maxfield]

Created attachment 349600 [details]
WIP

[Myles C. Maxfield]

Created attachment 349602 [details]
WIP

[Myles C. Maxfield]

Created attachment 349630 [details]
Patch

[Myles C. Maxfield]

Created attachment 349634 [details]
Patch

[Myles C. Maxfield]

Created attachment 349635 [details]
Patch

[EWS Watchlist]

Comment on attachment 349635 [details]
Patch

Attachment 349635 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/9200624

New failing tests:
fast/css/font-face-multiple-faces.html
fonts/monospace.html
fonts/cursive.html
fast/css/font-face-synthetic-bold-italic.html
fast/text/simple-synthetic-bold.html
fast/text/complex-synthetic-bold-space-width.html
fonts/fantasy.html
fast/text/atsui-multiple-renderers.html

[EWS Watchlist]

Created attachment 349645 [details]
Archive of layout-test-results from ews116 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 349635 [details]
Patch

Attachment 349635 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/9200727

New failing tests:
svg/W3C-SVG-1.1-SE/paths-dom-02-f.svg
svg/W3C-SVG-1.1-SE/struct-use-14-f.svg
fast/text/simple-synthetic-bold.html
svg/W3C-SVG-1.1-SE/filters-image-03-f.svg
svg/W3C-SVG-1.1-SE/pservers-pattern-03-f.svg
svg/W3C-SVG-1.1-SE/svgdom-over-01-f.svg

[EWS Watchlist]

Created attachment 349646 [details]
Archive of layout-test-results from ews204 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews204  Port: win-future  Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit

[EWS Watchlist]

Comment on attachment 349635 [details]
Patch

Attachment 349635 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/9201086

New failing tests:
fonts/fantasy.html
fast/text/simple-synthetic-bold.html

[EWS Watchlist]

Created attachment 349649 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.4

[EWS Watchlist]

Comment on attachment 349635 [details]
Patch

Attachment 349635 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/9201980

New failing tests:
fast/css/font-face-multiple-faces.html
fonts/monospace.html
fonts/cursive.html
fast/css/font-face-synthetic-bold-italic.html
fast/text/simple-synthetic-bold.html
fast/text/complex-synthetic-bold-space-width.html
fonts/fantasy.html
fast/text/atsui-multiple-renderers.html

[EWS Watchlist]

Created attachment 349657 [details]
Archive of layout-test-results from ews104 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 349635 [details]
Patch

Attachment 349635 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/9202619

New failing tests:
fast/css/font-face-multiple-faces.html
fonts/monospace.html
fonts/cursive.html
fast/css/font-face-synthetic-bold-italic.html
fast/text/simple-synthetic-bold.html
fast/text/complex-synthetic-bold-space-width.html
fonts/fantasy.html
fast/text/atsui-multiple-renderers.html

[EWS Watchlist]

Created attachment 349665 [details]
Archive of layout-test-results from ews103 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Myles C. Maxfield]

Created attachment 349681 [details]
Patch

[EWS Watchlist]

Comment on attachment 349681 [details]
Patch

Attachment 349681 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/9205075

New failing tests:
fonts/fantasy.html
fast/text/simple-synthetic-bold.html

[EWS Watchlist]

Created attachment 349692 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.4

[EWS Watchlist]

Comment on attachment 349681 [details]
Patch

Attachment 349681 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/9205533

New failing tests:
svg/text/select-textLength-spacingAndGlyphs-stretch-3.svg
svg/text/select-textLength-spacingAndGlyphs-squeeze-3.svg
svg/text/select-textLength-spacingAndGlyphs-stretch-2.svg
fast/text/simple-synthetic-bold.html
svg/text/select-textLength-spacingAndGlyphs-squeeze-1.svg
svg/text/select-textLength-spacingAndGlyphs-stretch-1.svg
svg/text/select-textLength-spacingAndGlyphs-squeeze-4.svg
svg/text/select-textLength-spacingAndGlyphs-squeeze-2.svg

[EWS Watchlist]

Created attachment 349694 [details]
Archive of layout-test-results from ews200 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews200  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[EWS Watchlist]

Comment on attachment 349681 [details]
Patch

Attachment 349681 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/9205622

New failing tests:
fast/css/font-face-weight-matching.html
fast/text/fallback-traits-fixup.html
fonts/monospace.html
fonts/cursive.html
fast/css/font-face-synthetic-bold-italic.html
fast/text/simple-synthetic-bold.html
fast/text/complex-synthetic-bold-space-width.html
fonts/fantasy.html
fast/text/atsui-multiple-renderers.html
fast/css/font-face-multiple-faces.html

[EWS Watchlist]

Created attachment 349703 [details]
Archive of layout-test-results from ews117 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 349681 [details]
Patch

Attachment 349681 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/9206335

New failing tests:
fast/css/font-face-weight-matching.html
fast/text/fallback-traits-fixup.html
fonts/monospace.html
fonts/cursive.html
fast/css/font-face-synthetic-bold-italic.html
fast/text/simple-synthetic-bold.html
fast/text/complex-synthetic-bold-space-width.html
fonts/fantasy.html
fast/text/atsui-multiple-renderers.html
fast/css/font-face-multiple-faces.html

[EWS Watchlist]

Created attachment 349705 [details]
Archive of layout-test-results from ews104 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 349681 [details]
Patch

Attachment 349681 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/9206904

New failing tests:
fast/css/font-face-weight-matching.html
fast/text/fallback-traits-fixup.html
fonts/monospace.html
fonts/cursive.html
fast/css/font-face-synthetic-bold-italic.html
fast/text/simple-synthetic-bold.html
fast/text/complex-synthetic-bold-space-width.html
fonts/fantasy.html
fast/text/atsui-multiple-renderers.html
fast/css/font-face-multiple-faces.html

[EWS Watchlist]

Created attachment 349710 [details]
Archive of layout-test-results from ews103 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Myles C. Maxfield]

(In reply to Build Bot from comment #40)
> Comment on attachment 349681 [details]
> Patch
> 
> Attachment 349681 [details] did not pass mac-ews (mac):
> Output: https://webkit-queues.webkit.org/results/9206904
> 
> New failing tests:
> fast/css/font-face-weight-matching.html
> fast/text/fallback-traits-fixup.html
> fonts/monospace.html
> fonts/cursive.html
> fast/css/font-face-synthetic-bold-italic.html
> fast/text/simple-synthetic-bold.html

I don't understand; that's not a test. https://trac.webkit.org/browser/webkit/trunk/LayoutTests/fast/text/simple-synthetic-bold.html

> fast/text/complex-synthetic-bold-space-width.html
> fonts/fantasy.html
> fast/text/atsui-multiple-renderers.html
> fast/css/font-face-multiple-faces.html

[Myles C. Maxfield]

(In reply to Myles C. Maxfield from comment #42)
> (In reply to Build Bot from comment #40)
> > Comment on attachment 349681 [details]
> > Patch
> > 
> > Attachment 349681 [details] did not pass mac-ews (mac):
> > Output: https://webkit-queues.webkit.org/results/9206904
> > 
> > New failing tests:
> > fast/css/font-face-weight-matching.html
> > fast/text/fallback-traits-fixup.html
> > fonts/monospace.html
> > fonts/cursive.html
> > fast/css/font-face-synthetic-bold-italic.html
> > fast/text/simple-synthetic-bold.html
> 
> I don't understand; that's not a test.
> https://trac.webkit.org/browser/webkit/trunk/LayoutTests/fast/text/simple-
> synthetic-bold.html

Oh wait, that's my new test. hahaha....

> 
> > fast/text/complex-synthetic-bold-space-width.html
> > fonts/fantasy.html
> > fast/text/atsui-multiple-renderers.html
> > fast/css/font-face-multiple-faces.html

[Myles C. Maxfield]

(In reply to Build Bot from comment #40)
> Comment on attachment 349681 [details]
> Patch
> 
> Attachment 349681 [details] did not pass mac-ews (mac):
> Output: https://webkit-queues.webkit.org/results/9206904
> 
> New failing tests:
> fast/css/font-face-weight-matching.html
> fast/text/fallback-traits-fixup.html
> fonts/monospace.html
> fonts/cursive.html
> fast/css/font-face-synthetic-bold-italic.html
> fast/text/simple-synthetic-bold.html
> fast/text/complex-synthetic-bold-space-width.html
> fonts/fantasy.html
> fast/text/atsui-multiple-renderers.html
> fast/css/font-face-multiple-faces.html

Unfortunately, all those tests pass on my High Sierra machine...

[Myles C. Maxfield]

(In reply to Myles C. Maxfield from comment #44)
> (In reply to Build Bot from comment #40)
> > Comment on attachment 349681 [details]
> > Patch
> > 
> > Attachment 349681 [details] did not pass mac-ews (mac):
> > Output: https://webkit-queues.webkit.org/results/9206904
> > 
> > New failing tests:
> > fast/css/font-face-weight-matching.html
> > fast/text/fallback-traits-fixup.html
> > fonts/monospace.html
> > fonts/cursive.html
> > fast/css/font-face-synthetic-bold-italic.html
> > fast/text/simple-synthetic-bold.html
> > fast/text/complex-synthetic-bold-space-width.html
> > fonts/fantasy.html
> > fast/text/atsui-multiple-renderers.html
> > fast/css/font-face-multiple-faces.html
> 
> Unfortunately, all those tests pass on my High Sierra machine...

Whoops, that's not true....

[Myles C. Maxfield]

Created attachment 349731 [details]
WIP

[Myles C. Maxfield]

Created attachment 349732 [details]
WIP

[Myles C. Maxfield]

Created attachment 349734 [details]
WIP

[Myles C. Maxfield]

Created attachment 349737 [details]
Need to figure out how ligatures should work

[Myles C. Maxfield]

Created attachment 349739 [details]
Need to figure out how ligatures should work

[Myles C. Maxfield]

Osaka counts as a user-installed font, so it isn't available in Safari. You have to use MiniBrowser to reproduce this.

[Myles C. Maxfield]

Created attachment 435524 [details]
Patch

Marking as r? despite this patch depending on https://bugs.webkit.org/show_bug.cgi?id=215643

[Myles C. Maxfield]

Created attachment 435525 [details]
Patch

[Myles C. Maxfield]

Lots of test failures :(

[Myles C. Maxfield]

Created attachment 435528 [details]
Patch

[Myles C. Maxfield]

I believe the tests are passing now. EWS is red because this patch depends on https://bugs.webkit.org/show_bug.cgi?id=215643

[Myles C. Maxfield]

Created attachment 436111 [details]
Patch

[Myles C. Maxfield]

Created attachment 436125 [details]
Patch

[Myles C. Maxfield]

Created attachment 436127 [details]
Patch

[Myles C. Maxfield]

fonts/cursive.html is a progression

[Myles C. Maxfield]

Created attachment 436132 [details]
Patch

[Myles C. Maxfield]

Created attachment 436134 [details]
Patch

[Myles C. Maxfield]

Created attachment 436141 [details]
Patch

[Myles C. Maxfield]

The Osaka test hangs because Osaka is a mobileasset font, and the download prompt blocks for user input.

The prompt is coming from autoActivateFont(). IIRC the way we got around this before was to add kCTFontEnabledAttribute:kCFBooleanTrue to the attribute dictionary.

[Myles C. Maxfield]

Created attachment 436150 [details]
Patch

[Myles C. Maxfield]

Created attachment 436152 [details]
Patch

[Myles C. Maxfield]

Some tests are failing because https://bugs.webkit.org/show_bug.cgi?id=206208 has not landed yet.

[Myles C. Maxfield]

Created attachment 436216 [details]
Will be some red because bug 229391 isn't landed yet

[Myles C. Maxfield]

Created attachment 436607 [details]
Patch for committing

[EWS]

Committed r281687 (241037@main): <https://commits.webkit.org/241037@main>

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