Plumb fence from dequeueBuffer into renderengine
This defers blocking from the ui thread, to the gpu driver, so
SurfaceFlinger can continue work without waiting on an old frame.
Bug: 123107664
Test: manual tests
Change-Id: Ied4ba84dd3fe63c65470ae3396dec0cb667a5ff0
diff --git a/libs/renderengine/gl/GLESRenderEngine.cpp b/libs/renderengine/gl/GLESRenderEngine.cpp
index 8f9071e..e7ff9ab 100644
--- a/libs/renderengine/gl/GLESRenderEngine.cpp
+++ b/libs/renderengine/gl/GLESRenderEngine.cpp
@@ -789,13 +789,18 @@
status_t GLESRenderEngine::drawLayers(const DisplaySettings& display,
const std::vector<LayerSettings>& layers,
ANativeWindowBuffer* const buffer,
- base::unique_fd* drawFence) {
+ base::unique_fd&& bufferFence, base::unique_fd* drawFence) {
ATRACE_CALL();
if (layers.empty()) {
ALOGV("Drawing empty layer stack");
return NO_ERROR;
}
+ if (bufferFence.get() >= 0 && !waitFence(std::move(bufferFence))) {
+ ATRACE_NAME("Waiting before draw");
+ sync_wait(bufferFence.get(), -1);
+ }
+
BindNativeBufferAsFramebuffer fbo(*this, buffer);
if (fbo.getStatus() != NO_ERROR) {
diff --git a/libs/renderengine/gl/GLESRenderEngine.h b/libs/renderengine/gl/GLESRenderEngine.h
index 7b72666..a86b4f5 100644
--- a/libs/renderengine/gl/GLESRenderEngine.h
+++ b/libs/renderengine/gl/GLESRenderEngine.h
@@ -81,7 +81,8 @@
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* drawFence) override;
+ ANativeWindowBuffer* buffer, base::unique_fd&& bufferFence,
+ base::unique_fd* drawFence) override;
// internal to RenderEngine
EGLDisplay getEGLDisplay() const { return mEGLDisplay; }
diff --git a/libs/renderengine/include/renderengine/RenderEngine.h b/libs/renderengine/include/renderengine/RenderEngine.h
index b51ed22..812d761 100644
--- a/libs/renderengine/include/renderengine/RenderEngine.h
+++ b/libs/renderengine/include/renderengine/RenderEngine.h
@@ -167,7 +167,9 @@
// drawing any layers.
// @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 displayFence fires.
+ // ready once drawFence fires.
+ // @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
// has been drawn to and is ready to be examined. The fence will be
// initialized by this method. The caller will be responsible for owning the
@@ -176,7 +178,8 @@
// now, this always returns NO_ERROR.
virtual status_t drawLayers(const DisplaySettings& display,
const std::vector<LayerSettings>& layers,
- ANativeWindowBuffer* buffer, base::unique_fd* drawFence) = 0;
+ ANativeWindowBuffer* buffer, base::unique_fd&& bufferFence,
+ base::unique_fd* drawFence) = 0;
protected:
// Gets a framebuffer to render to. This framebuffer may or may not be
diff --git a/libs/renderengine/include/renderengine/mock/RenderEngine.h b/libs/renderengine/include/renderengine/mock/RenderEngine.h
index 800eac3..5956c46 100644
--- a/libs/renderengine/include/renderengine/mock/RenderEngine.h
+++ b/libs/renderengine/include/renderengine/mock/RenderEngine.h
@@ -78,9 +78,9 @@
MOCK_CONST_METHOD0(isProtected, bool());
MOCK_CONST_METHOD0(supportsProtectedContent, bool());
MOCK_METHOD1(useProtectedContext, bool(bool));
- MOCK_METHOD4(drawLayers,
+ MOCK_METHOD5(drawLayers,
status_t(const DisplaySettings&, const std::vector<LayerSettings>&,
- ANativeWindowBuffer*, base::unique_fd*));
+ ANativeWindowBuffer*, base::unique_fd&&, base::unique_fd*));
};
} // namespace mock
diff --git a/libs/renderengine/tests/RenderEngineTest.cpp b/libs/renderengine/tests/RenderEngineTest.cpp
index f82beeb..f28f672 100644
--- a/libs/renderengine/tests/RenderEngineTest.cpp
+++ b/libs/renderengine/tests/RenderEngineTest.cpp
@@ -105,7 +105,8 @@
std::vector<renderengine::LayerSettings> layers,
sp<GraphicBuffer> buffer) {
base::unique_fd fence;
- status_t status = sRE->drawLayers(settings, layers, buffer->getNativeBuffer(), &fence);
+ status_t status = sRE->drawLayers(settings, layers, buffer->getNativeBuffer(),
+ base::unique_fd(), &fence);
int fd = fence.release();
if (fd >= 0) {
diff --git a/libs/ui/GraphicBuffer.cpp b/libs/ui/GraphicBuffer.cpp
index f487dfa..79958ec 100644
--- a/libs/ui/GraphicBuffer.cpp
+++ b/libs/ui/GraphicBuffer.cpp
@@ -15,6 +15,7 @@
*/
#define LOG_TAG "GraphicBuffer"
+#define ATRACE_TAG ATRACE_TAG_GRAPHICS
#include <ui/GraphicBuffer.h>
@@ -29,6 +30,7 @@
#include <ui/Gralloc2.h>
#include <ui/GraphicBufferAllocator.h>
#include <ui/GraphicBufferMapper.h>
+#include <utils/Trace.h>
namespace android {
@@ -111,6 +113,7 @@
GraphicBuffer::~GraphicBuffer()
{
+ ATRACE_CALL();
if (handle) {
free_handle();
}
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/RenderSurface.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/RenderSurface.h
index 2009380..e69b99f 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/RenderSurface.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/RenderSurface.h
@@ -71,7 +71,7 @@
virtual status_t prepareFrame() = 0;
// Allocates a buffer as scratch space for GPU composition
- virtual sp<GraphicBuffer> dequeueBuffer() = 0;
+ virtual sp<GraphicBuffer> dequeueBuffer(base::unique_fd* bufferFence) = 0;
// Queues the drawn buffer for consumption by HWC. readyFence is the fence
// which will fire when the buffer is ready for consumption.
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/RenderSurface.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/RenderSurface.h
index 58b13ed..3c79084 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/RenderSurface.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/RenderSurface.h
@@ -52,7 +52,7 @@
void setProtected(bool useProtected) override;
status_t beginFrame(bool mustRecompose) override;
status_t prepareFrame() override;
- sp<GraphicBuffer> dequeueBuffer() override;
+ sp<GraphicBuffer> dequeueBuffer(base::unique_fd* bufferFence) override;
void queueBuffer(base::unique_fd&& readyFence) override;
void onPresentDisplayCompleted() override;
void setViewportAndProjection() override;
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/RenderSurface.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/RenderSurface.h
index 8442bef..1562f58 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/RenderSurface.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/RenderSurface.h
@@ -37,7 +37,7 @@
MOCK_METHOD1(setBufferDataspace, void(ui::Dataspace));
MOCK_METHOD1(beginFrame, status_t(bool mustRecompose));
MOCK_METHOD0(prepareFrame, status_t());
- MOCK_METHOD0(dequeueBuffer, sp<GraphicBuffer>());
+ MOCK_METHOD1(dequeueBuffer, sp<GraphicBuffer>(base::unique_fd*));
MOCK_METHOD1(queueBuffer, void(base::unique_fd&&));
MOCK_METHOD0(onPresentDisplayCompleted, void());
MOCK_METHOD0(setViewportAndProjection, void());
diff --git a/services/surfaceflinger/CompositionEngine/src/RenderSurface.cpp b/services/surfaceflinger/CompositionEngine/src/RenderSurface.cpp
index ebb1bc2..b4dfba1 100644
--- a/services/surfaceflinger/CompositionEngine/src/RenderSurface.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/RenderSurface.cpp
@@ -14,6 +14,8 @@
* limitations under the License.
*/
+#define ATRACE_TAG ATRACE_TAG_GRAPHICS
+
#include <android-base/stringprintf.h>
#include <android/native_window.h>
#include <compositionengine/CompositionEngine.h>
@@ -28,6 +30,7 @@
#include <system/window.h>
#include <ui/GraphicBuffer.h>
#include <ui/Rect.h>
+#include <utils/Trace.h>
#include "DisplayHardware/HWComposer.h"
@@ -127,7 +130,8 @@
return mDisplaySurface->prepareFrame(compositionType);
}
-sp<GraphicBuffer> RenderSurface::dequeueBuffer() {
+sp<GraphicBuffer> RenderSurface::dequeueBuffer(base::unique_fd* bufferFence) {
+ ATRACE_CALL();
int fd = -1;
ANativeWindowBuffer* buffer = nullptr;
@@ -145,13 +149,7 @@
mGraphicBuffer->getNativeBuffer()->handle);
mGraphicBuffer = GraphicBuffer::from(buffer);
- // Block until the buffer is ready
- // TODO(alecmouri): it's perhaps more appropriate to block renderengine so
- // that the gl driver can block instead.
- if (fd >= 0) {
- sync_wait(fd, -1);
- close(fd);
- }
+ *bufferFence = base::unique_fd(fd);
return mGraphicBuffer;
}
@@ -172,7 +170,8 @@
// We shouldn't deadlock here, since mGraphicBuffer == nullptr only
// after a successful call to queueBuffer, or if dequeueBuffer has
// never been called.
- dequeueBuffer();
+ base::unique_fd unused;
+ dequeueBuffer(&unused);
}
if (mGraphicBuffer == nullptr) {
diff --git a/services/surfaceflinger/CompositionEngine/tests/RenderSurfaceTest.cpp b/services/surfaceflinger/CompositionEngine/tests/RenderSurfaceTest.cpp
index 0a7c462..84af9b9 100644
--- a/services/surfaceflinger/CompositionEngine/tests/RenderSurfaceTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/RenderSurfaceTest.cpp
@@ -358,7 +358,8 @@
.WillOnce(
DoAll(SetArgPointee<0>(buffer.get()), SetArgPointee<1>(-1), Return(NO_ERROR)));
- EXPECT_EQ(buffer.get(), mSurface.dequeueBuffer().get());
+ base::unique_fd fence;
+ EXPECT_EQ(buffer.get(), mSurface.dequeueBuffer(&fence).get());
EXPECT_EQ(buffer.get(), mSurface.mutableGraphicBufferForTest().get());
}
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index adc4a61..c624371 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3323,12 +3323,12 @@
renderengine::DisplaySettings clientCompositionDisplay;
std::vector<renderengine::LayerSettings> clientCompositionLayers;
sp<GraphicBuffer> buf;
+ base::unique_fd fd;
if (hasClientComposition) {
ALOGV("hasClientComposition");
- buf = display->getRenderSurface()->dequeueBuffer();
-
+ buf = display->getRenderSurface()->dequeueBuffer(&fd);
if (buf == nullptr) {
ALOGW("Dequeuing buffer for display [%s] failed, bailing out of "
"client composition for this frame",
@@ -3349,7 +3349,6 @@
m[3][0] = displayTransform[2][0];
m[3][1] = displayTransform[2][1];
m[3][3] = displayTransform[2][2];
-
clientCompositionDisplay.globalTransform = m;
const auto* profile = display->getDisplayColorProfile();
@@ -3449,7 +3448,7 @@
}
}
getRenderEngine().drawLayers(clientCompositionDisplay, clientCompositionLayers,
- buf->getNativeBuffer(), readyFence);
+ buf->getNativeBuffer(), std::move(fd), readyFence);
}
return true;
}
@@ -5626,9 +5625,12 @@
});
clientCompositionDisplay.clearRegion = clearRegion;
+ // Use an empty fence for the buffer fence, since we just created the buffer so
+ // there is no need for synchronization with the GPU.
+ base::unique_fd bufferFence;
base::unique_fd drawFence;
getRenderEngine().drawLayers(clientCompositionDisplay, clientCompositionLayers, buffer,
- &drawFence);
+ std::move(bufferFence), &drawFence);
*outSyncFd = drawFence.release();
}
diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
index d0f7b90..6deec29 100644
--- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
@@ -312,7 +312,7 @@
.WillRepeatedly(
[](const renderengine::DisplaySettings& displaySettings,
const std::vector<renderengine::LayerSettings>& /*layerSettings*/,
- ANativeWindowBuffer*, base::unique_fd*) -> status_t {
+ ANativeWindowBuffer*, 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,7 +351,7 @@
.WillRepeatedly(
[](const renderengine::DisplaySettings& displaySettings,
const std::vector<renderengine::LayerSettings>& /*layerSettings*/,
- ANativeWindowBuffer*, base::unique_fd*) -> status_t {
+ ANativeWindowBuffer*, 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);
@@ -580,7 +580,8 @@
EXPECT_CALL(*test->mRenderEngine, drawLayers)
.WillOnce([](const renderengine::DisplaySettings& displaySettings,
const std::vector<renderengine::LayerSettings>& layerSettings,
- ANativeWindowBuffer*, base::unique_fd*) -> status_t {
+ ANativeWindowBuffer*, 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);
@@ -624,7 +625,8 @@
EXPECT_CALL(*test->mRenderEngine, drawLayers)
.WillOnce([](const renderengine::DisplaySettings& displaySettings,
const std::vector<renderengine::LayerSettings>& layerSettings,
- ANativeWindowBuffer*, base::unique_fd*) -> status_t {
+ ANativeWindowBuffer*, 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);
@@ -693,7 +695,8 @@
EXPECT_CALL(*test->mRenderEngine, drawLayers)
.WillOnce([](const renderengine::DisplaySettings& displaySettings,
const std::vector<renderengine::LayerSettings>& layerSettings,
- ANativeWindowBuffer*, base::unique_fd*) -> status_t {
+ ANativeWindowBuffer*, 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);