[skotty] Animator cleanup pass

TBR=
Change-Id: I2849c1438a8b245f04a01977919b653f0a16492b
Reviewed-on: https://skia-review.googlesource.com/92380
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Florin Malita <fmalita@chromium.org>

5 files changed, 199 insertions, 199 deletions

diff --git a/experimental/skotty/Skotty.cpp b/experimental/skotty/Skotty.cpp
index 5c3016c6c9..f89e975ce8 100644
--- a/experimental/skotty/Skotty.cpp
+++ b/experimental/skotty/Skotty.cpp
@@ -72,9 +72,9 @@ bool AttachProperty(const Json::Value& jprop, AttachContext* ctx, const sk_sp<No
     // For those, we attempt to parse both ways.
     if (jpropA.isNull() || !ParseBool(jpropA, "false")) {
         ValueT val;
-        if (ValueT::Parse(jpropK, &val)) {
+        if (ValueTraits<ValueT>::Parse(jpropK, &val)) {
             // Static property.
-            apply(node, val.template as<AttrT>());
+            apply(node, ValueTraits<ValueT>::template As<AttrT>(val));
             return true;
         }
 
diff --git a/experimental/skotty/SkottyAnimator.cpp b/experimental/skotty/SkottyAnimator.cpp
index d9f9b39ff1..698e961b64 100644
--- a/experimental/skotty/SkottyAnimator.cpp
+++ b/experimental/skotty/SkottyAnimator.cpp
@@ -11,48 +11,84 @@ namespace skotty {
 
 namespace {
 
-SkScalar lerp_scalar(SkScalar v0, SkScalar v1, float t) {
+SkScalar lerp_scalar(float v0, float v1, float t) {
     SkASSERT(t >= 0 && t <= 1);
     return v0 * (1 - t) + v1 * t;
 }
 
 } // namespace
 
+bool KeyframeIntervalBase::parse(const Json::Value& k, KeyframeIntervalBase* prev) {
+    SkASSERT(k.isObject());
+
+    fT0 = fT1 = ParseScalar(k["t"], SK_ScalarMin);
+    if (fT0 == SK_ScalarMin) {
+        return false;
+    }
+
+    if (prev) {
+        if (prev->fT1 >= fT0) {
+            LOG("!! Dropping out-of-order key frame (t: %f < t: %f)\n", fT0, prev->fT1);
+            return false;
+        }
+        // Back-fill t1 in prev interval.  Note: we do this even if we end up discarding
+        // the current interval (to support "t"-only final frames).
+        prev->fT1 = fT0;
+    }
+
+    // default is linear lerp
+    static constexpr SkPoint kDefaultC0 = { 0, 0 },
+                             kDefaultC1 = { 1, 1 };
+    const auto c0 = ParsePoint(k["i"], kDefaultC0),
+               c1 = ParsePoint(k["o"], kDefaultC1);
+
+    if (c0 != kDefaultC0 || c1 != kDefaultC1) {
+        fCubicMap = skstd::make_unique<SkCubicMap>();
+        // TODO: why do we have to plug these inverted?
+        fCubicMap->setPts(c1, c0);
+    }
+
+    return true;
+}
+
+float KeyframeIntervalBase::localT(float t) const {
+    SkASSERT(this->isValid());
+    auto lt = (t - fT0) / (fT1 - fT0);
+
+    if (fCubicMap) {
+        lt = fCubicMap->computeYFromX(lt);
+    }
+
+    return SkTPin<float>(lt, 0, 1);
+}
+
 template <>
 void KeyframeInterval<ScalarValue>::lerp(float t, ScalarValue* v) const {
-    *v = lerp_scalar(fV0, fV1, t);
+    const auto lt = this->localT(t);
+    *v = lerp_scalar(fV0, fV1, lt);
 }
 
 template <>
 void KeyframeInterval<VectorValue>::lerp(float t, VectorValue* v) const {
-    SkASSERT(fV0.cardinality() == fV1.cardinality());
-    SkASSERT(v->cardinality() == 0);
+    SkASSERT(fV0.size() == fV1.size());
+    SkASSERT(v->size() == 0);
+
+    const auto lt = this->localT(t);
 
-    v->fVals.reserve(fV0.cardinality());
-    for (int i = 0; i < fV0.fVals.count(); ++i) {
-        v->fVals.emplace_back(lerp_scalar(fV0.fVals[i], fV1.fVals[i], t));
+    v->reserve(fV0.size());
+    for (size_t i = 0; i < fV0.size(); ++i) {
+        v->push_back(lerp_scalar(fV0[i], fV1[i], lt));
     }
 }
 
 template <>
 void KeyframeInterval<ShapeValue>::lerp(float t, ShapeValue* v) const {
-    SkASSERT(fV0.cardinality() == fV1.cardinality());
-    SkASSERT(v->cardinality() == 0);
+    SkASSERT(fV0.countVerbs() == fV1.countVerbs());
+    SkASSERT(v->isEmpty());
 
-    SkAssertResult(fV1.fPath.interpolate(fV0.fPath, t, &v->fPath));
-    v->fPath.setIsVolatile(true);
-}
-
-float AnimatorBase::ComputeLocalT(float t, float t0, float t1,
-                                  const SkCubicMap* cubicMap) {
-    SkASSERT(t1 > t0);
-    auto lt = (t - t0) / (t1 - t0);
-
-    if (cubicMap) {
-        lt = cubicMap->computeYFromX(lt);
-    }
-
-    return SkTPin<float>(lt, 0, 1);
+    const auto lt = this->localT(t);
+    SkAssertResult(fV1.interpolate(fV0, lt, v));
+    v->setIsVolatile(true);
 }
 
 } // namespace skotty
diff --git a/experimental/skotty/SkottyAnimator.h b/experimental/skotty/SkottyAnimator.h
index 0e5929334e..cd6f651ab8 100644
--- a/experimental/skotty/SkottyAnimator.h
+++ b/experimental/skotty/SkottyAnimator.h
@@ -27,69 +27,60 @@ public:
 
 protected:
     AnimatorBase() = default;
-
-    // Compute a cubic-Bezier-interpolated t relative to [t0..t1].
-    static float ComputeLocalT(float t, float t0, float t1,
-                               const SkCubicMap*);
 };
 
-// Describes a keyframe interpolation interval (v0@t0) -> (v1@t1).
-// TODO: add interpolation params.
-template <typename T>
-struct KeyframeInterval {
-    // Start/end values.
-    T       fV0,
-            fV1;
+class KeyframeIntervalBase : public SkNoncopyable {
+public:
+    KeyframeIntervalBase()                                  = default;
+    KeyframeIntervalBase(KeyframeIntervalBase&&)            = default;
+    KeyframeIntervalBase& operator=(KeyframeIntervalBase&&) = default;
 
-    // Start/end times.
-    float   fT0 = 0,
-            fT1 = 0;
+    float t0() const { return fT0; }
+    float t1() const { return fT1; }
 
-    // Initialized for non-linear lerp.
-    std::unique_ptr<SkCubicMap> fCubicMap;
+    bool isValid() const { return fT0 < fT1; }
+    bool contains(float t) const { return t >= fT0 && t <= fT1; }
 
+protected:
     // Parse the current interval AND back-fill prev interval t1.
-    bool parse(const Json::Value& k, KeyframeInterval* prev) {
-        SkASSERT(k.isObject());
-
-        fT0 = fT1 = ParseScalar(k["t"], SK_ScalarMin);
-        if (fT0 == SK_ScalarMin) {
-            return false;
-        }
+    bool parse(const Json::Value&, KeyframeIntervalBase* prev);
 
-        if (prev) {
-            if (prev->fT1 >= fT0) {
-                LOG("!! Dropping out-of-order key frame (t: %f < t: %f)\n", fT0, prev->fT1);
-                return false;
-            }
-            // Back-fill t1 in prev interval.  Note: we do this even if we end up discarding
-            // the current interval (to support "t"-only final frames).
-            prev->fT1 = fT0;
-        }
+    // Computes a "local" t (relative to [fT0..fT1]), and mapped
+    // through the cubic (if applicable).
+    float localT(float t) const;
 
-        if (!T::Parse(k["s"], &fV0) ||
-            !T::Parse(k["e"], &fV1) ||
-            fV0.cardinality() != fV1.cardinality() ||
-            (prev && fV0.cardinality() != prev->fV0.cardinality())) {
-            return false;
-        }
+private:
+    // Initialized for non-linear interpolation.
+    std::unique_ptr<SkCubicMap> fCubicMap;
 
-        // default is linear lerp
-        static constexpr SkPoint kDefaultC0 = { 0, 0 },
-                                 kDefaultC1 = { 1, 1 };
-        const auto c0 = ParsePoint(k["i"], kDefaultC0),
-                   c1 = ParsePoint(k["o"], kDefaultC1);
+    // Start/end times.
+    float                       fT0 = 0,
+                                fT1 = 0;
+};
 
-        if (c0 != kDefaultC0 || c1 != kDefaultC1) {
-            fCubicMap = skstd::make_unique<SkCubicMap>();
-            // TODO: why do we have to plug these inverted?
-            fCubicMap->setPts(c1, c0);
-        }
+// Describes a keyframe interpolation interval (v0@t0) -> (v1@t1).
+template <typename T>
+class KeyframeInterval final : public KeyframeIntervalBase {
+public:
+    // Parse the current interval AND back-fill prev interval t1.
+    bool parse(const Json::Value& k, KeyframeInterval* prev) {
+        SkASSERT(k.isObject());
 
-        return true;
+        return this->INHERITED::parse(k, prev) &&
+            ValueTraits<T>::Parse(k["s"], &fV0) &&
+            ValueTraits<T>::Parse(k["e"], &fV1) &&
+            ValueTraits<T>::Cardinality(fV0) == ValueTraits<T>::Cardinality(fV1) &&
+            (!prev || ValueTraits<T>::Cardinality(fV0) == ValueTraits<T>::Cardinality(prev->fV0));
     }
 
     void lerp(float t, T*) const;
+
+private:
+    // Start/end values.
+    T       fV0,
+            fV1;
+
+    using INHERITED = KeyframeIntervalBase;
 };
 
 // Binds an animated/keyframed property to a node attribute.
@@ -104,9 +95,9 @@ public:
         const auto& frame = this->findInterval(t);
 
         ValT val;
-        frame.lerp(ComputeLocalT(t, frame.fT0, frame.fT1, frame.fCubicMap.get()), &val);
+        frame.lerp(t, &val);
 
-        fFunc(fTarget, val.template as<AttrT>());
+        fFunc(fTarget, ValueTraits<ValT>::template As<AttrT>(val));
     }
 
 private:
@@ -118,9 +109,9 @@ private:
 
     const KeyframeInterval<ValT>& findInterval(float t) const;
 
-    const SkTArray<KeyframeInterval<ValT>>                 fIntervals;
-    sk_sp<NodeT>                                           fTarget;
-    ApplyFuncT fFunc;
+    const SkTArray<KeyframeInterval<ValT>> fIntervals;
+    sk_sp<NodeT>                           fTarget;
+    ApplyFuncT                             fFunc;
 };
 
 template <typename ValT, typename AttrT, typename NodeT>
@@ -134,21 +125,20 @@ Animator<ValT, AttrT, NodeT>::Make(const Json::Value& frames, sk_sp<NodeT> node,
     SkTArray<KeyframeInterval<ValT>> intervals;
     intervals.reserve(frames.size());
 
+    KeyframeInterval<ValT>* prev_interval = nullptr;
     for (const auto& frame : frames) {
         if (!frame.isObject())
             return nullptr;
 
-        auto& curr_interval = intervals.push_back();
-        auto* prev_interval = intervals.count() > 1 ? &intervals.fromBack(1) : nullptr;
-        if (!curr_interval.parse(frame, prev_interval)) {
-            // Invalid frame, or "t"-only frame.
-            intervals.pop_back();
-            continue;
+        KeyframeInterval<ValT> curr_interval;
+        if (curr_interval.parse(frame, prev_interval)) {
+            intervals.push_back(std::move(curr_interval));
+            prev_interval = &intervals.back();
         }
     }
 
     // If we couldn't determine a t1 for the last interval, discard it.
-    if (!intervals.empty() && intervals.back().fT0 == intervals.back().fT1) {
+    if (!intervals.empty() && !intervals.back().isValid()) {
         intervals.pop_back();
     }
 
@@ -169,25 +159,25 @@ const KeyframeInterval<ValT>& Animator<ValT, AttrT, NodeT>::findInterval(float t
     auto f0 = fIntervals.begin(),
          f1 = fIntervals.end() - 1;
 
-    SkASSERT(f0->fT0 < f0->fT1);
-    SkASSERT(f1->fT0 < f1->fT1);
+    SkASSERT(f0->isValid());
+    SkASSERT(f1->isValid());
 
-    if (t < f0->fT0) {
+    if (t < f0->t0()) {
         return *f0;
     }
 
-    if (t > f1->fT1) {
+    if (t > f1->t1()) {
         return *f1;
     }
 
     while (f0 != f1) {
         SkASSERT(f0 < f1);
-        SkASSERT(t >= f0->fT0 && t <= f1->fT1);
+        SkASSERT(t >= f0->t0() && t <= f1->t1());
 
         const auto f = f0 + (f1 - f0) / 2;
-        SkASSERT(f->fT0 < f->fT1);
+        SkASSERT(f->isValid());
 
-        if (t > f->fT1) {
+        if (t > f->t1()) {
             f0 = f + 1;
         } else {
             f1 = f;
@@ -195,7 +185,8 @@ const KeyframeInterval<ValT>& Animator<ValT, AttrT, NodeT>::findInterval(float t
     }
 
     SkASSERT(f0 == f1);
-    SkASSERT(t >= f0->fT0 && t <= f1->fT1);
+    SkASSERT(f0->contains(t));
+
     return *f0;
 }
 
diff --git a/experimental/skotty/SkottyProperties.cpp b/experimental/skotty/SkottyProperties.cpp
index 0783ce8aaa..3bd95d8969 100644
--- a/experimental/skotty/SkottyProperties.cpp
+++ b/experimental/skotty/SkottyProperties.cpp
@@ -42,7 +42,8 @@ bool ParsePoints(const Json::Value& v, PointArray* pts) {
 
 } // namespace
 
-bool ScalarValue::Parse(const Json::Value& v, ScalarValue* scalar) {
+template <>
+bool ValueTraits<ScalarValue>::Parse(const Json::Value& v, ScalarValue* scalar) {
     // Some files appear to wrap keyframes in arrays for no reason.
     if (v.isArray() && v.size() == 1) {
         return Parse(v[0], scalar);
@@ -51,28 +52,77 @@ bool ScalarValue::Parse(const Json::Value& v, ScalarValue* scalar) {
     if (v.isNull() || !v.isConvertibleTo(Json::realValue))
         return false;
 
-    scalar->fVal = ParseScalar(v, 0);
+    *scalar = v.asFloat();
     return true;
 }
 
-bool VectorValue::Parse(const Json::Value& v, VectorValue* vec) {
-    SkASSERT(vec->fVals.empty());
+template <>
+size_t ValueTraits<ScalarValue>::Cardinality(const ScalarValue&) {
+    return 1;
+}
+
+template <>
+template <>
+SkScalar ValueTraits<ScalarValue>::As<SkScalar>(const ScalarValue& v) {
+    return v;
+}
+
+template <>
+bool ValueTraits<VectorValue>::Parse(const Json::Value& v, VectorValue* vec) {
+    SkASSERT(vec->empty());
 
     if (!v.isArray())
         return false;
 
     for (Json::ArrayIndex i = 0; i < v.size(); ++i) {
-        const auto& el = v[i];
-        if (el.isNull() || !el.isConvertibleTo(Json::realValue))
+        ScalarValue scalar;
+        if (!ValueTraits<ScalarValue>::Parse(v[i], &scalar))
             return false;
 
-        vec->fVals.emplace_back(ParseScalar(el, 0));
+        vec->push_back(std::move(scalar));
     }
 
     return true;
 }
 
-bool ShapeValue::Parse(const Json::Value& v, ShapeValue* shape) {
+template <>
+size_t ValueTraits<VectorValue>::Cardinality(const VectorValue& vec) {
+    return vec.size();
+}
+
+template <>
+template <>
+SkColor ValueTraits<VectorValue>::As<SkColor>(const VectorValue& vec) {
+    // best effort to turn this into a color
+    const auto r = vec.size() > 0 ? vec[0] : 0,
+               g = vec.size() > 1 ? vec[1] : 0,
+               b = vec.size() > 2 ? vec[2] : 0,
+               a = vec.size() > 3 ? vec[3] : 1;
+
+    return SkColorSetARGB(SkTPin<SkScalar>(a, 0, 1) * 255,
+                          SkTPin<SkScalar>(r, 0, 1) * 255,
+                          SkTPin<SkScalar>(g, 0, 1) * 255,
+                          SkTPin<SkScalar>(b, 0, 1) * 255);
+}
+
+template <>
+template <>
+SkPoint ValueTraits<VectorValue>::As<SkPoint>(const VectorValue& vec) {
+    // best effort to turn this into a point
+    const auto x = vec.size() > 0 ? vec[0] : 0,
+               y = vec.size() > 1 ? vec[1] : 0;
+    return SkPoint::Make(x, y);
+}
+
+template <>
+template <>
+SkSize ValueTraits<VectorValue>::As<SkSize>(const VectorValue& vec) {
+    const auto pt = ValueTraits::As<SkPoint>(vec);
+    return SkSize::Make(pt.x(), pt.y());
+}
+
+template<>
+bool ValueTraits<ShapeValue>::Parse(const Json::Value& v, ShapeValue* shape) {
     PointArray inPts,  // Cubic Bezier "in" control points, relative to vertices.
                outPts, // Cubic Bezier "out" control points, relative to vertices.
                verts;  // Cubic Bezier vertices.
@@ -92,16 +142,16 @@ bool ShapeValue::Parse(const Json::Value& v, ShapeValue* shape) {
         return false;
     }
 
-    SkASSERT(shape->fPath.isEmpty());
+    SkASSERT(shape->isEmpty());
 
     if (!verts.empty()) {
-        shape->fPath.moveTo(verts.front());
+        shape->moveTo(verts.front());
     }
 
     const auto& addCubic = [&](int from, int to) {
-        shape->fPath.cubicTo(verts[from] + outPts[from],
-                             verts[to]   + inPts[to],
-                             verts[to]);
+        shape->cubicTo(verts[from] + outPts[from],
+                       verts[to]   + inPts[to],
+                       verts[to]);
     };
 
     for (int i = 1; i < verts.count(); ++i) {
@@ -110,55 +160,21 @@ bool ShapeValue::Parse(const Json::Value& v, ShapeValue* shape) {
 
     if (!verts.empty() && ParseBool(v["c"], false)) {
         addCubic(verts.count() - 1, 0);
-        shape->fPath.close();
+        shape->close();
     }
 
     return true;
 }
 
 template <>
-SkColor VectorValue::as<SkColor>() const {
-    // best effort to turn this into a color
-    const auto r = fVals.count() > 0 ? fVals[0].as<SkScalar>() : 0,
-               g = fVals.count() > 1 ? fVals[1].as<SkScalar>() : 0,
-               b = fVals.count() > 2 ? fVals[2].as<SkScalar>() : 0,
-               a = fVals.count() > 3 ? fVals[3].as<SkScalar>() : 1;
-
-    return SkColorSetARGB(SkTPin<SkScalar>(a, 0, 1) * 255,
-                          SkTPin<SkScalar>(r, 0, 1) * 255,
-                          SkTPin<SkScalar>(g, 0, 1) * 255,
-                          SkTPin<SkScalar>(b, 0, 1) * 255);
-}
-
-template <>
-SkPoint VectorValue::as<SkPoint>() const {
-    // best effort to turn this into a point
-    const auto x = fVals.count() > 0 ? fVals[0].as<SkScalar>() : 0,
-               y = fVals.count() > 1 ? fVals[1].as<SkScalar>() : 0;
-    return SkPoint::Make(x, y);
+size_t ValueTraits<ShapeValue>::Cardinality(const ShapeValue& path) {
+    return SkTo<size_t>(path.countVerbs());
 }
 
 template <>
-SkSize VectorValue::as<SkSize>() const {
-    const auto pt = this->as<SkPoint>();
-    return SkSize::Make(pt.x(), pt.y());
-}
-
-template <>
-std::vector<SkScalar> VectorValue::as<std::vector<SkScalar>>() const {
-    std::vector<SkScalar> vec;
-    vec.reserve(fVals.count());
-
-    for (const auto& val : fVals) {
-        vec.push_back(val);
-    }
-
-    return vec;
-}
-
 template <>
-SkPath ShapeValue::as<SkPath>() const {
-    return fPath;
+SkPath ValueTraits<ShapeValue>::As<SkPath>(const ShapeValue& path) {
+    return path;
 }
 
 CompositeRRect::CompositeRRect(sk_sp<sksg::RRect> wrapped_node)
diff --git a/experimental/skotty/SkottyProperties.h b/experimental/skotty/SkottyProperties.h
index 164f78093c..4357367b3f 100644
--- a/experimental/skotty/SkottyProperties.h
+++ b/experimental/skotty/SkottyProperties.h
@@ -17,8 +17,7 @@
 #include "SkTypes.h"
 
 #include <memory>
-
-class SkPath;
+#include <vector>
 
 namespace sksg {
 class Matrix;
@@ -29,60 +28,18 @@ class RenderNode;;
 
 namespace  skotty {
 
-struct ScalarValue {
-    float fVal;
-
-    static bool Parse(const Json::Value&, ScalarValue*);
-
-    ScalarValue() : fVal(0) {}
-    explicit ScalarValue(SkScalar v) : fVal(v) {}
-
-    ScalarValue& operator=(SkScalar v) { fVal = v; return *this; }
-
-    operator SkScalar() const { return fVal; }
+template <typename T>
+struct ValueTraits {
+    static bool   Parse(const Json::Value&, T*);
+    static size_t Cardinality(const T&);
 
-    size_t cardinality() const { return 1; }
-
-    template <typename T>
-    T as() const;
+    template <typename U>
+    static U As(const T&);
 };
 
-template <>
-inline SkScalar ScalarValue::as<SkScalar>() const {
-    return fVal;
-}
-
-struct VectorValue {
-    SkTArray<ScalarValue, true> fVals;
-
-    static bool Parse(const Json::Value&, VectorValue*);
-
-    VectorValue()                               = default;
-    VectorValue(const VectorValue&)             = delete;
-    VectorValue(VectorValue&&)                  = default;
-    VectorValue& operator==(const VectorValue&) = delete;
-
-    size_t cardinality() const { return SkTo<size_t>(fVals.count()); }
-
-    template <typename T>
-    T as() const;
-};
-
-struct ShapeValue {
-    SkPath fPath;
-
-    ShapeValue()                              = default;
-    ShapeValue(const ShapeValue&)             = delete;
-    ShapeValue(ShapeValue&&)                  = default;
-    ShapeValue& operator==(const ShapeValue&) = delete;
-
-    static bool Parse(const Json::Value&, ShapeValue*);
-
-    size_t cardinality() const { return SkTo<size_t>(fPath.countVerbs()); }
-
-    template <typename T>
-    T as() const;
-};
+using ScalarValue = SkScalar;
+using VectorValue = std::vector<ScalarValue>;
+using ShapeValue  = SkPath;
 
 // Composite properties.