Move image creation out of latchBuffer().
GLImage creation isn't necessary until an external texture needs to be
bound.
This also removes the BufferLayerConsumer::Image class, as it's not
really necessary now that image creation is deferred until the external
texture needs to be bound.
Bug: 116277151
Change-Id: Id9420941e9cf7df11233b427c7a98a947f9738d8
Test: SurfaceFlinger_test, libsurfaceflinger_unittest, go/wm-smoke
diff --git a/services/surfaceflinger/BufferLayer.h b/services/surfaceflinger/BufferLayer.h
index a294793..d000d85 100644
--- a/services/surfaceflinger/BufferLayer.h
+++ b/services/surfaceflinger/BufferLayer.h
@@ -138,7 +138,7 @@
virtual void setFilteringEnabled(bool enabled) = 0;
- virtual status_t bindTextureImage() const = 0;
+ virtual status_t bindTextureImage() = 0;
virtual status_t updateTexImage(bool& recomputeVisibleRegions, nsecs_t latchTime,
const sp<Fence>& flushFence) = 0;
diff --git a/services/surfaceflinger/BufferLayerConsumer.cpp b/services/surfaceflinger/BufferLayerConsumer.cpp
index f499f06..8bd98b7 100644
--- a/services/surfaceflinger/BufferLayerConsumer.cpp
+++ b/services/surfaceflinger/BufferLayerConsumer.cpp
@@ -232,8 +232,7 @@
return;
}
- auto buffer = mPendingRelease.isPending ? mPendingRelease.graphicBuffer
- : mCurrentTextureImage->graphicBuffer();
+ auto buffer = mPendingRelease.isPending ? mPendingRelease.graphicBuffer : mCurrentTextureBuffer;
auto err = addReleaseFence(slot, buffer, fence);
if (err != OK) {
BLC_LOGE("setReleaseFence: failed to add the fence: %s (%d)", strerror(-err), err);
@@ -269,10 +268,9 @@
}
// If item->mGraphicBuffer is not null, this buffer has not been acquired
- // before, so any prior EglImage created is using a stale buffer. This
- // replaces any old EglImage with a new one (using the new buffer).
+ // before.
if (item->mGraphicBuffer != nullptr) {
- mImages[item->mSlot] = new Image(item->mGraphicBuffer, mRE);
+ mImages[item->mSlot] = nullptr;
}
return NO_ERROR;
@@ -299,19 +297,18 @@
}
BLC_LOGV("updateAndRelease: (slot=%d buf=%p) -> (slot=%d buf=%p)", mCurrentTexture,
- mCurrentTextureImage != nullptr ? mCurrentTextureImage->graphicBufferHandle() : 0,
- slot, mSlots[slot].mGraphicBuffer->handle);
+ mCurrentTextureBuffer != nullptr ? mCurrentTextureBuffer->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<Image> nextTextureImage = mImages[slot];
+ sp<GraphicBuffer> nextTextureBuffer = mSlots[slot].mGraphicBuffer;
// release old buffer
if (mCurrentTexture != BufferQueue::INVALID_BUFFER_SLOT) {
if (pendingRelease == nullptr) {
- status_t status =
- releaseBufferLocked(mCurrentTexture, mCurrentTextureImage->graphicBuffer());
+ status_t status = releaseBufferLocked(mCurrentTexture, mCurrentTextureBuffer);
if (status < NO_ERROR) {
BLC_LOGE("updateAndRelease: failed to release buffer: %s (%d)", strerror(-status),
status);
@@ -320,14 +317,15 @@
}
} else {
pendingRelease->currentTexture = mCurrentTexture;
- pendingRelease->graphicBuffer = mCurrentTextureImage->graphicBuffer();
+ pendingRelease->graphicBuffer = mCurrentTextureBuffer;
pendingRelease->isPending = true;
}
}
// Update the BufferLayerConsumer state.
mCurrentTexture = slot;
- mCurrentTextureImage = nextTextureImage;
+ mCurrentTextureBuffer = nextTextureBuffer;
+ mCurrentTextureImageFreed = nullptr;
mCurrentCrop = item.mCrop;
mCurrentTransform = item.mTransform;
mCurrentScalingMode = item.mScalingMode;
@@ -348,22 +346,46 @@
status_t BufferLayerConsumer::bindTextureImageLocked() {
ATRACE_CALL();
+
mRE.checkErrors();
- if (mCurrentTexture == BufferQueue::INVALID_BUFFER_SLOT && mCurrentTextureImage == nullptr) {
+ // It is possible for the current slot's buffer to be freed before a new one
+ // is bound. In that scenario we still want to bind the image.
+ if (mCurrentTexture == BufferQueue::INVALID_BUFFER_SLOT && mCurrentTextureBuffer == nullptr) {
BLC_LOGE("bindTextureImage: no currently-bound texture");
mRE.bindExternalTextureImage(mTexName, *mRE.createImage());
return NO_INIT;
}
- status_t err = mCurrentTextureImage->createIfNeeded();
- if (err != NO_ERROR) {
- BLC_LOGW("bindTextureImage: can't create image on slot=%d", mCurrentTexture);
- mRE.bindExternalTextureImage(mTexName, *mRE.createImage());
- return UNKNOWN_ERROR;
+ renderengine::Image* imageToRender;
+
+ // mCurrentTextureImageFreed is non-null iff mCurrentTexture ==
+ // BufferQueue::INVALID_BUFFER_SLOT, so we can omit that check.
+ if (mCurrentTextureImageFreed) {
+ imageToRender = mCurrentTextureImageFreed.get();
+ } else if (mImages[mCurrentTexture]) {
+ imageToRender = mImages[mCurrentTexture].get();
+ } else {
+ std::unique_ptr<renderengine::Image> image = mRE.createImage();
+ bool success = image->setNativeWindowBuffer(mCurrentTextureBuffer->getNativeBuffer(),
+ mCurrentTextureBuffer->getUsage() &
+ GRALLOC_USAGE_PROTECTED);
+ if (!success) {
+ BLC_LOGE("bindTextureImage: Failed to create image. size=%ux%u st=%u usage=%#" PRIx64
+ " fmt=%d",
+ mCurrentTextureBuffer->getWidth(), mCurrentTextureBuffer->getHeight(),
+ mCurrentTextureBuffer->getStride(), mCurrentTextureBuffer->getUsage(),
+ mCurrentTextureBuffer->getPixelFormat());
+ BLC_LOGW("bindTextureImage: can't create image on slot=%d", mCurrentTexture);
+ mRE.bindExternalTextureImage(mTexName, *image);
+ return UNKNOWN_ERROR;
+ }
+ imageToRender = image.get();
+ // Cache the image here so that we can reuse it.
+ mImages[mCurrentTexture] = std::move(image);
}
- mRE.bindExternalTextureImage(mTexName, mCurrentTextureImage->image());
+ mRE.bindExternalTextureImage(mTexName, *imageToRender);
// Wait for the new buffer to be ready.
return doFenceWaitLocked();
@@ -380,8 +402,7 @@
return UNKNOWN_ERROR;
}
status_t err =
- addReleaseFenceLocked(mCurrentTexture, mCurrentTextureImage->graphicBuffer(),
- releaseFence);
+ addReleaseFenceLocked(mCurrentTexture, mCurrentTextureBuffer, releaseFence);
if (err != OK) {
BLC_LOGE("syncForReleaseLocked: error adding release fence: "
"%s (%d)",
@@ -408,24 +429,22 @@
bool needsRecompute = mFilteringEnabled != enabled;
mFilteringEnabled = enabled;
- if (needsRecompute && mCurrentTextureImage == nullptr) {
- BLC_LOGD("setFilteringEnabled called with mCurrentTextureImage == nullptr");
+ if (needsRecompute && mCurrentTextureBuffer == nullptr) {
+ BLC_LOGD("setFilteringEnabled called with mCurrentTextureBuffer == nullptr");
}
- if (needsRecompute && mCurrentTextureImage != nullptr) {
+ if (needsRecompute && mCurrentTextureBuffer != nullptr) {
computeCurrentTransformMatrixLocked();
}
}
void BufferLayerConsumer::computeCurrentTransformMatrixLocked() {
BLC_LOGV("computeCurrentTransformMatrixLocked");
- sp<GraphicBuffer> buf =
- (mCurrentTextureImage == nullptr) ? nullptr : mCurrentTextureImage->graphicBuffer();
- if (buf == nullptr) {
+ if (mCurrentTextureBuffer == nullptr) {
BLC_LOGD("computeCurrentTransformMatrixLocked: "
- "mCurrentTextureImage is nullptr");
+ "mCurrentTextureBuffer is nullptr");
}
- GLConsumer::computeTransformMatrix(mCurrentTransformMatrix, buf, mCurrentCrop,
+ GLConsumer::computeTransformMatrix(mCurrentTransformMatrix, mCurrentTextureBuffer, mCurrentCrop,
mCurrentTransform, mFilteringEnabled);
}
@@ -474,7 +493,7 @@
*outSlot = mCurrentTexture;
}
- return (mCurrentTextureImage == nullptr) ? nullptr : mCurrentTextureImage->graphicBuffer();
+ return mCurrentTextureBuffer;
}
Rect BufferLayerConsumer::getCurrentCrop() const {
@@ -509,7 +528,6 @@
BLC_LOGE("doFenceWait: RenderEngine is not current");
return INVALID_OPERATION;
}
-
if (mCurrentFence->isValid()) {
if (mRE.useWaitSync()) {
base::unique_fd fenceFd(mCurrentFence->dup());
@@ -537,8 +555,10 @@
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].clear();
ConsumerBase::freeBufferLocked(slotIndex);
}
@@ -577,7 +597,7 @@
void BufferLayerConsumer::abandonLocked() {
BLC_LOGV("abandonLocked");
- mCurrentTextureImage.clear();
+ mCurrentTextureBuffer.clear();
ConsumerBase::abandonLocked();
}
@@ -595,29 +615,4 @@
ConsumerBase::dumpLocked(result, prefix);
}
-BufferLayerConsumer::Image::Image(sp<GraphicBuffer> graphicBuffer,
- renderengine::RenderEngine& engine)
- : mGraphicBuffer(graphicBuffer),
- mImage{engine.createImage()},
- mCreated(false),
- mCropWidth(0),
- mCropHeight(0) {}
-
-BufferLayerConsumer::Image::~Image() = default;
-
-status_t BufferLayerConsumer::Image::createIfNeeded() {
- if (mCreated) return OK;
-
- mCreated = mImage->setNativeWindowBuffer(mGraphicBuffer->getNativeBuffer(),
- mGraphicBuffer->getUsage() & GRALLOC_USAGE_PROTECTED);
- if (!mCreated) {
- const sp<GraphicBuffer>& buffer = mGraphicBuffer;
- ALOGE("Failed to create image. size=%ux%u st=%u usage=%#" PRIx64 " fmt=%d",
- buffer->getWidth(), buffer->getHeight(), buffer->getStride(), buffer->getUsage(),
- buffer->getPixelFormat());
- }
-
- return mCreated ? OK : UNKNOWN_ERROR;
-}
-
}; // namespace android
diff --git a/services/surfaceflinger/BufferLayerConsumer.h b/services/surfaceflinger/BufferLayerConsumer.h
index dc00487..a6610b7 100644
--- a/services/surfaceflinger/BufferLayerConsumer.h
+++ b/services/surfaceflinger/BufferLayerConsumer.h
@@ -187,8 +187,7 @@
// specific info in addition to the ConsumerBase behavior.
virtual void dumpLocked(String8& result, const char* prefix) const;
- // acquireBufferLocked overrides the ConsumerBase method to update the
- // mImages array in addition to the ConsumerBase behavior.
+ // See ConsumerBase::acquireBufferLocked
virtual status_t acquireBufferLocked(BufferItem* item, nsecs_t presentWhen,
uint64_t maxFrameNumber = 0) override;
@@ -212,53 +211,14 @@
PendingRelease* pendingRelease = nullptr,
const sp<Fence>& releaseFence = Fence::NO_FENCE);
- // Binds mTexName and the current buffer to TEXTURE_EXTERNAL target. Uses
- // mCurrentTexture if it's set, mCurrentTextureImage if not. If the
- // bind succeeds, this calls doFenceWait.
+ // Binds mTexName and the current buffer to TEXTURE_EXTERNAL target.
+ // If the bind succeeds, this calls doFenceWait.
status_t bindTextureImageLocked();
private:
- // Image is a utility class for tracking and creating renderengine::Images. There
- // is primarily just one image per slot, but there is also special cases:
- // - After freeBuffer, we must still keep the current image/buffer
- // Reference counting renderengine::Images lets us handle all these cases easily while
- // also only creating new renderengine::Images from buffers when required.
- class Image : public LightRefBase<Image> {
- public:
- Image(sp<GraphicBuffer> graphicBuffer, renderengine::RenderEngine& engine);
-
- Image(const Image& rhs) = delete;
- Image& operator=(const Image& rhs) = delete;
-
- // createIfNeeded creates an renderengine::Image if we haven't created one yet.
- status_t createIfNeeded();
-
- const sp<GraphicBuffer>& graphicBuffer() { return mGraphicBuffer; }
- const native_handle* graphicBufferHandle() {
- return mGraphicBuffer == nullptr ? nullptr : mGraphicBuffer->handle;
- }
-
- const renderengine::Image& image() const { return *mImage; }
-
- private:
- // Only allow instantiation using ref counting.
- friend class LightRefBase<Image>;
- virtual ~Image();
-
- // mGraphicBuffer is the buffer that was used to create this image.
- sp<GraphicBuffer> mGraphicBuffer;
-
- // mImage is the image created from mGraphicBuffer.
- std::unique_ptr<renderengine::Image> mImage;
- bool mCreated;
- int32_t mCropWidth;
- int32_t mCropHeight;
- };
-
// freeBufferLocked frees up the given buffer slot. If the slot has been
// initialized this will release the reference to the GraphicBuffer in
- // that slot and destroy the renderengine::Image in that slot. Otherwise it has no
- // effect.
+ // that slot. Otherwise it has no effect.
//
// This method must be called with mMutex locked.
virtual void freeBufferLocked(int slotIndex);
@@ -292,10 +252,10 @@
// consume buffers as hardware textures.
static const uint64_t DEFAULT_USAGE_FLAGS = GraphicBuffer::USAGE_HW_TEXTURE;
- // mCurrentTextureImage is the Image/buffer of the current texture. It's
+ // mCurrentTextureImage 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<Image> mCurrentTextureImage;
+ sp<GraphicBuffer> mCurrentTextureBuffer;
// mCurrentCrop is the crop rectangle that applies to the current texture.
// It gets set each time updateTexImage is called.
@@ -365,15 +325,6 @@
wp<ContentsChangedListener> mContentsChangedListener;
- // mImages stores the buffers that have been allocated by the BufferQueue
- // for each buffer slot. It is initialized to null pointers, and gets
- // filled in with the result of BufferQueue::acquire when the
- // client dequeues a buffer from a
- // slot that has not yet been used. The buffer allocated to a slot will also
- // be replaced if the requested buffer usage or geometry differs from that
- // of the buffer allocated to a slot.
- sp<Image> mImages[BufferQueueDefs::NUM_BUFFER_SLOTS];
-
// mCurrentTexture is the buffer slot index of the buffer that is currently
// bound to the RenderEngine texture. It is initialized to INVALID_BUFFER_SLOT,
// indicating that no buffer slot is currently bound to the texture. Note,
@@ -382,6 +333,17 @@
// 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];
+
// A release that is pending on the receipt of a new release fence from
// presentDisplay
PendingRelease mPendingRelease;
diff --git a/services/surfaceflinger/BufferQueueLayer.cpp b/services/surfaceflinger/BufferQueueLayer.cpp
index 6822298..576724c 100644
--- a/services/surfaceflinger/BufferQueueLayer.cpp
+++ b/services/surfaceflinger/BufferQueueLayer.cpp
@@ -217,7 +217,7 @@
return mConsumer->setFilteringEnabled(enabled);
}
-status_t BufferQueueLayer::bindTextureImage() const {
+status_t BufferQueueLayer::bindTextureImage() {
return mConsumer->bindTextureImage();
}
diff --git a/services/surfaceflinger/BufferQueueLayer.h b/services/surfaceflinger/BufferQueueLayer.h
index dcf39ee..472abf2 100644
--- a/services/surfaceflinger/BufferQueueLayer.h
+++ b/services/surfaceflinger/BufferQueueLayer.h
@@ -89,7 +89,7 @@
void setFilteringEnabled(bool enabled) override;
- status_t bindTextureImage() const override;
+ status_t bindTextureImage() override;
status_t updateTexImage(bool& recomputeVisibleRegions, nsecs_t latchTime,
const sp<Fence>& releaseFence) override;
diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp
index 2ecf71c..ffed0a9 100644
--- a/services/surfaceflinger/BufferStateLayer.cpp
+++ b/services/surfaceflinger/BufferStateLayer.cpp
@@ -323,7 +323,7 @@
mCurrentTransform, enabled);
}
-status_t BufferStateLayer::bindTextureImage() const {
+status_t BufferStateLayer::bindTextureImage() {
const State& s(getDrawingState());
auto& engine(mFlinger->getRenderEngine());
@@ -334,11 +334,8 @@
engine.checkErrors();
- if (!mTextureImage) {
- ALOGE("no currently-bound texture");
- engine.bindExternalTextureImage(mTextureName, *engine.createImage());
- return NO_INIT;
- }
+ // TODO(marissaw): once buffers are cached, don't create a new image everytime
+ mTextureImage = engine.createImage();
bool created =
mTextureImage->setNativeWindowBuffer(s.buffer->getNativeBuffer(),
@@ -385,16 +382,6 @@
return NO_ERROR;
}
- auto& engine(mFlinger->getRenderEngine());
- if (!engine.isCurrent()) {
- ALOGE("RenderEngine is not current");
- return INVALID_OPERATION;
- }
- engine.checkErrors();
-
- // TODO(marissaw): once buffers are cached, don't create a new image everytime
- mTextureImage = engine.createImage();
-
// Reject if the layer is invalid
uint32_t bufferWidth = s.buffer->width;
uint32_t bufferHeight = s.buffer->height;
diff --git a/services/surfaceflinger/BufferStateLayer.h b/services/surfaceflinger/BufferStateLayer.h
index db11110..b106f23 100644
--- a/services/surfaceflinger/BufferStateLayer.h
+++ b/services/surfaceflinger/BufferStateLayer.h
@@ -114,7 +114,7 @@
void setFilteringEnabled(bool enabled) override;
- status_t bindTextureImage() const override;
+ status_t bindTextureImage() override;
status_t updateTexImage(bool& recomputeVisibleRegions, nsecs_t latchTime,
const sp<Fence>& releaseFence) override;
diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
index 7b79fbd..e45fca0 100644
--- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
@@ -476,8 +476,6 @@
EXPECT_CALL(*test->mRenderEngine, isCurrent()).WillRepeatedly(Return(true));
EXPECT_CALL(*test->mRenderEngine, useNativeFenceSync()).WillRepeatedly(Return(true));
- EXPECT_CALL(*test->mRenderEngine, createImage())
- .WillOnce(Return(ByMove(std::unique_ptr<renderengine::Image>(test->mReImage))));
bool ignoredRecomputeVisibleRegions;
layer->latchBuffer(ignoredRecomputeVisibleRegions, 0, Fence::NO_FENCE);
Mock::VerifyAndClear(test->mRenderEngine);
@@ -574,6 +572,9 @@
LayerProperties::COLOR[2], LayerProperties::COLOR[3])))
.Times(1);
+ EXPECT_CALL(*test->mRenderEngine, createImage())
+ .WillOnce(Return(ByMove(std::unique_ptr<renderengine::Image>(test->mReImage))));
+ EXPECT_CALL(*test->mReImage, setNativeWindowBuffer(_, _)).WillOnce(Return(true));
EXPECT_CALL(*test->mRenderEngine, bindExternalTextureImage(DEFAULT_TEXTURE_ID, _)).Times(1);
EXPECT_CALL(*test->mRenderEngine, setupLayerTexturing(_)).Times(1);
EXPECT_CALL(*test->mRenderEngine, setSourceDataSpace(ui::Dataspace::UNKNOWN)).Times(1);
@@ -669,6 +670,9 @@
static constexpr uint32_t LAYER_FLAGS = ISurfaceComposerClient::eSecure;
static void setupInsecureREBufferCompositionCommonCallExpectations(CompositionTest* test) {
+ EXPECT_CALL(*test->mRenderEngine, createImage())
+ .WillOnce(Return(ByMove(std::unique_ptr<renderengine::Image>(test->mReImage))));
+ EXPECT_CALL(*test->mReImage, setNativeWindowBuffer(_, _)).WillOnce(Return(true));
EXPECT_CALL(*test->mRenderEngine, bindExternalTextureImage(DEFAULT_TEXTURE_ID, _)).Times(1);
EXPECT_CALL(*test->mRenderEngine, setupLayerBlackedOut()).Times(1);