Update output's dirty region when content changes

nairvis-beep · nairvis-beep · commit a4b3a10e37f1 · 2024-11-05T20:01:55.000Z
Correctly track content changes and update the output's dirty
region. The region is used to update the virtual display
contents. If the changes are not tracked, we may fail to update
the virtual display surface. Fix by tracking the content changes
at the layer as well as accounting for inherited parent layer
changes.

Flag: EXEMPT bugfix
Test: atest com.google.android.gts.cast.VirtualDisplayHostTest#testTestActivityEmbeddingOnVirtualDisplay  --rerun-until-failure
Bug: 360050020, 338403827
Change-Id: Iea4d8bbf0e5d478e908daafb10d13432d524da77
diff --git a/services/surfaceflinger/FrontEnd/LayerSnapshot.cpp b/services/surfaceflinger/FrontEnd/LayerSnapshot.cpp
@@ -250,6 +250,7 @@ std::string LayerSnapshot::getIsVisibleReason() const {
     if (drawShadows()) reason << " shadowSettings.length=" << shadowSettings.length;
     if (backgroundBlurRadius > 0) reason << " backgroundBlurRadius=" << backgroundBlurRadius;
     if (blurRegions.size() > 0) reason << " blurRegions.size()=" << blurRegions.size();
+    if (contentDirty) reason << " contentDirty";
     return reason.str();
 }
 
@@ -359,8 +360,9 @@ void LayerSnapshot::merge(const RequestedLayerState& requested, bool forceUpdate
                           uint32_t displayRotationFlags) {
     clientChanges = requested.what;
     changes = requested.changes;
-    contentDirty = requested.what & layer_state_t::CONTENT_DIRTY;
-    hasReadyFrame = requested.autoRefresh;
+    autoRefresh = requested.autoRefresh;
+    contentDirty = requested.what & layer_state_t::CONTENT_DIRTY || autoRefresh;
+    hasReadyFrame = autoRefresh;
     sidebandStreamHasFrame = requested.hasSidebandStreamFrame();
     updateSurfaceDamage(requested, requested.hasReadyFrame(), forceFullDamage, surfaceDamage);
 
diff --git a/services/surfaceflinger/FrontEnd/LayerSnapshot.h b/services/surfaceflinger/FrontEnd/LayerSnapshot.h
@@ -77,6 +77,7 @@ struct LayerSnapshot : public compositionengine::LayerFECompositionState {
     gui::LayerMetadata layerMetadata;
     gui::LayerMetadata relativeLayerMetadata;
     bool hasReadyFrame; // used in post composition to check if there is another frame ready
+    bool autoRefresh;
     ui::Transform localTransformInverse;
     gui::WindowInfo inputInfo;
     ui::Transform localTransform;
diff --git a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp
@@ -314,8 +314,8 @@ void updateMetadataAndGameMode(LayerSnapshot& snapshot, const RequestedLayerStat
 void clearChanges(LayerSnapshot& snapshot) {
     snapshot.changes.clear();
     snapshot.clientChanges = 0;
-    snapshot.contentDirty = false;
-    snapshot.hasReadyFrame = false;
+    snapshot.contentDirty = snapshot.autoRefresh;
+    snapshot.hasReadyFrame = snapshot.autoRefresh;
     snapshot.sidebandStreamHasFrame = false;
     snapshot.surfaceDamage.clear();
 }
@@ -724,10 +724,12 @@ void LayerSnapshotBuilder::updateSnapshot(LayerSnapshot& snapshot, const Args& a
     if (args.displayChanges) snapshot.changes |= RequestedLayerState::Changes::Geometry;
     snapshot.reachablilty = LayerSnapshot::Reachablilty::Reachable;
     snapshot.clientChanges |= (parentSnapshot.clientChanges & layer_state_t::AFFECTS_CHILDREN);
+    // mark the content as dirty if the parent state changes can dirty the child's content (for
+    // example alpha)
+    snapshot.contentDirty |= (snapshot.clientChanges & layer_state_t::CONTENT_DIRTY) != 0;
     snapshot.isHiddenByPolicyFromParent = parentSnapshot.isHiddenByPolicyFromParent ||
             parentSnapshot.invalidTransform || requested.isHiddenByPolicy() ||
             (args.excludeLayerIds.find(path.id) != args.excludeLayerIds.end());
-
     const bool forceUpdate = args.forceUpdate == ForceUpdateFlags::ALL ||
             snapshot.clientChanges & layer_state_t::eReparent ||
             snapshot.changes.any(RequestedLayerState::Changes::Visibility |
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2526,17 +2526,13 @@ bool SurfaceFlinger::updateLayerSnapshots(VsyncId vsyncId, nsecs_t frameTimeNs,
         frontend::LayerSnapshot* snapshot = mLayerSnapshotBuilder.getSnapshot(it->second->sequence);
         gui::GameMode gameMode = (snapshot) ? snapshot->gameMode : gui::GameMode::Unsupported;
         mLayersWithQueuedFrames.emplace(it->second, gameMode);
-        mLayersIdsWithQueuedFrames.emplace(it->second->sequence);
     }
 
     updateLayerHistory(latchTime);
     mLayerSnapshotBuilder.forEachSnapshot([&](const frontend::LayerSnapshot& snapshot) {
-        // update output dirty region if we have a queued buffer that is visible or a snapshot
-        // recently became invisible
-        // TODO(b/360050020) investigate if we need to update dirty region when layer color changes
-        if ((snapshot.isVisible &&
-             (mLayersIdsWithQueuedFrames.find(snapshot.path.id) !=
-              mLayersIdsWithQueuedFrames.end())) ||
+        // update output's dirty region if a snapshot is visible and its
+        // content is dirty or if a snapshot recently became invisible
+        if ((snapshot.isVisible && snapshot.contentDirty) ||
             (!snapshot.isVisible && snapshot.changes.test(Changes::Visibility))) {
             Region visibleReg;
             visibleReg.set(snapshot.transformedBoundsWithoutTransparentRegion);
@@ -2926,7 +2922,6 @@ CompositeResultsPerDisplay SurfaceFlinger::composite(
     mScheduler->modulateVsync({}, &VsyncModulator::onDisplayRefresh, hasGpuUseOrReuse);
 
     mLayersWithQueuedFrames.clear();
-    mLayersIdsWithQueuedFrames.clear();
     doActiveLayersTracingIfNeeded(true, mVisibleRegionsDirty, pacesetterTarget.frameBeginTime(),
                                   vsyncId);
 
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
@@ -1253,7 +1253,6 @@ class SurfaceFlinger : public BnSurfaceComposer,
     // latched.
     std::unordered_set<std::pair<sp<Layer>, gui::GameMode>, LayerIntHash> mLayersWithQueuedFrames;
     std::unordered_set<sp<Layer>, SpHash<Layer>> mLayersWithBuffersRemoved;
-    std::unordered_set<uint32_t> mLayersIdsWithQueuedFrames;
 
     // Sorted list of layers that were composed during previous frame. This is used to
     // avoid an expensive traversal of the layer hierarchy when there are no
diff --git a/services/surfaceflinger/tests/common/LayerLifecycleManagerHelper.h b/services/surfaceflinger/tests/common/LayerLifecycleManagerHelper.h
@@ -218,6 +218,17 @@ class LayerLifecycleManagerHelper {
         mLifecycleManager.applyTransactions(transactions);
     }
 
+    void setAutoRefresh(uint32_t id, bool autoRefresh) {
+        std::vector<TransactionState> transactions;
+        transactions.emplace_back();
+        transactions.back().states.push_back({});
+
+        transactions.back().states.front().state.what = layer_state_t::eAutoRefreshChanged;
+        transactions.back().states.front().layerId = id;
+        transactions.back().states.front().state.autoRefresh = autoRefresh;
+        mLifecycleManager.applyTransactions(transactions);
+    }
+
     void hideLayer(uint32_t id) {
         setFlags(id, layer_state_t::eLayerHidden, layer_state_t::eLayerHidden);
     }
diff --git a/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp b/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp
@@ -1935,4 +1935,54 @@ TEST_F(LayerSnapshotTest, shouldUpdateInputWhenNoInputInfo) {
     EXPECT_FALSE(getSnapshot(2)->hasInputInfo());
 }
 
+// content dirty test
+TEST_F(LayerSnapshotTest, contentDirtyWhenParentAlphaChanges) {
+    setAlpha(1, 0.5);
+    UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
+    EXPECT_TRUE(getSnapshot(1)->contentDirty);
+    EXPECT_TRUE(getSnapshot(11)->contentDirty);
+    EXPECT_TRUE(getSnapshot(111)->contentDirty);
+
+    // subsequent updates clear the dirty bit
+    UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
+    EXPECT_FALSE(getSnapshot(1)->contentDirty);
+    EXPECT_FALSE(getSnapshot(11)->contentDirty);
+    EXPECT_FALSE(getSnapshot(111)->contentDirty);
+}
+
+TEST_F(LayerSnapshotTest, contentDirtyWhenAutoRefresh) {
+    setAutoRefresh(1, true);
+    UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
+    EXPECT_TRUE(getSnapshot(1)->contentDirty);
+
+    // subsequent updates don't clear the dirty bit
+    UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
+    EXPECT_TRUE(getSnapshot(1)->contentDirty);
+
+    // second update after removing auto refresh will clear content dirty
+    setAutoRefresh(1, false);
+    UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
+    UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
+    EXPECT_FALSE(getSnapshot(1)->contentDirty);
+}
+
+TEST_F(LayerSnapshotTest, contentDirtyWhenColorChanges) {
+    setColor(1, {1, 2, 3});
+    UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
+    EXPECT_TRUE(getSnapshot(1)->contentDirty);
+
+    // subsequent updates clear the dirty bit
+    UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
+    EXPECT_FALSE(getSnapshot(1)->contentDirty);
+}
+
+TEST_F(LayerSnapshotTest, contentDirtyWhenParentGeometryChanges) {
+    setPosition(1, 2, 3);
+    UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
+    EXPECT_TRUE(getSnapshot(1)->contentDirty);
+
+    // subsequent updates clear the dirty bit
+    UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
+    EXPECT_FALSE(getSnapshot(1)->contentDirty);
+}
 } // namespace android::surfaceflinger::frontend
