New issue
Advanced search Search tips

Issue 636347 link

Starred by 1 user
Status: Fixed
Owner:
drott@chromium.org
Closed: Aug 2017
Cc:
behdad@chromium.org
tzik@chromium.org
kojii@chromium.org
e...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Shaping process may try to shape with identical font several times

Project Member Reported by drott@chromium.org, Aug 10 2016 
HarfBuzzShaper asks FontFallbackIterator::next for a font by calling next()  each time it is done shaping with this font and still has non-rendered glyphs. Depending on what the font-stack, user-preference font, and fallback fonts returned by the system are, the shaper may try to shape with the same font several times, not ending up finding a result before the LastResort font.

Since HarfBuzzFaces are internally cached and unified using Skia unique SkTypeface IDs, we can filter for SkTypeFace uniqueIds earlier in FontFallbackIterator and not shape redundantly.

Examples would be:
Empty font-family CSS, but CSS code seems to set the default serif font before we reach shaping, then we try with that, and afterwards attempt to shape with the users default font again, which is the same, Times New Roman in both cases. Similarly, a font-family: that has Times or Times New Roman which we alias to the same font, etc.

Changing this should help performance-wise especially on pages where we fallback from web fonts to the default webkit-serif and preference font before attempting system fallbakc.
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 10 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5fdb26babe2d64e6f09175edffd5efe4abf220d8

commit 5fdb26babe2d64e6f09175edffd5efe4abf220d8
Author: drott <drott@chromium.org>
Date: Wed Aug 10 22:29:27 2016

Skip redundant fonts returned by FontFallbackIterator

HarfBuzzShaper asks FontFallbackIterator::next for a font by calling
next() each time it is done shaping with this font and still has
non-rendered glyphs. Depending on what the font-stack, user-preference
font, and fallback fonts returned by the system are, the shaper may try
to shape with the same font several times, and still end up not finding a
result before the LastResort font.

Since HarfBuzzFace objects are cached internally to that class and
unified using Skia unique SkTypeface IDs, we can filter for SkTypeFace
uniqueIds earlier in FontFallbackIterator and skip redundant shaping.

Examples would be:
Empty font-family CSS, but CSS code seems to set the default serif font
before we reach shaping, then we try with that, and afterwards attempt
to shape with the users default font again, which is the same, Times New
Roman in both cases. Similarly, a font-family: that has Times or Times
New Roman which we alias to the same font, etc.

BUG= 636347 
TEST=Manually tested this by adding debug code to HarfBuzzShaper and
detecting incoming duplicates.

Review-Url: https://codereview.chromium.org/2230233002
Cr-Commit-Position: refs/heads/master@{#411164}

[modify] https://crrev.com/5fdb26babe2d64e6f09175edffd5efe4abf220d8/third_party/WebKit/Source/platform/fonts/FontDataForRangeSet.h
[modify] https://crrev.com/5fdb26babe2d64e6f09175edffd5efe4abf220d8/third_party/WebKit/Source/platform/fonts/FontFallbackIterator.cpp
[modify] https://crrev.com/5fdb26babe2d64e6f09175edffd5efe4abf220d8/third_party/WebKit/Source/platform/fonts/FontFallbackIterator.h
[modify] https://crrev.com/5fdb26babe2d64e6f09175edffd5efe4abf220d8/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaper.cpp
[modify] https://crrev.com/5fdb26babe2d64e6f09175edffd5efe4abf220d8/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaper.h

Comment 2 by e...@chromium.org, Aug 30 2017

Status: Fixed (was: Started)
►

Sign in to add a comment