SF: Cleanup Layer membership test
Rather than passing the individual data for the layer (stack id,
primaryOnly flag), pass the entire layer, and have the Output class look
for the right data.
This makes it easier to change the membership logic later if desired.
Test: atest libsurfaceflinger_unittest libcompositionengine_test
Test: go/wm-smoke
Bug: 121291683
Change-Id: Ieccb3986dd4f045b68329825a7e3e05734811cc0
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h
index 2e44c07..80fce2c 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h
@@ -134,6 +134,9 @@
// this output allows that.
virtual bool belongsInOutput(std::optional<uint32_t> layerStackId, bool internalOnly) const = 0;
+ // Determines if a layer belongs to the output.
+ virtual bool belongsInOutput(const compositionengine::Layer*) const = 0;
+
// Returns a pointer to the output layer corresponding to the given layer on
// this output, or nullptr if the layer does not have one
virtual OutputLayer* getOutputLayerForLayer(Layer*) const = 0;
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h
index bdb35c2..6dde86d 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h
@@ -63,6 +63,7 @@
Region getDirtyRegion(bool repaintEverything) const override;
bool belongsInOutput(std::optional<uint32_t>, bool) const override;
+ bool belongsInOutput(const compositionengine::Layer*) const override;
compositionengine::OutputLayer* getOutputLayerForLayer(
compositionengine::Layer*) const override;
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h
index 93274e7..8cf7f79 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h
@@ -60,6 +60,7 @@
MOCK_CONST_METHOD1(getDirtyRegion, Region(bool));
MOCK_CONST_METHOD2(belongsInOutput, bool(std::optional<uint32_t>, bool));
+ MOCK_CONST_METHOD1(belongsInOutput, bool(const compositionengine::Layer*));
MOCK_CONST_METHOD1(getOutputLayerForLayer,
compositionengine::OutputLayer*(compositionengine::Layer*));
diff --git a/services/surfaceflinger/CompositionEngine/src/Output.cpp b/services/surfaceflinger/CompositionEngine/src/Output.cpp
index cb04e95..a9b1d55 100644
--- a/services/surfaceflinger/CompositionEngine/src/Output.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp
@@ -219,6 +219,15 @@
(!internalOnly || mState.layerStackInternal);
}
+bool Output::belongsInOutput(const compositionengine::Layer* layer) const {
+ if (!layer) {
+ return false;
+ }
+
+ const auto& layerFEState = layer->getState().frontEnd;
+ return belongsInOutput(layerFEState.layerStackId, layerFEState.internalOnly);
+}
+
compositionengine::OutputLayer* Output::getOutputLayerForLayer(
compositionengine::Layer* layer) const {
for (const auto& outputLayer : mOutputLayersOrderedByZ) {
diff --git a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp
index 10ec1ee..a0b8017 100644
--- a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp
@@ -376,6 +376,71 @@
EXPECT_FALSE(mOutput.belongsInOutput(layerStack2, false));
}
+TEST_F(OutputTest, belongsInOutputFiltersLayersAsExpected) {
+ StrictMock<mock::Layer> layer;
+ impl::LayerCompositionState layerState;
+
+ EXPECT_CALL(layer, getState()).WillRepeatedly(ReturnRef(layerState));
+
+ const uint32_t layerStack1 = 123u;
+ const uint32_t layerStack2 = 456u;
+
+ // If the output accepts layerStack1 and internal-only layers....
+ mOutput.setLayerStackFilter(layerStack1, true);
+
+ // A null layer pointer does not belong to the output
+ EXPECT_FALSE(mOutput.belongsInOutput(nullptr));
+
+ // A layer with no layerStack does not belong to it, internal-only or not.
+ layerState.frontEnd.layerStackId = std::nullopt;
+ layerState.frontEnd.internalOnly = false;
+ EXPECT_FALSE(mOutput.belongsInOutput(&layer));
+
+ layerState.frontEnd.layerStackId = std::nullopt;
+ layerState.frontEnd.internalOnly = true;
+ EXPECT_FALSE(mOutput.belongsInOutput(&layer));
+
+ // Any layer with layerStack1 belongs to it, internal-only or not.
+ layerState.frontEnd.layerStackId = layerStack1;
+ layerState.frontEnd.internalOnly = false;
+ EXPECT_TRUE(mOutput.belongsInOutput(&layer));
+
+ layerState.frontEnd.layerStackId = layerStack1;
+ layerState.frontEnd.internalOnly = true;
+ EXPECT_TRUE(mOutput.belongsInOutput(&layer));
+
+ layerState.frontEnd.layerStackId = layerStack2;
+ layerState.frontEnd.internalOnly = true;
+ EXPECT_FALSE(mOutput.belongsInOutput(&layer));
+
+ layerState.frontEnd.layerStackId = layerStack2;
+ layerState.frontEnd.internalOnly = false;
+ EXPECT_FALSE(mOutput.belongsInOutput(&layer));
+
+ // If the output accepts layerStack1 but not internal-only layers...
+ mOutput.setLayerStackFilter(layerStack1, false);
+
+ // A null layer pointer does not belong to the output
+ EXPECT_FALSE(mOutput.belongsInOutput(nullptr));
+
+ // Only non-internal layers with layerStack1 belong to it.
+ layerState.frontEnd.layerStackId = layerStack1;
+ layerState.frontEnd.internalOnly = false;
+ EXPECT_TRUE(mOutput.belongsInOutput(&layer));
+
+ layerState.frontEnd.layerStackId = layerStack1;
+ layerState.frontEnd.internalOnly = true;
+ EXPECT_FALSE(mOutput.belongsInOutput(&layer));
+
+ layerState.frontEnd.layerStackId = layerStack2;
+ layerState.frontEnd.internalOnly = true;
+ EXPECT_FALSE(mOutput.belongsInOutput(&layer));
+
+ layerState.frontEnd.layerStackId = layerStack2;
+ layerState.frontEnd.internalOnly = false;
+ EXPECT_FALSE(mOutput.belongsInOutput(&layer));
+}
+
/*
* Output::getOutputLayerForLayer()
*/