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>

3 files changed, 16 insertions, 8 deletions

diff --git a/src/ports/SkFontHost_FreeType.cpp b/src/ports/SkFontHost_FreeType.cpp
index 240f75c2e2..91801cdbd0 100644
--- a/src/ports/SkFontHost_FreeType.cpp
+++ b/src/ports/SkFontHost_FreeType.cpp
@@ -1855,7 +1855,9 @@ bool SkTypeface_FreeType::Scanner::scanFont(
         const SkScalar axisMin = SkFixedToScalar(axisDefinition.fMinimum);
         const SkScalar axisMax = SkFixedToScalar(axisDefinition.fMaximum);
         axisValues[i] = axisDefinition.fDefault;
-        for (int j = 0; j < position.coordinateCount; ++j) {
+        // 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;) {
             const auto& coordinate = position.coordinates[j];
             if (axisDefinition.fTag == coordinate.axis) {
                 const SkScalar axisValue = SkTPin(coordinate.value, axisMin, axisMax);
diff --git a/src/ports/SkFontHost_mac.cpp b/src/ports/SkFontHost_mac.cpp
index 176a20cd65..d13be5a03b 100644
--- a/src/ports/SkFontHost_mac.cpp
+++ b/src/ports/SkFontHost_mac.cpp
@@ -2488,7 +2488,9 @@ protected:
             }
 
             double value = defDouble;
-            for (int j = 0; j < position.coordinateCount; ++j) {
+            // 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);
diff --git a/tests/TypefaceTest.cpp b/tests/TypefaceTest.cpp
index 4786cc278c..8d7ac31b99 100644
--- a/tests/TypefaceTest.cpp
+++ b/tests/TypefaceTest.cpp
@@ -96,10 +96,14 @@ DEF_TEST(TypefaceAxes, reporter) {
         REPORT_FAILURE(reporter, "distortable", SkString());
         return;
     }
+    constexpr int numberOfAxesInDistortable = 1;
 
     sk_sp<SkFontMgr> fm = SkFontMgr::RefDefault();
+    // The position may be over specified. If there are multiple values for a given axis,
+    // ensure the last one since that's what css-fonts-4 requires.
     const SkFontArguments::VariationPosition::Coordinate position[] = {
-        { SkSetFourByteTag('w','g','h','t'), SK_ScalarSqrt2 }
+        { SkSetFourByteTag('w','g','h','t'), 1.618033988749895f },
+        { SkSetFourByteTag('w','g','h','t'), SK_ScalarSqrt2 },
     };
     SkFontArguments params;
     params.setVariationDesignPosition({position, SK_ARRAY_COUNT(position)});
@@ -110,17 +114,17 @@ DEF_TEST(TypefaceAxes, reporter) {
     if (count == -1) {
         return;
     }
-    REPORTER_ASSERT(reporter, count == SK_ARRAY_COUNT(position));
+    REPORTER_ASSERT(reporter, count == numberOfAxesInDistortable);
 
-    SkFontArguments::VariationPosition::Coordinate positionRead[SK_ARRAY_COUNT(position)];
+    SkFontArguments::VariationPosition::Coordinate positionRead[numberOfAxesInDistortable];
     count = typeface->getVariationDesignPosition(positionRead, SK_ARRAY_COUNT(positionRead));
-    REPORTER_ASSERT(reporter, count == SK_ARRAY_COUNT(position));
+    REPORTER_ASSERT(reporter, count == SK_ARRAY_COUNT(positionRead));
 
-    REPORTER_ASSERT(reporter, positionRead[0].axis == position[0].axis);
+    REPORTER_ASSERT(reporter, positionRead[0].axis == position[1].axis);
 
     // Convert to fixed for "almost equal".
     SkFixed fixedRead = SkScalarToFixed(positionRead[0].value);
-    SkFixed fixedOriginal = SkScalarToFixed(position[0].value);
+    SkFixed fixedOriginal = SkScalarToFixed(position[1].value);
     REPORTER_ASSERT(reporter, fixedRead == fixedOriginal);
 }