Skip to content

Commit 7814641

Browse files
yiiimnate-thegratedkwingsmt
authored
fix fade_transition issue (flutter#157663)
Fixes: flutter#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<MyApp> createState() => _MyAppState(); } class _MyAppState extends State<MyApp> 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]. <!-- Links --> [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 <[email protected]> Co-authored-by: Tong Mu <[email protected]>
1 parent df330d5 commit 7814641

File tree

3 files changed

+41
-2
lines changed

3 files changed

+41
-2
lines changed

packages/flutter/lib/src/rendering/object.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2958,7 +2958,7 @@ abstract class RenderObject with DiagnosticableTreeMixin implements HitTestTarge
29582958
if (!isRepaintBoundary && _wasRepaintBoundary) {
29592959
_needsPaint = false;
29602960
_needsCompositedLayerUpdate = false;
2961-
owner?._nodesNeedingPaint.remove(this);
2961+
owner?._nodesNeedingPaint.removeWhere((RenderObject t) => identical(t, this));
29622962
_needsCompositingBitsUpdate = false;
29632963
markNeedsPaint();
29642964
} else if (oldNeedsCompositing != _needsCompositing) {

packages/flutter/test/rendering/object_test.dart

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,25 @@ void main() {
378378
expect(calledBack, true);
379379
});
380380

381+
test('Change isRepaintBoundary after both markNeedsCompositedLayerUpdate and markNeedsPaint', () {
382+
List<FlutterErrorDetails?>? caughtErrors;
383+
TestRenderingFlutterBinding.instance.onErrors = () {
384+
caughtErrors = TestRenderingFlutterBinding.instance.takeAllFlutterErrorDetails().toList();
385+
};
386+
final TestRenderObject object = TestRenderObject(allowPaintBounds: true);
387+
object.isRepaintBoundary = true;
388+
object.attach(TestRenderingFlutterBinding.instance.pipelineOwner);
389+
object.layout(const BoxConstraints.tightForFinite());
390+
PaintingContext.repaintCompositedChild(object, debugAlsoPaintedParent: true);
391+
392+
object.markNeedsCompositedLayerUpdate();
393+
object.markNeedsPaint();
394+
object.isRepaintBoundary = false;
395+
object.markNeedsCompositingBitsUpdate();
396+
TestRenderingFlutterBinding.instance.pumpCompleteFrame();
397+
expect(caughtErrors, isNull);
398+
});
399+
381400
test('ContainerParentDataMixin asserts parentData type', () {
382401
final TestRenderObject renderObject = TestRenderObjectWithoutSetupParentData();
383402
final TestRenderObject child = TestRenderObject();

packages/flutter/test/widgets/fade_transition_test.dart

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ void main() {
1010
testWidgets('FadeTransition', (WidgetTester tester) async {
1111
final DebugPrintCallback oldPrint = debugPrint;
1212
final List<String> log = <String>[];
13-
debugPrint = (String? message, { int? wrapWidth }) {
13+
debugPrint = (String? message, {int? wrapWidth}) {
1414
log.add(message!);
1515
};
1616
debugPrintBuildScope = true;
@@ -33,4 +33,24 @@ void main() {
3333
debugPrint = oldPrint;
3434
debugPrintBuildScope = false;
3535
});
36+
37+
// Regression test for https://github.com/flutter/flutter/issues/157312
38+
testWidgets('No exception when calling markNeedsPaint during opacity changes', (WidgetTester tester) async {
39+
final GlobalKey key = GlobalKey();
40+
final AnimationController controller = AnimationController(
41+
vsync: const TestVSync(),
42+
value: 1,
43+
duration: const Duration(seconds: 2),
44+
);
45+
addTearDown(controller.dispose);
46+
await tester.pumpWidget(FadeTransition(
47+
opacity: controller,
48+
child: Placeholder(key: key),
49+
));
50+
controller.value = 0.5;
51+
key.currentContext?.findRenderObject()?.markNeedsPaint();
52+
controller.value = 0;
53+
await tester.pump();
54+
expect(tester.takeException(), isNull);
55+
});
3656
}

0 commit comments

Comments
 (0)