From 78ff2d6491b20a874d643358554dc352d51763a3 Mon Sep 17 00:00:00 2001 From: Brian Lindahl Date: Sun, 18 Dec 2022 11:21:41 -0700 Subject: [PATCH] Add VTS for new API for clearing buffer slots Bug: 262041682 Bug: 258196272 Test: VtsHalGraphicsComposer3_TargetTest Change-Id: If7e5fea56734f3ae7b4aae7378f4aea0eacc8e32 --- .../graphics/composer3/ComposerClientWriter.h | 8 + .../composer/aidl/vts/VtsComposerClient.cpp | 6 + .../composer/aidl/vts/VtsComposerClient.h | 2 + .../VtsHalGraphicsComposer3_ReadbackTest.cpp | 205 +++++++++++++++--- .../VtsHalGraphicsComposer3_TargetTest.cpp | 77 ++++++- 5 files changed, 262 insertions(+), 36 deletions(-) diff --git a/graphics/composer/aidl/include/android/hardware/graphics/composer3/ComposerClientWriter.h b/graphics/composer/aidl/include/android/hardware/graphics/composer3/ComposerClientWriter.h index e35d07bce9..22020c0ea9 100644 --- a/graphics/composer/aidl/include/android/hardware/graphics/composer3/ComposerClientWriter.h +++ b/graphics/composer/aidl/include/android/hardware/graphics/composer3/ComposerClientWriter.h @@ -142,6 +142,14 @@ class ComposerClientWriter final { flushLayerCommand(); } + void setLayerBufferSlotsToClear(int64_t display, int64_t layer, + const std::vector& slotsToClear) { + LayerCommand& layerCommand = getLayerCommand(display, layer); + for (auto slot : slotsToClear) { + layerCommand.bufferSlotsToClear.emplace(static_cast(slot)); + } + } + void setLayerSurfaceDamage(int64_t display, int64_t layer, const std::vector& damage) { getLayerCommand(display, layer).damage.emplace(damage.begin(), damage.end()); } diff --git a/graphics/composer/aidl/vts/VtsComposerClient.cpp b/graphics/composer/aidl/vts/VtsComposerClient.cpp index 34cc80241c..00b578b3f9 100644 --- a/graphics/composer/aidl/vts/VtsComposerClient.cpp +++ b/graphics/composer/aidl/vts/VtsComposerClient.cpp @@ -58,6 +58,12 @@ bool VtsComposerClient::tearDown() { return verifyComposerCallbackParams() && destroyAllLayers(); } +std::pair VtsComposerClient::getInterfaceVersion() { + int32_t version = 1; + auto status = mComposerClient->getInterfaceVersion(&version); + return {std::move(status), version}; +} + std::pair VtsComposerClient::createVirtualDisplay( int32_t width, int32_t height, PixelFormat pixelFormat, int32_t bufferSlotCount) { VirtualDisplay outVirtualDisplay; diff --git a/graphics/composer/aidl/vts/VtsComposerClient.h b/graphics/composer/aidl/vts/VtsComposerClient.h index 18833365e0..e61fde9571 100644 --- a/graphics/composer/aidl/vts/VtsComposerClient.h +++ b/graphics/composer/aidl/vts/VtsComposerClient.h @@ -61,6 +61,8 @@ class VtsComposerClient { bool tearDown(); + std::pair getInterfaceVersion(); + std::pair createVirtualDisplay(int32_t width, int32_t height, PixelFormat pixelFormat, int32_t bufferSlotCount); diff --git a/graphics/composer/aidl/vts/VtsHalGraphicsComposer3_ReadbackTest.cpp b/graphics/composer/aidl/vts/VtsHalGraphicsComposer3_ReadbackTest.cpp index 93d9693dbd..166127dfae 100644 --- a/graphics/composer/aidl/vts/VtsHalGraphicsComposer3_ReadbackTest.cpp +++ b/graphics/composer/aidl/vts/VtsHalGraphicsComposer3_ReadbackTest.cpp @@ -379,9 +379,16 @@ TEST_P(GraphicsCompositionTest, SetLayerBuffer) { } TEST_P(GraphicsCompositionTest, SetLayerBufferWithSlotsToClear) { - const auto& [status, readbackBufferAttributes] = + const auto& [versionStatus, version] = mComposerClient->getInterfaceVersion(); + ASSERT_TRUE(versionStatus.isOk()); + if (version == 1) { + GTEST_SUCCEED() << "Device does not support the new API for clearing buffer slots"; + return; + } + + const auto& [readbackStatus, readbackBufferAttributes] = mComposerClient->getReadbackBufferAttributes(getPrimaryDisplayId()); - if (!status.isOk()) { + if (!readbackStatus.isOk()) { GTEST_SUCCEED() << "Readback not supported"; return; } @@ -396,9 +403,6 @@ TEST_P(GraphicsCompositionTest, SetLayerBufferWithSlotsToClear) { // no fence needed for the readback buffer ScopedFileDescriptor noFence(-1); - sp clearSlotBuffer = allocateBuffer(1u, 1u, GRALLOC_USAGE_HW_COMPOSER); - ASSERT_NE(nullptr, clearSlotBuffer); - // red buffer uint64_t usage = GRALLOC_USAGE_SW_WRITE_OFTEN | GRALLOC_USAGE_SW_READ_OFTEN; sp redBuffer = allocateBuffer(usage); @@ -412,10 +416,16 @@ TEST_P(GraphicsCompositionTest, SetLayerBufferWithSlotsToClear) { int blueFence; ASSERT_NO_FATAL_FAILURE(ReadbackHelper::fillBufferAndGetFence(blueBuffer, BLUE, &blueFence)); + // green buffer + sp greenBuffer = allocateBuffer(usage); + ASSERT_NE(nullptr, greenBuffer); + int greenFence; + ASSERT_NO_FATAL_FAILURE(ReadbackHelper::fillBufferAndGetFence(greenBuffer, GREEN, &greenFence)); + // layer defaults common::Rect rectFullDisplay = {0, 0, getDisplayWidth(), getDisplayHeight()}; int64_t display = getPrimaryDisplayId(); - auto [layerStatus, layer] = mComposerClient->createLayer(getPrimaryDisplayId(), 3); + const auto& [layerStatus, layer] = mComposerClient->createLayer(getPrimaryDisplayId(), 3); ASSERT_TRUE(layerStatus.isOk()); mWriter->setLayerDisplayFrame(display, layer, rectFullDisplay); mWriter->setLayerCompositionType(display, layer, Composition::DEVICE); @@ -454,13 +464,12 @@ TEST_P(GraphicsCompositionTest, SetLayerBufferWithSlotsToClear) { ReadbackHelper::compareColorToBuffer(RED, readbackBuffer, fence); } - // clear the slot for the blue buffer - // should still be red + // change the layer to the green buffer + // should be green { auto status = mComposerClient->setReadbackBuffer(display, readbackBuffer->handle, noFence); ASSERT_TRUE(status.isOk()); - mWriter->setLayerBufferWithNewCommand(display, layer, /*slot*/ 0, clearSlotBuffer->handle, - /*fence*/ -1); + mWriter->setLayerBuffer(display, layer, /*slot*/ 2, greenBuffer->handle, greenFence); mWriter->validateDisplay(display, ComposerClientWriter::kNoTimestamp); execute(); ASSERT_TRUE(mReader.takeChangedCompositionTypes(display).empty()); @@ -469,17 +478,106 @@ TEST_P(GraphicsCompositionTest, SetLayerBufferWithSlotsToClear) { execute(); auto [fenceStatus, fence] = mComposerClient->getReadbackBufferFence(display); ASSERT_TRUE(fenceStatus.isOk()); - ReadbackHelper::compareColorToBuffer(RED, readbackBuffer, fence); + ReadbackHelper::compareColorToBuffer(GREEN, readbackBuffer, fence); } - // clear the slot for the red buffer, and set the buffer with the same slot to the blue buffer + // clear the slots for all buffers + // should still be green since the active buffer should not be cleared by the HAL implementation + { + auto status = mComposerClient->setReadbackBuffer(display, readbackBuffer->handle, noFence); + ASSERT_TRUE(status.isOk()); + mWriter->setLayerBufferSlotsToClear(display, layer, /*slotsToClear*/ {0, 1, 2}); + mWriter->validateDisplay(display, ComposerClientWriter::kNoTimestamp); + execute(); + ASSERT_TRUE(mReader.takeChangedCompositionTypes(display).empty()); + ASSERT_TRUE(mReader.takeErrors().empty()); + mWriter->presentDisplay(display); + execute(); + auto [fenceStatus, fence] = mComposerClient->getReadbackBufferFence(display); + ASSERT_TRUE(fenceStatus.isOk()); + ReadbackHelper::compareColorToBuffer(GREEN, readbackBuffer, fence); + } + + // clear the slot for the green buffer, and set the buffer with the same slot to the blue buffer // should be blue { auto status = mComposerClient->setReadbackBuffer(display, readbackBuffer->handle, noFence); ASSERT_TRUE(status.isOk()); - mWriter->setLayerBufferWithNewCommand(display, layer, /*slot*/ 1, clearSlotBuffer->handle, - /*fence*/ -1); - mWriter->setLayerBuffer(display, layer, /*slot*/ 1, blueBuffer->handle, blueFence); + mWriter->setLayerBufferSlotsToClear(display, layer, /*slotsToClear*/ {2}); + mWriter->setLayerBuffer(display, layer, /*slot*/ 2, blueBuffer->handle, blueFence); + mWriter->validateDisplay(display, ComposerClientWriter::kNoTimestamp); + execute(); + ASSERT_TRUE(mReader.takeChangedCompositionTypes(display).empty()); + ASSERT_TRUE(mReader.takeErrors().empty()); + mWriter->presentDisplay(display); + execute(); + auto [fenceStatus, fence] = mComposerClient->getReadbackBufferFence(display); + ASSERT_TRUE(fenceStatus.isOk()); + ReadbackHelper::compareColorToBuffer(BLUE, readbackBuffer, fence); + } +} + +TEST_P(GraphicsCompositionTest, SetLayerBufferWithSlotsToClear_backwardsCompatible) { + const auto& [versionStatus, version] = mComposerClient->getInterfaceVersion(); + ASSERT_TRUE(versionStatus.isOk()); + if (version > 1) { + GTEST_SUCCEED() << "Device does not need a backwards compatible way to clear buffer slots"; + return; + } + + const auto& [readbackStatus, readbackBufferAttributes] = + mComposerClient->getReadbackBufferAttributes(getPrimaryDisplayId()); + if (!readbackStatus.isOk()) { + GTEST_SUCCEED() << "Readback not supported"; + return; + } + + sp readbackBuffer; + ASSERT_NO_FATAL_FAILURE(ReadbackHelper::createReadbackBuffer( + readbackBufferAttributes, getPrimaryDisplay(), &readbackBuffer)); + if (readbackBuffer == nullptr) { + GTEST_SUCCEED() << "Unsupported readback buffer attributes"; + return; + } + // no fence needed for the readback buffer + ScopedFileDescriptor noFence(-1); + + sp clearSlotBuffer = allocateBuffer(1u, 1u, GRALLOC_USAGE_HW_COMPOSER); + ASSERT_NE(nullptr, clearSlotBuffer); + + // red buffer + uint64_t usage = GRALLOC_USAGE_SW_WRITE_OFTEN | GRALLOC_USAGE_SW_READ_OFTEN; + sp redBuffer = allocateBuffer(usage); + ASSERT_NE(nullptr, redBuffer); + int redFence; + ASSERT_NO_FATAL_FAILURE(ReadbackHelper::fillBufferAndGetFence(redBuffer, RED, &redFence)); + + // blue buffer + sp blueBuffer = allocateBuffer(usage); + ASSERT_NE(nullptr, blueBuffer); + int blueFence; + ASSERT_NO_FATAL_FAILURE(ReadbackHelper::fillBufferAndGetFence(blueBuffer, BLUE, &blueFence)); + + // green buffer + sp greenBuffer = allocateBuffer(usage); + ASSERT_NE(nullptr, greenBuffer); + int greenFence; + ASSERT_NO_FATAL_FAILURE(ReadbackHelper::fillBufferAndGetFence(greenBuffer, GREEN, &greenFence)); + + // layer defaults + common::Rect rectFullDisplay = {0, 0, getDisplayWidth(), getDisplayHeight()}; + int64_t display = getPrimaryDisplayId(); + const auto& [layerStatus, layer] = mComposerClient->createLayer(getPrimaryDisplayId(), 3); + ASSERT_TRUE(layerStatus.isOk()); + mWriter->setLayerDisplayFrame(display, layer, rectFullDisplay); + mWriter->setLayerCompositionType(display, layer, Composition::DEVICE); + + // set the layer to the blue buffer + // should be blue + { + auto status = mComposerClient->setReadbackBuffer(display, readbackBuffer->handle, noFence); + ASSERT_TRUE(status.isOk()); + mWriter->setLayerBuffer(display, layer, /*slot*/ 0, blueBuffer->handle, blueFence); mWriter->validateDisplay(display, ComposerClientWriter::kNoTimestamp); execute(); ASSERT_TRUE(mReader.takeChangedCompositionTypes(display).empty()); @@ -491,27 +589,82 @@ TEST_P(GraphicsCompositionTest, SetLayerBufferWithSlotsToClear) { ReadbackHelper::compareColorToBuffer(BLUE, readbackBuffer, fence); } - // clear the slot for the now-blue buffer - // should be black (no buffer) - // TODO(b/262037933) Ensure we never clear the active buffer's slot with the placeholder buffer - // by setting the layer to the color black + // change the layer to the red buffer + // should be red { auto status = mComposerClient->setReadbackBuffer(display, readbackBuffer->handle, noFence); ASSERT_TRUE(status.isOk()); - mWriter->setLayerBufferWithNewCommand(display, layer, /*slot*/ 1, clearSlotBuffer->handle, - /*fence*/ -1); + mWriter->setLayerBuffer(display, layer, /*slot*/ 1, redBuffer->handle, redFence); mWriter->validateDisplay(display, ComposerClientWriter::kNoTimestamp); execute(); - if (!mReader.takeChangedCompositionTypes(display).empty()) { - GTEST_SUCCEED(); - return; - } + ASSERT_TRUE(mReader.takeChangedCompositionTypes(display).empty()); ASSERT_TRUE(mReader.takeErrors().empty()); mWriter->presentDisplay(display); execute(); auto [fenceStatus, fence] = mComposerClient->getReadbackBufferFence(display); ASSERT_TRUE(fenceStatus.isOk()); - ReadbackHelper::compareColorToBuffer(BLACK, readbackBuffer, fence); + ReadbackHelper::compareColorToBuffer(RED, readbackBuffer, fence); + } + + // change the layer to the green buffer + // should be green + { + auto status = mComposerClient->setReadbackBuffer(display, readbackBuffer->handle, noFence); + ASSERT_TRUE(status.isOk()); + mWriter->setLayerBuffer(display, layer, /*slot*/ 2, greenBuffer->handle, greenFence); + mWriter->validateDisplay(display, ComposerClientWriter::kNoTimestamp); + execute(); + ASSERT_TRUE(mReader.takeChangedCompositionTypes(display).empty()); + ASSERT_TRUE(mReader.takeErrors().empty()); + mWriter->presentDisplay(display); + execute(); + auto [fenceStatus, fence] = mComposerClient->getReadbackBufferFence(display); + ASSERT_TRUE(fenceStatus.isOk()); + ReadbackHelper::compareColorToBuffer(GREEN, readbackBuffer, fence); + } + + // clear the slot for the blue buffer + // should still be green + { + auto status = mComposerClient->setReadbackBuffer(display, readbackBuffer->handle, noFence); + ASSERT_TRUE(status.isOk()); + mWriter->setLayerBufferWithNewCommand(display, layer, /*slot*/ 0, clearSlotBuffer->handle, + /*fence*/ -1); + // SurfaceFlinger will re-set the active buffer slot after other buffer slots are cleared + mWriter->setLayerBufferWithNewCommand(display, layer, /*slot*/ 2, /*handle*/ nullptr, + /*fence*/ -1); + mWriter->validateDisplay(display, ComposerClientWriter::kNoTimestamp); + execute(); + ASSERT_TRUE(mReader.takeChangedCompositionTypes(display).empty()); + ASSERT_TRUE(mReader.takeErrors().empty()); + mWriter->presentDisplay(display); + execute(); + auto [fenceStatus, fence] = mComposerClient->getReadbackBufferFence(display); + ASSERT_TRUE(fenceStatus.isOk()); + ReadbackHelper::compareColorToBuffer(GREEN, readbackBuffer, fence); + } + + // clear the slot for all buffers, and set the buffer with the same slot as the green buffer + // should be blue now + { + auto status = mComposerClient->setReadbackBuffer(display, readbackBuffer->handle, noFence); + ASSERT_TRUE(status.isOk()); + mWriter->setLayerBufferWithNewCommand(display, layer, /*slot*/ 0, clearSlotBuffer->handle, + /*fence*/ -1); + mWriter->setLayerBufferWithNewCommand(display, layer, /*slot*/ 1, clearSlotBuffer->handle, + /*fence*/ -1); + // SurfaceFlinger will never clear the active buffer (slot 2), but will free up the + // buffer slot so it will be re-used for the next setLayerBuffer command + mWriter->setLayerBuffer(display, layer, /*slot*/ 2, blueBuffer->handle, blueFence); + mWriter->validateDisplay(display, ComposerClientWriter::kNoTimestamp); + execute(); + ASSERT_TRUE(mReader.takeChangedCompositionTypes(display).empty()); + ASSERT_TRUE(mReader.takeErrors().empty()); + mWriter->presentDisplay(display); + execute(); + auto [fenceStatus, fence] = mComposerClient->getReadbackBufferFence(display); + ASSERT_TRUE(fenceStatus.isOk()); + ReadbackHelper::compareColorToBuffer(BLUE, readbackBuffer, fence); } } diff --git a/graphics/composer/aidl/vts/VtsHalGraphicsComposer3_TargetTest.cpp b/graphics/composer/aidl/vts/VtsHalGraphicsComposer3_TargetTest.cpp index 0f00fd6094..7b852e029d 100644 --- a/graphics/composer/aidl/vts/VtsHalGraphicsComposer3_TargetTest.cpp +++ b/graphics/composer/aidl/vts/VtsHalGraphicsComposer3_TargetTest.cpp @@ -1647,28 +1647,85 @@ TEST_P(GraphicsComposerAidlCommandTest, SetLayerBuffer) { } TEST_P(GraphicsComposerAidlCommandTest, SetLayerBufferWithSlotsToClear) { - auto clearSlotBuffer = allocate(1u, 1u, ::android::PIXEL_FORMAT_RGB_888); - ASSERT_NE(nullptr, clearSlotBuffer); - auto clearSlotBufferHandle = clearSlotBuffer->handle; + const auto& [versionStatus, version] = mComposerClient->getInterfaceVersion(); + ASSERT_TRUE(versionStatus.isOk()); + if (version == 1) { + GTEST_SUCCEED() << "Device does not support the new API for clearing buffer slots"; + return; + } + + const auto& [layerStatus, layer] = + mComposerClient->createLayer(getPrimaryDisplayId(), kBufferSlotCount); + EXPECT_TRUE(layerStatus.isOk()); + + auto& writer = getWriter(getPrimaryDisplayId()); const auto buffer1 = allocate(::android::PIXEL_FORMAT_RGBA_8888); ASSERT_NE(nullptr, buffer1); const auto handle1 = buffer1->handle; - const auto& [layerStatus, layer] = - mComposerClient->createLayer(getPrimaryDisplayId(), kBufferSlotCount); - EXPECT_TRUE(layerStatus.isOk()); - auto& writer = getWriter(getPrimaryDisplayId()); writer.setLayerBuffer(getPrimaryDisplayId(), layer, /*slot*/ 0, handle1, /*acquireFence*/ -1); execute(); ASSERT_TRUE(mReader.takeErrors().empty()); - // Ensure we can clear a buffer slot and then set that same slot with a new buffer const auto buffer2 = allocate(::android::PIXEL_FORMAT_RGBA_8888); ASSERT_NE(nullptr, buffer2); const auto handle2 = buffer2->handle; - writer.setLayerBufferWithNewCommand(getPrimaryDisplayId(), layer, /* slot */ 0, + writer.setLayerBuffer(getPrimaryDisplayId(), layer, /*slot*/ 1, handle2, /*acquireFence*/ -1); + execute(); + ASSERT_TRUE(mReader.takeErrors().empty()); + + // Ensure we can clear the buffer slots and then set the active slot with a new buffer + const auto buffer3 = allocate(::android::PIXEL_FORMAT_RGBA_8888); + ASSERT_NE(nullptr, buffer3); + const auto handle3 = buffer3->handle; + writer.setLayerBufferSlotsToClear(getPrimaryDisplayId(), layer, /*slotsToClear*/ {0, 1}); + writer.setLayerBuffer(getPrimaryDisplayId(), layer, /*slot*/ 1, handle3, /*acquireFence*/ -1); + execute(); + ASSERT_TRUE(mReader.takeErrors().empty()); +} + +TEST_P(GraphicsComposerAidlCommandTest, SetLayerBufferWithSlotsToClear_backwardsCompatible) { + const auto& [versionStatus, version] = mComposerClient->getInterfaceVersion(); + ASSERT_TRUE(versionStatus.isOk()); + if (version > 1) { + GTEST_SUCCEED() << "Device does not need a backwards compatible way to clear buffer slots"; + return; + } + + auto clearSlotBuffer = allocate(1u, 1u, ::android::PIXEL_FORMAT_RGB_888); + ASSERT_NE(nullptr, clearSlotBuffer); + auto clearSlotBufferHandle = clearSlotBuffer->handle; + + const auto& [layerStatus, layer] = + mComposerClient->createLayer(getPrimaryDisplayId(), kBufferSlotCount); + EXPECT_TRUE(layerStatus.isOk()); + + auto& writer = getWriter(getPrimaryDisplayId()); + + const auto buffer1 = allocate(::android::PIXEL_FORMAT_RGBA_8888); + ASSERT_NE(nullptr, buffer1); + const auto handle1 = buffer1->handle; + writer.setLayerBuffer(getPrimaryDisplayId(), layer, /*slot*/ 0, handle1, /*acquireFence*/ -1); + execute(); + ASSERT_TRUE(mReader.takeErrors().empty()); + + const auto buffer2 = allocate(::android::PIXEL_FORMAT_RGBA_8888); + ASSERT_NE(nullptr, buffer2); + const auto handle2 = buffer2->handle; + EXPECT_TRUE(layerStatus.isOk()); + writer.setLayerBuffer(getPrimaryDisplayId(), layer, /*slot*/ 1, handle2, /*acquireFence*/ -1); + execute(); + ASSERT_TRUE(mReader.takeErrors().empty()); + + // Ensure we can clear a buffer slot and then set that same slot with a new buffer + const auto buffer3 = allocate(::android::PIXEL_FORMAT_RGBA_8888); + ASSERT_NE(nullptr, buffer3); + const auto handle3 = buffer2->handle; + // SurfaceFlinger will never clear the active buffer, instead it will clear non-active buffers + // and then re-use the active buffer's slot for the new buffer + writer.setLayerBufferWithNewCommand(getPrimaryDisplayId(), layer, /*slot*/ 0, clearSlotBufferHandle, /*acquireFence*/ -1); - writer.setLayerBuffer(getPrimaryDisplayId(), layer, /*slot*/ 0, handle2, /*acquireFence*/ -1); + writer.setLayerBuffer(getPrimaryDisplayId(), layer, /*slot*/ 1, handle3, /*acquireFence*/ -1); execute(); ASSERT_TRUE(mReader.takeErrors().empty()); }