From 44685cb3f0e53c0f0b5f260891b9dd78d25410d4 Mon Sep 17 00:00:00 2001 From: Steven Thomas Date: Tue, 23 Jul 2019 16:19:31 -0700 Subject: Merge damage region when dropping an app buffer When we decide to drop a buffer from a buffer queue, make sure to merge the damage region from the dropped buffer into the current damage region. Otherwise we'll pass the wrong damage region to hardware composer and get broken rendering. Bug: 136158117 Test: Wrote a new test in Transaction_test.cpp to repro the problem and confirm the fix works. Change-Id: Icdc61e1be3297450f2869c496dad1453fb6dca6d --- services/surfaceflinger/BufferLayerConsumer.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'services/surfaceflinger/BufferLayerConsumer.cpp') diff --git a/services/surfaceflinger/BufferLayerConsumer.cpp b/services/surfaceflinger/BufferLayerConsumer.cpp index 6709fb4b48..bd9bd81e2a 100644 --- a/services/surfaceflinger/BufferLayerConsumer.cpp +++ b/services/surfaceflinger/BufferLayerConsumer.cpp @@ -369,6 +369,15 @@ const Region& BufferLayerConsumer::getSurfaceDamage() const { return mCurrentSurfaceDamage; } +void BufferLayerConsumer::mergeSurfaceDamage(const Region& damage) { + if (damage.bounds() == Rect::INVALID_RECT || + mCurrentSurfaceDamage.bounds() == Rect::INVALID_RECT) { + mCurrentSurfaceDamage = Region::INVALID_REGION; + } else { + mCurrentSurfaceDamage |= damage; + } +} + int BufferLayerConsumer::getCurrentApi() const { Mutex::Autolock lock(mMutex); return mCurrentApi; -- cgit v1.2.3-59-g8ed1b From 16a9940abee8752d8566e6c7b52b22d10693512b Mon Sep 17 00:00:00 2001 From: Alec Mouri Date: Mon, 29 Jul 2019 16:37:30 -0700 Subject: [RenderEngine] add an ImageManager thread Prior to this change, EGLImage management would be performed by both binder threads and the main rendering thread in a synchronized manner. But because of BufferQueue implementation details, it's possible for an app's Surface to be disconnected (destroying all backing EGLImages per the disconnect api spec), while still latching and presenting the most recently queued image (which requires recreating the EGLImage on the main thread), which can cause jank in some scenarios such as app-launch. To mitigate this, defer EGLImage creation and destruction to a separate thread behind RenderEngine so that creating the EGLImage should never be done on the main thread. So scenarios such as app-launch avoid jank by performing the EGLImage creation asynchronously with the main thread, so that the creation is done by the time RenderEngine needs to start rendering. Bug: 136806342 Bug: 137191934 Test: Launching photos app shows no bindExternalImage* calls during the rendering path Test: librenderengine_test Change-Id: Ib809e36267b5604bbbedd4089d15666b22cabc95 --- libs/renderengine/Android.bp | 2 + libs/renderengine/gl/GLESRenderEngine.cpp | 80 ++++++++++-- libs/renderengine/gl/GLESRenderEngine.h | 17 ++- libs/renderengine/gl/ImageManager.cpp | 140 +++++++++++++++++++++ libs/renderengine/gl/ImageManager.h | 70 +++++++++++ .../include/renderengine/RenderEngine.h | 12 +- .../include/renderengine/mock/RenderEngine.h | 2 +- libs/renderengine/tests/Android.bp | 1 + libs/renderengine/tests/RenderEngineTest.cpp | 54 ++++++-- services/surfaceflinger/BufferLayerConsumer.cpp | 7 +- services/surfaceflinger/BufferLayerConsumer.h | 3 +- 11 files changed, 360 insertions(+), 28 deletions(-) create mode 100644 libs/renderengine/gl/ImageManager.cpp create mode 100644 libs/renderengine/gl/ImageManager.h (limited to 'services/surfaceflinger/BufferLayerConsumer.cpp') diff --git a/libs/renderengine/Android.bp b/libs/renderengine/Android.bp index 36211ca733..cc252d67ae 100644 --- a/libs/renderengine/Android.bp +++ b/libs/renderengine/Android.bp @@ -26,6 +26,7 @@ cc_defaults { "libgui", "liblog", "libnativewindow", + "libprocessgroup", "libsync", "libui", "libutils", @@ -51,6 +52,7 @@ filegroup { "gl/GLExtensions.cpp", "gl/GLFramebuffer.cpp", "gl/GLImage.cpp", + "gl/ImageManager.cpp", "gl/Program.cpp", "gl/ProgramCache.cpp", ], diff --git a/libs/renderengine/gl/GLESRenderEngine.cpp b/libs/renderengine/gl/GLESRenderEngine.cpp index 7caaef35be..d2a7525113 100644 --- a/libs/renderengine/gl/GLESRenderEngine.cpp +++ b/libs/renderengine/gl/GLESRenderEngine.cpp @@ -19,9 +19,8 @@ #define LOG_TAG "RenderEngine" #define ATRACE_TAG ATRACE_TAG_GRAPHICS -#include "GLESRenderEngine.h" - -#include +#include +#include #include #include #include @@ -43,6 +42,7 @@ #include #include #include +#include "GLESRenderEngine.h" #include "GLExtensions.h" #include "GLFramebuffer.h" #include "GLImage.h" @@ -423,10 +423,13 @@ GLESRenderEngine::GLESRenderEngine(uint32_t featureFlags, EGLDisplay display, EG mTraceGpuCompletion = true; mFlushTracer = std::make_unique(this); } + mImageManager = std::make_unique(this); mDrawingBuffer = createFramebuffer(); } GLESRenderEngine::~GLESRenderEngine() { + // Destroy the image manager first. + mImageManager = nullptr; std::lock_guard lock(mRenderingMutex); unbindFrameBuffer(mDrawingBuffer.get()); mDrawingBuffer = nullptr; @@ -619,19 +622,41 @@ void GLESRenderEngine::bindExternalTextureImage(uint32_t texName, const Image& i status_t GLESRenderEngine::bindExternalTextureBuffer(uint32_t texName, const sp& buffer, const sp& bufferFence) { + if (buffer == nullptr) { + return BAD_VALUE; + } + ATRACE_CALL(); - status_t cacheResult = cacheExternalTextureBuffer(buffer); - if (cacheResult != NO_ERROR) { - return cacheResult; + bool found = false; + { + std::lock_guard lock(mRenderingMutex); + auto cachedImage = mImageCache.find(buffer->getId()); + found = (cachedImage != mImageCache.end()); } + // If we couldn't find the image in the cache at this time, then either + // SurfaceFlinger messed up registering the buffer ahead of time or we got + // backed up creating other EGLImages. + if (!found) { + status_t cacheResult = mImageManager->cache(buffer); + if (cacheResult != NO_ERROR) { + return cacheResult; + } + } + + // Whether or not we needed to cache, re-check mImageCache to make sure that + // there's an EGLImage. The current threading model guarantees that we don't + // destroy a cached image until it's really not needed anymore (i.e. this + // function should not be called), so the only possibility is that something + // terrible went wrong and we should just bind something and move on. { std::lock_guard lock(mRenderingMutex); auto cachedImage = mImageCache.find(buffer->getId()); if (cachedImage == mImageCache.end()) { // We failed creating the image if we got here, so bail out. + ALOGE("Failed to create an EGLImage when rendering"); bindExternalTextureImage(texName, *createImage()); return NO_INIT; } @@ -663,7 +688,18 @@ status_t GLESRenderEngine::bindExternalTextureBuffer(uint32_t texName, return NO_ERROR; } -status_t GLESRenderEngine::cacheExternalTextureBuffer(const sp& buffer) { +void GLESRenderEngine::cacheExternalTextureBuffer(const sp& buffer) { + mImageManager->cacheAsync(buffer, nullptr); +} + +std::shared_ptr GLESRenderEngine::cacheExternalTextureBufferForTesting( + const sp& buffer) { + auto barrier = std::make_shared(); + mImageManager->cacheAsync(buffer, barrier); + return barrier; +} + +status_t GLESRenderEngine::cacheExternalTextureBufferInternal(const sp& buffer) { if (buffer == nullptr) { return BAD_VALUE; } @@ -703,12 +739,30 @@ status_t GLESRenderEngine::cacheExternalTextureBuffer(const sp& b } void GLESRenderEngine::unbindExternalTextureBuffer(uint64_t bufferId) { - std::lock_guard lock(mRenderingMutex); - const auto& cachedImage = mImageCache.find(bufferId); - if (cachedImage != mImageCache.end()) { - ALOGV("Destroying image for buffer: %" PRIu64, bufferId); - mImageCache.erase(bufferId); - return; + mImageManager->releaseAsync(bufferId, nullptr); +} + +std::shared_ptr GLESRenderEngine::unbindExternalTextureBufferForTesting( + uint64_t bufferId) { + auto barrier = std::make_shared(); + mImageManager->releaseAsync(bufferId, barrier); + return barrier; +} + +void GLESRenderEngine::unbindExternalTextureBufferInternal(uint64_t bufferId) { + std::unique_ptr image; + { + std::lock_guard lock(mRenderingMutex); + const auto& cachedImage = mImageCache.find(bufferId); + + if (cachedImage != mImageCache.end()) { + ALOGV("Destroying image for buffer: %" PRIu64, bufferId); + // Move the buffer out of cache first, so that we can destroy + // without holding the cache's lock. + image = std::move(cachedImage->second); + mImageCache.erase(bufferId); + return; + } } ALOGV("Failed to find image for buffer: %" PRIu64, bufferId); } diff --git a/libs/renderengine/gl/GLESRenderEngine.h b/libs/renderengine/gl/GLESRenderEngine.h index a01162009b..dd60e50c67 100644 --- a/libs/renderengine/gl/GLESRenderEngine.h +++ b/libs/renderengine/gl/GLESRenderEngine.h @@ -17,9 +17,7 @@ #ifndef SF_GLESRENDERENGINE_H_ #define SF_GLESRENDERENGINE_H_ -#include #include -#include #include #include #include @@ -30,8 +28,11 @@ #include #include #include +#include #include #include +#include +#include "ImageManager.h" #define EGL_NO_CONFIG ((EGLConfig)0) @@ -74,7 +75,7 @@ public: void bindExternalTextureImage(uint32_t texName, const Image& image) override; status_t bindExternalTextureBuffer(uint32_t texName, const sp& buffer, const sp& fence) EXCLUDES(mRenderingMutex); - status_t cacheExternalTextureBuffer(const sp& buffer) EXCLUDES(mRenderingMutex); + void cacheExternalTextureBuffer(const sp& buffer) EXCLUDES(mRenderingMutex); void unbindExternalTextureBuffer(uint64_t bufferId) EXCLUDES(mRenderingMutex); status_t bindFrameBuffer(Framebuffer* framebuffer) override; void unbindFrameBuffer(Framebuffer* framebuffer) override; @@ -101,6 +102,11 @@ public: // Returns true iff mFramebufferImageCache contains an image keyed by bufferId bool isFramebufferImageCachedForTesting(uint64_t bufferId) EXCLUDES(mFramebufferImageCacheMutex); + // These are wrappers around public methods above, but exposing Barrier + // objects so that tests can block. + std::shared_ptr cacheExternalTextureBufferForTesting( + const sp& buffer); + std::shared_ptr unbindExternalTextureBufferForTesting(uint64_t bufferId); protected: Framebuffer* getFramebufferForDrawing() override; @@ -147,6 +153,9 @@ private: void setScissor(const Rect& region); void disableScissor(); bool waitSync(EGLSyncKHR sync, EGLint flags); + status_t cacheExternalTextureBufferInternal(const sp& buffer) + EXCLUDES(mRenderingMutex); + void unbindExternalTextureBufferInternal(uint64_t bufferId) EXCLUDES(mRenderingMutex); // A data space is considered HDR data space if it has BT2020 color space // with PQ or HLG transfer function. @@ -250,7 +259,9 @@ private: bool mRunning = true; }; friend class FlushTracer; + friend class ImageManager; std::unique_ptr mFlushTracer; + std::unique_ptr mImageManager = std::make_unique(this); }; } // namespace gl diff --git a/libs/renderengine/gl/ImageManager.cpp b/libs/renderengine/gl/ImageManager.cpp new file mode 100644 index 0000000000..5af0e4f857 --- /dev/null +++ b/libs/renderengine/gl/ImageManager.cpp @@ -0,0 +1,140 @@ +/* + * Copyright 2019 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. + */ + +#define ATRACE_TAG ATRACE_TAG_GRAPHICS + +#include + +#include +#include +#include "GLESRenderEngine.h" +#include "ImageManager.h" + +namespace android { +namespace renderengine { +namespace gl { + +ImageManager::ImageManager(GLESRenderEngine* engine) : mEngine(engine) { + pthread_setname_np(mThread.native_handle(), "ImageManager"); + // Use SCHED_FIFO to minimize jitter + struct sched_param param = {0}; + param.sched_priority = 2; + if (pthread_setschedparam(mThread.native_handle(), SCHED_FIFO, ¶m) != 0) { + ALOGE("Couldn't set SCHED_FIFO for ImageManager"); + } +} + +ImageManager::~ImageManager() { + { + std::lock_guard lock(mMutex); + mRunning = false; + } + mCondition.notify_all(); + if (mThread.joinable()) { + mThread.join(); + } +} + +void ImageManager::cacheAsync(const sp& buffer, + const std::shared_ptr& barrier) { + if (buffer == nullptr) { + { + std::lock_guard lock(barrier->mutex); + barrier->isOpen = true; + barrier->result = BAD_VALUE; + } + barrier->condition.notify_one(); + return; + } + ATRACE_CALL(); + QueueEntry entry = {QueueEntry::Operation::Insert, buffer, buffer->getId(), barrier}; + queueOperation(std::move(entry)); +} + +status_t ImageManager::cache(const sp& buffer) { + ATRACE_CALL(); + auto barrier = std::make_shared(); + cacheAsync(buffer, barrier); + std::lock_guard lock(barrier->mutex); + barrier->condition.wait(barrier->mutex, + [&]() REQUIRES(barrier->mutex) { return barrier->isOpen; }); + return barrier->result; +} + +void ImageManager::releaseAsync(uint64_t bufferId, const std::shared_ptr& barrier) { + ATRACE_CALL(); + QueueEntry entry = {QueueEntry::Operation::Delete, nullptr, bufferId, barrier}; + queueOperation(std::move(entry)); +} + +void ImageManager::queueOperation(const QueueEntry&& entry) { + { + std::lock_guard lock(mMutex); + mQueue.emplace(entry); + ATRACE_INT("ImageManagerQueueDepth", mQueue.size()); + } + mCondition.notify_one(); +} + +void ImageManager::threadMain() { + set_sched_policy(0, SP_FOREGROUND); + bool run; + { + std::lock_guard lock(mMutex); + run = mRunning; + } + while (run) { + QueueEntry entry; + { + std::lock_guard lock(mMutex); + mCondition.wait(mMutex, + [&]() REQUIRES(mMutex) { return !mQueue.empty() || !mRunning; }); + run = mRunning; + + if (!mRunning) { + // if mRunning is false, then ImageManager is being destroyed, so + // bail out now. + break; + } + + entry = mQueue.front(); + mQueue.pop(); + ATRACE_INT("ImageManagerQueueDepth", mQueue.size()); + } + + status_t result = NO_ERROR; + switch (entry.op) { + case QueueEntry::Operation::Delete: + mEngine->unbindExternalTextureBufferInternal(entry.bufferId); + break; + case QueueEntry::Operation::Insert: + result = mEngine->cacheExternalTextureBufferInternal(entry.buffer); + break; + } + if (entry.barrier != nullptr) { + { + std::lock_guard entryLock(entry.barrier->mutex); + entry.barrier->result = result; + entry.barrier->isOpen = true; + } + entry.barrier->condition.notify_one(); + } + } +} + +} // namespace gl +} // namespace renderengine +} // namespace android diff --git a/libs/renderengine/gl/ImageManager.h b/libs/renderengine/gl/ImageManager.h new file mode 100644 index 0000000000..b5ba554c4f --- /dev/null +++ b/libs/renderengine/gl/ImageManager.h @@ -0,0 +1,70 @@ +/* + * Copyright 2019 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. + */ + +#pragma once + +#include +#include +#include +#include + +#include + +namespace android { +namespace renderengine { +namespace gl { + +class GLESRenderEngine; + +class ImageManager { +public: + struct Barrier { + std::mutex mutex; + std::condition_variable_any condition; + bool isOpen GUARDED_BY(mutex) = false; + status_t result GUARDED_BY(mutex) = NO_ERROR; + }; + ImageManager(GLESRenderEngine* engine); + ~ImageManager(); + void cacheAsync(const sp& buffer, const std::shared_ptr& barrier) + EXCLUDES(mMutex); + status_t cache(const sp& buffer); + void releaseAsync(uint64_t bufferId, const std::shared_ptr& barrier) EXCLUDES(mMutex); + +private: + struct QueueEntry { + enum class Operation { Delete, Insert }; + + Operation op = Operation::Delete; + sp buffer = nullptr; + uint64_t bufferId = 0; + std::shared_ptr barrier = nullptr; + }; + + void queueOperation(const QueueEntry&& entry); + void threadMain(); + GLESRenderEngine* const mEngine; + std::thread mThread = std::thread([this]() { threadMain(); }); + std::condition_variable_any mCondition; + std::mutex mMutex; + std::queue mQueue GUARDED_BY(mMutex); + + bool mRunning GUARDED_BY(mMutex) = true; +}; + +} // namespace gl +} // namespace renderengine +} // namespace android diff --git a/libs/renderengine/include/renderengine/RenderEngine.h b/libs/renderengine/include/renderengine/RenderEngine.h index f92ccfbfc3..c6a7bd843a 100644 --- a/libs/renderengine/include/renderengine/RenderEngine.h +++ b/libs/renderengine/include/renderengine/RenderEngine.h @@ -116,10 +116,20 @@ public: const sp& fence) = 0; // Caches Image resources for this buffer, but does not bind the buffer to // a particular texture. - virtual status_t cacheExternalTextureBuffer(const sp& buffer) = 0; + // Note that work is deferred to an additional thread, i.e. this call + // is made asynchronously, but the caller can expect that cache/unbind calls + // are performed in a manner that's conflict serializable, i.e. unbinding + // a buffer should never occur before binding the buffer if the caller + // called {bind, cache}ExternalTextureBuffer before calling unbind. + virtual void cacheExternalTextureBuffer(const sp& buffer) = 0; // Removes internal resources referenced by the bufferId. This method should be // invoked when the caller will no longer hold a reference to a GraphicBuffer // and needs to clean up its resources. + // Note that work is deferred to an additional thread, i.e. this call + // is made asynchronously, but the caller can expect that cache/unbind calls + // are performed in a manner that's conflict serializable, i.e. unbinding + // a buffer should never occur before binding the buffer if the caller + // called {bind, cache}ExternalTextureBuffer before calling unbind. virtual void unbindExternalTextureBuffer(uint64_t bufferId) = 0; // When binding a native buffer, it must be done before setViewportAndProjection // Returns NO_ERROR when binds successfully, NO_MEMORY when there's no memory for allocation. diff --git a/libs/renderengine/include/renderengine/mock/RenderEngine.h b/libs/renderengine/include/renderengine/mock/RenderEngine.h index e33bcfd994..b4d3ef24ba 100644 --- a/libs/renderengine/include/renderengine/mock/RenderEngine.h +++ b/libs/renderengine/include/renderengine/mock/RenderEngine.h @@ -51,7 +51,7 @@ public: MOCK_METHOD2(genTextures, void(size_t, uint32_t*)); MOCK_METHOD2(deleteTextures, void(size_t, uint32_t const*)); MOCK_METHOD2(bindExternalTextureImage, void(uint32_t, const renderengine::Image&)); - MOCK_METHOD1(cacheExternalTextureBuffer, status_t(const sp&)); + MOCK_METHOD1(cacheExternalTextureBuffer, void(const sp&)); MOCK_METHOD3(bindExternalTextureBuffer, status_t(uint32_t, const sp&, const sp&)); MOCK_METHOD1(unbindExternalTextureBuffer, void(uint64_t)); diff --git a/libs/renderengine/tests/Android.bp b/libs/renderengine/tests/Android.bp index 9b483ef51d..e98babc30c 100644 --- a/libs/renderengine/tests/Android.bp +++ b/libs/renderengine/tests/Android.bp @@ -31,6 +31,7 @@ cc_test { "libgui", "liblog", "libnativewindow", + "libprocessgroup", "libsync", "libui", "libutils", diff --git a/libs/renderengine/tests/RenderEngineTest.cpp b/libs/renderengine/tests/RenderEngineTest.cpp index 7acaecf465..f47c7fd053 100644 --- a/libs/renderengine/tests/RenderEngineTest.cpp +++ b/libs/renderengine/tests/RenderEngineTest.cpp @@ -14,8 +14,10 @@ * limitations under the License. */ -#include +#include +#include +#include #include #include #include @@ -1001,8 +1003,15 @@ TEST_F(RenderEngineTest, drawLayers_fillsBufferAndCachesImages) { invokeDraw(settings, layers, mBuffer); uint64_t bufferId = layer.source.buffer.buffer->getId(); EXPECT_TRUE(sRE->isImageCachedForTesting(bufferId)); - sRE->unbindExternalTextureBuffer(bufferId); + std::shared_ptr barrier = + sRE->unbindExternalTextureBufferForTesting(bufferId); + std::lock_guard lock(barrier->mutex); + ASSERT_TRUE(barrier->condition.wait_for(barrier->mutex, std::chrono::seconds(5), + [&]() REQUIRES(barrier->mutex) { + return barrier->isOpen; + })); EXPECT_FALSE(sRE->isImageCachedForTesting(bufferId)); + EXPECT_EQ(NO_ERROR, barrier->result); } TEST_F(RenderEngineTest, drawLayers_bindExternalBufferWithNullBuffer) { @@ -1019,21 +1028,52 @@ TEST_F(RenderEngineTest, drawLayers_bindExternalBufferCachesImages) { sRE->bindExternalTextureBuffer(texName, buf, nullptr); uint64_t bufferId = buf->getId(); EXPECT_TRUE(sRE->isImageCachedForTesting(bufferId)); - sRE->unbindExternalTextureBuffer(bufferId); + std::shared_ptr barrier = + sRE->unbindExternalTextureBufferForTesting(bufferId); + std::lock_guard lock(barrier->mutex); + ASSERT_TRUE(barrier->condition.wait_for(barrier->mutex, std::chrono::seconds(5), + [&]() REQUIRES(barrier->mutex) { + return barrier->isOpen; + })); + EXPECT_EQ(NO_ERROR, barrier->result); EXPECT_FALSE(sRE->isImageCachedForTesting(bufferId)); } TEST_F(RenderEngineTest, drawLayers_cacheExternalBufferWithNullBuffer) { - status_t result = sRE->cacheExternalTextureBuffer(nullptr); - ASSERT_EQ(BAD_VALUE, result); + std::shared_ptr barrier = + sRE->cacheExternalTextureBufferForTesting(nullptr); + std::lock_guard lock(barrier->mutex); + ASSERT_TRUE(barrier->condition.wait_for(barrier->mutex, std::chrono::seconds(5), + [&]() REQUIRES(barrier->mutex) { + return barrier->isOpen; + })); + EXPECT_TRUE(barrier->isOpen); + EXPECT_EQ(BAD_VALUE, barrier->result); } TEST_F(RenderEngineTest, drawLayers_cacheExternalBufferCachesImages) { sp buf = allocateSourceBuffer(1, 1); uint64_t bufferId = buf->getId(); - sRE->cacheExternalTextureBuffer(buf); + std::shared_ptr barrier = + sRE->cacheExternalTextureBufferForTesting(buf); + { + std::lock_guard lock(barrier->mutex); + ASSERT_TRUE(barrier->condition.wait_for(barrier->mutex, std::chrono::seconds(5), + [&]() REQUIRES(barrier->mutex) { + return barrier->isOpen; + })); + EXPECT_EQ(NO_ERROR, barrier->result); + } EXPECT_TRUE(sRE->isImageCachedForTesting(bufferId)); - sRE->unbindExternalTextureBuffer(bufferId); + barrier = sRE->unbindExternalTextureBufferForTesting(bufferId); + { + std::lock_guard lock(barrier->mutex); + ASSERT_TRUE(barrier->condition.wait_for(barrier->mutex, std::chrono::seconds(5), + [&]() REQUIRES(barrier->mutex) { + return barrier->isOpen; + })); + EXPECT_EQ(NO_ERROR, barrier->result); + } EXPECT_FALSE(sRE->isImageCachedForTesting(bufferId)); } diff --git a/services/surfaceflinger/BufferLayerConsumer.cpp b/services/surfaceflinger/BufferLayerConsumer.cpp index bd9bd81e2a..414814a6f4 100644 --- a/services/surfaceflinger/BufferLayerConsumer.cpp +++ b/services/surfaceflinger/BufferLayerConsumer.cpp @@ -494,7 +494,6 @@ void BufferLayerConsumer::onBufferAvailable(const BufferItem& item) { if (oldImage == nullptr || oldImage->graphicBuffer() == nullptr || oldImage->graphicBuffer()->getId() != item.mGraphicBuffer->getId()) { mImages[item.mSlot] = std::make_shared(item.mGraphicBuffer, mRE); - mRE.cacheExternalTextureBuffer(item.mGraphicBuffer); } } } @@ -531,6 +530,12 @@ void BufferLayerConsumer::dumpLocked(String8& result, const char* prefix) const ConsumerBase::dumpLocked(result, prefix); } +BufferLayerConsumer::Image::Image(const sp& graphicBuffer, + renderengine::RenderEngine& engine) + : mGraphicBuffer(graphicBuffer), mRE(engine) { + mRE.cacheExternalTextureBuffer(mGraphicBuffer); +} + BufferLayerConsumer::Image::~Image() { if (mGraphicBuffer != nullptr) { ALOGV("Destroying buffer: %" PRId64, mGraphicBuffer->getId()); diff --git a/services/surfaceflinger/BufferLayerConsumer.h b/services/surfaceflinger/BufferLayerConsumer.h index 901556a53f..617b1c2323 100644 --- a/services/surfaceflinger/BufferLayerConsumer.h +++ b/services/surfaceflinger/BufferLayerConsumer.h @@ -223,8 +223,7 @@ private: // Utility class for managing GraphicBuffer references into renderengine class Image { public: - Image(sp graphicBuffer, renderengine::RenderEngine& engine) - : mGraphicBuffer(graphicBuffer), mRE(engine) {} + Image(const sp& graphicBuffer, renderengine::RenderEngine& engine); virtual ~Image(); const sp& graphicBuffer() { return mGraphicBuffer; } -- cgit v1.2.3-59-g8ed1b