[sf-newfe] Fix snapshot paths for recursive mirror paths
If we are mirroring a hierarchy that already contains a mirror
then we need to track all the mirror roots inorder to have
a unique path.
Test: presubmit
Test: can screenrecord magnifier
Test: dumpsys SurfaceFlinger --frontend
Bug: 238781169
Change-Id: Id2272981f6f6244f41328428b73184bccf5d1888
diff --git a/services/surfaceflinger/FrontEnd/LayerHierarchy.cpp b/services/surfaceflinger/FrontEnd/LayerHierarchy.cpp
index 962dc09..1e5a6fb 100644
--- a/services/surfaceflinger/FrontEnd/LayerHierarchy.cpp
+++ b/services/surfaceflinger/FrontEnd/LayerHierarchy.cpp
@@ -145,7 +145,8 @@
}
void LayerHierarchy::dump(std::ostream& out, const std::string& prefix,
- LayerHierarchy::Variant variant, bool isLastChild) const {
+ LayerHierarchy::Variant variant, bool isLastChild,
+ bool includeMirroredHierarchy) const {
if (!mLayer) {
out << " ROOT";
} else {
@@ -153,8 +154,11 @@
if (variant == LayerHierarchy::Variant::Relative) {
out << "(Relative) ";
} else if (variant == LayerHierarchy::Variant::Mirror) {
- out << "(Mirroring) " << *mLayer << "\n" + prefix + " └─ ...";
- return;
+ if (!includeMirroredHierarchy) {
+ out << "(Mirroring) " << *mLayer << "\n" + prefix + " └─ ...";
+ return;
+ }
+ out << "(Mirroring) ";
}
out << *mLayer;
}
@@ -168,7 +172,7 @@
childPrefix += (isLastChild ? " " : "│ ");
}
out << "\n";
- child->dump(out, childPrefix, childVariant, lastChild);
+ child->dump(out, childPrefix, childVariant, lastChild, includeMirroredHierarchy);
}
return;
}
@@ -435,8 +439,11 @@
std::stringstream ss;
ss << "TraversalPath{.id = " << id;
- if (mirrorRootId != UNASSIGNED_LAYER_ID) {
- ss << ", .mirrorRootId=" << mirrorRootId;
+ if (!mirrorRootIds.empty()) {
+ ss << ", .mirrorRootIds=";
+ for (auto rootId : mirrorRootIds) {
+ ss << rootId << ",";
+ }
}
if (!relativeRootIds.empty()) {
@@ -453,13 +460,6 @@
return ss.str();
}
-LayerHierarchy::TraversalPath LayerHierarchy::TraversalPath::getMirrorRoot() const {
- LLOG_ALWAYS_FATAL_WITH_TRACE_IF(!isClone(), "Cannot get mirror root of a non cloned node");
- TraversalPath mirrorRootPath = *this;
- mirrorRootPath.id = mirrorRootId;
- return mirrorRootPath;
-}
-
// Helper class to update a passed in TraversalPath when visiting a child. When the object goes out
// of scope the TraversalPath is reset to its original state.
LayerHierarchy::ScopedAddToTraversalPath::ScopedAddToTraversalPath(TraversalPath& traversalPath,
@@ -471,7 +471,7 @@
traversalPath.id = layerId;
traversalPath.variant = variant;
if (variant == LayerHierarchy::Variant::Mirror) {
- traversalPath.mirrorRootId = mParentPath.id;
+ traversalPath.mirrorRootIds.emplace_back(mParentPath.id);
} else if (variant == LayerHierarchy::Variant::Relative) {
if (std::find(traversalPath.relativeRootIds.begin(), traversalPath.relativeRootIds.end(),
layerId) != traversalPath.relativeRootIds.end()) {
@@ -486,7 +486,7 @@
// Reset the traversal id to its original parent state using the state that was saved in
// the constructor.
if (mTraversalPath.variant == LayerHierarchy::Variant::Mirror) {
- mTraversalPath.mirrorRootId = mParentPath.mirrorRootId;
+ 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 1e48387..ba2e262 100644
--- a/services/surfaceflinger/FrontEnd/LayerHierarchy.h
+++ b/services/surfaceflinger/FrontEnd/LayerHierarchy.h
@@ -57,25 +57,25 @@
// ├─ B {Traversal path id = 2}
// │ ├─ C {Traversal path id = 3}
// │ ├─ D {Traversal path id = 4}
- // │ └─ E {Traversal path id = 5}
- // ├─ F (Mirrors B) {Traversal path id = 6}
- // └─ G (Mirrors F) {Traversal path id = 7}
+ // │ └─ E (Mirrors C) {Traversal path id = 5}
+ // └─ F (Mirrors B) {Traversal path id = 6}
//
- // C, D and E can be traversed via B or via F then B or via G then F then B.
+ // C can be traversed via B or E or F and or via F then E.
// Depending on how the node is reached, its properties such as geometry or visibility might be
// different. And we can uniquely identify the node by keeping track of the nodes leading up to
// it. But to be more efficient we only need to track the nodes id and the top mirror root path.
// So C for example, would have the following unique traversal paths:
// - {Traversal path id = 3}
- // - {Traversal path id = 3, mirrorRootId = 6}
- // - {Traversal path id = 3, mirrorRootId = 7}
+ // - {Traversal path id = 3, mirrorRootIds = 5}
+ // - {Traversal path id = 3, mirrorRootIds = 6}
+ // - {Traversal path id = 3, mirrorRootIds = 6, 5}
struct TraversalPath {
uint32_t id;
LayerHierarchy::Variant variant;
// Mirrored layers can have a different geometry than their parents so we need to track
// the mirror roots in the traversal.
- uint32_t mirrorRootId = UNASSIGNED_LAYER_ID;
+ ftl::SmallVector<uint32_t, 5> mirrorRootIds;
// Relative layers can be visited twice, once by their parent and then once again by
// their relative parent. We keep track of the roots here to detect any loops in the
// hierarchy. If a relative root already exists in the list while building the
@@ -93,11 +93,10 @@
// Returns true if the node or its parents are not Detached.
bool isAttached() const { return !detached; }
// Returns true if the node is a clone.
- bool isClone() const { return mirrorRootId != UNASSIGNED_LAYER_ID; }
- TraversalPath getMirrorRoot() const;
+ bool isClone() const { return !mirrorRootIds.empty(); }
bool operator==(const TraversalPath& other) const {
- return id == other.id && mirrorRootId == other.mirrorRootId;
+ return id == other.id && mirrorRootIds == other.mirrorRootIds;
}
std::string toString() const;
@@ -107,8 +106,8 @@
struct TraversalPathHash {
std::size_t operator()(const LayerHierarchy::TraversalPath& key) const {
uint32_t hashCode = key.id * 31;
- if (key.mirrorRootId != UNASSIGNED_LAYER_ID) {
- hashCode += key.mirrorRootId * 31;
+ for (uint32_t mirrorRootId : key.mirrorRootIds) {
+ hashCode += mirrorRootId * 31;
}
return std::hash<size_t>{}(hashCode);
}
@@ -158,9 +157,17 @@
const LayerHierarchy* getParent() const;
friend std::ostream& operator<<(std::ostream& os, const LayerHierarchy& obj) {
std::string prefix = " ";
- obj.dump(os, prefix, LayerHierarchy::Variant::Attached, /*isLastChild=*/false);
+ obj.dump(os, prefix, LayerHierarchy::Variant::Attached, /*isLastChild=*/false,
+ /*includeMirroredHierarchy*/ false);
return os;
}
+ std::string dump() const {
+ std::string prefix = " ";
+ std::ostringstream os;
+ dump(os, prefix, LayerHierarchy::Variant::Attached, /*isLastChild=*/false,
+ /*includeMirroredHierarchy*/ true);
+ return os.str();
+ }
std::string getDebugStringShort() const;
// Traverse the hierarchy and return true if loops are found. The outInvalidRelativeRoot
@@ -178,7 +185,7 @@
void traverseInZOrder(const Visitor& visitor, LayerHierarchy::TraversalPath& parent) const;
void traverse(const Visitor& visitor, LayerHierarchy::TraversalPath& parent) const;
void dump(std::ostream& out, const std::string& prefix, LayerHierarchy::Variant variant,
- bool isLastChild) const;
+ bool isLastChild, bool includeMirroredHierarchy) const;
const RequestedLayerState* mLayer;
LayerHierarchy* mParent = nullptr;
diff --git a/services/surfaceflinger/FrontEnd/LayerSnapshot.cpp b/services/surfaceflinger/FrontEnd/LayerSnapshot.cpp
index 80bedf4..899d2de 100644
--- a/services/surfaceflinger/FrontEnd/LayerSnapshot.cpp
+++ b/services/surfaceflinger/FrontEnd/LayerSnapshot.cpp
@@ -294,8 +294,11 @@
std::ostream& operator<<(std::ostream& out, const LayerSnapshot& obj) {
out << "Layer [" << obj.path.id;
- if (obj.path.mirrorRootId != UNASSIGNED_LAYER_ID) {
- out << " mirrored from " << obj.path.mirrorRootId;
+ if (!obj.path.mirrorRootIds.empty()) {
+ out << " mirrored from ";
+ for (auto rootId : obj.path.mirrorRootIds) {
+ out << rootId << ",";
+ }
}
out << "] " << obj.name << "\n " << (obj.isVisible ? "visible" : "invisible")
<< " reason=" << obj.getIsVisibleReason();
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 9a5173b..2bbfa42 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -2648,7 +2648,7 @@
void Layer::setInitialValuesForClone(const sp<Layer>& clonedFrom, uint32_t mirrorRootId) {
mSnapshot->path.id = clonedFrom->getSequence();
- mSnapshot->path.mirrorRootId = mirrorRootId;
+ mSnapshot->path.mirrorRootIds.emplace_back(mirrorRootId);
cloneDrawingState(clonedFrom.get());
mClonedFrom = clonedFrom;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 517dc96..92e4881 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -5823,6 +5823,7 @@
{"--timestats"s, protoDumper(&SurfaceFlinger::dumpTimeStats)},
{"--vsync"s, dumper(&SurfaceFlinger::dumpVsync)},
{"--wide-color"s, dumper(&SurfaceFlinger::dumpWideColorInfo)},
+ {"--frontend"s, dumper(&SurfaceFlinger::dumpFrontEnd)},
};
const auto flag = args.empty() ? ""s : std::string(String8(args[0]));
@@ -6132,6 +6133,38 @@
}
}
+void SurfaceFlinger::dumpFrontEnd(std::string& result) {
+ mScheduler
+ ->schedule([&]() FTL_FAKE_GUARD(mStateLock) FTL_FAKE_GUARD(kMainThreadContext) {
+ std::ostringstream out;
+ out << "\nComposition list\n";
+ ui::LayerStack lastPrintedLayerStackHeader = ui::INVALID_LAYER_STACK;
+ for (const auto& snapshot : mLayerSnapshotBuilder.getSnapshots()) {
+ if (lastPrintedLayerStackHeader != snapshot->outputFilter.layerStack) {
+ lastPrintedLayerStackHeader = snapshot->outputFilter.layerStack;
+ out << "LayerStack=" << lastPrintedLayerStackHeader.id << "\n";
+ }
+ out << " " << *snapshot << "\n";
+ }
+
+ out << "\nInput list\n";
+ lastPrintedLayerStackHeader = ui::INVALID_LAYER_STACK;
+ mLayerSnapshotBuilder.forEachInputSnapshot(
+ [&](const frontend::LayerSnapshot& snapshot) {
+ if (lastPrintedLayerStackHeader != snapshot.outputFilter.layerStack) {
+ lastPrintedLayerStackHeader = snapshot.outputFilter.layerStack;
+ out << "LayerStack=" << lastPrintedLayerStackHeader.id << "\n";
+ }
+ out << " " << snapshot << "\n";
+ });
+
+ out << "\nLayer Hierarchy\n"
+ << mLayerHierarchyBuilder.getHierarchy().dump() << "\n\n";
+ result.append(out.str());
+ })
+ .get();
+}
+
perfetto::protos::LayersProto SurfaceFlinger::dumpDrawingStateProto(uint32_t traceFlags) const {
std::unordered_set<uint64_t> stackIdsToSkip;
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 59b1172..8fa5de7 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -1090,6 +1090,7 @@
void dumpRawDisplayIdentificationData(const DumpArgs&, std::string& result) const;
void dumpWideColorInfo(std::string& result) const REQUIRES(mStateLock);
void dumpHdrInfo(std::string& result) const REQUIRES(mStateLock);
+ void dumpFrontEnd(std::string& result);
perfetto::protos::LayersProto dumpDrawingStateProto(uint32_t traceFlags) const;
void dumpOffscreenLayersProto(perfetto::protos::LayersProto& layersProto,
diff --git a/services/surfaceflinger/tests/unittests/LayerHierarchyTest.cpp b/services/surfaceflinger/tests/unittests/LayerHierarchyTest.cpp
index e4f49e8..95f1940 100644
--- a/services/surfaceflinger/tests/unittests/LayerHierarchyTest.cpp
+++ b/services/surfaceflinger/tests/unittests/LayerHierarchyTest.cpp
@@ -736,4 +736,22 @@
EXPECT_EQ(getTraversalPath(hierarchyBuilder.getOffscreenHierarchy()), expected);
}
+TEST_F(LayerHierarchyTest, canMirrorDisplayWithMirrors) {
+ LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+ reparentLayer(12, UNASSIGNED_LAYER_ID);
+ mirrorLayer(/*layer*/ 14, /*parent*/ 1, /*layerToMirror*/ 11);
+ UPDATE_AND_VERIFY(hierarchyBuilder);
+
+ createDisplayMirrorLayer(3, ui::LayerStack::fromValue(0));
+ setLayerStack(3, 1);
+ UPDATE_AND_VERIFY(hierarchyBuilder);
+
+ std::vector<uint32_t> expected = {1, 11, 111, 13, 14, 11, 111, 2, 3,
+ 1, 11, 111, 13, 14, 11, 111, 2};
+ EXPECT_EQ(getTraversalPath(hierarchyBuilder.getHierarchy()), expected);
+ EXPECT_EQ(getTraversalPathInZOrder(hierarchyBuilder.getHierarchy()), expected);
+ expected = {12, 121, 122, 1221};
+ EXPECT_EQ(getTraversalPath(hierarchyBuilder.getOffscreenHierarchy()), expected);
+}
+
} // namespace android::surfaceflinger::frontend
diff --git a/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp b/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp
index 2353480..1227b99 100644
--- a/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp
+++ b/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp
@@ -410,8 +410,8 @@
std::vector<uint32_t> expected = {1, 11, 111, 13, 2, 3, 1, 11, 111,
13, 2, 4, 1, 11, 111, 13, 2};
UPDATE_AND_VERIFY(mSnapshotBuilder, expected);
- EXPECT_EQ(getSnapshot({.id = 111, .mirrorRootId = 3})->outputFilter.layerStack.id, 3u);
- EXPECT_EQ(getSnapshot({.id = 111, .mirrorRootId = 4})->outputFilter.layerStack.id, 4u);
+ EXPECT_EQ(getSnapshot({.id = 111, .mirrorRootIds = 3u})->outputFilter.layerStack.id, 3u);
+ EXPECT_EQ(getSnapshot({.id = 111, .mirrorRootIds = 4u})->outputFilter.layerStack.id, 4u);
}
// ROOT (DISPLAY 0)
@@ -435,7 +435,7 @@
UPDATE_AND_VERIFY(mSnapshotBuilder, expected);
EXPECT_TRUE(getSnapshot({.id = 111})->inputInfo.touchableRegion.hasSameRects(touch));
Region touchCroppedByMirrorRoot{Rect{0, 0, 50, 50}};
- EXPECT_TRUE(getSnapshot({.id = 111, .mirrorRootId = 3})
+ EXPECT_TRUE(getSnapshot({.id = 111, .mirrorRootIds = 3u})
->inputInfo.touchableRegion.hasSameRects(touchCroppedByMirrorRoot));
}
@@ -462,6 +462,21 @@
EXPECT_EQ(startingNumSnapshots, mSnapshotBuilder.getSnapshots().size());
}
+TEST_F(LayerSnapshotTest, canMirrorDisplayWithMirrors) {
+ reparentLayer(12, UNASSIGNED_LAYER_ID);
+ 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);
+
+ createDisplayMirrorLayer(3, ui::LayerStack::fromValue(0));
+ setLayerStack(3, 3);
+ expected = {1, 11, 111, 13, 14, 11, 111, 2, 3, 1, 11, 111, 13, 14, 11, 111, 2};
+ UPDATE_AND_VERIFY(mSnapshotBuilder, expected);
+ EXPECT_EQ(getSnapshot({.id = 11, .mirrorRootIds = 14u})->outputFilter.layerStack.id, 0u);
+ EXPECT_EQ(getSnapshot({.id = 11, .mirrorRootIds = 3u})->outputFilter.layerStack.id, 3u);
+ EXPECT_EQ(getSnapshot({.id = 11, .mirrorRootIds = 3u, 14u})->outputFilter.layerStack.id, 3u);
+}
+
// Rel z doesn't create duplicate snapshots but this is for completeness
TEST_F(LayerSnapshotTest, cleanUpUnreachableSnapshotsAfterRelZ) {
size_t startingNumSnapshots = mSnapshotBuilder.getSnapshots().size();