Bug 140015 – Remove GlyphPageTree

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Antti Koivisto]

This arcane data structure maps characters to glyphs taking font fallbacks into account. We can replace it with a simpler and more efficient caching scheme.

[Antti Koivisto]

Created attachment 243844 [details]
wip

[WebKit Commit Bot]

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


ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:196:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:255:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 2 in 10 files


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

[Antti Koivisto]

Created attachment 243846 [details]
wip2

[WebKit Commit Bot]

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


ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:196:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:255:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 2 in 12 files


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

[Build Bot]

Comment on attachment 243846 [details]
wip2

Attachment 243846 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5704538879164416

New failing tests:
fast/css/font-face-unicode-range.html
editing/deleting/5144139-2.html
http/tests/webfont/fallback-font-while-loading.html
fast/css/font-face-multiple-ranges-for-unicode-range.html
svg/custom/svg-fonts-segmented.xhtml
fast/css/font-face-implicit-local-font.html
transforms/2d/hindi-rotated.html
fast/css/font-face-download-error.html
fast/css/font-face-locally-installed.html
svg/custom/glyph-selection-non-bmp.svg
svg/custom/glyph-selection-bidi-mirror.svg
svg/custom/glyph-selection-arabic-forms.svg

[Build Bot]

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

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

[Build Bot]

Comment on attachment 243846 [details]
wip2

Attachment 243846 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5125486556479488

New failing tests:
fast/css/font-face-unicode-range.html
editing/deleting/5144139-2.html
http/tests/webfont/fallback-font-while-loading.html
fast/css/font-face-multiple-ranges-for-unicode-range.html
svg/custom/svg-fonts-segmented.xhtml
fast/css/font-face-implicit-local-font.html
transforms/2d/hindi-rotated.html
fast/css/font-face-download-error.html
fast/css/font-face-locally-installed.html
svg/custom/glyph-selection-non-bmp.svg
svg/custom/glyph-selection-bidi-mirror.svg
svg/custom/glyph-selection-arabic-forms.svg

[Build Bot]

Created attachment 243849 [details]
Archive of layout-test-results from ews102 for mac-mountainlion

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

[Myles C. Maxfield]

Woah.

[Antti Koivisto]

Created attachment 243889 [details]
wip3

[WebKit Commit Bot]

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


ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:196:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:255:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 2 in 18 files


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

[Antti Koivisto]

Created attachment 243890 [details]
wip4

[WebKit Commit Bot]

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


ERROR: Source/WebCore/platform/graphics/mac/GlyphPageMac.cpp:84:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:196:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:255:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 3 in 19 files


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

[Build Bot]

Comment on attachment 243890 [details]
wip4

Attachment 243890 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5070704785489920

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 243892 [details]
Archive of layout-test-results from ews103 for mac-mountainlion

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

[Build Bot]

Comment on attachment 243890 [details]
wip4

Attachment 243890 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6574334583767040

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 243893 [details]
Archive of layout-test-results from ews107 for mac-mountainlion-wk2

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

[Antti Koivisto]

Created attachment 243894 [details]
patch

[WebKit Commit Bot]

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


ERROR: Source/WebCore/platform/graphics/mac/GlyphPageMac.cpp:84:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:196:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:255:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 3 in 22 files


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

[Build Bot]

Comment on attachment 243894 [details]
patch

Attachment 243894 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6376431214919680

New failing tests:
http/tests/webfont/fallback-font-while-loading.html

[Build Bot]

Created attachment 243898 [details]
Archive of layout-test-results from ews101 for mac-mountainlion

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

[Antti Koivisto]

Created attachment 243900 [details]
patch

[WebKit Commit Bot]

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


ERROR: Source/WebCore/platform/graphics/mac/GlyphPageMac.cpp:84:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:198:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:260:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
ERROR: Source/WebKit2/WebProcess/WebProcess.cpp:73:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 4 in 24 files


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

[Antti Koivisto]

Created attachment 243903 [details]
patch

[WebKit Commit Bot]

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


ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:198:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:260:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 2 in 26 files


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

[Build Bot]

Comment on attachment 243903 [details]
patch

Attachment 243903 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6568718511374336

New failing tests:
mathml/opentype/munderover-layout-resize.html

[Build Bot]

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

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

[Darin Adler]

Comment on attachment 243903 [details]
patch

Any concern about the test failures?

[Antti Koivisto]

Yeah, looks related. Can't repro locally easily. I'll figure it out before landing.

Exception Type:  EXC_BAD_ACCESS (SIGSEGV)
Exception Codes: KERN_INVALID_ADDRESS at 0x00000000000004c8

CRASHING TEST: mathml/opentype/munderover-layout-resize.html

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.WebCore             	0x000000010c33d358 WebCore::RenderMathMLOperator::advanceForGlyph(WebCore::GlyphData const&) const + 24 (SimpleFontData.h:165)
1   com.apple.WebCore             	0x000000010c33c92e WebCore::RenderMathMLOperator::updateStyle() + 446 (RenderMathMLOperator.h:68)
2   com.apple.WebCore             	0x000000010c33ea87 WebCore::RenderMathMLOperator::rebuildTokenContent(WTF::String const&) + 343 (RenderObject.h:943)
3   com.apple.WebCore             	0x000000010c33b88f WebCore::RenderMathMLOperator::RenderMathMLOperator(WebCore::MathMLElement&, WTF::Ref<WebCore::RenderStyle>&&) + 207 (RefPtr.h:59)
4   com.apple.WebCore             	0x000000010c1afba5 WebCore::MathMLTextElement::createElementRenderer(WTF::Ref<WebCore::RenderStyle>&&) + 133 (RenderPtr.h:158)

[Antti Koivisto]

That's a clean null despite the high offset.

[Antti Koivisto]

Created attachment 243914 [details]
patch

[WebKit Commit Bot]

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


ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:203:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:262:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 2 in 26 files


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

[Build Bot]

Comment on attachment 243914 [details]
patch

Attachment 243914 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5378610990940160

New failing tests:
fast/css/font-face-implicit-local-font.html
fast/css/font-face-weight-matching.html
fast/css/font-face-multiple-ranges-for-unicode-range.html
fast/css/font-face-multiple-faces.html
fast/css/font-face-unicode-range.html
fast/css/font-face-in-media-rule.html
fast/css/font-face-default-font.html
svg/custom/font-face-cascade-order.svg
fast/css/font-face-download-error.html
fast/css/font-face-locally-installed.html

[Build Bot]

Created attachment 243915 [details]
Archive of layout-test-results from ews102 for mac-mountainlion

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

[Build Bot]

Comment on attachment 243914 [details]
patch

Attachment 243914 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6236357164793856

New failing tests:
fast/css/font-face-implicit-local-font.html
fast/css/font-face-multiple-faces.html
fast/css/font-face-multiple-ranges-for-unicode-range.html
fast/css/font-face-weight-matching.html
fast/css/font-face-unicode-range.html
fast/css/font-face-in-media-rule.html
fast/css/font-face-default-font.html
svg/custom/font-face-cascade-order.svg
fast/css/font-face-download-error.html
fast/css/font-face-locally-installed.html

[Build Bot]

Created attachment 243916 [details]
Archive of layout-test-results from ews107 for mac-mountainlion-wk2

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

[Antti Koivisto]

Created attachment 243929 [details]
another

[WebKit Commit Bot]

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


ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:203:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:263:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 2 in 26 files


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

[Antti Koivisto]

https://trac.webkit.org/r177876

[Antti Koivisto]

And https://trac.webkit.org/r177878

[Antti Koivisto]

Also http://trac.webkit.org/changeset/177877

[Alexey Proskuryakov]

This broke another test on bots: fonts/font-fallback-prefers-pictographs.html

WE are actually getting the wrong glyph, see https://build.webkit.org/results/Apple%20MountainLion%20Release%20WK2%20(Tests)/r177878%20(23244)/fonts/font-fallback-prefers-pictographs-diffs.html

[Alexey Proskuryakov]

And by "this", I mean the latest follow-up, r177878.

[Antti Koivisto]

https://trac.webkit.org/r177881 should help.