Add MultiDisplayList + memory leak fixes

MultiDisplayList can contain either a SkiaDisplayList
or a CanvasOpBuffer. However DisplayList itself
still points to the SkiaDisplayList-only wrapper
to avoid any std::variant or std::visit overhead
just yet.

Also fixes a memory leak in CanvasFrontend from an
uninitialized std::optional and a few minor leaks
in unit tests.

Test: build & hwui_unit passes

Change-Id: Ifa6b723b6456f5d3eeac1201e76f337250103d6f
This commit is contained in:
John Reck
2021-01-19 21:29:24 -05:00
parent f78234f112
commit d34d6cec97
14 changed files with 376 additions and 51 deletions

View File

@@ -615,6 +615,7 @@ cc_test {
"tests/unit/CommonPoolTests.cpp",
"tests/unit/DamageAccumulatorTests.cpp",
"tests/unit/DeferredLayerUpdaterTests.cpp",
"tests/unit/EglManagerTests.cpp",
"tests/unit/FatVectorTests.cpp",
"tests/unit/GraphicsStatsServiceTests.cpp",
"tests/unit/LayerUpdateQueueTests.cpp",

View File

@@ -17,8 +17,10 @@
#pragma once
#include "pipeline/skia/SkiaDisplayList.h"
#include "canvas/CanvasOpBuffer.h"
#include <memory>
#include <variant>
namespace android {
namespace uirenderer {
@@ -28,29 +30,25 @@ class Tree;
};
typedef uirenderer::VectorDrawable::Tree VectorDrawableRoot;
/**
* Data structure that holds the list of commands used in display list stream
*/
//using DisplayList = skiapipeline::SkiaDisplayList;
class DisplayList {
class SkiaDisplayListWrapper {
public:
// Constructs an empty (invalid) DisplayList
explicit DisplayList() {}
explicit SkiaDisplayListWrapper() {}
// Constructs a DisplayList from a SkiaDisplayList
explicit DisplayList(std::unique_ptr<skiapipeline::SkiaDisplayList> impl)
explicit SkiaDisplayListWrapper(std::unique_ptr<skiapipeline::SkiaDisplayList> impl)
: mImpl(std::move(impl)) {}
// Move support
DisplayList(DisplayList&& other) : mImpl(std::move(other.mImpl)) {}
DisplayList& operator=(DisplayList&& other) {
SkiaDisplayListWrapper(SkiaDisplayListWrapper&& other) : mImpl(std::move(other.mImpl)) {}
SkiaDisplayListWrapper& operator=(SkiaDisplayListWrapper&& other) {
mImpl = std::move(other.mImpl);
return *this;
}
// No copy support
DisplayList(const DisplayList& other) = delete;
DisplayList& operator=(const DisplayList&) = delete;
SkiaDisplayListWrapper(const SkiaDisplayListWrapper& other) = delete;
SkiaDisplayListWrapper& operator=(const SkiaDisplayListWrapper&) = delete;
void updateChildren(std::function<void(RenderNode*)> updateFn) {
mImpl->updateChildren(std::move(updateFn));
@@ -137,7 +135,7 @@ public:
void applyColorTransform(ColorTransform transform) {
if (mImpl) {
mImpl->mDisplayList.applyColorTransform(transform);
mImpl->applyColorTransform(transform);
}
}
@@ -145,5 +143,172 @@ private:
std::unique_ptr<skiapipeline::SkiaDisplayList> mImpl;
};
/**
* Data structure that holds the list of commands used in display list stream
*/
//using DisplayList = skiapipeline::SkiaDisplayList;
class MultiDisplayList {
private:
using SkiaDisplayList = skiapipeline::SkiaDisplayList;
struct EmptyList {
bool hasText() const { return false; }
void updateChildren(std::function<void(RenderNode*)> updateFn) {}
bool isEmpty() const { return true; }
bool containsProjectionReceiver() const { return false; }
bool hasVectorDrawables() const { return false; }
size_t getUsedSize() const { return 0; }
size_t getAllocatedSize() const { return 0; }
void output(std::ostream& output, uint32_t level) const { }
bool hasFunctor() const { return false; }
bool prepareListAndChildren(
TreeObserver& observer, TreeInfo& info, bool functorsNeedLayer,
std::function<void(RenderNode*, TreeObserver&, TreeInfo&, bool)> childFn) {
return false;
}
void syncContents(const WebViewSyncData& data) { }
void applyColorTransform(ColorTransform transform) { }
};
std::variant<EmptyList, std::unique_ptr<SkiaDisplayList>, CanvasOpBuffer> mImpls;
template <typename T>
static constexpr T& get(T& t) { return t; }
template <typename T>
static constexpr const T& get(const T& t) { return t; }
template <typename T>
static constexpr T& get(std::unique_ptr<T>& t) { return *t; }
template <typename T>
static constexpr const T& get(const std::unique_ptr<T>& t) { return *t; }
template <typename T>
auto apply(T&& t) {
return std::visit([&t](auto& it) -> auto {
return t(get(it));
}, mImpls);
}
template <typename T>
auto apply(T&& t) const {
return std::visit([&t](const auto& it) -> auto {
return t(get(it));
}, mImpls);
}
public:
// Constructs an empty (invalid) DisplayList
explicit MultiDisplayList() {}
// Constructs a DisplayList from a SkiaDisplayList
explicit MultiDisplayList(std::unique_ptr<SkiaDisplayList> impl)
: mImpls(std::move(impl)) {}
explicit MultiDisplayList(CanvasOpBuffer&& opBuffer) : mImpls(std::move(opBuffer)) {}
// Move support
MultiDisplayList(MultiDisplayList&& other) : mImpls(std::move(other.mImpls)) {}
MultiDisplayList& operator=(MultiDisplayList&& other) {
mImpls = std::move(other.mImpls);
return *this;
}
// No copy support
MultiDisplayList(const MultiDisplayList& other) = delete;
MultiDisplayList& operator=(const MultiDisplayList&) = delete;
void updateChildren(std::function<void(RenderNode*)> updateFn) {
apply([&](auto& it) { it.updateChildren(std::move(updateFn)); });
}
[[nodiscard]] explicit operator bool() const {
return isValid();
}
// If true this DisplayList contains a backing content, even if that content is empty
// If false, there this DisplayList is in an "empty" state
[[nodiscard]] bool isValid() const {
return mImpls.index() != 0;
}
[[nodiscard]] bool isEmpty() const {
return apply([](const auto& it) -> auto { return it.isEmpty(); });
}
[[nodiscard]] bool hasContent() const {
return !isEmpty();
}
[[nodiscard]] bool containsProjectionReceiver() const {
return apply([](const auto& it) -> auto { return it.containsProjectionReceiver(); });
}
[[nodiscard]] SkiaDisplayList* asSkiaDl() {
return std::get<1>(mImpls).get();
}
[[nodiscard]] const SkiaDisplayList* asSkiaDl() const {
return std::get<1>(mImpls).get();
}
[[nodiscard]] bool hasVectorDrawables() const {
return apply([](const auto& it) -> auto { return it.hasVectorDrawables(); });
}
void clear(RenderNode* owningNode = nullptr) {
if (owningNode && mImpls.index() == 1) {
auto& skiaDl = std::get<1>(mImpls);
if (skiaDl->reuseDisplayList(owningNode)) {
skiaDl.release();
}
}
mImpls = EmptyList{};
}
[[nodiscard]] size_t getUsedSize() const {
return apply([](const auto& it) -> auto { return it.getUsedSize(); });
}
[[nodiscard]] size_t getAllocatedSize() const {
return apply([](const auto& it) -> auto { return it.getAllocatedSize(); });
}
void output(std::ostream& output, uint32_t level) const {
apply([&](const auto& it) { it.output(output, level); });
}
[[nodiscard]] bool hasFunctor() const {
return apply([](const auto& it) -> auto { return it.hasFunctor(); });
}
bool prepareListAndChildren(
TreeObserver& observer, TreeInfo& info, bool functorsNeedLayer,
std::function<void(RenderNode*, TreeObserver&, TreeInfo&, bool)> childFn) {
return apply([&](auto& it) -> auto {
return it.prepareListAndChildren(observer, info, functorsNeedLayer, std::move(childFn));
});
}
void syncContents(const WebViewSyncData& data) {
apply([&](auto& it) { it.syncContents(data); });
}
[[nodiscard]] bool hasText() const {
return apply([](const auto& it) -> auto { return it.hasText(); });
}
void applyColorTransform(ColorTransform transform) {
apply([=](auto& it) { it.applyColorTransform(transform); });
}
[[nodiscard]] CanvasOpBuffer& asOpBuffer() {
return std::get<CanvasOpBuffer>(mImpls);
}
};
// For now stick to the original single-type container to avoid any regressions
using DisplayList = SkiaDisplayListWrapper;
} // namespace uirenderer
} // namespace android

View File

@@ -147,8 +147,7 @@ class CanvasFrontend final : public CanvasStateHelper {
public:
template<class... Args>
CanvasFrontend(int width, int height, Args&&... args) : CanvasStateHelper(width, height),
mReceiver(std::forward<Args>(args)...) { }
~CanvasFrontend() = default;
mReceiver(std::in_place, std::forward<Args>(args)...) { }
void save(SaveFlags::Flags flags = SaveFlags::MatrixClip) {
if (internalSave(flagsToSaveEntry(flags))) {
@@ -186,7 +185,10 @@ public:
submit(std::move(op));
}
const CanvasOpReceiver& receiver() const { return *mReceiver; }
const CanvasOpReceiver& receiver() const {
LOG_ALWAYS_FATAL_IF(!mReceiver.has_value());
return *mReceiver;
}
CanvasOpReceiver finish() {
auto ret = std::move(mReceiver.value());
@@ -205,6 +207,7 @@ private:
template <CanvasOpType T>
void submit(CanvasOp<T>&& op) {
LOG_ALWAYS_FATAL_IF(!mReceiver.has_value());
mReceiver->push_container(CanvasOpContainer(std::move(op), transform()));
}
};

View File

@@ -22,4 +22,32 @@ namespace android::uirenderer {
template class OpBuffer<CanvasOpType, CanvasOpContainer>;
void CanvasOpBuffer::updateChildren(std::function<void(RenderNode*)> updateFn) {
// TODO: Do we need a fast-path for finding children?
if (mHas.children) {
for (auto& iter : filter<CanvasOpType::DrawRenderNode>()) {
updateFn(iter->renderNode.get());
}
}
}
void CanvasOpBuffer::output(std::ostream& output, uint32_t level) const {
LOG_ALWAYS_FATAL("TODO");
}
bool CanvasOpBuffer::prepareListAndChildren(
TreeObserver& observer, TreeInfo& info, bool functorsNeedLayer,
std::function<void(RenderNode*, TreeObserver&, TreeInfo&, bool)> childFn) {
LOG_ALWAYS_FATAL("TODO");
return false;
}
void CanvasOpBuffer::syncContents(const WebViewSyncData& data) {
LOG_ALWAYS_FATAL("TODO");
}
void CanvasOpBuffer::applyColorTransform(ColorTransform transform) {
LOG_ALWAYS_FATAL("TODO");
}
} // namespace android::uirenderer

View File

@@ -19,10 +19,17 @@
#include <SkMatrix.h>
#include "CanvasOpTypes.h"
#include "CanvasTransform.h"
#include "OpBuffer.h"
#include "TreeInfo.h"
#include "private/hwui/WebViewFunctor.h"
#include <functional>
namespace android::uirenderer {
class RenderNode;
template <CanvasOpType T>
struct CanvasOp;
@@ -53,12 +60,74 @@ public:
};
extern template class OpBuffer<CanvasOpType, CanvasOpContainer>;
class CanvasOpBuffer final : public OpBuffer<CanvasOpType, CanvasOpContainer> {
class CanvasOpBuffer final : private OpBuffer<CanvasOpType, CanvasOpContainer> {
private:
using SUPER = OpBuffer<CanvasOpType, CanvasOpContainer>;
public:
// Expose select superclass methods publicly
using SUPER::for_each;
using SUPER::size;
using SUPER::resize;
template <CanvasOpType T>
void push(CanvasOp<T>&& op) {
push_container(CanvasOpContainer<T>(std::move(op)));
}
template <CanvasOpType T>
void push_container(CanvasOpContainer<T>&& op) {
if constexpr (IsDrawOp(T)) {
mHas.content = true;
}
if constexpr (T == CanvasOpType::DrawRenderNode) {
mHas.children = true;
// use staging property, since recording on UI thread
if (op->renderNode->stagingProperties().isProjectionReceiver()) {
mHas.projectionReceiver = true;
}
}
SUPER::push_container(std::move(op));
}
void clear() {
mHas = Contains{};
SUPER::clear();
}
void updateChildren(std::function<void(RenderNode*)> updateFn);
bool prepareListAndChildren(
TreeObserver& observer, TreeInfo& info, bool functorsNeedLayer,
std::function<void(RenderNode*, TreeObserver&, TreeInfo&, bool)> childFn);
void syncContents(const WebViewSyncData& data);
void applyColorTransform(ColorTransform transform);
[[nodiscard]] bool isEmpty() const { return !mHas.content; }
[[nodiscard]] bool hasText() const { return mHas.text; }
[[nodiscard]] bool hasVectorDrawables() const { return mHas.vectorDrawable; }
[[nodiscard]] bool containsProjectionReceiver() const { return mHas.projectionReceiver; }
[[nodiscard]] bool hasFunctor() const { return mHas.functor; }
[[nodiscard]] size_t getUsedSize() const {
return size();
}
[[nodiscard]] size_t getAllocatedSize() const {
return capacity();
}
void output(std::ostream& output, uint32_t level) const;
private:
struct Contains {
bool content : 1 = false;
bool children : 1 = false;
bool projectionReceiver : 1 = false;
bool text : 1 = false;
bool vectorDrawable : 1 = false;
bool functor : 1 = false;
};
Contains mHas;
};
} // namespace android::uirenderer

View File

@@ -33,21 +33,15 @@ void rasterizeCanvasBuffer(const CanvasOpBuffer& source, SkCanvas* destination)
SkMatrix& currentGlobalTransform = globalMatrixStack.emplace_back(SkMatrix::I());
source.for_each([&]<CanvasOpType T>(const CanvasOpContainer<T> * op) {
if constexpr (
T == CanvasOpType::BeginZ ||
T == CanvasOpType::EndZ ||
T == CanvasOpType::DrawLayer
) {
// Do beginZ or endZ
LOG_ALWAYS_FATAL("TODO");
return;
} else {
if constexpr (CanvasOpTraits::can_draw<CanvasOp<T>>) {
// Generic OP
// First apply the current transformation
destination->setMatrix(SkMatrix::Concat(currentGlobalTransform, op->transform()));
// Now draw it
(*op)->draw(destination);
return;
}
LOG_ALWAYS_FATAL("TODO, unable to rasterize %d", static_cast<int>(T));
});
}

View File

@@ -35,7 +35,8 @@ enum class CanvasOpType : int8_t {
ClipPath,
// Drawing ops
DrawColor,
DRAW_OP_BEGIN,
DrawColor = DRAW_OP_BEGIN,
DrawRect,
DrawRegion,
DrawRoundRect,
@@ -59,10 +60,16 @@ enum class CanvasOpType : int8_t {
DrawImageLattice,
DrawPicture,
DrawLayer,
DrawRenderNode,
DRAW_OP_END = DrawRenderNode,
// TODO: Rest
COUNT // must be last
};
static constexpr bool IsDrawOp(CanvasOpType t) {
return CanvasOpType::DRAW_OP_BEGIN <= t && t <= CanvasOpType::DRAW_OP_END;
}
} // namespace android::uirenderer

View File

@@ -24,13 +24,15 @@
#include <SkImage.h>
#include <SkPicture.h>
#include <SkRuntimeEffect.h>
#include <hwui/Bitmap.h>
#include <log/log.h>
#include "CanvasProperty.h"
#include "Points.h"
#include <log/log.h>
#include "hwui/Bitmap.h"
#include "CanvasProperty.h"
#include "CanvasOpTypes.h"
#include "Layer.h"
#include "Points.h"
#include "RenderNode.h"
#include <experimental/type_traits>
#include <utility>
@@ -440,6 +442,11 @@ struct CanvasOp<CanvasOpType::DrawLayer> {
sp<Layer> layer;
};
template<>
struct CanvasOp<CanvasOpType::DrawRenderNode> {
sp<RenderNode> renderNode;
};
// cleanup our macros
#undef ASSERT_DRAWABLE

View File

@@ -64,7 +64,7 @@ public:
static constexpr auto STARTING_SIZE = PadAlign(sizeof(BufferHeader));
using ItemHeader = OpBufferItemHeader<ItemTypes>;
OpBuffer() = default;
explicit OpBuffer() = default;
// Prevent copying by default
OpBuffer(const OpBuffer&) = delete;
@@ -135,7 +135,7 @@ public:
template <typename F>
void for_each(F&& f) const {
for_each(std::forward<F>(f), ItemTypesSequence{});
do_for_each(std::forward<F>(f), ItemTypesSequence{});
}
void clear();
@@ -225,7 +225,7 @@ private:
}
template <typename F, std::size_t... I>
void for_each(F&& f, std::index_sequence<I...>) const {
void do_for_each(F&& f, std::index_sequence<I...>) const {
// Validate we're not empty
if (isEmpty()) return;

View File

@@ -16,6 +16,8 @@
#pragma once
#include <deque>
#include "RecordingCanvas.h"
#include "RenderNodeDrawable.h"
#include "TreeInfo.h"
@@ -23,8 +25,6 @@
#include "utils/LinearAllocator.h"
#include "utils/Pair.h"
#include <deque>
namespace android {
namespace uirenderer {
@@ -46,8 +46,10 @@ class FunctorDrawable;
class SkiaDisplayList {
public:
size_t getUsedSize() { return allocator.usedSize() + mDisplayList.usedSize(); }
size_t getAllocatedSize() { return allocator.allocatedSize() + mDisplayList.allocatedSize(); }
size_t getUsedSize() const { return allocator.usedSize() + mDisplayList.usedSize(); }
size_t getAllocatedSize() const {
return allocator.allocatedSize() + mDisplayList.allocatedSize();
}
~SkiaDisplayList() {
/* Given that we are using a LinearStdAllocator to store some of the
@@ -109,6 +111,10 @@ public:
*/
void syncContents(const WebViewSyncData& data);
void applyColorTransform(ColorTransform transform) {
mDisplayList.applyColorTransform(transform);
}
/**
* ONLY to be called by RenderNode::prepareTree in order to prepare this
* list while the UI thread is blocked. Here we can upload mutable bitmaps
@@ -154,12 +160,12 @@ public:
std::deque<RenderNodeDrawable> mChildNodes;
std::deque<FunctorDrawable*> mChildFunctors;
std::vector<SkImage*> mMutableImages;
private:
std::vector<Pair<VectorDrawableRoot*, SkMatrix>> mVectorDrawables;
public:
void appendVD(VectorDrawableRoot* r) {
appendVD(r, SkMatrix::I());
}
void appendVD(VectorDrawableRoot* r) { appendVD(r, SkMatrix::I()); }
void appendVD(VectorDrawableRoot* r, const SkMatrix& mat) {
mVectorDrawables.push_back(Pair<VectorDrawableRoot*, SkMatrix>(r, mat));

View File

@@ -124,12 +124,12 @@ TEST(CanvasFrontend, transform) {
TEST(CanvasFrontend, drawOpTransform) {
CanvasFrontend<CanvasOpBuffer> opCanvas(100, 100);
const auto& receiver = opCanvas.receiver();
const auto &receiver = opCanvas.receiver();
auto makeDrawRect = [] {
return CanvasOp<CanvasOpType::DrawRect>{
.rect = SkRect::MakeWH(50, 50),
.paint = SkPaint(SkColors::kBlack),
.rect = SkRect::MakeWH(50, 50),
.paint = SkPaint(SkColors::kBlack),
};
};
@@ -167,14 +167,14 @@ TEST(CanvasFrontend, drawOpTransform) {
{
// First result should be identity
const auto& result = transforms[0];
const auto &result = transforms[0];
EXPECT_EQ(SkMatrix::kIdentity_Mask, result.getType());
EXPECT_EQ(SkMatrix::I(), result);
}
{
// Should be translate 10, 10
const auto& result = transforms[1];
const auto &result = transforms[1];
EXPECT_EQ(SkMatrix::kTranslate_Mask, result.getType());
SkMatrix m;
m.setTranslate(10, 10);
@@ -183,7 +183,7 @@ TEST(CanvasFrontend, drawOpTransform) {
{
// Should be translate 10, 10 + scale 2, 4
const auto& result = transforms[2];
const auto &result = transforms[2];
EXPECT_EQ(SkMatrix::kTranslate_Mask | SkMatrix::kScale_Mask, result.getType());
SkMatrix m;
m.setTranslate(10, 10);
@@ -193,7 +193,7 @@ TEST(CanvasFrontend, drawOpTransform) {
{
// Should be translate 10, 10 + translate 20, 15
const auto& result = transforms[3];
const auto &result = transforms[3];
EXPECT_EQ(SkMatrix::kTranslate_Mask, result.getType());
SkMatrix m;
m.setTranslate(30, 25);
@@ -202,9 +202,9 @@ TEST(CanvasFrontend, drawOpTransform) {
{
// Should be translate 10, 10 + translate 20, 15 + rotate 90
const auto& result = transforms[4];
const auto &result = transforms[4];
EXPECT_EQ(SkMatrix::kTranslate_Mask | SkMatrix::kAffine_Mask | SkMatrix::kScale_Mask,
result.getType());
result.getType());
SkMatrix m;
m.setTranslate(30, 25);
m.preRotate(90.f);

View File

@@ -149,7 +149,7 @@ TEST(CanvasOp, simpleDrawPoints) {
CanvasOpBuffer buffer;
EXPECT_EQ(buffer.size(), 0);
size_t numPts = 3;
auto pts = sk_ref_sp(
auto pts = sk_sp<Points>(
new Points({
{32, 16},
{48, 48},
@@ -192,7 +192,7 @@ TEST(CanvasOp, simpleDrawLines) {
CanvasOpBuffer buffer;
EXPECT_EQ(buffer.size(), 0);
size_t numPts = 3;
auto pts = sk_ref_sp(
auto pts = sk_sp<Points>(
new Points({
{32, 16},
{48, 48},

View File

@@ -0,0 +1,44 @@
/*
* Copyright (C) 2021 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#include <gtest/gtest.h>
#include "renderthread/EglManager.h"
#include "tests/common/TestContext.h"
using namespace android;
using namespace android::uirenderer;
using namespace android::uirenderer::renderthread;
using namespace android::uirenderer::test;
TEST(EglManager, doesSurfaceLeak) {
EglManager eglManager;
eglManager.initialize();
ASSERT_TRUE(eglManager.hasEglContext());
auto colorSpace = SkColorSpace::MakeSRGB();
for (int i = 0; i < 100; i++) {
TestContext context;
auto result =
eglManager.createSurface(context.surface().get(), ColorMode::Default, colorSpace);
EXPECT_TRUE(result);
EGLSurface surface = result.unwrap();
eglManager.destroySurface(surface);
}
eglManager.destroy();
}

View File

@@ -404,6 +404,7 @@ RENDERTHREAD_SKIA_PIPELINE_TEST(SkiaPipeline, context_lost) {
EXPECT_TRUE(pipeline->isSurfaceReady());
renderThread.destroyRenderingContext();
EXPECT_FALSE(pipeline->isSurfaceReady());
LOG_ALWAYS_FATAL_IF(pipeline->isSurfaceReady());
}
RENDERTHREAD_SKIA_PIPELINE_TEST(SkiaPipeline, pictureCallback) {