Skip to content

Commit b0f3016

Browse files
Treehugger RobotAndroid (Google) Code Review
authored andcommitted
Merge "Force snapshot update when requested transform changes from invalid to valid" into main
2 parents 2c710c3 + 9128208 commit b0f3016

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
@@ -150,6 +150,7 @@ RequestedLayerState::RequestedLayerState(const LayerCreationArgs& args)
150150
}
151151

152152
void RequestedLayerState::merge(const ResolvedComposerState& resolvedComposerState) {
153+
bool transformWasValid = transformIsValid;
153154
const uint32_t oldFlags = flags;
154155
const half oldAlpha = color.a;
155156
const bool hadBuffer = externalTexture != nullptr;
@@ -357,6 +358,14 @@ void RequestedLayerState::merge(const ResolvedComposerState& resolvedComposerSta
357358
clientDrawnCornerRadius = clientState.clientDrawnCornerRadius;
358359
changes |= RequestedLayerState::Changes::Geometry;
359360
}
361+
362+
// We can't just check requestedTransform here because LayerSnapshotBuilder uses
363+
// getTransform which reads destinationFrame or buffer dimensions.
364+
// Display rotation does not affect validity so just use ROT_0.
365+
transformIsValid = LayerSnapshot::isTransformValid(getTransform(ui::Transform::ROT_0));
366+
if (!transformWasValid && transformIsValid) {
367+
changes |= RequestedLayerState::Changes::Visibility;
368+
}
360369
}
361370

362371
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)