[Impeller] fix clip culling with exp canvas.

[jonahwilliams]

Fixes performance problem where image filters break clip culling, and lack of clip culling stops the clear color optimization from firing.

on the current canvas the cull rect computation is slightly incorrect, as we drop it as soon as we get a image filter. With the new canvas, we have the actual render target sizes, so we can correctly cull without it.

After switching to experimental canvas, I will remove the cull rect field from the canvas stack entry - as the clip coverage stack already performs basically the same culling.

This fixes the performance issue on the uncached zoom page transition where we lose the clear color optimization too early.

[jonahwilliams]

Once we switch to exp canvas we can delete the cull rect and just use the clip stack which is 99% the same data.

[flar]

I think the intention here is that if something outside of a DL applies an image filter then you need to cull the sub-dl by the "GetSourceCoverage" of that filter. No, it doesn't take that into account, the caller specifying the cull rect was supposed to...

[flar]

Oh, wait, layers. We do expand the bounds of sub-layers by a filter applied on a parent layer...

engine/display_list/dl_builder.cc

Line 680 in 09fa101

void DisplayListBuilder::TransferLayerBounds(const SkRect& content_bounds) {

If rtree, then here:

engine/display_list/dl_builder.cc

Line 722 in 09fa101

if (AdjustRTreeRects(rtree_data_.value(), *filter, matrix, clip,

And if no rtree, then here:

engine/display_list/dl_builder.cc

Line 730 in 09fa101

if (!filter->map_device_bounds(global_ibounds, matrix, global_ibounds)) {

[jonahwilliams]

This comment isn't really accurate. I updated it to document that the cull rect logic gets deleted with new canvas.

[flar]

The DL code can deal with non-axis alignment but unfortunately it doesn't have access to this information. If we are axis aligned then this is the fast answer, but if we aren't then a slightly slower answer is to reverse transform the existin clip bounds corners by the ctm and then see if they are inside the geometry.

See:

engine/display_list/utils/dl_matrix_clip_tracker.cc

Line 253 in 09fa101

bool DisplayListMatrixClipState::rect_covers_cull(const DlRect& content) const {

Also, it uses a different test - IsAligned2D instead of IsTranslationScaleOnly. Aligned allows for 90 degree rotations I think.

[flar]

Also, why only do this on the render pass size? Why not skip it if it covers the entire existing clip bounds?

[jonahwilliams]

hmm, we actually are also checking the clip bounds. These should already include the render pass size.

https://github.com/flutter/engine/blob/main/impeller/aiks/experimental_canvas.cc#L769-L797

Maybe this logic isn't necessary at all

[jonahwilliams]

Ahh, so the problem is that we always report the clip coverage as changing:

https://github.com/flutter/engine/blob/main/impeller/entity/entity_pass_clip_stack.cc#L69-L82

Even if the new clip coverage is bigger than the old clip coverage. We could move this check into the clip coverage stack and simplify this everywhere.

[jonahwilliams]

Yeah, lets just move this logic into the clip coverage stack

[flar]

Some questions.

[jonahwilliams]

Updated this to move the logic into clip coverage stack

[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 #54701 at sha 9464c00

[jonahwilliams]

Uh oh, seems very broken!

[jonahwilliams]

Ahh okay:

1. I forgot about difference clips.

2. The clip_contents returns the outer coverage of the clip, whereas for culling we need the inner rect. For now I've hacked so we flag non-axis aligned rects, but we can update impeller:Geometry with this concept and use that too.

[flar]

LGTM