diff options
author | Ady Abraham <adyabr@google.com> | 2021-08-04 13:04:37 -0700 |
---|---|---|
committer | Ady Abraham <adyabr@google.com> | 2021-08-05 11:40:12 -0700 |
commit | a850c185ff4170bea77c5b4e4ed4fa871daada65 (patch) | |
tree | e5c5e29f8901430e71340ec32f1781602d19f502 | |
parent | edec715c764be0e173056d9211fa079cdcb0cb98 (diff) | |
download | native-a850c185ff4170bea77c5b4e4ed4fa871daada65.tar.gz |
SF: fix frame rate for layer tree
With the current implementation we might mark layers as NoVote
incorrectly when a sibling has a frame rate.
Test: adb shell /data/nativetest64/libsurfaceflinger_unittest/libsurfaceflinger_unittest
Bug: 195205467
Change-Id: I62641f855a027e1192f9a85a4bc50692b1744764
-rw-r--r-- | services/surfaceflinger/Layer.cpp | 126 | ||||
-rw-r--r-- | services/surfaceflinger/Layer.h | 4 | ||||
-rw-r--r-- | services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp | 35 |
3 files changed, 97 insertions, 68 deletions
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index dbd2793276..e4a777f3c7 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -1117,46 +1117,59 @@ StretchEffect Layer::getStretchEffect() const { return StretchEffect{}; } -void Layer::updateTreeHasFrameRateVote() { - const auto traverseTree = [&](const LayerVector::Visitor& visitor) { - auto parent = getParent(); - while (parent) { - visitor(parent.get()); - parent = parent->getParent(); +bool Layer::propagateFrameRateForLayerTree(FrameRate parentFrameRate, bool* transactionNeeded) { + // The frame rate for layer tree is this layer's frame rate if present, or the parent frame rate + const auto frameRate = [&] { + if (mDrawingState.frameRate.rate.isValid() || + mDrawingState.frameRate.type == FrameRateCompatibility::NoVote) { + return mDrawingState.frameRate; } - traverse(LayerVector::StateSet::Current, visitor); - }; - - // update parents and children about the vote - // First traverse the tree and count how many layers has votes. - int layersWithVote = 0; - traverseTree([&layersWithVote](Layer* layer) { - const auto layerVotedWithDefaultCompatibility = - layer->mDrawingState.frameRate.rate.isValid() && - layer->mDrawingState.frameRate.type == FrameRateCompatibility::Default; - const auto layerVotedWithNoVote = - layer->mDrawingState.frameRate.type == FrameRateCompatibility::NoVote; - const auto layerVotedWithExactCompatibility = - layer->mDrawingState.frameRate.type == FrameRateCompatibility::Exact; - - // We do not count layers that are ExactOrMultiple for the same reason - // we are allowing touch boost for those layers. See - // RefreshRateConfigs::getBestRefreshRate for more details. - if (layerVotedWithDefaultCompatibility || layerVotedWithNoVote || - layerVotedWithExactCompatibility) { - layersWithVote++; + return parentFrameRate; + }(); + + *transactionNeeded |= setFrameRateForLayerTree(frameRate); + + // The frame rate is propagated to the children + bool childrenHaveFrameRate = false; + for (const sp<Layer>& child : mCurrentChildren) { + childrenHaveFrameRate |= + child->propagateFrameRateForLayerTree(frameRate, transactionNeeded); + } + + // 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 (!frameRate.rate.isValid() && frameRate.type != FrameRateCompatibility::NoVote && + childrenHaveFrameRate) { + *transactionNeeded |= + setFrameRateForLayerTree(FrameRate(Fps(0.0f), FrameRateCompatibility::NoVote)); + } + + // We return whether this layer ot its children has a vote. We ignore ExactOrMultiple votes for + // the same reason we are allowing touch boost for those layers. See + // RefreshRateConfigs::getBestRefreshRate for more details. + const auto layerVotedWithDefaultCompatibility = + frameRate.rate.isValid() && frameRate.type == FrameRateCompatibility::Default; + const auto layerVotedWithNoVote = frameRate.type == FrameRateCompatibility::NoVote; + const auto layerVotedWithExactCompatibility = + frameRate.rate.isValid() && frameRate.type == FrameRateCompatibility::Exact; + return layerVotedWithDefaultCompatibility || layerVotedWithNoVote || + layerVotedWithExactCompatibility || childrenHaveFrameRate; +} + +void Layer::updateTreeHasFrameRateVote() { + const auto root = [&]() -> sp<Layer> { + sp<Layer> layer = this; + while (auto parent = layer->getParent()) { + layer = parent; } - }); + return layer; + }(); - // Now we can update the tree frame rate vote for each layer in the tree - const bool treeHasFrameRateVote = layersWithVote > 0; bool transactionNeeded = false; + root->propagateFrameRateForLayerTree({}, &transactionNeeded); - traverseTree([treeHasFrameRateVote, &transactionNeeded](Layer* layer) { - transactionNeeded = layer->updateFrameRateForLayerTree(treeHasFrameRateVote); - }); - + // TODO(b/195668952): we probably don't need eTraversalNeeded here if (transactionNeeded) { mFlinger->setTransactionFlags(eTraversalNeeded); } @@ -1283,42 +1296,23 @@ std::shared_ptr<frametimeline::SurfaceFrame> Layer::createSurfaceFrameForBuffer( return surfaceFrame; } -bool Layer::updateFrameRateForLayerTree(bool treeHasFrameRateVote) { - const auto updateDrawingState = [&](FrameRate frameRate) { - if (mDrawingState.frameRateForLayerTree == frameRate) { - return false; - } - - mDrawingState.frameRateForLayerTree = frameRate; - mDrawingState.sequence++; - mDrawingState.modified = true; - setTransactionFlags(eTransactionNeeded); - - mFlinger->mScheduler->recordLayerHistory(this, systemTime(), - LayerHistory::LayerUpdateType::SetFrameRate); - - return true; - }; - - const auto frameRate = mDrawingState.frameRate; - if (frameRate.rate.isValid() || frameRate.type == FrameRateCompatibility::NoVote) { - return updateDrawingState(frameRate); +bool Layer::setFrameRateForLayerTree(FrameRate frameRate) { + if (mDrawingState.frameRateForLayerTree == frameRate) { + return false; } - // This layer doesn't have a frame rate. Check if its ancestors have a vote - for (sp<Layer> parent = getParent(); parent; parent = parent->getParent()) { - if (parent->mDrawingState.frameRate.rate.isValid()) { - return updateDrawingState(parent->mDrawingState.frameRate); - } - } + mDrawingState.frameRateForLayerTree = frameRate; - // This layer and its ancestors don't have a frame rate. If one of successors - // has a vote, return a NoVote for successors to set the vote - if (treeHasFrameRateVote) { - return updateDrawingState(FrameRate(Fps(0.0f), FrameRateCompatibility::NoVote)); - } + // TODO(b/195668952): we probably don't need to dirty visible regions here + // or even store frameRateForLayerTree in mDrawingState + mDrawingState.sequence++; + mDrawingState.modified = true; + setTransactionFlags(eTransactionNeeded); + + mFlinger->mScheduler->recordLayerHistory(this, systemTime(), + LayerHistory::LayerUpdateType::SetFrameRate); - return updateDrawingState(frameRate); + return true; } Layer::FrameRate Layer::getFrameRateForLayerTree() const { diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index c5cb17ffc7..4426c70c19 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -1053,6 +1053,8 @@ private: const std::vector<Layer*>& layersInTree); void updateTreeHasFrameRateVote(); + bool propagateFrameRateForLayerTree(FrameRate parentFrameRate, bool* transactionNeeded); + bool setFrameRateForLayerTree(FrameRate); void setZOrderRelativeOf(const wp<Layer>& relativeOf); bool isTrustedOverlay() const; @@ -1071,8 +1073,6 @@ private: // Fills in the frame and transform info for the InputWindowInfo void fillInputFrameInfo(InputWindowInfo& info, const ui::Transform& toPhysicalDisplay); - bool updateFrameRateForLayerTree(bool treeHasFrameRateVote); - // Cached properties computed from drawing state // Effective transform taking into account parent transforms and any parent scaling, which is // a transform from the current layer coordinate space to display(screen) coordinate space. diff --git a/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp b/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp index 1ed52ea389..2761470c31 100644 --- a/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp +++ b/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp @@ -485,5 +485,40 @@ TEST_P(SetFrameRateTest, SetOnParentActivatesTree) { EXPECT_TRUE(FRAME_RATE_VOTE1.rate.equalsWithMargin(layerHistorySummary[1].desiredRefreshRate)); } +TEST_P(SetFrameRateTest, addChildForParentWithTreeVote) { + EXPECT_CALL(*mMessageQueue, invalidate()).Times(1); + + const auto& layerFactory = GetParam(); + + const auto parent = mLayers.emplace_back(layerFactory->createLayer(mFlinger)); + const auto child1 = mLayers.emplace_back(layerFactory->createLayer(mFlinger)); + const auto child2 = mLayers.emplace_back(layerFactory->createLayer(mFlinger)); + const auto childOfChild1 = mLayers.emplace_back(layerFactory->createLayer(mFlinger)); + + addChild(parent, child1); + addChild(child1, childOfChild1); + + childOfChild1->setFrameRate(FRAME_RATE_VOTE1); + commitTransaction(); + EXPECT_EQ(FRAME_RATE_TREE, parent->getFrameRateForLayerTree()); + EXPECT_EQ(FRAME_RATE_TREE, child1->getFrameRateForLayerTree()); + EXPECT_EQ(FRAME_RATE_VOTE1, childOfChild1->getFrameRateForLayerTree()); + EXPECT_EQ(FRAME_RATE_NO_VOTE, child2->getFrameRateForLayerTree()); + + addChild(parent, child2); + commitTransaction(); + EXPECT_EQ(FRAME_RATE_TREE, parent->getFrameRateForLayerTree()); + EXPECT_EQ(FRAME_RATE_TREE, child1->getFrameRateForLayerTree()); + EXPECT_EQ(FRAME_RATE_VOTE1, childOfChild1->getFrameRateForLayerTree()); + EXPECT_EQ(FRAME_RATE_NO_VOTE, child2->getFrameRateForLayerTree()); + + childOfChild1->setFrameRate(FRAME_RATE_NO_VOTE); + commitTransaction(); + EXPECT_EQ(FRAME_RATE_NO_VOTE, parent->getFrameRateForLayerTree()); + EXPECT_EQ(FRAME_RATE_NO_VOTE, child1->getFrameRateForLayerTree()); + EXPECT_EQ(FRAME_RATE_NO_VOTE, childOfChild1->getFrameRateForLayerTree()); + EXPECT_EQ(FRAME_RATE_NO_VOTE, child2->getFrameRateForLayerTree()); +} + } // namespace } // namespace android |