Resolve subtle but severe flickering in layer caching
Inconsistencies in composer state was arising from a layer exiting layer
caching. Resolve this by:
* If a layer previously had an override buffer, but now does not for the
current frame, set the surface damage region to be the entire buffer.
* If a layer is currently being skipped, then don't update the
composition type, since the validated composition type is no longer
accurate for representing the current composition type of the device.
* If a layer was previously skipped, then update in HWC the new
composition type every time.
The inconsistencies have arised from the layer caching feature
because composer implementations cache the most recent requested
composition type for each layer, but SurfaceFlinger caches the most
recent {requested, validated} type for each layer, whichever
SurfaceFlinger sees last. So if we forced client composition, then
untoggled client composition, and if hwc was device compositing the
layer during validate instead of client compositing because hwc ignored
the layer, then SF caches DEVICE composition, but hwc caches CLIENT
composition internally. SF then doesn't update the layer's composition
type because it cached DEVICE composition, but hwc never displays the
layer because hwc cached CLIENT composition, and typical hwc
implementations never attempt to read from client composited layers.
Bug: 187193705
Test: libcompositionengine_test
Test: enable layer caching and repeatedly enable and disable client
composition when pip is playing
Change-Id: I9b6890bcf3f0612e8f99f5f1642015e53d59f862
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayer.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayer.h
index 2488c66..2ffd472 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayer.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayer.h
@@ -78,9 +78,10 @@
void writeSidebandStateToHWC(HWC2::Layer*, const LayerFECompositionState&);
void writeBufferStateToHWC(HWC2::Layer*, const LayerFECompositionState&);
void writeCompositionTypeToHWC(HWC2::Layer*, Hwc2::IComposerClient::Composition,
- bool isPeekingThrough);
+ bool isPeekingThrough, bool skipLayer);
void detectDisallowedCompositionTypeChange(Hwc2::IComposerClient::Composition from,
Hwc2::IComposerClient::Composition to) const;
+ bool isClientCompositionForced(bool isPeekingThrough) const;
};
// This template factory function standardizes the implementation details of the
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayerCompositionState.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayerCompositionState.h
index b98043b..3f670a1 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayerCompositionState.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayerCompositionState.h
@@ -125,6 +125,9 @@
// Set to true when overridden info has been sent to HW composer
bool stateOverridden = false;
+
+ // True when this layer was skipped as part of SF-side layer caching.
+ bool layerSkipped = false;
};
// The HWC state is optional, and is only set up if there is any potential
diff --git a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp
index 01da070..cd14327 100644
--- a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp
@@ -14,6 +14,7 @@
* limitations under the License.
*/
+#include <DisplayHardware/Hal.h>
#include <android-base/stringprintf.h>
#include <compositionengine/DisplayColorProfile.h>
#include <compositionengine/LayerFECompositionState.h>
@@ -350,12 +351,14 @@
writeOutputDependentPerFrameStateToHWC(hwcLayer.get());
writeOutputIndependentPerFrameStateToHWC(hwcLayer.get(), *outputIndependentState);
- writeCompositionTypeToHWC(hwcLayer.get(), requestedCompositionType, isPeekingThrough);
+ writeCompositionTypeToHWC(hwcLayer.get(), requestedCompositionType, isPeekingThrough,
+ skipLayer);
// Always set the layer color after setting the composition type.
writeSolidColorStateToHWC(hwcLayer.get(), *outputIndependentState);
editState().hwc->stateOverridden = isOverridden;
+ editState().hwc->layerSkipped = skipLayer;
}
void OutputLayer::writeOutputDependentGeometryStateToHWC(HWC2::Layer* hwcLayer,
@@ -482,7 +485,8 @@
const Region& surfaceDamage = getState().overrideInfo.buffer
? getState().overrideInfo.damageRegion
- : outputIndependentState.surfaceDamage;
+ : (getState().hwc->stateOverridden ? Region::INVALID_REGION
+ : outputIndependentState.surfaceDamage);
if (auto error = hwcLayer->setSurfaceDamage(surfaceDamage); error != hal::Error::NONE) {
ALOGE("[%s] Failed to set surface damage: %s (%d)", getLayerFE().getDebugName(),
@@ -573,17 +577,19 @@
void OutputLayer::writeCompositionTypeToHWC(HWC2::Layer* hwcLayer,
hal::Composition requestedCompositionType,
- bool isPeekingThrough) {
+ bool isPeekingThrough, bool skipLayer) {
auto& outputDependentState = editState();
- // If we are forcing client composition, we need to tell the HWC
- if (outputDependentState.forceClientComposition ||
- (!isPeekingThrough && getLayerFE().hasRoundedCorners())) {
+ if (isClientCompositionForced(isPeekingThrough)) {
+ // If we are forcing client composition, we need to tell the HWC
requestedCompositionType = hal::Composition::CLIENT;
}
// Set the requested composition type with the HWC whenever it changes
- if (outputDependentState.hwc->hwcCompositionType != requestedCompositionType) {
+ // We also resend the composition type when this layer was previously skipped, to ensure that
+ // the composition type is up-to-date.
+ if (outputDependentState.hwc->hwcCompositionType != requestedCompositionType ||
+ (outputDependentState.hwc->layerSkipped && !skipLayer)) {
outputDependentState.hwc->hwcCompositionType = requestedCompositionType;
if (auto error = hwcLayer->setCompositionType(requestedCompositionType);
@@ -663,12 +669,22 @@
}
}
+bool OutputLayer::isClientCompositionForced(bool isPeekingThrough) const {
+ return getState().forceClientComposition ||
+ (!isPeekingThrough && getLayerFE().hasRoundedCorners());
+}
+
void OutputLayer::applyDeviceCompositionTypeChange(hal::Composition compositionType) {
auto& state = editState();
LOG_FATAL_IF(!state.hwc);
auto& hwcState = *state.hwc;
- detectDisallowedCompositionTypeChange(hwcState.hwcCompositionType, compositionType);
+ // Only detected disallowed changes if this was not a skip layer, because the
+ // validated composition type may be arbitrary (usually DEVICE, to reflect that there were
+ // fewer GPU layers)
+ if (!hwcState.layerSkipped) {
+ detectDisallowedCompositionTypeChange(hwcState.hwcCompositionType, compositionType);
+ }
hwcState.hwcCompositionType = compositionType;
}
diff --git a/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp b/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp
index 3adfe40..5bd1216 100644
--- a/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp
@@ -1065,12 +1065,62 @@
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;
+
+ expectGeometryCommonCalls();
+ expectPerFrameCommonCalls(SimulateUnsupported::None, kDataspace, kOutputSpaceVisibleRegion,
+ Region::INVALID_REGION);
+ expectSetHdrMetadataAndBufferCalls();
+ 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, previousSkipLayerSendsUpdatedDeviceCompositionInfo) {
+ mLayerFEState.compositionType = Hwc2::IComposerClient::Composition::DEVICE;
+ mOutputLayer.editState().hwc->stateOverridden = true;
+ mOutputLayer.editState().hwc->layerSkipped = true;
+ mOutputLayer.editState().hwc->hwcCompositionType = Hwc2::IComposerClient::Composition::DEVICE;
+
+ expectGeometryCommonCalls();
+ expectPerFrameCommonCalls(SimulateUnsupported::None, kDataspace, kOutputSpaceVisibleRegion,
+ Region::INVALID_REGION);
+ expectSetHdrMetadataAndBufferCalls();
+ expectSetCompositionTypeCall(Hwc2::IComposerClient::Composition::DEVICE);
EXPECT_CALL(*mLayerFE, hasRoundedCorners()).WillOnce(Return(false));
mOutputLayer.writeStateToHWC(/*includeGeometry*/ true, /*skipLayer*/ false, 0,
/*zIsOverridden*/ false, /*isPeekingThrough*/ false);
}
+TEST_F(OutputLayerWriteStateToHWCTest, previousSkipLayerSendsUpdatedClientCompositionInfo) {
+ mLayerFEState.compositionType = Hwc2::IComposerClient::Composition::DEVICE;
+ mOutputLayer.editState().forceClientComposition = true;
+ mOutputLayer.editState().hwc->stateOverridden = true;
+ mOutputLayer.editState().hwc->layerSkipped = true;
+ mOutputLayer.editState().hwc->hwcCompositionType = Hwc2::IComposerClient::Composition::CLIENT;
+
+ expectGeometryCommonCalls();
+ expectPerFrameCommonCalls(SimulateUnsupported::None, kDataspace, kOutputSpaceVisibleRegion,
+ Region::INVALID_REGION);
+ expectSetHdrMetadataAndBufferCalls();
+ expectSetCompositionTypeCall(Hwc2::IComposerClient::Composition::CLIENT);
+ EXPECT_CALL(*mLayerFE, hasRoundedCorners()).WillRepeatedly(Return(false));
+
+ mOutputLayer.writeStateToHWC(/*includeGeometry*/ true, /*skipLayer*/ false, 0,
+ /*zIsOverridden*/ false, /*isPeekingThrough*/ false);
+}
+
TEST_F(OutputLayerWriteStateToHWCTest, peekThroughChangesBlendMode) {
auto peekThroughLayerFE = sp<compositionengine::mock::LayerFE>::make();
OutputLayer peekThroughLayer{mOutput, peekThroughLayerFE};