New issue
Advanced search Search tips

Issue 900955 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 29
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 3
Type: Bug



Sign in to add a comment

Do not synthesize small-caps for AAT fonts

Project Member Reported by drott@chromium.org, Nov 1

Issue description

When font-variant-caps CSS configures small-caps or petite-caps we execute font feature detection to determine whether we need to synthesize small caps or whether we can use the feature from the font.

Since AAT fonts handle feature activation differently from OpenType fonts with a GSUB feature, we need a different feature detection. And the current feature detection does not work for AAT fonts with a small-caps feature, leading to a always synthesizing small-caps for AAT fonts even though they may have the feature.

Examples are the ".SFNSText" system-ui font on Mac, as well as Apple Chancery, which is used as the font for the "cursive" generic font name.

We should have feature detection that correctly identifies for an AAT font whether it has small-caps or not.

This affects mostly Mac, but may affect other platforms if they have AAT-based small-caps.

 
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 29

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

commit b9385fec68a76e4f0d3a519b6aa6795c3a5fa695
Author: Dominik Röttsches <drott@chromium.org>
Date: Thu Nov 29 13:19:46 2018

Enable small-caps font feature in AAT system fonts on Mac

Shaping code relies on feature detection to determine whether the font
has small-caps support or not. If not, small capitals are synthesized
from uppercased letters with scaled-down font size. Add feature
detection for AAT fonts on Mac. Now the shaping code can enable the
built-in small-caps glyphs for a set of system font that have them.

Tests: open_type_caps_support_test.cc unit test for feature
detection, fast/text/small-caps-aat.html layout test.

Bug:  900955 
Change-Id: I9bb31719b0a1b5e81a428030cfdeaf91b8e6d17e
Reviewed-on: https://chromium-review.googlesource.com/c/1353927
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Reviewed-by: Behdad Esfahbod <behdad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612171}
[modify] https://crrev.com/b9385fec68a76e4f0d3a519b6aa6795c3a5fa695/third_party/blink/renderer/platform/BUILD.gn
[modify] https://crrev.com/b9385fec68a76e4f0d3a519b6aa6795c3a5fa695/third_party/blink/renderer/platform/fonts/opentype/open_type_caps_support.cc
[modify] https://crrev.com/b9385fec68a76e4f0d3a519b6aa6795c3a5fa695/third_party/blink/renderer/platform/fonts/opentype/open_type_caps_support.h
[add] https://crrev.com/b9385fec68a76e4f0d3a519b6aa6795c3a5fa695/third_party/blink/renderer/platform/fonts/opentype/open_type_caps_support_test.cc
[modify] https://crrev.com/b9385fec68a76e4f0d3a519b6aa6795c3a5fa695/third_party/blink/web_tests/NeverFixTests
[add] https://crrev.com/b9385fec68a76e4f0d3a519b6aa6795c3a5fa695/third_party/blink/web_tests/fast/text/small-caps-aat.html
[add] https://crrev.com/b9385fec68a76e4f0d3a519b6aa6795c3a5fa695/third_party/blink/web_tests/platform/mac/fast/text/small-caps-aat-expected.png

Status: Fixed (was: Assigned)
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 29

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

commit 9565c97bf98ccc9ef512ba0ef1f590a7393dbcef
Author: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Date: Thu Nov 29 15:27:13 2018

Revert "Enable small-caps font feature in AAT system fonts on Mac"

This reverts commit b9385fec68a76e4f0d3a519b6aa6795c3a5fa695.

Reason for revert: OpenTypeCapsSupportTest.SmallCapsForSFNSText
has failed the 2 times it was run since that CL was landed:

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.10%20Tests/36813
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.10%20Tests/36814

Original change's description:
> Enable small-caps font feature in AAT system fonts on Mac
> 
> Shaping code relies on feature detection to determine whether the font
> has small-caps support or not. If not, small capitals are synthesized
> from uppercased letters with scaled-down font size. Add feature
> detection for AAT fonts on Mac. Now the shaping code can enable the
> built-in small-caps glyphs for a set of system font that have them.
> 
> Tests: open_type_caps_support_test.cc unit test for feature
> detection, fast/text/small-caps-aat.html layout test.
> 
> Bug:  900955 
> Change-Id: I9bb31719b0a1b5e81a428030cfdeaf91b8e6d17e
> Reviewed-on: https://chromium-review.googlesource.com/c/1353927
> Commit-Queue: Dominik Röttsches <drott@chromium.org>
> Reviewed-by: Emil A Eklund <eae@chromium.org>
> Reviewed-by: Behdad Esfahbod <behdad@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#612171}

TBR=eae@chromium.org,kojii@chromium.org,drott@chromium.org,behdad@google.com,behdad@chromium.org

Change-Id: Ie6931d9b4442fd61b8f347e4616e7bd571d44ce5
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  900955 
Reviewed-on: https://chromium-review.googlesource.com/c/1354411
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Commit-Queue: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612200}
[modify] https://crrev.com/9565c97bf98ccc9ef512ba0ef1f590a7393dbcef/third_party/blink/renderer/platform/BUILD.gn
[modify] https://crrev.com/9565c97bf98ccc9ef512ba0ef1f590a7393dbcef/third_party/blink/renderer/platform/fonts/opentype/open_type_caps_support.cc
[modify] https://crrev.com/9565c97bf98ccc9ef512ba0ef1f590a7393dbcef/third_party/blink/renderer/platform/fonts/opentype/open_type_caps_support.h
[delete] https://crrev.com/0186601471d3a5866736a3c8146b8f23113f379b/third_party/blink/renderer/platform/fonts/opentype/open_type_caps_support_test.cc
[modify] https://crrev.com/9565c97bf98ccc9ef512ba0ef1f590a7393dbcef/third_party/blink/web_tests/NeverFixTests
[delete] https://crrev.com/0186601471d3a5866736a3c8146b8f23113f379b/third_party/blink/web_tests/fast/text/small-caps-aat.html
[delete] https://crrev.com/0186601471d3a5866736a3c8146b8f23113f379b/third_party/blink/web_tests/platform/mac/fast/text/small-caps-aat-expected.png

Project Member

Comment 6 by bugdroid1@chromium.org, Dec 3

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

commit 74edc05589ddce8bb148d9fa0fc09b9c8d6d428b
Author: Dominik Röttsches <drott@chromium.org>
Date: Mon Dec 03 17:02:46 2018

Reland: Enable small-caps font feature in AAT system fonts on Mac

Previously reviewed in:
https://chromium-review.googlesource.com/c/chromium/src/+/1353927
Reverted in:
https://chromium-review.googlesource.com/c/chromium/src/+/1354411

Enable unit-test, but only for Mac OS 10.13 and above due to fonts
before this version not supporting small-caps.

Shaping code relies on feature detection to determine whether the font
has small-caps support or not. If not, small capitals are synthesized
from uppercased letters with scaled-down font size. Add feature
detection for AAT fonts on Mac. Now the shaping code can enable the
built-in small-caps glyphs for a set of system font that have them.

Tests: open_type_caps_support_test.cc unit test for feature
detection, fast/text/small-caps-aat.html layout test.

Bug:  900955 
Change-Id: I354eea0c0d7e5fec40b508df85708933d2c0e06a
Reviewed-on: https://chromium-review.googlesource.com/c/1355168
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613126}
[modify] https://crrev.com/74edc05589ddce8bb148d9fa0fc09b9c8d6d428b/third_party/blink/renderer/platform/BUILD.gn
[modify] https://crrev.com/74edc05589ddce8bb148d9fa0fc09b9c8d6d428b/third_party/blink/renderer/platform/DEPS
[modify] https://crrev.com/74edc05589ddce8bb148d9fa0fc09b9c8d6d428b/third_party/blink/renderer/platform/fonts/opentype/open_type_caps_support.cc
[modify] https://crrev.com/74edc05589ddce8bb148d9fa0fc09b9c8d6d428b/third_party/blink/renderer/platform/fonts/opentype/open_type_caps_support.h
[add] https://crrev.com/74edc05589ddce8bb148d9fa0fc09b9c8d6d428b/third_party/blink/renderer/platform/fonts/opentype/open_type_caps_support_test.cc
[modify] https://crrev.com/74edc05589ddce8bb148d9fa0fc09b9c8d6d428b/third_party/blink/web_tests/NeverFixTests
[add] https://crrev.com/74edc05589ddce8bb148d9fa0fc09b9c8d6d428b/third_party/blink/web_tests/fast/text/small-caps-aat.html
[add] https://crrev.com/74edc05589ddce8bb148d9fa0fc09b9c8d6d428b/third_party/blink/web_tests/platform/mac/fast/text/small-caps-aat-expected.png

Sign in to add a comment