Preserve relative z layer info when moving layers offscreen
When a layer is removed from screen, by reparenting it to null, we need to
remove its relative z info as well. Otherwise, if the layer has a visible
relative parent, it will continue to be drawn.
However, there are some scenarios where it maybe useful to preserve relative z.
When a layer is temporarily moved offscreen, and the layer has children with
relative z ordered to other children, then the client logic can be made simpler
by preserving the relative z info for all the children and only removing relative
z info if its relative parent is not on the pending removal state.
Bug: 132613412
Test: atest SurfaceFlinger_test
Test: go/wm-smoke
Change-Id: Id9e2d164673c9028fb27a38ab2b3005269ad4932
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 3ca6ef5..898f3bc 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -158,18 +158,26 @@
}
}
-void Layer::onRemovedFromCurrentState() {
- mRemovedFromCurrentState = true;
+void Layer::removeRelativeZ(const std::vector<Layer*>& layersInTree) {
+ if (mCurrentState.zOrderRelativeOf == nullptr) {
+ return;
+ }
- // the layer is removed from SF mCurrentState to mLayersPendingRemoval
- if (mCurrentState.zOrderRelativeOf != nullptr) {
- sp<Layer> strongRelative = mCurrentState.zOrderRelativeOf.promote();
- if (strongRelative != nullptr) {
- strongRelative->removeZOrderRelative(this);
- mFlinger->setTransactionFlags(eTraversalNeeded);
- }
+ sp<Layer> strongRelative = mCurrentState.zOrderRelativeOf.promote();
+ if (strongRelative == nullptr) {
+ setZOrderRelativeOf(nullptr);
+ return;
+ }
+
+ if (!std::binary_search(layersInTree.begin(), layersInTree.end(), strongRelative.get())) {
+ strongRelative->removeZOrderRelative(this);
+ mFlinger->setTransactionFlags(eTraversalNeeded);
setZOrderRelativeOf(nullptr);
}
+}
+
+void Layer::removeFromCurrentState() {
+ mRemovedFromCurrentState = true;
// Since we are no longer reachable from CurrentState SurfaceFlinger
// will no longer invoke doTransaction for us, and so we will
@@ -186,13 +194,18 @@
mLocalSyncPoints.clear();
}
- for (const auto& child : mCurrentChildren) {
- child->onRemovedFromCurrentState();
- }
-
mFlinger->markLayerPendingRemovalLocked(this);
}
+void Layer::onRemovedFromCurrentState() {
+ auto layersInTree = getLayersInTree(LayerVector::StateSet::Current);
+ std::sort(layersInTree.begin(), layersInTree.end());
+ for (const auto& layer : layersInTree) {
+ layer->removeFromCurrentState();
+ layer->removeRelativeZ(layersInTree);
+ }
+}
+
void Layer::addToCurrentState() {
mRemovedFromCurrentState = false;
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index bbbc5b0..5c55111 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -571,6 +571,17 @@
virtual bool isBufferLatched() const { return false; }
/*
+ * Remove relative z for the layer if its relative parent is not part of the
+ * provided layer tree.
+ */
+ void removeRelativeZ(const std::vector<Layer*>& layersInTree);
+
+ /*
+ * Remove from current state and mark for removal.
+ */
+ void removeFromCurrentState();
+
+ /*
* called with the state lock from a binder thread when the layer is
* removed from the current list to the pending removal list
*/
diff --git a/services/surfaceflinger/tests/SurfaceFlinger_test.filter b/services/surfaceflinger/tests/SurfaceFlinger_test.filter
index 5ebae1e..6b4634a 100644
--- a/services/surfaceflinger/tests/SurfaceFlinger_test.filter
+++ b/services/surfaceflinger/tests/SurfaceFlinger_test.filter
@@ -1,5 +1,5 @@
{
"presubmit": {
- "filter": "CredentialsTest.*:SurfaceFlingerStress.*:SurfaceInterceptorTest.*:LayerTransactionTest.*:LayerTypeTransactionTest.*:LayerUpdateTest.*:GeometryLatchingTest.*:CropLatchingTest.*:ChildLayerTest.*:ScreenCaptureTest.*:ScreenCaptureChildOnlyTest.*:DereferenceSurfaceControlTest.*:BoundlessLayerTest.*:MultiDisplayLayerBoundsTest.*:InvalidHandleTest.*:VirtualDisplayTest.*"
+ "filter": "CredentialsTest.*:SurfaceFlingerStress.*:SurfaceInterceptorTest.*:LayerTransactionTest.*:LayerTypeTransactionTest.*:LayerUpdateTest.*:GeometryLatchingTest.*:CropLatchingTest.*:ChildLayerTest.*:ScreenCaptureTest.*:ScreenCaptureChildOnlyTest.*:DereferenceSurfaceControlTest.*:BoundlessLayerTest.*:MultiDisplayLayerBoundsTest.*:InvalidHandleTest.*:VirtualDisplayTest.*:RelativeZTest.*"
}
}
diff --git a/services/surfaceflinger/tests/Transaction_test.cpp b/services/surfaceflinger/tests/Transaction_test.cpp
index 62db5ec..f83b3ea 100644
--- a/services/surfaceflinger/tests/Transaction_test.cpp
+++ b/services/surfaceflinger/tests/Transaction_test.cpp
@@ -385,6 +385,18 @@
return createLayer(mClient, name, width, height, flags, parent);
}
+ sp<SurfaceControl> createColorLayer(const char* name, const Color& color,
+ SurfaceControl* parent = nullptr) {
+ auto colorLayer = createSurface(mClient, name, 0 /* buffer width */, 0 /* buffer height */,
+ PIXEL_FORMAT_RGBA_8888,
+ ISurfaceComposerClient::eFXSurfaceColor, parent);
+ asTransaction([&](Transaction& t) {
+ t.setColor(colorLayer, half3{color.r / 255.0f, color.g / 255.0f, color.b / 255.0f});
+ t.setAlpha(colorLayer, color.a / 255.0f);
+ });
+ return colorLayer;
+ }
+
ANativeWindow_Buffer getBufferQueueLayerBuffer(const sp<SurfaceControl>& layer) {
// wait for previous transactions (such as setSize) to complete
Transaction().apply(true);
@@ -5875,4 +5887,126 @@
allowedConfigs.end());
}
+class RelativeZTest : public LayerTransactionTest {
+protected:
+ virtual void SetUp() {
+ LayerTransactionTest::SetUp();
+ ASSERT_EQ(NO_ERROR, mClient->initCheck());
+
+ const auto display = SurfaceComposerClient::getInternalDisplayToken();
+ ASSERT_FALSE(display == nullptr);
+
+ // Back layer
+ mBackgroundLayer = createColorLayer("Background layer", Color::RED);
+
+ // Front layer
+ mForegroundLayer = createColorLayer("Foreground layer", Color::GREEN);
+
+ asTransaction([&](Transaction& t) {
+ t.setDisplayLayerStack(display, 0);
+ t.setLayer(mBackgroundLayer, INT32_MAX - 2).show(mBackgroundLayer);
+ t.setLayer(mForegroundLayer, INT32_MAX - 1).show(mForegroundLayer);
+ });
+ }
+
+ virtual void TearDown() {
+ LayerTransactionTest::TearDown();
+ mBackgroundLayer = 0;
+ mForegroundLayer = 0;
+ }
+
+ sp<SurfaceControl> mBackgroundLayer;
+ sp<SurfaceControl> mForegroundLayer;
+};
+
+// When a layer is reparented offscreen, remove relative z order if the relative parent
+// is still onscreen so that the layer is not drawn.
+TEST_F(RelativeZTest, LayerRemoved) {
+ std::unique_ptr<ScreenCapture> sc;
+
+ // Background layer (RED)
+ // Child layer (WHITE) (relative to foregroud layer)
+ // Foregroud layer (GREEN)
+ sp<SurfaceControl> childLayer =
+ createColorLayer("Child layer", Color::BLUE, mBackgroundLayer.get());
+
+ Transaction{}
+ .setRelativeLayer(childLayer, mForegroundLayer->getHandle(), 1)
+ .show(childLayer)
+ .apply();
+
+ {
+ // The childLayer should be in front of the FG control.
+ ScreenCapture::captureScreen(&sc);
+ sc->checkPixel(1, 1, Color::BLUE.r, Color::BLUE.g, Color::BLUE.b);
+ }
+
+ // Background layer (RED)
+ // Foregroud layer (GREEN)
+ Transaction{}.reparent(childLayer, nullptr).apply();
+
+ // Background layer (RED)
+ // Child layer (WHITE)
+ // Foregroud layer (GREEN)
+ Transaction{}.reparent(childLayer, mBackgroundLayer->getHandle()).apply();
+
+ {
+ // The relative z info for child layer should be reset, leaving FG control on top.
+ ScreenCapture::captureScreen(&sc);
+ sc->checkPixel(1, 1, Color::GREEN.r, Color::GREEN.g, Color::GREEN.b);
+ }
+}
+
+// When a layer is reparented offscreen, preseve relative z order if the relative parent
+// is also offscreen. Regression test b/132613412
+TEST_F(RelativeZTest, LayerRemovedOffscreenRelativeParent) {
+ std::unique_ptr<ScreenCapture> sc;
+
+ // Background layer (RED)
+ // Foregroud layer (GREEN)
+ // child level 1 (WHITE)
+ // child level 2a (BLUE)
+ // child level 3 (GREEN) (relative to child level 2b)
+ // child level 2b (BLACK)
+ sp<SurfaceControl> childLevel1 =
+ createColorLayer("child level 1", Color::WHITE, mForegroundLayer.get());
+ sp<SurfaceControl> childLevel2a =
+ createColorLayer("child level 2a", Color::BLUE, childLevel1.get());
+ sp<SurfaceControl> childLevel2b =
+ createColorLayer("child level 2b", Color::BLACK, childLevel1.get());
+ sp<SurfaceControl> childLevel3 =
+ createColorLayer("child level 3", Color::GREEN, childLevel2a.get());
+
+ Transaction{}
+ .setRelativeLayer(childLevel3, childLevel2b->getHandle(), 1)
+ .show(childLevel2a)
+ .show(childLevel2b)
+ .show(childLevel3)
+ .apply();
+
+ {
+ // The childLevel3 should be in front of childLevel2b.
+ ScreenCapture::captureScreen(&sc);
+ sc->checkPixel(1, 1, Color::GREEN.r, Color::GREEN.g, Color::GREEN.b);
+ }
+
+ // Background layer (RED)
+ // Foregroud layer (GREEN)
+ Transaction{}.reparent(childLevel1, nullptr).apply();
+
+ // Background layer (RED)
+ // Foregroud layer (GREEN)
+ // child level 1 (WHITE)
+ // child level 2 back (BLUE)
+ // child level 3 (GREEN) (relative to child level 2b)
+ // child level 2 front (BLACK)
+ Transaction{}.reparent(childLevel1, mForegroundLayer->getHandle()).apply();
+
+ {
+ // Nothing should change at this point since relative z info was preserved.
+ ScreenCapture::captureScreen(&sc);
+ sc->checkPixel(1, 1, Color::GREEN.r, Color::GREEN.g, Color::GREEN.b);
+ }
+}
+
} // namespace android