Currently only English can use glyphToWidth cache so another language's text layout and drawing performance is not good.
Created attachment 276820 [details] Patch
Comment on attachment 276820 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=276820&action=review > Source/WebCore/platform/graphics/GlyphMetricsMap.h:109 > + return page; Why do you skip the page filling for the non-primary pages, but not skip page filling for the primary page? These two should be the same (the zero page is just used as a cache)
(In reply to comment #2) > Comment on attachment 276820 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=276820&action=review > > > Source/WebCore/platform/graphics/GlyphMetricsMap.h:109 > > + return page; > > Why do you skip the page filling for the non-primary pages, but not skip > page filling for the primary page? These two should be the same (the zero > page is just used as a cache) Also, please add a test.
(In reply to comment #2) > Comment on attachment 276820 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=276820&action=review > > > Source/WebCore/platform/graphics/GlyphMetricsMap.h:109 > > + return page; > > Why do you skip the page filling for the non-primary pages, but not skip > page filling for the primary page? These two should be the same (the zero > page is just used as a cache) Current patch doesn't skip the page filling for the non-primary pages. I just prevent the page re-filling that already filled when we create it. I think the root cause of current issue is that we always re-filling non-primary pages even if we set glyph's width in Font::widthForGlyph()
(In reply to comment #3) > (In reply to comment #2) > > Comment on attachment 276820 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=276820&action=review > > > > > Source/WebCore/platform/graphics/GlyphMetricsMap.h:109 > > > + return page; > > > > Why do you skip the page filling for the non-primary pages, but not skip > > page filling for the primary page? These two should be the same (the zero > > page is just used as a cache) > > Also, please add a test. GlyphMetricsMap is not public API and it is related to Cache operation. Do you have idea to make a test?
(In reply to comment #5) > (In reply to comment #3) > > (In reply to comment #2) > > > Comment on attachment 276820 [details] > > > Patch > > > > > > View in context: > > > https://bugs.webkit.org/attachment.cgi?id=276820&action=review > > > > > > > Source/WebCore/platform/graphics/GlyphMetricsMap.h:109 > > > > + return page; > > > > > > Why do you skip the page filling for the non-primary pages, but not skip > > > page filling for the primary page? These two should be the same (the zero > > > page is just used as a cache) > > > > Also, please add a test. > > GlyphMetricsMap is not public API and it is related to Cache operation. > Do you have idea to make a test? I think we should make a layout test (not a unit test). Therefore, we should find a way that a webpage can cause this code to be invoked, and we should make a test page that does so, and make sure the result is as expected. If you have more questions about how to do this, I am happy to answer them.