From 420696961c8c36299d3b712b1e34811ac5bb8cf5 Mon Sep 17 00:00:00 2001
From: Florin Malita <fmalita@chromium.org>
Date: Sun, 26 Aug 2018 14:21:21 -0400
Subject: [PATCH] [skottie] Relax constant keyframe parsing

Constant keyframes currently requiere an "h" property/marker.

In order to support keyframed text parsing, relax this to consider any
keyframe missing an end value as constant.

TBR=
Change-Id: Idf332dc0174ee0ee6773708518a0bbc284f8860f
Reviewed-on: https://skia-review.googlesource.com/149265
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Florin Malita <fmalita@chromium.org>
---
 modules/skottie/src/SkottieAnimator.cpp | 48 ++++++++++++-------------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/modules/skottie/src/SkottieAnimator.cpp b/modules/skottie/src/SkottieAnimator.cpp
index 7837f3ee4ea90..3433d1e8ae7f9 100644
--- a/modules/skottie/src/SkottieAnimator.cpp
+++ b/modules/skottie/src/SkottieAnimator.cpp
@@ -88,35 +88,35 @@ class KeyframeAnimatorBase : public sksg::Animator {
                 fRecs.back().t1 = t0;
             }
 
-            const auto vidx0 = this->parseValue((*jframe)["s"], abuilder);
-            if (vidx0 < 0)
+            // Required start value.
+            const auto v0_idx = this->parseValue((*jframe)["s"], abuilder);
+            if (v0_idx < 0)
                 continue;
 
-            // Defaults for constant frames.
-            int vidx1 = vidx0, cmidx = -1;
-
-            if (!ParseDefault<bool>((*jframe)["h"], false)) {
-                // Regular frame, requires an end value.
-                vidx1 = this->parseValue((*jframe)["e"], abuilder);
-                if (vidx1 < 0)
-                    continue;
+            // Optional end value.
+            const auto v1_idx = this->parseValue((*jframe)["e"], abuilder);
+            if (v1_idx < 0) {
+                // Constant keyframe.
+                fRecs.push_back({t0, t0, v0_idx, v0_idx, -1 });
+                continue;
+            }
 
-                // default is linear lerp
-                static constexpr SkPoint kDefaultC0 = { 0, 0 },
-                                         kDefaultC1 = { 1, 1 };
-                const auto c0 = ParseDefault<SkPoint>((*jframe)["i"], kDefaultC0),
-                           c1 = ParseDefault<SkPoint>((*jframe)["o"], kDefaultC1);
-
-                if (c0 != kDefaultC0 || c1 != kDefaultC1) {
-                    // TODO: is it worth de-duping these?
-                    cmidx = fCubicMaps.count();
-                    fCubicMaps.emplace_back();
-                    // TODO: why do we have to plug these inverted?
-                    fCubicMaps.back().setPts(c1, c0);
-                }
+            // default is linear lerp
+            static constexpr SkPoint kDefaultC0 = { 0, 0 },
+                                     kDefaultC1 = { 1, 1 };
+            const auto c0 = ParseDefault<SkPoint>((*jframe)["i"], kDefaultC0),
+                       c1 = ParseDefault<SkPoint>((*jframe)["o"], kDefaultC1);
+
+            int cm_idx = -1;
+            if (c0 != kDefaultC0 || c1 != kDefaultC1) {
+                // TODO: is it worth de-duping these?
+                cm_idx = fCubicMaps.count();
+                fCubicMaps.emplace_back();
+                // TODO: why do we have to plug these inverted?
+                fCubicMaps.back().setPts(c1, c0);
             }
 
-            fRecs.push_back({t0, t0, vidx0, vidx1, cmidx });
+            fRecs.push_back({t0, t0, v0_idx, v1_idx, cm_idx });
         }
 
         // If we couldn't determine a valid t1 for the last frame, discard it.