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

Commit e4ef87f0 authored by Melody Hsu's avatar Melody Hsu
Browse files

Track the last release fence per layer stack

Merging fences prior to calling CompositionEngine#present results in
occassional missed frames in Surfaceflinger. Merging fences here was
originally needed to relieve memory pressure. Instead, we can track
each layer stack's future release fence with a map between a layer
stack and its last fence, as it can be assumed that multiple fences
for the same layer stack can be be dropped.

Fixes: b/330841053
Test: SurfaceFlinger_test
Test: presubmit
Change-Id: I7ca3a226ff77bc31d93fdb4708c3e9089f423803
parent 3fa9d850
Loading
Loading
Loading
Loading
+4 −0
Original line number Diff line number Diff line
@@ -55,6 +55,10 @@ inline bool operator>(LayerStack lhs, LayerStack rhs) {
    return lhs.id > rhs.id;
}

inline bool operator<(LayerStack lhs, LayerStack rhs) {
    return lhs.id < rhs.id;
}

// A LayerFilter determines if a layer is included for output to a display.
struct LayerFilter {
    LayerStack layerStack;
+13 −26
Original line number Diff line number Diff line
@@ -15,7 +15,7 @@
 */

// TODO(b/129481165): remove the #pragma below and fix conversion issues
#include "TransactionCallbackInvoker.h"

#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wconversion"

@@ -24,8 +24,6 @@
#define LOG_TAG "Layer"
#define ATRACE_TAG ATRACE_TAG_GRAPHICS

#include "Layer.h"

#include <android-base/properties.h>
#include <android-base/stringprintf.h>
#include <binder/IPCThreadState.h>
@@ -74,10 +72,12 @@
#include "FrameTracer/FrameTracer.h"
#include "FrontEnd/LayerCreationArgs.h"
#include "FrontEnd/LayerHandle.h"
#include "Layer.h"
#include "LayerProtoHelper.h"
#include "MutexUtils.h"
#include "SurfaceFlinger.h"
#include "TimeStats/TimeStats.h"
#include "TransactionCallbackInvoker.h"
#include "TunnelModeEnabledReporter.h"
#include "Utils/FenceUtils.h"

@@ -2937,26 +2937,13 @@ void Layer::prepareReleaseCallbacks(ftl::Future<FenceResult> futureFenceResult,
        ch->previousReleaseFences.emplace_back(std::move(futureFenceResult));
        ch->name = mName;
    } else {
        // If we didn't get a release callback yet, e.g. some scenarios when capturing
        // screenshots asynchronously, then make sure we don't drop the fence.
        mAdditionalPreviousReleaseFences.emplace_back(std::move(futureFenceResult));
        std::vector<ftl::Future<FenceResult>> mergedFences;
        sp<Fence> prevFence = nullptr;
        // For a layer that's frequently screenshotted, try to merge fences to make sure we
        // don't grow unbounded.
        for (auto& futureReleaseFence : mAdditionalPreviousReleaseFences) {
            auto result = futureReleaseFence.wait_for(0s);
            if (result != std::future_status::ready) {
                mergedFences.emplace_back(std::move(futureReleaseFence));
                continue;
            }
            mergeFence(getDebugName(), futureReleaseFence.get().value_or(Fence::NO_FENCE),
                       prevFence);
        }
        if (prevFence != nullptr) {
            mergedFences.emplace_back(ftl::yield(FenceResult(std::move(prevFence))));
        }
        mAdditionalPreviousReleaseFences.swap(mergedFences);
        // If we didn't get a release callback yet (e.g. some scenarios when capturing
        // screenshots asynchronously) then make sure we don't drop the fence.
        // Older fences for the same layer stack can be dropped when a new fence arrives.
        // An assumption here is that RenderEngine performs work sequentially, so an
        // incoming fence will not fire before an existing fence.
        mAdditionalPreviousReleaseFences.emplace_or_replace(layerStack,
                                                            std::move(futureFenceResult));
    }

    if (mBufferInfo.mBuffer) {
@@ -3506,10 +3493,10 @@ bool Layer::setTransactionCompletedListeners(const std::vector<sp<CallbackHandle
            handle->previousFrameNumber = mDrawingState.previousFrameNumber;
            if (FlagManager::getInstance().ce_fence_promise() &&
                mPreviousReleaseBufferEndpoint == handle->listener) {
                // Add fences from previous screenshots now so that they can be dispatched to the
                // Add fence from previous screenshot now so that it can be dispatched to the
                // client.
                for (auto& futureReleaseFence : mAdditionalPreviousReleaseFences) {
                    handle->previousReleaseFences.emplace_back(std::move(futureReleaseFence));
                for (auto& [_, future] : mAdditionalPreviousReleaseFences) {
                    handle->previousReleaseFences.emplace_back(std::move(future));
                }
                mAdditionalPreviousReleaseFences.clear();
            } else if (FlagManager::getInstance().screenshot_fence_preservation() &&
+8 −2
Original line number Diff line number Diff line
@@ -18,6 +18,7 @@

#include <android/gui/DropInputMode.h>
#include <android/gui/ISurfaceComposerClient.h>
#include <ftl/small_map.h>
#include <gui/BufferQueue.h>
#include <gui/LayerState.h>
#include <gui/WindowInfo.h>
@@ -25,9 +26,11 @@
#include <math/vec4.h>
#include <sys/types.h>
#include <ui/BlurRegion.h>
#include <ui/DisplayMap.h>
#include <ui/FloatRect.h>
#include <ui/FrameStats.h>
#include <ui/GraphicBuffer.h>
#include <ui/LayerStack.h>
#include <ui/PixelFormat.h>
#include <ui/Region.h>
#include <ui/StretchEffect.h>
@@ -958,8 +961,11 @@ public:
    // screenshots asynchronously. There may be no buffer update for the
    // layer, but the layer will still be composited on the screen in every
    // frame. Kepping track of these fences ensures that they are not dropped
    // and can be dispatched to the client at a later time.
    std::vector<ftl::Future<FenceResult>> mAdditionalPreviousReleaseFences;
    // and can be dispatched to the client at a later time. Older fences are
    // dropped when a layer stack receives a new fence.
    // TODO(b/300533018): Track fence per multi-instance RenderEngine
    ftl::SmallMap<ui::LayerStack, ftl::Future<FenceResult>, ui::kDisplayCapacity>
            mAdditionalPreviousReleaseFences;

    // Exposed so SurfaceFlinger can assert that it's held
    const sp<SurfaceFlinger> mFlinger;