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

Commit 79e81aa6 authored by arthurhung's avatar arthurhung
Browse files

Fix SurfaceFlinger_test failed when display color mode changed 

The screenshot will be used to verify if it contains the expected
colors in test cases. But the dataspace would depend on the color mode
of the display. And the captured screenshot could be different to the
native values and cause the test failed because it check the unexpected
color.

- Add 'useRGBColorSpace' in CaptureArgs to force capture screenshot in
  native rgb so we could use it to verify the expected color.
- getActiveConfig will return the id instead the status, we have to
  check the value if valid.

Bug: 166622214
Test: change color mode and run atest SurfaceFlinger_test
Change-Id: I2440e13f2ba0932b770158a11f2eab1903c77b30
parent 14f2174c

libs/gui/LayerState.cpp

+2 −1
Original line number Diff line number Diff line
@@ -541,6 +541,7 @@ status_t CaptureArgs::write(Parcel& output) const { 
    SAFE_PARCEL(output.writeFloat, frameScale);

    SAFE_PARCEL(output.writeBool, captureSecureLayers);

    SAFE_PARCEL(output.writeInt32, uid);

    SAFE_PARCEL(output.writeBool, useRGBColorSpace);

    return NO_ERROR;

}
@@ -552,7 +553,7 @@ status_t CaptureArgs::read(const Parcel& input) { 
    SAFE_PARCEL(input.readFloat, &frameScale);

    SAFE_PARCEL(input.readBool, &captureSecureLayers);

    SAFE_PARCEL(input.readInt32, &uid);



    SAFE_PARCEL(input.readBool, &useRGBColorSpace);

    return NO_ERROR;

}

libs/gui/include/gui/LayerState.h

+6 −0
Original line number Diff line number Diff line
@@ -341,6 +341,12 @@ struct CaptureArgs { 
    float frameScale{1};

    bool captureSecureLayers{false};

    int32_t uid{UNSET_UID};

    // True to force using RGB color as the capture result.

    // The display may use non-RGB dataspace (ex. displayP3) that could cause pixel data could be

    // different from RGB (byte per color), and failed when checking colors.

    // NOTE: This should only be used for testing since in normal cases, we want the screen

    // capture's colorspace to match the display's colorspace

    bool useRGBColorSpace{false};



    virtual status_t write(Parcel& output) const;

    virtual status_t read(const Parcel& input);

services/surfaceflinger/SurfaceFlinger.cpp

+20 −4
Original line number Diff line number Diff line
@@ -5518,9 +5518,17 @@ status_t SurfaceFlinger::captureDisplay(const DisplayCaptureArgs& args, 
            reqSize = display->getLayerStackSpaceRect().getSize();

        }



        // The dataspace is depended on the color mode of display, that could use non-native mode

        // (ex. displayP3) to enhance the content, but some cases are checking native RGB in bytes,

        // and failed if display is not in native mode. This provide a way to force using native

        // colors when capture.

        if (args.useRGBColorSpace) {

            dataspace = Dataspace::V0_SRGB;

        } else {

            const ui::ColorMode colorMode = display->getCompositionDisplay()->getState().colorMode;

            dataspace = pickDataspaceFromColorMode(colorMode);

        }

    }



    RenderAreaFuture renderAreaFuture = promise::defer([=] {

        return DisplayRenderArea::create(displayWeak, args.sourceCrop, reqSize, dataspace,
@@ -5676,8 +5684,16 @@ status_t SurfaceFlinger::captureLayers(const LayerCaptureArgs& args, 


        layerStackSpaceRect = display->getLayerStackSpaceRect();



        // The dataspace is depended on the color mode of display, that could use non-native mode

        // (ex. displayP3) to enhance the content, but some cases are checking native RGB in bytes,

        // and failed if display is not in native mode. This provide a way to force using native

        // colors when capture.

        if (args.useRGBColorSpace) {

            dataspace = Dataspace::V0_SRGB;

        } else {

            const ui::ColorMode colorMode = display->getCompositionDisplay()->getState().colorMode;

            dataspace = pickDataspaceFromColorMode(colorMode);

        }



        captureSecureLayers = args.captureSecureLayers && display->isSecure();

    } // mStateLock

services/surfaceflinger/tests/Credentials_test.cpp

+1 −1
Original line number Diff line number Diff line
@@ -207,7 +207,7 @@ TEST_F(CredentialsTest, AllowedGetterMethodsTest) { 
    Vector<DisplayConfig> configs;

    ASSERT_EQ(NO_ERROR, SurfaceComposerClient::getDisplayConfigs(display, &configs));



    ASSERT_EQ(NO_ERROR, SurfaceComposerClient::getActiveConfig(display));

    ASSERT_TRUE(SurfaceComposerClient::getActiveConfig(display) >= 0);



    ASSERT_NE(static_cast<ui::ColorMode>(BAD_VALUE),

              SurfaceComposerClient::getActiveColorMode(display));

services/surfaceflinger/tests/utils/ScreenshotUtils.h

+3 −0
Original line number Diff line number Diff line
@@ -50,8 +50,10 @@ public: 
        const auto sf = ComposerService::getComposerService();

        SurfaceComposerClient::Transaction().apply(true);



        captureArgs.useRGBColorSpace = true;

        const sp<SyncScreenCaptureListener> captureListener = new SyncScreenCaptureListener();

        status_t status = sf->captureDisplay(captureArgs, captureListener);



        if (status != NO_ERROR) {

            return status;

        }
@@ -81,6 +83,7 @@ public: 
        const auto sf = ComposerService::getComposerService();

        SurfaceComposerClient::Transaction().apply(true);



        captureArgs.useRGBColorSpace = true;

        const sp<SyncScreenCaptureListener> captureListener = new SyncScreenCaptureListener();

        status_t status = sf->captureLayers(captureArgs, captureListener);

        if (status != NO_ERROR) {