Surface: add unit test for GetAndFlushRemovedBuffers
Also fix the removed list flush behavior to match spec.
Test: the new test pass, smoke test GCA
Bug: 36869090
Change-Id: I8f7bdd8b168424f4e79980d21a7388aa9e35597e
diff --git a/libs/gui/Surface.cpp b/libs/gui/Surface.cpp
index a6d9e66..3ed7dcb 100644
--- a/libs/gui/Surface.cpp
+++ b/libs/gui/Surface.cpp
@@ -476,6 +476,9 @@
{
Mutex::Autolock lock(mMutex);
+ if (mReportRemovedBuffers) {
+ mRemovedBuffers.clear();
+ }
reqWidth = mReqWidth ? mReqWidth : mUserWidth;
reqHeight = mReqHeight ? mReqHeight : mUserHeight;
@@ -530,7 +533,6 @@
if ((result & IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION) || gbuf == nullptr) {
if (mReportRemovedBuffers && (gbuf != nullptr)) {
- mRemovedBuffers.clear();
mRemovedBuffers.push_back(gbuf);
}
result = mGraphicBufferProducer->requestBuffer(buf, &gbuf);
@@ -1202,6 +1204,9 @@
}
Mutex::Autolock lock(mMutex);
+ if (mReportRemovedBuffers) {
+ mRemovedBuffers.clear();
+ }
sp<GraphicBuffer> buffer(NULL);
sp<Fence> fence(NULL);
@@ -1218,10 +1223,6 @@
*outFence = Fence::NO_FENCE;
}
- if (mReportRemovedBuffers) {
- mRemovedBuffers.clear();
- }
-
for (int i = 0; i < NUM_BUFFER_SLOTS; i++) {
if (mSlots[i].buffer != NULL &&
mSlots[i].buffer->handle == buffer->handle) {
@@ -1241,6 +1242,9 @@
ALOGV("Surface::attachBuffer");
Mutex::Autolock lock(mMutex);
+ if (mReportRemovedBuffers) {
+ mRemovedBuffers.clear();
+ }
sp<GraphicBuffer> graphicBuffer(static_cast<GraphicBuffer*>(buffer));
uint32_t priorGeneration = graphicBuffer->mGenerationNumber;
@@ -1254,7 +1258,6 @@
return result;
}
if (mReportRemovedBuffers && (mSlots[attachedSlot].buffer != nullptr)) {
- mRemovedBuffers.clear();
mRemovedBuffers.push_back(mSlots[attachedSlot].buffer);
}
mSlots[attachedSlot].buffer = graphicBuffer;
diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp
index 08d6715..fcaa23a 100644
--- a/libs/gui/tests/Surface_test.cpp
+++ b/libs/gui/tests/Surface_test.cpp
@@ -24,6 +24,7 @@
#include <cutils/properties.h>
#include <gui/BufferItemConsumer.h>
#include <gui/IDisplayEventConnection.h>
+#include <gui/IProducerListener.h>
#include <gui/ISurfaceComposer.h>
#include <gui/Surface.h>
#include <gui/SurfaceComposerClient.h>
@@ -320,6 +321,77 @@
ASSERT_EQ(NO_ERROR, window->queueBuffer(window.get(), buffer, fence));
}
+TEST_F(SurfaceTest, GetAndFlushRemovedBuffers) {
+ sp<IGraphicBufferProducer> producer;
+ sp<IGraphicBufferConsumer> consumer;
+ BufferQueue::createBufferQueue(&producer, &consumer);
+
+ sp<DummyConsumer> dummyConsumer(new DummyConsumer);
+ consumer->consumerConnect(dummyConsumer, false);
+ consumer->setConsumerName(String8("TestConsumer"));
+
+ sp<Surface> surface = new Surface(producer);
+ sp<ANativeWindow> window(surface);
+ sp<DummyProducerListener> listener = new DummyProducerListener();
+ ASSERT_EQ(OK, surface->connect(
+ NATIVE_WINDOW_API_CPU,
+ /*listener*/listener,
+ /*reportBufferRemoval*/true));
+ const int BUFFER_COUNT = 4;
+ ASSERT_EQ(NO_ERROR, native_window_set_buffer_count(window.get(), BUFFER_COUNT));
+
+ sp<GraphicBuffer> detachedBuffer;
+ sp<Fence> outFence;
+ int fences[BUFFER_COUNT];
+ ANativeWindowBuffer* buffers[BUFFER_COUNT];
+ // Allocate buffers because detachNextBuffer requires allocated buffers
+ for (int i = 0; i < BUFFER_COUNT; i++) {
+ ASSERT_EQ(NO_ERROR, window->dequeueBuffer(window.get(), &buffers[i], &fences[i]));
+ }
+ for (int i = 0; i < BUFFER_COUNT; i++) {
+ ASSERT_EQ(NO_ERROR, window->cancelBuffer(window.get(), buffers[i], fences[i]));
+ }
+
+ // Test detached buffer is correctly reported
+ ASSERT_EQ(NO_ERROR, surface->detachNextBuffer(&detachedBuffer, &outFence));
+ std::vector<sp<GraphicBuffer>> removedBuffers;
+ ASSERT_EQ(OK, surface->getAndFlushRemovedBuffers(&removedBuffers));
+ ASSERT_EQ(1u, removedBuffers.size());
+ ASSERT_EQ(detachedBuffer->handle, removedBuffers.at(0)->handle);
+ // Test the list is flushed one getAndFlushRemovedBuffers returns
+ ASSERT_EQ(OK, surface->getAndFlushRemovedBuffers(&removedBuffers));
+ ASSERT_EQ(0u, removedBuffers.size());
+
+
+ // Test removed buffer list is cleanup after next dequeueBuffer call
+ ASSERT_EQ(NO_ERROR, surface->detachNextBuffer(&detachedBuffer, &outFence));
+ ASSERT_EQ(NO_ERROR, window->dequeueBuffer(window.get(), &buffers[0], &fences[0]));
+ ASSERT_EQ(OK, surface->getAndFlushRemovedBuffers(&removedBuffers));
+ ASSERT_EQ(0u, removedBuffers.size());
+ ASSERT_EQ(NO_ERROR, window->cancelBuffer(window.get(), buffers[0], fences[0]));
+
+ // Test removed buffer list is cleanup after next detachNextBuffer call
+ ASSERT_EQ(NO_ERROR, surface->detachNextBuffer(&detachedBuffer, &outFence));
+ ASSERT_EQ(NO_ERROR, surface->detachNextBuffer(&detachedBuffer, &outFence));
+ ASSERT_EQ(OK, surface->getAndFlushRemovedBuffers(&removedBuffers));
+ ASSERT_EQ(1u, removedBuffers.size());
+ ASSERT_EQ(detachedBuffer->handle, removedBuffers.at(0)->handle);
+
+ // Re-allocate buffers since all buffers are detached up to now
+ for (int i = 0; i < BUFFER_COUNT; i++) {
+ ASSERT_EQ(NO_ERROR, window->dequeueBuffer(window.get(), &buffers[i], &fences[i]));
+ }
+ for (int i = 0; i < BUFFER_COUNT; i++) {
+ ASSERT_EQ(NO_ERROR, window->cancelBuffer(window.get(), buffers[i], fences[i]));
+ }
+
+ ASSERT_EQ(NO_ERROR, surface->detachNextBuffer(&detachedBuffer, &outFence));
+ ASSERT_EQ(NO_ERROR, surface->attachBuffer(detachedBuffer.get()));
+ ASSERT_EQ(OK, surface->getAndFlushRemovedBuffers(&removedBuffers));
+ // Depends on which slot GraphicBufferProducer impl pick, the attach call might
+ // get 0 or 1 buffer removed.
+ ASSERT_LE(removedBuffers.size(), 1u);
+}
class FakeConsumer : public BnConsumerListener {
public: