SF: Dev Options should force client composition for all frames
While there was some code which tried to force it, it turned out that if
there was a geometry refresh for a frame, the flag would be cleared
after being set. It would then be forced on subsequent frames assuming
there were no geometry updates then.
This was caught by the libsurfaceflinger_unittest I was trying to add,
which caught it when it was re-run after I moved more code to
CompositionEngine.
This patch moves where the flag is set to after where it was being
cleared, which meant adding a parameter, and adding a bit more unit test
coverage for the revised function.
Test: atest libsurfaceflinger_unittest # With added test there
Test: atest libcompositionengine_test # With added test there
Bug: None
Change-Id: I4b96b21cdd4fe280f0943051962d3473e9113851
Merged-In: I4b96b21cdd4fe280f0943051962d3473e9113851
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/OutputLayer.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/OutputLayer.h
index 389b605..a9a95cd 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/OutputLayer.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/OutputLayer.h
@@ -74,7 +74,7 @@
// Recalculates the state of the output layer from the output-independent
// layer. If includeGeometry is false, the geometry state can be skipped.
- virtual void updateCompositionState(bool includeGeometry) = 0;
+ virtual void updateCompositionState(bool includeGeometry, bool forceClientComposition) = 0;
// Writes the geometry state to the HWC, or does nothing if this layer does
// not use the HWC. If includeGeometry is false, the geometry state can be
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayer.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayer.h
index 34dbfb7..95c8afb 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayer.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayer.h
@@ -39,7 +39,7 @@
void setHwcLayer(std::shared_ptr<HWC2::Layer>) override;
- void updateCompositionState(bool) override;
+ void updateCompositionState(bool includeGeometry, bool forceClientComposition) override;
void writeStateToHWC(bool) override;
void writeCursorPositionToHWC() const override;
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/OutputLayer.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/OutputLayer.h
index 4f2afac..631760a 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/OutputLayer.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/OutputLayer.h
@@ -40,7 +40,7 @@
MOCK_CONST_METHOD0(getState, const impl::OutputLayerCompositionState&());
MOCK_METHOD0(editState, impl::OutputLayerCompositionState&());
- MOCK_METHOD1(updateCompositionState, void(bool));
+ MOCK_METHOD2(updateCompositionState, void(bool, bool));
MOCK_METHOD1(writeStateToHWC, void(bool));
MOCK_CONST_METHOD0(writeCursorPositionToHWC, void());
diff --git a/services/surfaceflinger/CompositionEngine/src/Output.cpp b/services/surfaceflinger/CompositionEngine/src/Output.cpp
index 2007ea3..aa638b7 100644
--- a/services/surfaceflinger/CompositionEngine/src/Output.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp
@@ -552,11 +552,8 @@
ALOGV(__FUNCTION__);
for (auto* layer : getOutputLayersOrderedByZ()) {
- if (refreshArgs.devOptForceClientComposition) {
- layer->editState().forceClientComposition = true;
- }
-
- layer->updateCompositionState(refreshArgs.updatingGeometryThisFrame);
+ layer->updateCompositionState(refreshArgs.updatingGeometryThisFrame,
+ refreshArgs.devOptForceClientComposition);
// Send the updated state to the HWC, if appropriate.
layer->writeStateToHWC(refreshArgs.updatingGeometryThisFrame);
diff --git a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp
index 0124e5b..721e953 100644
--- a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp
@@ -259,7 +259,7 @@
return transform.getOrientation();
} // namespace impl
-void OutputLayer::updateCompositionState(bool includeGeometry) {
+void OutputLayer::updateCompositionState(bool includeGeometry, bool forceClientComposition) {
const auto& layerFEState = getLayer().getFEState();
const auto& outputState = getOutput().getState();
const auto& profile = *getOutput().getDisplayColorProfile();
@@ -294,7 +294,8 @@
// These are evaluated every frame as they can potentially change at any
// time.
- if (layerFEState.forceClientComposition || !profile.isDataspaceSupported(state.dataspace)) {
+ if (layerFEState.forceClientComposition || !profile.isDataspaceSupported(state.dataspace) ||
+ forceClientComposition) {
state.forceClientComposition = true;
}
}
diff --git a/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp b/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp
index 0347f75..a338784 100644
--- a/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp
@@ -501,7 +501,7 @@
setupGeometryChildCallValues();
- mOutputLayer.updateCompositionState(true);
+ mOutputLayer.updateCompositionState(true, false);
validateComputedGeometryState();
@@ -515,7 +515,7 @@
setupGeometryChildCallValues();
- mOutputLayer.updateCompositionState(true);
+ mOutputLayer.updateCompositionState(true, false);
validateComputedGeometryState();
@@ -531,7 +531,7 @@
setupGeometryChildCallValues();
- mOutputLayer.updateCompositionState(true);
+ mOutputLayer.updateCompositionState(true, false);
validateComputedGeometryState();
@@ -546,7 +546,7 @@
// should use the layers requested colorspace.
mLayerFEState.isColorspaceAgnostic = false;
- mOutputLayer.updateCompositionState(false);
+ mOutputLayer.updateCompositionState(false, false);
EXPECT_EQ(ui::Dataspace::DISPLAY_P3, mOutputLayer.getState().dataspace);
@@ -554,7 +554,7 @@
// should use the colorspace chosen for the whole output.
mLayerFEState.isColorspaceAgnostic = true;
- mOutputLayer.updateCompositionState(false);
+ mOutputLayer.updateCompositionState(false, false);
EXPECT_EQ(ui::Dataspace::V0_SCRGB, mOutputLayer.getState().dataspace);
}
@@ -562,7 +562,7 @@
TEST_F(OutputLayerUpdateCompositionStateTest, doesNotRecomputeGeometryIfNotRequested) {
mOutputLayer.editState().forceClientComposition = false;
- mOutputLayer.updateCompositionState(false);
+ mOutputLayer.updateCompositionState(false, false);
EXPECT_EQ(false, mOutputLayer.getState().forceClientComposition);
}
@@ -571,7 +571,7 @@
doesNotClearForceClientCompositionIfNotDoingGeometry) {
mOutputLayer.editState().forceClientComposition = true;
- mOutputLayer.updateCompositionState(false);
+ mOutputLayer.updateCompositionState(false, false);
EXPECT_EQ(true, mOutputLayer.getState().forceClientComposition);
}
@@ -580,7 +580,7 @@
mLayerFEState.forceClientComposition = true;
mOutputLayer.editState().forceClientComposition = false;
- mOutputLayer.updateCompositionState(false);
+ mOutputLayer.updateCompositionState(false, false);
EXPECT_EQ(true, mOutputLayer.getState().forceClientComposition);
}
@@ -590,7 +590,24 @@
mOutputLayer.editState().forceClientComposition = false;
EXPECT_CALL(mDisplayColorProfile, isDataspaceSupported(_)).WillRepeatedly(Return(false));
- mOutputLayer.updateCompositionState(false);
+ mOutputLayer.updateCompositionState(false, false);
+
+ EXPECT_EQ(true, mOutputLayer.getState().forceClientComposition);
+}
+
+TEST_F(OutputLayerUpdateCompositionStateTest, clientCompositionForcedFromArgumentFlag) {
+ mLayerFEState.forceClientComposition = false;
+ mOutputLayer.editState().forceClientComposition = false;
+
+ mOutputLayer.updateCompositionState(false, true);
+
+ EXPECT_EQ(true, mOutputLayer.getState().forceClientComposition);
+
+ mOutputLayer.editState().forceClientComposition = false;
+
+ setupGeometryChildCallValues();
+
+ mOutputLayer.updateCompositionState(true, true);
EXPECT_EQ(true, mOutputLayer.getState().forceClientComposition);
}