Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2517193002:
Reland of Minor style changes to HarfBuzzShaper to improve readability (Closed)
|
Created:
4 years, 1 month ago by eae Modified:
4 years, 1 month ago CC:
chromium-reviews, krit, drott+blinkwatch_chromium.org, blink-reviews-platform-graphics_chromium.org, dshwang, pdr+graphicswatchlist_chromium.org, jbroman, Justin Novosad, Rik, f(malita), blink-reviews, danakj+watch_chromium.org, ajuma+watch_chromium.org, Stephen Chennney, rwlbuis Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionReland of Minor style changes to HarfBuzzShaper to improve readability
Landed in r433265, reverted in r433469 due to an unrelated asan failure.
R=szager@chromium.org,mgiuca@chromium.org
BUG=667147
Committed: https://crrev.com/4a4861960ae3a020a3fcdaa417216e7f7411792c
Cr-Commit-Position: refs/heads/master@{#433768}
Patch Set 1 #Patch Set 2 : Rebase w/HEAD #Patch Set 3 : Rebase again #Messages
Total messages: 22 (13 generated)
Created Reland of Minor style changes to HarfBuzzShaper to improve readability
Description was changed from ========== Reland of Minor style changes to HarfBuzzShaper to improve readability (patchset #1 id:1 of https://codereview.chromium.org/2517023002/ ) Reason for revert: Was't the cause of the regression, see bug 667147. Original issue's description: > Revert of Minor style changes to HarfBuzzShaper to improve readability (patchset #1 id:1 of https://codereview.chromium.org/2513473007/ ) > > Reason for revert: > Appears to have broken HarfBuzzShaperTest.ResolveCandidateRunsUnicodeVariants on Linux ASan. > > BUG=667147 > > Original issue's description: > > Minor style changes to HarfBuzzShaper to improve readability > > > > TBR=szager@chromium.org > > > > Committed: https://crrev.com/95b18a0f83f57c597c435e09b4081d995e0defe0 > > Cr-Commit-Position: refs/heads/master@{#433265} > > TBR=szager@chromium.org,eae@chromium.org > # Not skipping CQ checks because original CL landed more than 1 days ago. > > Committed: https://crrev.com/c4c4fd40e57affab6a53322119636a09aed77c55 > Cr-Commit-Position: refs/heads/master@{#433469} TBR=szager@chromium.org,mgiuca@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=667147 ========== to ========== Reland of Minor style changes to HarfBuzzShaper to improve readability First landed in 433265, reverted in 433469 due an separate asan failure. TBR=szager@chromium.org,mgiuca@chromium.org BUG=667147 ==========
The CQ bit was checked by eae@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Failed to apply patch for
third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaper.cpp:
While running git apply --index -p1;
error: patch failed:
third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaper.cpp:379
error: third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaper.cpp:
patch does not apply
Patch: third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaper.cpp
Index: third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaper.cpp
diff --git a/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaper.cpp
b/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaper.cpp
index
a2c00106b81160939e748cfc0ac1d089051bfdf9..667a6ca144342fca323a1dd08782bd4f72ce47c8
100644
--- a/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaper.cpp
+++ b/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaper.cpp
@@ -379,31 +379,24 @@
smallCapsIterator.consume(&numCharactersUntilCaseChange, &smallCapsBehavior);
if (numCharactersUntilCaseChange > 0 &&
numCharactersUntilCaseChange < currentQueueItem.m_numCharacters) {
- unsigned startIndex =
- currentQueueItem.m_startIndex + numCharactersUntilCaseChange;
- unsigned numCharacters =
- currentQueueItem.m_numCharacters - numCharactersUntilCaseChange;
- queue->prepend(HolesQueueItem(HolesQueueItemAction::HolesQueueRange,
- startIndex, numCharacters));
+ queue->prepend(HolesQueueItem(
+ HolesQueueItemAction::HolesQueueRange,
+ currentQueueItem.m_startIndex + numCharactersUntilCaseChange,
+ currentQueueItem.m_numCharacters - numCharactersUntilCaseChange));
currentQueueItem.m_numCharacters = numCharactersUntilCaseChange;
}
}
-hb_feature_t createFeature(uint8_t c1,
- uint8_t c2,
- uint8_t c3,
- uint8_t c4,
- uint32_t value = 0) {
- return {HB_TAG(c1, c2, c3, c4), value, 0 /* start */,
- static_cast<unsigned>(-1) /* end */};
+hb_feature_t createFeature(hb_tag_t tag, uint32_t value = 0) {
+ return {tag, value, 0 /* start */, static_cast<unsigned>(-1) /* end */};
}
void setFontFeatures(const Font* font,
HarfBuzzShaper::FeaturesVector* features) {
const FontDescription& description = font->getFontDescription();
- static hb_feature_t noKern = createFeature('k', 'e', 'r', 'n');
- static hb_feature_t noVkrn = createFeature('v', 'k', 'r', 'n');
+ static hb_feature_t noKern = createFeature(HB_TAG('k', 'e', 'r', 'n'));
+ static hb_feature_t noVkrn = createFeature(HB_TAG('v', 'k', 'r', 'n'));
switch (description.getKerning()) {
case FontDescription::NormalKerning:
// kern/vkrn are enabled by default
@@ -415,8 +408,8 @@
break;
}
- static hb_feature_t noClig = createFeature('c', 'l', 'i', 'g');
- static hb_feature_t noLiga = createFeature('l', 'i', 'g', 'a');
+ static hb_feature_t noClig = createFeature(HB_TAG('c', 'l', 'i', 'g'));
+ static hb_feature_t noLiga = createFeature(HB_TAG('l', 'i', 'g', 'a'));
switch (description.commonLigaturesState()) {
case FontDescription::DisabledLigaturesState:
features->append(noLiga);
@@ -428,7 +421,7 @@
case FontDescription::NormalLigaturesState:
break;
}
- static hb_feature_t dlig = createFeature('d', 'l', 'i', 'g', 1);
+ static hb_feature_t dlig = createFeature(HB_TAG('d', 'l', 'i', 'g'), 1);
switch (description.discretionaryLigaturesState()) {
case FontDescription::DisabledLigaturesState:
// dlig is off by default
@@ -439,7 +432,7 @@
case FontDescription::NormalLigaturesState:
break;
}
- static hb_feature_t hlig = createFeature('h', 'l', 'i', 'g', 1);
+ static hb_feature_t hlig = createFeature(HB_TAG('h', 'l', 'i', 'g'), 1);
switch (description.historicalLigaturesState()) {
case FontDescription::DisabledLigaturesState:
// hlig is off by default
@@ -450,7 +443,7 @@
case FontDescription::NormalLigaturesState:
break;
}
- static hb_feature_t noCalt = createFeature('c', 'a', 'l', 't');
+ static hb_feature_t noCalt = createFeature(HB_TAG('c', 'a', 'l', 't'));
switch (description.contextualLigaturesState()) {
case FontDescription::DisabledLigaturesState:
features->append(noCalt);
@@ -462,9 +455,9 @@
break;
}
- static hb_feature_t hwid = createFeature('h', 'w', 'i', 'd', 1);
- static hb_feature_t twid = createFeature('t', 'w', 'i', 'd', 1);
- static hb_feature_t qwid = createFeature('q', 'w', 'i', 'd', 1);
+ static hb_feature_t hwid = createFeature(HB_TAG('h', 'w', 'i', 'd'), 1);
+ static hb_feature_t twid = createFeature(HB_TAG('t', 'w', 'i', 'd'), 1);
+ static hb_feature_t qwid = createFeature(HB_TAG('q', 'w', 'i', 'd'), 1);
switch (description.widthVariant()) {
case HalfWidth:
features->append(hwid);
@@ -480,40 +473,40 @@
}
// font-variant-numeric:
- static hb_feature_t lnum = createFeature('l', 'n', 'u', 'm', 1);
+ static hb_feature_t lnum = createFeature(HB_TAG('l', 'n', 'u', 'm'), 1);
if (description.variantNumeric().numericFigureValue() ==
FontVariantNumeric::LiningNums)
features->append(lnum);
- static hb_feature_t onum = createFeature('o', 'n', 'u', 'm', 1);
+ static hb_feature_t onum = createFeature(HB_TAG('o', 'n', 'u', 'm'), 1);
if (description.variantNumeric().numericFigureValue() ==
FontVariantNumeric::OldstyleNums)
features->append(onum);
- static hb_feature_t pnum = createFeature('p', 'n', 'u', 'm', 1);
+ static hb_feature_t pnum = createFeature(HB_TAG('p', 'n', 'u', 'm'), 1);
if (description.variantNumeric().numericSpacingValue() ==
FontVariantNumeric::ProportionalNums)
features->append(pnum);
- static hb_feature_t tnum = createFeature('t', 'n', 'u', 'm', 1);
+ static hb_feature_t tnum = createFeature(HB_TAG('t', 'n', 'u', 'm'), 1);
if (description.variantNumeric().numericSpacingValue() ==
FontVariantNumeric::TabularNums)
features->append(tnum);
- static hb_feature_t afrc = createFeature('a', 'f', 'r', 'c', 1);
+ static hb_feature_t afrc = createFeature(HB_TAG('a', 'f', 'r', 'c'), 1);
if (description.variantNumeric().numericFractionValue() ==
FontVariantNumeric::StackedFractions)
features->append(afrc);
- static hb_feature_t frac = createFeature('f', 'r', 'a', 'c', 1);
+ static hb_feature_t frac = createFeature(HB_TAG('f', 'r', 'a', 'c'), 1);
if (description.variantNumeric().numericFractionValue() ==
FontVariantNumeric::DiagonalFractions)
features->append(frac);
- static hb_feature_t ordn = createFeature('o', 'r', 'd', 'n', 1);
+ static hb_feature_t ordn = createFeature(HB_TAG('o', 'r', 'd', 'n'), 1);
if (description.variantNumeric().ordinalValue() ==
FontVariantNumeric::OrdinalOn)
features->append(ordn);
- static hb_feature_t zero = createFeature('z', 'e', 'r', 'o', 1);
+ static hb_feature_t zero = createFeature(HB_TAG('z', 'e', 'r', 'o'), 1);
if (description.variantNumeric().slashedZeroValue() ==
FontVariantNumeric::SlashedZeroOn)
features->append(zero);
@@ -561,12 +554,12 @@
void CapsFeatureSettingsScopedOverlay::overlayCapsFeatures(
FontDescription::FontVariantCaps variantCaps) {
- static hb_feature_t smcp = createFeature('s', 'm', 'c', 'p', 1);
- static hb_feature_t pcap = createFeature('p', 'c', 'a', 'p', 1);
- static hb_feature_t c2sc = createFeature('c', '2', 's', 'c', 1);
- static hb_feature_t c2pc = createFeature('c', '2', 'p', 'c', 1);
- static hb_feature_t unic = createFeature('u', 'n', 'i', 'c', 1);
- static hb_feature_t titl = createFeature('t', 'i', 't', 'l', 1);
+ static hb_feature_t smcp = createFeature(HB_TAG('s', 'm', 'c', 'p'), 1);
+ static hb_feature_t pcap = createFeature(HB_TAG('p', 'c', 'a', 'p'), 1);
+ static hb_feature_t c2sc = createFeature(HB_TAG('c', '2', 's', 'c'), 1);
+ static hb_feature_t c2pc = createFeature(HB_TAG('c', '2', 'p', 'c'), 1);
+ static hb_feature_t unic = createFeature(HB_TAG('u', 'n', 'i', 'c'), 1);
+ static hb_feature_t titl = createFeature(HB_TAG('t', 'i', 't', 'l'), 1);
if (variantCaps == FontDescription::SmallCaps ||
variantCaps == FontDescription::AllSmallCaps) {
prependCounting(smcp);
lgtm
Description was changed from ========== Reland of Minor style changes to HarfBuzzShaper to improve readability First landed in 433265, reverted in 433469 due an separate asan failure. TBR=szager@chromium.org,mgiuca@chromium.org BUG=667147 ========== to ========== Reland of Minor style changes to HarfBuzzShaper to improve readability First landed in 433265, reverted in 433469 due an separate asan failure. R=szager@chromium.org,mgiuca@chromium.org BUG=667147 ==========
The CQ bit was checked by eae@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from szager@chromium.org Link to the patchset: https://codereview.chromium.org/2517193002/#ps40001 (title: "Rebase w/HEAD")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by eae@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from szager@chromium.org Link to the patchset: https://codereview.chromium.org/2517193002/#ps60001 (title: "Rebase again")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Reland of Minor style changes to HarfBuzzShaper to improve readability First landed in 433265, reverted in 433469 due an separate asan failure. R=szager@chromium.org,mgiuca@chromium.org BUG=667147 ========== to ========== Reland of Minor style changes to HarfBuzzShaper to improve readability Landed in r433265, reverted in r433469 due to an unrelated asan failure. R=szager@chromium.org,mgiuca@chromium.org BUG=667147 ==========
CQ is committing da patch.
Bot data: {"patchset_id": 60001, "attempt_start_ts": 1479777795416650,
"parent_rev": "b085a44046ffc2424840bd8eb0c7ad2139e26612", "commit_rev":
"2ce683b9b63f84da6af6b2daec55044a52979bd0"}
Message was sent while issue was closed.
Description was changed from ========== Reland of Minor style changes to HarfBuzzShaper to improve readability Landed in r433265, reverted in r433469 due to an unrelated asan failure. R=szager@chromium.org,mgiuca@chromium.org BUG=667147 ========== to ========== Reland of Minor style changes to HarfBuzzShaper to improve readability Landed in r433265, reverted in r433469 due to an unrelated asan failure. R=szager@chromium.org,mgiuca@chromium.org BUG=667147 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Reland of Minor style changes to HarfBuzzShaper to improve readability Landed in r433265, reverted in r433469 due to an unrelated asan failure. R=szager@chromium.org,mgiuca@chromium.org BUG=667147 ========== to ========== Reland of Minor style changes to HarfBuzzShaper to improve readability Landed in r433265, reverted in r433469 due to an unrelated asan failure. R=szager@chromium.org,mgiuca@chromium.org BUG=667147 Committed: https://crrev.com/4a4861960ae3a020a3fcdaa417216e7f7411792c Cr-Commit-Position: refs/heads/master@{#433768} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/4a4861960ae3a020a3fcdaa417216e7f7411792c Cr-Commit-Position: refs/heads/master@{#433768} |
