Skip to content

Commit 5df9431

Browse files
nairvis-beepAndroid (Google) Code Review
nairvis-beep
authored and
Android (Google) Code Review
committed
Merge "Update output's dirty region when content changes" into main
2 parents 52683c6 + a4b3a10 commit 5df9431

7 files changed

Lines changed: 74 additions & 14 deletions

File tree

‎services/surfaceflinger/FrontEnd/LayerSnapshot.cpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,7 @@ std::string LayerSnapshot::getIsVisibleReason() const {
250250
if (drawShadows()) reason << " shadowSettings.length=" << shadowSettings.length;
251251
if (backgroundBlurRadius > 0) reason << " backgroundBlurRadius=" << backgroundBlurRadius;
252252
if (blurRegions.size() > 0) reason << " blurRegions.size()=" << blurRegions.size();
253+
if (contentDirty) reason << " contentDirty";
253254
return reason.str();
254255
}
255256

@@ -359,8 +360,9 @@ void LayerSnapshot::merge(const RequestedLayerState& requested, bool forceUpdate
359360
uint32_t displayRotationFlags) {
360361
clientChanges = requested.what;
361362
changes = requested.changes;
362-
contentDirty = requested.what & layer_state_t::CONTENT_DIRTY;
363-
hasReadyFrame = requested.autoRefresh;
363+
autoRefresh = requested.autoRefresh;
364+
contentDirty = requested.what & layer_state_t::CONTENT_DIRTY || autoRefresh;
365+
hasReadyFrame = autoRefresh;
364366
sidebandStreamHasFrame = requested.hasSidebandStreamFrame();
365367
updateSurfaceDamage(requested, requested.hasReadyFrame(), forceFullDamage, surfaceDamage);
366368

‎services/surfaceflinger/FrontEnd/LayerSnapshot.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ struct LayerSnapshot : public compositionengine::LayerFECompositionState {
7777
gui::LayerMetadata layerMetadata;
7878
gui::LayerMetadata relativeLayerMetadata;
7979
bool hasReadyFrame; // used in post composition to check if there is another frame ready
80+
bool autoRefresh;
8081
ui::Transform localTransformInverse;
8182
gui::WindowInfo inputInfo;
8283
ui::Transform localTransform;

‎services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -314,8 +314,8 @@ void updateMetadataAndGameMode(LayerSnapshot& snapshot, const RequestedLayerStat
314314
void clearChanges(LayerSnapshot& snapshot) {
315315
snapshot.changes.clear();
316316
snapshot.clientChanges = 0;
317-
snapshot.contentDirty = false;
318-
snapshot.hasReadyFrame = false;
317+
snapshot.contentDirty = snapshot.autoRefresh;
318+
snapshot.hasReadyFrame = snapshot.autoRefresh;
319319
snapshot.sidebandStreamHasFrame = false;
320320
snapshot.surfaceDamage.clear();
321321
}
@@ -724,10 +724,12 @@ void LayerSnapshotBuilder::updateSnapshot(LayerSnapshot& snapshot, const Args& a
724724
if (args.displayChanges) snapshot.changes |= RequestedLayerState::Changes::Geometry;
725725
snapshot.reachablilty = LayerSnapshot::Reachablilty::Reachable;
726726
snapshot.clientChanges |= (parentSnapshot.clientChanges & layer_state_t::AFFECTS_CHILDREN);
727+
// mark the content as dirty if the parent state changes can dirty the child's content (for
728+
// example alpha)
729+
snapshot.contentDirty |= (snapshot.clientChanges & layer_state_t::CONTENT_DIRTY) != 0;
727730
snapshot.isHiddenByPolicyFromParent = parentSnapshot.isHiddenByPolicyFromParent ||
728731
parentSnapshot.invalidTransform || requested.isHiddenByPolicy() ||
729732
(args.excludeLayerIds.find(path.id) != args.excludeLayerIds.end());
730-
731733
const bool forceUpdate = args.forceUpdate == ForceUpdateFlags::ALL ||
732734
snapshot.clientChanges & layer_state_t::eReparent ||
733735
snapshot.changes.any(RequestedLayerState::Changes::Visibility |

‎services/surfaceflinger/SurfaceFlinger.cpp‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2532,17 +2532,13 @@ bool SurfaceFlinger::updateLayerSnapshots(VsyncId vsyncId, nsecs_t frameTimeNs,
25322532
frontend::LayerSnapshot* snapshot = mLayerSnapshotBuilder.getSnapshot(it->second->sequence);
25332533
gui::GameMode gameMode = (snapshot) ? snapshot->gameMode : gui::GameMode::Unsupported;
25342534
mLayersWithQueuedFrames.emplace(it->second, gameMode);
2535-
mLayersIdsWithQueuedFrames.emplace(it->second->sequence);
25362535
}
25372536

25382537
updateLayerHistory(latchTime);
25392538
mLayerSnapshotBuilder.forEachSnapshot([&](const frontend::LayerSnapshot& snapshot) {
2540-
// update output dirty region if we have a queued buffer that is visible or a snapshot
2541-
// recently became invisible
2542-
// TODO(b/360050020) investigate if we need to update dirty region when layer color changes
2543-
if ((snapshot.isVisible &&
2544-
(mLayersIdsWithQueuedFrames.find(snapshot.path.id) !=
2545-
mLayersIdsWithQueuedFrames.end())) ||
2539+
// update output's dirty region if a snapshot is visible and its
2540+
// content is dirty or if a snapshot recently became invisible
2541+
if ((snapshot.isVisible && snapshot.contentDirty) ||
25462542
(!snapshot.isVisible && snapshot.changes.test(Changes::Visibility))) {
25472543
Region visibleReg;
25482544
visibleReg.set(snapshot.transformedBoundsWithoutTransparentRegion);
@@ -2932,7 +2928,6 @@ CompositeResultsPerDisplay SurfaceFlinger::composite(
29322928
mScheduler->modulateVsync({}, &VsyncModulator::onDisplayRefresh, hasGpuUseOrReuse);
29332929

29342930
mLayersWithQueuedFrames.clear();
2935-
mLayersIdsWithQueuedFrames.clear();
29362931
doActiveLayersTracingIfNeeded(true, mVisibleRegionsDirty, pacesetterTarget.frameBeginTime(),
29372932
vsyncId);
29382933

‎services/surfaceflinger/SurfaceFlinger.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1241,7 +1241,6 @@ class SurfaceFlinger : public BnSurfaceComposer,
12411241
// latched.
12421242
std::unordered_set<std::pair<sp<Layer>, gui::GameMode>, LayerIntHash> mLayersWithQueuedFrames;
12431243
std::unordered_set<sp<Layer>, SpHash<Layer>> mLayersWithBuffersRemoved;
1244-
std::unordered_set<uint32_t> mLayersIdsWithQueuedFrames;
12451244

12461245
// Sorted list of layers that were composed during previous frame. This is used to
12471246
// avoid an expensive traversal of the layer hierarchy when there are no

‎services/surfaceflinger/tests/common/LayerLifecycleManagerHelper.h‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,17 @@ class LayerLifecycleManagerHelper {
218218
mLifecycleManager.applyTransactions(transactions);
219219
}
220220

221+
void setAutoRefresh(uint32_t id, bool autoRefresh) {
222+
std::vector<TransactionState> transactions;
223+
transactions.emplace_back();
224+
transactions.back().states.push_back({});
225+
226+
transactions.back().states.front().state.what = layer_state_t::eAutoRefreshChanged;
227+
transactions.back().states.front().layerId = id;
228+
transactions.back().states.front().state.autoRefresh = autoRefresh;
229+
mLifecycleManager.applyTransactions(transactions);
230+
}
231+
221232
void hideLayer(uint32_t id) {
222233
setFlags(id, layer_state_t::eLayerHidden, layer_state_t::eLayerHidden);
223234
}

‎services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1935,4 +1935,54 @@ TEST_F(LayerSnapshotTest, shouldUpdateInputWhenNoInputInfo) {
19351935
EXPECT_FALSE(getSnapshot(2)->hasInputInfo());
19361936
}
19371937

1938+
// content dirty test
1939+
TEST_F(LayerSnapshotTest, contentDirtyWhenParentAlphaChanges) {
1940+
setAlpha(1, 0.5);
1941+
UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
1942+
EXPECT_TRUE(getSnapshot(1)->contentDirty);
1943+
EXPECT_TRUE(getSnapshot(11)->contentDirty);
1944+
EXPECT_TRUE(getSnapshot(111)->contentDirty);
1945+
1946+
// subsequent updates clear the dirty bit
1947+
UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
1948+
EXPECT_FALSE(getSnapshot(1)->contentDirty);
1949+
EXPECT_FALSE(getSnapshot(11)->contentDirty);
1950+
EXPECT_FALSE(getSnapshot(111)->contentDirty);
1951+
}
1952+
1953+
TEST_F(LayerSnapshotTest, contentDirtyWhenAutoRefresh) {
1954+
setAutoRefresh(1, true);
1955+
UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
1956+
EXPECT_TRUE(getSnapshot(1)->contentDirty);
1957+
1958+
// subsequent updates don't clear the dirty bit
1959+
UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
1960+
EXPECT_TRUE(getSnapshot(1)->contentDirty);
1961+
1962+
// second update after removing auto refresh will clear content dirty
1963+
setAutoRefresh(1, false);
1964+
UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
1965+
UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
1966+
EXPECT_FALSE(getSnapshot(1)->contentDirty);
1967+
}
1968+
1969+
TEST_F(LayerSnapshotTest, contentDirtyWhenColorChanges) {
1970+
setColor(1, {1, 2, 3});
1971+
UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
1972+
EXPECT_TRUE(getSnapshot(1)->contentDirty);
1973+
1974+
// subsequent updates clear the dirty bit
1975+
UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
1976+
EXPECT_FALSE(getSnapshot(1)->contentDirty);
1977+
}
1978+
1979+
TEST_F(LayerSnapshotTest, contentDirtyWhenParentGeometryChanges) {
1980+
setPosition(1, 2, 3);
1981+
UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
1982+
EXPECT_TRUE(getSnapshot(1)->contentDirty);
1983+
1984+
// subsequent updates clear the dirty bit
1985+
UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
1986+
EXPECT_FALSE(getSnapshot(1)->contentDirty);
1987+
}
19381988
} // namespace android::surfaceflinger::frontend

0 commit comments

Comments
 (0)