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

Commit 3e037cf6 authored by Alec Mouri's avatar Alec Mouri
Browse files

Remove stride from VTS classes 

Stride must be retrieved from the graphic buffer immediately prior to
CPU access. The previous code was tripped up by an error-prone situation
where stride was never initialized, causing UB.

Also this patch sneaks in a couple of const modifiers for methods that
pass-by-ref.

Bug: 213493262
Test: VtsHalGraphicsComposer3_TargetTest
Change-Id: Ic601cca128b71d36876ed25bd55140b02cb0ad0f
parent 704402e4

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

+20 −15
Original line number Diff line number Diff line
@@ -177,15 +177,15 @@ bool ReadbackHelper::readbackSupported(const common::PixelFormat& pixelFormat, 
    return true;

}



void ReadbackHelper::compareColorBuffers(std::vector<Color>& expectedColors, void* bufferData,

                                         const int32_t stride, const uint32_t width,

void ReadbackHelper::compareColorBuffers(const std::vector<Color>& expectedColors, void* bufferData,

                                         const uint32_t stride, const uint32_t width,

                                         const uint32_t height, common::PixelFormat pixelFormat) {

    const int32_t bytesPerPixel = ReadbackHelper::GetBytesPerPixel(pixelFormat);

    ASSERT_NE(-1, bytesPerPixel);

    for (int row = 0; row < height; row++) {

        for (int col = 0; col < width; col++) {

            auto pixel = row * static_cast<int32_t>(width) + col;

            int offset = (row * stride + col) * bytesPerPixel;

            int offset = (row * static_cast<int32_t>(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]);
@@ -239,16 +239,19 @@ void ReadbackBuffer::checkReadbackBuffer(std::vector<Color> expectedColors) { 
    ndk::ScopedFileDescriptor fenceHandle;

    EXPECT_TRUE(mComposerClient->getReadbackBufferFence(mDisplay, &fenceHandle).isOk());



    int outBytesPerPixel;

    int outBytesPerStride;

    int bytesPerPixel = -1;

    int bytesPerStride = -1;

    void* bufData = nullptr;



    auto status = mGraphicBuffer->lockAsync(mUsage, mAccessRegion, &bufData, dup(fenceHandle.get()),

                                            &outBytesPerPixel, &outBytesPerStride);

                                            &bytesPerPixel, &bytesPerStride);

    EXPECT_EQ(::android::OK, status);

    ASSERT_TRUE(mPixelFormat == PixelFormat::RGB_888 || mPixelFormat == PixelFormat::RGBA_8888);

    ReadbackHelper::compareColorBuffers(expectedColors, bufData, static_cast<int32_t>(mStride),

                                        mWidth, mHeight, mPixelFormat);

    const uint32_t stride = (bytesPerPixel > 0 && bytesPerStride > 0)

                                    ? static_cast<uint32_t>(bytesPerStride / bytesPerPixel)

                                    : mGraphicBuffer->getStride();

    ReadbackHelper::compareColorBuffers(expectedColors, bufData, stride, mWidth, mHeight,

                                        mPixelFormat);

    status = mGraphicBuffer->unlock();

    EXPECT_EQ(::android::OK, status);

}
@@ -304,10 +307,7 @@ LayerSettings TestBufferLayer::toRenderEngineLayerSettings() { 
    LayerSettings layerSettings = TestLayer::toRenderEngineLayerSettings();

    layerSettings.source.buffer.buffer =

            std::make_shared<::android::renderengine::impl::ExternalTexture>(

                    ::android::sp<::android::GraphicBuffer>::make(

                            mGraphicBuffer->handle, ::android::GraphicBuffer::CLONE_HANDLE, mWidth,

                            mHeight, static_cast<int32_t>(mPixelFormat), 1, mUsage, mStride),

                    mRenderEngine.getInternalRenderEngine(),

                    mGraphicBuffer, mRenderEngine.getInternalRenderEngine(),

                    ::android::renderengine::impl::ExternalTexture::Usage::READABLE);



    layerSettings.source.buffer.usePremultipliedAlpha = mBlendMode == BlendMode::PREMULTIPLIED;
@@ -318,7 +318,7 @@ LayerSettings TestBufferLayer::toRenderEngineLayerSettings() { 
    const float translateY = mSourceCrop.top / (static_cast<float>(mHeight));



    layerSettings.source.buffer.textureTransform =

            ::android::mat4::translate(::android::vec4(translateX, translateY, 0, 1)) *

            ::android::mat4::translate(::android::vec4(translateX, translateY, 0, 1.0)) *

            ::android::mat4::scale(::android::vec4(scaleX, scaleY, 1.0, 1.0));



    return layerSettings;
@@ -326,9 +326,14 @@ LayerSettings TestBufferLayer::toRenderEngineLayerSettings() { 


void TestBufferLayer::fillBuffer(std::vector<Color>& expectedColors) {

    void* bufData;

    auto status = mGraphicBuffer->lock(mUsage, &bufData);

    int32_t bytesPerPixel = -1;

    int32_t bytesPerStride = -1;

    auto status = mGraphicBuffer->lock(mUsage, &bufData, &bytesPerPixel, &bytesPerStride);

    const uint32_t stride = (bytesPerPixel > 0 && bytesPerStride > 0)

                                    ? static_cast<uint32_t>(bytesPerStride / bytesPerPixel)

                                    : mGraphicBuffer->getStride();

    EXPECT_EQ(::android::OK, status);

    ASSERT_NO_FATAL_FAILURE(ReadbackHelper::fillBuffer(mWidth, mHeight, mStride, bufData,

    ASSERT_NO_FATAL_FAILURE(ReadbackHelper::fillBuffer(mWidth, mHeight, stride, bufData,

                                                       mPixelFormat, expectedColors));

    EXPECT_EQ(::android::OK, mGraphicBuffer->unlock());

}

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

+11 −6
Original line number Diff line number Diff line
@@ -77,13 +77,18 @@ void TestRenderEngine::drawLayers() { 
    }

}



void TestRenderEngine::checkColorBuffer(std::vector<Color>& expectedColors) {

void TestRenderEngine::checkColorBuffer(const std::vector<Color>& expectedColors) {

    void* bufferData;

    ASSERT_EQ(0,

              mGraphicBuffer->lock(static_cast<uint32_t>(mGraphicBuffer->getUsage()), &bufferData));

    ReadbackHelper::compareColorBuffers(

            expectedColors, bufferData, static_cast<int32_t>(mGraphicBuffer->getStride()),

            mGraphicBuffer->getWidth(), mGraphicBuffer->getHeight(), mFormat);

    int32_t bytesPerPixel = -1;

    int32_t bytesPerStride = -1;

    ASSERT_EQ(0, mGraphicBuffer->lock(static_cast<uint32_t>(mGraphicBuffer->getUsage()),

                                      &bufferData, &bytesPerPixel, &bytesPerStride));

    const uint32_t stride = (bytesPerPixel > 0 && bytesPerStride > 0)

                                    ? static_cast<uint32_t>(bytesPerStride / bytesPerPixel)

                                    : mGraphicBuffer->getStride();

    ReadbackHelper::compareColorBuffers(expectedColors, bufferData, stride,

                                        mGraphicBuffer->getWidth(), mGraphicBuffer->getHeight(),

                                        mFormat);

    ASSERT_EQ(::android::OK, mGraphicBuffer->unlock());

}

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

+2 −4
Original line number Diff line number Diff line
@@ -161,7 +161,6 @@ class TestBufferLayer : public TestLayer { 
    uint32_t mLayerCount;

    PixelFormat mPixelFormat;

    uint32_t mUsage;

    uint32_t mStride;

    ::android::Rect mAccessRegion;

};
@@ -189,8 +188,8 @@ class ReadbackHelper { 
    static const std::vector<ColorMode> colorModes;

    static const std::vector<Dataspace> dataspaces;



    static void compareColorBuffers(std::vector<Color>& expectedColors, void* bufferData,

                                    const int32_t stride, const uint32_t width,

    static void compareColorBuffers(const std::vector<Color>& expectedColors, void* bufferData,

                                    const uint32_t stride, const uint32_t width,

                                    const uint32_t height, PixelFormat pixelFormat);

};
@@ -210,7 +209,6 @@ class ReadbackBuffer { 
    uint32_t mHeight;

    uint32_t mLayerCount;

    uint32_t mUsage;

    uint32_t mStride;

    PixelFormat mPixelFormat;

    Dataspace mDataspace;

    int64_t mDisplay;

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

+1 −1
Original line number Diff line number Diff line
@@ -54,7 +54,7 @@ class TestRenderEngine { 
        mDisplaySettings = displaySettings;

    };

    void drawLayers();

    void checkColorBuffer(std::vector<Color>& expectedColors);

    void checkColorBuffer(const std::vector<Color>& expectedColors);



    ::android::renderengine::RenderEngine& getInternalRenderEngine() { return *mRenderEngine; }