[DisplayList] bookmarks allow random access dispatching

[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 do analysis on the properties of the rendering operations and then remember bookmarks into the operations so that they can later be dispatched in any order it prefers.

[jonahwilliams]

I like the idea!

From my understanding - we'd still need to manage tracking of things like paint state / CTM correct? Since for a drawRect, the paint would be updated before hand?

I think that would be reasonable since for draw ordering we'd likely just need to know opaque / non-opaque, though to actually re-order I guess we'd need to store the paint state too.

FYI @bdero

[flar]

I like the idea!

From my understanding - we'd still need to manage tracking of things like paint state / CTM correct? Since for a drawRect, the paint would be updated before hand?

I think that would be reasonable since for draw ordering we'd likely just need to know opaque / non-opaque, though to actually re-order I guess we'd need to store the paint state too.

FYI @bdero

Yes, I think I demonstrated that in the code sample in the doc comments with little detail on the "other state" data. Unfortunately, I don't think the code sample will compile because I decided to delete the copy/move constructor/operators for safety (the object holds dispatch state so copying it will sever that association).

[flar]

For example, here is a code sample that wouldn't work:

  DisplayList::Dispatcher dispatcher(dl);

  // Without due consideration, this constructor might copy the Dispatcher into
  // a field internally so it can call GetBookmark() on it.
  MyReceiver receiver(dispatcher);

  // This dispatch operation will track the progress inside the stack allocated
  // object above, but the MyReceiver object will ask for bookmarks from its
  // local copy - and thus get "invalid" bookmarks.
  dispatcher.Dispatch(receiver);

To prevent this use case, I disabled the copy/move operations and the above wouldn't compile even though it is a natural sequence of events.

This could be worked around by either having the MyReceiver object allocate its own Dispatcher and then use that Dispatcher to do the operation, or to have its constructor take a pointer or a reference to the stack allocated object - thereby opening up the possibility of a dangling ref/ptr.

We could rework this design to allow copying of Dispatcher objects if they implemented an internally shared State object between instances using a managed pointer, but that would mean it would be doing allocations even if you stack allocated it.

Note that Bookmark objects are shareable since they are roughly immutable, but copying/moving them around will end up doing inc/decref on the internal sk_sp.

[flar]

My next thought that I'm going to prototype is simply adding DisplayList::Dispatch(receiver, index) and advertise the total number of records. No "Bookmark" object (with a shared pointer back to the DL) needed and pretty safe as the index will be range-checked before dispatching. Similarly the dispatch-to-tracking-info association will be handled by simply having the caller drive the dispatch using indices from 0 to num_records.

  for (size_t i = 0; i < dl->RecordCount(); i++) {
    dl->Dispatch(my_receiver, i);
  }

I need a solution for how to dispatch with a cull_rect, though. Perhaps DisplayList::GetCulledRecords(SkRect) that returns an iterator or a list/vector. That's what happens internally during Dispatch(rect), but there is additional logic since the list of indices stored in the RTree only includes the rendering ops and some logic is needed to consider whether to dispatch the other ops.

Internally I will be reworking the storage to keep a sized list of offsets rather than having the records store a size in them. That frees up space in the records at the cost of a size_t per record stored in a list. uint32_t could be used to save space, but probably not a huge space consideration.

This solution would be much simpler from the outside as it doesn't need a series of coordinated objects that not only have to be kept consistent, but which also store expensive shared pointers.

[flar]

Alternative implementation using indexing: #54484

[flar]

Closed in favor of the indexing method implemented in #54484