nit: _debugCheckForClipBehavior

Alternatively, remove the assert from this method and make it a bool Function() that could be passed directly to assert where it is called

Done

or I guess, more like a bool Function() Function(String)

What if I have two children, but I only want to clip one in my current config and to do that I call pushClipRect(clipBehavior: Clip.none) for one child and pushClipRect(clipBehavior: Clip.hardEdge) for the other one. describeApproximatePaintClip would return null for one of the children, but non-null for the other.

In that case, wouldn't I incorrectly see this error?

(I didn't see a test proving that this would work)

I think this error should work in that case, but I will add a test.

This seems like a very strange thing to do though, and it's not how any of the framework ROs are implemented today.

Added another test for this in object_test.dart.

I don't really understand how this can work for that case... the _debugCheckForClipBehavior method checks all the children, but is called for each pushed clip layer, which might only involve one child.

In general I don't think the PaintingContext has enough information to make this assert work, since it doesn't know which children are being painted in which layer. Some children might not be painted at all, some might be painted in one layer, some another, etc.

@goderbauer specifically the RO I added below does what I think you were asking for, and makes sure to return the right response from describeApproximatePaintClip based on which child you ask for.

My understanding is that this layout call doesn't actually execute the paint method of TestMultiClipBehaviorClipRenderObject because the phase parameter of layout defaults to EnginePhase.layout. To execute paint and exercise the code path in _debugCheckForClipBehavior that I am concerned about, can you change this line to:

I would expect that this test would now fail unexpectedly with the FlutterError thrown in _debugCheckForClipBehavior.

I'm calling the paint code explicitly in the next line.

I've tested this locally and the test still passes - would you prefer I update the test to expect that call returns normally instead of the more explicit call on PaintingContext.paintChild below?

You are of course right. I miss-read the next lines and paint is executed as expected. Apologies for that....

However, I still think the bug is there. Re-reading this test again, I think it is just hidden by the fact that TestMultiClipBehaviorClipRenderObject doesn't implement visitChildren correctly (it does not visit the specialChild). If you fix that and implement visitChildren in TestMultiClipBehaviorClipRenderObject correctly (i.e. as shown below) I am pretty sure that this test will fail.

  @override void visitChildren(RenderObjectVisitor visitor) {
    super.visitChildren(visitor);
    visitor(specialChild);
  }

Hah oops. I'll take a look

Yeah.

I think the two options are: remove the assert, or make it a (new) rule (that probably no one is breaking?) that RenderObjects must clip using the same clip behavior, and if you really want to clip using multiple clipBehaviors you have to create two separate RenderObjects to do it.

That seems like a slightly confusing rule, but it comes with the benefit of having a better error message when you mess things up in the much more common case of incorrectly applying your one and only clip behavior to children, and we could include details about it in the error message.

pushed/described?

this might be better named _debugCheckForNoneClipBehavior or some such, i was a bit confused at first as to why it was assuming the clip was "none" until i looked at all the call sites