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

Commit 8a64a835 authored by TreeHugger Robot's avatar TreeHugger Robot Committed by Android (Google) Code Review
Browse files

Merge "SF: Color changes should dirty entire display"

parents e04bf9f9 ef958122

services/surfaceflinger/CompositionEngine/src/Output.cpp

+16 −2
Original line number Diff line number Diff line
@@ -90,13 +90,25 @@ void Output::setLayerStackFilter(uint32_t layerStackId, bool isInternal) { 


void Output::setColorTransform(const mat4& transform) {

    const bool isIdentity = (transform == mat4());



    mState.colorTransform =

    const auto newColorTransform =

            isIdentity ? HAL_COLOR_TRANSFORM_IDENTITY : HAL_COLOR_TRANSFORM_ARBITRARY_MATRIX;



    if (mState.colorTransform == newColorTransform) {

        return;

    }



    mState.colorTransform = newColorTransform;



    dirtyEntireOutput();

}



void Output::setColorMode(ui::ColorMode mode, ui::Dataspace dataspace,

                          ui::RenderIntent renderIntent) {

    if (mState.colorMode == mode && mState.dataspace == dataspace &&

        mState.renderIntent == renderIntent) {

        return;

    }



    mState.colorMode = mode;

    mState.dataspace = dataspace;

    mState.renderIntent = renderIntent;
@@ -106,6 +118,8 @@ void Output::setColorMode(ui::ColorMode mode, ui::Dataspace dataspace, 
    ALOGV("Set active color mode: %s (%d), active render intent: %s (%d)",

          decodeColorMode(mode).c_str(), mode, decodeRenderIntent(renderIntent).c_str(),

          renderIntent);



    dirtyEntireOutput();

}



void Output::dump(std::string& out) const {

services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp

+6 −6
Original line number Diff line number Diff line
@@ -156,17 +156,17 @@ TEST_F(DisplayTest, setColorModeSetsModeUnlessNoChange) { 
    EXPECT_EQ(ui::RenderIntent::COLORIMETRIC, mDisplay.getState().renderIntent);



    // Otherwise if the values are different, updates happen

    EXPECT_CALL(*renderSurface, setBufferDataspace(ui::Dataspace::SRGB)).Times(1);

    EXPECT_CALL(*renderSurface, setBufferDataspace(ui::Dataspace::DISPLAY_P3)).Times(1);

    EXPECT_CALL(mHwComposer,

                setActiveColorMode(DEFAULT_DISPLAY_ID, ui::ColorMode::BT2100_PQ,

                setActiveColorMode(DEFAULT_DISPLAY_ID, ui::ColorMode::DISPLAY_P3,

                                   ui::RenderIntent::TONE_MAP_COLORIMETRIC))

            .Times(1);



    mDisplay.setColorMode(ui::ColorMode::BT2100_PQ, ui::Dataspace::SRGB,

    mDisplay.setColorMode(ui::ColorMode::DISPLAY_P3, ui::Dataspace::DISPLAY_P3,

                          ui::RenderIntent::TONE_MAP_COLORIMETRIC);



    EXPECT_EQ(ui::ColorMode::BT2100_PQ, mDisplay.getState().colorMode);

    EXPECT_EQ(ui::Dataspace::SRGB, mDisplay.getState().dataspace);

    EXPECT_EQ(ui::ColorMode::DISPLAY_P3, mDisplay.getState().colorMode);

    EXPECT_EQ(ui::Dataspace::DISPLAY_P3, mDisplay.getState().dataspace);

    EXPECT_EQ(ui::RenderIntent::TONE_MAP_COLORIMETRIC, mDisplay.getState().renderIntent);

}
@@ -174,7 +174,7 @@ TEST_F(DisplayTest, setColorModeDoesNothingForVirtualDisplay) { 
    impl::Display virtualDisplay{mCompositionEngine,

                                 DisplayCreationArgs{false, true, DEFAULT_DISPLAY_ID}};



    virtualDisplay.setColorMode(ui::ColorMode::BT2100_PQ, ui::Dataspace::SRGB,

    virtualDisplay.setColorMode(ui::ColorMode::DISPLAY_P3, ui::Dataspace::DISPLAY_P3,

                                ui::RenderIntent::TONE_MAP_COLORIMETRIC);



    EXPECT_EQ(ui::ColorMode::NATIVE, virtualDisplay.getState().colorMode);

services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp

+33 −18
Original line number Diff line number Diff line
@@ -43,15 +43,21 @@ public: 
        mOutput.setDisplayColorProfileForTest(

                std::unique_ptr<DisplayColorProfile>(mDisplayColorProfile));

        mOutput.setRenderSurfaceForTest(std::unique_ptr<RenderSurface>(mRenderSurface));



        mOutput.editState().bounds = kDefaultDisplaySize;

    }

    ~OutputTest() override = default;



    static const Rect kDefaultDisplaySize;



    StrictMock<mock::CompositionEngine> mCompositionEngine;

    mock::DisplayColorProfile* mDisplayColorProfile = new StrictMock<mock::DisplayColorProfile>();

    mock::RenderSurface* mRenderSurface = new StrictMock<mock::RenderSurface>();

    impl::Output mOutput{mCompositionEngine};

};



const Rect OutputTest::kDefaultDisplaySize{100, 200};



/* ------------------------------------------------------------------------

 * Basic construction

 */
@@ -76,8 +82,6 @@ TEST_F(OutputTest, canInstantiateOutput) { 
 */



TEST_F(OutputTest, setCompositionEnabledDoesNothingIfAlreadyEnabled) {

    const Rect displaySize{100, 200};

    mOutput.editState().bounds = displaySize;

    mOutput.editState().isEnabled = true;



    mOutput.setCompositionEnabled(true);
@@ -87,25 +91,21 @@ TEST_F(OutputTest, setCompositionEnabledDoesNothingIfAlreadyEnabled) { 
}



TEST_F(OutputTest, setCompositionEnabledSetsEnabledAndDirtiesEntireOutput) {

    const Rect displaySize{100, 200};

    mOutput.editState().bounds = displaySize;

    mOutput.editState().isEnabled = false;



    mOutput.setCompositionEnabled(true);



    EXPECT_TRUE(mOutput.getState().isEnabled);

    EXPECT_THAT(mOutput.getState().dirtyRegion, RegionEq(Region(displaySize)));

    EXPECT_THAT(mOutput.getState().dirtyRegion, RegionEq(Region(kDefaultDisplaySize)));

}



TEST_F(OutputTest, setCompositionEnabledSetsDisabledAndDirtiesEntireOutput) {

    const Rect displaySize{100, 200};

    mOutput.editState().bounds = displaySize;

    mOutput.editState().isEnabled = true;



    mOutput.setCompositionEnabled(false);



    EXPECT_FALSE(mOutput.getState().isEnabled);

    EXPECT_THAT(mOutput.getState().dirtyRegion, RegionEq(Region(displaySize)));

    EXPECT_THAT(mOutput.getState().dirtyRegion, RegionEq(Region(kDefaultDisplaySize)));

}



/* ------------------------------------------------------------------------
@@ -135,7 +135,7 @@ TEST_F(OutputTest, setProjectionTriviallyWorks) { 
 */



TEST_F(OutputTest, setBoundsSetsSizeAndDirtiesEntireOutput) {

    const ui::Size displaySize{100, 200};

    const ui::Size displaySize{200, 400};



    EXPECT_CALL(*mRenderSurface, setDisplaySize(displaySize)).Times(1);

    EXPECT_CALL(*mRenderSurface, getSize()).WillOnce(ReturnRef(displaySize));
@@ -152,16 +152,13 @@ TEST_F(OutputTest, setBoundsSetsSizeAndDirtiesEntireOutput) { 
 */



TEST_F(OutputTest, setLayerStackFilterSetsFilterAndDirtiesEntireOutput) {

    const Rect displaySize{100, 200};

    mOutput.editState().bounds = displaySize;



    const uint32_t layerStack = 123u;

    mOutput.setLayerStackFilter(layerStack, true);



    EXPECT_TRUE(mOutput.getState().layerStackInternal);

    EXPECT_EQ(layerStack, mOutput.getState().layerStackId);



    EXPECT_THAT(mOutput.getState().dirtyRegion, RegionEq(Region(displaySize)));

    EXPECT_THAT(mOutput.getState().dirtyRegion, RegionEq(Region(kDefaultDisplaySize)));

}



/* ------------------------------------------------------------------------
@@ -176,27 +173,45 @@ TEST_F(OutputTest, setColorTransformSetsTransform) { 


    EXPECT_EQ(HAL_COLOR_TRANSFORM_IDENTITY, mOutput.getState().colorTransform);



    // Since identity is the default, the dirty region should be unchanged (empty)

    EXPECT_THAT(mOutput.getState().dirtyRegion, RegionEq(Region()));



    // Non-identity matrix sets a non-identity state value

    const mat4 nonIdentity = mat4() * 2;



    mOutput.setColorTransform(nonIdentity);



    EXPECT_EQ(HAL_COLOR_TRANSFORM_ARBITRARY_MATRIX, mOutput.getState().colorTransform);



    // Since this is a state change, the entire output should now be dirty.

    EXPECT_THAT(mOutput.getState().dirtyRegion, RegionEq(Region(kDefaultDisplaySize)));

}



/* ------------------------------------------------------------------------

 * Output::setColorMode

 */



TEST_F(OutputTest, setColorModeSetsModeUnlessNoChange) {

    EXPECT_CALL(*mRenderSurface, setBufferDataspace(ui::Dataspace::SRGB)).Times(1);

TEST_F(OutputTest, setColorModeSetsStateAndDirtiesOutputIfChanged) {

    EXPECT_CALL(*mRenderSurface, setBufferDataspace(ui::Dataspace::DISPLAY_P3)).Times(1);



    mOutput.setColorMode(ui::ColorMode::BT2100_PQ, ui::Dataspace::SRGB,

    mOutput.setColorMode(ui::ColorMode::DISPLAY_P3, ui::Dataspace::DISPLAY_P3,

                         ui::RenderIntent::TONE_MAP_COLORIMETRIC);



    EXPECT_EQ(ui::ColorMode::BT2100_PQ, mOutput.getState().colorMode);

    EXPECT_EQ(ui::Dataspace::SRGB, mOutput.getState().dataspace);

    EXPECT_EQ(ui::ColorMode::DISPLAY_P3, mOutput.getState().colorMode);

    EXPECT_EQ(ui::Dataspace::DISPLAY_P3, mOutput.getState().dataspace);

    EXPECT_EQ(ui::RenderIntent::TONE_MAP_COLORIMETRIC, mOutput.getState().renderIntent);

    EXPECT_THAT(mOutput.getState().dirtyRegion, RegionEq(Region(kDefaultDisplaySize)));

}



TEST_F(OutputTest, setColorModeDoesNothingIfNoChange) {

    mOutput.editState().colorMode = ui::ColorMode::DISPLAY_P3;

    mOutput.editState().dataspace = ui::Dataspace::DISPLAY_P3;

    mOutput.editState().renderIntent = ui::RenderIntent::TONE_MAP_COLORIMETRIC;



    mOutput.setColorMode(ui::ColorMode::DISPLAY_P3, ui::Dataspace::DISPLAY_P3,

                         ui::RenderIntent::TONE_MAP_COLORIMETRIC);



    EXPECT_THAT(mOutput.getState().dirtyRegion, RegionEq(Region()));

}



/* ------------------------------------------------------------------------