add non-rendering operation culling to DisplayListBuilder

[flar]

This optimization avoids recording unnecessary render operations that will not affect the output and also eliminates the need for "draw detection" mechanisms like DlOpSpy and CanvasSpy by remembering if any non-transparent operations were included. The DlOpSpy unit tests were updated to check if the results from that object match the new DisplayList::affects_transparent_surface() method.

Fixes flutter/flutter#125338

In addition, this change will unblock some other Issues:

• [Impeller] Use CanvasSpy or DlOpSpy in more embedders flutter#125318
• DlOpSpy could be replaced by operation culling in DisplayListBuilder flutter#125403

[jonahwilliams]

This is probably related to or necessary for the changes I was thinking of, though from a different direction?

If I understand this PR, what you are doing is detecting operations that have no effect when they are added. However, with platform views what I was thinking of was more along the lines of adding operations that make previous operations irrelevant.

For example, I could draw a solid color rectangle, and then a drawPaint with src blend mode and fully transparent - I would want to no-op the entire display list. Today impeller gets these commands one at a time in the order they are added, so its extra hard to detect that there should be no rendering done.

[flar]

This is probably related to or necessary for the changes I was thinking of, though from a different direction?

If I understand this PR, what you are doing is detecting operations that have no effect when they are added. However, with platform views what I was thinking of was more along the lines of adding operations that make previous operations irrelevant.

For example, I could draw a solid color rectangle, and then a drawPaint with src blend mode and fully transparent - I would want to no-op the entire display list. Today impeller gets these commands one at a time in the order they are added, so its extra hard to detect that there should be no rendering done.

Is there an issue filed for that? Certainly the issue flutter/flutter#125318 that I was targetting said nothing about HSR.

[jonahwilliams]

Do you mean HSR as in hidden surface removal ? It doesn't have anything to do with that, just with the machinery by which the engine currently "splits up" a display list when there are platform views involved.

[jonahwilliams]

The example I gave in that bug was adding a difference clip and the resulting display list will render nothing as there isn't anything inside the difference clip.

[flar]

The example I gave in that bug was adding a difference clip and the resulting display list will render nothing as there isn't anything inside the difference clip.

That is an example that I understood from the issue that I linked to, but the description in your comment above talks about rendering operations that are covered by future operations which is an entirely different animal and which is essentially HSR on a per-operation basis.

If you are talking about clips and transparent rendering, this PR will eliminate many of those.

A difference clip might not be detected, though, as the clip is only tracked by bounds and so only specific types of clip operations are tracked. If a diff clip spans the horizontal or vertical extents of the current clip bounds, then it might reduce the tracked clip bounds, but more complicated diff clips are not going to be analyzed geometrically to see if they block out future rendering operations.

[jonahwilliams]

I don't have an opinion on the implementation, but my feature request is for a display list (specifically one created in the presence of platform views) to be able to tell me whether or not it renders anything.

[flar]

I don't have an opinion on the implementation, but my feature request is for a display list (specifically one created in the presence of platform views) to be able to tell me whether or not it renders anything.

That's a goal that can be approximated with increasing degrees of accuracy, but it can never be solved completely for many reasons. What kinds of operations do you want it to detect?

• blending plus source colors produce no effect?
• conservative estimates of op bounds do not intersect conservative estimates of clips?
  • how conservative an estimate?
  • under what transform? Remember that touching a pixel might touch different amounts of distance past the theoretical bounds of a given operation depending on the transform
• analyzing the filters and how they interact with each other to produce pixels that might not have an impact?
• analyzing custom color sources that are written as shaders?

This PR performs a number of operations with a fairly unobtrusive performance impact and with a moderate degree of accuracy to try to prevent forwarding operations which are provably invisible. It goes well beyond the scope of the existing mechanisms that had been used to track whether rendering is happening in various inter-embedding gaps.

[flar]

One issue that might arise is that CanvasSpy and DlOpSpy don't count an operation that is DrawColor(transparent, Src) which represents clearing the surface to transparent. When you are evaluating if creating a surface, rendering a DL to it, and then compositing that surface onto a scene, that operation shouldn't be "counted". But, when evaluating if a DL can modify a destination, it should be counted because there may have been pixels there that needed to be cleared away and the DL will do exactly that.

So, the new render culling created here can't distinguish that kind of operation. Perhaps we can count the number of non-clear ops and return that?

[jonahwilliams]

LGTM

[chinmaygarde]

Can we land this? Seems good to go on all fronts.

[chinmaygarde]

Update: @flar noticed a tiny regression in build times that he is still investigating.

[flar]

I was seeing about a 5-10% performance degradation running A/B tests for the (old) gallery transition perf, but @jonahwilliams suggested it might be due to lack of LTO on my local builds. I rebuilt my engine with LTO (profile) and the performance difference is negligible:

══╡ ••• Formatted summary for "/Users/jimgraham/Development/flutter/dev/devicelab/ABresults1.json" ••• ╞══

                   Score                     Average A (noise) Average B (noise) Speed-up
average_frame_build_time_millis                   1.72 (0.77%)      1.73 (1.03%)  0.99x  
worst_frame_build_time_millis                    51.56 (4.25%)     50.20 (3.94%)  1.03x  
90th_percentile_frame_build_time_millis           2.26 (2.02%)      2.30 (1.87%)  0.98x  
99th_percentile_frame_build_time_millis          17.19 (2.22%)     17.82 (1.23%)  0.96x  
average_frame_rasterizer_time_millis              5.66 (0.85%)      5.57 (0.96%)  1.02x  
worst_frame_rasterizer_time_millis               43.50 (0.79%)     42.81 (4.41%)  1.02x  
90th_percentile_frame_rasterizer_time_millis      7.30 (0.58%)      7.32 (0.18%)  1.00x  
99th_percentile_frame_rasterizer_time_millis     10.49 (7.50%)     10.70 (5.46%)  0.98x  

[jonahwilliams]

Thank you for looking into the performance more. Still LGTM!