Skip to content

Commit 9128208

Browse files
committed
Force snapshot update when requested transform changes from invalid to valid
An early out in snapshot builder prevents parent properties from being propagated to children when the parent is hidden (e.g. due to invalid transform). So we need to force update children snapshots when the parent becomes visible (i.e. transform becomes valid). I tested removing the early out but the added benchmark takes 2us longer. Bug: 319757092 Flag: EXEMPT bug fix Test: atest libsurfaceflinger_unittests Change-Id: I9c9a996bbc3de4e7f8cbfc947a99cdccae5090b9
1 parent 4a90260 commit 9128208

4 files changed

Lines changed: 85 additions & 0 deletions

File tree

services/surfaceflinger/FrontEnd/RequestedLayerState.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ RequestedLayerState::RequestedLayerState(const LayerCreationArgs& args)
147147
}
148148

149149
void RequestedLayerState::merge(const ResolvedComposerState& resolvedComposerState) {
150+
bool transformWasValid = transformIsValid;
150151
const uint32_t oldFlags = flags;
151152
const half oldAlpha = color.a;
152153
const bool hadBuffer = externalTexture != nullptr;
@@ -354,6 +355,14 @@ void RequestedLayerState::merge(const ResolvedComposerState& resolvedComposerSta
354355
clientDrawnCornerRadius = clientState.clientDrawnCornerRadius;
355356
changes |= RequestedLayerState::Changes::Geometry;
356357
}
358+
359+
// We can't just check requestedTransform here because LayerSnapshotBuilder uses
360+
// getTransform which reads destinationFrame or buffer dimensions.
361+
// Display rotation does not affect validity so just use ROT_0.
362+
transformIsValid = LayerSnapshot::isTransformValid(getTransform(ui::Transform::ROT_0));
363+
if (!transformWasValid && transformIsValid) {
364+
changes |= RequestedLayerState::Changes::Visibility;
365+
}
357366
}
358367

359368
ui::Size RequestedLayerState::getUnrotatedBufferSize(uint32_t displayRotationFlags) const {

services/surfaceflinger/FrontEnd/RequestedLayerState.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ struct RequestedLayerState : layer_state_t {
115115
const gui::Pid ownerPid;
116116
bool dataspaceRequested;
117117
bool hasColorTransform;
118+
bool transformIsValid = true;
118119
bool premultipliedAlpha{true};
119120
// This layer can be a cursor on some displays.
120121
bool potentialCursor{false};

services/surfaceflinger/tests/benchmarks/LayerLifecycleManager_benchmarks.cpp

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,5 +90,46 @@ static void updateClientStatesNoChanges(benchmark::State& state) {
9090
}
9191
BENCHMARK(updateClientStatesNoChanges);
9292

93+
static void propagateManyHiddenChildren(benchmark::State& state) {
94+
LayerLifecycleManager lifecycleManager;
95+
LayerLifecycleManagerHelper helper(lifecycleManager);
96+
97+
helper.createRootLayer(0);
98+
for (uint32_t i = 1; i < 50; ++i) {
99+
helper.createLayer(i, i - 1);
100+
}
101+
102+
helper.hideLayer(0);
103+
104+
LayerHierarchyBuilder hierarchyBuilder;
105+
DisplayInfo info;
106+
info.info.logicalHeight = 100;
107+
info.info.logicalWidth = 100;
108+
DisplayInfos displayInfos;
109+
displayInfos.emplace_or_replace(ui::LayerStack::fromValue(1), info);
110+
ShadowSettings globalShadowSettings;
111+
112+
LayerSnapshotBuilder snapshotBuilder;
113+
114+
int i = 1;
115+
for (auto _ : state) {
116+
i++;
117+
helper.setAlpha(0, (1 + (i % 255)) / 255.0f);
118+
119+
if (lifecycleManager.getGlobalChanges().test(RequestedLayerState::Changes::Hierarchy)) {
120+
hierarchyBuilder.update(lifecycleManager);
121+
}
122+
LayerSnapshotBuilder::Args args{.root = hierarchyBuilder.getHierarchy(),
123+
.layerLifecycleManager = lifecycleManager,
124+
.displays = displayInfos,
125+
.globalShadowSettings = globalShadowSettings,
126+
.supportedLayerGenericMetadata = {},
127+
.genericLayerMetadataKeyMap = {}};
128+
snapshotBuilder.update(args);
129+
lifecycleManager.commitChanges();
130+
}
131+
}
132+
BENCHMARK(propagateManyHiddenChildren);
133+
93134
} // namespace
94135
} // namespace android::surfaceflinger

services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,40 @@ TEST_F(LayerSnapshotTest, AlphaInheritedByChildren) {
261261
EXPECT_EQ(getSnapshot(1221)->alpha, 0.25f);
262262
}
263263

264+
TEST_F(LayerSnapshotTest, AlphaInheritedByChildWhenParentIsHiddenByInvalidTransform) {
265+
setMatrix(1, 0, 0, 0, 0);
266+
update(mSnapshotBuilder);
267+
mLifecycleManager.commitChanges();
268+
269+
setAlpha(1, 0.5);
270+
update(mSnapshotBuilder);
271+
mLifecycleManager.commitChanges();
272+
273+
setMatrix(1, 1, 0, 0, 1);
274+
update(mSnapshotBuilder);
275+
mLifecycleManager.commitChanges();
276+
277+
EXPECT_EQ(getSnapshot(1)->alpha, 0.5f);
278+
EXPECT_EQ(getSnapshot(11)->alpha, 0.5f);
279+
}
280+
281+
TEST_F(LayerSnapshotTest, AlphaInheritedByChildWhenParentIsHidden) {
282+
hideLayer(1);
283+
update(mSnapshotBuilder);
284+
mLifecycleManager.commitChanges();
285+
286+
setAlpha(1, 0.5);
287+
update(mSnapshotBuilder);
288+
mLifecycleManager.commitChanges();
289+
290+
showLayer(1);
291+
update(mSnapshotBuilder);
292+
mLifecycleManager.commitChanges();
293+
294+
EXPECT_EQ(getSnapshot(1)->alpha, 0.5f);
295+
EXPECT_EQ(getSnapshot(11)->alpha, 0.5f);
296+
}
297+
264298
// Change states
265299
TEST_F(LayerSnapshotTest, UpdateClearsPreviousChangeStates) {
266300
setCrop(1, Rect(1, 2, 3, 4));

0 commit comments

Comments
 (0)