`markNeedsPaint` has logical error

[fzyzcjy]

please see #112735

Copied from PR

The original code comment says:

If we're the root of the render tree (probably a RenderView), then we have to paint ourselves, since nobody else can paint us. We don't add ourselves to _nodesNeedingPaint in this case, because the root is always told to paint regardless.

However, IMHO it is wrong in two aspects.

Problem 1: RenderView does not come to this branch

Firstly, for a RenderView, it will not go into this branch, but instead go into the first branch (the if (isRepaintBoundary && _wasRepaintBoundary)). This is because RenderView.isRepaintBoundary is defined to be true, which can be seen in the code.

The experiment also confirms this. Click to expand below:

Add a few logs:

Code:

import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter/scheduler.dart';
import 'package:flutter_test/flutter_test.dart';

void main() {
  testWidgets('hello', (tester) async {
    debugPrintBeginFrameBanner = debugPrintEndFrameBanner = true;

    final dummy = ValueNotifier(0);
    await tester.pumpWidget(ValueListenableBuilder(
      valueListenable: dummy,
      builder: (_, dummy, __) => _DummyInner(dummy: dummy),
    ));

    dummy.value++;
    await tester.pump();

    debugPrintBeginFrameBanner = debugPrintEndFrameBanner = false;
  });
}

class _DummyInner extends SingleChildRenderObjectWidget {
  final int dummy;

  const _DummyInner({
    super.key,
    required this.dummy,
    super.child,
  });

  @override
  _RenderDummy createRenderObject(BuildContext context) =>
      _RenderDummy(dummy: dummy);

  @override
  void updateRenderObject(BuildContext context, _RenderDummy renderObject) {
    renderObject.dummy = dummy;
  }
}

class _RenderDummy extends RenderProxyBox {
  _RenderDummy({
    required int dummy,
    RenderBox? child,
  })  : _dummy = dummy,
        super(child);

  // not mark repaint yet
  int get dummy => _dummy;
  int _dummy;

  set dummy(int value) {
    if (_dummy == value) return;
    _dummy = value;
    print('hi ${describeIdentity(this)} set dummy thus markNeedsPaint START');
    markNeedsPaint();
    print('hi ${describeIdentity(this)} set dummy thus markNeedsPaint END');
  }

  @override
  void paint(PaintingContext context, Offset offset) {
    print('hi ${describeIdentity(this)}.paint SUPPOSE THIS IS THE REAL PAINT');
    super.paint(context, offset);
  }
}

output

/Volumes/MyExternal/ExternalRefCode/flutter/bin/flutter --no-color test --machine --start-paused --plain-name hello --local-engine-src-path=/Volumes/MyExternal/ExternalRefCode/engine/src --local-engine=host_debug_unopt test/hello.dart
Testing started at 09:21 ...

hi RenderParagraph#d9227.markNeedsPaint start _needsPaint=true
hi RenderPositionedBox#d9bb5.markNeedsPaint start _needsPaint=true
hi RenderView#fab3a.markNeedsPaint start _needsPaint=true
hi flushPaint PipelineOwner#89028 node=RenderView#fab3a NEEDS-PAINT _needsPaint=true owner=PipelineOwner#89028 node._layerHandle.layer!.attached=true
hi TransformLayer#64092.buildScene
hi PictureLayer#149d1._addToSceneWithRetainedRendering _needsAddToScene=true
▄▄▄▄▄▄▄▄ Frame 2                        0ms ▄▄▄▄▄▄▄▄
hi _RenderDummy#a17bc.markNeedsPaint start _needsPaint=true
hi RenderView#fab3a.markNeedsPaint start _needsPaint=false
hi RenderView#fab3a.markNeedsPaint case-repaintboundary owner=PipelineOwner#89028
hi flushPaint PipelineOwner#89028 node=RenderView#fab3a NEEDS-PAINT _needsPaint=true owner=PipelineOwner#89028 node._layerHandle.layer!.attached=true
hi _RenderDummy#a17bc.paint SUPPOSE THIS IS THE REAL PAINT
hi TransformLayer#64092.buildScene
▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀
▄▄▄▄▄▄▄▄ Frame 3                        0ms ▄▄▄▄▄▄▄▄
hi _RenderDummy#a17bc set dummy thus markNeedsPaint START
hi _RenderDummy#a17bc.markNeedsPaint start _needsPaint=false
hi _RenderDummy#a17bc.markNeedsPaint case-parent parent=RenderView#fab3a
hi RenderView#fab3a.markNeedsPaint start _needsPaint=false
hi RenderView#fab3a.markNeedsPaint case-repaintboundary owner=PipelineOwner#89028
hi _RenderDummy#a17bc set dummy thus markNeedsPaint END
hi flushPaint PipelineOwner#89028 node=RenderView#fab3a NEEDS-PAINT _needsPaint=true owner=PipelineOwner#89028 node._layerHandle.layer!.attached=true
hi _RenderDummy#a17bc.paint SUPPOSE THIS IS THE REAL PAINT
hi TransformLayer#64092.buildScene
▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀
hi RenderParagraph#c63b7.markNeedsPaint start _needsPaint=true
hi RenderPositionedBox#2fb99.markNeedsPaint start _needsPaint=true
hi RenderView#fab3a.markNeedsPaint start _needsPaint=false
hi RenderView#fab3a.markNeedsPaint case-repaintboundary owner=PipelineOwner#89028
hi flushPaint PipelineOwner#89028 node=RenderView#fab3a NEEDS-PAINT _needsPaint=true owner=PipelineOwner#89028 node._layerHandle.layer!.attached=true
hi TransformLayer#64092.buildScene
hi PictureLayer#18f7f._addToSceneWithRetainedRendering _needsAddToScene=true

By looking at the experiment above, we see that, the RenderView goes to the case-repaintboundary which is the first branch, instead of the third branch, so the comments seem incorrect.

Problem 2: Root is not always told to paint indeed

Theoretically, I do not find clues why "root is always told to paint" indeed. Experimentically, this is also confirmed as below.

We change the branching condition as follows, so RenderView is forced to go to the 3rd branch (the branch with comments), instead of the 1st branch.

-     if (isRepaintBoundary && _wasRepaintBoundary) {
+     if (isRepaintBoundary && _wasRepaintBoundary && /*HACK!!!*/(this is! RenderView)) {

Then we run the test code same as above (only with a few more logging), and get:

/Volumes/MyExternal/ExternalRefCode/flutter/bin/flutter --no-color test --machine --start-paused --plain-name hello --local-engine-src-path=/Volumes/MyExternal/ExternalRefCode/engine/src --local-engine=host_debug_unopt test/hello.dart
Testing started at 09:25 ...

hi RenderParagraph#c9872.markNeedsPaint start _needsPaint=true
hi RenderPositionedBox#f6896.markNeedsPaint start _needsPaint=true
hi RenderView#0fb85.markNeedsPaint start _needsPaint=true
hi flushPaint PipelineOwner#ce901 node=RenderView#0fb85 NEEDS-PAINT _needsPaint=true owner=PipelineOwner#ce901 node._layerHandle.layer!.attached=true
hi RenderView#0fb85.paint
hi TransformLayer#d7668.buildScene
hi PictureLayer#335b7._addToSceneWithRetainedRendering _needsAddToScene=true
▄▄▄▄▄▄▄▄ Frame 2                        0ms ▄▄▄▄▄▄▄▄
hi _RenderDummy#3427b.markNeedsPaint start _needsPaint=true
hi RenderView#0fb85.markNeedsPaint start _needsPaint=false
hi RenderView#0fb85.markNeedsPaint case-else owner=PipelineOwner#ce901
hi TransformLayer#d7668.buildScene
hi PictureLayer#335b7._addToSceneWithRetainedRendering _needsAddToScene=false
▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀
▄▄▄▄▄▄▄▄ Frame 3                        0ms ▄▄▄▄▄▄▄▄
hi _RenderDummy#3427b set dummy thus markNeedsPaint START
hi _RenderDummy#3427b.markNeedsPaint start _needsPaint=true
hi _RenderDummy#3427b set dummy thus markNeedsPaint END
hi TransformLayer#d7668.buildScene
hi PictureLayer#335b7._addToSceneWithRetainedRendering _needsAddToScene=false
▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀
hi RenderParagraph#66d6b.markNeedsPaint start _needsPaint=true
hi RenderPositionedBox#537a5.markNeedsPaint start _needsPaint=true
hi RenderView#0fb85.markNeedsPaint start _needsPaint=true
hi TransformLayer#d7668.buildScene
hi PictureLayer#335b7._addToSceneWithRetainedRendering _needsAddToScene=false

As we can see, RenderView.paint and RenderDummy.paint is only called once, even though we clearly call RenderDummy.markNeedsPaint. That is indeed a bug, and at least shows that the code comment is wrong - root is not always told to paint.

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.