From 42a81223147e364e113bcf2fb995fae66b0ca156 Mon Sep 17 00:00:00 2001 From: Amir Panahandeh Date: Thu, 27 Jan 2022 01:20:12 +0330 Subject: [PATCH] Mirror before scaling in _AnimatedIconPainter (#93312) --- AUTHORS | 1 + .../animated_icons/animated_icons.dart | 2 +- .../test/material/animated_icons_test.dart | 60 ++++++++++++++++--- .../animated_icons_private_test.dart.tmpl | 2 +- 4 files changed, 56 insertions(+), 9 deletions(-) diff --git a/AUTHORS b/AUTHORS index d30ad288ab..b11eaf45a6 100644 --- a/AUTHORS +++ b/AUTHORS @@ -91,3 +91,4 @@ Denis Grafov TheOneWithTheBraid Alberto Miola Twin Sun, LLC +Taskulu LDA diff --git a/packages/flutter/lib/src/material/animated_icons/animated_icons.dart b/packages/flutter/lib/src/material/animated_icons/animated_icons.dart index 08dfe2b70e..9187c905cd 100644 --- a/packages/flutter/lib/src/material/animated_icons/animated_icons.dart +++ b/packages/flutter/lib/src/material/animated_icons/animated_icons.dart @@ -156,11 +156,11 @@ class _AnimatedIconPainter extends CustomPainter { void paint(ui.Canvas canvas, Size size) { // The RenderCustomPaint render object performs canvas.save before invoking // this and canvas.restore after, so we don't need to do it here. - canvas.scale(scale, scale); if (shouldMirror) { canvas.rotate(math.pi); canvas.translate(-size.width, -size.height); } + canvas.scale(scale, scale); final double clampedProgress = progress.value.clamp(0.0, 1.0); for (final _PathFrames path in paths) diff --git a/packages/flutter/test/material/animated_icons_test.dart b/packages/flutter/test/material/animated_icons_test.dart index 5e0dffddf3..91d7439966 100644 --- a/packages/flutter/test/material/animated_icons_test.dart +++ b/packages/flutter/test/material/animated_icons_test.dart @@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +@Tags(['reduced-test-set']) + import 'dart:math' as math show pi; import 'package:flutter/material.dart'; @@ -26,6 +28,7 @@ class MockCanvas extends Fake implements Canvas { void scale(double sx, [double? sy]) { capturedSx = sx; capturedSy = sy!; + invocations.add(RecordedScale(sx, sy)); } final List invocations = []; @@ -75,6 +78,21 @@ class RecordedTranslate extends RecordedCanvasCall { int get hashCode => hashValues(dx, dy); } +class RecordedScale extends RecordedCanvasCall { + const RecordedScale(this.sx, this.sy); + + final double sx; + final double sy; + + @override + bool operator ==(Object other) { + return other is RecordedScale && other.sx == sx && other.sy == sy; + } + + @override + int get hashCode => hashValues(sx, sy); +} + void main() { testWidgets('IconTheme color', (WidgetTester tester) async { await tester.pumpWidget( @@ -218,9 +236,11 @@ void main() { data: IconThemeData( color: Color(0xFF666666), ), - child: AnimatedIcon( - progress: AlwaysStoppedAnimation(0.0), - icon: AnimatedIcons.arrow_menu, + child: RepaintBoundary( + child: AnimatedIcon( + progress: AlwaysStoppedAnimation(0.0), + icon: AnimatedIcons.arrow_menu, + ), ), ), ), @@ -231,7 +251,10 @@ void main() { expect(canvas.invocations, const [ RecordedRotate(math.pi), RecordedTranslate(-48, -48), + RecordedScale(0.5, 0.5), ]); + await expectLater(find.byType(AnimatedIcon), + matchesGoldenFile('animated_icons_test.icon.rtl.png')); }); testWidgets('Inherited text direction ltr', (WidgetTester tester) async { @@ -242,9 +265,11 @@ void main() { data: IconThemeData( color: Color(0xFF666666), ), - child: AnimatedIcon( - progress: AlwaysStoppedAnimation(0.0), - icon: AnimatedIcons.arrow_menu, + child: RepaintBoundary( + child: AnimatedIcon( + progress: AlwaysStoppedAnimation(0.0), + icon: AnimatedIcons.arrow_menu, + ), ), ), ), @@ -252,7 +277,11 @@ void main() { final CustomPaint customPaint = tester.widget(find.byType(CustomPaint)); final MockCanvas canvas = MockCanvas(); customPaint.painter!.paint(canvas, const Size(48.0, 48.0)); - expect(canvas.invocations, isEmpty); + expect(canvas.invocations, const [ + RecordedScale(0.5, 0.5), + ]); + await expectLater(find.byType(AnimatedIcon), + matchesGoldenFile('animated_icons_test.icon.ltr.png')); }); testWidgets('Inherited text direction overridden', (WidgetTester tester) async { @@ -277,8 +306,25 @@ void main() { expect(canvas.invocations, const [ RecordedRotate(math.pi), RecordedTranslate(-48, -48), + RecordedScale(0.5, 0.5), ]); }); + + testWidgets('Direction has no effect on position of widget', (WidgetTester tester) async { + const AnimatedIcon icon = AnimatedIcon( + progress: AlwaysStoppedAnimation(0.0), + icon: AnimatedIcons.arrow_menu, + ); + await tester.pumpWidget( + const Directionality(textDirection: TextDirection.rtl, child: icon), + ); + final Rect rtlRect = tester.getRect(find.byType(AnimatedIcon)); + await tester.pumpWidget( + const Directionality(textDirection: TextDirection.ltr, child: icon), + ); + final Rect ltrRect = tester.getRect(find.byType(AnimatedIcon)); + expect(rtlRect, ltrRect); + }); } PaintColorMatcher hasColor(int color) { diff --git a/packages/flutter/test_private/test/animated_icons_private_test.dart.tmpl b/packages/flutter/test_private/test/animated_icons_private_test.dart.tmpl index 8e2cc2ec75..482f76d355 100644 --- a/packages/flutter/test_private/test/animated_icons_private_test.dart.tmpl +++ b/packages/flutter/test_private/test/animated_icons_private_test.dart.tmpl @@ -187,9 +187,9 @@ void main() { ); painter.paint(mockCanvas, size); mockCanvas.verifyCallsInOrder([ - MockCall('scale', [1.0, 1.0]), MockCall('rotate', [math.pi]), MockCall('translate', [-48.0, -48.0]), + MockCall('scale', [1.0, 1.0]), MockCall.any('drawPath'), ]); });