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

Commit a0292284 authored by Vishnu Nair's avatar Vishnu Nair
Browse files

Detect and recover from relative z loops 

The caller can create loops in the hierarchy by relatively
reparenting layers to each other. This can be done directly
or via a chain of layers.

The logic to detect and fix this was not being called. Fix this
by making loop detection a bit harder to ignore and part of
hierarchy builder's update call.

Test: presubmit
Bug: 316236833
Change-Id: I484f9b8e2742fef22a5d76362a715eb6850c26f6
parent 8597755c

services/surfaceflinger/FrontEnd/LayerHierarchy.cpp

+34 −3
Original line number Diff line number Diff line
@@ -190,8 +190,12 @@ bool LayerHierarchy::hasRelZLoop(uint32_t& outInvalidRelativeRoot) const { 
    return outInvalidRelativeRoot != UNASSIGNED_LAYER_ID;

}



LayerHierarchyBuilder::LayerHierarchyBuilder(

        const std::vector<std::unique_ptr<RequestedLayerState>>& layers) {

void LayerHierarchyBuilder::init(const std::vector<std::unique_ptr<RequestedLayerState>>& layers) {

    mLayerIdToHierarchy.clear();

    mHierarchies.clear();

    mRoot = nullptr;

    mOffscreenRoot = nullptr;



    mHierarchies.reserve(layers.size());

    mLayerIdToHierarchy.reserve(layers.size());

    for (auto& layer : layers) {
@@ -202,6 +206,7 @@ LayerHierarchyBuilder::LayerHierarchyBuilder( 
        onLayerAdded(layer.get());

    }

    detachHierarchyFromRelativeParent(&mOffscreenRoot);

    mInitialized = true;

}



void LayerHierarchyBuilder::attachToParent(LayerHierarchy* hierarchy) {
@@ -332,7 +337,7 @@ void LayerHierarchyBuilder::updateMirrorLayer(RequestedLayerState* layer) { 
    }

}



void LayerHierarchyBuilder::update(

void LayerHierarchyBuilder::doUpdate(

        const std::vector<std::unique_ptr<RequestedLayerState>>& layers,

        const std::vector<std::unique_ptr<RequestedLayerState>>& destroyedLayers) {

    // rebuild map
@@ -381,6 +386,32 @@ void LayerHierarchyBuilder::update( 
    attachHierarchyToRelativeParent(&mRoot);

}



void LayerHierarchyBuilder::update(LayerLifecycleManager& layerLifecycleManager) {

    if (!mInitialized) {

        ATRACE_NAME("LayerHierarchyBuilder:init");

        init(layerLifecycleManager.getLayers());

    } else if (layerLifecycleManager.getGlobalChanges().test(

                       RequestedLayerState::Changes::Hierarchy)) {

        ATRACE_NAME("LayerHierarchyBuilder:update");

        doUpdate(layerLifecycleManager.getLayers(), layerLifecycleManager.getDestroyedLayers());

    } else {

        return; // nothing to do

    }



    uint32_t invalidRelativeRoot;

    bool hasRelZLoop = mRoot.hasRelZLoop(invalidRelativeRoot);

    while (hasRelZLoop) {

        ATRACE_NAME("FixRelZLoop");

        TransactionTraceWriter::getInstance().invoke("relz_loop_detected",

                                                     /*overwrite=*/false);

        layerLifecycleManager.fixRelativeZLoop(invalidRelativeRoot);

        // reinitialize the hierarchy with the updated layer data

        init(layerLifecycleManager.getLayers());

        // check if we have any remaining loops

        hasRelZLoop = mRoot.hasRelZLoop(invalidRelativeRoot);

    }

}



const LayerHierarchy& LayerHierarchyBuilder::getHierarchy() const {

    return mRoot;

}

services/surfaceflinger/FrontEnd/LayerHierarchy.h

+8 −4
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ 
#pragma once



#include "FrontEnd/LayerCreationArgs.h"

#include "FrontEnd/LayerLifecycleManager.h"

#include "RequestedLayerState.h"

#include "ftl/small_vector.h"
@@ -197,9 +198,8 @@ private: 
// hierarchy from a list of RequestedLayerState and associated change flags.

class LayerHierarchyBuilder {

public:

    LayerHierarchyBuilder(const std::vector<std::unique_ptr<RequestedLayerState>>&);

    void update(const std::vector<std::unique_ptr<RequestedLayerState>>& layers,

                const std::vector<std::unique_ptr<RequestedLayerState>>& destroyedLayers);

    LayerHierarchyBuilder() = default;

    void update(LayerLifecycleManager& layerLifecycleManager);

    LayerHierarchy getPartialHierarchy(uint32_t, bool childrenOnly) const;

    const LayerHierarchy& getHierarchy() const;

    const LayerHierarchy& getOffscreenHierarchy() const;
@@ -213,14 +213,18 @@ private: 
    void detachFromRelativeParent(LayerHierarchy*);

    void attachHierarchyToRelativeParent(LayerHierarchy*);

    void detachHierarchyFromRelativeParent(LayerHierarchy*);



    void init(const std::vector<std::unique_ptr<RequestedLayerState>>&);

    void doUpdate(const std::vector<std::unique_ptr<RequestedLayerState>>& layers,

                  const std::vector<std::unique_ptr<RequestedLayerState>>& destroyedLayers);

    void onLayerDestroyed(RequestedLayerState* layer);

    void updateMirrorLayer(RequestedLayerState* layer);

    LayerHierarchy* getHierarchyFromId(uint32_t layerId, bool crashOnFailure = true);



    std::unordered_map<uint32_t, LayerHierarchy*> mLayerIdToHierarchy;

    std::vector<std::unique_ptr<LayerHierarchy>> mHierarchies;

    LayerHierarchy mRoot{nullptr};

    LayerHierarchy mOffscreenRoot{nullptr};

    bool mInitialized = false;

};



} // namespace android::surfaceflinger::frontend

services/surfaceflinger/SurfaceFlinger.cpp

+1 −5
Original line number Diff line number Diff line
@@ -2327,11 +2327,7 @@ bool SurfaceFlinger::updateLayerSnapshots(VsyncId vsyncId, nsecs_t frameTimeNs, 
                mLegacyLayers[layer->sequence] = layer;

            }

        }

        if (mLayerLifecycleManager.getGlobalChanges().test(Changes::Hierarchy)) {

            ATRACE_NAME("LayerHierarchyBuilder:update");

            mLayerHierarchyBuilder.update(mLayerLifecycleManager.getLayers(),

                                          mLayerLifecycleManager.getDestroyedLayers());

        }

        mLayerHierarchyBuilder.update(mLayerLifecycleManager);

    }



    bool mustComposite = false;

services/surfaceflinger/SurfaceFlinger.h

+1 −1
Original line number Diff line number Diff line
@@ -1439,7 +1439,7 @@ private: 
    bool mLegacyFrontEndEnabled = true;



    frontend::LayerLifecycleManager mLayerLifecycleManager;

    frontend::LayerHierarchyBuilder mLayerHierarchyBuilder{{}};

    frontend::LayerHierarchyBuilder mLayerHierarchyBuilder;

    frontend::LayerSnapshotBuilder mLayerSnapshotBuilder;



    std::vector<std::pair<uint32_t, std::string>> mDestroyedHandles;

services/surfaceflinger/Tracing/tools/LayerTraceGenerator.cpp

+4 −6
Original line number Diff line number Diff line
@@ -52,7 +52,7 @@ bool LayerTraceGenerator::generate(const perfetto::protos::TransactionTraceFile& 


    // frontend

    frontend::LayerLifecycleManager lifecycleManager;

    frontend::LayerHierarchyBuilder hierarchyBuilder{{}};

    frontend::LayerHierarchyBuilder hierarchyBuilder;

    frontend::LayerSnapshotBuilder snapshotBuilder;

    ui::DisplayMap<ui::LayerStack, frontend::DisplayInfo> displayInfos;
@@ -119,12 +119,10 @@ bool LayerTraceGenerator::generate(const perfetto::protos::TransactionTraceFile& 
        lifecycleManager.applyTransactions(transactions, /*ignoreUnknownHandles=*/true);

        lifecycleManager.onHandlesDestroyed(destroyedHandles, /*ignoreUnknownHandles=*/true);



        if (lifecycleManager.getGlobalChanges().test(

                    frontend::RequestedLayerState::Changes::Hierarchy)) {

            hierarchyBuilder.update(lifecycleManager.getLayers(),

                                    lifecycleManager.getDestroyedLayers());

        }

        // update hierarchy

        hierarchyBuilder.update(lifecycleManager);



        // update snapshots

        frontend::LayerSnapshotBuilder::Args args{.root = hierarchyBuilder.getHierarchy(),

                                                  .layerLifecycleManager = lifecycleManager,

                                                  .displays = displayInfos,