[sf] Set visible regions change flag when layer starts drawing
If a layer state changes so it has a buffer or an effect like
color then, update the flags to indicate that visiblity of a
snapshot has changed. This will mark the visible regions
flag in CE args and force CE to rebuild its layer stack.
Without this flag, there was a scenario where CE
would get a null snapshot and crash.
Also crash early with traces if we ever pass a null snapshot to
CE.
Test: presubmit
Bug: 238781169, 295069311, 294889236
Change-Id: I355502fbe7f1be38c46a5ed25233b6b07c6b4eeb
diff --git a/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp b/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp
index a5d5563..8a39cd6 100644
--- a/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp
+++ b/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp
@@ -150,7 +150,7 @@
const bool hadSideStream = sidebandStream != nullptr;
const layer_state_t& clientState = resolvedComposerState.state;
- const bool hadBlur = hasBlur();
+ const bool hadSomethingToDraw = hasSomethingToDraw();
uint64_t clientChanges = what | layer_state_t::diff(clientState);
layer_state_t::merge(clientState);
what = clientChanges;
@@ -228,11 +228,10 @@
RequestedLayerState::Changes::VisibleRegion;
}
}
- if (what & (layer_state_t::eBackgroundBlurRadiusChanged | layer_state_t::eBlurRegionsChanged)) {
- if (hadBlur != hasBlur()) {
- changes |= RequestedLayerState::Changes::Visibility |
- RequestedLayerState::Changes::VisibleRegion;
- }
+
+ if (hadSomethingToDraw != hasSomethingToDraw()) {
+ changes |= RequestedLayerState::Changes::Visibility |
+ RequestedLayerState::Changes::VisibleRegion;
}
if (clientChanges & layer_state_t::HIERARCHY_CHANGES)
changes |= RequestedLayerState::Changes::Hierarchy;
@@ -579,6 +578,12 @@
return externalTexture && externalTexture->getUsage() & GRALLOC_USAGE_PROTECTED;
}
+bool RequestedLayerState::hasSomethingToDraw() const {
+ return externalTexture != nullptr || sidebandStream != nullptr || shadowRadius > 0.f ||
+ backgroundBlurRadius > 0 || blurRegions.size() > 0 ||
+ (color.r >= 0.0_hf && color.g >= 0.0_hf && color.b >= 0.0_hf);
+}
+
void RequestedLayerState::clearChanges() {
what = 0;
changes.clear();
diff --git a/services/surfaceflinger/FrontEnd/RequestedLayerState.h b/services/surfaceflinger/FrontEnd/RequestedLayerState.h
index 8eff22b..09f33de 100644
--- a/services/surfaceflinger/FrontEnd/RequestedLayerState.h
+++ b/services/surfaceflinger/FrontEnd/RequestedLayerState.h
@@ -87,6 +87,7 @@
bool backpressureEnabled() const;
bool isSimpleBufferUpdate(const layer_state_t&) const;
bool isProtected() const;
+ bool hasSomethingToDraw() const;
// Layer serial number. This gives layers an explicit ordering, so we
// have a stable sort order when their layer stack and Z-order are
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index fa07001..32bb46c 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -134,6 +134,7 @@
#include "FrontEnd/LayerCreationArgs.h"
#include "FrontEnd/LayerHandle.h"
#include "FrontEnd/LayerLifecycleManager.h"
+#include "FrontEnd/LayerLog.h"
#include "FrontEnd/LayerSnapshot.h"
#include "HdrLayerInfoReporter.h"
#include "Layer.h"
@@ -2620,6 +2621,23 @@
constexpr bool kCursorOnly = false;
const auto layers = moveSnapshotsToCompositionArgs(refreshArgs, kCursorOnly);
+ if (mLayerLifecycleManagerEnabled && !refreshArgs.updatingGeometryThisFrame) {
+ for (const auto& [token, display] : FTL_FAKE_GUARD(mStateLock, mDisplays)) {
+ auto compositionDisplay = display->getCompositionDisplay();
+ if (!compositionDisplay->getState().isEnabled) continue;
+ for (auto outputLayer : compositionDisplay->getOutputLayersOrderedByZ()) {
+ LLOG_ALWAYS_FATAL_WITH_TRACE_IF(outputLayer->getLayerFE().getCompositionState() ==
+ nullptr,
+ "Output layer %s for display %s %" PRIu64
+ " has a null "
+ "snapshot.",
+ outputLayer->getLayerFE().getDebugName(),
+ compositionDisplay->getName().c_str(),
+ compositionDisplay->getId().value);
+ }
+ }
+ }
+
mCompositionEngine->present(refreshArgs);
moveSnapshotsFromCompositionArgs(refreshArgs, layers);
diff --git a/services/surfaceflinger/tests/unittests/LayerLifecycleManagerTest.cpp b/services/surfaceflinger/tests/unittests/LayerLifecycleManagerTest.cpp
index e3c7837..d65277a 100644
--- a/services/surfaceflinger/tests/unittests/LayerLifecycleManagerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/LayerLifecycleManagerTest.cpp
@@ -389,4 +389,52 @@
listener->expectLayersDestroyed({1, bgLayerId});
}
+TEST_F(LayerLifecycleManagerTest, blurSetsVisibilityChangeFlag) {
+ // clear default color on layer so we start with a layer that does not draw anything.
+ setColor(1, {-1.f, -1.f, -1.f});
+ mLifecycleManager.commitChanges();
+
+ // layer has something to draw
+ setBackgroundBlurRadius(1, 2);
+ EXPECT_TRUE(
+ mLifecycleManager.getGlobalChanges().test(RequestedLayerState::Changes::Visibility));
+ mLifecycleManager.commitChanges();
+
+ // layer still has something to draw, so visibility shouldn't change
+ setBackgroundBlurRadius(1, 3);
+ EXPECT_FALSE(
+ mLifecycleManager.getGlobalChanges().test(RequestedLayerState::Changes::Visibility));
+ mLifecycleManager.commitChanges();
+
+ // layer has nothing to draw
+ setBackgroundBlurRadius(1, 0);
+ EXPECT_TRUE(
+ mLifecycleManager.getGlobalChanges().test(RequestedLayerState::Changes::Visibility));
+ mLifecycleManager.commitChanges();
+}
+
+TEST_F(LayerLifecycleManagerTest, colorSetsVisibilityChangeFlag) {
+ // clear default color on layer so we start with a layer that does not draw anything.
+ setColor(1, {-1.f, -1.f, -1.f});
+ mLifecycleManager.commitChanges();
+
+ // layer has something to draw
+ setColor(1, {2.f, 3.f, 4.f});
+ EXPECT_TRUE(
+ mLifecycleManager.getGlobalChanges().test(RequestedLayerState::Changes::Visibility));
+ mLifecycleManager.commitChanges();
+
+ // layer still has something to draw, so visibility shouldn't change
+ setColor(1, {0.f, 0.f, 0.f});
+ EXPECT_FALSE(
+ mLifecycleManager.getGlobalChanges().test(RequestedLayerState::Changes::Visibility));
+ mLifecycleManager.commitChanges();
+
+ // layer has nothing to draw
+ setColor(1, {-1.f, -1.f, -1.f});
+ EXPECT_TRUE(
+ mLifecycleManager.getGlobalChanges().test(RequestedLayerState::Changes::Visibility));
+ mLifecycleManager.commitChanges();
+}
+
} // namespace android::surfaceflinger::frontend
diff --git a/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp b/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp
index 84c3775..c8eda12 100644
--- a/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp
+++ b/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp
@@ -349,16 +349,22 @@
}
TEST_F(LayerSnapshotTest, blurUpdatesWhenAlphaChanges) {
- static constexpr int blurRadius = 42;
- setBackgroundBlurRadius(1221, blurRadius);
+ int blurRadius = 42;
+ setBackgroundBlurRadius(1221, static_cast<uint32_t>(blurRadius));
UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
EXPECT_EQ(getSnapshot({.id = 1221})->backgroundBlurRadius, blurRadius);
+ blurRadius = 21;
+ setBackgroundBlurRadius(1221, static_cast<uint32_t>(blurRadius));
+ UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
+ EXPECT_EQ(getSnapshot({.id = 1221})->backgroundBlurRadius, blurRadius);
+
static constexpr float alpha = 0.5;
setAlpha(12, alpha);
UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
- EXPECT_EQ(getSnapshot({.id = 1221})->backgroundBlurRadius, blurRadius * alpha);
+ EXPECT_EQ(getSnapshot({.id = 1221})->backgroundBlurRadius,
+ static_cast<int>(static_cast<float>(blurRadius) * alpha));
}
// Display Mirroring Tests