Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(430)

Issue 1806363002: Revert of Shape unicode-range: font faces in only one iteration (Closed)

Created:
4 years, 9 months ago by fgorski
Modified:
4 years, 9 months ago
Reviewers:
drott, eae, behdad
CC:
darktears, apavlov+blink_chromium.org, blink-reviews, blink-reviews-css, blink-reviews-platform-graphics_chromium.org, Rik, chromium-reviews, danakj+watch_chromium.org, dglazkov+blink, dshwang, drott+blinkwatch_chromium.org, krit, f(malita), jbroman, Justin Novosad, kinuko+watch, pdr+graphicswatchlist_chromium.org, rwlbuis, Stephen Chennney
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Shape unicode-range: font faces in only one iteration (patchset #7 id:180001 of https://codereview.chromium.org/1806653002/ ) Reason for revert: Reverting on a suspicion that this is a root cause to the failures of webkit_unit_tests on Webkit Android (Nexus4) bots, e.g. https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Android%20%28Nexus4%29/builds/44725 Original issue's description: > Shape unicode-range: font faces in only one iteration > > Previously, we incorrectly split CSS composite faces into multiple faces > for shaping, one for each comma-separated entry of unicode-range:. This > breaks shaping of ligatures and other features when the characters that > ought to be shaped combined were in different unicode-range entries. It > is also inefficient for subsetted web fonts that use unicode-range: > extensively, for example Google Fonts and Adobe TypeKit. > > The fix is to transfer the UnicodeRangeSet information from CSSFontFace > to HarfBuzzFace and only restrict the glyph lookup function to the whole > unicode-range information, instead of restricting it to a single entry > and shaping multiple times with the same face. This should have a slight > performance benefit as well. > > BUG=583450 > TEST=fast/css/font-face-unicode-range-ligatures.html > R=eae, behdad > > Committed: https://crrev.com/9694005f93116f9c9cc73fa99132fa4475a0cdab > Cr-Commit-Position: refs/heads/master@{#381683} TBR=behdad@chromium.org,eae@chromium.org,drott@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=583450 Committed: https://crrev.com/60adcbb3ec944fa45c3b7cd0c45b38b1120e7b08 Cr-Commit-Position: refs/heads/master@{#381736}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+274 lines, -331 lines) Patch
D third_party/WebKit/LayoutTests/fast/css/font-face-unicode-range-ligatures.html View 1 chunk +0 lines, -53 lines 0 comments Download
D third_party/WebKit/LayoutTests/fast/css/font-face-unicode-range-ligatures-expected.txt View 1 chunk +0 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/core/css/CSSFontFace.h View 4 chunks +4 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/core/css/CSSFontFace.cpp View 2 chunks +4 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/core/css/CSSFontSelector.h View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/css/CSSFontSelector.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/css/CSSSegmentedFontFace.h View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/css/CSSSegmentedFontFace.cpp View 6 chunks +20 lines, -7 lines 0 comments Download
M third_party/WebKit/Source/platform/blink_platform.gypi View 1 chunk +1 line, -1 line 0 comments Download
D third_party/WebKit/Source/platform/fonts/FontDataForRangeSet.h View 1 chunk +0 lines, -81 lines 0 comments Download
A third_party/WebKit/Source/platform/fonts/FontDataRange.h View 1 chunk +76 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/fonts/FontFallbackIterator.h View 5 chunks +7 lines, -6 lines 0 comments Download
M third_party/WebKit/Source/platform/fonts/FontFallbackIterator.cpp View 7 chunks +23 lines, -23 lines 0 comments Download
M third_party/WebKit/Source/platform/fonts/FontFallbackList.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/platform/fonts/FontSelector.h View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/platform/fonts/GlyphPageTreeNode.cpp View 1 chunk +19 lines, -23 lines 0 comments Download
M third_party/WebKit/Source/platform/fonts/GlyphPageTreeNodeTest.cpp View 4 chunks +9 lines, -9 lines 0 comments Download
M third_party/WebKit/Source/platform/fonts/SegmentedFontData.h View 3 chunks +5 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/platform/fonts/SegmentedFontData.cpp View 4 chunks +13 lines, -13 lines 0 comments Download
M third_party/WebKit/Source/platform/fonts/UnicodeRangeSet.h View 3 chunks +9 lines, -11 lines 0 comments Download
M third_party/WebKit/Source/platform/fonts/UnicodeRangeSet.cpp View 2 chunks +9 lines, -13 lines 0 comments Download
M third_party/WebKit/Source/platform/fonts/UnicodeRangeSetTest.cpp View 3 chunks +43 lines, -43 lines 0 comments Download
M third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzFace.h View 2 chunks +1 line, -2 lines 0 comments Download
M third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzFace.cpp View 5 chunks +10 lines, -10 lines 0 comments Download
M third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaper.h View 2 chunks +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaper.cpp View 5 chunks +11 lines, -8 lines 0 comments Download
M third_party/WebKit/Source/platform/testing/FontTestHelpers.cpp View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 6 (2 generated)
fgorski
Created Revert of Shape unicode-range: font faces in only one iteration
4 years, 9 months ago (2016-03-17 17:16:48 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1806363002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1806363002/1
4 years, 9 months ago (2016-03-17 17:17:01 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 9 months ago (2016-03-17 17:17:48 UTC) #4
commit-bot: I haz the power
4 years, 9 months ago (2016-03-17 17:18:58 UTC) #6
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/60adcbb3ec944fa45c3b7cd0c45b38b1120e7b08
Cr-Commit-Position: refs/heads/master@{#381736}

Powered by Google App Engine
This is Rietveld 408576698