Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 0dc207f

Browse files
fmalitaSkia Commit-Bot
authored andcommitted
[skottie] Cleanup: refactor asset lookup to avoid std::function
The main value of current AnimationBuilder::attachAssetRef is to provide scoping semantics for ref cycle detection. Refactor using a RAII helper (ScopedAssetRef), and avoid std::function callbacks. Change-Id: Idf5327465b8a06313cd9ea89be5f229ddc0aef7f Reviewed-on: https://skia-review.googlesource.com/c/skia/+/288617 Reviewed-by: Mike Reed <[email protected]> Commit-Queue: Florin Malita <[email protected]>
1 parent 71c3141 commit 0dc207f

File tree

4 files changed

+38
-25
lines changed

4 files changed

+38
-25
lines changed

modules/skottie/src/Composition.cpp

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,33 +18,29 @@
1818
namespace skottie {
1919
namespace internal {
2020

21-
sk_sp<sksg::RenderNode> AnimationBuilder::attachAssetRef(
22-
const skjson::ObjectValue& jlayer,
23-
const std::function<sk_sp<sksg::RenderNode>(const skjson::ObjectValue&)>& func) const {
24-
21+
AnimationBuilder::ScopedAssetRef::ScopedAssetRef(const AnimationBuilder* abuilder,
22+
const skjson::ObjectValue& jlayer) {
2523
const auto refId = ParseDefault<SkString>(jlayer["refId"], SkString());
2624
if (refId.isEmpty()) {
27-
this->log(Logger::Level::kError, nullptr, "Layer missing refId.");
28-
return nullptr;
25+
abuilder->log(Logger::Level::kError, nullptr, "Layer missing refId.");
26+
return;
2927
}
3028

31-
const auto* asset_info = fAssets.find(refId);
29+
const auto* asset_info = abuilder->fAssets.find(refId);
3230
if (!asset_info) {
33-
this->log(Logger::Level::kError, nullptr, "Asset not found: '%s'.", refId.c_str());
34-
return nullptr;
31+
abuilder->log(Logger::Level::kError, nullptr, "Asset not found: '%s'.", refId.c_str());
32+
return;
3533
}
3634

3735
if (asset_info->fIsAttaching) {
38-
this->log(Logger::Level::kError, nullptr,
36+
abuilder->log(Logger::Level::kError, nullptr,
3937
"Asset cycle detected for: '%s'", refId.c_str());
40-
return nullptr;
38+
return;
4139
}
4240

4341
asset_info->fIsAttaching = true;
44-
auto asset = func(*asset_info->fAsset);
45-
asset_info->fIsAttaching = false;
4642

47-
return asset;
43+
fInfo = asset_info;
4844
}
4945

5046
CompositionBuilder::CompositionBuilder(const AnimationBuilder& abuilder,

modules/skottie/src/SkottiePriv.h

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
#include "modules/sksg/include/SkSGScene.h"
2020
#include "src/utils/SkUTF.h"
2121

22-
#include <functional>
2322
#include <vector>
2423

2524
class SkFontMgr;
@@ -189,8 +188,6 @@ class AnimationBuilder final : public SkNoncopyable {
189188
sk_sp<sksg::RenderNode>) const;
190189

191190
sk_sp<sksg::RenderNode> attachShape(const skjson::ArrayValue*, AttachShapeContext*) const;
192-
sk_sp<sksg::RenderNode> attachAssetRef(const skjson::ObjectValue&,
193-
const std::function<sk_sp<sksg::RenderNode>(const skjson::ObjectValue&)>&) const;
194191
const FootageAssetInfo* loadFootageAsset(const skjson::ObjectValue&) const;
195192
sk_sp<sksg::RenderNode> attachFootageAsset(const skjson::ObjectValue&, LayerInfo*) const;
196193

@@ -253,6 +250,24 @@ class AnimationBuilder final : public SkNoncopyable {
253250
SkISize fSize;
254251
};
255252

253+
class ScopedAssetRef {
254+
public:
255+
ScopedAssetRef(const AnimationBuilder* abuilder, const skjson::ObjectValue& jlayer);
256+
257+
~ScopedAssetRef() {
258+
if (fInfo) {
259+
fInfo->fIsAttaching = false;
260+
}
261+
}
262+
263+
operator bool() const { return !!fInfo; }
264+
265+
const skjson::ObjectValue& operator*() const { return *fInfo->fAsset; }
266+
267+
private:
268+
const AssetInfo* fInfo = nullptr;
269+
};
270+
256271
SkTHashMap<SkString, AssetInfo> fAssets;
257272
SkTHashMap<SkString, FontInfo> fFonts;
258273
mutable SkTHashMap<SkString, FootageAssetInfo> fImageAssetCache;

modules/skottie/src/layers/FootageLayer.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -147,10 +147,11 @@ sk_sp<sksg::RenderNode> AnimationBuilder::attachFootageAsset(const skjson::Objec
147147

148148
sk_sp<sksg::RenderNode> AnimationBuilder::attachFootageLayer(const skjson::ObjectValue& jlayer,
149149
LayerInfo* layer_info) const {
150-
return this->attachAssetRef(jlayer,
151-
[this, &layer_info] (const skjson::ObjectValue& jimage) {
152-
return this->attachFootageAsset(jimage, layer_info);
153-
});
150+
const ScopedAssetRef footage_asset(this, jlayer);
151+
152+
return footage_asset
153+
? this->attachFootageAsset(*footage_asset, layer_info)
154+
: nullptr;
154155
}
155156

156157
} // namespace internal

modules/skottie/src/layers/PrecompLayer.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -186,10 +186,11 @@ sk_sp<sksg::RenderNode> AnimationBuilder::attachPrecompLayer(const skjson::Objec
186186
auto precomp_layer = this->attachExternalPrecompLayer(jlayer, *layer_info);
187187

188188
if (!precomp_layer) {
189-
precomp_layer = this->attachAssetRef(jlayer,
190-
[this, layer_info] (const skjson::ObjectValue& jcomp) {
191-
return CompositionBuilder(*this, layer_info->fSize, jcomp).build(*this);
192-
});
189+
const ScopedAssetRef precomp_asset(this, jlayer);
190+
if (precomp_asset) {
191+
precomp_layer =
192+
CompositionBuilder(*this, layer_info->fSize, *precomp_asset).build(*this);
193+
}
193194
}
194195

195196
if (requires_time_mapping) {

0 commit comments

Comments
 (0)