commit 3237758d5790d0f6ab8e8a2bffe4d9df869aae83
author chaviw <chaviw@google.com> Mon May 13 11:15:19 2019 -0700
committer chaviw <chaviw@google.com> Mon May 13 17:02:13 2019 -0700
tree cbe36e8b2c98cd85294c64da2cff94274a638220
parent 6dec1db01247a00564e433ac2ff904e3d0ccac2c

Don't allow both setRelative and setLayer for the same surface.

Both setRelativeLayer and setLayer are used to set z order. The two will
conflict if set on the same transaction for the same surface.
SurfaceFlinger will just apply setRelativeLayer since it's second in the
setClientState order. Instead, the code should only apply the
transaction that was requested last. This change removes the
setRelativeLayer flag if the setLayer flag is added and vice-versa.

Fixes: 131834423
Test: Steps from bug
Test: LayerTypeTransactionTest.SetLayerAndRelative
Change-Id: Ib14aa5c666e74455e5ccb26a0fea94699bcc5771

libs/gui/LayerState.cpp

diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp
index 075bb52..6066421 100644
--- a/libs/gui/LayerState.cpp
+++ b/libs/gui/LayerState.cpp
@@ -245,6 +245,7 @@
     }
     if (other.what & eLayerChanged) {
         what |= eLayerChanged;
+        what &= ~eRelativeLayerChanged;
         z = other.z;
     }
     if (other.what & eSizeChanged) {
@@ -303,6 +304,7 @@
     }
     if (other.what & eRelativeLayerChanged) {
         what |= eRelativeLayerChanged;
+        what &= ~eLayerChanged;
         z = other.z;
         relativeLayerHandle = other.relativeLayerHandle;
     }

libs/gui/SurfaceComposerClient.cpp

diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index 5d4367d..d059861 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -611,6 +611,7 @@
         return *this;
     }
     s->what |= layer_state_t::eLayerChanged;
+    s->what &= ~layer_state_t::eRelativeLayerChanged;
     s->z = z;
 
     registerSurfaceControlForCallback(sc);
@@ -624,6 +625,7 @@
         mStatus = BAD_INDEX;
     }
     s->what |= layer_state_t::eRelativeLayerChanged;
+    s->what &= ~layer_state_t::eLayerChanged;
     s->relativeLayerHandle = relativeTo;
     s->z = z;
 

services/surfaceflinger/tests/Transaction_test.cpp

diff --git a/services/surfaceflinger/tests/Transaction_test.cpp b/services/surfaceflinger/tests/Transaction_test.cpp
index d03c25d..ec1ac4b 100644
--- a/services/surfaceflinger/tests/Transaction_test.cpp
+++ b/services/surfaceflinger/tests/Transaction_test.cpp
@@ -1056,6 +1056,53 @@
     screenshot->expectColor(Rect(0, 0, 32, 32), Color::BLUE);
 }
 
+TEST_P(LayerTypeTransactionTest, SetLayerAndRelative) {
+    sp<SurfaceControl> parent =
+            LayerTransactionTest::createLayer("Parent", 0 /* buffer width */, 0 /* buffer height */,
+                                              ISurfaceComposerClient::eFXSurfaceColor);
+
+    sp<SurfaceControl> childLayer;
+    ASSERT_NO_FATAL_FAILURE(
+            childLayer = LayerTransactionTest::createLayer("childLayer", 0 /* buffer width */,
+                                                           0 /* buffer height */,
+                                                           ISurfaceComposerClient::eFXSurfaceColor,
+                                                           parent.get()));
+    Transaction()
+            .setColor(childLayer, half3{1.0f, 0.0f, 0.0f})
+            .setColor(parent, half3{0.0f, 0.0f, 0.0f})
+            .show(childLayer)
+            .show(parent)
+            .setCrop_legacy(parent, Rect(0, 0, mDisplayWidth, mDisplayHeight))
+            .setCrop_legacy(childLayer, Rect(0, 0, 20, 30))
+            .apply();
+
+    Transaction()
+            .setRelativeLayer(childLayer, parent->getHandle(), -1)
+            .setLayer(childLayer, 1)
+            .apply();
+
+    {
+        SCOPED_TRACE("setLayer above");
+        // Set layer should get applied and place the child above.
+        std::unique_ptr<ScreenCapture> screenshot;
+        ScreenCapture::captureScreen(&screenshot);
+        screenshot->expectColor(Rect(0, 0, 20, 30), Color::RED);
+    }
+
+    Transaction()
+            .setLayer(childLayer, 1)
+            .setRelativeLayer(childLayer, parent->getHandle(), -1)
+            .apply();
+
+    {
+        SCOPED_TRACE("setRelative below");
+        // Set relative layer should get applied and place the child below.
+        std::unique_ptr<ScreenCapture> screenshot;
+        ScreenCapture::captureScreen(&screenshot);
+        screenshot->expectColor(Rect(0, 0, 20, 30), Color::BLACK);
+    }
+}
+
 void LayerRenderTypeTransactionTest::setRelativeZGroupHelper(uint32_t layerType) {
     sp<SurfaceControl> layerR;
     sp<SurfaceControl> layerG;