Commit 9565c97b authored by Sebastien Seguin-Gagnon's avatar Sebastien Seguin-Gagnon Committed by Commit Bot

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

This reverts commit b9385fec.

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/1354411Reviewed-by: default avatarSebastien Seguin-Gagnon <sebsg@chromium.org>
Commit-Queue: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612200}
parent 01866014
......@@ -1682,7 +1682,6 @@ jumbo_source_set("blink_platform_unittests_sources") {
"fonts/generic_font_family_settings_test.cc",
"fonts/mac/font_matcher_mac_test.mm",
"fonts/opentype/font_settings_test.cc",
"fonts/opentype/open_type_caps_support_test.cc",
"fonts/opentype/open_type_vertical_data_test.cc",
"fonts/orientation_iterator_test.cc",
"fonts/script_run_iterator_test.cc",
......
......@@ -4,43 +4,13 @@
#include "third_party/blink/renderer/platform/fonts/opentype/open_type_caps_support.h"
#include <hb-aat.h>
#include <hb.h>
namespace blink {
namespace {
bool activationSelectorPresent(
hb_face_t* hb_face,
const hb_aat_layout_feature_type_t feature_type,
const hb_aat_layout_feature_selector_t enabled_selector_expectation) {
Vector<hb_aat_layout_feature_selector_info_t> feature_selectors;
unsigned num_feature_selectors = 0;
unsigned default_index = 0;
num_feature_selectors = hb_aat_layout_feature_type_get_selector_infos(
hb_face, feature_type, 0, nullptr, nullptr, nullptr);
feature_selectors.resize(num_feature_selectors);
if (!hb_aat_layout_feature_type_get_selector_infos(
hb_face, feature_type, 0, &num_feature_selectors,
feature_selectors.data(), &default_index)) {
return false;
}
for (hb_aat_layout_feature_selector_info_t selector_info :
feature_selectors) {
if (selector_info.enable == enabled_selector_expectation)
return true;
}
return false;
}
} // namespace
OpenTypeCapsSupport::OpenTypeCapsSupport()
: harfbuzz_face_(nullptr),
requested_caps_(FontDescription::kCapsNormal),
font_support_(FontSupport::kFull),
caps_synthesis_(CapsSynthesis::kNone),
font_format_(FontFormat::kUndetermined) {}
caps_synthesis_(CapsSynthesis::kNone) {}
OpenTypeCapsSupport::OpenTypeCapsSupport(
const HarfBuzzFace* harfbuzz_face,
......@@ -49,8 +19,7 @@ OpenTypeCapsSupport::OpenTypeCapsSupport(
: harfbuzz_face_(harfbuzz_face),
requested_caps_(requested_caps),
font_support_(FontSupport::kFull),
caps_synthesis_(CapsSynthesis::kNone),
font_format_(FontFormat::kUndetermined) {
caps_synthesis_(CapsSynthesis::kNone) {
if (requested_caps != FontDescription::kCapsNormal)
DetermineFontSupport(script);
}
......@@ -133,107 +102,24 @@ CaseMapIntend OpenTypeCapsSupport::NeedsCaseChange(
return case_map_intend;
}
OpenTypeCapsSupport::FontFormat OpenTypeCapsSupport::GetFontFormat() const {
if (font_format_ == FontFormat::kUndetermined) {
hb_face_t* hb_face = hb_font_get_face(
harfbuzz_face_->GetScaledFont(nullptr, HarfBuzzFace::NoVerticalLayout));
unsigned table_count = 0;
Vector<uint32_t> table_tags;
table_count = hb_face_get_table_tags(hb_face, 0, nullptr, nullptr);
table_tags.resize(table_count);
if (!hb_face_get_table_tags(hb_face, 0, &table_count, table_tags.data()))
table_tags.resize(0);
font_format_ = table_tags.Contains(HB_TAG('m', 'o', 'r', 'x')) &&
!table_tags.Contains(HB_TAG('G', 'S', 'U', 'B'))
? font_format_ = FontFormat::kAat
: font_format_ = FontFormat::kOpenType;
}
return font_format_;
}
bool OpenTypeCapsSupport::SupportsFeature(hb_script_t script,
uint32_t tag) const {
if (GetFontFormat() == FontFormat::kAat)
return SupportsAatFeature(tag);
return SupportsOpenTypeFeature(script, tag);
}
bool OpenTypeCapsSupport::SupportsAatFeature(uint32_t tag) const {
// We only want to detect small-caps and capitals-to-small-capitals features
// for aat-fonts, any other requests are returned as not supported.
if (tag != HB_TAG('s', 'm', 'c', 'p') && tag != HB_TAG('c', '2', 's', 'c')) {
return false;
}
hb_face_t* hb_face = hb_font_get_face(
harfbuzz_face_->GetScaledFont(nullptr, HarfBuzzFace::NoVerticalLayout));
Vector<hb_aat_layout_feature_type_t> aat_features;
unsigned feature_count =
hb_aat_layout_get_feature_types(hb_face, 0, nullptr, nullptr);
aat_features.resize(feature_count);
if (!hb_aat_layout_get_feature_types(hb_face, 0, &feature_count,
aat_features.data()))
return false;
if (tag == HB_TAG('s', 'm', 'c', 'p')) {
// Check for presence of new style (feature id 38) or old style (letter
// case, feature id 3) small caps feature presence, then check for the
// specific required activation selectors.
if (!aat_features.Contains(HB_AAT_LAYOUT_FEATURE_TYPE_LETTER_CASE) &&
!aat_features.Contains(HB_AAT_LAYOUT_FEATURE_TYPE_LOWER_CASE))
return false;
// Check for new style small caps, feature id 38.
if (aat_features.Contains(HB_AAT_LAYOUT_FEATURE_TYPE_LOWER_CASE)) {
if (activationSelectorPresent(
hb_face, HB_AAT_LAYOUT_FEATURE_TYPE_LOWER_CASE,
HB_AAT_LAYOUT_FEATURE_SELECTOR_LOWER_CASE_SMALL_CAPS))
return true;
}
// Check for old style small caps enabling selector, feature id 3.
if (aat_features.Contains(HB_AAT_LAYOUT_FEATURE_TYPE_LETTER_CASE)) {
if (activationSelectorPresent(hb_face,
HB_AAT_LAYOUT_FEATURE_TYPE_LETTER_CASE,
HB_AAT_LAYOUT_FEATURE_SELECTOR_SMALL_CAPS))
return true;
}
// Neither old or new style small caps present.
return false;
}
if (tag == HB_TAG('c', '2', 's', 'c')) {
if (!aat_features.Contains(HB_AAT_LAYOUT_FEATURE_TYPE_UPPER_CASE))
return false;
return activationSelectorPresent(
hb_face, HB_AAT_LAYOUT_FEATURE_TYPE_UPPER_CASE,
HB_AAT_LAYOUT_FEATURE_SELECTOR_UPPER_CASE_SMALL_CAPS);
}
return false;
}
void OpenTypeCapsSupport::DetermineFontSupport(hb_script_t script) {
switch (requested_caps_) {
case FontDescription::kSmallCaps:
if (!SupportsFeature(script, HB_TAG('s', 'm', 'c', 'p'))) {
if (!SupportsOpenTypeFeature(script, HB_TAG('s', 'm', 'c', 'p'))) {
font_support_ = FontSupport::kNone;
caps_synthesis_ = CapsSynthesis::kLowerToSmallCaps;
}
break;
case FontDescription::kAllSmallCaps:
if (!(SupportsFeature(script, HB_TAG('s', 'm', 'c', 'p')) &&
SupportsFeature(script, HB_TAG('c', '2', 's', 'c')))) {
if (!(SupportsOpenTypeFeature(script, HB_TAG('s', 'm', 'c', 'p')) &&
SupportsOpenTypeFeature(script, HB_TAG('c', '2', 's', 'c')))) {
font_support_ = FontSupport::kNone;
caps_synthesis_ = CapsSynthesis::kBothToSmallCaps;
}
break;
case FontDescription::kPetiteCaps:
if (!SupportsFeature(script, HB_TAG('p', 'c', 'a', 'p'))) {
if (SupportsFeature(script, HB_TAG('s', 'm', 'c', 'p'))) {
if (!SupportsOpenTypeFeature(script, HB_TAG('p', 'c', 'a', 'p'))) {
if (SupportsOpenTypeFeature(script, HB_TAG('s', 'm', 'c', 'p'))) {
font_support_ = FontSupport::kFallback;
} else {
font_support_ = FontSupport::kNone;
......@@ -242,10 +128,10 @@ void OpenTypeCapsSupport::DetermineFontSupport(hb_script_t script) {
}
break;
case FontDescription::kAllPetiteCaps:
if (!(SupportsFeature(script, HB_TAG('p', 'c', 'a', 'p')) &&
SupportsFeature(script, HB_TAG('c', '2', 'p', 'c')))) {
if (SupportsFeature(script, HB_TAG('s', 'm', 'c', 'p')) &&
SupportsFeature(script, HB_TAG('c', '2', 's', 'c'))) {
if (!(SupportsOpenTypeFeature(script, HB_TAG('p', 'c', 'a', 'p')) &&
SupportsOpenTypeFeature(script, HB_TAG('c', '2', 'p', 'c')))) {
if (SupportsOpenTypeFeature(script, HB_TAG('s', 'm', 'c', 'p')) &&
SupportsOpenTypeFeature(script, HB_TAG('c', '2', 's', 'c'))) {
font_support_ = FontSupport::kFallback;
} else {
font_support_ = FontSupport::kNone;
......@@ -254,9 +140,9 @@ void OpenTypeCapsSupport::DetermineFontSupport(hb_script_t script) {
}
break;
case FontDescription::kUnicase:
if (!SupportsFeature(script, HB_TAG('u', 'n', 'i', 'c'))) {
if (!SupportsOpenTypeFeature(script, HB_TAG('u', 'n', 'i', 'c'))) {
caps_synthesis_ = CapsSynthesis::kUpperToSmallCaps;
if (SupportsFeature(script, HB_TAG('s', 'm', 'c', 'p'))) {
if (SupportsOpenTypeFeature(script, HB_TAG('s', 'm', 'c', 'p'))) {
font_support_ = FontSupport::kFallback;
} else {
font_support_ = FontSupport::kNone;
......@@ -264,7 +150,7 @@ void OpenTypeCapsSupport::DetermineFontSupport(hb_script_t script) {
}
break;
case FontDescription::kTitlingCaps:
if (!SupportsFeature(script, HB_TAG('t', 'i', 't', 'l'))) {
if (!SupportsOpenTypeFeature(script, HB_TAG('t', 'i', 't', 'l'))) {
font_support_ = FontSupport::kNone;
}
break;
......
......@@ -29,13 +29,7 @@ class PLATFORM_EXPORT OpenTypeCapsSupport {
CaseMapIntend NeedsCaseChange(SmallCapsIterator::SmallCapsBehavior run_case);
private:
enum class FontFormat { kUndetermined, kOpenType, kAat };
// Lazily intializes font_format_ when needed and returns the format of the
// underlying HarfBuzzFace/Font.
FontFormat GetFontFormat() const;
void DetermineFontSupport(hb_script_t);
bool SupportsFeature(hb_script_t, uint32_t tag) const;
bool SupportsAatFeature(uint32_t tag) const;
bool SupportsOpenTypeFeature(hb_script_t, uint32_t tag) const;
const HarfBuzzFace* harfbuzz_face_;
......@@ -56,7 +50,6 @@ class PLATFORM_EXPORT OpenTypeCapsSupport {
FontSupport font_support_;
CapsSynthesis caps_synthesis_;
mutable FontFormat font_format_;
};
}; // namespace blink
......
// Copyright (c) 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "third_party/blink/renderer/platform/fonts/opentype/open_type_caps_support.h"
#include "build/build_config.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/renderer/platform/fonts/font_description.h"
#include "third_party/blink/renderer/platform/fonts/font_platform_data.h"
#include "third_party/skia/include/core/SkRefCnt.h"
#include "third_party/skia/include/core/SkTypeface.h"
namespace blink {
void ensureHasNativeSmallCaps(const std::string& font_family_name) {
sk_sp<SkTypeface> san_francisco_typeface =
SkTypeface::MakeFromName(font_family_name.c_str(), SkFontStyle());
FontPlatformData font_platform_data(
san_francisco_typeface, font_family_name.c_str(), 16, false, false);
ASSERT_EQ(font_platform_data.FontFamilyName(), font_family_name.c_str());
OpenTypeCapsSupport caps_support(font_platform_data.GetHarfBuzzFace(),
FontDescription::FontVariantCaps::kSmallCaps,
HB_SCRIPT_LATIN);
// If caps_support.NeedsRunCaseSplitting() is true, this means that synthetic
// upper-casing / lower-casing is required and the run needs to be segmented
// by upper-case, lower-case properties. If it is false, it means that the
// font feature can be used and no synthetic case-changing is needed.
ASSERT_FALSE(caps_support.NeedsRunCaseSplitting());
}
// The Fonts AAT fonts for testing are only available on Mac OS X.
#if defined(OS_MACOSX)
#define MAYBE_SmallCapsForSFNSText SmallCapsForSFNSText
#else
#define MAYBE_SmallCapsForSFNSText DISABLED_SmallCapsForSFNSText
#endif
TEST(OpenTypeCapsSupportTest, MAYBE_SmallCapsForSFNSText) {
std::vector<std::string> test_fonts = {
".SF NS Text", // has OpenType small-caps
"Apple Chancery", // has old-style (feature id 3,"Letter Case")
// small-caps
"Baskerville"}; // has new-style (feature id 38, "Upper Case")
// small-case.
for (auto& test_font : test_fonts)
ensureHasNativeSmallCaps(test_font);
}
} // namespace blink
......@@ -134,14 +134,6 @@ fast/harness/results.html [ WontFix ]
[ Win ] fast/text/aat-morx.html [ WontFix ]
[ Android ] fast/text/aat-morx.html [ WontFix ]
# AAT Small Caps test not supported on platforms other than Mac 10.13.
[ Linux ] fast/text/small-caps-aat.html [ WontFix ]
[ Win ] fast/text/small-caps-aat.html [ WontFix ]
[ Android ] fast/text/small-caps-aat.html [ WontFix ]
[ Mac10.10 ] fast/text/small-caps-aat.html [ WontFix ]
[ Mac10.11 ] fast/text/small-caps-aat.html [ WontFix ]
[ Mac10.12 ] fast/text/small-caps-aat.html [ WontFix ]
# Linux layout tests do not have a Myanmar fallback font.
[ Linux ] inspector-protocol/layout-fonts/fallback-myanmar.js [ WontFix ]
......
<DOCTYPE html>
<style>
.chancery {
font-family: "Apple Chancery", fantasy;
}
.system {
font-family: system-ui, fantasy;;
}
.baskerville {
font-family: "Baskerville", fantasy;
}
.fake_small_caps {
text-transform: uppercase;
font-size: 70%;
margin-bottom: 10px;
}
.small_caps {
font-variant-caps: small-caps;
}
</style>
<div>For each pair of lines, the first line should have different glyph shapes to the second, i.e. real small-caps
should be used in each of the pairs' first line.</div>
<div class="chancery small_caps">Apple Chancery Real Small Caps wwaammii</div>
<div class="chancery fake_small_caps">Apple Chancery Fake Small Caps wwaammii</div>
<div class="system small_caps">San Francisco Real Small Caps wwaammii</div>
<div class="system fake_small_caps">San Francisco Fake Small Caps wwaammii</div>
<div class="baskerville small_caps">Baskerville Real Small Caps wwaammii</div>
<div class="baskerville fake_small_caps">Baskerville Fake Small Caps wwaammii</div>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment