[Impeller] Add FilterContents::GetSourceCoverage to enable filtered saveLayer clipping.

[flar]

The new method allows a caller to ask for the coverage of the input pixels required to produce an output that spans a given area. This allows the code to clip a saveLayer that has an image filter applied to it.

[bdero]

Sorry for the delay! Doing a lot of catchup today. At a glance this looks really great and I'm not spotting anything unexpected...

Would you mind adding a quick golden to aiks_unittests.cc that reproduces the broken case?

[flar]

One step closer, the culling of the saveLayer is fixed, but it now paints too much of the layer it creates to the screen. The layer size was expanded to collect source pixels for the filter, but the output needs to be clipped back to the SaveLayer bounds (if it had any).

Also, the test case clearly shows that the DrawPaint inside of the SaveLayer is not rendering to the whole texture.

[flutter-dashboard]

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

Changes reported for pull request #47183 at sha 83a72a6

[flar]

I seem to have gone down a route with the new method to pass along the Matrix being used, but that matrix is already installed in the filter being traversed earlier in the GetSubpassCoverage method. I could...

• Remove the installation of the Matrix when the filter is produced at the top of the method since it causes a deep rewrite of the filter tree and is redundant with the current method signature of GetSourceCoverage.
• Remove the Matrix argument from GetSourceCoverage since it is already installed in the filter chain.
• ???

I lean towards methods that carry along the data they need rather than modifying the objects to contain all of the data and having to update that frequently as it is removes temporary state from the objects, but it looks like the design of Impeller is including some of this state in the objects here and there. Not sure how to untangle this particular case.

[flar]

And I still need to look into those golden failures.

[flutter-dashboard]

Golden file changes are available for triage from new commit, Click here to view.

Changes reported for pull request #47183 at sha 384d7ca

[flar]

I'm suddenly concerned about the new optional functions that I added to rect.h.

There seems to be some disagreement about what a nullopt means for the coverage code. Sometimes it means "there is no restriction" and sometimes it means "there is no content" which are opposite interpretations. As a result, the interpretation these methods place on what a nullopt argument means might conflict with a given piece of code.

I think we need to untangle that and come up with a single interpretation of nullopt for a rectangle. It should perhaps be "NaN", as in "there is no answer to your question", which can happen in a couple of places - but it should never equal "empty bounds" or "infinite bounds" - we have empty rectangles and (broken) infinite rectangles to handle those cases.

[flutter-dashboard]

Golden file changes are available for triage from new commit, Click here to view.

Changes reported for pull request #47183 at sha c705e6a

[bdero]

Yeah I 100% agree w.r.t. the coverage conventions, added a bug around this suggesting a convention: flutter/flutter#137306