[skottie] Cleanup KeyframeAnimatorBuilder
Instead of plumbing the target value through bindImpl as an opaque
void*, store explicitly in builders.
More typesafe/elegant/flexible/etc.
TBR=
Change-Id: Ie28787072a6be3b0bfcd528b68431f9fb3fa3a71
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/291576
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Florin Malita <fmalita@chromium.org>
diff --git a/modules/skottie/src/animator/Animator.cpp b/modules/skottie/src/animator/Animator.cpp
index 231ff51..3e89147 100644
--- a/modules/skottie/src/animator/Animator.cpp
+++ b/modules/skottie/src/animator/Animator.cpp
@@ -49,8 +49,7 @@
bool AnimatablePropertyContainer::bindImpl(const AnimationBuilder& abuilder,
const skjson::ObjectValue* jprop,
- KeyframeAnimatorBuilder& builder,
- void* target_value) {
+ KeyframeAnimatorBuilder& builder) {
if (!jprop) {
return false;
}
@@ -65,7 +64,7 @@
// Older Json versions don't have an "a" animation marker.
// For those, we attempt to parse both ways.
if (!ParseDefault<bool>(jpropA, false)) {
- if (builder.parseValue(abuilder, jpropK, target_value)) {
+ if (builder.parseValue(abuilder, jpropK)) {
// Static property.
return true;
}
@@ -81,7 +80,7 @@
sk_sp<KeyframeAnimator> animator;
const skjson::ArrayValue* jkfs = jpropK;
if (jkfs && jkfs->size() > 0) {
- animator = builder.make(abuilder, *jkfs, target_value);
+ animator = builder.make(abuilder, *jkfs);
}
if (!animator) {
diff --git a/modules/skottie/src/animator/Animator.h b/modules/skottie/src/animator/Animator.h
index 2c1918b..42c13d6 100644
--- a/modules/skottie/src/animator/Animator.h
+++ b/modules/skottie/src/animator/Animator.h
@@ -66,10 +66,7 @@
private:
StateChanged onSeek(float) final;
- bool bindImpl(const AnimationBuilder&,
- const skjson::ObjectValue*,
- KeyframeAnimatorBuilder&,
- void*);
+ bool bindImpl(const AnimationBuilder&, const skjson::ObjectValue*, KeyframeAnimatorBuilder&);
std::vector<sk_sp<Animator>> fAnimators;
bool fHasSynced = false;
diff --git a/modules/skottie/src/animator/KeyframeAnimator.h b/modules/skottie/src/animator/KeyframeAnimator.h
index 35eee98..d9955e4 100644
--- a/modules/skottie/src/animator/KeyframeAnimator.h
+++ b/modules/skottie/src/animator/KeyframeAnimator.h
@@ -108,11 +108,9 @@
public:
virtual ~KeyframeAnimatorBuilder();
- virtual sk_sp<KeyframeAnimator> make(const AnimationBuilder&,
- const skjson::ArrayValue&,
- void* target_value) = 0;
+ virtual sk_sp<KeyframeAnimator> make(const AnimationBuilder&, const skjson::ArrayValue&) = 0;
- virtual bool parseValue(const AnimationBuilder&, const skjson::Value&, void*) const = 0;
+ virtual bool parseValue(const AnimationBuilder&, const skjson::Value&) const = 0;
protected:
virtual bool parseKFValue(const AnimationBuilder&,
diff --git a/modules/skottie/src/animator/ScalarKeyframeAnimator.cpp b/modules/skottie/src/animator/ScalarKeyframeAnimator.cpp
index e9e6bd5..d0babc5 100644
--- a/modules/skottie/src/animator/ScalarKeyframeAnimator.cpp
+++ b/modules/skottie/src/animator/ScalarKeyframeAnimator.cpp
@@ -19,22 +19,21 @@
public:
class Builder final : public KeyframeAnimatorBuilder {
public:
+ explicit Builder(ScalarValue* target) : fTarget(target) {}
+
sk_sp<KeyframeAnimator> make(const AnimationBuilder& abuilder,
- const skjson::ArrayValue& jkfs,
- void* target_value) override {
+ const skjson::ArrayValue& jkfs) override {
SkASSERT(jkfs.size() > 0);
if (!this->parseKeyframes(abuilder, jkfs)) {
return nullptr;
}
return sk_sp<ScalarKeyframeAnimator>(
- new ScalarKeyframeAnimator(std::move(fKFs),
- std::move(fCMs),
- static_cast<ScalarValue*>(target_value)));
+ new ScalarKeyframeAnimator(std::move(fKFs), std::move(fCMs), fTarget));
}
- bool parseValue(const AnimationBuilder&, const skjson::Value& jv, void* v) const override {
- return Parse(jv, static_cast<float*>(v));
+ bool parseValue(const AnimationBuilder&, const skjson::Value& jv) const override {
+ return Parse(jv, fTarget);
}
private:
@@ -44,6 +43,8 @@
Keyframe::Value* v) override {
return Parse(jv, &v->flt);
}
+
+ ScalarValue* fTarget;
};
private:
@@ -73,9 +74,9 @@
bool AnimatablePropertyContainer::bind<ScalarValue>(const AnimationBuilder& abuilder,
const skjson::ObjectValue* jprop,
ScalarValue* v) {
- ScalarKeyframeAnimator::Builder builder;
+ ScalarKeyframeAnimator::Builder builder(v);
- return this->bindImpl(abuilder, jprop, builder, v);
+ return this->bindImpl(abuilder, jprop, builder);
}
} // namespace skottie::internal
diff --git a/modules/skottie/src/animator/ShapeKeyframeAnimator.cpp b/modules/skottie/src/animator/ShapeKeyframeAnimator.cpp
index b5f6fee..b53dc1b 100644
--- a/modules/skottie/src/animator/ShapeKeyframeAnimator.cpp
+++ b/modules/skottie/src/animator/ShapeKeyframeAnimator.cpp
@@ -169,9 +169,9 @@
bool AnimatablePropertyContainer::bind<ShapeValue>(const AnimationBuilder& abuilder,
const skjson::ObjectValue* jprop,
ShapeValue* v) {
- VectorKeyframeAnimatorBuilder builder(parse_encoding_len, parse_encoding_data);
+ VectorKeyframeAnimatorBuilder builder(v, parse_encoding_len, parse_encoding_data);
- return this->bindImpl(abuilder, jprop, builder, v);
+ return this->bindImpl(abuilder, jprop, builder);
}
} // namespace internal
diff --git a/modules/skottie/src/animator/TextKeyframeAnimator.cpp b/modules/skottie/src/animator/TextKeyframeAnimator.cpp
index e1f8d14..8fb1ad1 100644
--- a/modules/skottie/src/animator/TextKeyframeAnimator.cpp
+++ b/modules/skottie/src/animator/TextKeyframeAnimator.cpp
@@ -18,9 +18,10 @@
public:
class Builder final : public KeyframeAnimatorBuilder {
public:
+ explicit Builder(TextValue* target) : fTarget(target) {}
+
sk_sp<KeyframeAnimator> make(const AnimationBuilder& abuilder,
- const skjson::ArrayValue& jkfs,
- void* target_value) override {
+ const skjson::ArrayValue& jkfs) override {
SkASSERT(jkfs.size() > 0);
fValues.reserve(jkfs.size());
@@ -33,12 +34,11 @@
new TextKeyframeAnimator(std::move(fKFs),
std::move(fCMs),
std::move(fValues),
- static_cast<TextValue*>(target_value)));
+ fTarget));
}
- bool parseValue(const AnimationBuilder& abuilder,
- const skjson::Value& jv, void* v) const override {
- return Parse(jv, abuilder, static_cast<TextValue*>(v));
+ bool parseValue(const AnimationBuilder& abuilder, const skjson::Value& jv) const override {
+ return Parse(jv, abuilder, fTarget);
}
private:
@@ -62,6 +62,7 @@
}
std::vector<TextValue> fValues;
+ TextValue* fTarget;
};
private:
@@ -95,8 +96,8 @@
bool AnimatablePropertyContainer::bind<TextValue>(const AnimationBuilder& abuilder,
const skjson::ObjectValue* jprop,
TextValue* v) {
- TextKeyframeAnimator::Builder builder;
- return this->bindImpl(abuilder, jprop, builder, v);
+ TextKeyframeAnimator::Builder builder(v);
+ return this->bindImpl(abuilder, jprop, builder);
}
} // namespace skottie::internal
diff --git a/modules/skottie/src/animator/Vec2KeyframeAnimator.cpp b/modules/skottie/src/animator/Vec2KeyframeAnimator.cpp
index 6e531cd..42a3de3 100644
--- a/modules/skottie/src/animator/Vec2KeyframeAnimator.cpp
+++ b/modules/skottie/src/animator/Vec2KeyframeAnimator.cpp
@@ -25,9 +25,10 @@
public:
class Builder final : public KeyframeAnimatorBuilder {
public:
+ explicit Builder(Vec2Value* target) : fTarget(target) {}
+
sk_sp<KeyframeAnimator> make(const AnimationBuilder& abuilder,
- const skjson::ArrayValue& jkfs,
- void* target_value) override {
+ const skjson::ArrayValue& jkfs) override {
SkASSERT(jkfs.size() > 0);
fValues.reserve(jkfs.size());
@@ -40,11 +41,11 @@
new Vec2KeyframeAnimator(std::move(fKFs),
std::move(fCMs),
std::move(fValues),
- static_cast<Vec2Value*>(target_value)));
+ fTarget));
}
- bool parseValue(const AnimationBuilder&, const skjson::Value& jv, void* v) const override {
- return Parse(jv, static_cast<Vec2Value*>(v));
+ bool parseValue(const AnimationBuilder&, const skjson::Value& jv) const override {
+ return Parse(jv, fTarget);
}
private:
@@ -120,6 +121,7 @@
}
std::vector<SpatialValue> fValues;
+ Vec2Value* fTarget;
SkV2 fTi{0,0},
fTo{0,0};
};
@@ -173,8 +175,8 @@
if (!ParseDefault<bool>((*jprop)["s"], false)) {
// Regular (static or keyframed) 2D value.
- Vec2KeyframeAnimator::Builder builder;
- return this->bindImpl(abuilder, jprop, builder, v);
+ Vec2KeyframeAnimator::Builder builder(v);
+ return this->bindImpl(abuilder, jprop, builder);
}
// Separate-dimensions vector value: each component is animated independently.
diff --git a/modules/skottie/src/animator/VectorKeyframeAnimator.cpp b/modules/skottie/src/animator/VectorKeyframeAnimator.cpp
index 23a4826..5d3f09c 100644
--- a/modules/skottie/src/animator/VectorKeyframeAnimator.cpp
+++ b/modules/skottie/src/animator/VectorKeyframeAnimator.cpp
@@ -84,7 +84,7 @@
std::vector<SkCubicMap> cms,
std::vector<float> storage,
size_t vec_len,
- VectorValue* target_value)
+ std::vector<float>* target_value)
: INHERITED(std::move(kfs), std::move(cms))
, fStorage(std::move(storage))
, fVecLen(vec_len)
@@ -143,21 +143,22 @@
const std::vector<float> fStorage;
const size_t fVecLen;
- VectorValue* fTarget;
+ std::vector<float>* fTarget;
using INHERITED = KeyframeAnimator;
};
} // namespace
-VectorKeyframeAnimatorBuilder::VectorKeyframeAnimatorBuilder(VectorLenParser parse_len,
+VectorKeyframeAnimatorBuilder::VectorKeyframeAnimatorBuilder(std::vector<float>* target,
+ VectorLenParser parse_len,
VectorDataParser parse_data)
: fParseLen(parse_len)
- , fParseData(parse_data) {}
+ , fParseData(parse_data)
+ , fTarget(target) {}
sk_sp<KeyframeAnimator> VectorKeyframeAnimatorBuilder::make(const AnimationBuilder& abuilder,
- const skjson::ArrayValue& jkfs,
- void* target_value) {
+ const skjson::ArrayValue& jkfs) {
SkASSERT(jkfs.size() > 0);
// peek at the first keyframe value to find our vector length
@@ -190,20 +191,18 @@
std::move(fCMs),
std::move(fStorage),
fVecLen,
- static_cast<VectorValue*>(target_value)));
+ fTarget));
}
bool VectorKeyframeAnimatorBuilder::parseValue(const AnimationBuilder&,
- const skjson::Value& jv,
- void* raw_v) const {
+ const skjson::Value& jv) const {
size_t vec_len;
if (!this->fParseLen(jv, &vec_len)) {
return false;
}
- auto* v = static_cast<VectorValue*>(raw_v);
- v->resize(vec_len);
- return fParseData(jv, vec_len, v->data());
+ fTarget->resize(vec_len);
+ return fParseData(jv, vec_len, fTarget->data());
}
bool VectorKeyframeAnimatorBuilder::parseKFValue(const AnimationBuilder&,
@@ -246,6 +245,7 @@
if (!ParseDefault<bool>((*jprop)["s"], false)) {
// Regular (static or keyframed) vector value.
VectorKeyframeAnimatorBuilder builder(
+ v,
// Len parser.
[](const skjson::Value& jv, size_t* len) -> bool {
if (const skjson::ArrayValue* ja = jv) {
@@ -259,7 +259,7 @@
return parse_array(jv, data, len);
});
- return this->bindImpl(abuilder, jprop, builder, v);
+ return this->bindImpl(abuilder, jprop, builder);
}
// Separate-dimensions vector value: each component is animated independently.
diff --git a/modules/skottie/src/animator/VectorKeyframeAnimator.h b/modules/skottie/src/animator/VectorKeyframeAnimator.h
index b23ac46..5b8c209 100644
--- a/modules/skottie/src/animator/VectorKeyframeAnimator.h
+++ b/modules/skottie/src/animator/VectorKeyframeAnimator.h
@@ -19,16 +19,12 @@
using VectorLenParser = bool(*)(const skjson::Value&, size_t*);
using VectorDataParser = bool(*)(const skjson::Value&, size_t, float*);
- VectorKeyframeAnimatorBuilder(VectorLenParser, VectorDataParser);
+ VectorKeyframeAnimatorBuilder(std::vector<float>*, VectorLenParser, VectorDataParser);
- sk_sp<KeyframeAnimator> make(const AnimationBuilder&,
- const skjson::ArrayValue&,
- void*) override;
+ sk_sp<KeyframeAnimator> make(const AnimationBuilder&, const skjson::ArrayValue&) override;
private:
- bool parseValue(const AnimationBuilder&,
- const skjson::Value&,
- void*) const override;
+ bool parseValue(const AnimationBuilder&, const skjson::Value&) const override;
bool parseKFValue(const AnimationBuilder&,
const skjson::ObjectValue&,
@@ -42,6 +38,7 @@
size_t fVecLen, // size of individual vector values we store
fCurrentVec = 0; // vector value index being parsed (corresponding
// storage offset is fCurrentVec * fVecLen)
+ std::vector<float>* fTarget;
};
} // namespace skottie::internal
