[sf] Release the currently presented buffer when setBuffer is called
with null
Fixes a regression introduced in T which ignores a setBuffer call
with a null buffer. The expected behavior should be to release the
currently presented buffer from surfaceflinger. The subsequent frame
will not present this layer so the region behind the layer will be
composited instead.
Bug: 241271897
Test: presubmit
Change-Id: Ie06025c59c58cc75a267b783729996a3cbceef45
diff --git a/services/surfaceflinger/tests/LayerCallback_test.cpp b/services/surfaceflinger/tests/LayerCallback_test.cpp
index 26dbc76..79886bd 100644
--- a/services/surfaceflinger/tests/LayerCallback_test.cpp
+++ b/services/surfaceflinger/tests/LayerCallback_test.cpp
@@ -1224,4 +1224,75 @@
EXPECT_NO_FATAL_FAILURE(waitForCallback(callback, expected, true));
}
+TEST_F(LayerCallbackTest, SetNullBuffer) {
+ sp<SurfaceControl> layer;
+ ASSERT_NO_FATAL_FAILURE(layer = createLayerWithBuffer());
+
+ Transaction transaction;
+ CallbackHelper callback;
+ int err = fillTransaction(transaction, &callback, layer, /*setBuffer=*/true,
+ /*setBackgroundColor=*/false);
+ if (err) {
+ GTEST_SUCCEED() << "test not supported";
+ return;
+ }
+ transaction.apply();
+
+ {
+ ExpectedResult expected;
+ expected.addSurface(ExpectedResult::Transaction::PRESENTED, layer,
+ ExpectedResult::Buffer::ACQUIRED,
+ ExpectedResult::PreviousBuffer::NOT_RELEASED);
+ EXPECT_NO_FATAL_FAILURE(waitForCallback(callback, expected, true));
+ }
+
+ transaction.setBuffer(layer, nullptr);
+ transaction.addTransactionCompletedCallback(callback.function, callback.getContext());
+ transaction.apply();
+
+ {
+ ExpectedResult expected;
+ expected.addSurface(ExpectedResult::Transaction::PRESENTED, layer,
+ ExpectedResult::Buffer::ACQUIRED_NULL,
+ ExpectedResult::PreviousBuffer::RELEASED);
+ EXPECT_NO_FATAL_FAILURE(waitForCallback(callback, expected, true));
+ }
+
+ err = fillTransaction(transaction, &callback, layer, /*setBuffer=*/true,
+ /*setBackgroundColor=*/false);
+ if (err) {
+ GTEST_SUCCEED() << "test not supported";
+ return;
+ }
+
+ transaction.apply();
+
+ {
+ ExpectedResult expected;
+ expected.addSurface(ExpectedResult::Transaction::PRESENTED, layer,
+ ExpectedResult::Buffer::ACQUIRED,
+ ExpectedResult::PreviousBuffer::NOT_RELEASED);
+ EXPECT_NO_FATAL_FAILURE(waitForCallback(callback, expected, true));
+ }
+}
+
+TEST_F(LayerCallbackTest, SetNullBufferOnLayerWithoutBuffer) {
+ sp<SurfaceControl> layer;
+ ASSERT_NO_FATAL_FAILURE(layer = createLayerWithBuffer());
+
+ Transaction transaction;
+ transaction.setBuffer(layer, nullptr);
+ CallbackHelper callback;
+ transaction.addTransactionCompletedCallback(callback.function, callback.getContext());
+ transaction.apply();
+
+ {
+ ExpectedResult expected;
+ expected.addSurface(ExpectedResult::Transaction::NOT_PRESENTED, layer,
+ ExpectedResult::Buffer::NOT_ACQUIRED,
+ ExpectedResult::PreviousBuffer::NOT_RELEASED);
+ EXPECT_NO_FATAL_FAILURE(waitForCallback(callback, expected, true));
+ }
+}
+
} // namespace android
diff --git a/services/surfaceflinger/tests/LayerRenderTypeTransaction_test.cpp b/services/surfaceflinger/tests/LayerRenderTypeTransaction_test.cpp
index 0b8c51e..b8068f7 100644
--- a/services/surfaceflinger/tests/LayerRenderTypeTransaction_test.cpp
+++ b/services/surfaceflinger/tests/LayerRenderTypeTransaction_test.cpp
@@ -1636,6 +1636,65 @@
getScreenCapture()->expectColor(Rect(0, 0, 32, 32), expectedColor, tolerance);
}
}
+
+TEST_P(LayerRenderTypeTransactionTest, SetNullBuffer) {
+ const Rect bounds(0, 0, 32, 32);
+ sp<SurfaceControl> layer;
+ ASSERT_NO_FATAL_FAILURE(
+ layer = createLayer("test", 32, 32, ISurfaceComposerClient::eFXSurfaceBufferState));
+
+ sp<GraphicBuffer> buffer =
+ sp<GraphicBuffer>::make(32u, 32u, PIXEL_FORMAT_RGBA_8888, 1u, kUsageFlags, "test");
+
+ ASSERT_NO_FATAL_FAILURE(TransactionUtils::fillGraphicBufferColor(buffer, bounds, Color::GREEN));
+ Transaction().setBuffer(layer, buffer).apply();
+ {
+ SCOPED_TRACE("before null buffer");
+ auto shot = getScreenCapture();
+ shot->expectColor(bounds, Color::GREEN);
+ }
+
+ Transaction().setBuffer(layer, nullptr).apply();
+ {
+ SCOPED_TRACE("null buffer removes buffer");
+ auto shot = getScreenCapture();
+ shot->expectColor(bounds, Color::BLACK);
+ }
+
+ Transaction().setBuffer(layer, buffer).apply();
+ {
+ SCOPED_TRACE("after null buffer");
+ auto shot = getScreenCapture();
+ shot->expectColor(bounds, Color::GREEN);
+ }
+}
+
+TEST_P(LayerRenderTypeTransactionTest, SetNullBufferOnLayerWithoutBuffer) {
+ const Rect bounds(0, 0, 32, 32);
+ sp<SurfaceControl> layer;
+ ASSERT_NO_FATAL_FAILURE(
+ layer = createLayer("test", 32, 32, ISurfaceComposerClient::eFXSurfaceBufferState));
+ {
+ SCOPED_TRACE("starting state");
+ auto shot = getScreenCapture();
+ shot->expectColor(bounds, Color::BLACK);
+ }
+
+ Transaction().setBuffer(layer, nullptr).apply();
+ {
+ SCOPED_TRACE("null buffer has no effect");
+ auto shot = getScreenCapture();
+ shot->expectColor(bounds, Color::BLACK);
+ }
+
+ Transaction().setBuffer(layer, nullptr).apply();
+ {
+ SCOPED_TRACE("null buffer has no effect");
+ auto shot = getScreenCapture();
+ shot->expectColor(bounds, Color::BLACK);
+ }
+}
+
} // namespace android
// TODO(b/129481165): remove the #pragma below and fix conversion issues
diff --git a/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp b/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp
index 03c4e71..dbb7c6c 100644
--- a/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp
+++ b/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp
@@ -274,6 +274,34 @@
EXPECT_EQ(nullptr, ret.get());
}
+class FakeExternalTexture : public renderengine::ExternalTexture {
+ const sp<GraphicBuffer> mEmptyBuffer = nullptr;
+ uint32_t mWidth;
+ uint32_t mHeight;
+ uint64_t mId;
+ PixelFormat mPixelFormat;
+ uint64_t mUsage;
+
+public:
+ FakeExternalTexture(BufferData& bufferData)
+ : mWidth(bufferData.getWidth()),
+ mHeight(bufferData.getHeight()),
+ mId(bufferData.getId()),
+ mPixelFormat(bufferData.getPixelFormat()),
+ mUsage(bufferData.getUsage()) {}
+ const sp<GraphicBuffer>& getBuffer() const { return mEmptyBuffer; }
+ bool hasSameBuffer(const renderengine::ExternalTexture& other) const override {
+ return getId() == other.getId();
+ }
+ uint32_t getWidth() const override { return mWidth; }
+ uint32_t getHeight() const override { return mHeight; }
+ uint64_t getId() const override { return mId; }
+ PixelFormat getPixelFormat() const override { return mPixelFormat; }
+ uint64_t getUsage() const override { return mUsage; }
+ void remapBuffer() override {}
+ ~FakeExternalTexture() = default;
+};
+
class LatchUnsignaledTest : public TransactionApplicationTest {
public:
void TearDown() override {
@@ -346,7 +374,11 @@
std::vector<ResolvedComposerState> resolvedStates;
resolvedStates.reserve(transaction.states.size());
for (auto& state : transaction.states) {
- resolvedStates.emplace_back(std::move(state));
+ ResolvedComposerState resolvedState;
+ resolvedState.state = std::move(state.state);
+ resolvedState.externalTexture =
+ std::make_shared<FakeExternalTexture>(*resolvedState.state.bufferData);
+ resolvedStates.emplace_back(resolvedState);
}
TransactionState transactionState(transaction.frameTimelineInfo, resolvedStates,