Cleanup for readback buffer management:
* Tests exercising that a bad readback buffer is sent should actually
use a native_handle from a deallocated buffer rather than a valid
handle, since that would still work
* Creating a readback buffer should not require passing in a buffer at
construction. Instead ReadbackBuffer should always allocate its buffer
internally whenever a new buffer needs to be generated.
* Passing the readback buffer's fence to lockAsync should be a dup'd fd,
as lockAsync takes ownership of the fence
Bug: 213493262
Test: VtsHalGraphicsComposer3_TargetTest
Change-Id: I73396f982cf9a67f0dd81eaed509cf9cc7b03314
diff --git a/graphics/composer/aidl/android/hardware/graphics/composer3/vts/functional/VtsHalGraphicsComposer3_ReadbackTest.cpp b/graphics/composer/aidl/android/hardware/graphics/composer3/vts/functional/VtsHalGraphicsComposer3_ReadbackTest.cpp
index 3e0992e..fc527ec 100644
--- a/graphics/composer/aidl/android/hardware/graphics/composer3/vts/functional/VtsHalGraphicsComposer3_ReadbackTest.cpp
+++ b/graphics/composer/aidl/android/hardware/graphics/composer3/vts/functional/VtsHalGraphicsComposer3_ReadbackTest.cpp
@@ -328,8 +328,8 @@
std::vector<Color> expectedColors(static_cast<size_t>(mDisplayWidth * mDisplayHeight));
ReadbackHelper::fillColorsArea(expectedColors, mDisplayWidth, coloredSquare, BLUE);
- ReadbackBuffer readbackBuffer(mPrimaryDisplay, mComposerClient, mGraphicBuffer,
- mDisplayWidth, mDisplayHeight, mPixelFormat, mDataspace);
+ ReadbackBuffer readbackBuffer(mPrimaryDisplay, mComposerClient, mDisplayWidth,
+ mDisplayHeight, mPixelFormat, mDataspace);
ASSERT_NO_FATAL_FAILURE(readbackBuffer.setReadbackBuffer());
writeLayers(layers);
@@ -364,8 +364,8 @@
return;
}
- ReadbackBuffer readbackBuffer(mPrimaryDisplay, mComposerClient, mGraphicBuffer,
- mDisplayWidth, mDisplayHeight, mPixelFormat, mDataspace);
+ ReadbackBuffer readbackBuffer(mPrimaryDisplay, mComposerClient, mDisplayWidth,
+ mDisplayHeight, mPixelFormat, mDataspace);
ASSERT_NO_FATAL_FAILURE(readbackBuffer.setReadbackBuffer());
std::vector<Color> expectedColors(static_cast<size_t>(mDisplayWidth * mDisplayHeight));
ReadbackHelper::fillColorsArea(expectedColors, mDisplayWidth,
@@ -441,8 +441,8 @@
std::vector<Color> expectedColors(static_cast<size_t>(mDisplayWidth * mDisplayHeight));
ReadbackHelper::fillColorsArea(expectedColors, mDisplayWidth, coloredSquare, BLUE);
- ReadbackBuffer readbackBuffer(mPrimaryDisplay, mComposerClient, mGraphicBuffer,
- mDisplayWidth, mDisplayHeight, mPixelFormat, mDataspace);
+ ReadbackBuffer readbackBuffer(mPrimaryDisplay, mComposerClient, mDisplayWidth,
+ mDisplayHeight, mPixelFormat, mDataspace);
ASSERT_NO_FATAL_FAILURE(readbackBuffer.setReadbackBuffer());
mWriter.validateDisplay(mPrimaryDisplay, ComposerClientWriter::kNoTimestamp);
@@ -467,8 +467,8 @@
return;
}
- ReadbackBuffer readbackBuffer(mPrimaryDisplay, mComposerClient, mGraphicBuffer, mDisplayWidth,
- mDisplayHeight, mPixelFormat, mDataspace);
+ ReadbackBuffer readbackBuffer(mPrimaryDisplay, mComposerClient, mDisplayWidth, mDisplayHeight,
+ mPixelFormat, mDataspace);
ASSERT_NO_FATAL_FAILURE(readbackBuffer.setReadbackBuffer());
}
@@ -497,8 +497,13 @@
return;
}
- aidl::android::hardware::common::NativeHandle bufferHandle =
- ::android::dupToAidl(mGraphicBuffer->handle);
+ aidl::android::hardware::common::NativeHandle bufferHandle;
+ {
+ ::android::sp<::android::GraphicBuffer> buffer = allocate();
+ ASSERT_EQ(::android::OK, mGraphicBuffer->initCheck());
+ ::android::makeToAidl(mGraphicBuffer->handle);
+ }
+
ndk::ScopedFileDescriptor releaseFence = ndk::ScopedFileDescriptor(-1);
const auto error =
mComposerClient->setReadbackBuffer(mPrimaryDisplay, bufferHandle, releaseFence);
@@ -552,8 +557,8 @@
std::vector<std::shared_ptr<TestLayer>> layers = {layer};
- ReadbackBuffer readbackBuffer(mPrimaryDisplay, mComposerClient, mGraphicBuffer,
- mDisplayWidth, mDisplayHeight, mPixelFormat, mDataspace);
+ ReadbackBuffer readbackBuffer(mPrimaryDisplay, mComposerClient, mDisplayWidth,
+ mDisplayHeight, mPixelFormat, mDataspace);
ASSERT_NO_FATAL_FAILURE(readbackBuffer.setReadbackBuffer());
writeLayers(layers);
ASSERT_TRUE(mReader.takeErrors().empty());
@@ -631,8 +636,8 @@
ReadbackHelper::fillColorsArea(expectedColors, mDisplayWidth,
{0, mDisplayHeight / 2, mDisplayWidth, mDisplayHeight}, RED);
- ReadbackBuffer readbackBuffer(mPrimaryDisplay, mComposerClient, mGraphicBuffer,
- mDisplayWidth, mDisplayHeight, mPixelFormat, mDataspace);
+ ReadbackBuffer readbackBuffer(mPrimaryDisplay, mComposerClient, mDisplayWidth,
+ mDisplayHeight, mPixelFormat, mDataspace);
ASSERT_NO_FATAL_FAILURE(readbackBuffer.setReadbackBuffer());
auto deviceLayer = std::make_shared<TestBufferLayer>(
@@ -735,8 +740,8 @@
std::vector<std::shared_ptr<TestLayer>> layers = {layer};
- ReadbackBuffer readbackBuffer(mPrimaryDisplay, mComposerClient, mGraphicBuffer,
- mDisplayWidth, mDisplayHeight, mPixelFormat, mDataspace);
+ ReadbackBuffer readbackBuffer(mPrimaryDisplay, mComposerClient, mDisplayWidth,
+ mDisplayHeight, mPixelFormat, mDataspace);
ASSERT_NO_FATAL_FAILURE(readbackBuffer.setReadbackBuffer());
writeLayers(layers);
@@ -798,8 +803,8 @@
std::vector<std::shared_ptr<TestLayer>> layers = {layer};
- ReadbackBuffer readbackBuffer(mPrimaryDisplay, mComposerClient, mGraphicBuffer,
- mDisplayWidth, mDisplayHeight, mPixelFormat, mDataspace);
+ ReadbackBuffer readbackBuffer(mPrimaryDisplay, mComposerClient, mDisplayWidth,
+ mDisplayHeight, mPixelFormat, mDataspace);
ASSERT_NO_FATAL_FAILURE(readbackBuffer.setReadbackBuffer());
@@ -859,8 +864,8 @@
// update expected colors to match crop
ReadbackHelper::fillColorsArea(expectedColors, mDisplayWidth,
{0, 0, mDisplayWidth, mDisplayHeight}, BLUE);
- ReadbackBuffer readbackBuffer(mPrimaryDisplay, mComposerClient, mGraphicBuffer,
- mDisplayWidth, mDisplayHeight, mPixelFormat, mDataspace);
+ ReadbackBuffer readbackBuffer(mPrimaryDisplay, mComposerClient, mDisplayWidth,
+ mDisplayHeight, mPixelFormat, mDataspace);
ASSERT_NO_FATAL_FAILURE(readbackBuffer.setReadbackBuffer());
writeLayers(layers);
ASSERT_TRUE(mReader.takeErrors().empty());
@@ -912,8 +917,8 @@
ReadbackHelper::fillColorsArea(expectedColors, mDisplayWidth, blueRect, BLUE);
ReadbackHelper::fillColorsArea(expectedColors, mDisplayWidth, redRect, RED);
- ReadbackBuffer readbackBuffer(mPrimaryDisplay, mComposerClient, mGraphicBuffer,
- mDisplayWidth, mDisplayHeight, mPixelFormat, mDataspace);
+ ReadbackBuffer readbackBuffer(mPrimaryDisplay, mComposerClient, mDisplayWidth,
+ mDisplayHeight, mPixelFormat, mDataspace);
ASSERT_NO_FATAL_FAILURE(readbackBuffer.setReadbackBuffer());
writeLayers(layers);
@@ -1013,8 +1018,8 @@
ReadbackHelper::fillColorsArea(expectedColors, mDisplayWidth, redRect, RED);
ReadbackHelper::fillColorsArea(expectedColors, mDisplayWidth, dimmerRedRect, DIM_RED);
- ReadbackBuffer readbackBuffer(mPrimaryDisplay, mComposerClient, mGraphicBuffer,
- mDisplayWidth, mDisplayHeight, mPixelFormat, mDataspace);
+ ReadbackBuffer readbackBuffer(mPrimaryDisplay, mComposerClient, mDisplayWidth,
+ mDisplayHeight, mPixelFormat, mDataspace);
ASSERT_NO_FATAL_FAILURE(readbackBuffer.setReadbackBuffer());
writeLayers(layers);
@@ -1135,8 +1140,8 @@
setUpLayers(BlendMode::NONE);
setExpectedColors(expectedColors);
- ReadbackBuffer readbackBuffer(mPrimaryDisplay, mComposerClient, mGraphicBuffer,
- mDisplayWidth, mDisplayHeight, mPixelFormat, mDataspace);
+ ReadbackBuffer readbackBuffer(mPrimaryDisplay, mComposerClient, mDisplayWidth,
+ mDisplayHeight, mPixelFormat, mDataspace);
ASSERT_NO_FATAL_FAILURE(readbackBuffer.setReadbackBuffer());
writeLayers(mLayers);
ASSERT_TRUE(mReader.takeErrors().empty());
@@ -1176,8 +1181,8 @@
setUpLayers(BlendMode::COVERAGE);
setExpectedColors(expectedColors);
- ReadbackBuffer readbackBuffer(mPrimaryDisplay, mComposerClient, mGraphicBuffer,
- mDisplayWidth, mDisplayHeight, mPixelFormat, mDataspace);
+ ReadbackBuffer readbackBuffer(mPrimaryDisplay, mComposerClient, mDisplayWidth,
+ mDisplayHeight, mPixelFormat, mDataspace);
ASSERT_NO_FATAL_FAILURE(readbackBuffer.setReadbackBuffer());
writeLayers(mLayers);
ASSERT_TRUE(mReader.takeErrors().empty());
@@ -1212,8 +1217,8 @@
setUpLayers(BlendMode::PREMULTIPLIED);
setExpectedColors(expectedColors);
- ReadbackBuffer readbackBuffer(mPrimaryDisplay, mComposerClient, mGraphicBuffer,
- mDisplayWidth, mDisplayHeight, mPixelFormat, mDataspace);
+ ReadbackBuffer readbackBuffer(mPrimaryDisplay, mComposerClient, mDisplayWidth,
+ mDisplayHeight, mPixelFormat, mDataspace);
ASSERT_NO_FATAL_FAILURE(readbackBuffer.setReadbackBuffer());
writeLayers(mLayers);
ASSERT_TRUE(mReader.takeErrors().empty());
@@ -1282,8 +1287,8 @@
GTEST_SUCCEED() << "Readback not supported or unsupported pixelFormat/dataspace";
return;
}
- ReadbackBuffer readbackBuffer(mPrimaryDisplay, mComposerClient, mGraphicBuffer,
- mDisplayWidth, mDisplayHeight, mPixelFormat, mDataspace);
+ ReadbackBuffer readbackBuffer(mPrimaryDisplay, mComposerClient, mDisplayWidth,
+ mDisplayHeight, mPixelFormat, mDataspace);
ASSERT_NO_FATAL_FAILURE(readbackBuffer.setReadbackBuffer());
mLayer->setTransform(Transform::FLIP_H);
mLayer->setDataspace(ReadbackHelper::getDataspaceForColorMode(mode), mWriter);
@@ -1323,8 +1328,8 @@
GTEST_SUCCEED() << "Readback not supported or unsupported pixelFormat/dataspace";
return;
}
- ReadbackBuffer readbackBuffer(mPrimaryDisplay, mComposerClient, mGraphicBuffer,
- mDisplayWidth, mDisplayHeight, mPixelFormat, mDataspace);
+ ReadbackBuffer readbackBuffer(mPrimaryDisplay, mComposerClient, mDisplayWidth,
+ mDisplayHeight, mPixelFormat, mDataspace);
ASSERT_NO_FATAL_FAILURE(readbackBuffer.setReadbackBuffer());
mLayer->setTransform(Transform::FLIP_V);
@@ -1364,8 +1369,8 @@
GTEST_SUCCEED() << "Readback not supported or unsupported pixelFormat/dataspace";
return;
}
- ReadbackBuffer readbackBuffer(mPrimaryDisplay, mComposerClient, mGraphicBuffer,
- mDisplayWidth, mDisplayHeight, mPixelFormat, mDataspace);
+ ReadbackBuffer readbackBuffer(mPrimaryDisplay, mComposerClient, mDisplayWidth,
+ mDisplayHeight, mPixelFormat, mDataspace);
ASSERT_NO_FATAL_FAILURE(readbackBuffer.setReadbackBuffer());
mLayer->setTransform(Transform::ROT_180);
diff --git a/graphics/composer/aidl/android/hardware/graphics/composer3/vts/functional/composer-vts/ReadbackVts.cpp b/graphics/composer/aidl/android/hardware/graphics/composer3/vts/functional/composer-vts/ReadbackVts.cpp
index fa1b08d..e05c32d 100644
--- a/graphics/composer/aidl/android/hardware/graphics/composer3/vts/functional/composer-vts/ReadbackVts.cpp
+++ b/graphics/composer/aidl/android/hardware/graphics/composer3/vts/functional/composer-vts/ReadbackVts.cpp
@@ -187,7 +187,6 @@
int offset = (row * stride + col) * bytesPerPixel;
uint8_t* pixelColor = (uint8_t*)bufferData + offset;
const Color expectedColor = expectedColors[static_cast<size_t>(pixel)];
-
ASSERT_EQ(std::round(255.0f * expectedColor.r), pixelColor[0]);
ASSERT_EQ(std::round(255.0f * expectedColor.g), pixelColor[1]);
ASSERT_EQ(std::round(255.0f * expectedColor.b), pixelColor[2]);
@@ -196,13 +195,11 @@
}
ReadbackBuffer::ReadbackBuffer(int64_t display, const std::shared_ptr<IComposerClient>& client,
- const ::android::sp<::android::GraphicBuffer>& graphicBuffer,
int32_t width, int32_t height, common::PixelFormat pixelFormat,
common::Dataspace dataspace) {
mDisplay = display;
mComposerClient = client;
- mGraphicBuffer = graphicBuffer;
mPixelFormat = pixelFormat;
mDataspace = dataspace;
@@ -236,6 +233,7 @@
}
void ReadbackBuffer::checkReadbackBuffer(std::vector<Color> expectedColors) {
+ ASSERT_NE(nullptr, mGraphicBuffer);
// lock buffer for reading
ndk::ScopedFileDescriptor fenceHandle;
EXPECT_TRUE(mComposerClient->getReadbackBufferFence(mDisplay, &fenceHandle).isOk());
@@ -243,7 +241,8 @@
int outBytesPerPixel;
int outBytesPerStride;
void* bufData = nullptr;
- auto status = mGraphicBuffer->lockAsync(mUsage, mAccessRegion, &bufData, fenceHandle.get(),
+
+ auto status = mGraphicBuffer->lockAsync(mUsage, mAccessRegion, &bufData, dup(fenceHandle.get()),
&outBytesPerPixel, &outBytesPerStride);
EXPECT_EQ(::android::OK, status);
ASSERT_TRUE(mPixelFormat == PixelFormat::RGB_888 || mPixelFormat == PixelFormat::RGBA_8888);
diff --git a/graphics/composer/aidl/android/hardware/graphics/composer3/vts/functional/composer-vts/include/ReadbackVts.h b/graphics/composer/aidl/android/hardware/graphics/composer3/vts/functional/composer-vts/include/ReadbackVts.h
index 8d84667..8785513 100644
--- a/graphics/composer/aidl/android/hardware/graphics/composer3/vts/functional/composer-vts/include/ReadbackVts.h
+++ b/graphics/composer/aidl/android/hardware/graphics/composer3/vts/functional/composer-vts/include/ReadbackVts.h
@@ -196,8 +196,7 @@
class ReadbackBuffer {
public:
- ReadbackBuffer(int64_t display, const std::shared_ptr<IComposerClient>& client,
- const ::android::sp<::android::GraphicBuffer>& graphicBuffer, int32_t width,
+ ReadbackBuffer(int64_t display, const std::shared_ptr<IComposerClient>& client, int32_t width,
int32_t height, common::PixelFormat pixelFormat, common::Dataspace dataspace);
void setReadbackBuffer();