Bug 138906

Summary:

[iOS] Codepoints not associated with languages are drawn as boxes

Product:

WebKit

Reporter:

Myles C. Maxfield <mmaxfield>

Component:

New Bugs

Assignee:

Myles C. Maxfield <mmaxfield>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

dino, enrica, jonlee, simon.fraser, thorton, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

139134

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	darin: review+

Myles C. Maxfield

Reported 2014-11-19 19:06:11 PST

[iOS] Codepoints not associated with languages are drawn as boxes

Attachments
Patch (21.93 KB, patch) 2014-11-19 19:07 PST, Myles C. Maxfield	no flags	Details Formatted Diff Diff
Patch (21.85 KB, patch) 2014-11-20 21:36 PST, Myles C. Maxfield	no flags	Details Formatted Diff Diff
Patch (4.66 KB, patch) 2014-12-11 16:57 PST, Myles C. Maxfield	darin: review+	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Myles C. Maxfield

Comment 1 2014-11-19 19:07:15 PST

Created attachment 241919 [details] Patch

Myles C. Maxfield

Comment 2 2014-11-19 19:07:55 PST

<rdar://problem/18568920>

Myles C. Maxfield

Comment 3 2014-11-20 21:36:45 PST

Created attachment 242022 [details] Patch

Simon Fraser (smfr)

Comment 4 2014-11-20 21:40:34 PST

Comment on attachment 242022 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=242022&action=review > Source/WebCore/ChangeLog:8 > + Use CTFontCreateForString instead of hardcoded tables. What's the perf impact? > LayoutTests/ChangeLog:11 > + * platform/ios-simulator/fast/text/non-language-font-fallback-expected.html: Added. > + * platform/ios-simulator/fast/text/non-language-font-fallback.html: Added. Why does the test need to be iOS-specific?

Myles C. Maxfield

Comment 5 2014-12-01 11:37:10 PST

Comment on attachment 242022 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=242022&action=review >> LayoutTests/ChangeLog:11 >> + * platform/ios-simulator/fast/text/non-language-font-fallback.html: Added. > > Why does the test need to be iOS-specific? It hardcodes the platform fallback font.

Myles C. Maxfield

Comment 6 2014-12-05 16:46:08 PST

Comment on attachment 242022 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=242022&action=review >> Source/WebCore/ChangeLog:8 >> + Use CTFontCreateForString instead of hardcoded tables. > > What's the perf impact? 10 runs: With patch: Aggregate n mean std dev Cov Mean Time 10 1109.035 36.169 3.3% Square-Mean-Root Time 10 945.647 24.060 2.5% Geometric Mean Time 10 776.209 17.717 2.3% Total Time 10 64324.012 2097.807 3.3% Without patch: Aggregate n mean std dev Cov Mean Time 10 1102.113 26.177 2.4% Square-Mean-Root Time 10 947.070 21.834 2.3% Geometric Mean Time 10 781.305 19.639 2.5% Total Time 10 63922.530 1518.268 2.4% Pathological case: Without patch: Aggregate n mean std dev Cov Geometric Mean Time 20 497.801 21.473 4.3% Square-Mean-Root Time 20 497.801 21.473 4.3% Mean Time 20 497.801 21.473 4.3% Total Time 20 497.801 21.473 4.3% With patch: Aggregate n mean std dev Cov Geometric Mean Time 20 833.098 27.362 3.3% Square-Mean-Root Time 20 833.098 27.362 3.3% Mean Time 20 833.098 27.362 3.3% Total Time 20 833.098 27.362 3.3%

Myles C. Maxfield

Comment 7 2014-12-05 20:33:10 PST

See https://bugs.webkit.org/show_bug.cgi?id=139332 for a performance test

Myles C. Maxfield

Comment 8 2014-12-09 17:42:26 PST

Comment on attachment 242022 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=242022&action=review >>> Source/WebCore/ChangeLog:8 >>> + Use CTFontCreateForString instead of hardcoded tables. >> >> What's the perf impact? > > 10 runs: > With patch: > Aggregate n mean std dev Cov > Mean Time 10 1109.035 36.169 3.3% > Square-Mean-Root Time 10 945.647 24.060 2.5% > Geometric Mean Time 10 776.209 17.717 2.3% > Total Time 10 64324.012 2097.807 3.3% > Without patch: > Aggregate n mean std dev Cov > Mean Time 10 1102.113 26.177 2.4% > Square-Mean-Root Time 10 947.070 21.834 2.3% > Geometric Mean Time 10 781.305 19.639 2.5% > Total Time 10 63922.530 1518.268 2.4% > > > > > > Pathological case: > Without patch: > Aggregate n mean std dev Cov > Geometric Mean Time 20 497.801 21.473 4.3% > Square-Mean-Root Time 20 497.801 21.473 4.3% > Mean Time 20 497.801 21.473 4.3% > Total Time 20 497.801 21.473 4.3% > With patch: > Aggregate n mean std dev Cov > Geometric Mean Time 20 833.098 27.362 3.3% > Square-Mean-Root Time 20 833.098 27.362 3.3% > Mean Time 20 833.098 27.362 3.3% > Total Time 20 833.098 27.362 3.3% If I make the patch only call CTFontCreateForString when the language is unknown, I get these results for PLT (on a different device, so the numbers are not comparable to the previous comment): Without patch Aggregate n mean std dev Cov Mean Time 10 2583.726 41.309 1.6% Square-Mean-Root Time 10 2204.121 33.596 1.5% Geometric Mean Time 10 1802.632 29.113 1.6% Total Time 10 149856.118 2395.939 1.6% With patch Aggregate n mean std dev Cov Mean Time 10 2592.926 35.085 1.4% Square-Mean-Root Time 10 2219.600 24.063 1.1% Geometric Mean Time 10 1822.290 19.138 1.1% Total Time 10 150389.680 2034.925 1.4%

Myles C. Maxfield

Comment 9 2014-12-10 15:50:25 PST

Comment on attachment 242022 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=242022&action=review >>>> Source/WebCore/ChangeLog:8 >>>> + Use CTFontCreateForString instead of hardcoded tables. >>> >>> What's the perf impact? >> >> 10 runs: >> With patch: >> Aggregate n mean std dev Cov >> Mean Time 10 1109.035 36.169 3.3% >> Square-Mean-Root Time 10 945.647 24.060 2.5% >> Geometric Mean Time 10 776.209 17.717 2.3% >> Total Time 10 64324.012 2097.807 3.3% >> Without patch: >> Aggregate n mean std dev Cov >> Mean Time 10 1102.113 26.177 2.4% >> Square-Mean-Root Time 10 947.070 21.834 2.3% >> Geometric Mean Time 10 781.305 19.639 2.5% >> Total Time 10 63922.530 1518.268 2.4% >> >> >> >> >> >> Pathological case: >> Without patch: >> Aggregate n mean std dev Cov >> Geometric Mean Time 20 497.801 21.473 4.3% >> Square-Mean-Root Time 20 497.801 21.473 4.3% >> Mean Time 20 497.801 21.473 4.3% >> Total Time 20 497.801 21.473 4.3% >> With patch: >> Aggregate n mean std dev Cov >> Geometric Mean Time 20 833.098 27.362 3.3% >> Square-Mean-Root Time 20 833.098 27.362 3.3% >> Mean Time 20 833.098 27.362 3.3% >> Total Time 20 833.098 27.362 3.3% > > If I make the patch only call CTFontCreateForString when the language is unknown, I get these results for PLT (on a different device, so the numbers are not comparable to the previous comment): > Without patch > Aggregate n mean std dev Cov > Mean Time 10 2583.726 41.309 1.6% > Square-Mean-Root Time 10 2204.121 33.596 1.5% > Geometric Mean Time 10 1802.632 29.113 1.6% > Total Time 10 149856.118 2395.939 1.6% > With patch > Aggregate n mean std dev Cov > Mean Time 10 2592.926 35.085 1.4% > Square-Mean-Root Time 10 2219.600 24.063 1.1% > Geometric Mean Time 10 1822.290 19.138 1.1% > Total Time 10 150389.680 2034.925 1.4% Using SPI CTFontCreateForCharacters() isn't any better: Aggregate n mean std dev Cov Mean Time 10 2596.975 37.488 1.4% Square-Mean-Root Time 10 2220.777 34.736 1.6% Geometric Mean Time 10 1822.128 36.250 2.0% Total Time 10 150624.567 2174.284 1.4%

Myles C. Maxfield

Comment 10 2014-12-11 16:57:42 PST

Created attachment 243163 [details] Patch

Darin Adler

Comment 11 2014-12-15 09:01:27 PST

Comment on attachment 243163 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=243163&action=review > Source/WebCore/platform/graphics/ios/FontCacheIOS.mm:433 > + RetainPtr<CFStringRef> foundFontName = adoptCF(CTFontCopyPostScriptName(fallbackFont.get())); > + if (foundFontName) Seems like this could go inside the if.

Myles C. Maxfield

Comment 12 2014-12-15 11:04:33 PST

Committed r177292: <http://trac.webkit.org/changeset/177292>

Note You need to log in before you can comment on or make changes to this bug.