Skip to content

Commit a89f063

Browse files
Treehugger RobotAndroid (Google) Code Review
Treehugger Robot
authored and
Android (Google) Code Review
committed
Merge "Remove ScopedAddToTraversalPath" into main
2 parents b0ef411 + f1a5340 commit a89f063

6 files changed

Lines changed: 48 additions & 85 deletions

File tree

‎services/surfaceflinger/FrontEnd/LayerHierarchy.cpp‎

Lines changed: 18 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ LayerHierarchy::LayerHierarchy(const LayerHierarchy& hierarchy, bool childrenOnl
5454
mChildren = hierarchy.mChildren;
5555
}
5656

57-
void LayerHierarchy::traverse(const Visitor& visitor, LayerHierarchy::TraversalPath& traversalPath,
57+
void LayerHierarchy::traverse(const Visitor& visitor,
58+
const LayerHierarchy::TraversalPath& traversalPath,
5859
uint32_t depth) const {
5960
LLOG_ALWAYS_FATAL_WITH_TRACE_IF(depth > 50,
6061
"Cycle detected in LayerHierarchy::traverse. See "
@@ -70,14 +71,13 @@ void LayerHierarchy::traverse(const Visitor& visitor, LayerHierarchy::TraversalP
7071
LLOG_ALWAYS_FATAL_WITH_TRACE_IF(traversalPath.hasRelZLoop(), "Found relative z loop layerId:%d",
7172
traversalPath.invalidRelativeRootId);
7273
for (auto& [child, childVariant] : mChildren) {
73-
ScopedAddToTraversalPath addChildToTraversalPath(traversalPath, child->mLayer->id,
74-
childVariant);
75-
child->traverse(visitor, traversalPath, depth + 1);
74+
child->traverse(visitor, traversalPath.makeChild(child->mLayer->id, childVariant),
75+
depth + 1);
7676
}
7777
}
7878

7979
void LayerHierarchy::traverseInZOrder(const Visitor& visitor,
80-
LayerHierarchy::TraversalPath& traversalPath) const {
80+
const LayerHierarchy::TraversalPath& traversalPath) const {
8181
bool traverseThisLayer = (mLayer != nullptr);
8282
for (auto it = mChildren.begin(); it < mChildren.end(); it++) {
8383
auto& [child, childVariant] = *it;
@@ -91,9 +91,7 @@ void LayerHierarchy::traverseInZOrder(const Visitor& visitor,
9191
if (childVariant == LayerHierarchy::Variant::Detached) {
9292
continue;
9393
}
94-
ScopedAddToTraversalPath addChildToTraversalPath(traversalPath, child->mLayer->id,
95-
childVariant);
96-
child->traverseInZOrder(visitor, traversalPath);
94+
child->traverseInZOrder(visitor, traversalPath.makeChild(child->mLayer->id, childVariant));
9795
}
9896

9997
if (traverseThisLayer) {
@@ -568,42 +566,23 @@ std::string LayerHierarchy::TraversalPath::toString() const {
568566
return ss.str();
569567
}
570568

571-
// Helper class to update a passed in TraversalPath when visiting a child. When the object goes out
572-
// of scope the TraversalPath is reset to its original state.
573-
LayerHierarchy::ScopedAddToTraversalPath::ScopedAddToTraversalPath(TraversalPath& traversalPath,
574-
uint32_t layerId,
575-
LayerHierarchy::Variant variant)
576-
: mTraversalPath(traversalPath), mParentPath(traversalPath) {
577-
// Update the traversal id with the child layer id and variant. Parent id and variant are
578-
// stored to reset the id upon destruction.
579-
traversalPath.id = layerId;
580-
traversalPath.variant = variant;
569+
LayerHierarchy::TraversalPath LayerHierarchy::TraversalPath::makeChild(
570+
uint32_t layerId, LayerHierarchy::Variant variant) const {
571+
TraversalPath child{*this};
572+
child.id = layerId;
573+
child.variant = variant;
581574
if (LayerHierarchy::isMirror(variant)) {
582-
traversalPath.mirrorRootIds.emplace_back(mParentPath.id);
575+
child.mirrorRootIds.emplace_back(id);
583576
} else if (variant == LayerHierarchy::Variant::Relative) {
584-
if (std::find(traversalPath.relativeRootIds.begin(), traversalPath.relativeRootIds.end(),
585-
layerId) != traversalPath.relativeRootIds.end()) {
586-
traversalPath.invalidRelativeRootId = layerId;
577+
if (std::find(relativeRootIds.begin(), relativeRootIds.end(), layerId) !=
578+
relativeRootIds.end()) {
579+
child.invalidRelativeRootId = layerId;
587580
}
588-
traversalPath.relativeRootIds.emplace_back(layerId);
581+
child.relativeRootIds.emplace_back(layerId);
589582
} else if (variant == LayerHierarchy::Variant::Detached) {
590-
traversalPath.detached = true;
583+
child.detached = true;
591584
}
592-
}
593-
LayerHierarchy::ScopedAddToTraversalPath::~ScopedAddToTraversalPath() {
594-
// Reset the traversal id to its original parent state using the state that was saved in
595-
// the constructor.
596-
if (LayerHierarchy::isMirror(mTraversalPath.variant)) {
597-
mTraversalPath.mirrorRootIds.pop_back();
598-
} else if (mTraversalPath.variant == LayerHierarchy::Variant::Relative) {
599-
mTraversalPath.relativeRootIds.pop_back();
600-
}
601-
if (mTraversalPath.invalidRelativeRootId == mTraversalPath.id) {
602-
mTraversalPath.invalidRelativeRootId = UNASSIGNED_LAYER_ID;
603-
}
604-
mTraversalPath.id = mParentPath.id;
605-
mTraversalPath.variant = mParentPath.variant;
606-
mTraversalPath.detached = mParentPath.detached;
585+
return child;
607586
}
608587

609588
} // namespace android::surfaceflinger::frontend

‎services/surfaceflinger/FrontEnd/LayerHierarchy.h‎

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,8 @@ class LayerHierarchy {
104104

105105
TraversalPath getClonedFrom() const { return {.id = id, .variant = variant}; }
106106

107+
TraversalPath makeChild(uint32_t layerId, LayerHierarchy::Variant variant) const;
108+
107109
bool operator==(const TraversalPath& other) const {
108110
return id == other.id && mirrorRootIds == other.mirrorRootIds;
109111
}
@@ -122,18 +124,6 @@ class LayerHierarchy {
122124
}
123125
};
124126

125-
// Helper class to add nodes to an existing traversal id and removes the
126-
// node when it goes out of scope.
127-
class ScopedAddToTraversalPath {
128-
public:
129-
ScopedAddToTraversalPath(TraversalPath& traversalPath, uint32_t layerId,
130-
LayerHierarchy::Variant variantArg);
131-
~ScopedAddToTraversalPath();
132-
133-
private:
134-
TraversalPath& mTraversalPath;
135-
TraversalPath mParentPath;
136-
};
137127
LayerHierarchy(RequestedLayerState* layer);
138128

139129
// Visitor function that provides the hierarchy node and a traversal id which uniquely
@@ -191,8 +181,9 @@ class LayerHierarchy {
191181
void removeChild(LayerHierarchy*);
192182
void sortChildrenByZOrder();
193183
void updateChild(LayerHierarchy*, LayerHierarchy::Variant);
194-
void traverseInZOrder(const Visitor& visitor, LayerHierarchy::TraversalPath& parent) const;
195-
void traverse(const Visitor& visitor, LayerHierarchy::TraversalPath& parent,
184+
void traverseInZOrder(const Visitor& visitor,
185+
const LayerHierarchy::TraversalPath& parent) const;
186+
void traverse(const Visitor& visitor, const LayerHierarchy::TraversalPath& parent,
196187
uint32_t depth = 0) const;
197188
void dump(std::ostream& out, const std::string& prefix, LayerHierarchy::Variant variant,
198189
bool isLastChild, bool includeMirroredHierarchy) const;

‎services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp‎

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -447,15 +447,14 @@ void LayerSnapshotBuilder::updateSnapshots(const Args& args) {
447447
if (args.root.getLayer()) {
448448
// The hierarchy can have a root layer when used for screenshots otherwise, it will have
449449
// multiple children.
450-
LayerHierarchy::ScopedAddToTraversalPath addChildToPath(root, args.root.getLayer()->id,
451-
LayerHierarchy::Variant::Attached);
452-
updateSnapshotsInHierarchy(args, args.root, root, rootSnapshot, /*depth=*/0);
450+
LayerHierarchy::TraversalPath childPath =
451+
root.makeChild(args.root.getLayer()->id, LayerHierarchy::Variant::Attached);
452+
updateSnapshotsInHierarchy(args, args.root, childPath, rootSnapshot, /*depth=*/0);
453453
} else {
454454
for (auto& [childHierarchy, variant] : args.root.mChildren) {
455-
LayerHierarchy::ScopedAddToTraversalPath addChildToPath(root,
456-
childHierarchy->getLayer()->id,
457-
variant);
458-
updateSnapshotsInHierarchy(args, *childHierarchy, root, rootSnapshot, /*depth=*/0);
455+
LayerHierarchy::TraversalPath childPath =
456+
root.makeChild(childHierarchy->getLayer()->id, variant);
457+
updateSnapshotsInHierarchy(args, *childHierarchy, childPath, rootSnapshot, /*depth=*/0);
459458
}
460459
}
461460

@@ -520,7 +519,7 @@ void LayerSnapshotBuilder::update(const Args& args) {
520519

521520
const LayerSnapshot& LayerSnapshotBuilder::updateSnapshotsInHierarchy(
522521
const Args& args, const LayerHierarchy& hierarchy,
523-
LayerHierarchy::TraversalPath& traversalPath, const LayerSnapshot& parentSnapshot,
522+
const LayerHierarchy::TraversalPath& traversalPath, const LayerSnapshot& parentSnapshot,
524523
int depth) {
525524
LLOG_ALWAYS_FATAL_WITH_TRACE_IF(depth > 50,
526525
"Cycle detected in LayerSnapshotBuilder. See "
@@ -549,12 +548,10 @@ const LayerSnapshot& LayerSnapshotBuilder::updateSnapshotsInHierarchy(
549548

550549
bool childHasValidFrameRate = false;
551550
for (auto& [childHierarchy, variant] : hierarchy.mChildren) {
552-
LayerHierarchy::ScopedAddToTraversalPath addChildToPath(traversalPath,
553-
childHierarchy->getLayer()->id,
554-
variant);
551+
LayerHierarchy::TraversalPath childPath =
552+
traversalPath.makeChild(childHierarchy->getLayer()->id, variant);
555553
const LayerSnapshot& childSnapshot =
556-
updateSnapshotsInHierarchy(args, *childHierarchy, traversalPath, *snapshot,
557-
depth + 1);
554+
updateSnapshotsInHierarchy(args, *childHierarchy, childPath, *snapshot, depth + 1);
558555
updateFrameRateFromChildSnapshot(*snapshot, childSnapshot, *childHierarchy->getLayer(),
559556
args, &childHasValidFrameRate);
560557
}

‎services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.h‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,9 +106,10 @@ class LayerSnapshotBuilder {
106106

107107
void updateSnapshots(const Args& args);
108108

109-
const LayerSnapshot& updateSnapshotsInHierarchy(const Args&, const LayerHierarchy& hierarchy,
110-
LayerHierarchy::TraversalPath& traversalPath,
111-
const LayerSnapshot& parentSnapshot, int depth);
109+
const LayerSnapshot& updateSnapshotsInHierarchy(
110+
const Args&, const LayerHierarchy& hierarchy,
111+
const LayerHierarchy::TraversalPath& traversalPath, const LayerSnapshot& parentSnapshot,
112+
int depth);
112113
void updateSnapshot(LayerSnapshot&, const Args&, const RequestedLayerState&,
113114
const LayerSnapshot& parentSnapshot, const LayerHierarchy::TraversalPath&);
114115
static void updateRelativeState(LayerSnapshot& snapshot, const LayerSnapshot& parentSnapshot,

‎services/surfaceflinger/LayerProtoHelper.cpp‎

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -278,10 +278,9 @@ LayerProtoFromSnapshotGenerator& LayerProtoFromSnapshotGenerator::with(
278278
stackIdsToSkip.find(child->getLayer()->layerStack.id) != stackIdsToSkip.end()) {
279279
continue;
280280
}
281-
frontend::LayerHierarchy::ScopedAddToTraversalPath addChildToPath(path,
282-
child->getLayer()->id,
283-
variant);
284-
LayerProtoFromSnapshotGenerator::writeHierarchyToProto(*child, path);
281+
LayerProtoFromSnapshotGenerator::writeHierarchyToProto(*child,
282+
path.makeChild(child->getLayer()->id,
283+
variant));
285284
}
286285

287286
// fill in relative and parent info
@@ -338,7 +337,8 @@ LayerProtoFromSnapshotGenerator& LayerProtoFromSnapshotGenerator::withOffscreenL
338337
}
339338

340339
frontend::LayerSnapshot* LayerProtoFromSnapshotGenerator::getSnapshot(
341-
frontend::LayerHierarchy::TraversalPath& path, const frontend::RequestedLayerState& layer) {
340+
const frontend::LayerHierarchy::TraversalPath& path,
341+
const frontend::RequestedLayerState& layer) {
342342
frontend::LayerSnapshot* snapshot = mSnapshotBuilder.getSnapshot(path);
343343
if (snapshot) {
344344
return snapshot;
@@ -349,7 +349,7 @@ frontend::LayerSnapshot* LayerProtoFromSnapshotGenerator::getSnapshot(
349349
}
350350

351351
void LayerProtoFromSnapshotGenerator::writeHierarchyToProto(
352-
const frontend::LayerHierarchy& root, frontend::LayerHierarchy::TraversalPath& path) {
352+
const frontend::LayerHierarchy& root, const frontend::LayerHierarchy::TraversalPath& path) {
353353
using Variant = frontend::LayerHierarchy::Variant;
354354
perfetto::protos::LayerProto* layerProto = mLayersProto.add_layers();
355355
const frontend::RequestedLayerState& layer = *root.getLayer();
@@ -362,10 +362,8 @@ void LayerProtoFromSnapshotGenerator::writeHierarchyToProto(
362362
LayerProtoHelper::writeSnapshotToProto(layerProto, layer, *snapshot, mTraceFlags);
363363

364364
for (const auto& [child, variant] : root.mChildren) {
365-
frontend::LayerHierarchy::ScopedAddToTraversalPath addChildToPath(path,
366-
child->getLayer()->id,
367-
variant);
368-
frontend::LayerSnapshot* childSnapshot = getSnapshot(path, layer);
365+
frontend::LayerSnapshot* childSnapshot =
366+
getSnapshot(path.makeChild(child->getLayer()->id, variant), layer);
369367
if (variant == Variant::Attached || variant == Variant::Detached ||
370368
frontend::LayerHierarchy::isMirror(variant)) {
371369
mChildToParent[childSnapshot->uniqueSequence] = snapshot->uniqueSequence;
@@ -388,10 +386,7 @@ void LayerProtoFromSnapshotGenerator::writeHierarchyToProto(
388386
if (variant == Variant::Detached) {
389387
continue;
390388
}
391-
frontend::LayerHierarchy::ScopedAddToTraversalPath addChildToPath(path,
392-
child->getLayer()->id,
393-
variant);
394-
writeHierarchyToProto(*child, path);
389+
writeHierarchyToProto(*child, path.makeChild(child->getLayer()->id, variant));
395390
}
396391
}
397392

‎services/surfaceflinger/LayerProtoHelper.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,8 @@ class LayerProtoFromSnapshotGenerator {
9898

9999
private:
100100
void writeHierarchyToProto(const frontend::LayerHierarchy& root,
101-
frontend::LayerHierarchy::TraversalPath& path);
102-
frontend::LayerSnapshot* getSnapshot(frontend::LayerHierarchy::TraversalPath& path,
101+
const frontend::LayerHierarchy::TraversalPath& path);
102+
frontend::LayerSnapshot* getSnapshot(const frontend::LayerHierarchy::TraversalPath& path,
103103
const frontend::RequestedLayerState& layer);
104104

105105
const frontend::LayerSnapshotBuilder& mSnapshotBuilder;

0 commit comments

Comments
 (0)