Skip to content

Commit c6a0cc6

Browse files
author
Melody Hsu
committed
Rename invalid LayerStacks to "unassigned"
"Invalid" can be confusing since this LayerStack value is actually valid and can be used to represent offscreen layers. Bug: b/397775142 Test: atest SurfaceFlinger_test Flag: EXEMPT, renaming Change-Id: I1432107ae4a088aecc5baefad7db54e49cdc7bbf
1 parent 2dc2afe commit c6a0cc6

File tree

11 files changed

+35
-32
lines changed

11 files changed

+35
-32
lines changed

libs/ui/include/ui/LayerStack.h

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
namespace android::ui {
2626

2727
// A LayerStack identifies a Z-ordered group of layers. A layer can only be associated to a single
28-
// LayerStack, but a LayerStack can be associated to multiple displays, mirroring the same content.
28+
// LayerStack, and a LayerStack should be unique to each display in the composition target.
2929
struct LayerStack {
3030
uint32_t id = UINT32_MAX;
3131

@@ -40,7 +40,9 @@ struct LayerStack {
4040
}
4141
};
4242

43-
constexpr LayerStack INVALID_LAYER_STACK;
43+
// An unassigned LayerStack can indicate that a layer is offscreen and will not be
44+
// rendered onto a display. Multiple displays are allowed to have unassigned LayerStacks.
45+
constexpr LayerStack UNASSIGNED_LAYER_STACK;
4446
constexpr LayerStack DEFAULT_LAYER_STACK{0u};
4547

4648
inline bool operator==(LayerStack lhs, LayerStack rhs) {
@@ -70,7 +72,7 @@ struct LayerFilter {
7072
// Returns true if the input filter can be output to this filter.
7173
bool includes(LayerFilter other) const {
7274
// The layer stacks must match.
73-
if (other.layerStack == INVALID_LAYER_STACK || other.layerStack != layerStack) {
75+
if (other.layerStack == UNASSIGNED_LAYER_STACK || other.layerStack != layerStack) {
7476
return false;
7577
}
7678

services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@ TEST_F(DisplaySetConfigurationTest, configuresPhysicalDisplay) {
302302
EXPECT_FALSE(mDisplay->isValid());
303303

304304
const auto& filter = mDisplay->getState().layerFilter;
305-
EXPECT_EQ(ui::INVALID_LAYER_STACK, filter.layerStack);
305+
EXPECT_EQ(ui::UNASSIGNED_LAYER_STACK, filter.layerStack);
306306
EXPECT_FALSE(filter.toInternalDisplay);
307307
}
308308

@@ -322,7 +322,7 @@ TEST_F(DisplaySetConfigurationTest, configuresHalVirtualDisplay) {
322322
EXPECT_FALSE(mDisplay->isValid());
323323

324324
const auto& filter = mDisplay->getState().layerFilter;
325-
EXPECT_EQ(ui::INVALID_LAYER_STACK, filter.layerStack);
325+
EXPECT_EQ(ui::UNASSIGNED_LAYER_STACK, filter.layerStack);
326326
EXPECT_FALSE(filter.toInternalDisplay);
327327
}
328328

@@ -342,7 +342,7 @@ TEST_F(DisplaySetConfigurationTest, configuresGpuVirtualDisplay) {
342342
EXPECT_FALSE(mDisplay->isValid());
343343

344344
const auto& filter = mDisplay->getState().layerFilter;
345-
EXPECT_EQ(ui::INVALID_LAYER_STACK, filter.layerStack);
345+
EXPECT_EQ(ui::UNASSIGNED_LAYER_STACK, filter.layerStack);
346346
EXPECT_FALSE(filter.toInternalDisplay);
347347
}
348348

services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -646,8 +646,8 @@ TEST_F(OutputTest, layerFiltering) {
646646
mOutput->setLayerFilter({layerStack1, true});
647647

648648
// It excludes layers with no layer stack, internal-only or not.
649-
EXPECT_FALSE(mOutput->includesLayer({ui::INVALID_LAYER_STACK, false}));
650-
EXPECT_FALSE(mOutput->includesLayer({ui::INVALID_LAYER_STACK, true}));
649+
EXPECT_FALSE(mOutput->includesLayer({ui::UNASSIGNED_LAYER_STACK, false}));
650+
EXPECT_FALSE(mOutput->includesLayer({ui::UNASSIGNED_LAYER_STACK, true}));
651651

652652
// It includes layers on layerStack1, internal-only or not.
653653
EXPECT_TRUE(mOutput->includesLayer({layerStack1, false}));
@@ -685,10 +685,10 @@ TEST_F(OutputTest, layerFilteringWithCompositionState) {
685685
mOutput->setLayerFilter({layerStack1, true});
686686

687687
// It excludes layers with no layer stack, internal-only or not.
688-
layer.layerFEState.outputFilter = {ui::INVALID_LAYER_STACK, false};
688+
layer.layerFEState.outputFilter = {ui::UNASSIGNED_LAYER_STACK, false};
689689
EXPECT_FALSE(mOutput->includesLayer(layerFE));
690690

691-
layer.layerFEState.outputFilter = {ui::INVALID_LAYER_STACK, true};
691+
layer.layerFEState.outputFilter = {ui::UNASSIGNED_LAYER_STACK, true};
692692
EXPECT_FALSE(mOutput->includesLayer(layerFE));
693693

694694
// It includes layers on layerStack1, internal-only or not.

services/surfaceflinger/FrontEnd/LayerCreationArgs.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ std::string LayerCreationArgs::getDebugString() const {
8686
if (layerIdToMirror != UNASSIGNED_LAYER_ID) {
8787
stream << " layerIdToMirror=" << layerIdToMirror;
8888
}
89-
if (layerStackToMirror != ui::INVALID_LAYER_STACK) {
89+
if (layerStackToMirror != ui::UNASSIGNED_LAYER_STACK) {
9090
stream << " layerStackToMirror=" << layerStackToMirror.id;
9191
}
9292
return stream.str();

services/surfaceflinger/FrontEnd/LayerCreationArgs.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ struct LayerCreationArgs {
5858
bool addToRoot = true;
5959
wp<IBinder> parentHandle = nullptr;
6060
wp<IBinder> mirrorLayerHandle = nullptr;
61-
ui::LayerStack layerStackToMirror = ui::INVALID_LAYER_STACK;
61+
ui::LayerStack layerStackToMirror = ui::UNASSIGNED_LAYER_STACK;
6262
uint32_t parentId = UNASSIGNED_LAYER_ID;
6363
uint32_t layerIdToMirror = UNASSIGNED_LAYER_ID;
6464
std::atomic<int32_t>* pendingBuffers = 0;

services/surfaceflinger/FrontEnd/LayerLifecycleManager.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,10 @@ void LayerLifecycleManager::addLayers(std::vector<std::unique_ptr<RequestedLayer
5555
mChangedLayers.push_back(newLayer.get());
5656
layer.parentId = linkLayer(layer.parentId, layer.id);
5757
layer.relativeParentId = linkLayer(layer.relativeParentId, layer.id);
58-
if (layer.layerStackToMirror != ui::INVALID_LAYER_STACK) {
58+
if (layer.layerStackToMirror != ui::UNASSIGNED_LAYER_STACK) {
5959
// Set mirror layer's default layer stack to -1 so it doesn't end up rendered on a
6060
// display accidentally.
61-
layer.layerStack = ui::INVALID_LAYER_STACK;
61+
layer.layerStack = ui::UNASSIGNED_LAYER_STACK;
6262

6363
// if this layer is mirroring a display, then walk though all the existing root layers
6464
// for the layer stack and add them as children to be mirrored.
@@ -124,7 +124,7 @@ void LayerLifecycleManager::onHandlesDestroyed(
124124

125125
layer.parentId = unlinkLayer(layer.parentId, layer.id);
126126
layer.relativeParentId = unlinkLayer(layer.relativeParentId, layer.id);
127-
if (layer.layerStackToMirror != ui::INVALID_LAYER_STACK) {
127+
if (layer.layerStackToMirror != ui::UNASSIGNED_LAYER_STACK) {
128128
layer.mirrorIds = unlinkLayers(layer.mirrorIds, layer.id);
129129
swapErase(mDisplayMirroringLayers, layer.id);
130130
} else {

services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -784,7 +784,7 @@ void LayerSnapshotBuilder::updateSnapshot(LayerSnapshot& snapshot, const Args& a
784784
// Display mirrors are always placed in a VirtualDisplay so we never want to capture layers
785785
// marked as skip capture
786786
snapshot.handleSkipScreenshotFlag = parentSnapshot.handleSkipScreenshotFlag ||
787-
(requested.layerStackToMirror != ui::INVALID_LAYER_STACK);
787+
(requested.layerStackToMirror != ui::UNASSIGNED_LAYER_STACK);
788788
}
789789

790790
if (forceUpdate || snapshot.clientChanges & layer_state_t::eAlphaChanged) {

services/surfaceflinger/FrontEnd/RequestedLayerState.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ RequestedLayerState::RequestedLayerState(const LayerCreationArgs& args)
7373
}
7474
if (layerIdToMirror != UNASSIGNED_LAYER_ID) {
7575
changes |= RequestedLayerState::Changes::Mirror;
76-
} else if (args.layerStackToMirror != ui::INVALID_LAYER_STACK) {
76+
} else if (args.layerStackToMirror != ui::UNASSIGNED_LAYER_STACK) {
7777
layerStackToMirror = args.layerStackToMirror;
7878
changes |= RequestedLayerState::Changes::Mirror;
7979
}

services/surfaceflinger/FrontEnd/RequestedLayerState.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ struct RequestedLayerState : layer_state_t {
128128
uint32_t parentId = UNASSIGNED_LAYER_ID;
129129
uint32_t relativeParentId = UNASSIGNED_LAYER_ID;
130130
uint32_t layerIdToMirror = UNASSIGNED_LAYER_ID;
131-
ui::LayerStack layerStackToMirror = ui::INVALID_LAYER_STACK;
131+
ui::LayerStack layerStackToMirror = ui::UNASSIGNED_LAYER_STACK;
132132
uint32_t touchCropId = UNASSIGNED_LAYER_ID;
133133
uint32_t bgColorLayerId = UNASSIGNED_LAYER_ID;
134134
uint64_t barrierFrameNumber = 0;

services/surfaceflinger/SurfaceFlinger.cpp

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2887,7 +2887,8 @@ CompositeResultsPerDisplay SurfaceFlinger::composite(
28872887
ui::DisplayMap<ui::LayerStack, ftl::Unit> outputLayerStacks;
28882888
auto isUniqueOutputLayerStack = [&outputLayerStacks](DisplayId id, ui::LayerStack layerStack) {
28892889
if (FlagManager::getInstance().reject_dupe_layerstacks()) {
2890-
if (layerStack != ui::INVALID_LAYER_STACK && outputLayerStacks.contains(layerStack)) {
2890+
if (layerStack != ui::UNASSIGNED_LAYER_STACK &&
2891+
outputLayerStacks.contains(layerStack)) {
28912892
// TODO: remove log and DisplayId from params once reject_dupe_layerstacks flag is
28922893
// removed
28932894
ALOGD("Existing layer stack ID %d output to another display %" PRIu64
@@ -3016,9 +3017,9 @@ CompositeResultsPerDisplay SurfaceFlinger::composite(
30163017
LayerFE::ReleaseFencePromiseStatus::UNINITIALIZED ||
30173018
layerFE->getReleaseFencePromiseStatus() ==
30183019
LayerFE::ReleaseFencePromiseStatus::FULFILLED) {
3019-
// layerStack is invalid because layer is not on a display
3020+
// layerStack is unassigned because layer is not on a display
30203021
attachReleaseFenceFutureToLayer(layer.get(), layerFE.get(),
3021-
ui::INVALID_LAYER_STACK);
3022+
ui::UNASSIGNED_LAYER_STACK);
30223023
}
30233024
}
30243025
}
@@ -3035,13 +3036,13 @@ CompositeResultsPerDisplay SurfaceFlinger::composite(
30353036

30363037
int index = 0;
30373038
ftl::StaticVector<char, WorkloadTracer::COMPOSITION_SUMMARY_SIZE> compositionSummary;
3038-
auto lastLayerStack = ui::INVALID_LAYER_STACK;
3039+
auto lastLayerStack = ui::UNASSIGNED_LAYER_STACK;
30393040

30403041
uint64_t prevOverrideBufferId = 0;
30413042
for (auto& [layer, layerFE] : layers) {
30423043
CompositionResult compositionResult{layerFE->stealCompositionResult()};
30433044
if (lastLayerStack != layerFE->mSnapshot->outputFilter.layerStack) {
3044-
if (lastLayerStack != ui::INVALID_LAYER_STACK) {
3045+
if (lastLayerStack != ui::UNASSIGNED_LAYER_STACK) {
30453046
// add a space to separate displays
30463047
compositionSummary.push_back(' ');
30473048
}
@@ -3356,7 +3357,7 @@ void SurfaceFlinger::onCompositionPresented(PhysicalDisplayId pacesetterId,
33563357
if (optDisplay && !optDisplay->get()->isVirtual()) {
33573358
auto fence = getHwComposer().getPresentFence(optDisplay->get()->getPhysicalId());
33583359
layer->prepareReleaseCallbacks(ftl::yield<FenceResult>(fence),
3359-
ui::INVALID_LAYER_STACK);
3360+
ui::UNASSIGNED_LAYER_STACK);
33603361
}
33613362
}
33623363
layer->releasePendingBuffer(presentTime.ns());
@@ -6219,7 +6220,7 @@ void SurfaceFlinger::dumpHdrInfo(std::string& result) const {
62196220
void SurfaceFlinger::dumpFrontEnd(std::string& result) {
62206221
std::ostringstream out;
62216222
out << "\nComposition list (bottom to top)\n";
6222-
ui::LayerStack lastPrintedLayerStackHeader = ui::INVALID_LAYER_STACK;
6223+
ui::LayerStack lastPrintedLayerStackHeader = ui::UNASSIGNED_LAYER_STACK;
62236224
for (const auto& snapshot : mLayerSnapshotBuilder.getSnapshots()) {
62246225
if (lastPrintedLayerStackHeader != snapshot->outputFilter.layerStack) {
62256226
lastPrintedLayerStackHeader = snapshot->outputFilter.layerStack;
@@ -6229,7 +6230,7 @@ void SurfaceFlinger::dumpFrontEnd(std::string& result) {
62296230
}
62306231

62316232
out << "\nInput list\n";
6232-
lastPrintedLayerStackHeader = ui::INVALID_LAYER_STACK;
6233+
lastPrintedLayerStackHeader = ui::UNASSIGNED_LAYER_STACK;
62336234
mLayerSnapshotBuilder.forEachInputSnapshot([&](const frontend::LayerSnapshot& snapshot) {
62346235
if (lastPrintedLayerStackHeader != snapshot.outputFilter.layerStack) {
62356236
lastPrintedLayerStackHeader = snapshot.outputFilter.layerStack;
@@ -6247,7 +6248,7 @@ void SurfaceFlinger::dumpFrontEnd(std::string& result) {
62476248
void SurfaceFlinger::dumpVisibleFrontEnd(std::string& result) {
62486249
std::ostringstream out;
62496250
out << "\nComposition list (bottom to top)\n";
6250-
ui::LayerStack lastPrintedLayerStackHeader = ui::INVALID_LAYER_STACK;
6251+
ui::LayerStack lastPrintedLayerStackHeader = ui::UNASSIGNED_LAYER_STACK;
62516252
mLayerSnapshotBuilder.forEachVisibleSnapshot(
62526253
[&](std::unique_ptr<frontend::LayerSnapshot>& snapshot) {
62536254
if (snapshot->hasSomethingToDraw()) {
@@ -6260,7 +6261,7 @@ void SurfaceFlinger::dumpVisibleFrontEnd(std::string& result) {
62606261
});
62616262

62626263
out << "\nInput list\n";
6263-
lastPrintedLayerStackHeader = ui::INVALID_LAYER_STACK;
6264+
lastPrintedLayerStackHeader = ui::UNASSIGNED_LAYER_STACK;
62646265
mLayerSnapshotBuilder.forEachInputSnapshot([&](const frontend::LayerSnapshot& snapshot) {
62656266
if (lastPrintedLayerStackHeader != snapshot.outputFilter.layerStack) {
62666267
lastPrintedLayerStackHeader = snapshot.outputFilter.layerStack;
@@ -7693,7 +7694,7 @@ bool SurfaceFlinger::getSnapshotsFromMainThread(
76937694
if (mRenderEngine->isThreaded()) {
76947695
for (auto& [layer, layerFE] : layers) {
76957696
attachReleaseFenceFutureToLayer(layer, layerFE.get(),
7696-
ui::INVALID_LAYER_STACK);
7697+
ui::UNASSIGNED_LAYER_STACK);
76977698
}
76987699
}
76997700
return getDisplayStateOnMainThread(args);
@@ -7975,7 +7976,7 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::renderScreenImpl(
79757976
// deadlocks between main thread and binder threads waiting for the future fence
79767977
// result, fences should be added to layers in the same hop onto the main thread.
79777978
if (!mRenderEngine->isThreaded()) {
7978-
attachReleaseFenceFutureToLayer(layer, layerFE.get(), ui::INVALID_LAYER_STACK);
7979+
attachReleaseFenceFutureToLayer(layer, layerFE.get(), ui::UNASSIGNED_LAYER_STACK);
79797980
}
79807981
layerFEs.push_back(layerFE);
79817982
}

0 commit comments

Comments
 (0)