Re-parent invoked on child instead of on parent.
The function to re-parent an individual child is now invoked on
the child instead of the parent. This ensures the child ends up with
the last parent set if multiple reparent requests are made in the same
transaction.
This also allows adding a parent to a layer that didn't have one
previously.
Test: Transaction_test -> Reparent, ReparentToNoParent,
ReparentFromNoParent
Change-Id: Idab429eb2dca5a4ae1b020a5a7629d719dd4d995
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 9435a18..fd30e16 100755
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -2625,8 +2625,8 @@
return true;
}
-bool Layer::reparentChild(const sp<IBinder>& newParentHandle, const sp<IBinder>& childHandle) {
- if (newParentHandle == nullptr || childHandle == nullptr) {
+bool Layer::reparent(const sp<IBinder>& newParentHandle) {
+ if (newParentHandle == nullptr) {
return false;
}
@@ -2637,29 +2637,19 @@
return false;
}
- handle = static_cast<Handle*>(childHandle.get());
- sp<Layer> child = handle->owner.promote();
- if (child == nullptr) {
- ALOGE("Unable to promote child Layer handle");
- return false;
+ sp<Layer> parent = getParent();
+ if (parent != nullptr) {
+ parent->removeChild(this);
}
+ newParent->addChild(this);
- if (mCurrentChildren.indexOf(child) < 0) {
- ALOGE("Child layer is not child of current layer");
- return false;
- }
-
- sp<Client> parentClient(mClientRef.promote());
- sp<Client> childClient(child->mClientRef.promote());
+ sp<Client> client(mClientRef.promote());
sp<Client> newParentClient(newParent->mClientRef.promote());
- if (parentClient != childClient || childClient != newParentClient) {
- ALOGE("Current layer, child layer, and new parent layer must have the same client");
- return false;
+ if (client != newParentClient) {
+ client->setParentLayer(newParent);
}
- newParent->addChild(child);
- mCurrentChildren.remove(child);
return true;
}
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index f94833b..e7ece45 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -241,7 +241,7 @@
bool setOverrideScalingMode(int32_t overrideScalingMode);
void setInfo(uint32_t type, uint32_t appId);
bool reparentChildren(const sp<IBinder>& layer);
- bool reparentChild(const sp<IBinder>& newParentHandle, const sp<IBinder>& childHandle);
+ bool reparent(const sp<IBinder>& newParentHandle);
bool detachChildren();
// If we have received a new buffer this frame, we will pass its surface
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index cae4dea..6ee14fe 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3121,10 +3121,8 @@
// We don't trigger a traversal here because if no other state is
// changed, we don't want this to cause any more work
}
- // Always re-parent the children that explicitly requested to get
- // re-parented before the general re-parent of all children.
- if (what & layer_state_t::eReparentChild) {
- if (layer->reparentChild(s.parentHandleForChild, s.childHandle)) {
+ if (what & layer_state_t::eReparent) {
+ if (layer->reparent(s.parentHandleForChild)) {
flags |= eTransactionNeeded|eTraversalNeeded;
}
}
diff --git a/services/surfaceflinger/SurfaceFlinger_hwc1.cpp b/services/surfaceflinger/SurfaceFlinger_hwc1.cpp
index 71aa52d..1cf6ce3 100644
--- a/services/surfaceflinger/SurfaceFlinger_hwc1.cpp
+++ b/services/surfaceflinger/SurfaceFlinger_hwc1.cpp
@@ -2679,10 +2679,8 @@
// We don't trigger a traversal here because if no other state is
// changed, we don't want this to cause any more work
}
- // Always re-parent the children that explicitly requested to get
- // re-parented before the general re-parent of all children.
- if (what & layer_state_t::eReparentChild) {
- if (layer->reparentChild(s.parentHandleForChild, s.childHandle)) {
+ if (what & layer_state_t::eReparent) {
+ if (layer->reparent(s.parentHandleForChild)) {
flags |= eTransactionNeeded|eTraversalNeeded;
}
}
diff --git a/services/surfaceflinger/tests/Transaction_test.cpp b/services/surfaceflinger/tests/Transaction_test.cpp
index dea6503..2119492 100644
--- a/services/surfaceflinger/tests/Transaction_test.cpp
+++ b/services/surfaceflinger/tests/Transaction_test.cpp
@@ -1169,7 +1169,7 @@
fillSurfaceRGBA8(mFGSurfaceControl, 0, 255, 0);
}
-TEST_F(ChildLayerTest, ReparentChild) {
+TEST_F(ChildLayerTest, Reparent) {
SurfaceComposerClient::openGlobalTransaction();
mChild->show();
mChild->setPosition(10, 10);
@@ -1185,7 +1185,7 @@
// And 10 more pixels we should be back to the foreground surface
mCapture->expectFGColor(84, 84);
}
- mFGSurfaceControl->reparentChild(mBGSurfaceControl->getHandle(), mChild->getHandle());
+ mChild->reparent(mBGSurfaceControl->getHandle());
{
ScreenCapture::captureScreen(&mCapture);
mCapture->expectFGColor(64, 64);
@@ -1198,6 +1198,69 @@
}
}
+TEST_F(ChildLayerTest, ReparentToNoParent) {
+ SurfaceComposerClient::openGlobalTransaction();
+ mChild->show();
+ mChild->setPosition(10, 10);
+ mFGSurfaceControl->setPosition(64, 64);
+ SurfaceComposerClient::closeGlobalTransaction(true);
+
+ {
+ ScreenCapture::captureScreen(&mCapture);
+ // Top left of foreground must now be visible
+ mCapture->expectFGColor(64, 64);
+ // But 10 pixels in we should see the child surface
+ mCapture->expectChildColor(74, 74);
+ // And 10 more pixels we should be back to the foreground surface
+ mCapture->expectFGColor(84, 84);
+ }
+ mChild->reparent(nullptr);
+ {
+ ScreenCapture::captureScreen(&mCapture);
+ // Nothing should have changed.
+ mCapture->expectFGColor(64, 64);
+ mCapture->expectChildColor(74, 74);
+ mCapture->expectFGColor(84, 84);
+ }
+}
+
+TEST_F(ChildLayerTest, ReparentFromNoParent) {
+ sp<SurfaceControl> newSurface = mComposerClient->createSurface(
+ String8("New Surface"), 10, 10, PIXEL_FORMAT_RGBA_8888, 0);
+ ASSERT_TRUE(newSurface != NULL);
+ ASSERT_TRUE(newSurface->isValid());
+
+ fillSurfaceRGBA8(newSurface, 63, 195, 63);
+ SurfaceComposerClient::openGlobalTransaction();
+ mChild->hide();
+ newSurface->show();
+ newSurface->setPosition(10, 10);
+ newSurface->setLayer(INT32_MAX-2);
+ mFGSurfaceControl->setPosition(64, 64);
+ SurfaceComposerClient::closeGlobalTransaction(true);
+
+ {
+ ScreenCapture::captureScreen(&mCapture);
+ // Top left of foreground must now be visible
+ mCapture->expectFGColor(64, 64);
+ // At 10, 10 we should see the new surface
+ mCapture->checkPixel(10, 10, 63, 195, 63);
+ }
+
+ SurfaceComposerClient::openGlobalTransaction();
+ newSurface->reparent(mFGSurfaceControl->getHandle());
+ SurfaceComposerClient::closeGlobalTransaction(true);
+
+ {
+ ScreenCapture::captureScreen(&mCapture);
+ // newSurface will now be a child of mFGSurface so it will be 10, 10 offset from
+ // mFGSurface, putting it at 74, 74.
+ mCapture->expectFGColor(64, 64);
+ mCapture->checkPixel(74, 74, 63, 195, 63);
+ mCapture->expectFGColor(84, 84);
+ }
+}
+
TEST_F(ChildLayerTest, NestedChildren) {
sp<SurfaceControl> grandchild = mComposerClient->createSurface(
String8("Grandchild surface"),