Use current variation position in cloning typeface.
The FreeType and CoreText ports would use the default value for an axis
instead of the current value for any unspecified axes. Change this so
that when cloning a typeface any unspecified axes preferentially use the
current axis value if it is known.
Also adds a test that unspecified axes are not changed when cloning.
Change-Id: I751ee5517f1d6b827c6d4ab245e9d681c8df6b42
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/370456
Reviewed-by: Herb Derby <herb@google.com>
Reviewed-by: Dominik Röttsches <drott@chromium.org>
Commit-Queue: Ben Wagner <bungeman@google.com>
diff --git a/gn/gm.gni b/gn/gm.gni
index 8fda918..158682c 100644
--- a/gn/gm.gni
+++ b/gn/gm.gni
@@ -191,6 +191,7 @@
"$_gm/giantbitmap.cpp",
"$_gm/glyph_pos.cpp",
"$_gm/gm.cpp",
+ "$_gm/gm.h",
"$_gm/gpu_blur_utils.cpp",
"$_gm/gradient_dirty_laundry.cpp",
"$_gm/gradient_matrix.cpp",
diff --git a/src/ports/SkFontHost_FreeType.cpp b/src/ports/SkFontHost_FreeType.cpp
index eb801b6..43f7ebc 100644
--- a/src/ports/SkFontHost_FreeType.cpp
+++ b/src/ports/SkFontHost_FreeType.cpp
@@ -749,6 +749,64 @@
sk_ref_sp(const_cast<SkTypeface_FreeType*>(this)), effects, desc);
}
+/** Copy the design variation coordinates into 'coordinates'.
+ *
+ * @param coordinates the buffer into which to write the design variation coordinates.
+ * @param coordinateCount the number of entries available through 'coordinates'.
+ *
+ * @return The number of axes, or -1 if there is an error.
+ * If 'coordinates != nullptr' and 'coordinateCount >= numAxes' then 'coordinates' will be
+ * filled with the variation coordinates describing the position of this typeface in design
+ * variation space. It is possible the number of axes can be retrieved but actual position
+ * cannot.
+ */
+static int GetVariationDesignPosition(AutoFTAccess& fta,
+ SkFontArguments::VariationPosition::Coordinate coordinates[], int coordinateCount)
+{
+ FT_Face face = fta.face();
+ if (!face) {
+ return -1;
+ }
+
+ if (!(face->face_flags & FT_FACE_FLAG_MULTIPLE_MASTERS)) {
+ return 0;
+ }
+
+ FT_MM_Var* variations = nullptr;
+ if (FT_Get_MM_Var(face, &variations)) {
+ return -1;
+ }
+ SkAutoFree autoFreeVariations(variations);
+
+ if (!coordinates || coordinateCount < SkToInt(variations->num_axis)) {
+ return variations->num_axis;
+ }
+
+ SkAutoSTMalloc<4, FT_Fixed> coords(variations->num_axis);
+ // FT_Get_{MM,Var}_{Blend,Design}_Coordinates were added in FreeType 2.7.1.
+ if (gFTLibrary->fGetVarDesignCoordinates &&
+ !gFTLibrary->fGetVarDesignCoordinates(face, variations->num_axis, coords.get()))
+ {
+ for (FT_UInt i = 0; i < variations->num_axis; ++i) {
+ coordinates[i].axis = variations->axis[i].tag;
+ coordinates[i].value = SkFixedToScalar(coords[i]);
+ }
+ } else if (static_cast<FT_UInt>(fta.getAxesCount()) == variations->num_axis) {
+ for (FT_UInt i = 0; i < variations->num_axis; ++i) {
+ coordinates[i].axis = variations->axis[i].tag;
+ coordinates[i].value = SkFixedToScalar(fta.getAxes()[i]);
+ }
+ } else if (fta.isNamedVariationSpecified()) {
+ // The font has axes, they cannot be retrieved, and some named axis was specified.
+ return -1;
+ } else {
+ // The font has axes, they cannot be retrieved, but no named instance was specified.
+ return 0;
+ }
+
+ return variations->num_axis;
+}
+
std::unique_ptr<SkFontData> SkTypeface_FreeType::cloneFontData(const SkFontArguments& args) const {
AutoFTAccess fta(this);
FT_Face face = fta.face();
@@ -760,15 +818,18 @@
if (!Scanner::GetAxes(face, &axisDefinitions)) {
return nullptr;
}
+ int axisCount = axisDefinitions.count();
+
+ SkAutoSTMalloc<4, SkFontArguments::VariationPosition::Coordinate> currentPosition(axisCount);
+ int currentAxisCount = GetVariationDesignPosition(fta, currentPosition, axisCount);
SkString name;
- SkAutoSTMalloc<4, SkFixed> axisValues(axisDefinitions.count());
- Scanner::computeAxisValues(axisDefinitions, args.getVariationDesignPosition(),
- axisValues, name);
+ SkAutoSTMalloc<4, SkFixed> axisValues(axisCount);
+ Scanner::computeAxisValues(axisDefinitions, args.getVariationDesignPosition(), axisValues, name,
+ currentAxisCount == axisCount ? currentPosition.get() : nullptr);
int ttcIndex;
std::unique_ptr<SkStreamAsset> stream = this->openStream(&ttcIndex);
- return std::make_unique<SkFontData>(std::move(stream), ttcIndex, axisValues.get(),
- axisDefinitions.count());
+ return std::make_unique<SkFontData>(std::move(stream), ttcIndex, axisValues.get(), axisCount);
}
void SkTypeface_FreeType::onFilterRec(SkScalerContextRec* rec) const {
@@ -1753,48 +1814,7 @@
SkFontArguments::VariationPosition::Coordinate coordinates[], int coordinateCount) const
{
AutoFTAccess fta(this);
- FT_Face face = fta.face();
- if (!face) {
- return -1;
- }
-
- if (!(face->face_flags & FT_FACE_FLAG_MULTIPLE_MASTERS)) {
- return 0;
- }
-
- FT_MM_Var* variations = nullptr;
- if (FT_Get_MM_Var(face, &variations)) {
- return -1;
- }
- SkAutoFree autoFreeVariations(variations);
-
- if (!coordinates || coordinateCount < SkToInt(variations->num_axis)) {
- return variations->num_axis;
- }
-
- SkAutoSTMalloc<4, FT_Fixed> coords(variations->num_axis);
- // FT_Get_{MM,Var}_{Blend,Design}_Coordinates were added in FreeType 2.7.1.
- if (gFTLibrary->fGetVarDesignCoordinates &&
- !gFTLibrary->fGetVarDesignCoordinates(face, variations->num_axis, coords.get()))
- {
- for (FT_UInt i = 0; i < variations->num_axis; ++i) {
- coordinates[i].axis = variations->axis[i].tag;
- coordinates[i].value = SkFixedToScalar(coords[i]);
- }
- } else if (static_cast<FT_UInt>(fta.getAxesCount()) == variations->num_axis) {
- for (FT_UInt i = 0; i < variations->num_axis; ++i) {
- coordinates[i].axis = variations->axis[i].tag;
- coordinates[i].value = SkFixedToScalar(fta.getAxes()[i]);
- }
- } else if (fta.isNamedVariationSpecified()) {
- // The font has axes, they cannot be retrieved, and some named axis was specified.
- return -1;
- } else {
- // The font has axes, they cannot be retrieved, but no named instance was specified.
- return 0;
- }
-
- return variations->num_axis;
+ return GetVariationDesignPosition(fta, coordinates, coordinateCount);
}
int SkTypeface_FreeType::onGetVariationDesignParameters(
@@ -2108,13 +2128,30 @@
AxisDefinitions axisDefinitions,
const SkFontArguments::VariationPosition position,
SkFixed* axisValues,
- const SkString& name)
+ const SkString& name,
+ const SkFontArguments::VariationPosition::Coordinate* current)
{
for (int i = 0; i < axisDefinitions.count(); ++i) {
const Scanner::AxisDefinition& axisDefinition = axisDefinitions[i];
const SkScalar axisMin = SkFixedToScalar(axisDefinition.fMinimum);
const SkScalar axisMax = SkFixedToScalar(axisDefinition.fMaximum);
+
+ // Start with the default value.
axisValues[i] = axisDefinition.fDefault;
+
+ // Then the current value.
+ if (current) {
+ for (int j = 0; j < axisDefinitions.count(); ++j) {
+ const auto& coordinate = current[j];
+ if (axisDefinition.fTag == coordinate.axis) {
+ const SkScalar axisValue = SkTPin(coordinate.value, axisMin, axisMax);
+ axisValues[i] = SkScalarToFixed(axisValue);
+ break;
+ }
+ }
+ }
+
+ // Then the requested value.
// The position may be over specified. If there are multiple values for a given axis,
// use the last one since that's what css-fonts-4 requires.
for (int j = position.coordinateCount; j --> 0;) {
diff --git a/src/ports/SkFontHost_FreeType_common.h b/src/ports/SkFontHost_FreeType_common.h
index 8d1ee1f..fdbc25c 100644
--- a/src/ports/SkFontHost_FreeType_common.h
+++ b/src/ports/SkFontHost_FreeType_common.h
@@ -77,7 +77,8 @@
AxisDefinitions axisDefinitions,
const SkFontArguments::VariationPosition position,
SkFixed* axisValues,
- const SkString& name);
+ const SkString& name,
+ const SkFontArguments::VariationPosition::Coordinate* currentPosition = nullptr);
static bool GetAxes(FT_Face face, AxisDefinitions* axes);
private:
diff --git a/src/ports/SkTypeface_mac_ct.cpp b/src/ports/SkTypeface_mac_ct.cpp
index 9b250dc..94903f4 100644
--- a/src/ports/SkTypeface_mac_ct.cpp
+++ b/src/ports/SkTypeface_mac_ct.cpp
@@ -1116,6 +1116,9 @@
}
CFIndex axisCount = CFArrayGetCount(ctAxes.get());
+ // On 10.12 and later, this only returns non-default variations.
+ SkUniqueCFRef<CFDictionaryRef> ctVariation(CTFontCopyVariation(ct));
+
const SkFontArguments::VariationPosition position = args.getVariationDesignPosition();
SkUniqueCFRef<CFMutableDictionaryRef> dict(
@@ -1151,13 +1154,25 @@
return CTFontVariation();
}
+ // Start with the default value.
double value = defDouble;
+
+ // Then the current value.
+ if (ctVariation) {
+ CFTypeRef currentValue = CFDictionaryGetValue(ctVariation.get(), tagNumber);
+ if (currentValue) {
+ if (!SkCFNumberDynamicCast(currentValue, &value, nullptr, "Variation value")) {
+ return CTFontVariation();
+ }
+ }
+ }
+
+ // Then the requested value.
// The position may be over specified. If there are multiple values for a given axis,
// use the last one since that's what css-fonts-4 requires.
for (int j = position.coordinateCount; j --> 0;) {
if (position.coordinates[j].axis == tagLong) {
- value = SkTPin(SkScalarToDouble(position.coordinates[j].value),
- minDouble, maxDouble);
+ value = SkTPin<double>(position.coordinates[j].value, minDouble, maxDouble);
if (tagLong == opszTag) {
opsz.isSet = true;
}
diff --git a/tests/TypefaceTest.cpp b/tests/TypefaceTest.cpp
index 04a365a..8558ca0 100644
--- a/tests/TypefaceTest.cpp
+++ b/tests/TypefaceTest.cpp
@@ -218,9 +218,7 @@
// Convert to fixed for "almost equal".
SkFixed fixedRead = SkScalarToFixed(actual[actualIdx].value);
SkFixed fixedOriginal = SkScalarToFixed(expected.coordinates[expectedIdx].value);
- if (!(SkTAbs(fixedRead - fixedOriginal) < 2 || // variation set correctly
- SkTAbs(fixedRead - SK_Fixed1 ) < 2) ) // variation remained default
- {
+ if (!(SkTAbs(fixedRead - fixedOriginal) < 2)) {
continue;
}
@@ -295,6 +293,12 @@
params.setVariationDesignPosition({position, SK_ARRAY_COUNT(position)});
sk_sp<SkTypeface> typeface = fm->makeFromStream(std::move(distortable), params);
test(typeface.get(), Variation{&position[1], 1}, -1);
+
+ if (typeface) {
+ // Cloning without specifying any parameters should produce an equivalent variation.
+ sk_sp<SkTypeface> clone = typeface->makeClone(SkFontArguments());
+ test(clone.get(), Variation{&position[1], 1}, -1);
+ }
}
}
