[skottie] More efficient group sizing
Replace sksg::Group::shrink_to_fit() with a factory accepting an
externally-built children array.
This provides more control for clients (e.g. reserve() instead of
shrink_to_fit()).
Change-Id: Iad587435e0e9da15251a9d3bc2510ca945950b5d
Reviewed-on: https://skia-review.googlesource.com/152861
Commit-Queue: Florin Malita <fmalita@chromium.org>
Reviewed-by: Mike Klein <mtklein@google.com>
diff --git a/modules/skottie/src/SkottieLayer.cpp b/modules/skottie/src/SkottieLayer.cpp
index a79a47f..b3a88ea 100644
--- a/modules/skottie/src/SkottieLayer.cpp
+++ b/modules/skottie/src/SkottieLayer.cpp
@@ -27,6 +27,7 @@
#include "SkSGRect.h"
#include "SkSGTransform.h"
+#include <algorithm>
#include <vector>
namespace skottie {
@@ -151,15 +152,23 @@
return sksg::ClipEffect::Make(std::move(childNode), std::move(clip_node), true);
}
- auto mask_group = sksg::Group::Make();
- for (const auto& rec : mask_stack) {
- mask_group->addChild(sksg::Draw::Make(std::move(rec.mask_path),
- std::move(rec.mask_paint)));
+ sk_sp<sksg::RenderNode> maskNode;
+ if (mask_stack.count() == 1) {
+ // no group needed for single mask
+ maskNode = sksg::Draw::Make(std::move(mask_stack.front().mask_path),
+ std::move(mask_stack.front().mask_paint));
+ } else {
+ std::vector<sk_sp<sksg::RenderNode>> masks;
+ masks.reserve(SkToSizeT(mask_stack.count()));
+ for (const auto& rec : mask_stack) {
+ masks.push_back(sksg::Draw::Make(std::move(rec.mask_path),
+ std::move(rec.mask_paint)));
+ }
+ maskNode = sksg::Group::Make(std::move(masks));
}
- mask_group->shrink_to_fit();
- return sksg::MaskEffect::Make(std::move(childNode), std::move(mask_group));
+ return sksg::MaskEffect::Make(std::move(childNode), std::move(maskNode));
}
} // namespace
@@ -502,12 +511,13 @@
const skjson::ArrayValue* jlayers = comp["layers"];
if (!jlayers) return nullptr;
- SkSTArray<16, sk_sp<sksg::RenderNode>, true> layers;
- AttachLayerContext layerCtx(*jlayers, scope);
+ std::vector<sk_sp<sksg::RenderNode>> layers;
+ AttachLayerContext layerCtx(*jlayers, scope);
+ layers.reserve(jlayers->size());
for (const auto& l : *jlayers) {
- if (auto layer_fragment = this->attachLayer(l, &layerCtx)) {
- layers.push_back(std::move(layer_fragment));
+ if (auto layer = this->attachLayer(l, &layerCtx)) {
+ layers.push_back(std::move(layer));
}
}
@@ -516,13 +526,10 @@
}
// Layers are painted in bottom->top order.
- auto comp_group = sksg::Group::Make();
- for (int i = layers.count() - 1; i >= 0; --i) {
- comp_group->addChild(std::move(layers[i]));
- }
- comp_group->shrink_to_fit();
+ std::reverse(layers.begin(), layers.end());
+ layers.shrink_to_fit();
- return std::move(comp_group);
+ return sksg::Group::Make(std::move(layers));
}
} // namespace internal
diff --git a/modules/skottie/src/SkottieShapeLayer.cpp b/modules/skottie/src/SkottieShapeLayer.cpp
index 3767fe4..b6e7a27 100644
--- a/modules/skottie/src/SkottieShapeLayer.cpp
+++ b/modules/skottie/src/SkottieShapeLayer.cpp
@@ -24,6 +24,7 @@
#include "SkSGTransform.h"
#include "SkSGTrimEffect.h"
+#include <algorithm>
#include <iterator>
namespace skottie {
@@ -577,16 +578,12 @@
// For a single draw, we don't need a group.
shape_wrapper = std::move(draws.front());
} else if (!draws.empty()) {
- // We need a group to dispatch multiple draws.
- auto group = sksg::Group::Make();
-
// Emit local draws reversed (bottom->top, per spec).
- for (auto it = draws.rbegin(); it != draws.rend(); ++it) {
- group->addChild(std::move(*it));
- }
- group->shrink_to_fit();
+ std::reverse(draws.begin(), draws.end());
+ draws.shrink_to_fit();
- shape_wrapper = std::move(group);
+ // We need a group to dispatch multiple draws.
+ shape_wrapper = sksg::Group::Make(std::move(draws));
}
sk_sp<sksg::Matrix> shape_matrix;
diff --git a/modules/sksg/include/SkSGGroup.h b/modules/sksg/include/SkSGGroup.h
index 04506dc..cc8635a 100644
--- a/modules/sksg/include/SkSGGroup.h
+++ b/modules/sksg/include/SkSGGroup.h
@@ -20,7 +20,11 @@
class Group : public RenderNode {
public:
static sk_sp<Group> Make() {
- return sk_sp<Group>(new Group());
+ return sk_sp<Group>(new Group(std::vector<sk_sp<RenderNode>>()));
+ }
+
+ static sk_sp<Group> Make(std::vector<sk_sp<RenderNode>> children) {
+ return sk_sp<Group>(new Group(std::move(children)));
}
void addChild(sk_sp<RenderNode>);
@@ -29,10 +33,8 @@
size_t size() const { return fChildren.size(); }
bool empty() const { return fChildren.empty(); }
- void shrink_to_fit();
-
protected:
- Group();
+ explicit Group(std::vector<sk_sp<RenderNode>>);
~Group() override;
void onRender(SkCanvas*, const RenderContext*) const override;
diff --git a/modules/sksg/src/SkSGGroup.cpp b/modules/sksg/src/SkSGGroup.cpp
index 2c0a9d7..4841714 100644
--- a/modules/sksg/src/SkSGGroup.cpp
+++ b/modules/sksg/src/SkSGGroup.cpp
@@ -11,7 +11,12 @@
namespace sksg {
-Group::Group() {}
+Group::Group(std::vector<sk_sp<RenderNode>> children)
+ : fChildren(std::move(children)) {
+ for (const auto& child : fChildren) {
+ this->observeInval(child);
+ }
+}
Group::~Group() {
for (const auto& child : fChildren) {
@@ -41,10 +46,6 @@
this->invalidate();
}
-void Group::shrink_to_fit() {
- fChildren.shrink_to_fit();
-}
-
void Group::onRender(SkCanvas* canvas, const RenderContext* ctx) const {
// TODO: this heuristic works at the moment, but:
// a) it is fragile because it relies on all leaf render nodes being atomic draws