I'm not sure whether I should default to ClipOption.none here. It seems ironical to have no clip as default for a clipper. Currently, only ShapeBorderClipper default to ClipOption.none.

yeah the clipping widgets should probably default to Clip.antiAlias

Ack.

We should definitely have more documentation than this. Also, all documentation (here and elsewhere) should be fully grammatical sentences (e.g. end with a period).

Sure, writing now.

for these, we should follow the following convention (used elsewhere in the docs):

/// Brief sentence fragment that says what it does.
///
/// This is a longer sentence that describes the semantics of the value in detail,
/// including edge cases. There might be more sentences here.
///
/// Then, have a sentence or more than talks about the current performance
/// implications.
///
/// Finally, a brief comment if the feature is unlikely to be used.
///
/// See also:
///
///  * [SomethingElse], which is something relevant.

The "see also" block would be omitted if not useful. But e.g. antiAlias might point to Canvas.antiAlias.

So for example:

/// Clip, but do not apply anti-aliasing.
///
/// This mode enables clipping, but curves and non-axis-aligned straight lines will be
/// jagged as no effort is made to anti-alias.
///
/// Faster than other clipping modes, but slower than [none].
///
/// This is a reasonable choice when clipping is needed, if the container is an axis-
/// aligned rectangle or an axis-aligned rounded rectangle with very small corner radii.
///
/// See also:
///
///  * [antiAlias], which is more reasonable when clipping is needed and the shape is not
///    an axis-aligned rectangle.
hardEdge,

Thank you @Hixie for the sample writing! However, I couldn't find Canvas.antiAlias. It seems that we don't offer non-anti-aliased draw in Canvas?

Ah it's on Paint: https://master-docs-flutter-io.firebaseapp.com/flutter/dart-ui/Paint/isAntiAlias.html

assert clip != null

by the way if you come up with detailed docs for these and want to avoid copying-and-pasting them all over the place, and if they do in fact apply equally everywhere, then consider using a dartdoc macro/template to avoid duplication. See the child macro (used for the child property of most widgets) as an example.

Done.

the word "option" here doesn't seem to add much. Is Clip ok?

Yeah, that's Ok. Renaming now.

nit: extraneous blank line

nit: end sentences with a period

put a blank line between the two paragraphs

put the sentence starting with [Clip.none] in a separate paragraph.

(the first sentence is in its own paragraph because it's used in more places)

Done.

this says it's not allowed to be Clip.none, which seems reasonable, but you don't assert it above (which you do in other constructors) and you do support it in addToScene (which you don't in the others), which seems confusing. I think they should all be consistent (and probably not support none).

Thanks for spotting this! That's my intention and somehow I forgot to add assert to ClipRRectLayer, and enabled = clipBehavior != Clip.none to ClipRectLayer. (I guess that I somehow saw ClipRRectLayer as the same class as ClipRectLayer 😅 )

It seems weird that you support it in addToScene and indeed have a mutable field that can be set to none but don't allow it in the constructor. I hadn't realized it was a mutable field before (it makes sense that it is). Maybe the field should have a setter that asserts it's valid?

as in:

  Clip get clipBehavior => _clipBehavior;
  Clip _clipBehavior;
  set clipBehavior(Clip value) {
    assert(value != null);
    assert(value != Clip.none);
    _clipBehavior = value;
  }

if you did that you could remove the behaviour in addToScene that cares about none.

why are these on Layer? Seems like an odd place for them. Since they don't do anything with layers, i'd put them in the painting layer of our system.

Oops, misunderstood #18576 (comment) by only seeing layer at the end withot painting... Now fixed.

all identifiers of members in docs should be in [] every time (and all identifiers of arguments in docs should be in `` every time, but that doesn't apply here)

Done.

I think we should just expose the _clipBehavior directly, I don't see any reason to have a getter that defers to clipper sometimes.

Done.