Skip to content

Commit 440605b

Browse files
gspencergoogInconnu08
authored andcommitted
Optimize focus operations by caching descendants and ancestors. (flutter#42683)
This optimizes certain paths in the FocusManager, FocusNode, and FocusScopeNode classes in order to fix a regression in stock_animation_open_first_frame_average when I added more focus nodes to the tree to do focus traversal. Mainly I removed some remaining sync* iterators, and also started caching the computation of descendants and ancestors, since those are iterated over fairly often. This improves stock_animation_open_first_frame_average by about 2.8% overall (so about half of a 4.9% regression, both averaged over 10 runs). Addresses flutter#42564
1 parent dcfb96b commit 440605b

File tree

5 files changed

+102
-48
lines changed

5 files changed

+102
-48
lines changed

packages/flutter/lib/src/material/ink_well.dart

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -489,20 +489,20 @@ class _InkResponseState<T extends InkResponse> extends State<T> with AutomaticKe
489489

490490
bool get highlightsExist => _highlights.values.where((InkHighlight highlight) => highlight != null).isNotEmpty;
491491

492+
Action _createAction() {
493+
return CallbackAction(
494+
ActivateAction.key,
495+
onInvoke: (FocusNode node, Intent intent) {
496+
_startSplash(context: node.context);
497+
_handleTap(node.context);
498+
},
499+
);
500+
}
501+
492502
@override
493503
void initState() {
494504
super.initState();
495-
_actionMap = <LocalKey, ActionFactory>{
496-
ActivateAction.key: () {
497-
return CallbackAction(
498-
ActivateAction.key,
499-
onInvoke: (FocusNode node, Intent intent) {
500-
_startSplash(context: node.context);
501-
_handleTap(node.context);
502-
},
503-
);
504-
},
505-
};
505+
_actionMap = <LocalKey, ActionFactory>{ ActivateAction.key: _createAction };
506506
WidgetsBinding.instance.focusManager.addHighlightModeListener(_handleFocusHighlightModeChange);
507507
}
508508

packages/flutter/lib/src/widgets/focus_manager.dart

Lines changed: 39 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -119,8 +119,8 @@ class FocusAttachment {
119119
assert(_node != null);
120120
if (isAttached) {
121121
assert(_node.context != null);
122-
parent ??= Focus.of(_node.context, nullOk: true);
123-
parent ??= FocusScope.of(_node.context);
122+
parent ??= Focus.of(_node.context, nullOk: true, scopeOk: true);
123+
parent ??= _node.context.owner.focusManager.rootScope;
124124
assert(parent != null);
125125
parent._reparent(_node);
126126
}
@@ -421,7 +421,10 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier {
421421
/// its descendants.
422422
/// - [FocusTraversalPolicy], a class that can be extended to describe a
423423
/// traversal policy.
424-
bool get canRequestFocus => _canRequestFocus && (enclosingScope == null || enclosingScope.canRequestFocus);
424+
bool get canRequestFocus {
425+
final FocusScopeNode scope = enclosingScope;
426+
return _canRequestFocus && (scope == null || scope.canRequestFocus);
427+
}
425428
bool _canRequestFocus;
426429
@mustCallSuper
427430
set canRequestFocus(bool value) {
@@ -450,6 +453,8 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier {
450453
FocusOnKeyCallback _onKey;
451454

452455
FocusManager _manager;
456+
List<FocusNode> _ancestors;
457+
List<FocusNode> _descendants;
453458
bool _hasKeyboardToken = false;
454459

455460
/// Returns the parent node for this object.
@@ -471,7 +476,7 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier {
471476
return const <FocusNode>[];
472477
}
473478
return children.where(
474-
(FocusNode node) => !node.skipTraversal && node._canRequestFocus,
479+
(FocusNode node) => !node.skipTraversal && node.canRequestFocus,
475480
);
476481
}
477482

@@ -492,11 +497,16 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier {
492497

493498
/// An [Iterable] over the hierarchy of children below this one, in
494499
/// depth-first order.
495-
Iterable<FocusNode> get descendants sync* {
496-
for (FocusNode child in _children) {
497-
yield* child.descendants;
498-
yield child;
500+
Iterable<FocusNode> get descendants {
501+
if (_descendants == null) {
502+
final List<FocusNode> result = <FocusNode>[];
503+
for (FocusNode child in _children) {
504+
result.addAll(child.descendants);
505+
result.add(child);
506+
}
507+
_descendants = result;
499508
}
509+
return _descendants;
500510
}
501511

502512
/// Returns all descendants which do not have the [skipTraversal] flag set.
@@ -507,12 +517,17 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier {
507517
/// Iterates the ancestors of this node starting at the parent and iterating
508518
/// over successively more remote ancestors of this node, ending at the root
509519
/// [FocusScope] ([FocusManager.rootScope]).
510-
Iterable<FocusNode> get ancestors sync* {
511-
FocusNode parent = _parent;
512-
while (parent != null) {
513-
yield parent;
514-
parent = parent._parent;
520+
Iterable<FocusNode> get ancestors {
521+
if (_ancestors == null) {
522+
final List<FocusNode> result = <FocusNode>[];
523+
FocusNode parent = _parent;
524+
while (parent != null) {
525+
result.add(parent);
526+
parent = parent._parent;
527+
}
528+
_ancestors = result;
515529
}
530+
return _ancestors;
516531
}
517532

518533
/// Whether this node has input focus.
@@ -710,13 +725,18 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier {
710725

711726
node._parent = null;
712727
_children.remove(node);
728+
for (FocusNode ancestor in ancestors) {
729+
ancestor._descendants = null;
730+
}
731+
_descendants = null;
713732
assert(_manager == null || !_manager.rootScope.descendants.contains(node));
714733
}
715734

716735
void _updateManager(FocusManager manager) {
717736
_manager = manager;
718737
for (FocusNode descendant in descendants) {
719738
descendant._manager = manager;
739+
descendant._ancestors = null;
720740
}
721741
}
722742

@@ -737,7 +757,11 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier {
737757
child._parent?._removeChild(child, removeScopeFocus: oldScope != nearestScope);
738758
_children.add(child);
739759
child._parent = this;
760+
child._ancestors = null;
740761
child._updateManager(_manager);
762+
for (FocusNode ancestor in child.ancestors) {
763+
ancestor._descendants = null;
764+
}
741765
if (hadFocus) {
742766
// Update the focus chain for the current focus without changing it.
743767
_manager?.primaryFocus?._setAsFocusedChild();
@@ -1266,15 +1290,8 @@ class FocusManager with DiagnosticableTreeMixin {
12661290
assert(_focusDebug('No primary focus for key event, ignored: $event'));
12671291
return;
12681292
}
1269-
Iterable<FocusNode> allNodes(FocusNode node) sync* {
1270-
yield node;
1271-
for (FocusNode ancestor in node.ancestors) {
1272-
yield ancestor;
1273-
}
1274-
}
1275-
12761293
bool handled = false;
1277-
for (FocusNode node in allNodes(_primaryFocus)) {
1294+
for (FocusNode node in <FocusNode>[_primaryFocus, ..._primaryFocus.ancestors]) {
12781295
if (node.onKey != null && node.onKey(node, event)) {
12791296
assert(_focusDebug('Node $node handled key event $event.'));
12801297
handled = true;
@@ -1338,7 +1355,7 @@ class FocusManager with DiagnosticableTreeMixin {
13381355
final FocusNode previousFocus = _primaryFocus;
13391356
if (_primaryFocus == null && _nextFocus == null) {
13401357
// If we don't have any current focus, and nobody has asked to focus yet,
1341-
// then pick a first one using widget order as a default.
1358+
// then revert to the root scope.
13421359
_nextFocus = rootScope;
13431360
}
13441361
if (_nextFocus != null && _nextFocus != _primaryFocus) {

packages/flutter/lib/src/widgets/focus_scope.dart

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -262,33 +262,34 @@ class Focus extends StatefulWidget {
262262
/// [nullOk].
263263
///
264264
/// The [context] and [nullOk] arguments must not be null.
265-
static FocusNode of(BuildContext context, { bool nullOk = false }) {
265+
static FocusNode of(BuildContext context, { bool nullOk = false, bool scopeOk = false }) {
266266
assert(context != null);
267267
assert(nullOk != null);
268+
assert(scopeOk != null);
268269
final _FocusMarker marker = context.inheritFromWidgetOfExactType(_FocusMarker);
269270
final FocusNode node = marker?.notifier;
270-
if (node is FocusScopeNode) {
271+
if (node == null) {
271272
if (!nullOk) {
272273
throw FlutterError(
273-
'Focus.of() was called with a context that does not contain a Focus between the given '
274-
'context and the nearest FocusScope widget.\n'
275-
'No Focus ancestor could be found starting from the context that was passed to '
276-
'Focus.of() to the point where it found the nearest FocusScope widget. This can happen '
277-
'because you are using a widget that looks for a Focus ancestor, and do not have a '
278-
'Focus widget ancestor in the current FocusScope.\n'
279-
'The context used was:\n'
280-
' $context'
274+
'Focus.of() was called with a context that does not contain a Focus widget.\n'
275+
'No Focus widget ancestor could be found starting from the context that was passed to '
276+
'Focus.of(). This can happen because you are using a widget that looks for a Focus '
277+
'ancestor, and do not have a Focus widget descendant in the nearest FocusScope.\n'
278+
'The context used was:\n'
279+
' $context'
281280
);
282281
}
283282
return null;
284283
}
285-
if (node == null) {
284+
if (!scopeOk && node is FocusScopeNode) {
286285
if (!nullOk) {
287286
throw FlutterError(
288-
'Focus.of() was called with a context that does not contain a Focus widget.\n'
289-
'No Focus widget ancestor could be found starting from the context that was passed to '
290-
'Focus.of(). This can happen because you are using a widget that looks for a Focus '
291-
'ancestor, and do not have a Focus widget descendant in the nearest FocusScope.\n'
287+
'Focus.of() was called with a context that does not contain a Focus between the given '
288+
'context and the nearest FocusScope widget.\n'
289+
'No Focus ancestor could be found starting from the context that was passed to '
290+
'Focus.of() to the point where it found the nearest FocusScope widget. This can happen '
291+
'because you are using a widget that looks for a Focus ancestor, and do not have a '
292+
'Focus widget ancestor in the current FocusScope.\n'
292293
'The context used was:\n'
293294
' $context'
294295
);

packages/flutter/test/widgets/focus_manager_test.dart

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,41 @@ void main() {
300300
expect(scope1.focusedChild, isNull);
301301
expect(parent2.children.contains(child1), isTrue);
302302
});
303+
testWidgets('ancestors and descendants are computed and recomputed properly', (WidgetTester tester) async {
304+
final BuildContext context = await setupWidget(tester);
305+
final FocusScopeNode scope1 = FocusScopeNode(debugLabel: 'scope1');
306+
final FocusAttachment scope1Attachment = scope1.attach(context);
307+
final FocusScopeNode scope2 = FocusScopeNode(debugLabel: 'scope2');
308+
final FocusAttachment scope2Attachment = scope2.attach(context);
309+
final FocusNode parent1 = FocusNode(debugLabel: 'parent1');
310+
final FocusAttachment parent1Attachment = parent1.attach(context);
311+
final FocusNode parent2 = FocusNode(debugLabel: 'parent2');
312+
final FocusAttachment parent2Attachment = parent2.attach(context);
313+
final FocusNode child1 = FocusNode(debugLabel: 'child1');
314+
final FocusAttachment child1Attachment = child1.attach(context);
315+
final FocusNode child2 = FocusNode(debugLabel: 'child2');
316+
final FocusAttachment child2Attachment = child2.attach(context);
317+
final FocusNode child3 = FocusNode(debugLabel: 'child3');
318+
final FocusAttachment child3Attachment = child3.attach(context);
319+
final FocusNode child4 = FocusNode(debugLabel: 'child4');
320+
final FocusAttachment child4Attachment = child4.attach(context);
321+
scope1Attachment.reparent(parent: tester.binding.focusManager.rootScope);
322+
scope2Attachment.reparent(parent: tester.binding.focusManager.rootScope);
323+
parent1Attachment.reparent(parent: scope1);
324+
parent2Attachment.reparent(parent: scope2);
325+
child1Attachment.reparent(parent: parent1);
326+
child2Attachment.reparent(parent: parent1);
327+
child3Attachment.reparent(parent: parent2);
328+
child4Attachment.reparent(parent: parent2);
329+
child4.requestFocus();
330+
await tester.pump();
331+
expect(child4.ancestors, equals(<FocusNode>[parent2, scope2, tester.binding.focusManager.rootScope]));
332+
expect(tester.binding.focusManager.rootScope.descendants, equals(<FocusNode>[child1, child2, parent1, scope1, child3, child4, parent2, scope2]));
333+
scope2Attachment.reparent(parent: child2);
334+
await tester.pump();
335+
expect(child4.ancestors, equals(<FocusNode>[parent2, scope2, child2, parent1, scope1, tester.binding.focusManager.rootScope]));
336+
expect(tester.binding.focusManager.rootScope.descendants, equals(<FocusNode>[child1, child3, child4, parent2, scope2, child2, parent1, scope1]));
337+
});
303338
testWidgets('Can move focus between scopes and keep focus', (WidgetTester tester) async {
304339
final BuildContext context = await setupWidget(tester);
305340
final FocusScopeNode scope1 = FocusScopeNode();

packages/flutter/test/widgets/focus_scope_test.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -504,6 +504,7 @@ void main() {
504504

505505
FocusScope.of(keyA.currentContext).requestFocus(keyA.currentState.focusNode);
506506
expect(FocusScope.of(keyA.currentContext), equals(childFocusScope));
507+
expect(Focus.of(keyA.currentContext, scopeOk: true), equals(childFocusScope));
507508
WidgetsBinding.instance.focusManager.rootScope.setFirstFocus(FocusScope.of(keyA.currentContext));
508509

509510
await tester.pumpAndSettle();

0 commit comments

Comments
 (0)