From 20880784fb7301d497225e7e7e4a3cd1452b8aae Mon Sep 17 00:00:00 2001 From: Florin Malita Date: Wed, 9 May 2018 11:35:00 -0400 Subject: [skottie] Json cleanup pass Assorted Json tweaks: - more defensive internal object access - drop unneeded isObject checks - drop unneeded namespace - restrict the iterator to arrays TBR= Change-Id: I02f1c5d84c429cf5206bc2a0a7843097b92bac94 Reviewed-on: https://skia-review.googlesource.com/126930 Reviewed-by: Florin Malita Commit-Queue: Florin Malita --- experimental/skottie/Skottie.cpp | 18 ++++----- experimental/skottie/SkottieAnimator.cpp | 3 -- experimental/skottie/SkottieJson.cpp | 66 +++++++++++++------------------- 3 files changed, 33 insertions(+), 54 deletions(-) (limited to 'experimental') diff --git a/experimental/skottie/Skottie.cpp b/experimental/skottie/Skottie.cpp index 99549bfccd..0ac1b4ed71 100644 --- a/experimental/skottie/Skottie.cpp +++ b/experimental/skottie/Skottie.cpp @@ -63,7 +63,7 @@ struct AttachContext { bool LogFail(const json::ValueRef& json, const char* msg) { const auto dump = json.toString(); - LOG("!! %s: %s", msg, dump.c_str()); + LOG("!! %s: %s\n", msg, dump.c_str()); return false; } @@ -126,9 +126,8 @@ sk_sp AttachOpacity(const json::ValueRef& jtransform, AttachCo // This is more peeky than other attachers, because we want to avoid redundant opacity // nodes for the extremely common case of static opaciy == 100. - const auto& opacity = jtransform["o"]; - if (opacity.isObject() && - !opacity["a"].toDefault(true) && + const auto opacity = jtransform["o"]; + if (!opacity["a"].toDefault(true) && opacity["k"].toDefault(-1) == 100) { // Ignoring static full opacity. return childNode; @@ -276,7 +275,7 @@ sk_sp AttachColor(const json::ValueRef& obj, AttachContext* ctx) { sk_sp AttachGradient(const json::ValueRef& obj, AttachContext* ctx) { SkASSERT(obj.isObject()); - const auto& stops = obj["g"]; + const auto stops = obj["g"]; if (!stops.isObject()) return nullptr; @@ -528,11 +527,8 @@ const ShapeInfo* FindShapeInfo(const json::ValueRef& shape) { { "tr", ShapeType::kTransform , 0 }, // transform -> Inline handler }; - if (!shape.isObject()) - return nullptr; - - const auto type = shape["ty"].toDefault(SkString()); - if (type.isEmpty()) + SkString type; + if (!shape["ty"].to(&type) || type.isEmpty()) return nullptr; const auto* info = bsearch(type.c_str(), @@ -594,7 +590,7 @@ sk_sp AttachShape(const json::ValueRef& jshape, AttachShapeCon const auto s = jshape[jshape.size() - 1 - i]; const auto* info = FindShapeInfo(s); if (!info) { - LogFail(s.isObject() ? s["ty"] : s, "Unknown shape"); + LogFail(s["ty"], "Unknown shape"); continue; } diff --git a/experimental/skottie/SkottieAnimator.cpp b/experimental/skottie/SkottieAnimator.cpp index d8cc6e7769..4554409761 100644 --- a/experimental/skottie/SkottieAnimator.cpp +++ b/experimental/skottie/SkottieAnimator.cpp @@ -74,9 +74,6 @@ protected: return; for (const json::ValueRef jframe : jframes) { - if (!jframe.isObject()) - continue; - float t0; if (!jframe["t"].to(&t0)) continue; diff --git a/experimental/skottie/SkottieJson.cpp b/experimental/skottie/SkottieJson.cpp index 5d00f19f2b..23e616d1ea 100644 --- a/experimental/skottie/SkottieJson.cpp +++ b/experimental/skottie/SkottieJson.cpp @@ -26,12 +26,12 @@ namespace skottie { namespace json { template <> -bool json::ValueRef::to(SkScalar* v) const { +bool ValueRef::to(SkScalar* v) const { if (!fValue) return false; // Some versions wrap values as single-element arrays. if (fValue->IsArray() && fValue->Size() == 1) { - return json::ValueRef(fValue->operator[](0)).to(v); + return ValueRef(fValue->operator[](0)).to(v); } if (!fValue->IsNumber()) @@ -43,7 +43,7 @@ bool json::ValueRef::to(SkScalar* v) const { } template <> -bool json::ValueRef::to(bool* v) const { +bool ValueRef::to(bool* v) const { if (!fValue) return false; switch(fValue->GetType()) { @@ -62,7 +62,7 @@ bool json::ValueRef::to(bool* v) const { } template <> -bool json::ValueRef::to(int* v) const { +bool ValueRef::to(int* v) const { if (!fValue || !fValue->IsInt()) return false; @@ -72,7 +72,7 @@ bool json::ValueRef::to(int* v) const { } template <> -bool json::ValueRef::to(SkString* v) const { +bool ValueRef::to(SkString* v) const { if (!fValue || !fValue->IsString()) return false; @@ -82,20 +82,20 @@ bool json::ValueRef::to(SkString* v) const { } template <> -bool json::ValueRef::to(SkPoint* v) const { +bool ValueRef::to(SkPoint* v) const { if (!fValue || !fValue->IsObject()) return false; - const auto jvx = ValueRef(fValue->operator[]("x")), - jvy = ValueRef(fValue->operator[]("y")); + const auto jvx = ValueRef(this->operator[]("x")), + jvy = ValueRef(this->operator[]("y")); - // Some BM versions seem to store x/y as single-element arrays. - return json::ValueRef(jvx.isArray() ? jvx.operator[](size_t(0)) : jvx).to(&v->fX) - && json::ValueRef(jvy.isArray() ? jvy.operator[](size_t(0)) : jvy).to(&v->fY); + // Some BM versions seem to store x/y as single-element arrays. + return ValueRef(jvx.isArray() ? jvx.operator[](size_t(0)) : jvx).to(&v->fX) + && ValueRef(jvy.isArray() ? jvy.operator[](size_t(0)) : jvy).to(&v->fY); } template <> -bool json::ValueRef::to>(std::vector* v) const { +bool ValueRef::to>(std::vector* v) const { if (!fValue || !fValue->IsArray()) return false; @@ -111,16 +111,16 @@ bool json::ValueRef::to>(std::vector* v) const { namespace { -bool ParsePointVec(const rapidjson::Value& jv, std::vector* pts) { - if (!jv.IsArray()) +bool ParsePointVec(const ValueRef& jv, std::vector* pts) { + if (!jv.isArray()) return false; pts->clear(); - pts->reserve(jv.Size()); + pts->reserve(jv.size()); std::vector vec; - for (size_t i = 0; i < jv.Size(); ++i) { - if (!ValueRef(jv[i]).to(&vec) || vec.size() != 2) + for (size_t i = 0; i < jv.size(); ++i) { + if (!jv[i].to(&vec) || vec.size() != 2) return false; pts->push_back(SkPoint::Make(vec[0], vec[1])); } @@ -131,7 +131,7 @@ bool ParsePointVec(const rapidjson::Value& jv, std::vector* pts) { } // namespace template <> -bool json::ValueRef::to(ShapeValue* v) const { +bool ValueRef::to(ShapeValue* v) const { SkASSERT(v->fVertices.empty()); if (!fValue) @@ -139,7 +139,7 @@ bool json::ValueRef::to(ShapeValue* v) const { // Some versions wrap values as single-element arrays. if (fValue->IsArray() && fValue->Size() == 1) { - return json::ValueRef(fValue->operator[](0)).to(v); + return ValueRef(fValue->operator[](0)).to(v); } std::vector inPts, // Cubic Bezier "in" control points, relative to vertices. @@ -147,9 +147,9 @@ bool json::ValueRef::to(ShapeValue* v) const { verts; // Cubic Bezier vertices. if (!fValue->IsObject() || - !ParsePointVec(fValue->operator[]("i"), &inPts) || - !ParsePointVec(fValue->operator[]("o"), &outPts) || - !ParsePointVec(fValue->operator[]("v"), &verts) || + !ParsePointVec(this->operator[]("i"), &inPts) || + !ParsePointVec(this->operator[]("o"), &outPts) || + !ParsePointVec(this->operator[]("v"), &verts) || inPts.size() != outPts.size() || inPts.size() != verts.size()) { @@ -160,7 +160,7 @@ bool json::ValueRef::to(ShapeValue* v) const { for (size_t i = 0; i < inPts.size(); ++i) { v->fVertices.push_back(BezierVertex({inPts[i], outPts[i], verts[i]})); } - v->fClosed = json::ValueRef(fValue->operator[]("c")).toDefault(false); + v->fClosed = this->operator[]("c").toDefault(false); return true; } @@ -182,28 +182,14 @@ ValueRef ValueRef::operator[](const char* key) const { } const rapidjson::Value* ValueRef::begin() const { - if (this->isArray()) { - return fValue->Begin(); - } - if (this->isObject()) { - return &fValue->MemberBegin()->value; - } - - return nullptr; + return this->isArray() ? fValue->Begin() : nullptr; } const rapidjson::Value* ValueRef::end() const { - if (this->isArray()) { - return fValue->End(); - } - if (this->isObject()) { - return &fValue->MemberEnd()->value; - } - - return nullptr; + return this->isArray() ? fValue->End() : nullptr; } -SkString json::ValueRef::toString() const { +SkString ValueRef::toString() const { #ifdef SK_DEBUG rapidjson::StringBuffer buf; if (fValue) { -- cgit v1.2.3