Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix crash from invalid ListWheelViewport assertion #126539

Merged
merged 1 commit into from
May 13, 2023
Merged

Fix crash from invalid ListWheelViewport assertion #126539

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
52 changes: 19 additions & 33 deletions packages/flutter/lib/src/rendering/list_wheel_viewport.dart
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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