Skip to content

Commit

Permalink
Fix crash from invalid ListWheelViewport assertion (flutter#126539)
Browse files Browse the repository at this point in the history 
Fixes flutter#126491
  • Loading branch information
@nt4f04uNd
nt4f04uNd authored and Casey Hillers committed May 24, 2023
1 parent 12442a9 commit bf12d2b
Show file tree
Hide file tree
Showing 3 changed files with 136 additions and 33 deletions.
52 changes: 19 additions & 33 deletions packages/flutter/lib/src/rendering/list_wheel_viewport.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1067,18 +1067,6 @@ class RenderListWheelViewport
return result;
}

static bool _debugAssertValidPaintTransform(ListWheelParentData parentData) {
if (parentData.transform == null) {
throw FlutterError(
'Child paint transform happened to be null. \n'
'$RenderListWheelViewport normally paints all of the children it has laid out. \n'
'Did you forget to update the $ListWheelParentData.transform during the paint() call? \n'
'If this is intetional, change or remove this assertion accordingly.'
);
}
return true;
}

static bool _debugAssertValidHitTestOffsets(String context, Offset offset1, Offset offset2) {
if (offset1 != offset2) {
throw FlutterError("$context - hit test expected values didn't match: $offset1 != $offset2");
Expand All @@ -1090,7 +1078,6 @@ class RenderListWheelViewport
void applyPaintTransform(RenderBox child, Matrix4 transform) {
final ListWheelParentData parentData = child.parentData! as ListWheelParentData;
final Matrix4? paintTransform = parentData.transform;
assert(_debugAssertValidPaintTransform(parentData));
if (paintTransform != null) {
transform.multiply(paintTransform);
}
Expand All @@ -1110,26 +1097,25 @@ class RenderListWheelViewport
while (child != null) {
final ListWheelParentData childParentData = child.parentData! as ListWheelParentData;
final Matrix4? transform = childParentData.transform;
assert(_debugAssertValidPaintTransform(childParentData));
final bool isHit = result.addWithPaintTransform(
transform: transform,
position: position,
hitTest: (BoxHitTestResult result, Offset transformed) {
assert(() {
if (transform == null) {
return _debugAssertValidHitTestOffsets('Null transform', transformed, position);
}
final Matrix4? inverted = Matrix4.tryInvert(PointerEvent.removePerspectiveTransform(transform));
if (inverted == null) {
return _debugAssertValidHitTestOffsets('Null inverted transform', transformed, position);
}
return _debugAssertValidHitTestOffsets('MatrixUtils.transformPoint', transformed, MatrixUtils.transformPoint(inverted, position));
}());
return child!.hitTest(result, position: transformed);
},
);
if (isHit) {
return true;
// Skip not painted children
if (transform != null) {
final bool isHit = result.addWithPaintTransform(
transform: transform,
position: position,
hitTest: (BoxHitTestResult result, Offset transformed) {
assert(() {
final Matrix4? inverted = Matrix4.tryInvert(PointerEvent.removePerspectiveTransform(transform));
if (inverted == null) {
return _debugAssertValidHitTestOffsets('Null inverted transform', transformed, position);
}
return _debugAssertValidHitTestOffsets('MatrixUtils.transformPoint', transformed, MatrixUtils.transformPoint(inverted, position));
}());
return child!.hitTest(result, position: transformed);
},
);
if (isHit) {
return true;
}
}
child = childParentData.previousSibling;
}
Expand Down
58 changes: 58 additions & 0 deletions packages/flutter/test/cupertino/picker_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart';

import '../rendering/mock_canvas.dart';
import '../rendering/rendering_tester.dart';

class SpyFixedExtentScrollController extends FixedExtentScrollController {
/// Override for test visibility only.
Expand Down Expand Up @@ -528,4 +529,61 @@ void main() {
expect(controller.positions.length, 0);
});

testWidgets('Registers taps and does not crash with certain diameterRatio', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/126491

final List<int> children = List<int>.generate(100, (int index) => index);
final List<int> paintedChildren = <int>[];
final Set<int> tappedChildren = <int>{};

await tester.pumpWidget(CupertinoApp(
home: Align(
alignment: Alignment.topLeft,
child: Center(
child: SizedBox(
height: 120,
child: CupertinoPicker(
itemExtent: 55,
diameterRatio: 0.9,
onSelectedItemChanged: (int index) {},
children: children
.map<Widget>((int index) =>
GestureDetector(
key: ValueKey<int>(index),
onTap: () {
tappedChildren.add(index);
},
child: SizedBox(
width: 55,
height: 55,
child: CustomPaint(
painter: TestCallbackPainter(onPaint: () {
paintedChildren.add(index);
}),
),
),
),
)
.toList(),
),
),
),
),
));

// Children are painted two times for whatever reason
expect(paintedChildren, <int>[0, 1, 0, 1]);

// Expect hitting 0 and 1, which are painted
await tester.tap(find.byKey(const ValueKey<int>(0)));
expect(tappedChildren, const <int>[0]);

await tester.tap(find.byKey(const ValueKey<int>(1)));
expect(tappedChildren, const <int>[0, 1]);

// The third child is not painted, so is not hit
await tester.tap(find.byKey(const ValueKey<int>(2)), warnIfMissed: false);
expect(tappedChildren, const <int>[0, 1]);
});

}
59 changes: 59 additions & 0 deletions packages/flutter/test/widgets/list_wheel_scroll_view_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1642,6 +1642,65 @@ void main() {

expect(pageController.page, 1.0);
});

testWidgets('ListWheelScrollView does not crash and does not allow taps on children that were laid out, but not painted', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/12649

final FixedExtentScrollController controller = FixedExtentScrollController();
final List<int> children = List<int>.generate(100, (int index) => index);
final List<int> paintedChildren = <int>[];
final Set<int> tappedChildren = <int>{};

await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: Center(
child: SizedBox(
height: 120,
child: ListWheelScrollView.useDelegate(
controller: controller,
physics: const FixedExtentScrollPhysics(),
diameterRatio: 0.9,
itemExtent: 55,
squeeze: 1.45,
childDelegate: ListWheelChildListDelegate(
children: children
.map((int index) => GestureDetector(
key: ValueKey<int>(index),
onTap: () {
tappedChildren.add(index);
},
child: SizedBox(
width: 55,
height: 55,
child: CustomPaint(
painter: TestCallbackPainter(onPaint: () {
paintedChildren.add(index);
}),
),
),
))
.toList(),
),
),
),
),
),
);

expect(paintedChildren, <int>[0, 1]);

// Expect hitting 0 and 1, which are painted
await tester.tap(find.byKey(const ValueKey<int>(0)));
expect(tappedChildren, const <int>[0]);

await tester.tap(find.byKey(const ValueKey<int>(1)));
expect(tappedChildren, const <int>[0, 1]);

// The third child is not painted, so is not hit
await tester.tap(find.byKey(const ValueKey<int>(2)), warnIfMissed: false);
expect(tappedChildren, const <int>[0, 1]);
});
});

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

0 comments on commit bf12d2b

Please sign in to comment.