Skip to content

Commit 16dce76

Browse files
authored
Fix excessive rebuilds of DSS (flutter#69724)
1 parent 0da8f13 commit 16dce76

File tree

2 files changed

+175
-9
lines changed

2 files changed

+175
-9
lines changed

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

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -288,17 +288,17 @@ class _DraggableSheetExtent {
288288
class _DraggableScrollableSheetState extends State<DraggableScrollableSheet> {
289289
late _DraggableScrollableSheetScrollController _scrollController;
290290
late _DraggableSheetExtent _extent;
291+
// The child only gets rebuilt when dependencies or the widget change.
292+
// Otherwise, excessive rebuilds of the child are triggered every time the
293+
// scroll extent changes, which is very expensive and does not provide any
294+
// helpful information to the child. If the child needs to rebuild whenever
295+
// the scroll position changes, they can always subscribe to it.
296+
Widget? _child;
291297

292298
@override
293299
void initState() {
294300
super.initState();
295-
_extent = _DraggableSheetExtent(
296-
minExtent: widget.minChildSize,
297-
maxExtent: widget.maxChildSize,
298-
initialExtent: widget.initialChildSize,
299-
listener: _setExtent,
300-
);
301-
_scrollController = _DraggableScrollableSheetScrollController(extent: _extent);
301+
_updateExtent();
302302
}
303303

304304
@override
@@ -317,13 +317,33 @@ class _DraggableScrollableSheetState extends State<DraggableScrollableSheet> {
317317
}
318318
_extent._currentExtent.value = _extent.initialExtent;
319319
}
320+
_child = widget.builder(context, _scrollController);
321+
}
322+
323+
@override
324+
void didUpdateWidget(DraggableScrollableSheet oldWidget) {
325+
super.didUpdateWidget(oldWidget);
326+
_updateExtent();
327+
// Call this unconditionally - the closure may not change even though it
328+
// refers to things outside of its identity, e.g. a tearoff from state that
329+
// has an `if (stateVariable)`.
330+
_child = widget.builder(context, _scrollController);
331+
}
332+
333+
void _updateExtent() {
334+
_extent = _DraggableSheetExtent(
335+
minExtent: widget.minChildSize,
336+
maxExtent: widget.maxChildSize,
337+
initialExtent: widget.initialChildSize,
338+
listener: _setExtent,
339+
);
340+
_scrollController = _DraggableScrollableSheetScrollController(extent: _extent);
320341
}
321342

322343
void _setExtent() {
323344
setState(() {
324345
// _extent has been updated when this is called.
325346
});
326-
327347
}
328348

329349
@override
@@ -333,7 +353,7 @@ class _DraggableScrollableSheetState extends State<DraggableScrollableSheet> {
333353
_extent.availablePixels = widget.maxChildSize * constraints.biggest.height;
334354
final Widget sheet = FractionallySizedBox(
335355
heightFactor: _extent.currentExtent,
336-
child: widget.builder(context, _scrollController),
356+
child: _child,
337357
alignment: Alignment.bottomCenter,
338358
);
339359
return widget.expand ? SizedBox.expand(child: sheet) : sheet;
@@ -344,6 +364,7 @@ class _DraggableScrollableSheetState extends State<DraggableScrollableSheet> {
344364
@override
345365
void dispose() {
346366
_scrollController.dispose();
367+
_child = null;
347368
super.dispose();
348369
}
349370
}

packages/flutter/test/widgets/draggable_scrollable_sheet_test.dart

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,4 +309,149 @@ void main() {
309309
expect(notificationTypes, types);
310310
});
311311
}
312+
313+
testWidgets('Builder is not called excessively', (WidgetTester tester) async {
314+
int buildCount = 0;
315+
await tester.pumpWidget(Directionality(
316+
textDirection: TextDirection.ltr,
317+
child: Stack(
318+
children: <Widget>[
319+
DraggableScrollableSheet(
320+
builder: (BuildContext context, ScrollController scrollController) {
321+
buildCount += 1;
322+
return Container(
323+
color: const Color(0xFFABCDEF),
324+
child: ListView.builder(
325+
controller: scrollController,
326+
itemExtent: 100,
327+
itemCount: 100,
328+
itemBuilder: (BuildContext context, int index) => Text('Item $index'),
329+
),
330+
);
331+
},
332+
),
333+
],
334+
),
335+
));
336+
expect(buildCount, 1);
337+
await tester.flingFrom(const Offset(0, 325), const Offset(0, -325), 200);
338+
expect(buildCount, 1);
339+
await tester.pumpAndSettle();
340+
expect(buildCount, 1);
341+
});
342+
343+
testWidgets('Builder is called if widget updates', (WidgetTester tester) async {
344+
int buildCount = 0;
345+
final GlobalKey key = GlobalKey();
346+
await tester.pumpWidget(Directionality(
347+
textDirection: TextDirection.ltr,
348+
child: Stack(
349+
children: <Widget>[
350+
DraggableScrollableSheet(
351+
key: key,
352+
builder: (BuildContext context, ScrollController scrollController) {
353+
buildCount += 1;
354+
return Container(
355+
color: const Color(0xFFABCDEF),
356+
child: ListView.builder(
357+
controller: scrollController,
358+
itemExtent: 100,
359+
itemCount: 100,
360+
itemBuilder: (BuildContext context, int index) => Text('Item $index'),
361+
),
362+
);
363+
},
364+
),
365+
],
366+
),
367+
));
368+
expect(buildCount, 1);
369+
expect(find.text('Item 1'), findsOneWidget);
370+
371+
await tester.pumpWidget(Directionality(
372+
textDirection: TextDirection.ltr,
373+
child: Stack(
374+
children: <Widget>[
375+
DraggableScrollableSheet(
376+
key: key,
377+
builder: (BuildContext context, ScrollController scrollController) {
378+
buildCount += 1;
379+
return Container(
380+
color: const Color(0xFFFEDCBA),
381+
child: ListView.builder(
382+
controller: scrollController,
383+
itemExtent: 50,
384+
itemCount: 100,
385+
itemBuilder: (BuildContext context, int index) => Text('New Item $index'),
386+
),
387+
);
388+
},
389+
),
390+
],
391+
),
392+
));
393+
expect(buildCount, 2);
394+
expect(find.text('Item 1'), findsNothing);
395+
expect(find.text('New Item 1'), findsOneWidget);
396+
});
397+
398+
testWidgets('Changes to min/max/initial child size are respected', (WidgetTester tester) async {
399+
final GlobalKey key = GlobalKey();
400+
final Key childKey = UniqueKey();
401+
await tester.pumpWidget(Directionality(
402+
textDirection: TextDirection.ltr,
403+
child: Stack(
404+
children: <Widget>[
405+
DraggableScrollableSheet(
406+
key: key,
407+
minChildSize: .25,
408+
maxChildSize: 1.0,
409+
initialChildSize: .5,
410+
builder: (BuildContext context, ScrollController scrollController) {
411+
return Container(
412+
key: childKey,
413+
color: const Color(0xFFABCDEF),
414+
child: ListView.builder(
415+
controller: scrollController,
416+
itemExtent: 100,
417+
itemCount: 100,
418+
itemBuilder: (BuildContext context, int index) => Text('Item $index'),
419+
),
420+
);
421+
},
422+
),
423+
],
424+
),
425+
));
426+
expect(find.text('Item 1'), findsOneWidget);
427+
expect(tester.getRect(find.byKey(childKey)), const Rect.fromLTRB(0, 300, 800, 600));
428+
429+
await tester.pumpWidget(Directionality(
430+
textDirection: TextDirection.ltr,
431+
child: Stack(
432+
children: <Widget>[
433+
DraggableScrollableSheet(
434+
key: key,
435+
minChildSize: .5,
436+
maxChildSize: .75,
437+
initialChildSize: .6,
438+
builder: (BuildContext context, ScrollController scrollController) {
439+
return Container(
440+
key: childKey,
441+
color: const Color(0xFFFEDCBA),
442+
child: ListView.builder(
443+
controller: scrollController,
444+
itemExtent: 50,
445+
itemCount: 100,
446+
itemBuilder: (BuildContext context, int index) => Text('New Item $index'),
447+
),
448+
);
449+
},
450+
),
451+
],
452+
),
453+
));
454+
expect(find.text('New Item 1'), findsOneWidget);
455+
expect(tester.getRect(find.byKey(childKey)), const Rect.fromLTRB(0, 240, 800, 600));
456+
});
312457
}

0 commit comments

Comments
 (0)