Fix crash in BackdropFilterLayer::Diff

[knopp]

Fixes flutter/flutter#95211

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
• I listed at least one issue that this PR fixes in the description above.
• I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
  writing and running engine tests.
• I updated/added relevant documentation (doc comments with ///).
• I signed the CLA.
• All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[flar]

LGTM.

I mentioned elsewhere that there are a number of places in the bounds calculation code that introduce the type of inaccuracy mentioned here, but if we can address at least one operation slightly more accurately with a small amount of code then that is fine.

[flar]

I just had a thought, though, that might indicate that this approach may be incorrect. It depends on how the filter is applied. If it is applied to the source content and then transformed then the roundOut is actually required in order to compute accurate bounds because content bounds that touch part of a pixel put a color into that entire pixel and you must take the bounds of the pixel data to compute the impact of the filter - in other words you must roundOut because that accurately takes the coverage of the border pixels into account.

In this case, you are computing the area of the source that contributes to the area under the backdrop content. In that case the backdrop content may extend from 0.5, 0.5 to 10.5, 10.5. In order to render that correctly, you need to include any pixel that contributes to the pixels hit by the 0.5 and the 10.5 - which means any pixel that contributes to 0,0 and 10.9999, 10.9999. So, the roundOut may actually be more "correct"?

[flar]

I had a thought that I left in the PR comments about whether or not the roundOut in the "non optimal" case actually better reflects the operation we are modeling here. I'm marking my review as "Request changes" until we evaluate that consideration.

[knopp]

I definitely might be missing something here, but as far as I can tell the filter is sampling the pixels from the underlying frame-buffer (which is in device space). So any rounding that needs to be done to get actual affected pixels should be done in device space (i.e. after transform).

So do I think this approach is correct. Whether it's worth the extra branch is another question (especially after seeing the cullrect of SkPicture).

[flar]

I look at it this way. The bounds start as the theoretical bounds for the content of the backdrop filter. They identify the pixels that will be modified.

The background of those pixels came from up to N pixels away - from the edges of those pixels (i.e. not from the edges of the theoretical bounds of the content).

So, to determine what of the background needs to be rendered, you must determine which background pixels contributed to any part of the pixels that will be touched by the inner content.

To compute this:

• find the bounds of the inner content (which is given here)
• find the bounds of the pixels that will be overwritten (i.e. roundOut)
• find the bounds of any pixels that contribute to any part of those bg pixels (filter bounds of integer bounds of the bg pixels)

After having written this, I realize that this is exactly what the typical branch of the code is doing, so ignore my comments.

[flar]

LGTM - my previous comments about a potential problem were off-base.

[knopp]

@flar, I pushed another variant where I instead of scaling the filter I pass the transform to filterBounds. It doesn't choke on perspective transform like makeWithLocalMatrix would and existing diff tests pass (they include scale layer in hierarchy so we know if filter bounds are properly scaled). I'll give this some more thought but it seems like this might be the way to go.

[knopp]

Looking at SkBlurImageFilter::onFilterNodeBounds for example it uses the specified ctm to scale sigma which seems like the thing we'd want it to do.

[TetrixGauss]

Hello all! After i updated to the latest version of Flutter, i faced the above problem! Is there something i could do? some temporary fix ?

[flar]

@TetrixGauss if you could be more specific on the version you discovered this on, the fix is currently being considered to be cherry picked for the stable release so it would be good to know if you found this on the stable branch or the ToT development branch.

Otherwise, the fix looks like it should be good for cherry-picking to the latest release train.

[knopp]

It should be cherry-picked for 2.10.1.

[itsjustkevin]

Added to the queue for 2.10.2.

[TetrixGauss]

@TetrixGauss if you could be more specific on the version you discovered this on, the fix is currently being considered to be cherry picked for the stable release so it would be good to know if you found this on the stable branch or the ToT development branch.

Otherwise, the fix looks like it should be good for cherry-picking to the latest release train.

@flar i just did flutter upgrade in stable channel and in version 2.10.0 and i found that bug! i dont know if that helps..

[TetrixGauss]

Hello all! does the 2.10.2 has the fix for the backdropFillter bug ?

[TetrixGauss]

Hello all! does the 2.10.2 has the fix for the backdropFillter bug ?

It's fixed!!! Thank you guys!!!