[skottie] Ref-counted ResourceProvider
Update the ResourceProvider interface to inherit from SkRefCnt, to
clarify sharing/ownership semantics in the Skottie Builder API. Now it
follows the same pattern as SkFontMgr.
Change-Id: I7ff8ad39023d9ecfe609e0180b5aabf776672d48
Reviewed-on: https://skia-review.googlesource.com/148991
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
diff --git a/modules/skottie/include/Skottie.h b/modules/skottie/include/Skottie.h
index 95d50c0..e82ba70 100644
--- a/modules/skottie/include/Skottie.h
+++ b/modules/skottie/include/Skottie.h
@@ -26,7 +26,7 @@
namespace skottie {
-class SK_API ResourceProvider {
+class SK_API ResourceProvider : public SkRefCnt {
public:
ResourceProvider() = default;
virtual ~ResourceProvider() = default;
@@ -57,10 +57,9 @@
const Stats& getStats() const { return fStats; }
/**
- * Specify a loader for external resources (images, etc.). Ownership stays with the caller
- * (the ResrouceProvider must be valid for this builder's lifespan).
+ * Specify a loader for external resources (images, etc.).
*/
- Builder& setResourceProvider(const ResourceProvider*);
+ Builder& setResourceProvider(sk_sp<ResourceProvider>);
/**
* Specify a font manager for loading animation fonts.
@@ -75,7 +74,7 @@
sk_sp<Animation> makeFromFile(const char path[]);
private:
- const ResourceProvider* fResourceProvider = nullptr;
+ sk_sp<ResourceProvider> fResourceProvider;
sk_sp<SkFontMgr> fFontMgr;
Stats fStats;
};
diff --git a/modules/skottie/src/Skottie.cpp b/modules/skottie/src/Skottie.cpp
index 75b0c4e..40e94ec 100644
--- a/modules/skottie/src/Skottie.cpp
+++ b/modules/skottie/src/Skottie.cpp
@@ -129,9 +129,10 @@
return color_node;
}
-AnimationBuilder::AnimationBuilder(const ResourceProvider& rp, sk_sp<SkFontMgr> fontmgr,
- Animation::Builder::Stats* stats, float duration, float framerate)
- : fResourceProvider(rp)
+AnimationBuilder::AnimationBuilder(sk_sp<ResourceProvider> rp, sk_sp<SkFontMgr> fontmgr,
+ Animation::Builder::Stats* stats,
+ float duration, float framerate)
+ : fResourceProvider(std::move(rp))
, fFontMgr(std::move(fontmgr))
, fStats(stats)
, fDuration(duration)
@@ -163,8 +164,8 @@
} // namespace internal
-Animation::Builder& Animation::Builder::setResourceProvider(const ResourceProvider* rp) {
- fResourceProvider = rp;
+Animation::Builder& Animation::Builder::setResourceProvider(sk_sp<ResourceProvider> rp) {
+ fResourceProvider = std::move(rp);
return *this;
}
@@ -194,10 +195,10 @@
class NullResourceProvider final : public ResourceProvider {
sk_sp<SkData> load(const char[], const char[]) const override { return nullptr; }
};
-
- const NullResourceProvider nullProvider;
- auto resolvedProvider = fResourceProvider ? fResourceProvider : &nullProvider;
- auto resolvedFontMgr = fFontMgr ? fFontMgr : SkFontMgr::RefDefault();
+ auto resolvedProvider = fResourceProvider
+ ? fResourceProvider : sk_make_sp<NullResourceProvider>();
+ auto resolvedFontMgr = fFontMgr
+ ? fFontMgr : SkFontMgr::RefDefault();
memset(&fStats, 0, sizeof(struct Stats));
@@ -233,8 +234,8 @@
SkASSERT(resolvedProvider);
SkASSERT(resolvedFontMgr);
- internal::AnimationBuilder builder(*resolvedProvider, std::move(resolvedFontMgr), &fStats,
- duration, fps);
+ internal::AnimationBuilder builder(std::move(resolvedProvider), std::move(resolvedFontMgr),
+ &fStats, duration, fps);
auto scene = builder.parse(json);
const auto t2 = SkTime::GetMSecs();
@@ -268,17 +269,16 @@
if (!data)
return nullptr;
- const auto* origProvider = fResourceProvider;
- std::unique_ptr<ResourceProvider> localProvider;
- if (!fResourceProvider) {
- localProvider = skstd::make_unique<DirectoryResourceProvider>(SkOSPath::Dirname(path));
- fResourceProvider = localProvider.get();
+ const auto useLocalProvider = !fResourceProvider;
+ if (useLocalProvider) {
+ fResourceProvider = sk_make_sp<DirectoryResourceProvider>(SkOSPath::Dirname(path));
}
auto animation = this->make(static_cast<const char*>(data->data()), data->size());
- // Don't leak localProvider references.
- fResourceProvider = origProvider;
+ if (useLocalProvider) {
+ fResourceProvider.reset();
+ }
return animation;
}
diff --git a/modules/skottie/src/SkottieLayer.cpp b/modules/skottie/src/SkottieLayer.cpp
index 55e5e33..a796683 100644
--- a/modules/skottie/src/SkottieLayer.cpp
+++ b/modules/skottie/src/SkottieLayer.cpp
@@ -206,14 +206,14 @@
const float fTimeScale;
};
- const auto data = fResourceProvider.load("", name);
+ const auto data = fResourceProvider->load("", name);
if (!data) {
LOG("!! Could not load: %s\n", name);
return nullptr;
}
auto animation = Animation::Builder()
- .setResourceProvider(&fResourceProvider)
+ .setResourceProvider(fResourceProvider)
.setFontManager(fFontMgr)
.make(static_cast<const char*>(data->data()), data->size());
if (!animation) {
@@ -294,7 +294,7 @@
return *attached_image;
}
- const auto data = fResourceProvider.load(path_cstr, name_cstr);
+ const auto data = fResourceProvider->load(path_cstr, name_cstr);
if (!data) {
LOG("!! Could not load image resource: %s/%s\n", path_cstr, name_cstr);
return nullptr;
diff --git a/modules/skottie/src/SkottiePriv.h b/modules/skottie/src/SkottiePriv.h
index 7300c9d..37332f6 100644
--- a/modules/skottie/src/SkottiePriv.h
+++ b/modules/skottie/src/SkottiePriv.h
@@ -44,7 +44,7 @@
class AnimationBuilder final : public SkNoncopyable {
public:
- AnimationBuilder(const ResourceProvider&, sk_sp<SkFontMgr>, Animation::Builder::Stats*,
+ AnimationBuilder(sk_sp<ResourceProvider>, sk_sp<SkFontMgr>, Animation::Builder::Stats*,
float duration, float framerate);
std::unique_ptr<sksg::Scene> parse(const skjson::ObjectValue&);
@@ -75,7 +75,7 @@
sk_sp<sksg::RenderNode> attachSolidLayer (const skjson::ObjectValue&, AnimatorScope*);
sk_sp<sksg::RenderNode> attachTextLayer (const skjson::ObjectValue&, AnimatorScope*);
- const ResourceProvider& fResourceProvider;
+ sk_sp<ResourceProvider> fResourceProvider;
sk_sp<SkFontMgr> fFontMgr;
Animation::Builder::Stats* fStats;
const float fDuration,