Clean up Renderengine caching semantics
* Add unbindExternalTextureBuffer, so that callers such as
BufferLayerConsumer can help manage the cache directly, which improves
system utilization.
* Remove the CacheHint, as it's no longer needed.
* Remove the backing image cache in BufferLayerConsumer, as it has not
been used. We still need a shadow array to properly callback into
RenderEngine, but we no longer need to store EGLImages here.
Bug: 123107664
Test: systrace
Test: open and close apps with GL composition forced, check for buffer
leakage through dumpsys
Change-Id: Ie48f99868dd46a4e18b5d659eea520e97a9eb300
diff --git a/services/surfaceflinger/BufferLayer.cpp b/services/surfaceflinger/BufferLayer.cpp
index 6badc73..756ca42 100644
--- a/services/surfaceflinger/BufferLayer.cpp
+++ b/services/surfaceflinger/BufferLayer.cpp
@@ -163,9 +163,6 @@
layer.source.buffer.buffer = mActiveBuffer;
layer.source.buffer.isOpaque = isOpaque(s);
layer.source.buffer.fence = mActiveBufferFence;
- layer.source.buffer.cacheHint = useCachedBufferForClientComposition()
- ? renderengine::Buffer::CachingHint::USE_CACHE
- : renderengine::Buffer::CachingHint::NO_CACHE;
layer.source.buffer.textureName = mTextureName;
layer.source.buffer.usePremultipliedAlpha = getPremultipledAlpha();
layer.source.buffer.isY410BT2020 = isHdrY410();
diff --git a/services/surfaceflinger/BufferLayer.h b/services/surfaceflinger/BufferLayer.h
index c48146f..e9dbdea 100644
--- a/services/surfaceflinger/BufferLayer.h
+++ b/services/surfaceflinger/BufferLayer.h
@@ -161,10 +161,6 @@
bool mRefreshPending{false};
- // Returns true if, when drawing the active buffer during gpu compositon, we
- // should use a cached buffer or not.
- virtual bool useCachedBufferForClientComposition() const = 0;
-
// prepareClientLayer - constructs a RenderEngine layer for GPU composition.
bool prepareClientLayer(const RenderArea& renderArea, const Region& clip,
bool useIdentityTransform, Region& clearRegion,
diff --git a/services/surfaceflinger/BufferLayerConsumer.cpp b/services/surfaceflinger/BufferLayerConsumer.cpp
index 6866e5c..7d2dcba 100644
--- a/services/surfaceflinger/BufferLayerConsumer.cpp
+++ b/services/surfaceflinger/BufferLayerConsumer.cpp
@@ -178,7 +178,8 @@
return;
}
- auto buffer = mPendingRelease.isPending ? mPendingRelease.graphicBuffer : mCurrentTextureBuffer;
+ auto buffer = mPendingRelease.isPending ? mPendingRelease.graphicBuffer
+ : mCurrentTextureBuffer->graphicBuffer();
auto err = addReleaseFence(slot, buffer, fence);
if (err != OK) {
BLC_LOGE("setReleaseFence: failed to add the fence: %s (%d)", strerror(-err), err);
@@ -214,9 +215,9 @@
}
// If item->mGraphicBuffer is not null, this buffer has not been acquired
- // before.
+ // before, so we need to clean up old references.
if (item->mGraphicBuffer != nullptr) {
- mImages[item->mSlot] = nullptr;
+ mImages[item->mSlot] = std::make_shared<Image>(item->mGraphicBuffer, mRE);
}
return NO_ERROR;
@@ -229,18 +230,21 @@
int slot = item.mSlot;
BLC_LOGV("updateAndRelease: (slot=%d buf=%p) -> (slot=%d buf=%p)", mCurrentTexture,
- mCurrentTextureBuffer != nullptr ? mCurrentTextureBuffer->handle : 0, slot,
- mSlots[slot].mGraphicBuffer->handle);
+ (mCurrentTextureBuffer != nullptr && mCurrentTextureBuffer->graphicBuffer() != nullptr)
+ ? mCurrentTextureBuffer->graphicBuffer()->handle
+ : 0,
+ slot, mSlots[slot].mGraphicBuffer->handle);
// Hang onto the pointer so that it isn't freed in the call to
// releaseBufferLocked() if we're in shared buffer mode and both buffers are
// the same.
- sp<GraphicBuffer> nextTextureBuffer = mSlots[slot].mGraphicBuffer;
+ std::shared_ptr<Image> nextTextureBuffer = mImages[slot];
// release old buffer
if (mCurrentTexture != BufferQueue::INVALID_BUFFER_SLOT) {
if (pendingRelease == nullptr) {
- status_t status = releaseBufferLocked(mCurrentTexture, mCurrentTextureBuffer);
+ status_t status =
+ releaseBufferLocked(mCurrentTexture, mCurrentTextureBuffer->graphicBuffer());
if (status < NO_ERROR) {
BLC_LOGE("updateAndRelease: failed to release buffer: %s (%d)", strerror(-status),
status);
@@ -249,7 +253,7 @@
}
} else {
pendingRelease->currentTexture = mCurrentTexture;
- pendingRelease->graphicBuffer = mCurrentTextureBuffer;
+ pendingRelease->graphicBuffer = mCurrentTextureBuffer->graphicBuffer();
pendingRelease->isPending = true;
}
}
@@ -257,8 +261,6 @@
// Update the BufferLayerConsumer state.
mCurrentTexture = slot;
mCurrentTextureBuffer = nextTextureBuffer;
- mCurrentTextureBufferStaleForGpu = false;
- mCurrentTextureImageFreed = nullptr;
mCurrentCrop = item.mCrop;
mCurrentTransform = item.mTransform;
mCurrentScalingMode = item.mScalingMode;
@@ -280,7 +282,12 @@
status_t BufferLayerConsumer::bindTextureImageLocked() {
ATRACE_CALL();
- return mRE.bindExternalTextureBuffer(mTexName, mCurrentTextureBuffer, mCurrentFence, false);
+ if (mCurrentTextureBuffer != nullptr && mCurrentTextureBuffer->graphicBuffer() != nullptr) {
+ return mRE.bindExternalTextureBuffer(mTexName, mCurrentTextureBuffer->graphicBuffer(),
+ mCurrentFence);
+ }
+
+ return NO_INIT;
}
void BufferLayerConsumer::getTransformMatrix(float mtx[16]) {
@@ -308,12 +315,15 @@
void BufferLayerConsumer::computeCurrentTransformMatrixLocked() {
BLC_LOGV("computeCurrentTransformMatrixLocked");
- if (mCurrentTextureBuffer == nullptr) {
+ if (mCurrentTextureBuffer == nullptr || mCurrentTextureBuffer->graphicBuffer() == nullptr) {
BLC_LOGD("computeCurrentTransformMatrixLocked: "
"mCurrentTextureBuffer is nullptr");
}
- GLConsumer::computeTransformMatrix(mCurrentTransformMatrix, mCurrentTextureBuffer, mCurrentCrop,
- mCurrentTransform, mFilteringEnabled);
+ GLConsumer::computeTransformMatrix(mCurrentTransformMatrix,
+ mCurrentTextureBuffer == nullptr
+ ? nullptr
+ : mCurrentTextureBuffer->graphicBuffer(),
+ mCurrentCrop, mCurrentTransform, mFilteringEnabled);
}
nsecs_t BufferLayerConsumer::getTimestamp() {
@@ -365,16 +375,7 @@
*outFence = mCurrentFence;
}
- return mCurrentTextureBuffer;
-}
-
-bool BufferLayerConsumer::getAndSetCurrentBufferCacheHint() {
- Mutex::Autolock lock(mMutex);
- bool useCache = mCurrentTextureBufferStaleForGpu;
- // Set the staleness bit here, as this function is only called during a
- // client composition path.
- mCurrentTextureBufferStaleForGpu = true;
- return useCache;
+ return mCurrentTextureBuffer == nullptr ? nullptr : mCurrentTextureBuffer->graphicBuffer();
}
Rect BufferLayerConsumer::getCurrentCrop() const {
@@ -432,10 +433,8 @@
BLC_LOGV("freeBufferLocked: slotIndex=%d", slotIndex);
if (slotIndex == mCurrentTexture) {
mCurrentTexture = BufferQueue::INVALID_BUFFER_SLOT;
- mCurrentTextureImageFreed = std::move(mImages[slotIndex]);
- } else {
- mImages[slotIndex] = nullptr;
}
+ mImages[slotIndex] = nullptr;
ConsumerBase::freeBufferLocked(slotIndex);
}
@@ -474,7 +473,10 @@
void BufferLayerConsumer::abandonLocked() {
BLC_LOGV("abandonLocked");
- mCurrentTextureBuffer.clear();
+ mCurrentTextureBuffer = nullptr;
+ for (int i = 0; i < BufferQueue::NUM_BUFFER_SLOTS; i++) {
+ mImages[i] = nullptr;
+ }
ConsumerBase::abandonLocked();
}
@@ -492,4 +494,11 @@
ConsumerBase::dumpLocked(result, prefix);
}
+BufferLayerConsumer::Image::~Image() {
+ if (mGraphicBuffer != nullptr) {
+ ALOGE("Destroying buffer: %" PRId64, mGraphicBuffer->getId());
+ mRE.unbindExternalTextureBuffer(mGraphicBuffer->getId());
+ }
+}
+
}; // namespace android
diff --git a/services/surfaceflinger/BufferLayerConsumer.h b/services/surfaceflinger/BufferLayerConsumer.h
index e2a6d2e..32ccfbb 100644
--- a/services/surfaceflinger/BufferLayerConsumer.h
+++ b/services/surfaceflinger/BufferLayerConsumer.h
@@ -149,11 +149,6 @@
// for use with bilinear filtering.
void setFilteringEnabled(bool enabled);
- // Sets mCurrentTextureBufferStaleForGpu to true to indicate that the
- // buffer is now "stale" for GPU composition, and returns the old staleness
- // bit as a caching hint.
- bool getAndSetCurrentBufferCacheHint();
-
// getCurrentBuffer returns the buffer associated with the current image.
// When outSlot is not nullptr, the current buffer slot index is also
// returned. Simiarly, when outFence is not nullptr, the current output
@@ -218,6 +213,22 @@
status_t bindTextureImageLocked();
private:
+ // Utility class for managing GraphicBuffer references into renderengine
+ class Image {
+ public:
+ Image(sp<GraphicBuffer> graphicBuffer, renderengine::RenderEngine& engine)
+ : mGraphicBuffer(graphicBuffer), mRE(engine) {}
+ virtual ~Image();
+ const sp<GraphicBuffer>& graphicBuffer() { return mGraphicBuffer; }
+
+ private:
+ // mGraphicBuffer is the buffer that was used to create this image.
+ sp<GraphicBuffer> mGraphicBuffer;
+ // Back-reference into renderengine to initiate cleanup.
+ renderengine::RenderEngine& mRE;
+ DISALLOW_COPY_AND_ASSIGN(Image);
+ };
+
// freeBufferLocked frees up the given buffer slot. If the slot has been
// initialized this will release the reference to the GraphicBuffer in
// that slot. Otherwise it has no effect.
@@ -248,14 +259,10 @@
// consume buffers as hardware textures.
static const uint64_t DEFAULT_USAGE_FLAGS = GraphicBuffer::USAGE_HW_TEXTURE;
- // mCurrentTextureImage is the buffer containing the current texture. It's
+ // mCurrentTextureBuffer is the buffer containing the current texture. It's
// possible that this buffer is not associated with any buffer slot, so we
// must track it separately in order to support the getCurrentBuffer method.
- sp<GraphicBuffer> mCurrentTextureBuffer;
-
- // True if the buffer was used for the previous client composition frame,
- // and false otherwise.
- bool mCurrentTextureBufferStaleForGpu;
+ std::shared_ptr<Image> mCurrentTextureBuffer;
// mCurrentCrop is the crop rectangle that applies to the current texture.
// It gets set each time updateTexImage is called.
@@ -333,16 +340,8 @@
// reset mCurrentTexture to INVALID_BUFFER_SLOT.
int mCurrentTexture;
- // Cached image used for rendering the current texture through GPU
- // composition, which contains the cached image after freeBufferLocked is
- // called on the current buffer. Whenever latchBuffer is called, this is
- // expected to be cleared. Then, if bindTexImage is called before the next
- // buffer is acquired, then this image is bound.
- std::unique_ptr<renderengine::Image> mCurrentTextureImageFreed;
-
- // Cached images used for rendering the current texture through GPU
- // composition.
- std::unique_ptr<renderengine::Image> mImages[BufferQueueDefs::NUM_BUFFER_SLOTS];
+ // Shadow buffer cache for cleaning up renderengine references.
+ std::shared_ptr<Image> mImages[BufferQueueDefs::NUM_BUFFER_SLOTS];
// A release that is pending on the receipt of a new release fence from
// presentDisplay
diff --git a/services/surfaceflinger/BufferQueueLayer.cpp b/services/surfaceflinger/BufferQueueLayer.cpp
index 215dea1..16e8e95 100644
--- a/services/surfaceflinger/BufferQueueLayer.cpp
+++ b/services/surfaceflinger/BufferQueueLayer.cpp
@@ -342,10 +342,6 @@
return NO_ERROR;
}
-bool BufferQueueLayer::useCachedBufferForClientComposition() const {
- return mConsumer->getAndSetCurrentBufferCacheHint();
-}
-
status_t BufferQueueLayer::updateFrameNumber(nsecs_t latchTime) {
mPreviousFrameNumber = mCurrentFrameNumber;
mCurrentFrameNumber = mConsumer->getFrameNumber();
diff --git a/services/surfaceflinger/BufferQueueLayer.h b/services/surfaceflinger/BufferQueueLayer.h
index fbb8c14..a2aad17 100644
--- a/services/surfaceflinger/BufferQueueLayer.h
+++ b/services/surfaceflinger/BufferQueueLayer.h
@@ -62,9 +62,6 @@
public:
bool fenceHasSignaled() const override;
-protected:
- bool useCachedBufferForClientComposition() const override;
-
private:
nsecs_t getDesiredPresentTime() override;
std::shared_ptr<FenceTime> getCurrentFenceTime() const override;
diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp
index f6b69eb..6390e85 100644
--- a/services/surfaceflinger/BufferStateLayer.cpp
+++ b/services/surfaceflinger/BufferStateLayer.cpp
@@ -48,7 +48,12 @@
mOverrideScalingMode = NATIVE_WINDOW_SCALING_MODE_SCALE_TO_WINDOW;
mCurrentState.dataspace = ui::Dataspace::V0_SRGB;
}
-BufferStateLayer::~BufferStateLayer() = default;
+BufferStateLayer::~BufferStateLayer() {
+ if (mActiveBuffer != nullptr) {
+ auto& engine(mFlinger->getRenderEngine());
+ engine.unbindExternalTextureBuffer(mActiveBuffer->getId());
+ }
+}
// -----------------------------------------------------------------------
// Interface implementation for Layer
@@ -468,7 +473,7 @@
const State& s(getDrawingState());
auto& engine(mFlinger->getRenderEngine());
- return engine.bindExternalTextureBuffer(mTextureName, s.buffer, s.acquireFence, false);
+ return engine.bindExternalTextureBuffer(mTextureName, s.buffer, s.acquireFence);
}
status_t BufferStateLayer::updateTexImage(bool& /*recomputeVisibleRegions*/, nsecs_t latchTime) {
@@ -542,6 +547,11 @@
return BAD_VALUE;
}
+ if (mActiveBuffer != nullptr) {
+ // todo: get this to work with BufferStateLayerCache
+ auto& engine(mFlinger->getRenderEngine());
+ engine.unbindExternalTextureBuffer(mActiveBuffer->getId());
+ }
mActiveBuffer = s.buffer;
mActiveBufferFence = s.acquireFence;
auto& layerCompositionState = getCompositionLayer()->editState().frontEnd;
@@ -551,11 +561,6 @@
return NO_ERROR;
}
-bool BufferStateLayer::useCachedBufferForClientComposition() const {
- // TODO: Store a proper staleness bit to support EGLImage caching.
- return false;
-}
-
status_t BufferStateLayer::updateFrameNumber(nsecs_t /*latchTime*/) {
// TODO(marissaw): support frame history events
mCurrentFrameNumber = mFrameNumber;
diff --git a/services/surfaceflinger/BufferStateLayer.h b/services/surfaceflinger/BufferStateLayer.h
index 0b03f49..97662e8 100644
--- a/services/surfaceflinger/BufferStateLayer.h
+++ b/services/surfaceflinger/BufferStateLayer.h
@@ -96,9 +96,6 @@
// -----------------------------------------------------------------------
bool fenceHasSignaled() const override;
-protected:
- bool useCachedBufferForClientComposition() const override;
-
private:
nsecs_t getDesiredPresentTime() override;
std::shared_ptr<FenceTime> getCurrentFenceTime() const override;
diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
index 6deec29..30ae764 100644
--- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
@@ -592,8 +592,6 @@
renderengine::LayerSettings layer = layerSettings.back();
EXPECT_THAT(layer.source.buffer.buffer, Not(IsNull()));
EXPECT_THAT(layer.source.buffer.fence, Not(IsNull()));
- EXPECT_EQ(renderengine::Buffer::CachingHint::NO_CACHE,
- layer.source.buffer.cacheHint);
EXPECT_EQ(DEFAULT_TEXTURE_ID, layer.source.buffer.textureName);
EXPECT_EQ(false, layer.source.buffer.isY410BT2020);
EXPECT_EQ(true, layer.source.buffer.usePremultipliedAlpha);