New issue
Advanced search Search tips

Issue 674878 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 669453



Sign in to add a comment

Second duplicate font variation settings axis value should supersede the first

Project Member Reported by drott@chromium.org, Dec 16 2016

Issue description

Compare https://drafts.csswg.org/css-fonts-4/#low-level-font-variation-settings-control-the-font-variation-settings-property

while in the initial implementation we leave it to Skia, which prioritizes the first.
 

Comment 1 by drott@chromium.org, Feb 6 2017

Cc: drott@chromium.org
Owner: bunge...@chromium.org
Compare variation settings assignment on Blink side in: FontPlatformData FontCustomPlatformData::fontPlatformData

and
void SkTypeface_FreeType::Scanner::computeAxisValues
which iterates over font properties and picks the first one.

Spec ref:
https://drafts.csswg.org/css-fonts-4/#propdef-font-variation-settings
"If the same axis name appears twice, the second one supercedes the first."

Comment 2 by drott@chromium.org, Feb 6 2017

Components: Blink>Fonts

Comment 3 by drott@chromium.org, Feb 6 2017

Also, ~line 2265 in SkFontHost_Mac.cpp 
            double value = defDouble;
            for (int j = 0; j < paramAxisCount; ++j) {
                if (paramAxes[j].fTag == tagLong) {
                    value = SkTPin(SkScalarToDouble(paramAxes[j].fStyleValue),minDouble,maxDouble);
                    break;
                }
            }


Project Member

Comment 4 by bugdroid1@chromium.org, Mar 30 2017

The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/9aec8945f0e0cc88c33c2440f2163d36c7198bf6

commit 9aec8945f0e0cc88c33c2440f2163d36c7198bf6
Author: bungeman <bungeman@google.com>
Date: Thu Mar 30 15:23:53 2017

Use last value for axis for variation position.

SkFontArguments::VariationPosition may be over specified. If there are
multiple values for a given axis, ensure the last value specified is
used, since that's what css-fonts-4 requires.

BUG= chromium:674878 

Change-Id: I6704c15c520c89efb9ee84659a3e16e0d07691c9
Reviewed-on: https://skia-review.googlesource.com/10513
Commit-Queue: Mike Klein <mtklein@chromium.org>
Reviewed-by: Mike Klein <mtklein@chromium.org>

[modify] https://crrev.com/9aec8945f0e0cc88c33c2440f2163d36c7198bf6/tests/TypefaceTest.cpp
[modify] https://crrev.com/9aec8945f0e0cc88c33c2440f2163d36c7198bf6/src/ports/SkFontHost_mac.cpp
[modify] https://crrev.com/9aec8945f0e0cc88c33c2440f2163d36c7198bf6/src/ports/SkFontHost_FreeType.cpp

Project Member

Comment 5 by bugdroid1@chromium.org, Mar 30 2017

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

commit e5a16b7e69683d64574421ec0287997639bca765
Author: skia-deps-roller@chromium.org <skia-deps-roller@chromium.org>
Date: Thu Mar 30 21:19:48 2017

Roll src/third_party/skia/ 13071c5c7..609e7ccc6 (11 commits)

https://skia.googlesource.com/skia.git/+log/13071c5c7aa5..609e7ccc664f

$ git log 13071c5c7..609e7ccc6 --date=short --no-merges --format='%ad %ae %s'
2017-03-30 jvanverth Add glPolygonMode support.
2017-03-29 mtklein make _win.S know if it's 64-bit
2017-03-30 ethannicholas Revert "skslc can now be compiled with no Skia dependencies, in preparation for"
2017-03-30 ethannicholas skslc can now be compiled with no Skia dependencies, in preparation for its eventual role in Skia's build process.
2017-03-30 msarett Make xformer class for SkCSXCanvas, use for draw loopers
2017-03-30 bungeman Use the rounded text size for gasp.
2017-03-30 robertphillips Minor cleanup (remove unused GrRenderTargetContext::asTexture method)
2017-03-30 bsalomon Use correct tolerance for conic chopping in MSAA and default path renderers
2017-03-29 bungeman Update instructions on Skia/Chromium multi try.
2017-03-29 bungeman Use last value for axis for variation position.
2017-03-30 bsalomon Renames of processor analysis-related classes and method.

Created with:
  roll-dep src/third_party/skia
BUG= 706693 , 674878 


Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, see:
http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls


CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
TBR=stephana@chromium.org

Change-Id: Idf9ea1e89298b6feb93f7666bd555ea222ab80a1
Reviewed-on: https://chromium-review.googlesource.com/463806
Reviewed-by: Skia Deps Roller <skia-deps-roller@chromium.org>
Commit-Queue: Skia Deps Roller <skia-deps-roller@chromium.org>
Cr-Commit-Position: refs/heads/master@{#460881}
[modify] https://crrev.com/e5a16b7e69683d64574421ec0287997639bca765/DEPS

Project Member

Comment 6 by bugdroid1@chromium.org, Mar 31 2017

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

commit 757a311011d6d1393ecde5e586bcc99ed58481c8
Author: drott <drott@chromium.org>
Date: Fri Mar 31 08:06:36 2017

Update comment on overspecified font-variation-settings

After https://crrev.com/9aec8945f0e0cc88c33c2440f2163d36c7198bf6

TBR=bungeman, eae
BUG= 674878 

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

[modify] https://crrev.com/757a311011d6d1393ecde5e586bcc99ed58481c8/third_party/WebKit/Source/platform/fonts/FontCustomPlatformData.cpp

Comment 7 by drott@chromium.org, Mar 31 2017

Status: Fixed (was: Assigned)
Thank you!

Sign in to add a comment