SF: Move preComposition to CompositionEngine
This moves SurfaceFlinger::preComposition() to
CompositionEngine::preComposition().
As part of the move, CompositionRefreshArgs is also introduced, and to
start contains an array of outputs (not yet used), and potentially
visible layers.
Test: atest libsurfaceflinger_unittest libcompositionengine_test
Bug: 121291683
Change-Id: I0888bd6de1214381222bbae0da93bf966392b303
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionEngine.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionEngine.h
index 896f8aa..31d6365 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionEngine.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionEngine.h
@@ -18,6 +18,8 @@
#include <memory>
+#include <utils/Timers.h>
+
namespace android {
class HWComposer;
@@ -31,6 +33,7 @@
class Display;
class Layer;
+struct CompositionRefreshArgs;
struct DisplayCreationArgs;
struct LayerCreationArgs;
@@ -51,6 +54,12 @@
virtual renderengine::RenderEngine& getRenderEngine() const = 0;
virtual void setRenderEngine(std::unique_ptr<renderengine::RenderEngine>) = 0;
+
+ virtual bool needsAnotherUpdate() const = 0;
+ virtual nsecs_t getLastFrameRefreshTimestamp() const = 0;
+
+ // TODO(b/121291683): These will become private/internal
+ virtual void preComposition(CompositionRefreshArgs&) = 0;
};
} // namespace compositionengine
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h
new file mode 100644
index 0000000..20f131e
--- /dev/null
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h
@@ -0,0 +1,40 @@
+/*
+ * Copyright 2019 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include <compositionengine/Display.h>
+#include <compositionengine/Layer.h>
+
+namespace android::compositionengine {
+
+using Layers = std::vector<std::shared_ptr<compositionengine::Layer>>;
+using Outputs = std::vector<std::shared_ptr<compositionengine::Output>>;
+
+/**
+ * A parameter object for refreshing a set of outputs
+ */
+struct CompositionRefreshArgs {
+ // All the outputs being refreshed
+ Outputs outputs;
+
+ // All the layers that are potentially visible in the outputs. The order of
+ // the layers is important, and should be in traversal order from back to
+ // front.
+ Layers layers;
+};
+
+} // namespace android::compositionengine
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/Layer.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/Layer.h
index 8cb9203..451608b 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/Layer.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/Layer.h
@@ -21,11 +21,7 @@
#include <utils/StrongPointer.h>
-namespace android {
-
-typedef int64_t nsecs_t;
-
-namespace compositionengine {
+namespace android::compositionengine {
class Display;
class LayerFE;
@@ -62,5 +58,4 @@
virtual void dump(std::string& result) const = 0;
};
-} // namespace compositionengine
-} // namespace android
+} // namespace android::compositionengine
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/CompositionEngine.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/CompositionEngine.h
index b01eb64..96e609d 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/CompositionEngine.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/CompositionEngine.h
@@ -36,9 +36,19 @@
renderengine::RenderEngine& getRenderEngine() const override;
void setRenderEngine(std::unique_ptr<renderengine::RenderEngine>) override;
+ bool needsAnotherUpdate() const override;
+ nsecs_t getLastFrameRefreshTimestamp() const override;
+
+ void preComposition(CompositionRefreshArgs&) override;
+
+ // Testing
+ void setNeedsAnotherUpdateForTest(bool);
+
private:
std::unique_ptr<HWComposer> mHwComposer;
std::unique_ptr<renderengine::RenderEngine> mRenderEngine;
+ bool mNeedsAnotherUpdate = false;
+ nsecs_t mRefreshStartTime = 0;
};
std::unique_ptr<compositionengine::CompositionEngine> createCompositionEngine();
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/CompositionEngine.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/CompositionEngine.h
index 0f57685..82ecec5 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/CompositionEngine.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/CompositionEngine.h
@@ -17,6 +17,7 @@
#pragma once
#include <compositionengine/CompositionEngine.h>
+#include <compositionengine/CompositionRefreshArgs.h>
#include <compositionengine/DisplayCreationArgs.h>
#include <compositionengine/LayerCreationArgs.h>
#include <gmock/gmock.h>
@@ -39,6 +40,11 @@
MOCK_CONST_METHOD0(getRenderEngine, renderengine::RenderEngine&());
MOCK_METHOD1(setRenderEngine, void(std::unique_ptr<renderengine::RenderEngine>));
+
+ MOCK_CONST_METHOD0(needsAnotherUpdate, bool());
+ MOCK_CONST_METHOD0(getLastFrameRefreshTimestamp, nsecs_t());
+
+ MOCK_METHOD1(preComposition, void(CompositionRefreshArgs&));
};
} // namespace android::compositionengine::mock
diff --git a/services/surfaceflinger/CompositionEngine/src/CompositionEngine.cpp b/services/surfaceflinger/CompositionEngine/src/CompositionEngine.cpp
index cb08b81..9558266 100644
--- a/services/surfaceflinger/CompositionEngine/src/CompositionEngine.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/CompositionEngine.cpp
@@ -14,10 +14,13 @@
* limitations under the License.
*/
+#include <compositionengine/CompositionRefreshArgs.h>
+#include <compositionengine/LayerFE.h>
#include <compositionengine/impl/CompositionEngine.h>
#include <compositionengine/impl/Display.h>
#include <compositionengine/impl/Layer.h>
#include <renderengine/RenderEngine.h>
+#include <utils/Trace.h>
#include "DisplayHardware/HWComposer.h"
@@ -59,5 +62,35 @@
mRenderEngine = std::move(renderEngine);
}
+bool CompositionEngine::needsAnotherUpdate() const {
+ return mNeedsAnotherUpdate;
+}
+
+nsecs_t CompositionEngine::getLastFrameRefreshTimestamp() const {
+ return mRefreshStartTime;
+}
+
+void CompositionEngine::preComposition(CompositionRefreshArgs& args) {
+ ATRACE_CALL();
+ ALOGV(__FUNCTION__);
+
+ bool needsAnotherUpdate = false;
+
+ mRefreshStartTime = systemTime(SYSTEM_TIME_MONOTONIC);
+
+ for (auto& layer : args.layers) {
+ sp<compositionengine::LayerFE> layerFE = layer->getLayerFE();
+ if (layerFE && layerFE->onPreComposition(mRefreshStartTime)) {
+ needsAnotherUpdate = true;
+ }
+ }
+
+ mNeedsAnotherUpdate = needsAnotherUpdate;
+}
+
+void CompositionEngine::setNeedsAnotherUpdateForTest(bool value) {
+ mNeedsAnotherUpdate = value;
+}
+
} // namespace impl
} // namespace android::compositionengine
diff --git a/services/surfaceflinger/CompositionEngine/tests/CompositionEngineTest.cpp b/services/surfaceflinger/CompositionEngine/tests/CompositionEngineTest.cpp
index 3766f27..0dbf8f0 100644
--- a/services/surfaceflinger/CompositionEngine/tests/CompositionEngineTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/CompositionEngineTest.cpp
@@ -14,7 +14,11 @@
* limitations under the License.
*/
+#include <compositionengine/CompositionRefreshArgs.h>
#include <compositionengine/impl/CompositionEngine.h>
+#include <compositionengine/mock/Layer.h>
+#include <compositionengine/mock/LayerFE.h>
+#include <compositionengine/mock/Output.h>
#include <gtest/gtest.h>
#include <renderengine/mock/RenderEngine.h>
@@ -23,19 +27,19 @@
namespace android::compositionengine {
namespace {
+using ::testing::_;
+using ::testing::Return;
+using ::testing::SaveArg;
using ::testing::StrictMock;
class CompositionEngineTest : public testing::Test {
public:
- ~CompositionEngineTest() override;
- mock::HWComposer* mHwc = new StrictMock<mock::HWComposer>();
+ android::mock::HWComposer* mHwc = new StrictMock<android::mock::HWComposer>();
renderengine::mock::RenderEngine* mRenderEngine =
new StrictMock<renderengine::mock::RenderEngine>();
impl::CompositionEngine mEngine;
};
-CompositionEngineTest::~CompositionEngineTest() = default;
-
TEST_F(CompositionEngineTest, canInstantiateCompositionEngine) {
auto engine = impl::createCompositionEngine();
EXPECT_TRUE(engine.get() != nullptr);
@@ -53,5 +57,84 @@
EXPECT_EQ(mRenderEngine, &mEngine.getRenderEngine());
}
+/*
+ * CompositionEngine::preComposition
+ */
+
+class PreCompositionTest : public CompositionEngineTest {
+public:
+ PreCompositionTest() {
+ EXPECT_CALL(*mLayer1, getLayerFE()).WillRepeatedly(Return(mLayer1FE));
+ EXPECT_CALL(*mLayer2, getLayerFE()).WillRepeatedly(Return(mLayer2FE));
+ EXPECT_CALL(*mLayer3, getLayerFE()).WillRepeatedly(Return(mLayer3FE));
+ // getLayerFE() can return nullptr. Ensure that this is handled.
+ EXPECT_CALL(*mLayer4, getLayerFE()).WillRepeatedly(Return(nullptr));
+
+ mRefreshArgs.outputs = {mOutput};
+ mRefreshArgs.layers = {mLayer1, mLayer2, mLayer3, mLayer4};
+ }
+
+ std::shared_ptr<mock::Output> mOutput{std::make_shared<StrictMock<mock::Output>>()};
+ std::shared_ptr<mock::Layer> mLayer1{std::make_shared<StrictMock<mock::Layer>>()};
+ std::shared_ptr<mock::Layer> mLayer2{std::make_shared<StrictMock<mock::Layer>>()};
+ std::shared_ptr<mock::Layer> mLayer3{std::make_shared<StrictMock<mock::Layer>>()};
+ std::shared_ptr<mock::Layer> mLayer4{std::make_shared<StrictMock<mock::Layer>>()};
+ sp<StrictMock<mock::LayerFE>> mLayer1FE{new StrictMock<mock::LayerFE>()};
+ sp<StrictMock<mock::LayerFE>> mLayer2FE{new StrictMock<mock::LayerFE>()};
+ sp<StrictMock<mock::LayerFE>> mLayer3FE{new StrictMock<mock::LayerFE>()};
+
+ CompositionRefreshArgs mRefreshArgs;
+};
+
+TEST_F(PreCompositionTest, preCompositionSetsFrameTimestamp) {
+ const nsecs_t before = systemTime(SYSTEM_TIME_MONOTONIC);
+ CompositionRefreshArgs emptyArgs;
+ mEngine.preComposition(emptyArgs);
+ const nsecs_t after = systemTime(SYSTEM_TIME_MONOTONIC);
+
+ // The frame timestamp should be between the before and after timestamps
+ EXPECT_GE(mEngine.getLastFrameRefreshTimestamp(), before);
+ EXPECT_LE(mEngine.getLastFrameRefreshTimestamp(), after);
+}
+
+TEST_F(PreCompositionTest, preCompositionInvokesLayerPreCompositionWithFrameTimestamp) {
+ nsecs_t ts1 = 0;
+ nsecs_t ts2 = 0;
+ nsecs_t ts3 = 0;
+ EXPECT_CALL(*mLayer1FE, onPreComposition(_)).WillOnce(DoAll(SaveArg<0>(&ts1), Return(false)));
+ EXPECT_CALL(*mLayer2FE, onPreComposition(_)).WillOnce(DoAll(SaveArg<0>(&ts2), Return(false)));
+ EXPECT_CALL(*mLayer3FE, onPreComposition(_)).WillOnce(DoAll(SaveArg<0>(&ts3), Return(false)));
+
+ mEngine.preComposition(mRefreshArgs);
+
+ // Each of the onPreComposition calls should used the same refresh timestamp
+ EXPECT_EQ(ts1, mEngine.getLastFrameRefreshTimestamp());
+ EXPECT_EQ(ts2, mEngine.getLastFrameRefreshTimestamp());
+ EXPECT_EQ(ts3, mEngine.getLastFrameRefreshTimestamp());
+}
+
+TEST_F(PreCompositionTest, preCompositionDefaultsToNoUpdateNeeded) {
+ EXPECT_CALL(*mLayer1FE, onPreComposition(_)).WillOnce(Return(false));
+ EXPECT_CALL(*mLayer2FE, onPreComposition(_)).WillOnce(Return(false));
+ EXPECT_CALL(*mLayer3FE, onPreComposition(_)).WillOnce(Return(false));
+
+ mEngine.setNeedsAnotherUpdateForTest(true);
+
+ mEngine.preComposition(mRefreshArgs);
+
+ // The call should have cleared the needsAnotherUpdate flag
+ EXPECT_FALSE(mEngine.needsAnotherUpdate());
+}
+
+TEST_F(PreCompositionTest, preCompositionSetsNeedsAnotherUpdateIfAtLeastOneLayerRequestsIt) {
+ EXPECT_CALL(*mLayer1FE, onPreComposition(_)).WillOnce(Return(true));
+ EXPECT_CALL(*mLayer2FE, onPreComposition(_)).WillOnce(Return(false));
+ EXPECT_CALL(*mLayer3FE, onPreComposition(_)).WillOnce(Return(false));
+
+ mEngine.preComposition(mRefreshArgs);
+
+ EXPECT_TRUE(mEngine.needsAnotherUpdate());
+}
+
} // namespace
} // namespace android::compositionengine
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 7a9a9a7..799dd3c 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -38,6 +38,7 @@
#include <binder/PermissionCache.h>
#include <compositionengine/CompositionEngine.h>
+#include <compositionengine/CompositionRefreshArgs.h>
#include <compositionengine/Display.h>
#include <compositionengine/DisplayColorProfile.h>
#include <compositionengine/Layer.h>
@@ -1772,8 +1773,17 @@
mRefreshPending = false;
+ compositionengine::CompositionRefreshArgs refreshArgs;
+ for (const auto& [_, display] : mDisplays) {
+ refreshArgs.outputs.push_back(display->getCompositionDisplay());
+ }
+ mDrawingState.traverseInZOrder([&refreshArgs](Layer* layer) {
+ auto compositionLayer = layer->getCompositionLayer();
+ if (compositionLayer) refreshArgs.layers.push_back(compositionLayer);
+ });
+
const bool repaintEverything = mRepaintEverything.exchange(false);
- preComposition();
+ mCompositionEngine->preComposition(refreshArgs);
rebuildLayerStacks();
calculateWorkingSet();
for (const auto& [token, display] : mDisplays) {
@@ -1808,6 +1818,10 @@
mTracing.notify("visibleRegionsDirty");
}
}
+
+ if (mCompositionEngine->needsAnotherUpdate()) {
+ signalLayerUpdate();
+ }
}
@@ -1953,30 +1967,6 @@
}
}
-void SurfaceFlinger::preComposition()
-{
- ATRACE_CALL();
- ALOGV("preComposition");
-
- mRefreshStartTime = systemTime(SYSTEM_TIME_MONOTONIC);
-
- bool needExtraInvalidate = false;
- mDrawingState.traverseInZOrder([&](Layer* layer) {
- auto compositionLayer = layer->getCompositionLayer();
- if (compositionLayer) {
- auto layerFE = compositionLayer->getLayerFE();
-
- if (layerFE && layerFE->onPreComposition(mRefreshStartTime)) {
- needExtraInvalidate = true;
- }
- }
- });
-
- if (needExtraInvalidate) {
- signalLayerUpdate();
- }
-}
-
void SurfaceFlinger::updateCompositorTiming(const DisplayStatInfo& stats, nsecs_t compositeTime,
std::shared_ptr<FenceTime>& presentFenceTime) {
// Update queue of past composite+present times and determine the
@@ -2070,10 +2060,11 @@
DisplayStatInfo stats;
mScheduler->getDisplayStatInfo(&stats);
- // We use the mRefreshStartTime which might be sampled a little later than
- // when we started doing work for this frame, but that should be okay
- // since updateCompositorTiming has snapping logic.
- updateCompositorTiming(stats, mRefreshStartTime, presentFenceTime);
+ // We use the CompositionEngine::getLastFrameRefreshTimestamp() which might
+ // be sampled a little later than when we started doing work for this frame,
+ // but that should be okay since updateCompositorTiming has snapping logic.
+ updateCompositorTiming(stats, mCompositionEngine->getLastFrameRefreshTimestamp(),
+ presentFenceTime);
CompositorTiming compositorTiming;
{
std::lock_guard<std::mutex> lock(getBE().mCompositorTimingLock);
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index fa801af..13cd137 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -747,7 +747,6 @@
void computeVisibleRegions(const sp<const DisplayDevice>& display, Region& dirtyRegion,
Region& opaqueRegion);
- void preComposition();
void postComposition();
void getCompositorTiming(CompositorTiming* compositorTiming);
void updateCompositorTiming(const DisplayStatInfo& stats, nsecs_t compositeTime,