diff options
5 files changed, 38 insertions, 26 deletions
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/Flattener.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/Flattener.h index b09f1d1a09..864251f182 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/Flattener.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/Flattener.h @@ -36,8 +36,7 @@ class Predictor; class Flattener { public: - Flattener(Predictor& predictor, bool enableHolePunch = false) - : mEnableHolePunch(enableHolePunch), mPredictor(predictor) {} + Flattener(bool enableHolePunch = false) : mEnableHolePunch(enableHolePunch) {} void setDisplaySize(ui::Size size) { mDisplaySize = size; } @@ -64,7 +63,6 @@ private: void buildCachedSets(std::chrono::steady_clock::time_point now); const bool mEnableHolePunch; - Predictor& mPredictor; ui::Size mDisplaySize; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/Planner.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/Planner.h index c2037a899e..4365b93bb9 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/Planner.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/Planner.h @@ -76,6 +76,8 @@ private: std::optional<Predictor::PredictedPlan> mPredictedPlan; NonBufferHash mFlattenedHash = 0; + + bool mPredictorEnabled = false; }; } // namespace compositionengine::impl::planner diff --git a/services/surfaceflinger/CompositionEngine/src/planner/Flattener.cpp b/services/surfaceflinger/CompositionEngine/src/planner/Flattener.cpp index 6d624ffe56..4453a996f9 100644 --- a/services/surfaceflinger/CompositionEngine/src/planner/Flattener.cpp +++ b/services/surfaceflinger/CompositionEngine/src/planner/Flattener.cpp @@ -20,7 +20,6 @@ #include <compositionengine/impl/planner/Flattener.h> #include <compositionengine/impl/planner/LayerState.h> -#include <compositionengine/impl/planner/Predictor.h> using time_point = std::chrono::steady_clock::time_point; using namespace std::chrono_literals; @@ -410,7 +409,7 @@ void Flattener::buildCachedSets(time_point now) { } // TODO(b/181192467): Actually compute new LayerState vector and corresponding hash for each run - mPredictor.getPredictedPlan({}, 0); + // and feedback into the predictor ++mCachedSetCreationCount; mCachedSetCreationCost += mNewCachedSet->getCreationCost(); diff --git a/services/surfaceflinger/CompositionEngine/src/planner/Planner.cpp b/services/surfaceflinger/CompositionEngine/src/planner/Planner.cpp index 7e85dcaee0..c26eb1e107 100644 --- a/services/surfaceflinger/CompositionEngine/src/planner/Planner.cpp +++ b/services/surfaceflinger/CompositionEngine/src/planner/Planner.cpp @@ -27,8 +27,13 @@ namespace android::compositionengine::impl::planner { Planner::Planner() - : mFlattener(mPredictor, - base::GetBoolProperty(std::string("debug.sf.enable_hole_punch_pip"), false)) {} + : mFlattener(base::GetBoolProperty(std::string("debug.sf.enable_hole_punch_pip"), false)) { + // Implicitly, layer caching must also be enabled. + // E.g., setprop debug.sf.enable_layer_caching 1, or + // adb shell service call SurfaceFlinger 1040 i32 1 [i64 <display ID>] + mPredictorEnabled = + base::GetBoolProperty(std::string("debug.sf.enable_planner_prediction"), false); +} void Planner::setDisplaySize(ui::Size size) { mFlattener.setDisplaySize(size); @@ -99,19 +104,25 @@ void Planner::plan( const bool layersWereFlattened = hash != mFlattenedHash; ALOGV("[%s] Initial hash %zx flattened hash %zx", __func__, hash, mFlattenedHash); - mPredictedPlan = - mPredictor.getPredictedPlan(layersWereFlattened ? std::vector<const LayerState*>() - : mCurrentLayers, - mFlattenedHash); - if (mPredictedPlan) { - ALOGV("[%s] Predicting plan %s", __func__, to_string(mPredictedPlan->plan).c_str()); - } else { - ALOGV("[%s] No prediction found\n", __func__); + if (mPredictorEnabled) { + mPredictedPlan = + mPredictor.getPredictedPlan(layersWereFlattened ? std::vector<const LayerState*>() + : mCurrentLayers, + mFlattenedHash); + if (mPredictedPlan) { + ALOGV("[%s] Predicting plan %s", __func__, to_string(mPredictedPlan->plan).c_str()); + } else { + ALOGV("[%s] No prediction found\n", __func__); + } } } void Planner::reportFinalPlan( compositionengine::Output::OutputLayersEnumerator<compositionengine::Output>&& layers) { + if (!mPredictorEnabled) { + return; + } + Plan finalPlan; const GraphicBuffer* currentOverrideBuffer = nullptr; bool hasSkippedLayers = false; @@ -185,7 +196,9 @@ void Planner::dump(const Vector<String16>& args, std::string& result) { return; } - mPredictor.compareLayerStacks(leftHash, rightHash, result); + if (mPredictorEnabled) { + mPredictor.compareLayerStacks(leftHash, rightHash, result); + } } else if (command == "--describe" || command == "-d") { if (args.size() < 3) { base::StringAppendF(&result, @@ -209,7 +222,9 @@ void Planner::dump(const Vector<String16>& args, std::string& result) { return; } - mPredictor.describeLayerStack(hash, result); + if (mPredictorEnabled) { + mPredictor.describeLayerStack(hash, result); + } } else if (command == "--help" || command == "-h") { dumpUsage(result); } else if (command == "--similar" || command == "-s") { @@ -232,7 +247,9 @@ void Planner::dump(const Vector<String16>& args, std::string& result) { return; } - mPredictor.listSimilarStacks(*plan, result); + if (mPredictorEnabled) { + mPredictor.listSimilarStacks(*plan, result); + } } else if (command == "--layers" || command == "-l") { mFlattener.dumpLayers(result); } else { @@ -247,7 +264,9 @@ void Planner::dump(const Vector<String16>& args, std::string& result) { mFlattener.dump(result); result.append("\n"); - mPredictor.dump(result); + if (mPredictorEnabled) { + mPredictor.dump(result); + } } void Planner::dumpUsage(std::string& result) const { diff --git a/services/surfaceflinger/CompositionEngine/tests/planner/FlattenerTest.cpp b/services/surfaceflinger/CompositionEngine/tests/planner/FlattenerTest.cpp index 71757f69ea..25fab4934b 100644 --- a/services/surfaceflinger/CompositionEngine/tests/planner/FlattenerTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/planner/FlattenerTest.cpp @@ -18,7 +18,6 @@ #include <compositionengine/impl/planner/CachedSet.h> #include <compositionengine/impl/planner/Flattener.h> #include <compositionengine/impl/planner/LayerState.h> -#include <compositionengine/impl/planner/Predictor.h> #include <compositionengine/mock/LayerFE.h> #include <compositionengine/mock/OutputLayer.h> #include <gtest/gtest.h> @@ -31,7 +30,6 @@ using namespace std::chrono_literals; using impl::planner::Flattener; using impl::planner::LayerState; using impl::planner::NonBufferHash; -using impl::planner::Predictor; using testing::_; using testing::ByMove; @@ -47,7 +45,7 @@ namespace { class FlattenerTest : public testing::Test { public: - FlattenerTest() : mFlattener(std::make_unique<Flattener>(mPredictor, true)) {} + FlattenerTest() : mFlattener(std::make_unique<Flattener>(true)) {} void SetUp() override; protected: @@ -55,10 +53,6 @@ protected: void initializeFlattener(const std::vector<const LayerState*>& layers); void expectAllLayersFlattened(const std::vector<const LayerState*>& layers); - // TODO(b/181192467): Once Flattener starts to do something useful with Predictor, - // mPredictor should be mocked and checked for expectations. - Predictor mPredictor; - // mRenderEngine may be held as a pointer to mFlattener, so mFlattener must be destroyed first. renderengine::mock::RenderEngine mRenderEngine; std::unique_ptr<Flattener> mFlattener; |