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

Commit f07b85be authored by Dominik Laskowski's avatar Dominik Laskowski
Browse files

SF: Remove DisplayDeviceState::isValid 

SurfaceFlinger::mCurrentState::displays does not contain invalid
DisplayDeviceState, so lookup is sufficient to determine whether
a display token has associated DisplayDeviceState.

Bug: 74619554
Test: libsurfaceflinger_unittest
Change-Id: I618abcb4f0e0f4a808247d59d7d4c4f6ac07a352
parent 00a6fa2f

services/surfaceflinger/DisplayDevice.h

+0 −1
Original line number Diff line number Diff line
@@ -315,7 +315,6 @@ private: 
};



struct DisplayDeviceState {

    bool isValid() const { return type >= 0; }

    bool isVirtual() const { return type >= DisplayDevice::DISPLAY_VIRTUAL; }



    int32_t sequenceId = sNextSequenceId++;

services/surfaceflinger/SurfaceFlinger.cpp

+38 −40
Original line number Diff line number Diff line
@@ -3198,53 +3198,51 @@ void SurfaceFlinger::setTransactionState( 
    }

}



uint32_t SurfaceFlinger::setDisplayStateLocked(const DisplayState& s)

{

    ssize_t dpyIdx = mCurrentState.displays.indexOfKey(s.token);

    if (dpyIdx < 0)

        return 0;

uint32_t SurfaceFlinger::setDisplayStateLocked(const DisplayState& s) {

    const ssize_t index = mCurrentState.displays.indexOfKey(s.token);

    if (index < 0) return 0;



    uint32_t flags = 0;

    DisplayDeviceState& disp(mCurrentState.displays.editValueAt(dpyIdx));

    if (disp.isValid()) {

    DisplayDeviceState& state = mCurrentState.displays.editValueAt(index);



    const uint32_t what = s.what;

    if (what & DisplayState::eSurfaceChanged) {

            if (IInterface::asBinder(disp.surface) != IInterface::asBinder(s.surface)) {

                disp.surface = s.surface;

        if (IInterface::asBinder(state.surface) != IInterface::asBinder(s.surface)) {

            state.surface = s.surface;

            flags |= eDisplayTransactionNeeded;

        }

    }

    if (what & DisplayState::eLayerStackChanged) {

            if (disp.layerStack != s.layerStack) {

                disp.layerStack = s.layerStack;

        if (state.layerStack != s.layerStack) {

            state.layerStack = s.layerStack;

            flags |= eDisplayTransactionNeeded;

        }

    }

    if (what & DisplayState::eDisplayProjectionChanged) {

            if (disp.orientation != s.orientation) {

                disp.orientation = s.orientation;

        if (state.orientation != s.orientation) {

            state.orientation = s.orientation;

            flags |= eDisplayTransactionNeeded;

        }

            if (disp.frame != s.frame) {

                disp.frame = s.frame;

        if (state.frame != s.frame) {

            state.frame = s.frame;

            flags |= eDisplayTransactionNeeded;

        }

            if (disp.viewport != s.viewport) {

                disp.viewport = s.viewport;

        if (state.viewport != s.viewport) {

            state.viewport = s.viewport;

            flags |= eDisplayTransactionNeeded;

        }

    }

    if (what & DisplayState::eDisplaySizeChanged) {

            if (disp.width != s.width) {

                disp.width = s.width;

        if (state.width != s.width) {

            state.width = s.width;

            flags |= eDisplayTransactionNeeded;

        }

            if (disp.height != s.height) {

                disp.height = s.height;

        if (state.height != s.height) {

            state.height = s.height;

            flags |= eDisplayTransactionNeeded;

        }

    }

    }



    return flags;

}

services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp

+3 −42
Original line number Diff line number Diff line
@@ -245,6 +245,8 @@ struct DisplayVariant { 


    // The type for this display

    static constexpr DisplayDevice::DisplayType TYPE = type;

    static_assert(TYPE != DisplayDevice::DISPLAY_ID_INVALID);



    static constexpr DisplayDevice::DisplayType DISPLAY_ID = displayId;



    // When creating native window surfaces for the framebuffer, whether those should be critical
@@ -385,11 +387,6 @@ struct PhysicalDisplayVariant 
                                 DisplayVariant<type, type, width, height, critical, Async::FALSE,

                                                Secure::TRUE, GRALLOC_USAGE_PHYSICAL_DISPLAY>> {};



// An invalid display

using InvalidDisplayVariant =

        DisplayVariant<DisplayDevice::DISPLAY_ID_INVALID, DisplayDevice::DISPLAY_ID_INVALID, 0, 0,

                       Critical::FALSE, Async::FALSE, Secure::FALSE, 0>;



// A primary display is a physical display that is critical

using PrimaryDisplayVariant =

        PhysicalDisplayVariant<1001, DisplayDevice::DISPLAY_PRIMARY, 3840, 2160, Critical::TRUE>;
@@ -686,9 +683,7 @@ using HdrCta861_3_DisplayCase = 
        Case<PrimaryDisplayVariant, WideColorNotSupportedVariant<PrimaryDisplayVariant>,

             HdrNotSupportedVariant<PrimaryDisplayVariant>,

             Cta861_3_PerFrameMetadataSupportVariant<PrimaryDisplayVariant>>;

using InvalidDisplayCase = Case<InvalidDisplayVariant, WideColorSupportNotConfiguredVariant,

                                NonHwcDisplayHdrSupportVariant,

                                NoPerFrameMetadataSupportVariant<InvalidDisplayVariant>>;



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

 *

 * SurfaceFlinger::onHotplugReceived
@@ -1834,40 +1829,6 @@ TEST_F(DisplayTransactionTest, setDisplayStateLockedDoesNothingWithUnknownDispla 
    EXPECT_FALSE(hasCurrentDisplayState(displayToken));

}



TEST_F(DisplayTransactionTest, setDisplayStateLockedDoesNothingWithInvalidDisplay) {

    using Case = InvalidDisplayCase;



    // --------------------------------------------------------------------

    // Preconditions



    // An invalid display is set up

    auto display = Case::Display::makeFakeExistingDisplayInjector(this);

    display.inject();



    // The invalid display has some state

    display.mutableCurrentDisplayState().layerStack = 654u;



    // The requested display state tries to change the display state.

    DisplayState state;

    state.what = DisplayState::eLayerStackChanged;

    state.token = display.token();

    state.layerStack = 456;



    // --------------------------------------------------------------------

    // Invocation



    uint32_t flags = mFlinger.setDisplayStateLocked(state);



    // --------------------------------------------------------------------

    // Postconditions



    // The returned flags are empty

    EXPECT_EQ(0u, flags);



    // The current display layer stack value is unchanged.

    EXPECT_EQ(654u, getCurrentDisplayState(display.token()).layerStack);

}



TEST_F(DisplayTransactionTest, setDisplayStateLockedDoesNothingWhenNoChanges) {

    using Case = SimplePrimaryDisplayCase;