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

Commit 3be61905 authored by Vishnu Nair's avatar Vishnu Nair
Browse files

[sf] Properly update clones of clones

Relanding with the change to only update onscreen mirror
roots and cleaning up some debug logs.

When updating a cloned layer hierarchy that contains
another cloned layer hierarchy, we are dependent on
the order in which we update each hierarchy.
The inner cloned hierarchy must be updated first
otherwise the outer clone will not capture the entire
tree and some layers might be omitted from
composition.
To fix this we track all layer mirror roots. When updating
each root, we check to see if they mirror another root. If
they do mirror another root and that root has not been updated,
we update the root again at the end.

Test: repro steps in bug
Fixes: 279622227
Change-Id: I125f7c788716d59909b77a88ef4098b7cac08919
parent 50fe0d96
Loading
Loading
Loading
Loading
+14 −6
Original line number Diff line number Diff line
@@ -254,7 +254,8 @@ Layer::~Layer() {
        mFlinger->mTunnelModeEnabledReporter->decrementTunnelModeCount();
    }
    if (mHadClonedChild) {
        mFlinger->mNumClones--;
        auto& roots = mFlinger->mLayerMirrorRoots;
        roots.erase(std::remove(roots.begin(), roots.end(), this), roots.end());
    }
    if (hasTrustedPresentationListener()) {
        mFlinger->mNumTrustedPresentationListeners--;
@@ -1649,8 +1650,8 @@ void Layer::getFrameStats(FrameStats* outStats) const {

void Layer::dumpOffscreenDebugInfo(std::string& result) const {
    std::string hasBuffer = hasBufferOrSidebandStream() ? " (contains buffer)" : "";
    StringAppendF(&result, "Layer %s%s pid:%d uid:%d\n", getName().c_str(), hasBuffer.c_str(),
                  mOwnerPid, mOwnerUid);
    StringAppendF(&result, "Layer %s%s pid:%d uid:%d%s\n", getName().c_str(), hasBuffer.c_str(),
                  mOwnerPid, mOwnerUid, isHandleAlive() ? " handleAlive" : "");
}

void Layer::onDisconnect() {
@@ -2594,7 +2595,7 @@ void Layer::updateCloneBufferInfo() {
    mDrawingState.inputInfo = tmpInputInfo;
}

void Layer::updateMirrorInfo() {
bool Layer::updateMirrorInfo(const std::deque<Layer*>& cloneRootsPendingUpdates) {
    if (mClonedChild == nullptr || !mClonedChild->isClonedFromAlive()) {
        // If mClonedChild is null, there is nothing to mirror. If isClonedFromAlive returns false,
        // it means that there is a clone, but the layer it was cloned from has been destroyed. In
@@ -2602,7 +2603,7 @@ void Layer::updateMirrorInfo() {
        // destroyed. The root, this layer, will still be around since the client can continue
        // to hold a reference, but no cloned layers will be displayed.
        mClonedChild = nullptr;
        return;
        return true;
    }

    std::map<sp<Layer>, sp<Layer>> clonedLayersMap;
@@ -2617,6 +2618,13 @@ void Layer::updateMirrorInfo() {
    mClonedChild->updateClonedDrawingState(clonedLayersMap);
    mClonedChild->updateClonedChildren(sp<Layer>::fromExisting(this), clonedLayersMap);
    mClonedChild->updateClonedRelatives(clonedLayersMap);

    for (Layer* root : cloneRootsPendingUpdates) {
        if (clonedLayersMap.find(sp<Layer>::fromExisting(root)) != clonedLayersMap.end()) {
            return false;
        }
    }
    return true;
}

void Layer::updateClonedDrawingState(std::map<sp<Layer>, sp<Layer>>& clonedLayersMap) {
@@ -2764,7 +2772,7 @@ bool Layer::isInternalDisplayOverlay() const {
void Layer::setClonedChild(const sp<Layer>& clonedChild) {
    mClonedChild = clonedChild;
    mHadClonedChild = true;
    mFlinger->mNumClones++;
    mFlinger->mLayerMirrorRoots.push_back(this);
}

bool Layer::setDropInputMode(gui::DropInputMode mode) {
+1 −1
Original line number Diff line number Diff line
@@ -651,7 +651,7 @@ public:

    gui::WindowInfo::Type getWindowType() const { return mWindowType; }

    void updateMirrorInfo();
    bool updateMirrorInfo(const std::deque<Layer*>& cloneRootsPendingUpdates);

    /*
     * doTransaction - process the transaction. This is a good place to figure
+1 −1
Original line number Diff line number Diff line
@@ -85,7 +85,7 @@ void LayerRenderArea::render(std::function<void()> drawLayers) {
    // If layer is offscreen, update mirroring info if it exists
    if (mLayer->isRemovedFromCurrentState()) {
        mLayer->traverse(LayerVector::StateSet::Drawing,
                         [&](Layer* layer) { layer->updateMirrorInfo(); });
                         [&](Layer* layer) { layer->updateMirrorInfo({}); });
        mLayer->traverse(LayerVector::StateSet::Drawing,
                         [&](Layer* layer) { layer->updateCloneBufferInfo(); });
    }
+31 −8
Original line number Diff line number Diff line
@@ -4025,8 +4025,24 @@ void SurfaceFlinger::doCommitTransactions() {
    }

    commitOffscreenLayers();
    if (mNumClones > 0) {
        mDrawingState.traverse([&](Layer* layer) { layer->updateMirrorInfo(); });
    if (mLayerMirrorRoots.size() > 0) {
        std::deque<Layer*> pendingUpdates;
        pendingUpdates.insert(pendingUpdates.end(), mLayerMirrorRoots.begin(),
                              mLayerMirrorRoots.end());
        std::vector<Layer*> needsUpdating;
        for (Layer* cloneRoot : mLayerMirrorRoots) {
            pendingUpdates.pop_front();
            if (cloneRoot->isRemovedFromCurrentState()) {
                continue;
            }
            if (cloneRoot->updateMirrorInfo(pendingUpdates)) {
            } else {
                needsUpdating.push_back(cloneRoot);
            }
        }
        for (Layer* cloneRoot : needsUpdating) {
            cloneRoot->updateMirrorInfo({});
        }
    }
}

@@ -4133,7 +4149,7 @@ bool SurfaceFlinger::latchBuffers() {
        mBootStage = BootStage::BOOTANIMATION;
    }

    if (mNumClones > 0) {
    if (mLayerMirrorRoots.size() > 0) {
        mDrawingState.traverse([&](Layer* layer) { layer->updateCloneBufferInfo(); });
    }

@@ -4158,8 +4174,8 @@ status_t SurfaceFlinger::addClientLayer(LayerCreationArgs& args, const sp<IBinde
                    leakingParentLayerFound = true;
                    sp<Layer> parent = sp<Layer>::fromExisting(layer);
                    while (parent) {
                        ALOGE("Parent Layer: %s handleIsAlive: %s", parent->getName().c_str(),
                              std::to_string(parent->isHandleAlive()).c_str());
                        ALOGE("Parent Layer: %s%s", parent->getName().c_str(),
                              (parent->isHandleAlive() ? "handleAlive" : ""));
                        parent = parent->getParent();
                    }
                    // Sample up to 100 layers
@@ -4174,21 +4190,28 @@ status_t SurfaceFlinger::addClientLayer(LayerCreationArgs& args, const sp<IBinde
                }
            });

            ALOGE("Dumping random sampling of on-screen layers: ");
            int numLayers = 0;
            mDrawingState.traverse([&](Layer* layer) { numLayers++; });

            ALOGE("Dumping random sampling of on-screen layers total(%u):", numLayers);
            mDrawingState.traverse([&](Layer* layer) {
                // Aim to dump about 200 layers to avoid totally trashing
                // logcat. On the other hand, if there really are 4096 layers
                // something has gone totally wrong its probably the most
                // useful information in logcat.
                if (rand() % 20 == 13) {
                    ALOGE("Layer: %s", layer->getName().c_str());
                    ALOGE("Layer: %s%s", layer->getName().c_str(),
                          (layer->isHandleAlive() ? "handleAlive" : ""));
                    std::this_thread::sleep_for(std::chrono::milliseconds(5));
                }
            });
            ALOGE("Dumping random sampling of off-screen layers total(%zu): ",
                  mOffscreenLayers.size());
            for (Layer* offscreenLayer : mOffscreenLayers) {
                if (rand() % 20 == 13) {
                    ALOGE("Offscreen-layer: %s", offscreenLayer->getName().c_str());
                    ALOGE("Offscreen-layer: %s%s", offscreenLayer->getName().c_str(),
                          (offscreenLayer->isHandleAlive() ? "handleAlive" : ""));
                    std::this_thread::sleep_for(std::chrono::milliseconds(5));
                }
            }
        }));
+1 −2
Original line number Diff line number Diff line
@@ -296,8 +296,7 @@ public:
    // the client can no longer modify this layer directly.
    void onHandleDestroyed(BBinder* handle, sp<Layer>& layer, uint32_t layerId);

    // TODO: Remove atomic if move dtor to main thread CL lands
    std::atomic<uint32_t> mNumClones;
    std::vector<Layer*> mLayerMirrorRoots;

    TransactionCallbackInvoker& getTransactionCallbackInvoker() {
        return mTransactionCallbackInvoker;