Ignore local transforms when mirroring a partial hierarchy
When mirroring a partial hierarchy remove the local transform
on the mirrored root. This is necessary because we are placing
the mirrored root under a new parent layer and its original
position relative to the old parent is no longer relevant. This
fixes offsets when mirroring a freeform window.
Flag: com.android.graphics.surfaceflinger.flags.detached_mirror
Fixes: 337845753
Test: atest libsurfaceflinger_unittest
Change-Id: I0453026217f62868a4f6d1362cdc8187baf3f9b0
diff --git a/services/surfaceflinger/FrontEnd/LayerHierarchy.cpp b/services/surfaceflinger/FrontEnd/LayerHierarchy.cpp
index 821ac0c..0dcbb3c 100644
--- a/services/surfaceflinger/FrontEnd/LayerHierarchy.cpp
+++ b/services/surfaceflinger/FrontEnd/LayerHierarchy.cpp
@@ -153,7 +153,7 @@
out << prefix + (isLastChild ? "└─ " : "├─ ");
if (variant == LayerHierarchy::Variant::Relative) {
out << "(Relative) ";
- } else if (variant == LayerHierarchy::Variant::Mirror) {
+ } else if (LayerHierarchy::isMirror(variant)) {
if (!includeMirroredHierarchy) {
out << "(Mirroring) " << *mLayer << "\n" + prefix + " └─ ...";
return;
@@ -289,6 +289,12 @@
LayerHierarchy* mirror = getHierarchyFromId(mirrorId);
hierarchy->addChild(mirror, LayerHierarchy::Variant::Mirror);
}
+ if (FlagManager::getInstance().detached_mirror()) {
+ if (layer->layerIdToMirror != UNASSIGNED_LAYER_ID) {
+ LayerHierarchy* mirror = getHierarchyFromId(layer->layerIdToMirror);
+ hierarchy->addChild(mirror, LayerHierarchy::Variant::Detached_Mirror);
+ }
+ }
}
void LayerHierarchyBuilder::onLayerDestroyed(RequestedLayerState* layer) {
@@ -325,7 +331,7 @@
LayerHierarchy* hierarchy = getHierarchyFromId(layer->id);
auto it = hierarchy->mChildren.begin();
while (it != hierarchy->mChildren.end()) {
- if (it->second == LayerHierarchy::Variant::Mirror) {
+ if (LayerHierarchy::isMirror(it->second)) {
it = hierarchy->mChildren.erase(it);
} else {
it++;
@@ -335,6 +341,12 @@
for (uint32_t mirrorId : layer->mirrorIds) {
hierarchy->addChild(getHierarchyFromId(mirrorId), LayerHierarchy::Variant::Mirror);
}
+ if (FlagManager::getInstance().detached_mirror()) {
+ if (layer->layerIdToMirror != UNASSIGNED_LAYER_ID) {
+ hierarchy->addChild(getHierarchyFromId(layer->layerIdToMirror),
+ LayerHierarchy::Variant::Detached_Mirror);
+ }
+ }
}
void LayerHierarchyBuilder::doUpdate(
@@ -501,7 +513,7 @@
// stored to reset the id upon destruction.
traversalPath.id = layerId;
traversalPath.variant = variant;
- if (variant == LayerHierarchy::Variant::Mirror) {
+ if (LayerHierarchy::isMirror(variant)) {
traversalPath.mirrorRootIds.emplace_back(mParentPath.id);
} else if (variant == LayerHierarchy::Variant::Relative) {
if (std::find(traversalPath.relativeRootIds.begin(), traversalPath.relativeRootIds.end(),
@@ -516,7 +528,7 @@
LayerHierarchy::ScopedAddToTraversalPath::~ScopedAddToTraversalPath() {
// Reset the traversal id to its original parent state using the state that was saved in
// the constructor.
- if (mTraversalPath.variant == LayerHierarchy::Variant::Mirror) {
+ if (LayerHierarchy::isMirror(mTraversalPath.variant)) {
mTraversalPath.mirrorRootIds.pop_back();
} else if (mTraversalPath.variant == LayerHierarchy::Variant::Relative) {
mTraversalPath.relativeRootIds.pop_back();
diff --git a/services/surfaceflinger/FrontEnd/LayerHierarchy.h b/services/surfaceflinger/FrontEnd/LayerHierarchy.h
index a1c73c3..f62e758 100644
--- a/services/surfaceflinger/FrontEnd/LayerHierarchy.h
+++ b/services/surfaceflinger/FrontEnd/LayerHierarchy.h
@@ -35,6 +35,7 @@
// Detached - child of the parent but currently relative parented to another layer
// Relative - relative child of the parent
// Mirror - mirrored from another layer
+// Detached_Mirror - mirrored from another layer, ignoring local transform
//
// By representing the hierarchy as a graph, we can represent mirrored layer hierarchies without
// cloning the layer requested state. The mirrored hierarchy and its corresponding
@@ -43,13 +44,18 @@
class LayerHierarchy {
public:
enum Variant : uint32_t {
- Attached, // child of the parent
- Detached, // child of the parent but currently relative parented to another layer
- Relative, // relative child of the parent
- Mirror, // mirrored from another layer
+ Attached, // child of the parent
+ Detached, // child of the parent but currently relative parented to another layer
+ Relative, // relative child of the parent
+ Mirror, // mirrored from another layer
+ Detached_Mirror, // mirrored from another layer, ignoring local transform
ftl_first = Attached,
- ftl_last = Mirror,
+ ftl_last = Detached_Mirror,
};
+ static inline bool isMirror(Variant variant) {
+ return ((variant == Mirror) || (variant == Detached_Mirror));
+ }
+
// Represents a unique path to a node.
// The layer hierarchy is represented as a graph. Each node can be visited by multiple parents.
// This allows us to represent mirroring in an efficient way. See the example below:
diff --git a/services/surfaceflinger/FrontEnd/LayerLifecycleManager.cpp b/services/surfaceflinger/FrontEnd/LayerLifecycleManager.cpp
index 0983e7c..4b0618e 100644
--- a/services/surfaceflinger/FrontEnd/LayerLifecycleManager.cpp
+++ b/services/surfaceflinger/FrontEnd/LayerLifecycleManager.cpp
@@ -72,8 +72,10 @@
// Check if we are mirroring a single layer, and if so add it to the list of children
// to be mirrored.
layer.layerIdToMirror = linkLayer(layer.layerIdToMirror, layer.id);
- if (layer.layerIdToMirror != UNASSIGNED_LAYER_ID) {
- layer.mirrorIds.emplace_back(layer.layerIdToMirror);
+ if (!FlagManager::getInstance().detached_mirror()) {
+ if (layer.layerIdToMirror != UNASSIGNED_LAYER_ID) {
+ layer.mirrorIds.emplace_back(layer.layerIdToMirror);
+ }
}
}
layer.touchCropId = linkLayer(layer.touchCropId, layer.id);
diff --git a/services/surfaceflinger/FrontEnd/LayerSnapshot.cpp b/services/surfaceflinger/FrontEnd/LayerSnapshot.cpp
index ea06cf6..70e3c64 100644
--- a/services/surfaceflinger/FrontEnd/LayerSnapshot.cpp
+++ b/services/surfaceflinger/FrontEnd/LayerSnapshot.cpp
@@ -127,9 +127,8 @@
pid = state.ownerPid;
changes = RequestedLayerState::Changes::Created;
clientChanges = 0;
- mirrorRootPath = path.variant == LayerHierarchy::Variant::Mirror
- ? path
- : LayerHierarchy::TraversalPath::ROOT;
+ mirrorRootPath =
+ LayerHierarchy::isMirror(path.variant) ? path : LayerHierarchy::TraversalPath::ROOT;
reachablilty = LayerSnapshot::Reachablilty::Unreachable;
frameRateSelectionPriority = state.frameRateSelectionPriority;
layerMetadata = state.metadata;
@@ -472,13 +471,14 @@
geomContentCrop = requested.getBufferCrop();
}
- if (forceUpdate ||
- requested.what &
- (layer_state_t::eFlagsChanged | layer_state_t::eDestinationFrameChanged |
- layer_state_t::ePositionChanged | layer_state_t::eMatrixChanged |
- layer_state_t::eBufferTransformChanged |
- layer_state_t::eTransformToDisplayInverseChanged) ||
- requested.changes.test(RequestedLayerState::Changes::BufferSize) || displayChanges) {
+ if ((forceUpdate ||
+ requested.what &
+ (layer_state_t::eFlagsChanged | layer_state_t::eDestinationFrameChanged |
+ layer_state_t::ePositionChanged | layer_state_t::eMatrixChanged |
+ layer_state_t::eBufferTransformChanged |
+ layer_state_t::eTransformToDisplayInverseChanged) ||
+ requested.changes.test(RequestedLayerState::Changes::BufferSize) || displayChanges) &&
+ !ignoreLocalTransform) {
localTransform = requested.getTransform(displayRotationFlags);
localTransformInverse = localTransform.inverse();
}
diff --git a/services/surfaceflinger/FrontEnd/LayerSnapshot.h b/services/surfaceflinger/FrontEnd/LayerSnapshot.h
index 73ee22f..eef8dff 100644
--- a/services/surfaceflinger/FrontEnd/LayerSnapshot.h
+++ b/services/surfaceflinger/FrontEnd/LayerSnapshot.h
@@ -80,6 +80,9 @@
ui::Transform localTransformInverse;
gui::WindowInfo inputInfo;
ui::Transform localTransform;
+ // set to true if this snapshot will ignore local transforms. Used when the snapshot
+ // is a mirror root
+ bool ignoreLocalTransform;
gui::DropInputMode dropInputMode;
bool isTrustedOverlay;
gui::GameMode gameMode;
diff --git a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp
index 7daeefe..a2b5329 100644
--- a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp
+++ b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp
@@ -575,9 +575,11 @@
mSnapshots.emplace_back(std::make_unique<LayerSnapshot>(layer, path));
LayerSnapshot* snapshot = mSnapshots.back().get();
snapshot->globalZ = static_cast<size_t>(mSnapshots.size()) - 1;
- if (path.isClone() && path.variant != LayerHierarchy::Variant::Mirror) {
+ if (path.isClone() && !LayerHierarchy::isMirror(path.variant)) {
snapshot->mirrorRootPath = parentSnapshot.mirrorRootPath;
}
+ snapshot->ignoreLocalTransform =
+ path.isClone() && path.variant == LayerHierarchy::Variant::Detached_Mirror;
mPathToSnapshot[path] = snapshot;
mIdToSnapshots.emplace(path.id, snapshot);
diff --git a/services/surfaceflinger/LayerProtoHelper.cpp b/services/surfaceflinger/LayerProtoHelper.cpp
index aa6026e..513c943 100644
--- a/services/surfaceflinger/LayerProtoHelper.cpp
+++ b/services/surfaceflinger/LayerProtoHelper.cpp
@@ -334,7 +334,7 @@
variant);
frontend::LayerSnapshot* childSnapshot = getSnapshot(path, layer);
if (variant == Variant::Attached || variant == Variant::Detached ||
- variant == Variant::Mirror) {
+ frontend::LayerHierarchy::isMirror(variant)) {
mChildToParent[childSnapshot->uniqueSequence] = snapshot->uniqueSequence;
layerProto->add_children(childSnapshot->uniqueSequence);
} else if (variant == Variant::Relative) {
diff --git a/services/surfaceflinger/common/FlagManager.cpp b/services/surfaceflinger/common/FlagManager.cpp
index 929388f..f074b7d 100644
--- a/services/surfaceflinger/common/FlagManager.cpp
+++ b/services/surfaceflinger/common/FlagManager.cpp
@@ -143,6 +143,7 @@
DUMP_READ_ONLY_FLAG(latch_unsignaled_with_auto_refresh_changed);
DUMP_READ_ONLY_FLAG(deprecate_vsync_sf);
DUMP_READ_ONLY_FLAG(allow_n_vsyncs_in_targeter);
+ DUMP_READ_ONLY_FLAG(detached_mirror);
#undef DUMP_READ_ONLY_FLAG
#undef DUMP_SERVER_FLAG
@@ -236,6 +237,7 @@
FLAG_MANAGER_READ_ONLY_FLAG(latch_unsignaled_with_auto_refresh_changed, "");
FLAG_MANAGER_READ_ONLY_FLAG(deprecate_vsync_sf, "");
FLAG_MANAGER_READ_ONLY_FLAG(allow_n_vsyncs_in_targeter, "");
+FLAG_MANAGER_READ_ONLY_FLAG(detached_mirror, "");
/// Trunk stable server flags ///
FLAG_MANAGER_SERVER_FLAG(refresh_rate_overlay_on_external_display, "")
diff --git a/services/surfaceflinger/common/include/common/FlagManager.h b/services/surfaceflinger/common/include/common/FlagManager.h
index 4170c8a..0acf754 100644
--- a/services/surfaceflinger/common/include/common/FlagManager.h
+++ b/services/surfaceflinger/common/include/common/FlagManager.h
@@ -81,6 +81,7 @@
bool latch_unsignaled_with_auto_refresh_changed() const;
bool deprecate_vsync_sf() const;
bool allow_n_vsyncs_in_targeter() const;
+ bool detached_mirror() const;
protected:
// overridden for unit tests
diff --git a/services/surfaceflinger/surfaceflinger_flags_new.aconfig b/services/surfaceflinger/surfaceflinger_flags_new.aconfig
index 4a60987..4d3195d 100644
--- a/services/surfaceflinger/surfaceflinger_flags_new.aconfig
+++ b/services/surfaceflinger/surfaceflinger_flags_new.aconfig
@@ -32,6 +32,17 @@
}
} # deprecate_vsync_sf
+ flag {
+ name: "detached_mirror"
+ namespace: "window_surfaces"
+ description: "Ignore local transform when mirroring a partial hierarchy"
+ bug: "337845753"
+ is_fixed_read_only: true
+ metadata {
+ purpose: PURPOSE_BUGFIX
+ }
+} # deprecate_vsync_sf
+
flag {
name: "frame_rate_category_mrr"
namespace: "core_graphics"
diff --git a/services/surfaceflinger/tests/unittests/LayerHierarchyTest.h b/services/surfaceflinger/tests/unittests/LayerHierarchyTest.h
index e8e7667..fd15eef 100644
--- a/services/surfaceflinger/tests/unittests/LayerHierarchyTest.h
+++ b/services/surfaceflinger/tests/unittests/LayerHierarchyTest.h
@@ -155,6 +155,17 @@
mLifecycleManager.applyTransactions(transactions);
}
+ void setPosition(uint32_t id, float x, float y) {
+ std::vector<TransactionState> transactions;
+ transactions.emplace_back();
+ transactions.back().states.push_back({});
+ transactions.back().states.front().state.what = layer_state_t::ePositionChanged;
+ transactions.back().states.front().state.x = x;
+ transactions.back().states.front().state.y = y;
+ transactions.back().states.front().layerId = id;
+ mLifecycleManager.applyTransactions(transactions);
+ }
+
virtual void mirrorLayer(uint32_t id, uint32_t parentId, uint32_t layerIdToMirror) {
std::vector<std::unique_ptr<RequestedLayerState>> layers;
layers.emplace_back(std::make_unique<RequestedLayerState>(
diff --git a/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp b/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp
index ae9a89c..7c6cff0 100644
--- a/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp
+++ b/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp
@@ -1302,4 +1302,33 @@
EXPECT_EQ(getSnapshot(1221)->inputInfo.canOccludePresentation, true);
}
+TEST_F(LayerSnapshotTest, mirroredHierarchyIgnoresLocalTransform) {
+ SET_FLAG_FOR_TEST(flags::detached_mirror, true);
+ reparentLayer(12, UNASSIGNED_LAYER_ID);
+ setPosition(11, 2, 20);
+ setPosition(111, 20, 200);
+ mirrorLayer(/*layer*/ 14, /*parent*/ 1, /*layerToMirror*/ 11);
+ std::vector<uint32_t> expected = {1, 11, 111, 13, 14, 11, 111, 2};
+ UPDATE_AND_VERIFY(mSnapshotBuilder, expected);
+
+ // mirror root has no position set
+ EXPECT_EQ(getSnapshot({.id = 11, .mirrorRootIds = 14u})->localTransform.tx(), 0);
+ EXPECT_EQ(getSnapshot({.id = 11, .mirrorRootIds = 14u})->localTransform.ty(), 0);
+ // original root still has a position
+ EXPECT_EQ(getSnapshot({.id = 11})->localTransform.tx(), 2);
+ EXPECT_EQ(getSnapshot({.id = 11})->localTransform.ty(), 20);
+
+ // mirror child still has the correct position
+ EXPECT_EQ(getSnapshot({.id = 111, .mirrorRootIds = 14u})->localTransform.tx(), 20);
+ EXPECT_EQ(getSnapshot({.id = 111, .mirrorRootIds = 14u})->localTransform.ty(), 200);
+ EXPECT_EQ(getSnapshot({.id = 111, .mirrorRootIds = 14u})->geomLayerTransform.tx(), 20);
+ EXPECT_EQ(getSnapshot({.id = 111, .mirrorRootIds = 14u})->geomLayerTransform.ty(), 200);
+
+ // original child still has the correct position including its parent's position
+ EXPECT_EQ(getSnapshot({.id = 111})->localTransform.tx(), 20);
+ EXPECT_EQ(getSnapshot({.id = 111})->localTransform.ty(), 200);
+ EXPECT_EQ(getSnapshot({.id = 111})->geomLayerTransform.tx(), 22);
+ EXPECT_EQ(getSnapshot({.id = 111})->geomLayerTransform.ty(), 220);
+}
+
} // namespace android::surfaceflinger::frontend