From 7814641bd86617956881aa11e790b4fcfacaec7e Mon Sep 17 00:00:00 2001 From: yim Date: Wed, 11 Dec 2024 08:54:07 +0800 Subject: [PATCH] fix fade_transition issue (#157663) Fixes: #157312 A simpler way to reproduce this issue: ```dart import 'package:flutter/material.dart'; void main() { runApp(MyApp()); } class MyApp extends StatefulWidget { const MyApp({super.key}); @override State createState() => _MyAppState(); } class _MyAppState extends State with SingleTickerProviderStateMixin { late AnimationController controller = AnimationController( duration: const Duration(seconds: 2), value: 1, vsync: this, ); @override Widget build(BuildContext context) { return MaterialApp( home: Scaffold( body: Center( child: FadeTransition( opacity: controller, child: Builder( builder: (context) { return GestureDetector( onTap: () { controller.value = 0.5; context.findRenderObject()?.markNeedsPaint(); controller.value = 0; }, child: Text("Click"), ); }, ), ), ), ), ); } } ``` The main reason is that updating the opacity with `markNeedsCompositedLayerUpdate` followed by `markNeedsPaint` causes it to be added to `owner!._nodesNeedingPaint` twice. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [ ] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --------- Co-authored-by: Nate Wilson Co-authored-by: Tong Mu --- .../flutter/lib/src/rendering/object.dart | 2 +- .../flutter/test/rendering/object_test.dart | 19 ++++++++++++++++ .../test/widgets/fade_transition_test.dart | 22 ++++++++++++++++++- 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/packages/flutter/lib/src/rendering/object.dart b/packages/flutter/lib/src/rendering/object.dart index 9c0df3882c..140ee48a6e 100644 --- a/packages/flutter/lib/src/rendering/object.dart +++ b/packages/flutter/lib/src/rendering/object.dart @@ -2958,7 +2958,7 @@ abstract class RenderObject with DiagnosticableTreeMixin implements HitTestTarge if (!isRepaintBoundary && _wasRepaintBoundary) { _needsPaint = false; _needsCompositedLayerUpdate = false; - owner?._nodesNeedingPaint.remove(this); + owner?._nodesNeedingPaint.removeWhere((RenderObject t) => identical(t, this)); _needsCompositingBitsUpdate = false; markNeedsPaint(); } else if (oldNeedsCompositing != _needsCompositing) { diff --git a/packages/flutter/test/rendering/object_test.dart b/packages/flutter/test/rendering/object_test.dart index ae9b38c31d..f38c0f0055 100644 --- a/packages/flutter/test/rendering/object_test.dart +++ b/packages/flutter/test/rendering/object_test.dart @@ -378,6 +378,25 @@ void main() { expect(calledBack, true); }); + test('Change isRepaintBoundary after both markNeedsCompositedLayerUpdate and markNeedsPaint', () { + List? caughtErrors; + TestRenderingFlutterBinding.instance.onErrors = () { + caughtErrors = TestRenderingFlutterBinding.instance.takeAllFlutterErrorDetails().toList(); + }; + final TestRenderObject object = TestRenderObject(allowPaintBounds: true); + object.isRepaintBoundary = true; + object.attach(TestRenderingFlutterBinding.instance.pipelineOwner); + object.layout(const BoxConstraints.tightForFinite()); + PaintingContext.repaintCompositedChild(object, debugAlsoPaintedParent: true); + + object.markNeedsCompositedLayerUpdate(); + object.markNeedsPaint(); + object.isRepaintBoundary = false; + object.markNeedsCompositingBitsUpdate(); + TestRenderingFlutterBinding.instance.pumpCompleteFrame(); + expect(caughtErrors, isNull); + }); + test('ContainerParentDataMixin asserts parentData type', () { final TestRenderObject renderObject = TestRenderObjectWithoutSetupParentData(); final TestRenderObject child = TestRenderObject(); diff --git a/packages/flutter/test/widgets/fade_transition_test.dart b/packages/flutter/test/widgets/fade_transition_test.dart index b606c95137..4f8136cf6f 100644 --- a/packages/flutter/test/widgets/fade_transition_test.dart +++ b/packages/flutter/test/widgets/fade_transition_test.dart @@ -10,7 +10,7 @@ void main() { testWidgets('FadeTransition', (WidgetTester tester) async { final DebugPrintCallback oldPrint = debugPrint; final List log = []; - debugPrint = (String? message, { int? wrapWidth }) { + debugPrint = (String? message, {int? wrapWidth}) { log.add(message!); }; debugPrintBuildScope = true; @@ -33,4 +33,24 @@ void main() { debugPrint = oldPrint; debugPrintBuildScope = false; }); + + // Regression test for https://github.com/flutter/flutter/issues/157312 + testWidgets('No exception when calling markNeedsPaint during opacity changes', (WidgetTester tester) async { + final GlobalKey key = GlobalKey(); + final AnimationController controller = AnimationController( + vsync: const TestVSync(), + value: 1, + duration: const Duration(seconds: 2), + ); + addTearDown(controller.dispose); + await tester.pumpWidget(FadeTransition( + opacity: controller, + child: Placeholder(key: key), + )); + controller.value = 0.5; + key.currentContext?.findRenderObject()?.markNeedsPaint(); + controller.value = 0; + await tester.pump(); + expect(tester.takeException(), isNull); + }); }