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

Commit 0fd773f8 authored by Vishnu Nair's avatar Vishnu Nair
Browse files

Fix framerate propagation from child to parent layers 

We were not resetting the framerate vote of the parent layer
after the child frame rate vote was removed. Fix this and
add tests to verify this behavior does not regress.

Flag: EXEMPT (removing old flags)
Bug: 330785038
Test: presubmit
Change-Id: I88d60484d4dfdfa4b09c98484867a49545f60257
parent 057e5772

services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp

+16 −8
Original line number Diff line number Diff line
@@ -561,6 +561,7 @@ const LayerSnapshot& LayerSnapshotBuilder::updateSnapshotsInHierarchy( 
        updateSnapshot(*snapshot, args, *layer, parentSnapshot, traversalPath);

    }



    bool childHasValidFrameRate = false;

    for (auto& [childHierarchy, variant] : hierarchy.mChildren) {

        LayerHierarchy::ScopedAddToTraversalPath addChildToPath(traversalPath,

                                                                childHierarchy->getLayer()->id,
@@ -568,7 +569,8 @@ const LayerSnapshot& LayerSnapshotBuilder::updateSnapshotsInHierarchy( 
        const LayerSnapshot& childSnapshot =

                updateSnapshotsInHierarchy(args, *childHierarchy, traversalPath, *snapshot,

                                           depth + 1);

        updateFrameRateFromChildSnapshot(*snapshot, childSnapshot, args);

        updateFrameRateFromChildSnapshot(*snapshot, childSnapshot, *childHierarchy->getLayer(),

                                         args, &childHasValidFrameRate);

    }



    return *snapshot;
@@ -675,9 +677,10 @@ void LayerSnapshotBuilder::updateRelativeState(LayerSnapshot& snapshot, 
    }

}



void LayerSnapshotBuilder::updateFrameRateFromChildSnapshot(LayerSnapshot& snapshot,

                                                            const LayerSnapshot& childSnapshot,

                                                            const Args& args) {

void LayerSnapshotBuilder::updateFrameRateFromChildSnapshot(

        LayerSnapshot& snapshot, const LayerSnapshot& childSnapshot,

        const RequestedLayerState& /* requestedChildState */, const Args& args,

        bool* outChildHasValidFrameRate) {

    if (args.forceUpdate == ForceUpdateFlags::NONE &&

        !args.layerLifecycleManager.getGlobalChanges().any(

                RequestedLayerState::Changes::Hierarchy) &&
@@ -687,7 +690,7 @@ void LayerSnapshotBuilder::updateFrameRateFromChildSnapshot(LayerSnapshot& snaps 
    }



    using FrameRateCompatibility = scheduler::FrameRateCompatibility;

    if (snapshot.frameRate.isValid()) {

    if (snapshot.inheritedFrameRate.isValid() || *outChildHasValidFrameRate) {

        // we already have a valid framerate.

        return;

    }
@@ -704,13 +707,18 @@ void LayerSnapshotBuilder::updateFrameRateFromChildSnapshot(LayerSnapshot& snaps 
    const auto layerVotedWithExactCompatibility = childSnapshot.frameRate.vote.rate.isValid() &&

            childSnapshot.frameRate.vote.type == FrameRateCompatibility::Exact;



    bool childHasValidFrameRate = layerVotedWithDefaultCompatibility || layerVotedWithNoVote ||

    *outChildHasValidFrameRate |= layerVotedWithDefaultCompatibility || layerVotedWithNoVote ||

            layerVotedWithCategory || layerVotedWithExactCompatibility;



    // If we don't have a valid frame rate, but the children do, we set this

    // layer as NoVote to allow the children to control the refresh rate

    if (childHasValidFrameRate) {

        snapshot.frameRate = scheduler::LayerInfo::FrameRate(Fps(), FrameRateCompatibility::NoVote);

    static const auto noVote =

            scheduler::LayerInfo::FrameRate(Fps(), FrameRateCompatibility::NoVote);

    if (*outChildHasValidFrameRate) {

        snapshot.frameRate = noVote;

        snapshot.changes |= RequestedLayerState::Changes::FrameRate;

    } else if (snapshot.frameRate != snapshot.inheritedFrameRate) {

        snapshot.frameRate = snapshot.inheritedFrameRate;

        snapshot.changes |= RequestedLayerState::Changes::FrameRate;

    }

}

services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.h

+3 −1
Original line number Diff line number Diff line
@@ -130,7 +130,9 @@ private: 
                                  const RequestedLayerState& layer,

                                  const LayerSnapshot& parentSnapshot);

    void updateFrameRateFromChildSnapshot(LayerSnapshot& snapshot,

                                          const LayerSnapshot& childSnapshot, const Args& args);

                                          const LayerSnapshot& childSnapshot,

                                          const RequestedLayerState& requestedCHildState,

                                          const Args& args, bool* outChildHasValidFrameRate);

    void updateTouchableRegionCrop(const Args& args);



    std::unordered_map<LayerHierarchy::TraversalPath, LayerSnapshot*,

services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp

+162 −2
Original line number Diff line number Diff line
@@ -56,6 +56,17 @@ using namespace com::android::graphics::surfaceflinger; 


class LayerSnapshotTest : public LayerSnapshotTestBase {

protected:

    const Layer::FrameRate FRAME_RATE_VOTE1 =

            Layer::FrameRate(67_Hz, scheduler::FrameRateCompatibility::Default);

    const Layer::FrameRate FRAME_RATE_VOTE2 =

            Layer::FrameRate(14_Hz, scheduler::FrameRateCompatibility::Default);

    const Layer::FrameRate FRAME_RATE_VOTE3 =

            Layer::FrameRate(99_Hz, scheduler::FrameRateCompatibility::Default);

    const Layer::FrameRate FRAME_RATE_TREE =

            Layer::FrameRate(Fps(), scheduler::FrameRateCompatibility::NoVote);

    const Layer::FrameRate FRAME_RATE_NO_VOTE =

            Layer::FrameRate(Fps(), scheduler::FrameRateCompatibility::Default);



    LayerSnapshotTest() : LayerSnapshotTestBase() {

        UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);

    }
@@ -787,6 +798,155 @@ TEST_F(LayerSnapshotTest, framerate) { 
              scheduler::FrameRateCompatibility::Default);

}



TEST_F(LayerSnapshotTest, frameRateSetAndGet) {

    setFrameRate(1, FRAME_RATE_VOTE1.vote.rate.getValue(), 0, 0);

    UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);

    // verify parent is gets no vote

    EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_VOTE1);

}



TEST_F(LayerSnapshotTest, frameRateSetAndGetParent) {

    setFrameRate(111, FRAME_RATE_VOTE1.vote.rate.getValue(), 0, 0);

    UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);

    EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_TREE);

    EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_TREE);

    EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_VOTE1);



    setFrameRate(111, FRAME_RATE_NO_VOTE.vote.rate.getValue(), 0, 0);

    UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);

    EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_NO_VOTE);

    EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_NO_VOTE);

    EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_NO_VOTE);

}



TEST_F(LayerSnapshotTest, frameRateSetAndGetParentAllVote) {

    setFrameRate(1, FRAME_RATE_VOTE3.vote.rate.getValue(), 0, 0);

    setFrameRate(11, FRAME_RATE_VOTE2.vote.rate.getValue(), 0, 0);

    setFrameRate(111, FRAME_RATE_VOTE1.vote.rate.getValue(), 0, 0);

    UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);



    EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_VOTE3);

    EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_VOTE2);

    EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_VOTE1);



    setFrameRate(111, FRAME_RATE_NO_VOTE.vote.rate.getValue(), 0, 0);

    UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);

    EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_VOTE3);

    EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_VOTE2);

    EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_VOTE2);



    setFrameRate(11, FRAME_RATE_NO_VOTE.vote.rate.getValue(), 0, 0);

    UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);

    EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_VOTE3);

    EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_VOTE3);

    EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_VOTE3);



    setFrameRate(1, FRAME_RATE_NO_VOTE.vote.rate.getValue(), 0, 0);

    UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);

    EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_NO_VOTE);

    EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_NO_VOTE);

    EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_NO_VOTE);

}



TEST_F(LayerSnapshotTest, frameRateSetAndGetChild) {

    setFrameRate(1, FRAME_RATE_VOTE1.vote.rate.getValue(), 0, 0);

    UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);

    EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_VOTE1);

    EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_VOTE1);

    EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_VOTE1);



    setFrameRate(1, FRAME_RATE_NO_VOTE.vote.rate.getValue(), 0, 0);

    UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);

    EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_NO_VOTE);

    EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_NO_VOTE);

    EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_NO_VOTE);

}



TEST_F(LayerSnapshotTest, frameRateSetAndGetChildAllVote) {

    setFrameRate(1, FRAME_RATE_VOTE3.vote.rate.getValue(), 0, 0);

    setFrameRate(11, FRAME_RATE_VOTE2.vote.rate.getValue(), 0, 0);

    setFrameRate(111, FRAME_RATE_VOTE1.vote.rate.getValue(), 0, 0);

    UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);

    EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_VOTE3);

    EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_VOTE2);

    EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_VOTE1);



    setFrameRate(1, FRAME_RATE_NO_VOTE.vote.rate.getValue(), 0, 0);

    UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);

    EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_TREE);

    EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_VOTE2);

    EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_VOTE1);



    setFrameRate(11, FRAME_RATE_NO_VOTE.vote.rate.getValue(), 0, 0);

    UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);

    EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_TREE);

    EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_TREE);

    EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_VOTE1);



    setFrameRate(111, FRAME_RATE_NO_VOTE.vote.rate.getValue(), 0, 0);

    UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);

    EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_NO_VOTE);

    EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_NO_VOTE);

    EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_NO_VOTE);

}



TEST_F(LayerSnapshotTest, frameRateSetAndGetChildAddAfterVote) {

    setFrameRate(1, FRAME_RATE_VOTE1.vote.rate.getValue(), 0, 0);

    reparentLayer(111, 2);

    std::vector<uint32_t> traversalOrder = {1, 11, 12, 121, 122, 1221, 13, 2, 111};

    UPDATE_AND_VERIFY(mSnapshotBuilder, traversalOrder);

    EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_VOTE1);

    EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_VOTE1);

    EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_NO_VOTE);



    reparentLayer(111, 11);

    UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);

    EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_VOTE1);

    EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_VOTE1);

    EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_VOTE1);



    setFrameRate(1, FRAME_RATE_NO_VOTE.vote.rate.getValue(), 0, 0);

    UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);

    EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_NO_VOTE);

    EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_NO_VOTE);

    EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_NO_VOTE);

}



TEST_F(LayerSnapshotTest, frameRateSetAndGetChildRemoveAfterVote) {

    setFrameRate(1, FRAME_RATE_VOTE1.vote.rate.getValue(), 0, 0);

    UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);

    EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_VOTE1);

    EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_VOTE1);

    EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_VOTE1);



    reparentLayer(111, 2);

    std::vector<uint32_t> traversalOrder = {1, 11, 12, 121, 122, 1221, 13, 2, 111};

    UPDATE_AND_VERIFY(mSnapshotBuilder, traversalOrder);

    EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_VOTE1);

    EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_VOTE1);

    EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_NO_VOTE);



    setFrameRate(1, FRAME_RATE_NO_VOTE.vote.rate.getValue(), 0, 0);

    UPDATE_AND_VERIFY(mSnapshotBuilder, traversalOrder);

    EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_NO_VOTE);

    EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_NO_VOTE);

    EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_NO_VOTE);

}



TEST_F(LayerSnapshotTest, frameRateAddChildForParentWithTreeVote) {

    setFrameRate(11, FRAME_RATE_VOTE1.vote.rate.getValue(), 0, 0);

    UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);

    EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_TREE);

    EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_VOTE1);

    EXPECT_EQ(getSnapshot({.id = 12})->frameRate, FRAME_RATE_NO_VOTE);



    setFrameRate(11, FRAME_RATE_NO_VOTE.vote.rate.getValue(), 0, 0);

    UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);

    EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_NO_VOTE);

    EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_NO_VOTE);

    EXPECT_EQ(getSnapshot({.id = 12})->frameRate, FRAME_RATE_NO_VOTE);

}



TEST_F(LayerSnapshotTest, translateDataspace) {

    setDataspace(1, ui::Dataspace::UNKNOWN);

    UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
@@ -965,7 +1125,7 @@ TEST_F(LayerSnapshotTest, frameRateSelectionStrategy) { 
    EXPECT_FALSE(getSnapshot({.id = 1})->frameRate.vote.rate.isValid());

    EXPECT_EQ(getSnapshot({.id = 1})->frameRate.vote.type,

              scheduler::FrameRateCompatibility::NoVote);

    EXPECT_FALSE(getSnapshot({.id = 1})->changes.test(RequestedLayerState::Changes::FrameRate));

    EXPECT_TRUE(getSnapshot({.id = 1})->changes.test(RequestedLayerState::Changes::FrameRate));



    // verify layer 12 and all descendants (121, 122, 1221) get the requested vote

    EXPECT_FALSE(getSnapshot({.id = 12})->frameRate.vote.rate.isValid());
@@ -1056,7 +1216,7 @@ TEST_F(LayerSnapshotTest, frameRateSelectionStrategyWithCategory) { 
    EXPECT_EQ(getSnapshot({.id = 1})->frameRate.vote.type,

              scheduler::FrameRateCompatibility::NoVote);

    EXPECT_EQ(getSnapshot({.id = 1})->frameRate.category, FrameRateCategory::Default);

    EXPECT_FALSE(getSnapshot({.id = 1})->changes.test(RequestedLayerState::Changes::FrameRate));

    EXPECT_TRUE(getSnapshot({.id = 1})->changes.test(RequestedLayerState::Changes::FrameRate));



    // verify layer 12 and all descendants (121, 122, 1221) get the requested vote

    EXPECT_FALSE(getSnapshot({.id = 12})->frameRate.vote.rate.isValid());