Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d1bf1b52 authored by Alec Mouri's avatar Alec Mouri
Browse files

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
parent 023c188f

services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayer.h

+2 −1
Original line number Original line Diff line number Diff line
@@ -78,9 +78,10 @@ private: 
    void writeSidebandStateToHWC(HWC2::Layer*, const LayerFECompositionState&);

    void writeSidebandStateToHWC(HWC2::Layer*, const LayerFECompositionState&);

    void writeBufferStateToHWC(HWC2::Layer*, const LayerFECompositionState&);

    void writeBufferStateToHWC(HWC2::Layer*, const LayerFECompositionState&);

    void writeCompositionTypeToHWC(HWC2::Layer*, Hwc2::IComposerClient::Composition,

    void writeCompositionTypeToHWC(HWC2::Layer*, Hwc2::IComposerClient::Composition,

                                   bool isPeekingThrough);

                                   bool isPeekingThrough, bool skipLayer);

    void detectDisallowedCompositionTypeChange(Hwc2::IComposerClient::Composition from,

    void detectDisallowedCompositionTypeChange(Hwc2::IComposerClient::Composition from,

                                               Hwc2::IComposerClient::Composition to) const;

                                               Hwc2::IComposerClient::Composition to) const;

    bool isClientCompositionForced(bool isPeekingThrough) const;

};

};





// This template factory function standardizes the implementation details of the

// This template factory function standardizes the implementation details of the

services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayerCompositionState.h

+3 −0
Original line number Original line Diff line number Diff line
@@ -125,6 +125,9 @@ struct OutputLayerCompositionState { 




        // Set to true when overridden info has been sent to HW composer

        // Set to true when overridden info has been sent to HW composer

        bool stateOverridden = false;

        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

    // The HWC state is optional, and is only set up if there is any potential

services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp

+24 −8
Original line number Original line Diff line number Diff line
@@ -14,6 +14,7 @@ 
 * limitations under the License.

 * limitations under the License.

 */

 */





#include <DisplayHardware/Hal.h>

#include <android-base/stringprintf.h>

#include <android-base/stringprintf.h>

#include <compositionengine/DisplayColorProfile.h>

#include <compositionengine/DisplayColorProfile.h>

#include <compositionengine/LayerFECompositionState.h>

#include <compositionengine/LayerFECompositionState.h>
@@ -350,12 +351,14 @@ void OutputLayer::writeStateToHWC(bool includeGeometry, bool skipLayer, uint32_t 
    writeOutputDependentPerFrameStateToHWC(hwcLayer.get());

    writeOutputDependentPerFrameStateToHWC(hwcLayer.get());

    writeOutputIndependentPerFrameStateToHWC(hwcLayer.get(), *outputIndependentState);

    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.

    // Always set the layer color after setting the composition type.

    writeSolidColorStateToHWC(hwcLayer.get(), *outputIndependentState);

    writeSolidColorStateToHWC(hwcLayer.get(), *outputIndependentState);





    editState().hwc->stateOverridden = isOverridden;

    editState().hwc->stateOverridden = isOverridden;

    editState().hwc->layerSkipped = skipLayer;

}

}





void OutputLayer::writeOutputDependentGeometryStateToHWC(HWC2::Layer* hwcLayer,

void OutputLayer::writeOutputDependentGeometryStateToHWC(HWC2::Layer* hwcLayer,
@@ -482,7 +485,8 @@ void OutputLayer::writeOutputIndependentPerFrameStateToHWC( 




    const Region& surfaceDamage = getState().overrideInfo.buffer

    const Region& surfaceDamage = getState().overrideInfo.buffer

            ? getState().overrideInfo.damageRegion

            ? getState().overrideInfo.damageRegion

            : outputIndependentState.surfaceDamage;

            : (getState().hwc->stateOverridden ? Region::INVALID_REGION

                                               : outputIndependentState.surfaceDamage);





    if (auto error = hwcLayer->setSurfaceDamage(surfaceDamage); error != hal::Error::NONE) {

    if (auto error = hwcLayer->setSurfaceDamage(surfaceDamage); error != hal::Error::NONE) {

        ALOGE("[%s] Failed to set surface damage: %s (%d)", getLayerFE().getDebugName(),

        ALOGE("[%s] Failed to set surface damage: %s (%d)", getLayerFE().getDebugName(),
@@ -573,17 +577,19 @@ void OutputLayer::writeBufferStateToHWC(HWC2::Layer* hwcLayer, 




void OutputLayer::writeCompositionTypeToHWC(HWC2::Layer* hwcLayer,

void OutputLayer::writeCompositionTypeToHWC(HWC2::Layer* hwcLayer,

                                            hal::Composition requestedCompositionType,

                                            hal::Composition requestedCompositionType,

                                            bool isPeekingThrough) {

                                            bool isPeekingThrough, bool skipLayer) {

    auto& outputDependentState = editState();

    auto& outputDependentState = editState();





    if (isClientCompositionForced(isPeekingThrough)) {

        // If we are forcing client composition, we need to tell the HWC

        // If we are forcing client composition, we need to tell the HWC

    if (outputDependentState.forceClientComposition ||

        (!isPeekingThrough && getLayerFE().hasRoundedCorners())) {

        requestedCompositionType = hal::Composition::CLIENT;

        requestedCompositionType = hal::Composition::CLIENT;

    }

    }





    // Set the requested composition type with the HWC whenever it changes

    // 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;

        outputDependentState.hwc->hwcCompositionType = requestedCompositionType;





        if (auto error = hwcLayer->setCompositionType(requestedCompositionType);

        if (auto error = hwcLayer->setCompositionType(requestedCompositionType);
@@ -663,12 +669,22 @@ void OutputLayer::detectDisallowedCompositionTypeChange(hal::Composition from, 
    }

    }

}

}





bool OutputLayer::isClientCompositionForced(bool isPeekingThrough) const {

    return getState().forceClientComposition ||

            (!isPeekingThrough && getLayerFE().hasRoundedCorners());

}



void OutputLayer::applyDeviceCompositionTypeChange(hal::Composition compositionType) {

void OutputLayer::applyDeviceCompositionTypeChange(hal::Composition compositionType) {

    auto& state = editState();

    auto& state = editState();

    LOG_FATAL_IF(!state.hwc);

    LOG_FATAL_IF(!state.hwc);

    auto& hwcState = *state.hwc;

    auto& hwcState = *state.hwc;





    // 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);

        detectDisallowedCompositionTypeChange(hwcState.hwcCompositionType, compositionType);

    }





    hwcState.hwcCompositionType = compositionType;

    hwcState.hwcCompositionType = compositionType;

}

}

services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp

+50 −0
Original line number Original line Diff line number Diff line
@@ -1065,12 +1065,62 @@ TEST_F(OutputLayerWriteStateToHWCTest, includesOverrideInfoIfPresent) { 
                              kOverrideSurfaceDamage);

                              kOverrideSurfaceDamage);

    expectSetHdrMetadataAndBufferCalls(kOverrideHwcSlot, kOverrideBuffer, kOverrideFence);

    expectSetHdrMetadataAndBufferCalls(kOverrideHwcSlot, kOverrideBuffer, kOverrideFence);

    expectSetCompositionTypeCall(Hwc2::IComposerClient::Composition::DEVICE);

    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));

    EXPECT_CALL(*mLayerFE, hasRoundedCorners()).WillOnce(Return(false));





    mOutputLayer.writeStateToHWC(/*includeGeometry*/ true, /*skipLayer*/ false, 0,

    mOutputLayer.writeStateToHWC(/*includeGeometry*/ true, /*skipLayer*/ false, 0,

                                 /*zIsOverridden*/ false, /*isPeekingThrough*/ false);

                                 /*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) {

TEST_F(OutputLayerWriteStateToHWCTest, peekThroughChangesBlendMode) {

    auto peekThroughLayerFE = sp<compositionengine::mock::LayerFE>::make();

    auto peekThroughLayerFE = sp<compositionengine::mock::LayerFE>::make();

    OutputLayer peekThroughLayer{mOutput, peekThroughLayerFE};

    OutputLayer peekThroughLayer{mOutput, peekThroughLayerFE};