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

Commit 44e9f3b5 authored by Ady Abraham's avatar Ady Abraham
Browse files

SF: fix layer activation with scheduler 

When a layer frame rate vote changes (via setFrameRate call),
the layers needs to be activated in scheduler. This change fixes a
bug that was activating all layers instead of just the layers that
has a vote change

Bug: 163079696
Bug: 180014293
Test: run a test app that calls setFrameRate
Change-Id: Iaa538d04c535b185161a24a4bbadeff2ac99bcd8
parent 76a16cf5

services/surfaceflinger/Layer.cpp

+8 −7
Original line number Original line Diff line number Diff line
@@ -1468,7 +1468,7 @@ void Layer::updateTreeHasFrameRateVote() { 
    // First traverse the tree and count how many layers has votes. In addition

    // First traverse the tree and count how many layers has votes. In addition

    // activate the layers in Scheduler's LayerHistory for it to check for changes

    // activate the layers in Scheduler's LayerHistory for it to check for changes

    int layersWithVote = 0;

    int layersWithVote = 0;

    traverseTree([&layersWithVote, this](Layer* layer) {

    traverseTree([&layersWithVote](Layer* layer) {

        const auto layerVotedWithDefaultCompatibility =

        const auto layerVotedWithDefaultCompatibility =

                layer->mCurrentState.frameRate.rate.isValid() &&

                layer->mCurrentState.frameRate.rate.isValid() &&

                layer->mCurrentState.frameRate.type == FrameRateCompatibility::Default;

                layer->mCurrentState.frameRate.type == FrameRateCompatibility::Default;
@@ -1484,20 +1484,21 @@ void Layer::updateTreeHasFrameRateVote() { 
            layerVotedWithExactCompatibility) {

            layerVotedWithExactCompatibility) {

            layersWithVote++;

            layersWithVote++;

        }

        }



        mFlinger->mScheduler->recordLayerHistory(layer, systemTime(),

                                                 LayerHistory::LayerUpdateType::SetFrameRate);

    });

    });





    // Now update the other layers

    // Now update the other layers

    bool transactionNeeded = false;

    bool transactionNeeded = false;

    traverseTree([layersWithVote, &transactionNeeded](Layer* layer) {

    traverseTree([layersWithVote, &transactionNeeded, this](Layer* layer) {

        if (layer->mCurrentState.treeHasFrameRateVote != layersWithVote > 0) {

        const bool treeHasFrameRateVote = layersWithVote > 0;

        if (layer->mCurrentState.treeHasFrameRateVote != treeHasFrameRateVote) {

            layer->mCurrentState.sequence++;

            layer->mCurrentState.sequence++;

            layer->mCurrentState.treeHasFrameRateVote = layersWithVote > 0;

            layer->mCurrentState.treeHasFrameRateVote = treeHasFrameRateVote;

            layer->mCurrentState.modified = true;

            layer->mCurrentState.modified = true;

            layer->setTransactionFlags(eTransactionNeeded);

            layer->setTransactionFlags(eTransactionNeeded);

            transactionNeeded = true;

            transactionNeeded = true;



            mFlinger->mScheduler->recordLayerHistory(layer, systemTime(),

                                                     LayerHistory::LayerUpdateType::SetFrameRate);

        }

        }

    });

    });

services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp

+26 −2
Original line number Original line Diff line number Diff line
@@ -81,7 +81,7 @@ public: 
    std::string name() override { return "BufferStateLayer"; }

    std::string name() override { return "BufferStateLayer"; }

    sp<Layer> createLayer(TestableSurfaceFlinger& flinger) override {

    sp<Layer> createLayer(TestableSurfaceFlinger& flinger) override {

        sp<Client> client;

        sp<Client> client;

        LayerCreationArgs args(flinger.flinger(), client, "buffer-queue-layer", WIDTH, HEIGHT,

        LayerCreationArgs args(flinger.flinger(), client, "buffer-state-layer", WIDTH, HEIGHT,

                               LAYER_FLAGS, LayerMetadata());

                               LAYER_FLAGS, LayerMetadata());

        return new BufferStateLayer(args);

        return new BufferStateLayer(args);

    }

    }
@@ -188,7 +188,8 @@ void SetFrameRateTest::setupScheduler() { 
            .WillRepeatedly(Return(FakeHwcDisplayInjector::DEFAULT_VSYNC_PERIOD));

            .WillRepeatedly(Return(FakeHwcDisplayInjector::DEFAULT_VSYNC_PERIOD));

    EXPECT_CALL(*vsyncTracker, nextAnticipatedVSyncTimeFrom(_)).WillRepeatedly(Return(0));

    EXPECT_CALL(*vsyncTracker, nextAnticipatedVSyncTimeFrom(_)).WillRepeatedly(Return(0));

    mFlinger.setupScheduler(std::move(vsyncController), std::move(vsyncTracker),

    mFlinger.setupScheduler(std::move(vsyncController), std::move(vsyncTracker),

                            std::move(eventThread), std::move(sfEventThread));

                            std::move(eventThread), std::move(sfEventThread), /*callback*/ nullptr,

                            /*hasMultipleModes*/ true);

}

}





void SetFrameRateTest::setupComposer(uint32_t virtualDisplayCount) {

void SetFrameRateTest::setupComposer(uint32_t virtualDisplayCount) {
@@ -488,5 +489,28 @@ TEST_F(SetFrameRateTest, ValidateFrameRate) { 
    EXPECT_FALSE(ValidateFrameRate(60.0f, ANATIVEWINDOW_FRAME_RATE_EXACT, ""));

    EXPECT_FALSE(ValidateFrameRate(60.0f, ANATIVEWINDOW_FRAME_RATE_EXACT, ""));

}

}





TEST_P(SetFrameRateTest, SetOnParentActivatesTree) {

    const auto& layerFactory = GetParam();



    auto parent = mLayers.emplace_back(layerFactory->createLayer(mFlinger));

    if (!parent->isVisible()) {

        // This is a hack as all the test layers except EffectLayer are not visible,

        // but since the logic is unified in Layer, it should be fine.

        return;

    }



    auto child = mLayers.emplace_back(layerFactory->createLayer(mFlinger));

    addChild(parent, child);



    parent->setFrameRate(FRAME_RATE_VOTE1);

    commitTransaction();



    const auto layerHistorySummary =

            mFlinger.mutableScheduler().mutableLayerHistory()->summarize(0);

    ASSERT_EQ(2u, layerHistorySummary.size());

    EXPECT_TRUE(FRAME_RATE_VOTE1.rate.equalsWithMargin(layerHistorySummary[0].desiredRefreshRate));

    EXPECT_TRUE(FRAME_RATE_VOTE1.rate.equalsWithMargin(layerHistorySummary[1].desiredRefreshRate));

}



} // namespace

} // namespace

} // namespace android
 
} // namespace android

services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h

+2 −0
Original line number Original line Diff line number Diff line
@@ -246,6 +246,8 @@ public: 




    void resetScheduler(Scheduler* scheduler) { mFlinger->mScheduler.reset(scheduler); }

    void resetScheduler(Scheduler* scheduler) { mFlinger->mScheduler.reset(scheduler); }





    TestableScheduler& mutableScheduler() const { return *mScheduler; }



    using CreateBufferQueueFunction = surfaceflinger::test::Factory::CreateBufferQueueFunction;

    using CreateBufferQueueFunction = surfaceflinger::test::Factory::CreateBufferQueueFunction;

    void setCreateBufferQueueFunction(CreateBufferQueueFunction f) {

    void setCreateBufferQueueFunction(CreateBufferQueueFunction f) {

        mFactory.mCreateBufferQueue = f;

        mFactory.mCreateBufferQueue = f;