Don't cache framebuffer images for screenshots.
Screenshots always create a new GraphicBuffer, so they shouldn't be
cached since they'll never be redrawn to.
Bug: 129072814
Test: systrace
Change-Id: I116444804b80fbdacf5b8f8ef23f943032b3b777
diff --git a/libs/renderengine/gl/GLESRenderEngine.cpp b/libs/renderengine/gl/GLESRenderEngine.cpp
index 1980f50..5847402 100644
--- a/libs/renderengine/gl/GLESRenderEngine.cpp
+++ b/libs/renderengine/gl/GLESRenderEngine.cpp
@@ -808,12 +808,14 @@
return success;
}
EGLImageKHR GLESRenderEngine::createFramebufferImageIfNeeded(ANativeWindowBuffer* nativeBuffer,
- bool isProtected) {
+ bool isProtected,
+ bool useFramebufferCache) {
sp<GraphicBuffer> graphicBuffer = GraphicBuffer::from(nativeBuffer);
- uint64_t bufferId = graphicBuffer->getId();
- for (const auto& image : mFramebufferImageCache) {
- if (image.first == bufferId) {
- return image.second;
+ if (useFramebufferCache) {
+ for (const auto& image : mFramebufferImageCache) {
+ if (image.first == graphicBuffer->getId()) {
+ return image.second;
+ }
}
}
EGLint attributes[] = {
@@ -823,13 +825,15 @@
};
EGLImageKHR image = eglCreateImageKHR(mEGLDisplay, EGL_NO_CONTEXT, EGL_NATIVE_BUFFER_ANDROID,
nativeBuffer, attributes);
- if (image != EGL_NO_IMAGE_KHR) {
- if (mFramebufferImageCache.size() >= mFramebufferImageCacheSize) {
- EGLImageKHR expired = mFramebufferImageCache.front().second;
- mFramebufferImageCache.pop_front();
- eglDestroyImageKHR(mEGLDisplay, expired);
+ if (useFramebufferCache) {
+ if (image != EGL_NO_IMAGE_KHR) {
+ if (mFramebufferImageCache.size() >= mFramebufferImageCacheSize) {
+ EGLImageKHR expired = mFramebufferImageCache.front().second;
+ mFramebufferImageCache.pop_front();
+ eglDestroyImageKHR(mEGLDisplay, expired);
+ }
+ mFramebufferImageCache.push_back({graphicBuffer->getId(), image});
}
- mFramebufferImageCache.push_back({bufferId, image});
}
return image;
}
@@ -837,7 +841,8 @@
status_t GLESRenderEngine::drawLayers(const DisplaySettings& display,
const std::vector<LayerSettings>& layers,
ANativeWindowBuffer* const buffer,
- base::unique_fd&& bufferFence, base::unique_fd* drawFence) {
+ const bool useFramebufferCache, base::unique_fd&& bufferFence,
+ base::unique_fd* drawFence) {
ATRACE_CALL();
if (layers.empty()) {
ALOGV("Drawing empty layer stack");
@@ -857,7 +862,7 @@
{
std::lock_guard<std::mutex> lock(mRenderingMutex);
- BindNativeBufferAsFramebuffer fbo(*this, buffer);
+ BindNativeBufferAsFramebuffer fbo(*this, buffer, useFramebufferCache);
if (fbo.getStatus() != NO_ERROR) {
ALOGE("Failed to bind framebuffer! Aborting GPU composition for buffer (%p).",
diff --git a/libs/renderengine/gl/GLESRenderEngine.h b/libs/renderengine/gl/GLESRenderEngine.h
index 8c8f308..cbde5f4 100644
--- a/libs/renderengine/gl/GLESRenderEngine.h
+++ b/libs/renderengine/gl/GLESRenderEngine.h
@@ -83,14 +83,16 @@
bool supportsProtectedContent() const override;
bool useProtectedContext(bool useProtectedContext) override;
status_t drawLayers(const DisplaySettings& display, const std::vector<LayerSettings>& layers,
- ANativeWindowBuffer* buffer, base::unique_fd&& bufferFence,
- base::unique_fd* drawFence) EXCLUDES(mRenderingMutex) override;
+ ANativeWindowBuffer* buffer, const bool useFramebufferCache,
+ base::unique_fd&& bufferFence, base::unique_fd* drawFence)
+ EXCLUDES(mRenderingMutex) override;
// internal to RenderEngine
EGLDisplay getEGLDisplay() const { return mEGLDisplay; }
EGLConfig getEGLConfig() const { return mEGLConfig; }
// Creates an output image for rendering to
- EGLImageKHR createFramebufferImageIfNeeded(ANativeWindowBuffer* nativeBuffer, bool isProtected);
+ EGLImageKHR createFramebufferImageIfNeeded(ANativeWindowBuffer* nativeBuffer, bool isProtected,
+ bool useFramebufferCache);
// Test-only methods
// Returns true iff mImageCache contains an image keyed by bufferId
diff --git a/libs/renderengine/gl/GLFramebuffer.cpp b/libs/renderengine/gl/GLFramebuffer.cpp
index c45598c..dacf8d3 100644
--- a/libs/renderengine/gl/GLFramebuffer.cpp
+++ b/libs/renderengine/gl/GLFramebuffer.cpp
@@ -41,19 +41,25 @@
glDeleteTextures(1, &mTextureName);
}
-bool GLFramebuffer::setNativeWindowBuffer(ANativeWindowBuffer* nativeBuffer, bool isProtected) {
+bool GLFramebuffer::setNativeWindowBuffer(ANativeWindowBuffer* nativeBuffer, bool isProtected,
+ const bool useFramebufferCache) {
ATRACE_CALL();
if (mEGLImage != EGL_NO_IMAGE_KHR) {
+ if (!usingFramebufferCache) {
+ eglDestroyImageKHR(mEGLDisplay, mEGLImage);
+ }
mEGLImage = EGL_NO_IMAGE_KHR;
mBufferWidth = 0;
mBufferHeight = 0;
}
if (nativeBuffer) {
- mEGLImage = mEngine.createFramebufferImageIfNeeded(nativeBuffer, isProtected);
+ mEGLImage = mEngine.createFramebufferImageIfNeeded(nativeBuffer, isProtected,
+ useFramebufferCache);
if (mEGLImage == EGL_NO_IMAGE_KHR) {
return false;
}
+ usingFramebufferCache = useFramebufferCache;
mBufferWidth = nativeBuffer->width;
mBufferHeight = nativeBuffer->height;
}
diff --git a/libs/renderengine/gl/GLFramebuffer.h b/libs/renderengine/gl/GLFramebuffer.h
index 1289fbf..b7650bb 100644
--- a/libs/renderengine/gl/GLFramebuffer.h
+++ b/libs/renderengine/gl/GLFramebuffer.h
@@ -35,7 +35,8 @@
explicit GLFramebuffer(GLESRenderEngine& engine);
~GLFramebuffer() override;
- bool setNativeWindowBuffer(ANativeWindowBuffer* nativeBuffer, bool isProtected) override;
+ bool setNativeWindowBuffer(ANativeWindowBuffer* nativeBuffer, bool isProtected,
+ const bool useFramebufferCache) override;
EGLImageKHR getEGLImage() const { return mEGLImage; }
uint32_t getTextureName() const { return mTextureName; }
uint32_t getFramebufferName() const { return mFramebufferName; }
@@ -46,6 +47,7 @@
GLESRenderEngine& mEngine;
EGLDisplay mEGLDisplay;
EGLImageKHR mEGLImage;
+ bool usingFramebufferCache = false;
uint32_t mTextureName, mFramebufferName;
int32_t mBufferHeight = 0;
diff --git a/libs/renderengine/include/renderengine/Framebuffer.h b/libs/renderengine/include/renderengine/Framebuffer.h
index 66eb9ef..6511127 100644
--- a/libs/renderengine/include/renderengine/Framebuffer.h
+++ b/libs/renderengine/include/renderengine/Framebuffer.h
@@ -27,7 +27,8 @@
public:
virtual ~Framebuffer() = default;
- virtual bool setNativeWindowBuffer(ANativeWindowBuffer* nativeBuffer, bool isProtected) = 0;
+ virtual bool setNativeWindowBuffer(ANativeWindowBuffer* nativeBuffer, bool isProtected,
+ const bool useFramebufferCache) = 0;
};
} // namespace renderengine
diff --git a/libs/renderengine/include/renderengine/RenderEngine.h b/libs/renderengine/include/renderengine/RenderEngine.h
index b211551..fa57215 100644
--- a/libs/renderengine/include/renderengine/RenderEngine.h
+++ b/libs/renderengine/include/renderengine/RenderEngine.h
@@ -170,6 +170,9 @@
// @param layers The layers to draw onto the display, in Z-order.
// @param buffer The buffer which will be drawn to. This buffer will be
// ready once drawFence fires.
+ // @param useFramebufferCache True if the framebuffer cache should be used.
+ // If an implementation does not cache output framebuffers, then this
+ // parameter does nothing.
// @param bufferFence Fence signalling that the buffer is ready to be drawn
// to.
// @param drawFence A pointer to a fence, which will fire when the buffer
@@ -180,8 +183,8 @@
// now, this always returns NO_ERROR.
virtual status_t drawLayers(const DisplaySettings& display,
const std::vector<LayerSettings>& layers,
- ANativeWindowBuffer* buffer, base::unique_fd&& bufferFence,
- base::unique_fd* drawFence) = 0;
+ ANativeWindowBuffer* buffer, const bool useFramebufferCache,
+ base::unique_fd&& bufferFence, base::unique_fd* drawFence) = 0;
protected:
// Gets a framebuffer to render to. This framebuffer may or may not be
@@ -195,14 +198,16 @@
class BindNativeBufferAsFramebuffer {
public:
- BindNativeBufferAsFramebuffer(RenderEngine& engine, ANativeWindowBuffer* buffer)
+ BindNativeBufferAsFramebuffer(RenderEngine& engine, ANativeWindowBuffer* buffer,
+ const bool useFramebufferCache)
: mEngine(engine), mFramebuffer(mEngine.getFramebufferForDrawing()), mStatus(NO_ERROR) {
- mStatus = mFramebuffer->setNativeWindowBuffer(buffer, mEngine.isProtected())
+ mStatus = mFramebuffer->setNativeWindowBuffer(buffer, mEngine.isProtected(),
+ useFramebufferCache)
? mEngine.bindFrameBuffer(mFramebuffer)
: NO_MEMORY;
}
~BindNativeBufferAsFramebuffer() {
- mFramebuffer->setNativeWindowBuffer(nullptr, false);
+ mFramebuffer->setNativeWindowBuffer(nullptr, false, /*arbitrary*/ true);
mEngine.unbindFrameBuffer(mFramebuffer);
}
status_t getStatus() const { return mStatus; }
diff --git a/libs/renderengine/include/renderengine/mock/Framebuffer.h b/libs/renderengine/include/renderengine/mock/Framebuffer.h
index 7695885..dfb6a4e 100644
--- a/libs/renderengine/include/renderengine/mock/Framebuffer.h
+++ b/libs/renderengine/include/renderengine/mock/Framebuffer.h
@@ -28,7 +28,7 @@
Framebuffer();
~Framebuffer() override;
- MOCK_METHOD2(setNativeWindowBuffer, bool(ANativeWindowBuffer*, bool));
+ MOCK_METHOD3(setNativeWindowBuffer, bool(ANativeWindowBuffer*, bool, const bool));
};
} // namespace mock
diff --git a/libs/renderengine/include/renderengine/mock/RenderEngine.h b/libs/renderengine/include/renderengine/mock/RenderEngine.h
index 479c7ac..7393f7a 100644
--- a/libs/renderengine/include/renderengine/mock/RenderEngine.h
+++ b/libs/renderengine/include/renderengine/mock/RenderEngine.h
@@ -77,9 +77,9 @@
MOCK_CONST_METHOD0(isProtected, bool());
MOCK_CONST_METHOD0(supportsProtectedContent, bool());
MOCK_METHOD1(useProtectedContext, bool(bool));
- MOCK_METHOD5(drawLayers,
+ MOCK_METHOD6(drawLayers,
status_t(const DisplaySettings&, const std::vector<LayerSettings>&,
- ANativeWindowBuffer*, base::unique_fd&&, base::unique_fd*));
+ ANativeWindowBuffer*, const bool, base::unique_fd&&, base::unique_fd*));
};
} // namespace mock
diff --git a/libs/renderengine/tests/RenderEngineTest.cpp b/libs/renderengine/tests/RenderEngineTest.cpp
index 8c93cf4..351ca6b 100644
--- a/libs/renderengine/tests/RenderEngineTest.cpp
+++ b/libs/renderengine/tests/RenderEngineTest.cpp
@@ -118,7 +118,7 @@
void invokeDraw(renderengine::DisplaySettings settings,
std::vector<renderengine::LayerSettings> layers, sp<GraphicBuffer> buffer) {
base::unique_fd fence;
- status_t status = sRE->drawLayers(settings, layers, buffer->getNativeBuffer(),
+ status_t status = sRE->drawLayers(settings, layers, buffer->getNativeBuffer(), true,
base::unique_fd(), &fence);
sCurrentBuffer = buffer;
@@ -770,7 +770,7 @@
BufferSourceVariant<ForceOpaqueBufferVariant>::fillColor(layer, 1.0f, 0.0f, 0.0f, this);
layers.push_back(layer);
base::unique_fd fence;
- status_t status = sRE->drawLayers(settings, layers, nullptr, base::unique_fd(), &fence);
+ status_t status = sRE->drawLayers(settings, layers, nullptr, true, base::unique_fd(), &fence);
ASSERT_EQ(BAD_VALUE, status);
}
@@ -787,13 +787,33 @@
layer.alpha = 1.0;
layers.push_back(layer);
- status_t status = sRE->drawLayers(settings, layers, mBuffer->getNativeBuffer(),
+ status_t status = sRE->drawLayers(settings, layers, mBuffer->getNativeBuffer(), true,
base::unique_fd(), nullptr);
sCurrentBuffer = mBuffer;
ASSERT_EQ(NO_ERROR, status);
expectBufferColor(fullscreenRect(), 255, 0, 0, 255);
}
+TEST_F(RenderEngineTest, drawLayers_doesNotCacheFramebuffer) {
+ renderengine::DisplaySettings settings;
+ settings.physicalDisplay = fullscreenRect();
+ settings.clip = fullscreenRect();
+
+ std::vector<renderengine::LayerSettings> layers;
+ renderengine::LayerSettings layer;
+ layer.geometry.boundaries = fullscreenRect().toFloatRect();
+ BufferSourceVariant<ForceOpaqueBufferVariant>::fillColor(layer, 1.0f, 0.0f, 0.0f, this);
+ layer.alpha = 1.0;
+ layers.push_back(layer);
+
+ status_t status = sRE->drawLayers(settings, layers, mBuffer->getNativeBuffer(), false,
+ base::unique_fd(), nullptr);
+ sCurrentBuffer = mBuffer;
+ ASSERT_EQ(NO_ERROR, status);
+ ASSERT_FALSE(sRE->isFramebufferImageCachedForTesting(mBuffer->getId()));
+ expectBufferColor(fullscreenRect(), 255, 0, 0, 255);
+}
+
TEST_F(RenderEngineTest, drawLayers_fillRedBuffer_colorSource) {
fillRedBuffer<ColorSourceVariant>();
}
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index a5f025d..916e531 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3436,7 +3436,8 @@
}
}
renderEngine.drawLayers(clientCompositionDisplay, clientCompositionLayers,
- buf->getNativeBuffer(), std::move(fd), readyFence);
+ buf->getNativeBuffer(), /*useFramebufferCache=*/true, std::move(fd),
+ readyFence);
if (expensiveRenderingExpected && displayId) {
mPowerAdvisor.setExpensiveRenderingExpected(*displayId, false);
}
@@ -5722,7 +5723,7 @@
base::unique_fd drawFence;
getRenderEngine().useProtectedContext(false);
getRenderEngine().drawLayers(clientCompositionDisplay, clientCompositionLayers, buffer,
- std::move(bufferFence), &drawFence);
+ /*useFramebufferCache=*/false, std::move(bufferFence), &drawFence);
*outSyncFd = drawFence.release();
}
diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
index ea2818d..ef3dfef 100644
--- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
@@ -175,7 +175,6 @@
renderengine::mock::RenderEngine* mRenderEngine = new renderengine::mock::RenderEngine();
mock::MessageQueue* mMessageQueue = new mock::MessageQueue();
mock::DispSync* mPrimaryDispSync = new mock::DispSync();
- renderengine::mock::Framebuffer* mReFrameBuffer = new renderengine::mock::Framebuffer();
sp<Fence> mClientTargetAcquireFence = Fence::NO_FENCE;
@@ -312,8 +311,8 @@
EXPECT_CALL(*test->mRenderEngine, drawLayers)
.WillRepeatedly(
[](const renderengine::DisplaySettings& displaySettings,
- const std::vector<renderengine::LayerSettings>& /*layerSettings*/,
- ANativeWindowBuffer*, base::unique_fd&&, base::unique_fd*) -> status_t {
+ const std::vector<renderengine::LayerSettings>&, ANativeWindowBuffer*,
+ const bool, base::unique_fd&&, base::unique_fd*) -> status_t {
EXPECT_EQ(DEFAULT_DISPLAY_MAX_LUMINANCE, displaySettings.maxLuminance);
EXPECT_EQ(Rect(DEFAULT_DISPLAY_WIDTH, DEFAULT_DISPLAY_HEIGHT),
displaySettings.physicalDisplay);
@@ -351,8 +350,8 @@
EXPECT_CALL(*test->mRenderEngine, drawLayers)
.WillRepeatedly(
[](const renderengine::DisplaySettings& displaySettings,
- const std::vector<renderengine::LayerSettings>& /*layerSettings*/,
- ANativeWindowBuffer*, base::unique_fd&&, base::unique_fd*) -> status_t {
+ const std::vector<renderengine::LayerSettings>&, ANativeWindowBuffer*,
+ const bool, base::unique_fd&&, base::unique_fd*) -> status_t {
EXPECT_EQ(DEFAULT_DISPLAY_MAX_LUMINANCE, displaySettings.maxLuminance);
EXPECT_EQ(Rect(DEFAULT_DISPLAY_WIDTH, DEFAULT_DISPLAY_HEIGHT),
displaySettings.physicalDisplay);
@@ -581,7 +580,7 @@
EXPECT_CALL(*test->mRenderEngine, drawLayers)
.WillOnce([](const renderengine::DisplaySettings& displaySettings,
const std::vector<renderengine::LayerSettings>& layerSettings,
- ANativeWindowBuffer*, base::unique_fd&&,
+ ANativeWindowBuffer*, const bool, base::unique_fd&&,
base::unique_fd*) -> status_t {
EXPECT_EQ(DEFAULT_DISPLAY_MAX_LUMINANCE, displaySettings.maxLuminance);
EXPECT_EQ(Rect(DEFAULT_DISPLAY_WIDTH, DEFAULT_DISPLAY_HEIGHT),
@@ -624,7 +623,7 @@
EXPECT_CALL(*test->mRenderEngine, drawLayers)
.WillOnce([](const renderengine::DisplaySettings& displaySettings,
const std::vector<renderengine::LayerSettings>& layerSettings,
- ANativeWindowBuffer*, base::unique_fd&&,
+ ANativeWindowBuffer*, const bool, base::unique_fd&&,
base::unique_fd*) -> status_t {
EXPECT_EQ(DEFAULT_DISPLAY_MAX_LUMINANCE, displaySettings.maxLuminance);
EXPECT_EQ(Rect(DEFAULT_DISPLAY_WIDTH, DEFAULT_DISPLAY_HEIGHT),
@@ -694,7 +693,7 @@
EXPECT_CALL(*test->mRenderEngine, drawLayers)
.WillOnce([](const renderengine::DisplaySettings& displaySettings,
const std::vector<renderengine::LayerSettings>& layerSettings,
- ANativeWindowBuffer*, base::unique_fd&&,
+ ANativeWindowBuffer*, const bool, base::unique_fd&&,
base::unique_fd*) -> status_t {
EXPECT_EQ(DEFAULT_DISPLAY_MAX_LUMINANCE, displaySettings.maxLuminance);
EXPECT_EQ(Rect(DEFAULT_DISPLAY_WIDTH, DEFAULT_DISPLAY_HEIGHT),