Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 7b4d5ee4 authored by Alec Mouri's avatar Alec Mouri
Browse files

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
parent 51067019

graphics/composer/aidl/android/hardware/graphics/composer3/vts/functional/VtsHalGraphicsComposer3_ReadbackTest.cpp

+41 −36
Original line number Diff line number Diff line
@@ -328,8 +328,8 @@ TEST_P(GraphicsCompositionTest, SingleSolidColorLayer) { 
        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 @@ TEST_P(GraphicsCompositionTest, SetLayerBuffer) { 
            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 @@ TEST_P(GraphicsCompositionTest, SetLayerBufferNoEffect) { 
        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 @@ TEST_P(GraphicsCompositionTest, SetReadbackBuffer) { 
        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 @@ TEST_P(GraphicsCompositionTest, SetReadbackBufferBadParameter) { 
        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 @@ TEST_P(GraphicsCompositionTest, ClientComposition) { 


        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 @@ TEST_P(GraphicsCompositionTest, DeviceAndClientComposition) { 
        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 @@ TEST_P(GraphicsCompositionTest, SetLayerDamage) { 


        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 @@ TEST_P(GraphicsCompositionTest, SetLayerPlaneAlpha) { 


        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 @@ TEST_P(GraphicsCompositionTest, SetLayerSourceCrop) { 
        // 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 @@ TEST_P(GraphicsCompositionTest, SetLayerZOrder) { 
        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 @@ TEST_P(GraphicsCompositionTest, SetLayerWhitePointDims) { 
        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 @@ TEST_P(GraphicsBlendModeCompositionTest, None) { 
        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 @@ TEST_P(GraphicsBlendModeCompositionTest, Coverage) { 
        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 @@ TEST_P(GraphicsBlendModeCompositionTest, Premultiplied) { 
        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 @@ TEST_P(GraphicsTransformCompositionTest, FLIP_H) { 
            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 @@ TEST_P(GraphicsTransformCompositionTest, FLIP_V) { 
            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 @@ TEST_P(GraphicsTransformCompositionTest, ROT_180) { 
            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);

graphics/composer/aidl/android/hardware/graphics/composer3/vts/functional/composer-vts/ReadbackVts.cpp

+3 −4
Original line number Diff line number Diff line
@@ -187,7 +187,6 @@ void ReadbackHelper::compareColorBuffers(std::vector<Color>& expectedColors, voi 
            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 @@ void ReadbackHelper::compareColorBuffers(std::vector<Color>& expectedColors, voi 
}



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::setReadbackBuffer() { 
}



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 @@ void ReadbackBuffer::checkReadbackBuffer(std::vector<Color> expectedColors) { 
    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);

graphics/composer/aidl/android/hardware/graphics/composer3/vts/functional/composer-vts/include/ReadbackVts.h

+1 −2
Original line number Diff line number Diff line
@@ -196,8 +196,7 @@ class ReadbackHelper { 


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();