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

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

SF: Remove multithreaded_present flag 

Change (and rename) CompositionEnginePresentTest.worksAsExpected to
avoid an assumption that would now break the test.

Bug: 380251940
Flag: EXEMPT removing multithreaded_present
Test: presubmit
Change-Id: Ifbcbfc7d82e64d909a6e4d310f34bcb6578d1e1f
parent b61e79d3

services/surfaceflinger/CompositionEngine/src/CompositionEngine.cpp

+1 −1
Original line number Original line Diff line number Diff line
@@ -91,7 +91,7 @@ nsecs_t CompositionEngine::getLastFrameRefreshTimestamp() const { 




namespace {

namespace {

void offloadOutputs(Outputs& outputs) {

void offloadOutputs(Outputs& outputs) {

    if (!FlagManager::getInstance().multithreaded_present() || outputs.size() < 2) {

    if (outputs.size() < 2) {

        return;

        return;

    }

    }

services/surfaceflinger/CompositionEngine/tests/CompositionEngineTest.cpp

+27 −30
Original line number Original line Diff line number Diff line
@@ -43,6 +43,10 @@ using ::testing::ReturnRef; 
using ::testing::SaveArg;

using ::testing::SaveArg;

using ::testing::StrictMock;

using ::testing::StrictMock;





static constexpr PhysicalDisplayId kDisplayId1 = PhysicalDisplayId::fromPort(123u);

static constexpr PhysicalDisplayId kDisplayId2 = PhysicalDisplayId::fromPort(234u);

static constexpr PhysicalDisplayId kDisplayId3 = PhysicalDisplayId::fromPort(567u);



struct CompositionEngineTest : public testing::Test {

struct CompositionEngineTest : public testing::Test {

    std::shared_ptr<TimeStats> mTimeStats;

    std::shared_ptr<TimeStats> mTimeStats;
@@ -52,6 +56,26 @@ struct CompositionEngineTest : public testing::Test { 
    std::shared_ptr<mock::Output> mOutput1{std::make_shared<StrictMock<mock::Output>>()};

    std::shared_ptr<mock::Output> mOutput1{std::make_shared<StrictMock<mock::Output>>()};

    std::shared_ptr<mock::Output> mOutput2{std::make_shared<StrictMock<mock::Output>>()};

    std::shared_ptr<mock::Output> mOutput2{std::make_shared<StrictMock<mock::Output>>()};

    std::shared_ptr<mock::Output> mOutput3{std::make_shared<StrictMock<mock::Output>>()};

    std::shared_ptr<mock::Output> mOutput3{std::make_shared<StrictMock<mock::Output>>()};



    std::array<impl::OutputCompositionState, 3> mOutputStates;



    void SetUp() override {

        EXPECT_CALL(*mOutput1, getDisplayId)

                .WillRepeatedly(Return(std::make_optional<DisplayId>(kDisplayId1)));

        EXPECT_CALL(*mOutput2, getDisplayId)

                .WillRepeatedly(Return(std::make_optional<DisplayId>(kDisplayId2)));

        EXPECT_CALL(*mOutput3, getDisplayId)

                .WillRepeatedly(Return(std::make_optional<DisplayId>(kDisplayId3)));



        // Most tests will depend on the outputs being enabled.

        for (auto& state : mOutputStates) {

            state.isEnabled = true;

        }



        EXPECT_CALL(*mOutput1, getState).WillRepeatedly(ReturnRef(mOutputStates[0]));

        EXPECT_CALL(*mOutput2, getState).WillRepeatedly(ReturnRef(mOutputStates[1]));

        EXPECT_CALL(*mOutput3, getState).WillRepeatedly(ReturnRef(mOutputStates[2]));

    }

};

};





TEST_F(CompositionEngineTest, canInstantiateCompositionEngine) {

TEST_F(CompositionEngineTest, canInstantiateCompositionEngine) {
@@ -94,7 +118,7 @@ struct CompositionEnginePresentTest : public CompositionEngineTest { 
    StrictMock<CompositionEnginePartialMock> mEngine;

    StrictMock<CompositionEnginePartialMock> mEngine;

};

};





TEST_F(CompositionEnginePresentTest, worksWithEmptyRequest) {

TEST_F(CompositionEnginePresentTest, zeroOutputs) {

    // present() always calls preComposition() and postComposition()

    // present() always calls preComposition() and postComposition()

    EXPECT_CALL(mEngine, preComposition(Ref(mRefreshArgs)));

    EXPECT_CALL(mEngine, preComposition(Ref(mRefreshArgs)));

    EXPECT_CALL(mEngine, postComposition(Ref(mRefreshArgs)));

    EXPECT_CALL(mEngine, postComposition(Ref(mRefreshArgs)));
@@ -102,7 +126,7 @@ TEST_F(CompositionEnginePresentTest, worksWithEmptyRequest) { 
    mEngine.present(mRefreshArgs);

    mEngine.present(mRefreshArgs);

}

}





TEST_F(CompositionEnginePresentTest, worksAsExpected) {

TEST_F(CompositionEnginePresentTest, threeOutputs) {

    // Expect calls to in a certain sequence

    // Expect calls to in a certain sequence

    InSequence seq;

    InSequence seq;
@@ -114,9 +138,7 @@ TEST_F(CompositionEnginePresentTest, worksAsExpected) { 
    EXPECT_CALL(*mOutput2, prepare(Ref(mRefreshArgs), _));

    EXPECT_CALL(*mOutput2, prepare(Ref(mRefreshArgs), _));

    EXPECT_CALL(*mOutput3, prepare(Ref(mRefreshArgs), _));

    EXPECT_CALL(*mOutput3, prepare(Ref(mRefreshArgs), _));





    // All of mOutput<i> are StrictMocks. If the flag is true, it will introduce

    EXPECT_CALL(*mOutput1, supportsOffloadPresent).WillOnce(Return(false));

    // calls to getDisplayId, which are not relevant to this test.

    SET_FLAG_FOR_TEST(flags::multithreaded_present, false);





    // The last step is to actually present each output.

    // The last step is to actually present each output.

    EXPECT_CALL(*mOutput1, present(Ref(mRefreshArgs)))

    EXPECT_CALL(*mOutput1, present(Ref(mRefreshArgs)))
@@ -284,8 +306,6 @@ struct CompositionEngineOffloadTest : public testing::Test { 
    std::shared_ptr<mock::Output> mVirtualDisplay{std::make_shared<StrictMock<mock::Output>>()};

    std::shared_ptr<mock::Output> mVirtualDisplay{std::make_shared<StrictMock<mock::Output>>()};

    std::shared_ptr<mock::Output> mHalVirtualDisplay{std::make_shared<StrictMock<mock::Output>>()};

    std::shared_ptr<mock::Output> mHalVirtualDisplay{std::make_shared<StrictMock<mock::Output>>()};





    static constexpr PhysicalDisplayId kDisplayId1 = PhysicalDisplayId::fromPort(123u);

    static constexpr PhysicalDisplayId kDisplayId2 = PhysicalDisplayId::fromPort(234u);

    static constexpr GpuVirtualDisplayId kGpuVirtualDisplayId{789u};

    static constexpr GpuVirtualDisplayId kGpuVirtualDisplayId{789u};

    static constexpr HalVirtualDisplayId kHalVirtualDisplayId{456u};

    static constexpr HalVirtualDisplayId kHalVirtualDisplayId{456u};
@@ -332,7 +352,6 @@ TEST_F(CompositionEngineOffloadTest, basic) { 
    EXPECT_CALL(*mDisplay1, offloadPresentNextFrame).Times(1);

    EXPECT_CALL(*mDisplay1, offloadPresentNextFrame).Times(1);

    EXPECT_CALL(*mDisplay2, offloadPresentNextFrame).Times(0);

    EXPECT_CALL(*mDisplay2, offloadPresentNextFrame).Times(0);





    SET_FLAG_FOR_TEST(flags::multithreaded_present, true);

    setOutputs({mDisplay1, mDisplay2});

    setOutputs({mDisplay1, mDisplay2});





    mEngine.present(mRefreshArgs);

    mEngine.present(mRefreshArgs);
@@ -345,7 +364,6 @@ TEST_F(CompositionEngineOffloadTest, dependsOnSupport) { 
    EXPECT_CALL(*mDisplay1, offloadPresentNextFrame).Times(0);

    EXPECT_CALL(*mDisplay1, offloadPresentNextFrame).Times(0);

    EXPECT_CALL(*mDisplay2, offloadPresentNextFrame).Times(0);

    EXPECT_CALL(*mDisplay2, offloadPresentNextFrame).Times(0);





    SET_FLAG_FOR_TEST(flags::multithreaded_present, true);

    setOutputs({mDisplay1, mDisplay2});

    setOutputs({mDisplay1, mDisplay2});





    mEngine.present(mRefreshArgs);

    mEngine.present(mRefreshArgs);
@@ -358,20 +376,6 @@ TEST_F(CompositionEngineOffloadTest, dependsOnSupport2) { 
    EXPECT_CALL(*mDisplay1, offloadPresentNextFrame).Times(0);

    EXPECT_CALL(*mDisplay1, offloadPresentNextFrame).Times(0);

    EXPECT_CALL(*mDisplay2, offloadPresentNextFrame).Times(0);

    EXPECT_CALL(*mDisplay2, offloadPresentNextFrame).Times(0);





    SET_FLAG_FOR_TEST(flags::multithreaded_present, true);

    setOutputs({mDisplay1, mDisplay2});



    mEngine.present(mRefreshArgs);

}



TEST_F(CompositionEngineOffloadTest, dependsOnFlag) {

    EXPECT_CALL(*mDisplay1, supportsOffloadPresent).Times(0);

    EXPECT_CALL(*mDisplay2, supportsOffloadPresent).Times(0);



    EXPECT_CALL(*mDisplay1, offloadPresentNextFrame).Times(0);

    EXPECT_CALL(*mDisplay2, offloadPresentNextFrame).Times(0);



    SET_FLAG_FOR_TEST(flags::multithreaded_present, false);

    setOutputs({mDisplay1, mDisplay2});

    setOutputs({mDisplay1, mDisplay2});





    mEngine.present(mRefreshArgs);

    mEngine.present(mRefreshArgs);
@@ -382,7 +386,6 @@ TEST_F(CompositionEngineOffloadTest, oneDisplay) { 




    EXPECT_CALL(*mDisplay1, offloadPresentNextFrame).Times(0);

    EXPECT_CALL(*mDisplay1, offloadPresentNextFrame).Times(0);





    SET_FLAG_FOR_TEST(flags::multithreaded_present, true);

    setOutputs({mDisplay1});

    setOutputs({mDisplay1});





    mEngine.present(mRefreshArgs);

    mEngine.present(mRefreshArgs);
@@ -397,7 +400,6 @@ TEST_F(CompositionEngineOffloadTest, virtualDisplay) { 
    EXPECT_CALL(*mDisplay2, offloadPresentNextFrame).Times(0);

    EXPECT_CALL(*mDisplay2, offloadPresentNextFrame).Times(0);

    EXPECT_CALL(*mVirtualDisplay, offloadPresentNextFrame).Times(0);

    EXPECT_CALL(*mVirtualDisplay, offloadPresentNextFrame).Times(0);





    SET_FLAG_FOR_TEST(flags::multithreaded_present, true);

    setOutputs({mDisplay1, mDisplay2, mVirtualDisplay});

    setOutputs({mDisplay1, mDisplay2, mVirtualDisplay});





    mEngine.present(mRefreshArgs);

    mEngine.present(mRefreshArgs);
@@ -410,7 +412,6 @@ TEST_F(CompositionEngineOffloadTest, virtualDisplay2) { 
    EXPECT_CALL(*mDisplay1, offloadPresentNextFrame).Times(0);

    EXPECT_CALL(*mDisplay1, offloadPresentNextFrame).Times(0);

    EXPECT_CALL(*mVirtualDisplay, offloadPresentNextFrame).Times(0);

    EXPECT_CALL(*mVirtualDisplay, offloadPresentNextFrame).Times(0);





    SET_FLAG_FOR_TEST(flags::multithreaded_present, true);

    setOutputs({mDisplay1, mVirtualDisplay});

    setOutputs({mDisplay1, mVirtualDisplay});





    mEngine.present(mRefreshArgs);

    mEngine.present(mRefreshArgs);
@@ -423,7 +424,6 @@ TEST_F(CompositionEngineOffloadTest, halVirtual) { 
    EXPECT_CALL(*mDisplay1, offloadPresentNextFrame).Times(1);

    EXPECT_CALL(*mDisplay1, offloadPresentNextFrame).Times(1);

    EXPECT_CALL(*mHalVirtualDisplay, offloadPresentNextFrame).Times(0);

    EXPECT_CALL(*mHalVirtualDisplay, offloadPresentNextFrame).Times(0);





    SET_FLAG_FOR_TEST(flags::multithreaded_present, true);

    setOutputs({mDisplay1, mHalVirtualDisplay});

    setOutputs({mDisplay1, mHalVirtualDisplay});





    mEngine.present(mRefreshArgs);

    mEngine.present(mRefreshArgs);
@@ -440,7 +440,6 @@ TEST_F(CompositionEngineOffloadTest, ordering) { 
    EXPECT_CALL(*mDisplay1, offloadPresentNextFrame).Times(1);

    EXPECT_CALL(*mDisplay1, offloadPresentNextFrame).Times(1);

    EXPECT_CALL(*mDisplay2, offloadPresentNextFrame).Times(0);

    EXPECT_CALL(*mDisplay2, offloadPresentNextFrame).Times(0);





    SET_FLAG_FOR_TEST(flags::multithreaded_present, true);

    setOutputs({mVirtualDisplay, mHalVirtualDisplay, mDisplay1, mDisplay2});

    setOutputs({mVirtualDisplay, mHalVirtualDisplay, mDisplay1, mDisplay2});





    mEngine.present(mRefreshArgs);

    mEngine.present(mRefreshArgs);
@@ -458,7 +457,6 @@ TEST_F(CompositionEngineOffloadTest, dependsOnEnabled) { 
    EXPECT_CALL(*mDisplay1, offloadPresentNextFrame).Times(0);

    EXPECT_CALL(*mDisplay1, offloadPresentNextFrame).Times(0);

    EXPECT_CALL(*mDisplay2, offloadPresentNextFrame).Times(0);

    EXPECT_CALL(*mDisplay2, offloadPresentNextFrame).Times(0);





    SET_FLAG_FOR_TEST(flags::multithreaded_present, true);

    setOutputs({mDisplay1, mDisplay2});

    setOutputs({mDisplay1, mDisplay2});





    mEngine.present(mRefreshArgs);

    mEngine.present(mRefreshArgs);
@@ -478,7 +476,6 @@ TEST_F(CompositionEngineOffloadTest, disabledDisplaysDoNotPreventOthersFromOfflo 
    EXPECT_CALL(*mDisplay2, offloadPresentNextFrame).Times(0);

    EXPECT_CALL(*mDisplay2, offloadPresentNextFrame).Times(0);

    EXPECT_CALL(*mHalVirtualDisplay, offloadPresentNextFrame).Times(0);

    EXPECT_CALL(*mHalVirtualDisplay, offloadPresentNextFrame).Times(0);





    SET_FLAG_FOR_TEST(flags::multithreaded_present, true);

    setOutputs({mDisplay1, mDisplay2, mHalVirtualDisplay});

    setOutputs({mDisplay1, mDisplay2, mHalVirtualDisplay});





    mEngine.present(mRefreshArgs);

    mEngine.present(mRefreshArgs);

services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp

+0 −1
Original line number Original line Diff line number Diff line
@@ -1773,7 +1773,6 @@ void AidlComposer::onHotplugDisconnect(Display display) { 
}

}





bool AidlComposer::hasMultiThreadedPresentSupport(Display display) {

bool AidlComposer::hasMultiThreadedPresentSupport(Display display) {

    if (!FlagManager::getInstance().multithreaded_present()) return false;

    const auto displayId = translate<int64_t>(display);

    const auto displayId = translate<int64_t>(display);

    std::vector<AidlDisplayCapability> capabilities;

    std::vector<AidlDisplayCapability> capabilities;

    const auto status = mAidlComposerClient->getDisplayCapabilities(displayId, &capabilities);

    const auto status = mAidlComposerClient->getDisplayCapabilities(displayId, &capabilities);

services/surfaceflinger/Scheduler/Scheduler.cpp

+1 −2
Original line number Original line Diff line number Diff line
@@ -554,8 +554,7 @@ void Scheduler::resyncAllToHardwareVsync(bool allowToEnable) { 
    ftl::FakeGuard guard(kMainThreadContext);

    ftl::FakeGuard guard(kMainThreadContext);





    for (const auto& [id, display] : mDisplays) {

    for (const auto& [id, display] : mDisplays) {

        if (display.powerMode != hal::PowerMode::OFF ||

        if (display.powerMode != hal::PowerMode::OFF) {

            !FlagManager::getInstance().multithreaded_present()) {

            resyncToHardwareVsyncLocked(id, allowToEnable);

            resyncToHardwareVsyncLocked(id, allowToEnable);

        }

        }

    }

    }

services/surfaceflinger/SurfaceFlinger.cpp

+6 −12
Original line number Original line Diff line number Diff line
@@ -5731,8 +5731,7 @@ void SurfaceFlinger::setPhysicalDisplayPowerMode(const sp<DisplayDevice>& displa 
        }

        }





        getHwComposer().setPowerMode(displayId, mode);

        getHwComposer().setPowerMode(displayId, mode);

        if (mode != hal::PowerMode::DOZE_SUSPEND &&

        if (mode != hal::PowerMode::DOZE_SUSPEND) {

            (displayId == mActiveDisplayId || FlagManager::getInstance().multithreaded_present())) {

            const bool enable =

            const bool enable =

                    mScheduler->getVsyncSchedule(displayId)->getPendingHardwareVsyncState();

                    mScheduler->getVsyncSchedule(displayId)->getPendingHardwareVsyncState();

            requestHardwareVsync(displayId, enable);

            requestHardwareVsync(displayId, enable);
@@ -5759,14 +5758,11 @@ void SurfaceFlinger::setPhysicalDisplayPowerMode(const sp<DisplayDevice>& displa 
                setSchedAttr(false, kWhence);

                setSchedAttr(false, kWhence);





                if (currentModeNotDozeSuspend) {

                if (currentModeNotDozeSuspend) {

                    if (!FlagManager::getInstance().multithreaded_present()) {

                        mScheduler->disableHardwareVsync(displayId, true);

                    }

                    mScheduler->enableSyntheticVsync();

                    mScheduler->enableSyntheticVsync();

                }

                }

            }

            }

        }

        }

        if (currentModeNotDozeSuspend && FlagManager::getInstance().multithreaded_present()) {

        if (currentModeNotDozeSuspend) {

            constexpr bool kDisallow = true;

            constexpr bool kDisallow = true;

            mScheduler->disableHardwareVsync(displayId, kDisallow);

            mScheduler->disableHardwareVsync(displayId, kDisallow);

        }

        }
@@ -5784,8 +5780,7 @@ void SurfaceFlinger::setPhysicalDisplayPowerMode(const sp<DisplayDevice>& displa 
    } else if (mode == hal::PowerMode::DOZE || mode == hal::PowerMode::ON) {

    } else if (mode == hal::PowerMode::DOZE || mode == hal::PowerMode::ON) {

        // Update display while dozing

        // Update display while dozing

        getHwComposer().setPowerMode(displayId, mode);

        getHwComposer().setPowerMode(displayId, mode);

        if (currentMode == hal::PowerMode::DOZE_SUSPEND &&

        if (currentMode == hal::PowerMode::DOZE_SUSPEND) {

            (displayId == mActiveDisplayId || FlagManager::getInstance().multithreaded_present())) {

            if (displayId == mActiveDisplayId) {

            if (displayId == mActiveDisplayId) {

                ALOGI("Force repainting for DOZE_SUSPEND -> DOZE or ON.");

                ALOGI("Force repainting for DOZE_SUSPEND -> DOZE or ON.");

                mVisibleRegionsDirty = true;

                mVisibleRegionsDirty = true;
@@ -5797,10 +5792,9 @@ void SurfaceFlinger::setPhysicalDisplayPowerMode(const sp<DisplayDevice>& displa 
        }

        }

    } else if (mode == hal::PowerMode::DOZE_SUSPEND) {

    } else if (mode == hal::PowerMode::DOZE_SUSPEND) {

        // Leave display going to doze

        // Leave display going to doze

        if (displayId == mActiveDisplayId || FlagManager::getInstance().multithreaded_present()) {

        constexpr bool kDisallow = true;

        constexpr bool kDisallow = true;

        mScheduler->disableHardwareVsync(displayId, kDisallow);

        mScheduler->disableHardwareVsync(displayId, kDisallow);

        }



        if (displayId == mActiveDisplayId) {

        if (displayId == mActiveDisplayId) {

            mScheduler->enableSyntheticVsync();

            mScheduler->enableSyntheticVsync();

        }

        }