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
diff --git a/libs/ui/include/ui/LayerStack.h b/libs/ui/include/ui/LayerStack.h
index d6ffeb7..f4c8ba2 100644
--- a/libs/ui/include/ui/LayerStack.h
+++ b/libs/ui/include/ui/LayerStack.h
@@ -55,6 +55,10 @@
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;
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index cd2120e..edd9f9c 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -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 @@
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 @@
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() &&
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index d283d6f..fc39660 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -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 @@
// 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;