Bug 93442 – [chromium] Base class for CCResourceProvider

• Component: New Bugs
• Status: RESOLVED WONTFIX

[Alexandre Elias]

[chromium] Base class for CCResourceProvider

[Alexandre Elias]

Created attachment 157134 [details]
Patch

[Alexandre Elias]

I haven't updated the tests yet, I'll do that tomorrow.

[Alexandre Elias]

Created attachment 157135 [details]
Patch

Actually added the CCResourceProviderGL.h/.cpp files

[Antoine Labour]

So I thought a bit more about this, and I'm thinking that it would be possibly better to handle the different type of resources inside a single CCResourceProvider class.
The rationale is 2-fold:
- I foresee some amount of duplicated code for meta-data tracking, etc. in the CCResourceProviderGL and the CCResourceProviderSoftware (or w/e). In particular I'm worried about creeping slight differences in how we check read/write lock state, which resource has been sent to the parent, pools, etc.
- at some point we may want to mix & match resource types, e.g. use the "software" resource type that doesn't involve a context (e.g. for tiled layer) at the same time as "3d" resource types for resources that need them.

I think CCResourceProvider::Resource could easily keep a ResourceImpl that itself has a virtual upload() and whatever else is needed instead of glId, and have different implementations on different resource types. Only when creating the resource would we need to do something different, but we could still share a bunch of the code.

WDYT?

[Alexandre Elias]

Seems like a good idea.  I'm still a bit vague on how it would look exactly, here are some random thoughts:

- Skia provides its own read/write lock.  Not sure what value ResourceProvider's locking stuff provides in the software context (I'm not clear on exactly the scenarios it protects against in the hardware case, either).  OTOH, it probably doesn't hurt to have two locking schemes on top of each other.

- For the mix-and-match, there's an asymmetry in that the leaf-compositor may want to mix-and-match, but the root-compositor wants to convert everything it receives into a unified format for the CCRenderer.  I wonder what the best way to express this would be.  For example, we could have a new class CCResourceReceiver, that would take care of converting received resources according to policy and feed them to the local CCResourceProvider.

[Antoine Labour]

(In reply to comment #5)
> Seems like a good idea.  I'm still a bit vague on how it would look exactly, here are some random thoughts:
> 
> - Skia provides its own read/write lock.  Not sure what value ResourceProvider's locking stuff provides in the software context (I'm not clear on exactly the scenarios it protects against in the hardware case, either).  OTOH, it probably doesn't hurt to have two locking schemes on top of each other.

Arguably, the locking-for-write stuff is to ensure proper behavior from the clients, it's only used in ASSERTs. But without it, it will get hairy to debug synchronization issues cross process. The lock-for-read stuff has visible impact though, through the isInUseByCompositor that will at some point be needed to figure out if you have to double-buffer your tiles or not.


Even with the 3D path, we'll want to add a couple of other ways to feed the data besides upload():
1- a lock/unlock scheme, that gives you the actual bits (used for the zero-copy path, saves 2 copies). This would follow similar rules as the  lockForWrite path, in that the resource must be !inUseByConsumer()
2- a mapBufferForUpload / commitBufferForUpload scheme, where you could get the buffer even if the resource is in-use. The commit would be passed to the parent (together with the sendFrameToParent scheme) that is responsible for doing the upload. Adds a copy, but the child won't have to double-buffer resources any more and can do partial updates, which may translate into a win.

I think both would make sense as well for the software path.

> 
> - For the mix-and-match, there's an asymmetry in that the leaf-compositor may want to mix-and-match, but the root-compositor wants to convert everything it receives into a unified format for the CCRenderer.  I wonder what the best way to express this would be.  For example, we could have a new class CCResourceReceiver, that would take care of converting received resources according to policy and feed them to the local CCResourceProvider.

Yes, I think we'll need something there. I believe we'll need to go to the parent to allocate the resource in either case (if only because the renderer can't create a SHM/TransportDIB without talking to the browser anyway), but the details of the transport mechanism could be hidden in a 2D allocator class (that we talked about in https://bugs.webkit.org/show_bug.cgi?id=90736 ) on the child side, and implemented by a CCResourceReceiver on the parent side.

[Alexandre Elias]

(In reply to comment #6)
> > - For the mix-and-match, there's an asymmetry in that the leaf-compositor may want to mix-and-match, but the root-compositor wants to convert everything it receives into a unified format for the CCRenderer.  I wonder what the best way to express this would be.  For example, we could have a new class CCResourceReceiver, that would take care of converting received resources according to policy and feed them to the local CCResourceProvider.
> 
> Yes, I think we'll need something there. I believe we'll need to go to the parent to allocate the resource in either case (if only because the renderer can't create a SHM/TransportDIB without talking to the browser anyway), but the details of the transport mechanism could be hidden in a 2D allocator class (that we talked about in https://bugs.webkit.org/show_bug.cgi?id=90736 ) on the child side, and implemented by a CCResourceReceiver on the parent side.

OK, two proposals come to mind:
1. The allocator could always pre-emptively create both a SHM and a texture of the same size, and at a low level they would always be kept tightly bundled together.  This magic would be invisible to the ResourceProviders -- the parent ResourceProvider will only see textureIds and the child ResourceProvider will only see SHM handles, and the allocator can accept either of them as a key (since the allocator wouldn't know about resourceIds).  For this to work, the creation/destruction and mailbox transfer stuff needs to be moved out of ResourceProvider.

2. Conversely, the ResourceProvider could explicitly orchestrate the whole thing.  In that case, a single ResourceImpl in the parent would have both a SHM handle and a textureId.  The parent would have to remember resources after sending them back to the client in order not to lose track of the textureId.

I think (1) is better encapsulated and easier to reason about, but (2) might open more optimization opportunities such as finer-grained locking.  Thoughts?

[Alexandre Elias]

Comment on attachment 157135 [details]
Patch

Removing r? on this patch; I'll work on the alternate approach.