Remove ScopedAddToTraversalPath
ScopedAddToTraversalPath is an RAII wrapper that copies the existing traversal path, modifies it, and then restores the modified properties from the copy when deleted. It's simpler to make the child path a copy and not modify the parent.
Bug: 403312802
Flag: EXEMPT refactor
Test: presubmits
Change-Id: I8f06ed557f29444be6df51c1c8ea60958a82ee95
diff --git a/services/surfaceflinger/FrontEnd/LayerHierarchy.cpp b/services/surfaceflinger/FrontEnd/LayerHierarchy.cpp
index da536b6..00ec863 100644
--- a/services/surfaceflinger/FrontEnd/LayerHierarchy.cpp
+++ b/services/surfaceflinger/FrontEnd/LayerHierarchy.cpp
@@ -54,7 +54,8 @@
mChildren = hierarchy.mChildren;
}
-void LayerHierarchy::traverse(const Visitor& visitor, LayerHierarchy::TraversalPath& traversalPath,
+void LayerHierarchy::traverse(const Visitor& visitor,
+ const LayerHierarchy::TraversalPath& traversalPath,
uint32_t depth) const {
LLOG_ALWAYS_FATAL_WITH_TRACE_IF(depth > 50,
"Cycle detected in LayerHierarchy::traverse. See "
@@ -70,14 +71,13 @@
LLOG_ALWAYS_FATAL_WITH_TRACE_IF(traversalPath.hasRelZLoop(), "Found relative z loop layerId:%d",
traversalPath.invalidRelativeRootId);
for (auto& [child, childVariant] : mChildren) {
- ScopedAddToTraversalPath addChildToTraversalPath(traversalPath, child->mLayer->id,
- childVariant);
- child->traverse(visitor, traversalPath, depth + 1);
+ child->traverse(visitor, traversalPath.makeChild(child->mLayer->id, childVariant),
+ depth + 1);
}
}
void LayerHierarchy::traverseInZOrder(const Visitor& visitor,
- LayerHierarchy::TraversalPath& traversalPath) const {
+ const LayerHierarchy::TraversalPath& traversalPath) const {
bool traverseThisLayer = (mLayer != nullptr);
for (auto it = mChildren.begin(); it < mChildren.end(); it++) {
auto& [child, childVariant] = *it;
@@ -91,9 +91,7 @@
if (childVariant == LayerHierarchy::Variant::Detached) {
continue;
}
- ScopedAddToTraversalPath addChildToTraversalPath(traversalPath, child->mLayer->id,
- childVariant);
- child->traverseInZOrder(visitor, traversalPath);
+ child->traverseInZOrder(visitor, traversalPath.makeChild(child->mLayer->id, childVariant));
}
if (traverseThisLayer) {
@@ -568,42 +566,23 @@
return ss.str();
}
-// 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,
- uint32_t layerId,
- LayerHierarchy::Variant variant)
- : mTraversalPath(traversalPath), mParentPath(traversalPath) {
- // Update the traversal id with the child layer id and variant. Parent id and variant are
- // stored to reset the id upon destruction.
- traversalPath.id = layerId;
- traversalPath.variant = variant;
+LayerHierarchy::TraversalPath LayerHierarchy::TraversalPath::makeChild(
+ uint32_t layerId, LayerHierarchy::Variant variant) const {
+ TraversalPath child{*this};
+ child.id = layerId;
+ child.variant = variant;
if (LayerHierarchy::isMirror(variant)) {
- traversalPath.mirrorRootIds.emplace_back(mParentPath.id);
+ child.mirrorRootIds.emplace_back(id);
} else if (variant == LayerHierarchy::Variant::Relative) {
- if (std::find(traversalPath.relativeRootIds.begin(), traversalPath.relativeRootIds.end(),
- layerId) != traversalPath.relativeRootIds.end()) {
- traversalPath.invalidRelativeRootId = layerId;
+ if (std::find(relativeRootIds.begin(), relativeRootIds.end(), layerId) !=
+ relativeRootIds.end()) {
+ child.invalidRelativeRootId = layerId;
}
- traversalPath.relativeRootIds.emplace_back(layerId);
+ child.relativeRootIds.emplace_back(layerId);
} else if (variant == LayerHierarchy::Variant::Detached) {
- traversalPath.detached = true;
+ child.detached = true;
}
-}
-LayerHierarchy::ScopedAddToTraversalPath::~ScopedAddToTraversalPath() {
- // Reset the traversal id to its original parent state using the state that was saved in
- // the constructor.
- if (LayerHierarchy::isMirror(mTraversalPath.variant)) {
- mTraversalPath.mirrorRootIds.pop_back();
- } else if (mTraversalPath.variant == LayerHierarchy::Variant::Relative) {
- mTraversalPath.relativeRootIds.pop_back();
- }
- if (mTraversalPath.invalidRelativeRootId == mTraversalPath.id) {
- mTraversalPath.invalidRelativeRootId = UNASSIGNED_LAYER_ID;
- }
- mTraversalPath.id = mParentPath.id;
- mTraversalPath.variant = mParentPath.variant;
- mTraversalPath.detached = mParentPath.detached;
+ return child;
}
} // namespace android::surfaceflinger::frontend
diff --git a/services/surfaceflinger/FrontEnd/LayerHierarchy.h b/services/surfaceflinger/FrontEnd/LayerHierarchy.h
index 4fdbae1..c8c6b4d 100644
--- a/services/surfaceflinger/FrontEnd/LayerHierarchy.h
+++ b/services/surfaceflinger/FrontEnd/LayerHierarchy.h
@@ -104,6 +104,8 @@
TraversalPath getClonedFrom() const { return {.id = id, .variant = variant}; }
+ TraversalPath makeChild(uint32_t layerId, LayerHierarchy::Variant variant) const;
+
bool operator==(const TraversalPath& other) const {
return id == other.id && mirrorRootIds == other.mirrorRootIds;
}
@@ -122,18 +124,6 @@
}
};
- // Helper class to add nodes to an existing traversal id and removes the
- // node when it goes out of scope.
- class ScopedAddToTraversalPath {
- public:
- ScopedAddToTraversalPath(TraversalPath& traversalPath, uint32_t layerId,
- LayerHierarchy::Variant variantArg);
- ~ScopedAddToTraversalPath();
-
- private:
- TraversalPath& mTraversalPath;
- TraversalPath mParentPath;
- };
LayerHierarchy(RequestedLayerState* layer);
// Visitor function that provides the hierarchy node and a traversal id which uniquely
@@ -191,8 +181,9 @@
void removeChild(LayerHierarchy*);
void sortChildrenByZOrder();
void updateChild(LayerHierarchy*, LayerHierarchy::Variant);
- void traverseInZOrder(const Visitor& visitor, LayerHierarchy::TraversalPath& parent) const;
- void traverse(const Visitor& visitor, LayerHierarchy::TraversalPath& parent,
+ void traverseInZOrder(const Visitor& visitor,
+ const LayerHierarchy::TraversalPath& parent) const;
+ void traverse(const Visitor& visitor, const LayerHierarchy::TraversalPath& parent,
uint32_t depth = 0) const;
void dump(std::ostream& out, const std::string& prefix, LayerHierarchy::Variant variant,
bool isLastChild, bool includeMirroredHierarchy) const;
diff --git a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp
index 28a6031..50ed72d 100644
--- a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp
+++ b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp
@@ -447,15 +447,14 @@
if (args.root.getLayer()) {
// The hierarchy can have a root layer when used for screenshots otherwise, it will have
// multiple children.
- LayerHierarchy::ScopedAddToTraversalPath addChildToPath(root, args.root.getLayer()->id,
- LayerHierarchy::Variant::Attached);
- updateSnapshotsInHierarchy(args, args.root, root, rootSnapshot, /*depth=*/0);
+ LayerHierarchy::TraversalPath childPath =
+ root.makeChild(args.root.getLayer()->id, LayerHierarchy::Variant::Attached);
+ updateSnapshotsInHierarchy(args, args.root, childPath, rootSnapshot, /*depth=*/0);
} else {
for (auto& [childHierarchy, variant] : args.root.mChildren) {
- LayerHierarchy::ScopedAddToTraversalPath addChildToPath(root,
- childHierarchy->getLayer()->id,
- variant);
- updateSnapshotsInHierarchy(args, *childHierarchy, root, rootSnapshot, /*depth=*/0);
+ LayerHierarchy::TraversalPath childPath =
+ root.makeChild(childHierarchy->getLayer()->id, variant);
+ updateSnapshotsInHierarchy(args, *childHierarchy, childPath, rootSnapshot, /*depth=*/0);
}
}
@@ -520,7 +519,7 @@
const LayerSnapshot& LayerSnapshotBuilder::updateSnapshotsInHierarchy(
const Args& args, const LayerHierarchy& hierarchy,
- LayerHierarchy::TraversalPath& traversalPath, const LayerSnapshot& parentSnapshot,
+ const LayerHierarchy::TraversalPath& traversalPath, const LayerSnapshot& parentSnapshot,
int depth) {
LLOG_ALWAYS_FATAL_WITH_TRACE_IF(depth > 50,
"Cycle detected in LayerSnapshotBuilder. See "
@@ -549,12 +548,10 @@
bool childHasValidFrameRate = false;
for (auto& [childHierarchy, variant] : hierarchy.mChildren) {
- LayerHierarchy::ScopedAddToTraversalPath addChildToPath(traversalPath,
- childHierarchy->getLayer()->id,
- variant);
+ LayerHierarchy::TraversalPath childPath =
+ traversalPath.makeChild(childHierarchy->getLayer()->id, variant);
const LayerSnapshot& childSnapshot =
- updateSnapshotsInHierarchy(args, *childHierarchy, traversalPath, *snapshot,
- depth + 1);
+ updateSnapshotsInHierarchy(args, *childHierarchy, childPath, *snapshot, depth + 1);
updateFrameRateFromChildSnapshot(*snapshot, childSnapshot, *childHierarchy->getLayer(),
args, &childHasValidFrameRate);
}
diff --git a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.h b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.h
index 486cb33..94b7e5f 100644
--- a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.h
+++ b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.h
@@ -106,9 +106,10 @@
void updateSnapshots(const Args& args);
- const LayerSnapshot& updateSnapshotsInHierarchy(const Args&, const LayerHierarchy& hierarchy,
- LayerHierarchy::TraversalPath& traversalPath,
- const LayerSnapshot& parentSnapshot, int depth);
+ const LayerSnapshot& updateSnapshotsInHierarchy(
+ const Args&, const LayerHierarchy& hierarchy,
+ const LayerHierarchy::TraversalPath& traversalPath, const LayerSnapshot& parentSnapshot,
+ int depth);
void updateSnapshot(LayerSnapshot&, const Args&, const RequestedLayerState&,
const LayerSnapshot& parentSnapshot, const LayerHierarchy::TraversalPath&);
static void updateRelativeState(LayerSnapshot& snapshot, const LayerSnapshot& parentSnapshot,
diff --git a/services/surfaceflinger/LayerProtoHelper.cpp b/services/surfaceflinger/LayerProtoHelper.cpp
index 84b1a73..280d66e 100644
--- a/services/surfaceflinger/LayerProtoHelper.cpp
+++ b/services/surfaceflinger/LayerProtoHelper.cpp
@@ -278,10 +278,9 @@
stackIdsToSkip.find(child->getLayer()->layerStack.id) != stackIdsToSkip.end()) {
continue;
}
- frontend::LayerHierarchy::ScopedAddToTraversalPath addChildToPath(path,
- child->getLayer()->id,
- variant);
- LayerProtoFromSnapshotGenerator::writeHierarchyToProto(*child, path);
+ LayerProtoFromSnapshotGenerator::writeHierarchyToProto(*child,
+ path.makeChild(child->getLayer()->id,
+ variant));
}
// fill in relative and parent info
@@ -338,7 +337,8 @@
}
frontend::LayerSnapshot* LayerProtoFromSnapshotGenerator::getSnapshot(
- frontend::LayerHierarchy::TraversalPath& path, const frontend::RequestedLayerState& layer) {
+ const frontend::LayerHierarchy::TraversalPath& path,
+ const frontend::RequestedLayerState& layer) {
frontend::LayerSnapshot* snapshot = mSnapshotBuilder.getSnapshot(path);
if (snapshot) {
return snapshot;
@@ -349,7 +349,7 @@
}
void LayerProtoFromSnapshotGenerator::writeHierarchyToProto(
- const frontend::LayerHierarchy& root, frontend::LayerHierarchy::TraversalPath& path) {
+ const frontend::LayerHierarchy& root, const frontend::LayerHierarchy::TraversalPath& path) {
using Variant = frontend::LayerHierarchy::Variant;
perfetto::protos::LayerProto* layerProto = mLayersProto.add_layers();
const frontend::RequestedLayerState& layer = *root.getLayer();
@@ -362,10 +362,8 @@
LayerProtoHelper::writeSnapshotToProto(layerProto, layer, *snapshot, mTraceFlags);
for (const auto& [child, variant] : root.mChildren) {
- frontend::LayerHierarchy::ScopedAddToTraversalPath addChildToPath(path,
- child->getLayer()->id,
- variant);
- frontend::LayerSnapshot* childSnapshot = getSnapshot(path, layer);
+ frontend::LayerSnapshot* childSnapshot =
+ getSnapshot(path.makeChild(child->getLayer()->id, variant), layer);
if (variant == Variant::Attached || variant == Variant::Detached ||
frontend::LayerHierarchy::isMirror(variant)) {
mChildToParent[childSnapshot->uniqueSequence] = snapshot->uniqueSequence;
@@ -388,10 +386,7 @@
if (variant == Variant::Detached) {
continue;
}
- frontend::LayerHierarchy::ScopedAddToTraversalPath addChildToPath(path,
- child->getLayer()->id,
- variant);
- writeHierarchyToProto(*child, path);
+ writeHierarchyToProto(*child, path.makeChild(child->getLayer()->id, variant));
}
}
diff --git a/services/surfaceflinger/LayerProtoHelper.h b/services/surfaceflinger/LayerProtoHelper.h
index 3ca553a..28924e4 100644
--- a/services/surfaceflinger/LayerProtoHelper.h
+++ b/services/surfaceflinger/LayerProtoHelper.h
@@ -98,8 +98,8 @@
private:
void writeHierarchyToProto(const frontend::LayerHierarchy& root,
- frontend::LayerHierarchy::TraversalPath& path);
- frontend::LayerSnapshot* getSnapshot(frontend::LayerHierarchy::TraversalPath& path,
+ const frontend::LayerHierarchy::TraversalPath& path);
+ frontend::LayerSnapshot* getSnapshot(const frontend::LayerHierarchy::TraversalPath& path,
const frontend::RequestedLayerState& layer);
const frontend::LayerSnapshotBuilder& mSnapshotBuilder;