commit 3fdc05a334d5acca449526a8ba61589906098835
author Florin Malita <fmalita@chromium.org> Fri May 24 12:07:15 2019 -0400
committer Skia Commit-Bot <skia-commit-bot@chromium.org> Fri May 24 18:05:37 2019 +0000
tree 89528ec15a3ba34d0baeb5b5004370fab19081fa
parent ae9633b352206423464e7fa3bf7163be3c275e42

[skottie] Always use explicit text line height

Line height is always specified for JSON text values, there is no need
to treat as optional or have a fallback based on font metrics.

Change-Id: I468666e82dab74203fee985503c020217e0d4db8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/215829
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>

modules/skottie/src/text/SkottieShaper.cpp

diff --git a/modules/skottie/src/text/SkottieShaper.cpp b/modules/skottie/src/text/SkottieShaper.cpp
index 6c83d52..59c7922 100644
--- a/modules/skottie/src/text/SkottieShaper.cpp
+++ b/modules/skottie/src/text/SkottieShaper.cpp
@@ -7,7 +7,6 @@
 
 #include "modules/skottie/src/text/SkottieShaper.h"
 
-#include "include/core/SkFontMetrics.h"
 #include "include/core/SkTextBlob.h"
 #include "modules/skshaper/include/SkShaper.h"
 #include "src/core/SkTextBlobPriv.h"
@@ -62,18 +61,10 @@
     void beginLine() override {
         fCurrentPosition = fOffset;
         fPendingLineAdvance  = { 0, 0 };
-        fMaxRunAscent = 0;
-        fMaxRunDescent = 0;
-        fMaxRunLeading = 0;
     }
 
     void runInfo(const RunInfo& info) override {
         fPendingLineAdvance += info.fAdvance;
-        SkFontMetrics metrics;
-        info.fFont.getMetrics(&metrics);
-        fMaxRunAscent = SkTMin(fMaxRunAscent, metrics.fAscent);
-        fMaxRunDescent = SkTMax(fMaxRunDescent, metrics.fDescent);
-        fMaxRunLeading = SkTMax(fMaxRunLeading, metrics.fLeading);
     }
 
     void commitRunInfo() override {}
@@ -81,9 +72,6 @@
     Buffer runBuffer(const RunInfo& info) override {
         int glyphCount = SkTFitsIn<int>(info.glyphCount) ? info.glyphCount : INT_MAX;
 
-        SkFontMetrics metrics;
-        info.fFont.getMetrics(&metrics);
-
         const auto& blobBuffer = fBuilder.allocRunPos(info.fFont, glyphCount);
 
         SkVector alignmentOffset { fHAlignFactor * (fPendingLineAdvance.x() - fBox.width()), 0 };
@@ -102,11 +90,7 @@
     }
 
     void commitLine() override {
-        // Observe explicit line height, if specified; otherwise use line metrics.
-        const auto lh = fDesc.fLineHeight > 0
-                ? fDesc.fLineHeight
-                : fMaxRunDescent + fMaxRunLeading - fMaxRunAscent;
-        fOffset.fY += lh;
+        fOffset.fY += fDesc.fLineHeight;
     }
 
     Shaper::Result makeBlob() {
@@ -193,9 +177,6 @@
     SkTextBlobBuilder         fBuilder;
     std::unique_ptr<SkShaper> fShaper;
 
-    SkScalar fMaxRunAscent;
-    SkScalar fMaxRunDescent;
-    SkScalar fMaxRunLeading;
     SkPoint  fCurrentPosition{ 0, 0 };
     SkPoint  fOffset{ 0, 0 };
     SkVector fPendingLineAdvance{ 0, 0 };

modules/skottie/src/text/SkottieShaper.h

diff --git a/modules/skottie/src/text/SkottieShaper.h b/modules/skottie/src/text/SkottieShaper.h
index 56145a7..b3c763d 100644
--- a/modules/skottie/src/text/SkottieShaper.h
+++ b/modules/skottie/src/text/SkottieShaper.h
@@ -45,7 +45,7 @@
     struct TextDesc {
         const sk_sp<SkTypeface>&  fTypeface;
         SkScalar                  fTextSize,
-                                  fLineHeight; // when 0, use natural/computed line height.
+                                  fLineHeight;
         SkTextUtils::Align        fHAlign;
         VAlign                    fVAlign;
     };

modules/skottie/src/text/TextValue.cpp

diff --git a/modules/skottie/src/text/TextValue.cpp b/modules/skottie/src/text/TextValue.cpp
index 4fe6c52..0229d75 100644
--- a/modules/skottie/src/text/TextValue.cpp
+++ b/modules/skottie/src/text/TextValue.cpp
@@ -40,16 +40,17 @@
         return false;
     }
 
-    const skjson::StringValue* font_name = (*jtxt)["f"];
-    const skjson::StringValue* text      = (*jtxt)["t"];
-    const skjson::NumberValue* text_size = (*jtxt)["s"];
-    if (!font_name || !text || !text_size ||
+    const skjson::StringValue* font_name   = (*jtxt)["f"];
+    const skjson::StringValue* text        = (*jtxt)["t"];
+    const skjson::NumberValue* text_size   = (*jtxt)["s"];
+    const skjson::NumberValue* line_height = (*jtxt)["lh"];
+    if (!font_name || !text || !text_size || !line_height ||
         !(v->fTypeface = abuilder->findFont(SkString(font_name->begin(), font_name->size())))) {
         return false;
     }
     v->fText.set(text->begin(), text->size());
-    v->fTextSize = **text_size;
-    v->fLineHeight = ParseDefault<float>((*jtxt)["lh"], 0);
+    v->fTextSize   = **text_size;
+    v->fLineHeight = **line_height;
 
     static constexpr SkTextUtils::Align gAlignMap[] = {
         SkTextUtils::kLeft_Align,  // 'j': 0