Clean up uses of refreshStartTime for stats.
refreshStartTime is keeps track of frame refreshes for
stats and can be set from SurfaceFlinger. This makes
subsequent changes in refactoring
SurfaceFlinger#composite cleaner and makes tracking the
metric more consistent between changes.
The refreshStartTime field can be removed entirely from
LayerFE's CompositionResult and removed as an argument
in LayerFE#onPreComposition.
Bug: b/294936197, b/329275965
Test: presubmit
Change-Id: I97988315f8982d05aec4b2684d4f5f1826a0fefa
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h
index 18a96f4..843b5c5 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h
@@ -19,6 +19,7 @@
#include <chrono>
#include <optional>
#include <vector>
+#include "utils/Timers.h"
#include <compositionengine/Display.h>
#include <compositionengine/LayerFE.h>
@@ -105,6 +106,9 @@
bool hasTrustedPresentationListener = false;
ICEPowerCallback* powerCallback = nullptr;
+
+ // System time for when frame refresh starts. Used for stats.
+ nsecs_t refreshStartTime = 0;
};
} // namespace android::compositionengine
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFE.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFE.h
index a1d6132..a499928 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFE.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFE.h
@@ -58,8 +58,7 @@
// Called before composition starts. Should return true if this layer has
// pending updates which would require an extra display refresh cycle to
// process.
- virtual bool onPreComposition(nsecs_t refreshStartTime,
- bool updatingOutputGeometryThisFrame) = 0;
+ virtual bool onPreComposition(bool updatingOutputGeometryThisFrame) = 0;
struct ClientCompositionTargetSettings {
enum class BlurSetting {
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h
index 15e4577..1b8cc27 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h
@@ -43,7 +43,7 @@
MOCK_CONST_METHOD0(getCompositionState, const LayerFECompositionState*());
- MOCK_METHOD2(onPreComposition, bool(nsecs_t, bool));
+ MOCK_METHOD1(onPreComposition, bool(bool));
MOCK_CONST_METHOD1(prepareClientComposition,
std::optional<compositionengine::LayerFE::LayerSettings>(
diff --git a/services/surfaceflinger/CompositionEngine/src/CompositionEngine.cpp b/services/surfaceflinger/CompositionEngine/src/CompositionEngine.cpp
index d87eae3..b470208 100644
--- a/services/surfaceflinger/CompositionEngine/src/CompositionEngine.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/CompositionEngine.cpp
@@ -181,10 +181,10 @@
bool needsAnotherUpdate = false;
- mRefreshStartTime = systemTime(SYSTEM_TIME_MONOTONIC);
+ mRefreshStartTime = args.refreshStartTime;
for (auto& layer : args.layers) {
- if (layer->onPreComposition(mRefreshStartTime, args.updatingOutputGeometryThisFrame)) {
+ if (layer->onPreComposition(args.updatingOutputGeometryThisFrame)) {
needsAnotherUpdate = true;
}
}
diff --git a/services/surfaceflinger/CompositionEngine/tests/CompositionEngineTest.cpp b/services/surfaceflinger/CompositionEngine/tests/CompositionEngineTest.cpp
index da578e2..042010e 100644
--- a/services/surfaceflinger/CompositionEngine/tests/CompositionEngineTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/CompositionEngineTest.cpp
@@ -214,6 +214,7 @@
TEST_F(CompositionTestPreComposition, preCompositionSetsFrameTimestamp) {
const nsecs_t before = systemTime(SYSTEM_TIME_MONOTONIC);
+ mRefreshArgs.refreshStartTime = systemTime(SYSTEM_TIME_MONOTONIC);
mEngine.preComposition(mRefreshArgs);
const nsecs_t after = systemTime(SYSTEM_TIME_MONOTONIC);
@@ -226,12 +227,9 @@
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)));
+ 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)));
mRefreshArgs.outputs = {mOutput1};
mRefreshArgs.layers = {mLayer1FE, mLayer2FE, mLayer3FE};
@@ -245,9 +243,9 @@
}
TEST_F(CompositionTestPreComposition, preCompositionDefaultsToNoUpdateNeeded) {
- EXPECT_CALL(*mLayer1FE, onPreComposition(_, _)).WillOnce(Return(false));
- EXPECT_CALL(*mLayer2FE, onPreComposition(_, _)).WillOnce(Return(false));
- EXPECT_CALL(*mLayer3FE, onPreComposition(_, _)).WillOnce(Return(false));
+ EXPECT_CALL(*mLayer1FE, onPreComposition(_)).WillOnce(Return(false));
+ EXPECT_CALL(*mLayer2FE, onPreComposition(_)).WillOnce(Return(false));
+ EXPECT_CALL(*mLayer3FE, onPreComposition(_)).WillOnce(Return(false));
mEngine.setNeedsAnotherUpdateForTest(true);
@@ -262,9 +260,9 @@
TEST_F(CompositionTestPreComposition,
preCompositionSetsNeedsAnotherUpdateIfAtLeastOneLayerRequestsIt) {
- EXPECT_CALL(*mLayer1FE, onPreComposition(_, _)).WillOnce(Return(true));
- EXPECT_CALL(*mLayer2FE, onPreComposition(_, _)).WillOnce(Return(false));
- EXPECT_CALL(*mLayer3FE, onPreComposition(_, _)).WillOnce(Return(false));
+ EXPECT_CALL(*mLayer1FE, onPreComposition(_)).WillOnce(Return(true));
+ EXPECT_CALL(*mLayer2FE, onPreComposition(_)).WillOnce(Return(false));
+ EXPECT_CALL(*mLayer3FE, onPreComposition(_)).WillOnce(Return(false));
mRefreshArgs.outputs = {mOutput1};
mRefreshArgs.layers = {mLayer1FE, mLayer2FE, mLayer3FE};
diff --git a/services/surfaceflinger/LayerFE.cpp b/services/surfaceflinger/LayerFE.cpp
index 2dbcb84..43a4397 100644
--- a/services/surfaceflinger/LayerFE.cpp
+++ b/services/surfaceflinger/LayerFE.cpp
@@ -82,8 +82,7 @@
return mSnapshot.get();
}
-bool LayerFE::onPreComposition(nsecs_t refreshStartTime, bool) {
- mCompositionResult.refreshStartTime = refreshStartTime;
+bool LayerFE::onPreComposition(bool) {
return mSnapshot->hasReadyFrame;
}
diff --git a/services/surfaceflinger/LayerFE.h b/services/surfaceflinger/LayerFE.h
index d584fb7..66cb88b 100644
--- a/services/surfaceflinger/LayerFE.h
+++ b/services/surfaceflinger/LayerFE.h
@@ -26,9 +26,6 @@
namespace android {
struct CompositionResult {
- // TODO(b/238781169) update CE to no longer pass refreshStartTime to LayerFE::onPreComposition
- // and remove this field.
- nsecs_t refreshStartTime = 0;
std::vector<std::pair<ftl::SharedFuture<FenceResult>, ui::LayerStack>> releaseFences;
sp<Fence> lastClientCompositionFence = nullptr;
};
@@ -39,7 +36,7 @@
// compositionengine::LayerFE overrides
const compositionengine::LayerFECompositionState* getCompositionState() const override;
- bool onPreComposition(nsecs_t refreshStartTime, bool updatingOutputGeometryThisFrame) override;
+ bool onPreComposition(bool updatingOutputGeometryThisFrame) override;
void onLayerDisplayed(ftl::SharedFuture<FenceResult>, ui::LayerStack) override;
const char* getDebugName() const override;
int32_t getSequence() const override;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 78e14ae..bf210af 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2783,12 +2783,16 @@
}
}
+ refreshArgs.refreshStartTime = systemTime(SYSTEM_TIME_MONOTONIC);
+ for (auto [layer, layerFE] : layers) {
+ layer->onPreComposition(refreshArgs.refreshStartTime);
+ }
+
mCompositionEngine->present(refreshArgs);
moveSnapshotsFromCompositionArgs(refreshArgs, layers);
for (auto [layer, layerFE] : layers) {
CompositionResult compositionResult{layerFE->stealCompositionResult()};
- layer->onPreComposition(compositionResult.refreshStartTime);
for (auto& [releaseFence, layerStack] : compositionResult.releaseFences) {
Layer* clonedFrom = layer->getClonedFrom().get();
auto owningLayer = clonedFrom ? clonedFrom : layer;