Defer deleting ExternalTextures that go out of scope during DrawLayers
Some buffers (e.g. protected content) are not pre-mapped into a
texture. Their textures are created in drawLayers and go out of scope
at the end of drawLayers drawLayers when those temporary textures are
sent to the GPU.
This CL ensures that in those cases we defer unbinding and deleting the
temporary texture until the cleanupPostRender method is called. Also to
avoid unecessary thread hops into cleanupPostRender this CL also adds a
thread-safe check to see if cleanup is necessary. Finally, we also make
the threaded variant asynchronous to further improve the availability of
SurfaceFlinger.
Bug: 190628682
Bug: 191132989
Test: atest librenderengine_test and perfetto traces
Change-Id: Ic2f4384cd1957c928a0ef656a98eb0041e29622c
diff --git a/libs/renderengine/skia/AutoBackendTexture.cpp b/libs/renderengine/skia/AutoBackendTexture.cpp
index 8356005..5c122d4 100644
--- a/libs/renderengine/skia/AutoBackendTexture.cpp
+++ b/libs/renderengine/skia/AutoBackendTexture.cpp
@@ -29,8 +29,8 @@
namespace skia {
AutoBackendTexture::AutoBackendTexture(GrDirectContext* context, AHardwareBuffer* buffer,
- bool isOutputBuffer)
- : mIsOutputBuffer(isOutputBuffer) {
+ bool isOutputBuffer, CleanupManager& cleanupMgr)
+ : mCleanupMgr(cleanupMgr), mIsOutputBuffer(isOutputBuffer) {
ATRACE_CALL();
AHardwareBuffer_Desc desc;
AHardwareBuffer_describe(buffer, &desc);
@@ -49,6 +49,13 @@
this, desc.width, desc.height, createProtectedImage, isOutputBuffer, desc.format);
}
+AutoBackendTexture::~AutoBackendTexture() {
+ if (mBackendTexture.isValid()) {
+ mDeleteProc(mImageCtx);
+ mBackendTexture = {};
+ }
+}
+
void AutoBackendTexture::unref(bool releaseLocalResources) {
if (releaseLocalResources) {
mSurface = nullptr;
@@ -57,11 +64,7 @@
mUsageCount--;
if (mUsageCount <= 0) {
- if (mBackendTexture.isValid()) {
- mDeleteProc(mImageCtx);
- mBackendTexture = {};
- }
- delete this;
+ mCleanupMgr.add(this);
}
}
diff --git a/libs/renderengine/skia/AutoBackendTexture.h b/libs/renderengine/skia/AutoBackendTexture.h
index a9e8430..00b901b 100644
--- a/libs/renderengine/skia/AutoBackendTexture.h
+++ b/libs/renderengine/skia/AutoBackendTexture.h
@@ -25,6 +25,9 @@
#include "android-base/macros.h"
+#include <mutex>
+#include <vector>
+
namespace android {
namespace renderengine {
namespace skia {
@@ -36,13 +39,50 @@
*/
class AutoBackendTexture {
public:
+ // Manager class that is responsible for the immediate or deferred cleanup
+ // of AutoBackendTextures. Clients of AutoBackendTexture are responsible for
+ // ensuring that access to this class is thread safe. Clients also control when
+ // the resources are reclaimed by setting the manager into deferred mode.
+ class CleanupManager {
+ public:
+ CleanupManager() = default;
+ void add(AutoBackendTexture* abt) {
+ if (mDeferCleanup) {
+ mCleanupList.push_back(abt);
+ } else {
+ delete abt;
+ }
+ }
+
+ void setDeferredStatus(bool enabled) { mDeferCleanup = enabled; }
+
+ bool isEmpty() const { return mCleanupList.empty(); }
+
+ // If any AutoBackedTextures were added while in deferred mode this method
+ // will ensure they are deleted before returning. It must only be called
+ // on the thread where the GPU context that created the AutoBackedTexture
+ // is active.
+ void cleanup() {
+ for (auto abt : mCleanupList) {
+ delete abt;
+ }
+ mCleanupList.clear();
+ }
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(CleanupManager);
+ bool mDeferCleanup = false;
+ std::vector<AutoBackendTexture*> mCleanupList;
+ };
+
// Local reference that supports RAII-style management of an AutoBackendTexture
// AutoBackendTexture by itself can't be managed in a similar fashion because
// of shared ownership with Skia objects, so we wrap it here instead.
class LocalRef {
public:
- LocalRef(GrDirectContext* context, AHardwareBuffer* buffer, bool isOutputBuffer) {
- mTexture = new AutoBackendTexture(context, buffer, isOutputBuffer);
+ LocalRef(GrDirectContext* context, AHardwareBuffer* buffer, bool isOutputBuffer,
+ CleanupManager& cleanupMgr) {
+ mTexture = new AutoBackendTexture(context, buffer, isOutputBuffer, cleanupMgr);
mTexture->ref();
}
@@ -75,10 +115,11 @@
private:
// Creates a GrBackendTexture whose contents come from the provided buffer.
- AutoBackendTexture(GrDirectContext* context, AHardwareBuffer* buffer, bool isOutputBuffer);
+ AutoBackendTexture(GrDirectContext* context, AHardwareBuffer* buffer, bool isOutputBuffer,
+ CleanupManager& cleanupMgr);
// The only way to invoke dtor is with unref, when mUsageCount is 0.
- ~AutoBackendTexture() {}
+ ~AutoBackendTexture();
void ref() { mUsageCount++; }
@@ -100,6 +141,8 @@
GrAHardwareBufferUtils::UpdateImageProc mUpdateProc;
GrAHardwareBufferUtils::TexImageCtx mImageCtx;
+ CleanupManager& mCleanupMgr;
+
static void releaseSurfaceProc(SkSurface::ReleaseContext releaseContext);
static void releaseImageProc(SkImage::ReleaseContext releaseContext);
diff --git a/libs/renderengine/skia/SkiaGLRenderEngine.cpp b/libs/renderengine/skia/SkiaGLRenderEngine.cpp
index f0986a3..d28d623 100644
--- a/libs/renderengine/skia/SkiaGLRenderEngine.cpp
+++ b/libs/renderengine/skia/SkiaGLRenderEngine.cpp
@@ -531,7 +531,7 @@
std::shared_ptr<AutoBackendTexture::LocalRef> imageTextureRef =
std::make_shared<AutoBackendTexture::LocalRef>(grContext,
buffer->toAHardwareBuffer(),
- isRenderable);
+ isRenderable, mTextureCleanupMgr);
cache.insert({buffer->getId(), imageTextureRef});
}
}
@@ -559,6 +559,31 @@
}
}
+bool SkiaGLRenderEngine::canSkipPostRenderCleanup() const {
+ std::lock_guard<std::mutex> lock(mRenderingMutex);
+ return mTextureCleanupMgr.isEmpty();
+}
+
+void SkiaGLRenderEngine::cleanupPostRender() {
+ ATRACE_CALL();
+ std::lock_guard<std::mutex> lock(mRenderingMutex);
+ mTextureCleanupMgr.cleanup();
+}
+
+// Helper class intended to be used on the stack to ensure that texture cleanup
+// is deferred until after this class goes out of scope.
+class DeferTextureCleanup final {
+public:
+ DeferTextureCleanup(AutoBackendTexture::CleanupManager& mgr) : mMgr(mgr) {
+ mMgr.setDeferredStatus(true);
+ }
+ ~DeferTextureCleanup() { mMgr.setDeferredStatus(false); }
+
+private:
+ DISALLOW_COPY_AND_ASSIGN(DeferTextureCleanup);
+ AutoBackendTexture::CleanupManager& mMgr;
+};
+
sk_sp<SkShader> SkiaGLRenderEngine::createRuntimeEffectShader(
sk_sp<SkShader> shader,
const LayerSettings* layer, const DisplaySettings& display, bool undoPremultipliedAlpha,
@@ -707,6 +732,9 @@
auto grContext = getActiveGrContext();
auto& cache = mTextureCache;
+ // any AutoBackendTexture deletions will now be deferred until cleanupPostRender is called
+ DeferTextureCleanup dtc(mTextureCleanupMgr);
+
std::shared_ptr<AutoBackendTexture::LocalRef> surfaceTextureRef;
if (const auto& it = cache.find(buffer->getBuffer()->getId()); it != cache.end()) {
surfaceTextureRef = it->second;
@@ -715,7 +743,7 @@
std::make_shared<AutoBackendTexture::LocalRef>(grContext,
buffer->getBuffer()
->toAHardwareBuffer(),
- true);
+ true, mTextureCleanupMgr);
}
const ui::Dataspace dstDataspace =
@@ -971,7 +999,7 @@
imageTextureRef = std::make_shared<
AutoBackendTexture::LocalRef>(grContext,
item.buffer->getBuffer()->toAHardwareBuffer(),
- false);
+ false, mTextureCleanupMgr);
}
// isOpaque means we need to ignore the alpha in the image,
diff --git a/libs/renderengine/skia/SkiaGLRenderEngine.h b/libs/renderengine/skia/SkiaGLRenderEngine.h
index b972c73..b30355b 100644
--- a/libs/renderengine/skia/SkiaGLRenderEngine.h
+++ b/libs/renderengine/skia/SkiaGLRenderEngine.h
@@ -59,7 +59,8 @@
const std::shared_ptr<ExternalTexture>& buffer,
const bool useFramebufferCache, base::unique_fd&& bufferFence,
base::unique_fd* drawFence) override;
- void cleanFramebufferCache() override {}
+ void cleanupPostRender() override;
+ void cleanFramebufferCache() override{};
int getContextPriority() override;
bool isProtected() const override { return mInProtectedContext; }
bool supportsProtectedContent() const override;
@@ -75,6 +76,7 @@
size_t getMaxViewportDims() const override;
void mapExternalTextureBuffer(const sp<GraphicBuffer>& buffer, bool isRenderable) override;
void unmapExternalTextureBuffer(const sp<GraphicBuffer>& buffer) override;
+ bool canSkipPostRenderCleanup() const override;
private:
static EGLConfig chooseEglConfig(EGLDisplay display, int format, bool logConfig);
@@ -130,13 +132,14 @@
std::unordered_map<GraphicBufferId, std::shared_ptr<AutoBackendTexture::LocalRef>> mTextureCache
GUARDED_BY(mRenderingMutex);
std::unordered_map<LinearEffect, sk_sp<SkRuntimeEffect>, LinearEffectHasher> mRuntimeEffects;
+ AutoBackendTexture::CleanupManager mTextureCleanupMgr GUARDED_BY(mRenderingMutex);
StretchShaderFactory mStretchShaderFactory;
// Mutex guarding rendering operations, so that:
// 1. GL operations aren't interleaved, and
// 2. Internal state related to rendering that is potentially modified by
// multiple threads is guaranteed thread-safe.
- std::mutex mRenderingMutex;
+ mutable std::mutex mRenderingMutex;
sp<Fence> mLastDrawFence;
diff --git a/libs/renderengine/skia/SkiaRenderEngine.h b/libs/renderengine/skia/SkiaRenderEngine.h
index f098a86..31ad63e 100644
--- a/libs/renderengine/skia/SkiaRenderEngine.h
+++ b/libs/renderengine/skia/SkiaRenderEngine.h
@@ -53,7 +53,6 @@
base::unique_fd* /*drawFence*/) override {
return 0;
};
- virtual bool cleanupPostRender(CleanupMode) override { return true; };
virtual int getContextPriority() override { return 0; }
virtual void assertShadersCompiled(int numShaders) {}
virtual int reportShadersCompiled() { return 0; }