[DisplayList] Allow random access to ops through indexing

[flar]

Being able to reorder rendering commands leads to optimization opportunities in the graphics package. A graphics package being fed from a DisplayList either has to take the commands in the order given or implement their own storage format for the rendering data.

With this new dispatching mechanism, the graphics package can both query basic information about the recorded ops and even dispatch them by the index into the list. Query information includes either the "category" of the op (clip/transform/render, etc.) or a specific op type enum. The package can dispatch some categories (or ops) immediately and remember other categories (or ops) along with their state for dispatching later.

[flar]

Alternative implementation using a "Bookmark" concept: #54299

[flar]

Some notes on simplification, because it looks like a huge number of changes at first.

The main source of diffs was reorganizing the way that culled dispatching is done. It used to be a complicated interaction between the dispatch method maintaining state in a Culler object with 2 different implementations for culled and unculled dispatches. These objects then maintained yet more state in a DispatchContext object that was handed to the ops. The ops would then apply their own logic to the DispatchContext object to figure out if they should dispatch or skip themselves. The dispatching loop in Dispatch(receiver, ptr, end, culler) method then had to coordinate all of these objects.

All of that is deleted now and we simply get the vector of rects from the rtree, then a single method (RTreeResultsToIndexVector) turns that into a vector of DlIndex to dispatch. All of the culling logic is now encapsulated in that one method. The logic in that method should match all of the logic that used to be distributed between Culler, DispatchContext, all of the *Ops, and the Dispatch(receiver, ptr, end, culler) method.

It is worth noting that the performance of culled dispatching is better with this new arrangement even though an extra vector is allocated.

[flar]

Dispatch benchmarks before chnages:

Benchmark                                                Time             CPU   Iterations
------------------------------------------------------------------------------------------
BM_DisplayListDispatchDefault/kDefaultNoRtree         5.73 us         5.72 us       111735
BM_DisplayListDispatchDefault/kDefaultWithRtree       5.74 us         5.73 us       121210
BM_DisplayListDispatchCull/kCulledWithRtree           7.91 us         7.89 us        89708

After:

Benchmark                                                      Time             CPU   Iterations
------------------------------------------------------------------------------------------------
BM_DisplayListDispatchDefault/kDefaultNoRtree               6.15 us         6.12 us       104615
BM_DisplayListDispatchDefault/kDefaultWithRtree             6.10 us         6.07 us       114880
BM_DisplayListDispatchCull/kCulledWithRtree                 6.21 us         6.15 us       112998
BM_DisplayListDispatchByIndexDefault/kDefaultNoRtree        8.77 us         8.73 us        80377
BM_DisplayListDispatchByVectorDefault/kDefaultNoRtree       9.19 us         9.16 us        76627
BM_DisplayListDispatchByVectorCull/kCulledWithRtree         7.35 us         7.29 us        94698

Default dispatching took a minor 6% hit (keep in mind that these are microseconds and the DisplayList is huge), but culled dispatching got faster by 22%.

Dispatching manually by indices is slower, but the added flexibility of being able to control the dispatch order will hopefully make up for it - and these are measured in microseconds for a large display list.

---------------- Update ----------------

Made some good improvements by asserting the unsigned nature of the index type to prove that < 0 tests are unnecessary and also turned some FML_CHECK calls into FML_DCHECK because we should be able to trust our offsets_ vector as long as the index has been range checked...

Benchmark                                                        Time             CPU   Iterations
--------------------------------------------------------------------------------------------------
BM_DisplayListDispatchDefault/kDefaultNoRtree                 5.96 us         5.95 us       107729
BM_DisplayListDispatchDefault/kDefaultWithRtree               5.99 us         5.98 us       115678
BM_DisplayListDispatchCull/kCulledWithRtree                   6.06 us         6.05 us       115094
BM_DisplayListDispatchByIndexDefault/kDefaultNoRtree          7.28 us         7.25 us        96080
BM_DisplayListDispatchByIteratorDefault/kDefaultNoRtree       7.28 us         7.27 us        96347
BM_DisplayListDispatchByVectorDefault/kDefaultNoRtree         7.73 us         7.71 us        90363
BM_DisplayListDispatchByVectorCull/kCulledWithRtree           6.67 us         6.66 us       104709

The new data looks like default dispatch is better by a couple of %, but I think that is just run-to-run variance.
The overhead of Indexed iteration is down from 42% to just 21% and you can use an iterator paradigm for the same cost (for (auto i : *dl)).

[jonahwilliams]

Overhead in us seems small enough to not worry about. Will take more look at the code next week, but I like the idea and seems fairly straightforward

[jonahwilliams]

So this is really the meat of what this change allows: dispatch takes an offset and so I can choose to dispatch at any particular offset.

The tricky part is the op kinds right? We'd still need to manage things like paint state, clip state, transform state and distinguish between them when re-dispatching, correct?

[flar]

That's where GetOpCategory comes into play. You can easily figure out if it is a Save/Restore or a Rendering op or an Attribute/Transform/Clip op.

[flar]

With the latest change you can also do for (DlIndex i : *display_list) - woot...

[jonahwilliams]

Some other potential usage of this: we may compute that a saveLayer could be edlided during dispatch: it would be nice if we could manipulate the dispatch/iterator structure to skip to the corresponding restore. Right now with experimental canvas we need to rely on the cull rect checks

[flar]

Some other potential usage of this: we may compute that a saveLayer could be edlided during dispatch: it would be nice if we could manipulate the dispatch/iterator structure to skip to the corresponding restore. Right now with experimental canvas we need to rely on the cull rect checks

You just skip the Dispatch(receiver, index) calls until the category display_list->GetOpCategory(index) is Restore.

Check out the new DisplayListOpCategory enum in display_list.h - I figured that would include the necessary information you might need in order to control your dispatch. If not, it could be enhanced. I'd discourage using GetOpType as that is very specific and kind of relies on the current state of how we encode the various ops (for instance, there are 2 types associated with SaveLayer). It's useful for debugging mostly, but if we need to rely on it in any way during production-level iteration then we should look for other solutions.

[flar]

Right now with experimental canvas we need to rely on the cull rect checks

Is there a reason why this wouldn't already be culled by the culling form of dispatch? (You can still do culling with indexed dispatches, btw - you just use the new GetCulledIndices method to drive your dispatch loop.)

[jonahwilliams]

I'm not certain why it doesn't happen. I can say that there are scenarios with the aiks canvas today where aiks will cull save layers that display list does not.

[flar]

I'm not certain why it doesn't happen. I can say that there are scenarios with the aiks canvas today where aiks will cull save layers that display list does not.

Oh, right, like when the DL had to make a pessimal assumption about the destination size because it didn't actually have a destination in mind when it was recorded. Thus, there may be additional culling that can happen in the final stages like Impeller.

[jonahwilliams]

LGTM with minor nit

[jonahwilliams]

Minor nit, but I would prefer push_back if we're not using the aggregate initialization features of emplace_back.

[flar]

There were other places that also used emplace_back with a simple integer type (all in this PR) so I changed them all. Only one emplace_back that was actually invoking an aggregate constructor.