From 7a234910c213658d640cbff334c7e23e6b65f0a4 Mon Sep 17 00:00:00 2001 From: Lloyd Pique Date: Thu, 3 Oct 2019 11:54:27 -0700 Subject: 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 --- .../include/compositionengine/OutputLayer.h | 2 +- .../include/compositionengine/impl/OutputLayer.h | 2 +- .../include/compositionengine/mock/OutputLayer.h | 2 +- .../CompositionEngine/src/Output.cpp | 7 ++--- .../CompositionEngine/src/OutputLayer.cpp | 5 ++-- .../CompositionEngine/tests/OutputLayerTest.cpp | 35 ++++++++++++++++------ 6 files changed, 34 insertions(+), 19 deletions(-) diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/OutputLayer.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/OutputLayer.h index 389b6059f3..a9a95cdb92 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/OutputLayer.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/OutputLayer.h @@ -74,7 +74,7 @@ public: // 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 34dbfb777d..95c8afbede 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayer.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayer.h @@ -39,7 +39,7 @@ public: void setHwcLayer(std::shared_ptr) 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 4f2afacd63..631760afe7 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/OutputLayer.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/OutputLayer.h @@ -40,7 +40,7 @@ public: 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 2007ea3068..aa638b7d91 100644 --- a/services/surfaceflinger/CompositionEngine/src/Output.cpp +++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp @@ -552,11 +552,8 @@ void Output::updateAndWriteCompositionState( 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 0124e5b5d9..721e953d25 100644 --- a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp +++ b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp @@ -259,7 +259,7 @@ uint32_t OutputLayer::calculateOutputRelativeBufferTransform() const { 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 @@ void OutputLayer::updateCompositionState(bool includeGeometry) { // 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 0347f759e7..a33878448f 100644 --- a/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp @@ -501,7 +501,7 @@ TEST_F(OutputLayerUpdateCompositionStateTest, setsStateNormally) { setupGeometryChildCallValues(); - mOutputLayer.updateCompositionState(true); + mOutputLayer.updateCompositionState(true, false); validateComputedGeometryState(); @@ -515,7 +515,7 @@ TEST_F(OutputLayerUpdateCompositionStateTest, setupGeometryChildCallValues(); - mOutputLayer.updateCompositionState(true); + mOutputLayer.updateCompositionState(true, false); validateComputedGeometryState(); @@ -531,7 +531,7 @@ TEST_F(OutputLayerUpdateCompositionStateTest, setupGeometryChildCallValues(); - mOutputLayer.updateCompositionState(true); + mOutputLayer.updateCompositionState(true, false); validateComputedGeometryState(); @@ -546,7 +546,7 @@ TEST_F(OutputLayerUpdateCompositionStateTest, setsOutputLayerColorspaceCorrectly // 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 @@ TEST_F(OutputLayerUpdateCompositionStateTest, setsOutputLayerColorspaceCorrectly // 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, setsOutputLayerColorspaceCorrectly 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 @@ TEST_F(OutputLayerUpdateCompositionStateTest, doesNotClearForceClientCompositionIfNotDoingGeometry) { mOutputLayer.editState().forceClientComposition = true; - mOutputLayer.updateCompositionState(false); + mOutputLayer.updateCompositionState(false, false); EXPECT_EQ(true, mOutputLayer.getState().forceClientComposition); } @@ -580,7 +580,7 @@ TEST_F(OutputLayerUpdateCompositionStateTest, clientCompositionForcedFromFrontEn mLayerFEState.forceClientComposition = true; mOutputLayer.editState().forceClientComposition = false; - mOutputLayer.updateCompositionState(false); + mOutputLayer.updateCompositionState(false, false); EXPECT_EQ(true, mOutputLayer.getState().forceClientComposition); } @@ -590,7 +590,24 @@ TEST_F(OutputLayerUpdateCompositionStateTest, 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); } -- cgit v1.2.3-59-g8ed1b