Merge "Revert "Allow for solid color layers to start some candidate cached sets.""
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayer.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayer.h
index c15249d..244f8ab 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayer.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayer.h
@@ -73,9 +73,8 @@
void writeOutputIndependentGeometryStateToHWC(HWC2::Layer*, const LayerFECompositionState&,
bool skipLayer);
void writeOutputDependentPerFrameStateToHWC(HWC2::Layer*);
- void writeOutputIndependentPerFrameStateToHWC(
- HWC2::Layer*, const LayerFECompositionState&,
- Hwc2::IComposerClient::Composition compositionType, bool skipLayer);
+ void writeOutputIndependentPerFrameStateToHWC(HWC2::Layer*, const LayerFECompositionState&,
+ bool skipLayer);
void writeSolidColorStateToHWC(HWC2::Layer*, const LayerFECompositionState&);
void writeSidebandStateToHWC(HWC2::Layer*, const LayerFECompositionState&);
void writeBufferStateToHWC(HWC2::Layer*, const LayerFECompositionState&, bool skipLayer);
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/Flattener.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/Flattener.h
index 92cc484..c132041 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/Flattener.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/Flattener.h
@@ -134,22 +134,21 @@
class Builder {
private:
std::vector<CachedSet>::const_iterator mStart;
- int32_t mNumSets = 0;
+ std::vector<size_t> mLengths;
const CachedSet* mHolePunchCandidate = nullptr;
const CachedSet* mBlurringLayer = nullptr;
- bool mBuilt = false;
public:
// Initializes a Builder a CachedSet to start from.
// This start iterator must be an iterator for mLayers
void init(const std::vector<CachedSet>::const_iterator& start) {
mStart = start;
- mNumSets = 1;
+ mLengths.push_back(start->getLayerCount());
}
// Appends a new CachedSet to the end of the run
// The provided length must be the size of the next sequential CachedSet in layers
- void increment() { mNumSets++; }
+ void append(size_t length) { mLengths.push_back(length); }
// Sets the hole punch candidate for the Run.
void setHolePunchCandidate(const CachedSet* holePunchCandidate) {
@@ -162,36 +161,19 @@
// Builds a Run instance, if a valid Run may be built.
std::optional<Run> validateAndBuild() {
- const bool built = mBuilt;
- mBuilt = true;
- if (mNumSets <= 0 || built) {
+ if (mLengths.size() == 0) {
+ return std::nullopt;
+ }
+ // Runs of length 1 which are hole punch candidates are allowed if the candidate is
+ // going to be used.
+ if (mLengths.size() == 1 &&
+ (!mHolePunchCandidate || !(mHolePunchCandidate->requiresHolePunch()))) {
return std::nullopt;
}
- const bool requiresHolePunch =
- mHolePunchCandidate && mHolePunchCandidate->requiresHolePunch();
-
- if (!requiresHolePunch) {
- // If we don't require a hole punch, then treat solid color layers at the front
- // to be "cheap", so remove them from the candidate cached set.
- while (mNumSets > 1 && mStart->getLayerCount() == 1 &&
- mStart->getFirstLayer().getBuffer() == nullptr) {
- mStart++;
- mNumSets--;
- }
-
- // Only allow for single cached sets if a hole punch is required. If we're here,
- // then we don't require a hole punch, so don't build a run.
- if (mNumSets <= 1) {
- return std::nullopt;
- }
- }
-
return Run(mStart,
- std::reduce(mStart, mStart + mNumSets, 0u,
- [](size_t length, const CachedSet& set) {
- return length + set.getLayerCount();
- }),
+ std::reduce(mLengths.cbegin(), mLengths.cend(), 0u,
+ [](size_t left, size_t right) { return left + right; }),
mHolePunchCandidate, mBlurringLayer);
}
diff --git a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp
index cf76183..e958549 100644
--- a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp
@@ -347,10 +347,6 @@
auto requestedCompositionType = outputIndependentState->compositionType;
- if (requestedCompositionType == hal::Composition::SOLID_COLOR && state.overrideInfo.buffer) {
- requestedCompositionType = hal::Composition::DEVICE;
- }
-
// TODO(b/181172795): We now update geometry for all flattened layers. We should update it
// only when the geometry actually changes
const bool isOverridden =
@@ -363,15 +359,13 @@
}
writeOutputDependentPerFrameStateToHWC(hwcLayer.get());
- writeOutputIndependentPerFrameStateToHWC(hwcLayer.get(), *outputIndependentState,
- requestedCompositionType, skipLayer);
+ writeOutputIndependentPerFrameStateToHWC(hwcLayer.get(), *outputIndependentState, skipLayer);
writeCompositionTypeToHWC(hwcLayer.get(), requestedCompositionType, isPeekingThrough,
skipLayer);
- if (requestedCompositionType == hal::Composition::SOLID_COLOR) {
- writeSolidColorStateToHWC(hwcLayer.get(), *outputIndependentState);
- }
+ // Always set the layer color after setting the composition type.
+ writeSolidColorStateToHWC(hwcLayer.get(), *outputIndependentState);
editState().hwc->stateOverridden = isOverridden;
editState().hwc->layerSkipped = skipLayer;
@@ -483,7 +477,7 @@
void OutputLayer::writeOutputIndependentPerFrameStateToHWC(
HWC2::Layer* hwcLayer, const LayerFECompositionState& outputIndependentState,
- hal::Composition compositionType, bool skipLayer) {
+ bool skipLayer) {
switch (auto error = hwcLayer->setColorTransform(outputIndependentState.colorTransform)) {
case hal::Error::NONE:
break;
@@ -507,7 +501,7 @@
}
// Content-specific per-frame state
- switch (compositionType) {
+ switch (outputIndependentState.compositionType) {
case hal::Composition::SOLID_COLOR:
// For compatibility, should be written AFTER the composition type.
break;
@@ -527,6 +521,10 @@
void OutputLayer::writeSolidColorStateToHWC(HWC2::Layer* hwcLayer,
const LayerFECompositionState& outputIndependentState) {
+ if (outputIndependentState.compositionType != hal::Composition::SOLID_COLOR) {
+ return;
+ }
+
hal::Color color = {static_cast<uint8_t>(std::round(255.0f * outputIndependentState.color.r)),
static_cast<uint8_t>(std::round(255.0f * outputIndependentState.color.g)),
static_cast<uint8_t>(std::round(255.0f * outputIndependentState.color.b)),
diff --git a/services/surfaceflinger/CompositionEngine/src/planner/Flattener.cpp b/services/surfaceflinger/CompositionEngine/src/planner/Flattener.cpp
index c14effc..8a476c5 100644
--- a/services/surfaceflinger/CompositionEngine/src/planner/Flattener.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/planner/Flattener.cpp
@@ -425,13 +425,18 @@
if (layerIsInactive && (firstLayer || runHasFirstLayer || !layerHasBlur) &&
!currentSet->hasUnsupportedDataspace()) {
if (isPartOfRun) {
- builder.increment();
+ builder.append(currentSet->getLayerCount());
} else {
- builder.init(currentSet);
- if (firstLayer) {
- runHasFirstLayer = true;
+ // Runs can't start with a non-buffer layer
+ if (currentSet->getFirstLayer().getBuffer() == nullptr) {
+ ALOGV("[%s] Skipping initial non-buffer layer", __func__);
+ } else {
+ builder.init(currentSet);
+ if (firstLayer) {
+ runHasFirstLayer = true;
+ }
+ isPartOfRun = true;
}
- isPartOfRun = true;
}
} else if (isPartOfRun) {
builder.setHolePunchCandidate(&(*currentSet));
diff --git a/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp b/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp
index 31b131a..fbc2089 100644
--- a/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp
@@ -1109,22 +1109,6 @@
/*zIsOverridden*/ false, /*isPeekingThrough*/ false);
}
-TEST_F(OutputLayerWriteStateToHWCTest, includesOverrideInfoForSolidColorIfPresent) {
- mLayerFEState.compositionType = Hwc2::IComposerClient::Composition::SOLID_COLOR;
- includeOverrideInfo();
-
- expectGeometryCommonCalls(kOverrideDisplayFrame, kOverrideSourceCrop, kOverrideBufferTransform,
- kOverrideBlendMode, kOverrideAlpha);
- expectPerFrameCommonCalls(SimulateUnsupported::None, kOverrideDataspace, kOverrideVisibleRegion,
- kOverrideSurfaceDamage);
- expectSetHdrMetadataAndBufferCalls(kOverrideHwcSlot, kOverrideBuffer, kOverrideFence);
- expectSetCompositionTypeCall(Hwc2::IComposerClient::Composition::DEVICE);
- EXPECT_CALL(*mLayerFE, hasRoundedCorners()).WillRepeatedly(Return(false));
-
- mOutputLayer.writeStateToHWC(/*includeGeometry*/ true, /*skipLayer*/ false, 0,
- /*zIsOverridden*/ false, /*isPeekingThrough*/ false);
-}
-
TEST_F(OutputLayerWriteStateToHWCTest, previousOverriddenLayerSendsSurfaceDamage) {
mLayerFEState.compositionType = Hwc2::IComposerClient::Composition::DEVICE;
mOutputLayer.editState().hwc->stateOverridden = true;
diff --git a/services/surfaceflinger/CompositionEngine/tests/planner/FlattenerTest.cpp b/services/surfaceflinger/CompositionEngine/tests/planner/FlattenerTest.cpp
index 35d051e..6fbcc75 100644
--- a/services/surfaceflinger/CompositionEngine/tests/planner/FlattenerTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/planner/FlattenerTest.cpp
@@ -762,76 +762,6 @@
EXPECT_EQ(nullptr, peekThroughLayer1);
}
-TEST_F(FlattenerTest, flattenLayers_holePunchSingleColorLayer) {
- mTestLayers[0]->outputLayerCompositionState.displayFrame = Rect(0, 0, 5, 5);
- mTestLayers[0]->layerFECompositionState.color = half4(255.f, 0.f, 0.f, 255.f);
- mTestLayers[0]->layerFECompositionState.buffer = nullptr;
-
- // An opaque static background
- auto& layerState0 = mTestLayers[0]->layerState;
- const auto& overrideBuffer0 = layerState0->getOutputLayer()->getState().overrideInfo.buffer;
-
- // a rounded updating layer
- auto& layerState1 = mTestLayers[1]->layerState;
- const auto& overrideBuffer1 = layerState1->getOutputLayer()->getState().overrideInfo.buffer;
-
- EXPECT_CALL(*mTestLayers[1]->layerFE, hasRoundedCorners()).WillRepeatedly(Return(true));
-
- std::vector<LayerFE::LayerSettings> clientCompositionList = {
- LayerFE::LayerSettings{},
- };
- clientCompositionList[0].source.buffer.buffer = std::make_shared<
- renderengine::ExternalTexture>(mTestLayers[1]->layerFECompositionState.buffer,
- mRenderEngine,
- renderengine::ExternalTexture::Usage::READABLE);
- EXPECT_CALL(*mTestLayers[1]->layerFE, prepareClientCompositionList(_))
- .WillOnce(Return(clientCompositionList));
-
- const std::vector<const LayerState*> layers = {
- layerState0.get(),
- layerState1.get(),
- };
-
- initializeFlattener(layers);
-
- // layer 1 satisfies every condition in CachedSet::requiresHolePunch()
- mTime += 200ms;
- layerState1->resetFramesSinceBufferUpdate(); // it is updating
-
- initializeOverrideBuffer(layers);
- // Expect no cache invalidation the first time (there's no cache yet)
- EXPECT_EQ(getNonBufferHash(layers),
- mFlattener->flattenLayers(layers, getNonBufferHash(layers), mTime));
-
- // This will render a CachedSet of layer 0. Though it is just one layer, it satisfies the
- // exception that there would be a hole punch above it.
- EXPECT_CALL(mRenderEngine, drawLayers(_, _, _, _, _))
- .WillOnce(Return(ByMove(
- futureOf<renderengine::RenderEngineResult>({NO_ERROR, base::unique_fd()}))));
- mFlattener->renderCachedSets(mOutputState, std::nullopt);
-
- // We've rendered a CachedSet, but we haven't merged it in.
- EXPECT_EQ(nullptr, overrideBuffer0);
-
- // This time we merge the CachedSet in and we should still have only two sets.
- EXPECT_CALL(mRenderEngine, drawLayers(_, _, _, _, _)).Times(0);
- initializeOverrideBuffer(layers);
- EXPECT_EQ(getNonBufferHash(layers),
- mFlattener->flattenLayers(layers, getNonBufferHash(layers), mTime));
- mFlattener->renderCachedSets(mOutputState, std::nullopt);
-
- EXPECT_NE(nullptr, overrideBuffer0); // got overridden
- EXPECT_EQ(nullptr, overrideBuffer1); // did not
-
- // expect 0's peek though layer to be 1's output layer
- const auto* peekThroughLayer0 =
- layerState0->getOutputLayer()->getState().overrideInfo.peekThroughLayer;
- const auto* peekThroughLayer1 =
- layerState1->getOutputLayer()->getState().overrideInfo.peekThroughLayer;
- EXPECT_EQ(&mTestLayers[1]->outputLayer, peekThroughLayer0);
- EXPECT_EQ(nullptr, peekThroughLayer1);
-}
-
TEST_F(FlattenerTest, flattenLayers_flattensBlurBehindRunIfFirstRun) {
auto& layerState1 = mTestLayers[0]->layerState;
@@ -1277,61 +1207,5 @@
EXPECT_EQ(nullptr, overrideBuffer3);
}
-TEST_F(FlattenerTest, flattenLayers_skipsColorLayers) {
- auto& layerState1 = mTestLayers[0]->layerState;
- const auto& overrideBuffer1 = layerState1->getOutputLayer()->getState().overrideInfo.buffer;
- auto& layerState2 = mTestLayers[1]->layerState;
- const auto& overrideBuffer2 = layerState2->getOutputLayer()->getState().overrideInfo.buffer;
- auto& layerState3 = mTestLayers[2]->layerState;
- const auto& overrideBuffer3 = layerState3->getOutputLayer()->getState().overrideInfo.buffer;
- auto& layerState4 = mTestLayers[3]->layerState;
- const auto& overrideBuffer4 = layerState4->getOutputLayer()->getState().overrideInfo.buffer;
-
- // Rewrite the first two layers to just be a solid color.
- mTestLayers[0]->layerFECompositionState.color = half4(255.f, 0.f, 0.f, 255.f);
- mTestLayers[0]->layerFECompositionState.buffer = nullptr;
- mTestLayers[1]->layerFECompositionState.color = half4(0.f, 255.f, 0.f, 255.f);
- mTestLayers[1]->layerFECompositionState.buffer = nullptr;
-
- const std::vector<const LayerState*> layers = {
- layerState1.get(),
- layerState2.get(),
- layerState3.get(),
- layerState4.get(),
- };
-
- initializeFlattener(layers);
-
- mTime += 200ms;
- initializeOverrideBuffer(layers);
- EXPECT_EQ(getNonBufferHash(layers),
- mFlattener->flattenLayers(layers, getNonBufferHash(layers), mTime));
-
- // This will render a CachedSet.
- EXPECT_CALL(mRenderEngine, drawLayers(_, _, _, _, _))
- .WillOnce(Return(ByMove(
- futureOf<renderengine::RenderEngineResult>({NO_ERROR, base::unique_fd()}))));
- mFlattener->renderCachedSets(mOutputState, std::nullopt);
-
- // We've rendered a CachedSet, but we haven't merged it in.
- EXPECT_EQ(nullptr, overrideBuffer1);
- EXPECT_EQ(nullptr, overrideBuffer2);
- EXPECT_EQ(nullptr, overrideBuffer3);
- EXPECT_EQ(nullptr, overrideBuffer4);
-
- // This time we merge the CachedSet in, so we have a new hash, and we should
- // only have two sets.
- EXPECT_CALL(mRenderEngine, drawLayers(_, _, _, _, _)).Times(0);
- initializeOverrideBuffer(layers);
- EXPECT_NE(getNonBufferHash(layers),
- mFlattener->flattenLayers(layers, getNonBufferHash(layers), mTime));
- mFlattener->renderCachedSets(mOutputState, std::nullopt);
-
- EXPECT_EQ(nullptr, overrideBuffer1);
- EXPECT_EQ(nullptr, overrideBuffer2);
- EXPECT_EQ(overrideBuffer3, overrideBuffer4);
- EXPECT_NE(nullptr, overrideBuffer4);
-}
-
} // namespace
} // namespace android::compositionengine