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

Commit 2f5addbd authored by Alec Mouri's avatar Alec Mouri
Browse files

Improve Planner stability 

This fixes two crashes in Planner:

1. During device rotation, an Output's layer stack may change, which
invalidates OutputLayer pointers. When updating a LayerState, swap in the
updated OutputLayer pointer as well to keep references from going stale.
2. Retain the set of removedLayers until after Flattener has updated its
internal state in flattenLayers(). Otherwise, removed LayerStates may be
deleted which invalidates dangling pointers in the Flattener.

Bug: 188743022
Test: builds
Test: libcompositionengine_test
Test: rotate Chrome

Change-Id: I889703a2675bc0eddadd344f1a550d93d7f4c8df
parent 95c0a889

services/surfaceflinger/CompositionEngine/src/planner/LayerState.cpp

+7 −1
Original line number Diff line number Diff line
@@ -42,7 +42,13 @@ LayerState::LayerState(compositionengine::OutputLayer* layer) 
}



Flags<LayerStateField> LayerState::update(compositionengine::OutputLayer* layer) {

    ALOGE_IF(layer != mOutputLayer, "[%s] Expected mOutputLayer to never change", __func__);

    ALOGE_IF(mOutputLayer != layer && layer->getLayerFE().getSequence() != mId.get(),

             "[%s] Expected mOutputLayer ID to never change: %d, %d", __func__,

             layer->getLayerFE().getSequence(), mId.get());



    // It's possible for the OutputLayer pointer to change even when the layer is logically the

    // same, i.e., the LayerFE is the same. An example use-case is screen rotation.

    mOutputLayer = layer;



    Flags<LayerStateField> differences;

services/surfaceflinger/CompositionEngine/src/planner/Planner.cpp

+12 −9
Original line number Diff line number Diff line
@@ -113,15 +113,6 @@ void Planner::plan( 
        }

    }



    for (LayerId removedLayer : removedLayers) {

        if (const auto layerEntry = mPreviousLayers.find(removedLayer);

            layerEntry != mPreviousLayers.end()) {

            const auto& [id, state] = *layerEntry;

            ALOGV("Removed layer %s", state.getName().c_str());

            mPreviousLayers.erase(removedLayer);

        }

    }



    mCurrentLayers.clear();

    mCurrentLayers.reserve(currentLayerIds.size());

    std::transform(currentLayerIds.cbegin(), currentLayerIds.cend(),
@@ -135,6 +126,7 @@ void Planner::plan( 
    mFlattenedHash =

            mFlattener.flattenLayers(mCurrentLayers, hash, std::chrono::steady_clock::now());

    const bool layersWereFlattened = hash != mFlattenedHash;



    ALOGV("[%s] Initial hash %zx flattened hash %zx", __func__, hash, mFlattenedHash);



    if (mPredictorEnabled) {
@@ -148,6 +140,17 @@ void Planner::plan( 
            ALOGV("[%s] No prediction found\n", __func__);

        }

    }



    // Clean up the set of previous layers now that the view of the LayerStates in the flattener are

    // up-to-date.

    for (LayerId removedLayer : removedLayers) {

        if (const auto layerEntry = mPreviousLayers.find(removedLayer);

            layerEntry != mPreviousLayers.end()) {

            const auto& [id, state] = *layerEntry;

            ALOGV("Removed layer %s", state.getName().c_str());

            mPreviousLayers.erase(removedLayer);

        }

    }

}



void Planner::reportFinalPlan(

services/surfaceflinger/CompositionEngine/tests/planner/LayerStateTest.cpp

+16 −0
Original line number Diff line number Diff line
@@ -117,6 +117,22 @@ TEST_F(LayerStateTest, getOutputLayer) { 
    EXPECT_EQ(&mOutputLayer, mLayerState->getOutputLayer());

}



TEST_F(LayerStateTest, updateOutputLayer) {

    OutputLayerCompositionState outputLayerCompositionState;

    LayerFECompositionState layerFECompositionState;

    setupMocksForLayer(mOutputLayer, mLayerFE, outputLayerCompositionState,

                       layerFECompositionState);

    mLayerState = std::make_unique<LayerState>(&mOutputLayer);

    EXPECT_EQ(&mOutputLayer, mLayerState->getOutputLayer());



    mock::OutputLayer newOutputLayer;

    mock::LayerFE newLayerFE;

    setupMocksForLayer(newOutputLayer, newLayerFE, outputLayerCompositionState,

                       layerFECompositionState);

    mLayerState->update(&newOutputLayer);

    EXPECT_EQ(&newOutputLayer, mLayerState->getOutputLayer());

}



TEST_F(LayerStateTest, getId) {

    OutputLayerCompositionState outputLayerCompositionState;

    LayerFECompositionState layerFECompositionState;