[skottie] Cleanup: finalize adapter conversion

Convert remaining bindProperty clients to adapters, and remove the
legacy API.

TBR=
Change-Id: I06eda513a75b55a1ba7eae328b0916e738c9ddd7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/268309
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
diff --git a/modules/skottie/skottie.gni b/modules/skottie/skottie.gni
index 20060ce..7347d4b 100644
--- a/modules/skottie/skottie.gni
+++ b/modules/skottie/skottie.gni
@@ -22,8 +22,8 @@
   "$_src/Composition.h",
   "$_src/Layer.cpp",
   "$_src/Layer.h",
+  "$_src/Path.cpp",
   "$_src/Skottie.cpp",
-  "$_src/SkottieAnimator.cpp",
   "$_src/SkottieJson.cpp",
   "$_src/SkottieJson.h",
   "$_src/SkottiePriv.h",
diff --git a/modules/skottie/src/Layer.cpp b/modules/skottie/src/Layer.cpp
index bfcc0dc..e4880bc 100644
--- a/modules/skottie/src/Layer.cpp
+++ b/modules/skottie/src/Layer.cpp
@@ -60,22 +60,65 @@
     return nullptr;
 }
 
+class MaskAdapter final : public AnimatablePropertyContainer {
+public:
+    MaskAdapter(const skjson::ObjectValue& jmask, const AnimationBuilder& abuilder, SkBlendMode bm)
+        : fMaskPaint(sksg::Color::Make(SK_ColorBLACK)) {
+        fMaskPaint->setAntiAlias(true);
+        fMaskPaint->setBlendMode(bm);
+
+        this->bind(abuilder, jmask["o"], fOpacity);
+
+        if (this->bind(abuilder, jmask["f"], fFeather)) {
+            fMaskFilter = sksg::BlurImageFilter::Make();
+        }
+    }
+
+    bool hasEffect() const {
+        return !this->isStatic()
+            || fOpacity < 100
+            || ValueTraits<VectorValue>::As<SkVector>(fFeather) != SkVector{ 0, 0 };
+    }
+
+    sk_sp<sksg::RenderNode> makeMask(sk_sp<sksg::Path> mask_path) const {
+        auto mask = sksg::Draw::Make(std::move(mask_path), fMaskPaint);
+
+        // Optional mask blur (feather).
+        return sksg::ImageFilterEffect::Make(std::move(mask), fMaskFilter);
+    }
+
+private:
+    void onSync() override {
+        fMaskPaint->setOpacity(fOpacity * 0.01f);
+        if (fMaskFilter) {
+            const auto f = ValueTraits<VectorValue>::As<SkVector>(fFeather);
+
+            // Close enough to AE.
+            static constexpr SkScalar kFeatherToSigma = 0.38f;
+            fMaskFilter->setSigma(f * kFeatherToSigma);
+        }
+    }
+
+    const sk_sp<sksg::PaintNode> fMaskPaint;
+    sk_sp<sksg::BlurImageFilter> fMaskFilter; // optional "feather"
+
+    VectorValue fFeather;
+    ScalarValue fOpacity = 100;
+};
+
 sk_sp<sksg::RenderNode> AttachMask(const skjson::ArrayValue* jmask,
                                    const AnimationBuilder* abuilder,
                                    sk_sp<sksg::RenderNode> childNode) {
     if (!jmask) return childNode;
 
     struct MaskRecord {
-        sk_sp<sksg::Path>            mask_path;  // for clipping and masking
-        sk_sp<sksg::Color>           mask_paint; // for masking
-        sk_sp<sksg::BlurImageFilter> mask_blur;  // for masking
-        sksg::Merge::Mode            merge_mode; // for clipping
+        sk_sp<sksg::Path>  mask_path;    // for clipping and masking
+        sk_sp<MaskAdapter> mask_adapter; // for masking
+        sksg::Merge::Mode  merge_mode;   // for clipping
     };
 
     SkSTArray<4, MaskRecord, true> mask_stack;
-
     bool has_effect = false;
-    auto blur_effect = sksg::BlurImageFilter::Make();
 
     for (const skjson::ObjectValue* m : *jmask) {
         if (!m) continue;
@@ -110,38 +153,21 @@
         mask_path->setFillType(inverted ? SkPathFillType::kInverseWinding
                                         : SkPathFillType::kWinding);
 
-        auto mask_paint = sksg::Color::Make(SK_ColorBLACK);
-        mask_paint->setAntiAlias(true);
-        // First mask in the stack initializes the mask buffer.
-        mask_paint->setBlendMode(mask_stack.empty() ? SkBlendMode::kSrc
-                                                    : mask_info->fBlendMode);
+        const auto blend_mode = mask_stack.empty() ? SkBlendMode::kSrc
+                                                   : mask_info->fBlendMode;
 
-        has_effect |= abuilder->bindProperty<ScalarValue>((*m)["o"],
-            [mask_paint](const ScalarValue& o) {
-                mask_paint->setOpacity(o * 0.01f);
-        }, 100.0f);
+        auto mask_adapter = sk_make_sp<MaskAdapter>(*m, *abuilder, blend_mode);
+        abuilder->attachDiscardableAdapter(mask_adapter);
 
-        static const VectorValue default_feather = { 0, 0 };
-        if (abuilder->bindProperty<VectorValue>((*m)["f"],
-            [blur_effect](const VectorValue& feather) {
-                // Close enough to AE.
-                static constexpr SkScalar kFeatherToSigma = 0.38f;
-                auto sX = feather.size() > 0 ? feather[0] * kFeatherToSigma : 0,
-                     sY = feather.size() > 1 ? feather[1] * kFeatherToSigma : 0;
-                blur_effect->setSigma({ sX, sY });
-            }, default_feather)) {
+        has_effect |= mask_adapter->hasEffect();
 
-            has_effect = true;
-            mask_stack.push_back({ mask_path,
-                                   mask_paint,
-                                   std::move(blur_effect),
-                                   mask_info->fMergeMode});
-            blur_effect = sksg::BlurImageFilter::Make();
-        } else {
-            mask_stack.push_back({mask_path, mask_paint, nullptr, mask_info->fMergeMode});
-        }
+
+        mask_stack.push_back({ std::move(mask_path),
+                               std::move(mask_adapter),
+                               mask_info->fMergeMode });
     }
 
+
     if (mask_stack.empty())
         return childNode;
 
@@ -167,22 +193,17 @@
         return sksg::ClipEffect::Make(std::move(childNode), std::move(clip_node), true);
     }
 
-    const auto make_mask = [](const MaskRecord& rec) {
-        auto mask = sksg::Draw::Make(std::move(rec.mask_path),
-                                     std::move(rec.mask_paint));
-        // Optional mask blur (feather).
-        return sksg::ImageFilterEffect::Make(std::move(mask), std::move(rec.mask_blur));
-    };
-
+    // Complex masks (non-opaque or blurred) turn into a mask node stack.
     sk_sp<sksg::RenderNode> maskNode;
     if (mask_stack.count() == 1) {
         // no group needed for single mask
-        maskNode = make_mask(mask_stack.front());
+        const auto rec = mask_stack.front();
+        maskNode = rec.mask_adapter->makeMask(std::move(rec.mask_path));
     } else {
         std::vector<sk_sp<sksg::RenderNode>> masks;
         masks.reserve(SkToSizeT(mask_stack.count()));
         for (auto& rec : mask_stack) {
-            masks.push_back(make_mask(rec));
+            masks.push_back(rec.mask_adapter->makeMask(std::move(rec.mask_path)));
         }
 
         maskNode = sksg::Group::Make(std::move(masks));
diff --git a/modules/skottie/src/Path.cpp b/modules/skottie/src/Path.cpp
new file mode 100644
index 0000000..c4a0fa4
--- /dev/null
+++ b/modules/skottie/src/Path.cpp
@@ -0,0 +1,49 @@
+/*
+ * Copyright 2020 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "modules/skottie/src/Adapter.h"
+#include "modules/skottie/src/SkottieJson.h"
+#include "modules/skottie/src/SkottiePriv.h"
+#include "modules/skottie/src/SkottieValue.h"
+#include "modules/sksg/include/SkSGPath.h"
+
+namespace skottie {
+namespace internal {
+
+namespace  {
+
+class PathAdapter final : public DiscardableAdapterBase<PathAdapter, sksg::Path> {
+public:
+    PathAdapter(const skjson::Value& jpath, const AnimationBuilder& abuilder)
+        : INHERITED(sksg::Path::Make()) {
+        this->bind(abuilder, jpath, fShape);
+    }
+
+private:
+    void onSync() override {
+        const auto& path_node = this->node();
+
+        auto path = ValueTraits<ShapeValue>::As<SkPath>(fShape);
+
+        // FillType is tracked in the SG node, not in keyframes -- make sure we preserve it.
+        path.setFillType(path_node->getFillType());
+        path_node->setPath(path);
+    }
+
+    ShapeValue fShape;
+
+    using INHERITED = DiscardableAdapterBase<PathAdapter, sksg::Path>;
+};
+
+} // namespace
+
+sk_sp<sksg::Path> AnimationBuilder::attachPath(const skjson::Value& jpath) const {
+    return this->attachDiscardableAdapter<PathAdapter, sk_sp<sksg::Path>>(jpath, *this);
+}
+
+} // namespace internal
+} // namespace skottie
diff --git a/modules/skottie/src/Skottie.cpp b/modules/skottie/src/Skottie.cpp
index 560ea0f..7505f8e 100644
--- a/modules/skottie/src/Skottie.cpp
+++ b/modules/skottie/src/Skottie.cpp
@@ -68,26 +68,27 @@
     fLogger->log(lvl, buff, jsonstr.c_str());
 }
 
-sk_sp<sksg::RenderNode> AnimationBuilder::attachOpacity(const skjson::ObjectValue& jtransform,
-                                                        sk_sp<sksg::RenderNode> childNode) const {
-    if (!childNode)
-        return nullptr;
-
-    auto opacityNode = sksg::OpacityEffect::Make(childNode);
-
-    const auto bound = this->bindProperty<ScalarValue>(jtransform["o"],
-        [opacityNode](const ScalarValue& o) {
-            // BM opacity is [0..100]
-            opacityNode->setOpacity(o * 0.01f);
-        }, 100.0f);
-    const auto dispatched = this->dispatchOpacityProperty(opacityNode);
-
-    // We can ignore constant full opacity.
-    return (bound || dispatched) ? std::move(opacityNode) : childNode;
-}
-
 namespace  {
 
+class OpacityAdapter final : public DiscardableAdapterBase<OpacityAdapter, sksg::OpacityEffect> {
+public:
+    OpacityAdapter(const skjson::ObjectValue& jobject,
+                   sk_sp<sksg::RenderNode> child,
+                   const AnimationBuilder& abuilder)
+        : INHERITED(sksg::OpacityEffect::Make(child)) {
+        this->bind(abuilder, jobject["o"], fOpacity);
+    }
+
+private:
+    void onSync() override {
+        this->node()->setOpacity(fOpacity * 0.01f);
+    }
+
+    ScalarValue fOpacity = 100;
+
+    using INHERITED = DiscardableAdapterBase<OpacityAdapter, sksg::OpacityEffect>;
+};
+
 static SkBlendMode GetBlendMode(const skjson::ObjectValue& jobject,
                                 const AnimationBuilder* abuilder) {
     static constexpr SkBlendMode kBlendModeMap[] = {
@@ -122,6 +123,27 @@
 
 } // namespace
 
+sk_sp<sksg::RenderNode> AnimationBuilder::attachOpacity(const skjson::ObjectValue& jobject,
+                                                        sk_sp<sksg::RenderNode> child_node) const {
+    if (!child_node)
+        return nullptr;
+
+    auto adapter = OpacityAdapter::Make(jobject, child_node, *this);
+    const auto dispatched = this->dispatchOpacityProperty(adapter->node());
+
+    if (adapter->isStatic()) {
+        adapter->tick(0);
+        if (!dispatched && adapter->node()->getOpacity() >= 1) {
+            // No obeservable effects - we can discard.
+            return child_node;
+        }
+    } else {
+        fCurrentAnimatorScope->push_back(adapter);
+    }
+
+    return adapter->node();
+}
+
 sk_sp<sksg::RenderNode> AnimationBuilder::attachBlendMode(const skjson::ObjectValue& jobject,
                                                           sk_sp<sksg::RenderNode> child) const {
     const auto bm = GetBlendMode(jobject, this);
@@ -133,19 +155,6 @@
     return child;
 }
 
-sk_sp<sksg::Path> AnimationBuilder::attachPath(const skjson::Value& jpath) const {
-    auto path_node = sksg::Path::Make();
-    return this->bindProperty<ShapeValue>(jpath,
-        [path_node](const ShapeValue& p) {
-            // FillType is tracked in the SG node, not in keyframes -- make sure we preserve it.
-            auto path = ValueTraits<ShapeValue>::As<SkPath>(p);
-            path.setFillType(path_node->getFillType());
-            path_node->setPath(path);
-        })
-        ? path_node
-        : nullptr;
-}
-
 AnimationBuilder::AnimationBuilder(sk_sp<ResourceProvider> rp, sk_sp<SkFontMgr> fontmgr,
                                    sk_sp<PropertyObserver> pobserver, sk_sp<Logger> logger,
                                    sk_sp<MarkerObserver> mobserver,
diff --git a/modules/skottie/src/SkottieAnimator.cpp b/modules/skottie/src/SkottieAnimator.cpp
deleted file mode 100644
index 7f691cd..0000000
--- a/modules/skottie/src/SkottieAnimator.cpp
+++ /dev/null
@@ -1,97 +0,0 @@
-/*
- * Copyright 2017 Google Inc.
- *
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- */
-
-#include "modules/skottie/src/Animator.h"
-#include "modules/skottie/src/SkottieJson.h"
-#include "modules/skottie/src/SkottiePriv.h"
-#include "modules/skottie/src/SkottieValue.h"
-#include "modules/skottie/src/text/TextValue.h"
-
-namespace skottie {
-namespace internal {
-
-namespace {
-
-template <typename T>
-class LegacyAnimatorAdapter final : public AnimatablePropertyContainer {
-public:
-    LegacyAnimatorAdapter(const AnimationBuilder& abuilder,
-                          const skjson::ObjectValue* jprop,
-                          std::function<void(const T&)>&& apply)
-        : fApplyFunc(std::move(apply)) {
-        if (!this->bind<T>(abuilder, jprop, &fValue)) {
-            fValue = T();
-        }
-    }
-
-    const T& value() const { return fValue; }
-
-private:
-    void onSync() override {
-        fApplyFunc(fValue);
-    }
-
-    const std::function<void(const T&)> fApplyFunc;
-    T                                   fValue;
-};
-
-template <typename T>
-bool BindLegacyPropertyImpl(const AnimationBuilder& abuilder,
-                            const skjson::ObjectValue* jprop,
-                            AnimatorScope* ascope,
-                            std::function<void(const T&)>&& apply,
-                            const T* noop) {
-    if (!jprop) {
-        return false;
-    }
-
-    auto adapter = sk_make_sp<LegacyAnimatorAdapter<T>>(abuilder, jprop, std::move(apply));
-
-    if (adapter->isStatic()) {
-        if (noop && *noop == adapter->value()) {
-            return false;
-        }
-        adapter->tick(0);
-    } else {
-        ascope->push_back(std::move(adapter));
-    }
-
-    return true;
-}
-
-} // namespace
-
-template <>
-bool AnimationBuilder::bindProperty(const skjson::Value& jv,
-                  std::function<void(const ScalarValue&)>&& apply,
-                  const ScalarValue* noop) const {
-    return BindLegacyPropertyImpl(*this, jv, fCurrentAnimatorScope, std::move(apply), noop);
-}
-
-template <>
-bool AnimationBuilder::bindProperty(const skjson::Value& jv,
-                  std::function<void(const VectorValue&)>&& apply,
-                  const VectorValue* noop) const {
-    return BindLegacyPropertyImpl(*this, jv, fCurrentAnimatorScope, std::move(apply), noop);
-}
-
-template <>
-bool AnimationBuilder::bindProperty(const skjson::Value& jv,
-                  std::function<void(const ShapeValue&)>&& apply,
-                  const ShapeValue* noop) const {
-    return BindLegacyPropertyImpl(*this, jv, fCurrentAnimatorScope, std::move(apply), noop);
-}
-
-template <>
-bool AnimationBuilder::bindProperty(const skjson::Value& jv,
-                  std::function<void(const TextValue&)>&& apply,
-                  const TextValue* noop) const {
-    return BindLegacyPropertyImpl(*this, jv, fCurrentAnimatorScope, std::move(apply), noop);
-}
-
-} // namespace internal
-} // namespace skottie
diff --git a/modules/skottie/src/SkottiePriv.h b/modules/skottie/src/SkottiePriv.h
index 153499f..e764e70 100644
--- a/modules/skottie/src/SkottiePriv.h
+++ b/modules/skottie/src/SkottiePriv.h
@@ -63,19 +63,6 @@
     };
     const FontInfo* findFont(const SkString& name) const;
 
-    // DEPRECATED/TO-BE-REMOVED: use AnimatablePropertyContainer::bind<> instead.
-    template <typename T>
-    bool bindProperty(const skjson::Value&,
-                      std::function<void(const T&)>&&,
-                      const T* default_igore = nullptr) const;
-
-    template <typename T>
-    bool bindProperty(const skjson::Value& jv,
-                      std::function<void(const T&)>&& apply,
-                      const T& default_ignore) const {
-        return this->bindProperty(jv, std::move(apply), &default_ignore);
-    }
-
     void log(Logger::Level, const skjson::Value*, const char fmt[], ...) const;
 
     sk_sp<sksg::Transform> attachMatrix2D(const skjson::ObjectValue&, sk_sp<sksg::Transform>) const;
@@ -118,16 +105,22 @@
         AnimatorScope*          fPrevScope;
     };
 
+    template <typename T>
+    void attachDiscardableAdapter(sk_sp<T> adapter) const {
+        if (adapter->isStatic()) {
+            // Fire off a synthetic tick to force a single SG sync before discarding.
+            adapter->tick(0);
+        } else {
+            fCurrentAnimatorScope->push_back(std::move(adapter));
+        }
+    }
+
     template <typename T,  typename NodeType = sk_sp<sksg::RenderNode>, typename... Args>
     NodeType attachDiscardableAdapter(Args&&... args) const {
         if (auto adapter = T::Make(std::forward<Args>(args)...)) {
             auto node = adapter->node();
-            if (adapter->isStatic()) {
-                // Fire off a synthetic tick to force a single SG sync before discarding.
-                adapter->tick(0);
-            } else {
-                fCurrentAnimatorScope->push_back(std::move(adapter));
-            }
+            this->attachDiscardableAdapter(std::move(adapter));
+
             return node;
         }
 
diff --git a/modules/skottie/src/layers/PrecompLayer.cpp b/modules/skottie/src/layers/PrecompLayer.cpp
index cf936a0..0afe8ee 100644
--- a/modules/skottie/src/layers/PrecompLayer.cpp
+++ b/modules/skottie/src/layers/PrecompLayer.cpp
@@ -7,6 +7,7 @@
 
 #include "modules/skottie/src/SkottiePriv.h"
 
+#include "modules/skottie/src/Animator.h"
 #include "modules/skottie/src/Composition.h"
 #include "modules/skottie/src/SkottieJson.h"
 #include "modules/skottie/src/SkottieValue.h"
@@ -18,15 +19,73 @@
 namespace skottie {
 namespace internal {
 
+namespace  {
+
+// "Animates" time based on the layer's "tm" property.
+class TimeRemapper final : public AnimatablePropertyContainer {
+public:
+    TimeRemapper(const skjson::ObjectValue& jtm, const AnimationBuilder* abuilder, float scale)
+        : fScale(scale) {
+        this->bind(*abuilder, jtm, fT);
+    }
+
+    float t() const { return fT * fScale; }
+
+private:
+    void onSync() override {
+        // nothing to sync - we just track t
+    }
+
+    const float fScale;
+
+    ScalarValue fT = 0;
+};
+
+// Applies a bias/scale/remap t-adjustment to child animators.
+class CompTimeMapper final : public sksg::GroupAnimator {
+public:
+    CompTimeMapper(sksg::AnimatorList&& layer_animators,
+                   sk_sp<TimeRemapper> remapper,
+                   float time_bias, float time_scale)
+        : INHERITED(std::move(layer_animators))
+        , fRemapper(std::move(remapper))
+        , fTimeBias(time_bias)
+        , fTimeScale(time_scale) {}
+
+    void onTick(float t) override {
+        if (fRemapper) {
+            // When time remapping is active, |t| is fully driven externally.
+            fRemapper->tick(t);
+            t = fRemapper->t();
+        } else {
+            t = (t + fTimeBias) * fTimeScale;
+        }
+
+        this->INHERITED::onTick(t);
+    }
+
+private:
+    const sk_sp<TimeRemapper> fRemapper;
+    const float               fTimeBias,
+                              fTimeScale;
+
+    using INHERITED = sksg::GroupAnimator;
+};
+
+} // namespace
+
 sk_sp<sksg::RenderNode> AnimationBuilder::attachPrecompLayer(const skjson::ObjectValue& jlayer,
                                                              LayerInfo* layer_info) const {
-    const skjson::ObjectValue* time_remap = jlayer["tm"];
-    // Empirically, a time mapper supersedes start/stretch.
-    const auto start_time = time_remap ? 0.0f : ParseDefault<float>(jlayer["st"], 0.0f),
-             stretch_time = time_remap ? 1.0f : ParseDefault<float>(jlayer["sr"], 1.0f);
+    sk_sp<TimeRemapper> time_remapper;
+    if (const skjson::ObjectValue* jtm = jlayer["tm"]) {
+        time_remapper = sk_make_sp<TimeRemapper>(*jtm, this, fFrameRate);
+    }
+
+    const auto start_time = ParseDefault<float>(jlayer["st"], 0.0f),
+             stretch_time = ParseDefault<float>(jlayer["sr"], 1.0f);
     const auto requires_time_mapping = !SkScalarNearlyEqual(start_time  , 0) ||
                                        !SkScalarNearlyEqual(stretch_time, 1) ||
-                                       time_remap;
+                                       time_remapper;
 
     // Precomp layers are sized explicitly.
     layer_info->fSize = SkSize::Make(ParseDefault<float>(jlayer["w"], 0.0f),
@@ -42,45 +101,14 @@
             return CompositionBuilder(*this, layer_info->fSize, jcomp).build(*this);
         });
 
-    // Applies a bias/scale/remap t-adjustment to child animators.
-    class CompTimeMapper final : public sksg::GroupAnimator {
-    public:
-        CompTimeMapper(sksg::AnimatorList&& layer_animators, float time_bias, float time_scale)
-            : INHERITED(std::move(layer_animators))
-            , fTimeBias(time_bias)
-            , fTimeScale(time_scale) {}
-
-        void onTick(float t) override {
-            // When time remapping is active, |t| is driven externally.
-            if (fRemappedTime.isValid()) {
-                t = *fRemappedTime.get();
-            }
-
-            this->INHERITED::onTick((t + fTimeBias) * fTimeScale);
-        }
-
-        void remapTime(float t) { fRemappedTime.set(t); }
-
-    private:
-        const float    fTimeBias,
-                       fTimeScale;
-        SkTLazy<float> fRemappedTime;
-
-        using INHERITED = sksg::GroupAnimator;
-    };
-
     if (requires_time_mapping) {
         const auto t_bias  = -start_time,
                    t_scale = sk_ieee_float_divide(1, stretch_time);
-        auto time_mapper = sk_make_sp<CompTimeMapper>(local_scope->release(), t_bias,
+        auto time_mapper = sk_make_sp<CompTimeMapper>(local_scope->release(),
+                                                      std::move(time_remapper),
+                                                      t_bias,
                                                       sk_float_isfinite(t_scale) ? t_scale : 0);
-        if (time_remap) {
-            auto  frame_rate = fFrameRate;
-            this->bindProperty<ScalarValue>(*time_remap,
-                    [time_mapper, frame_rate](const ScalarValue& t) {
-                        time_mapper->remapTime(t * frame_rate);
-                    });
-        }
+
         fCurrentAnimatorScope->push_back(std::move(time_mapper));
     }