SF: partially revert df59f4744c0672cc69dad72b230a757c1e4be116
Avoid a composition cycle when the FrameRate votes updates,
but keep the old behavior for all other cases.
Bug: 339759346
Change-Id: Ic0696ddeee6ed10f1e6efcc0dbe9589d638900cd
Test: android.platform.test.scenario.gmail.OpenCloseComposeEmailMicrobenchmark#testOpenCloseComposeEmail
diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h
index ebdf232..ca7acf9 100644
--- a/libs/gui/include/gui/LayerState.h
+++ b/libs/gui/include/gui/LayerState.h
@@ -279,9 +279,6 @@
layer_state_t::eDropInputModeChanged | layer_state_t::eTrustedOverlayChanged |
layer_state_t::eLayerStackChanged;
- // Changes requiring a composition pass.
- static constexpr uint64_t REQUIRES_COMPOSITION = layer_state_t::CONTENT_DIRTY;
-
// Changes that affect the visible region on a display.
static constexpr uint64_t VISIBLE_REGION_CHANGES = layer_state_t::GEOMETRY_CHANGES |
layer_state_t::HIERARCHY_CHANGES | layer_state_t::eAlphaChanged;
diff --git a/services/surfaceflinger/FrontEnd/LayerLifecycleManager.cpp b/services/surfaceflinger/FrontEnd/LayerLifecycleManager.cpp
index 52f8bea..4b0618e 100644
--- a/services/surfaceflinger/FrontEnd/LayerLifecycleManager.cpp
+++ b/services/surfaceflinger/FrontEnd/LayerLifecycleManager.cpp
@@ -41,8 +41,7 @@
return;
}
- mGlobalChanges |= RequestedLayerState::Changes::Hierarchy |
- RequestedLayerState::Changes::RequiresComposition;
+ mGlobalChanges |= RequestedLayerState::Changes::Hierarchy;
for (auto& newLayer : newLayers) {
RequestedLayerState& layer = *newLayer.get();
auto [it, inserted] = mIdToLayer.try_emplace(layer.id, References{.owner = layer});
@@ -105,8 +104,7 @@
if (!layer.canBeDestroyed()) {
continue;
}
- layer.changes |= RequestedLayerState::Changes::Destroyed |
- RequestedLayerState::Changes::RequiresComposition;
+ layer.changes |= RequestedLayerState::Changes::Destroyed;
layersToBeDestroyed.emplace_back(layerId);
}
@@ -114,8 +112,7 @@
return;
}
- mGlobalChanges |= RequestedLayerState::Changes::Hierarchy |
- RequestedLayerState::Changes::RequiresComposition;
+ mGlobalChanges |= RequestedLayerState::Changes::Hierarchy;
for (size_t i = 0; i < layersToBeDestroyed.size(); i++) {
uint32_t layerId = layersToBeDestroyed[i];
auto it = mIdToLayer.find(layerId);
@@ -145,8 +142,7 @@
if (linkedLayer->parentId == layer.id) {
linkedLayer->parentId = UNASSIGNED_LAYER_ID;
if (linkedLayer->canBeDestroyed()) {
- linkedLayer->changes |= RequestedLayerState::Changes::Destroyed |
- RequestedLayerState::Changes::RequiresComposition;
+ linkedLayer->changes |= RequestedLayerState::Changes::Destroyed;
layersToBeDestroyed.emplace_back(linkedLayer->id);
}
}
@@ -253,8 +249,7 @@
layer_state_t::eDataspaceChanged | layer_state_t::eAlphaChanged;
bgColorLayer->changes |= RequestedLayerState::Changes::Content;
mChangedLayers.push_back(bgColorLayer);
- mGlobalChanges |= RequestedLayerState::Changes::Content |
- RequestedLayerState::Changes::RequiresComposition;
+ mGlobalChanges |= RequestedLayerState::Changes::Content;
}
}
@@ -412,8 +407,7 @@
layer.relativeParentId = unlinkLayer(layer.relativeParentId, layer.id);
layer.changes |=
RequestedLayerState::Changes::Hierarchy | RequestedLayerState::Changes::RelativeParent;
- mGlobalChanges |= RequestedLayerState::Changes::Hierarchy |
- RequestedLayerState::Changes::RequiresComposition;
+ mGlobalChanges |= RequestedLayerState::Changes::Hierarchy;
}
// Some layers mirror the entire display stack. Since we don't have a single root layer per display
diff --git a/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp b/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp
index f5e5b02..5631fac 100644
--- a/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp
+++ b/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp
@@ -58,8 +58,7 @@
parentId(args.parentId),
layerIdToMirror(args.layerIdToMirror) {
layerId = static_cast<int32_t>(args.sequence);
- changes |= RequestedLayerState::Changes::Created |
- RequestedLayerState::Changes::RequiresComposition;
+ changes |= RequestedLayerState::Changes::Created;
metadata.merge(args.metadata);
changes |= RequestedLayerState::Changes::Metadata;
handleAlive = true;
@@ -249,8 +248,7 @@
if (hadSomethingToDraw != hasSomethingToDraw()) {
changes |= RequestedLayerState::Changes::Visibility |
- RequestedLayerState::Changes::VisibleRegion |
- RequestedLayerState::Changes::RequiresComposition;
+ RequestedLayerState::Changes::VisibleRegion;
}
if (clientChanges & layer_state_t::HIERARCHY_CHANGES)
changes |= RequestedLayerState::Changes::Hierarchy;
@@ -260,8 +258,6 @@
changes |= RequestedLayerState::Changes::Geometry;
if (clientChanges & layer_state_t::AFFECTS_CHILDREN)
changes |= RequestedLayerState::Changes::AffectsChildren;
- if (clientChanges & layer_state_t::REQUIRES_COMPOSITION)
- changes |= RequestedLayerState::Changes::RequiresComposition;
if (clientChanges & layer_state_t::INPUT_CHANGES)
changes |= RequestedLayerState::Changes::Input;
if (clientChanges & layer_state_t::VISIBLE_REGION_CHANGES)
diff --git a/services/surfaceflinger/FrontEnd/RequestedLayerState.h b/services/surfaceflinger/FrontEnd/RequestedLayerState.h
index 4829d4c..48b9640 100644
--- a/services/surfaceflinger/FrontEnd/RequestedLayerState.h
+++ b/services/surfaceflinger/FrontEnd/RequestedLayerState.h
@@ -26,6 +26,7 @@
#include "TransactionState.h"
namespace android::surfaceflinger::frontend {
+using namespace ftl::flag_operators;
// Stores client requested states for a layer.
// This struct does not store any other states or states pertaining to
@@ -57,8 +58,14 @@
BufferSize = 1u << 18,
GameMode = 1u << 19,
BufferUsageFlags = 1u << 20,
- RequiresComposition = 1u << 21,
};
+
+ static constexpr ftl::Flags<Changes> kMustComposite = Changes::Created | Changes::Destroyed |
+ Changes::Hierarchy | Changes::Geometry | Changes::Content | Changes::Input |
+ Changes::Z | Changes::Mirror | Changes::Parent | Changes::RelativeParent |
+ Changes::Metadata | Changes::Visibility | Changes::VisibleRegion | Changes::Buffer |
+ Changes::SidebandStream | Changes::Animation | Changes::BufferSize | Changes::GameMode |
+ Changes::BufferUsageFlags;
static Rect reduce(const Rect& win, const Region& exclude);
RequestedLayerState(const LayerCreationArgs&);
void merge(const ResolvedComposerState&);
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 28d8018..e2af5cf 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2447,8 +2447,8 @@
}
outTransactionsAreEmpty = mLayerLifecycleManager.getGlobalChanges().get() == 0;
if (FlagManager::getInstance().vrr_bugfix_24q4()) {
- mustComposite |= mLayerLifecycleManager.getGlobalChanges().test(
- frontend::RequestedLayerState::Changes::RequiresComposition);
+ mustComposite |= mLayerLifecycleManager.getGlobalChanges().any(
+ frontend::RequestedLayerState::kMustComposite);
} else {
mustComposite |= mLayerLifecycleManager.getGlobalChanges().get() != 0;
}
diff --git a/services/surfaceflinger/tests/unittests/LayerLifecycleManagerTest.cpp b/services/surfaceflinger/tests/unittests/LayerLifecycleManagerTest.cpp
index 158db75..cfc8e99 100644
--- a/services/surfaceflinger/tests/unittests/LayerLifecycleManagerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/LayerLifecycleManagerTest.cpp
@@ -462,8 +462,7 @@
GRALLOC_USAGE_PROTECTED /*usage*/));
EXPECT_EQ(mLifecycleManager.getGlobalChanges().get(),
ftl::Flags<RequestedLayerState::Changes>(
- RequestedLayerState::Changes::Buffer | RequestedLayerState::Changes::Content |
- RequestedLayerState::Changes::RequiresComposition)
+ RequestedLayerState::Changes::Buffer | RequestedLayerState::Changes::Content)
.get());
mLifecycleManager.commitChanges();
@@ -497,8 +496,7 @@
ftl::Flags<RequestedLayerState::Changes>(
RequestedLayerState::Changes::Buffer | RequestedLayerState::Changes::Content |
RequestedLayerState::Changes::VisibleRegion |
- RequestedLayerState::Changes::Visibility |
- RequestedLayerState::Changes::RequiresComposition)
+ RequestedLayerState::Changes::Visibility)
.get());
mLifecycleManager.commitChanges();
}
@@ -522,8 +520,7 @@
RequestedLayerState::Changes::AffectsChildren |
RequestedLayerState::Changes::Content |
RequestedLayerState::Changes::Geometry |
- RequestedLayerState::Changes::VisibleRegion |
- RequestedLayerState::Changes::RequiresComposition)
+ RequestedLayerState::Changes::VisibleRegion)
.get());
mLifecycleManager.commitChanges();
}
@@ -541,8 +538,7 @@
ftl::Flags<RequestedLayerState::Changes>(
RequestedLayerState::Changes::Content |
RequestedLayerState::Changes::AffectsChildren |
- RequestedLayerState::Changes::VisibleRegion |
- RequestedLayerState::Changes::RequiresComposition)
+ RequestedLayerState::Changes::VisibleRegion)
.string());
EXPECT_EQ(mLifecycleManager.getChangedLayers()[0]->color.a, static_cast<half>(startingAlpha));
mLifecycleManager.commitChanges();
@@ -555,8 +551,7 @@
ftl::Flags<RequestedLayerState::Changes>(
RequestedLayerState::Changes::Content |
RequestedLayerState::Changes::AffectsChildren |
- RequestedLayerState::Changes::VisibleRegion |
- RequestedLayerState::Changes::RequiresComposition)
+ RequestedLayerState::Changes::VisibleRegion)
.string());
EXPECT_EQ(mLifecycleManager.getChangedLayers()[0]->color.a, static_cast<half>(endingAlpha));
mLifecycleManager.commitChanges();
@@ -586,8 +581,7 @@
GRALLOC_USAGE_SW_READ_NEVER /*usage*/));
EXPECT_EQ(mLifecycleManager.getGlobalChanges().get(),
ftl::Flags<RequestedLayerState::Changes>(
- RequestedLayerState::Changes::Buffer | RequestedLayerState::Changes::Content |
- RequestedLayerState::Changes::RequiresComposition)
+ RequestedLayerState::Changes::Buffer | RequestedLayerState::Changes::Content)
.get());
mLifecycleManager.commitChanges();
diff --git a/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp b/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp
index ce4d798..23b6851 100644
--- a/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp
+++ b/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp
@@ -255,8 +255,7 @@
setColor(11, {1._hf, 0._hf, 0._hf});
UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
EXPECT_EQ(getSnapshot(11)->changes,
- RequestedLayerState::Changes::Content |
- RequestedLayerState::Changes::RequiresComposition);
+ RequestedLayerState::Changes::Content);
EXPECT_EQ(getSnapshot(11)->clientChanges, layer_state_t::eColorChanged);
EXPECT_EQ(getSnapshot(1)->changes.get(), 0u);
UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
@@ -267,8 +266,7 @@
setColor(1, {1._hf, 0._hf, 0._hf});
UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
EXPECT_EQ(getSnapshot(1)->changes,
- RequestedLayerState::Changes::Content |
- RequestedLayerState::Changes::RequiresComposition);
+ RequestedLayerState::Changes::Content);
EXPECT_EQ(getSnapshot(1)->clientChanges, layer_state_t::eColorChanged);
}