Skip to content

Commit ae82f3c

Browse files
nt4f04uNdnt4f04uNd
authored
Fix crash from invalid ListWheelViewport assertion (#126539)
Fixes flutter/flutter#126491
1 parent 3ef9bc5 commit ae82f3c

3 files changed

Lines changed: 136 additions & 33 deletions

File tree

‎packages/flutter/lib/src/rendering/list_wheel_viewport.dart‎

Lines changed: 19 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1067,18 +1067,6 @@ class RenderListWheelViewport
10671067
return result;
10681068
}
10691069

1070-
static bool _debugAssertValidPaintTransform(ListWheelParentData parentData) {
1071-
if (parentData.transform == null) {
1072-
throw FlutterError(
1073-
'Child paint transform happened to be null. \n'
1074-
'$RenderListWheelViewport normally paints all of the children it has laid out. \n'
1075-
'Did you forget to update the $ListWheelParentData.transform during the paint() call? \n'
1076-
'If this is intetional, change or remove this assertion accordingly.'
1077-
);
1078-
}
1079-
return true;
1080-
}
1081-
10821070
static bool _debugAssertValidHitTestOffsets(String context, Offset offset1, Offset offset2) {
10831071
if (offset1 != offset2) {
10841072
throw FlutterError("$context - hit test expected values didn't match: $offset1 != $offset2");
@@ -1090,7 +1078,6 @@ class RenderListWheelViewport
10901078
void applyPaintTransform(RenderBox child, Matrix4 transform) {
10911079
final ListWheelParentData parentData = child.parentData! as ListWheelParentData;
10921080
final Matrix4? paintTransform = parentData.transform;
1093-
assert(_debugAssertValidPaintTransform(parentData));
10941081
if (paintTransform != null) {
10951082
transform.multiply(paintTransform);
10961083
}
@@ -1110,26 +1097,25 @@ class RenderListWheelViewport
11101097
while (child != null) {
11111098
final ListWheelParentData childParentData = child.parentData! as ListWheelParentData;
11121099
final Matrix4? transform = childParentData.transform;
1113-
assert(_debugAssertValidPaintTransform(childParentData));
1114-
final bool isHit = result.addWithPaintTransform(
1115-
transform: transform,
1116-
position: position,
1117-
hitTest: (BoxHitTestResult result, Offset transformed) {
1118-
assert(() {
1119-
if (transform == null) {
1120-
return _debugAssertValidHitTestOffsets('Null transform', transformed, position);
1121-
}
1122-
final Matrix4? inverted = Matrix4.tryInvert(PointerEvent.removePerspectiveTransform(transform));
1123-
if (inverted == null) {
1124-
return _debugAssertValidHitTestOffsets('Null inverted transform', transformed, position);
1125-
}
1126-
return _debugAssertValidHitTestOffsets('MatrixUtils.transformPoint', transformed, MatrixUtils.transformPoint(inverted, position));
1127-
}());
1128-
return child!.hitTest(result, position: transformed);
1129-
},
1130-
);
1131-
if (isHit) {
1132-
return true;
1100+
// Skip not painted children
1101+
if (transform != null) {
1102+
final bool isHit = result.addWithPaintTransform(
1103+
transform: transform,
1104+
position: position,
1105+
hitTest: (BoxHitTestResult result, Offset transformed) {
1106+
assert(() {
1107+
final Matrix4? inverted = Matrix4.tryInvert(PointerEvent.removePerspectiveTransform(transform));
1108+
if (inverted == null) {
1109+
return _debugAssertValidHitTestOffsets('Null inverted transform', transformed, position);
1110+
}
1111+
return _debugAssertValidHitTestOffsets('MatrixUtils.transformPoint', transformed, MatrixUtils.transformPoint(inverted, position));
1112+
}());
1113+
return child!.hitTest(result, position: transformed);
1114+
},
1115+
);
1116+
if (isHit) {
1117+
return true;
1118+
}
11331119
}
11341120
child = childParentData.previousSibling;
11351121
}

‎packages/flutter/test/cupertino/picker_test.dart‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import 'package:flutter/services.dart';
1010
import 'package:flutter_test/flutter_test.dart';
1111

1212
import '../rendering/mock_canvas.dart';
13+
import '../rendering/rendering_tester.dart';
1314

1415
class SpyFixedExtentScrollController extends FixedExtentScrollController {
1516
/// Override for test visibility only.
@@ -528,4 +529,61 @@ void main() {
528529
expect(controller.positions.length, 0);
529530
});
530531

532+
testWidgets('Registers taps and does not crash with certain diameterRatio', (WidgetTester tester) async {
533+
// Regression test for https://github.com/flutter/flutter/issues/126491
534+
535+
final List<int> children = List<int>.generate(100, (int index) => index);
536+
final List<int> paintedChildren = <int>[];
537+
final Set<int> tappedChildren = <int>{};
538+
539+
await tester.pumpWidget(CupertinoApp(
540+
home: Align(
541+
alignment: Alignment.topLeft,
542+
child: Center(
543+
child: SizedBox(
544+
height: 120,
545+
child: CupertinoPicker(
546+
itemExtent: 55,
547+
diameterRatio: 0.9,
548+
onSelectedItemChanged: (int index) {},
549+
children: children
550+
.map<Widget>((int index) =>
551+
GestureDetector(
552+
key: ValueKey<int>(index),
553+
onTap: () {
554+
tappedChildren.add(index);
555+
},
556+
child: SizedBox(
557+
width: 55,
558+
height: 55,
559+
child: CustomPaint(
560+
painter: TestCallbackPainter(onPaint: () {
561+
paintedChildren.add(index);
562+
}),
563+
),
564+
),
565+
),
566+
)
567+
.toList(),
568+
),
569+
),
570+
),
571+
),
572+
));
573+
574+
// Children are painted two times for whatever reason
575+
expect(paintedChildren, <int>[0, 1, 0, 1]);
576+
577+
// Expect hitting 0 and 1, which are painted
578+
await tester.tap(find.byKey(const ValueKey<int>(0)));
579+
expect(tappedChildren, const <int>[0]);
580+
581+
await tester.tap(find.byKey(const ValueKey<int>(1)));
582+
expect(tappedChildren, const <int>[0, 1]);
583+
584+
// The third child is not painted, so is not hit
585+
await tester.tap(find.byKey(const ValueKey<int>(2)), warnIfMissed: false);
586+
expect(tappedChildren, const <int>[0, 1]);
587+
});
588+
531589
}

‎packages/flutter/test/widgets/list_wheel_scroll_view_test.dart‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1642,6 +1642,65 @@ void main() {
16421642

16431643
expect(pageController.page, 1.0);
16441644
});
1645+
1646+
testWidgets('ListWheelScrollView does not crash and does not allow taps on children that were laid out, but not painted', (WidgetTester tester) async {
1647+
// Regression test for https://github.com/flutter/flutter/issues/12649
1648+
1649+
final FixedExtentScrollController controller = FixedExtentScrollController();
1650+
final List<int> children = List<int>.generate(100, (int index) => index);
1651+
final List<int> paintedChildren = <int>[];
1652+
final Set<int> tappedChildren = <int>{};
1653+
1654+
await tester.pumpWidget(
1655+
Directionality(
1656+
textDirection: TextDirection.ltr,
1657+
child: Center(
1658+
child: SizedBox(
1659+
height: 120,
1660+
child: ListWheelScrollView.useDelegate(
1661+
controller: controller,
1662+
physics: const FixedExtentScrollPhysics(),
1663+
diameterRatio: 0.9,
1664+
itemExtent: 55,
1665+
squeeze: 1.45,
1666+
childDelegate: ListWheelChildListDelegate(
1667+
children: children
1668+
.map((int index) => GestureDetector(
1669+
key: ValueKey<int>(index),
1670+
onTap: () {
1671+
tappedChildren.add(index);
1672+
},
1673+
child: SizedBox(
1674+
width: 55,
1675+
height: 55,
1676+
child: CustomPaint(
1677+
painter: TestCallbackPainter(onPaint: () {
1678+
paintedChildren.add(index);
1679+
}),
1680+
),
1681+
),
1682+
))
1683+
.toList(),
1684+
),
1685+
),
1686+
),
1687+
),
1688+
),
1689+
);
1690+
1691+
expect(paintedChildren, <int>[0, 1]);
1692+
1693+
// Expect hitting 0 and 1, which are painted
1694+
await tester.tap(find.byKey(const ValueKey<int>(0)));
1695+
expect(tappedChildren, const <int>[0]);
1696+
1697+
await tester.tap(find.byKey(const ValueKey<int>(1)));
1698+
expect(tappedChildren, const <int>[0, 1]);
1699+
1700+
// The third child is not painted, so is not hit
1701+
await tester.tap(find.byKey(const ValueKey<int>(2)), warnIfMissed: false);
1702+
expect(tappedChildren, const <int>[0, 1]);
1703+
});
16451704
});
16461705

16471706
testWidgets('ListWheelScrollView creates only one opacity layer for all children', (WidgetTester tester) async {

0 commit comments

Comments
 (0)