Pretty much everything CompositorParent does on its LayerManager is actually Compositor stuff, we could cut out a whole bunch of redirect methods by having CompositorParent hold a reference to the Compositor instead of the LayerManager and let AsyncCompositionManager keep the LayerManager reference.

Yeah it's kinda silly to have all these methods of the form: Bar Foo() { return mCompositor->Foo() } What's AsyncCompositionManager ?

> What's AsyncCompositionManager ? oh nevermind, it's in bug 861727

It would be nice if LayerManager was just a LayerTree, (just a data structure) and the all of the logic would be in AsyncCompositionManager. That would be a step in the direction of reusing code on both the compositor and content sides.

(In reply to Nicolas Silva [:nical] from comment #3) > It would be nice if LayerManager was just a LayerTree, (just a data > structure) and the all of the logic would be in AsyncCompositionManager. > That would be a step in the direction of reusing code on both the > compositor and content sides. I think there are a lot of directions we could go, and some are mutually exclusive - like if we end up with just one layer type (or two) on the compositor, I'm not sure how much we could share with content. I kind of wanted to move AsyncCompositionManager into LayerManager, but maybe moving stuff out of LayerManager into AsyncCompositionManager is better, I don't know.

Well, I generally like when classes don't do too many things, and also like to separate logic from data. But After giving some thought about the LayerManager case, it probably doesn't have much to do as a data structure since the interesting stuff for that is in how ContainerLayer work. So it probably doesn't deserve it's own class for this aspect after all. (In reply to Nick Cameron [:nrc] from comment #4) > I think there are a lot of directions we could go, and some are mutually > exclusive - like if we end up with just one layer type (or two) on the > compositor, I'm not sure how much we could share with content. I guess it depends on how much logic we move from layers to components like compositables

(Mostly this is notes to self) Plan: 1. Figure out what to do about AndroidCompositorParent 2. Move mRenderBounds and its uses from LayerManagerComposite to Compositor 3. Split ShadowLayersUpdated and move most of it to AsyncCompositionManager (see also NotifyShadowTreeTransaction) 4. Fix initialisation (AllocPLayerTransaction) 5. Composite, CanComposite, and ComposeToTarget - not sure what to do here 6. Change uses of mLayerManager->GetCompositor(), change fields, tidy up remaining uses.

This is a bit yuck to have both a layer manager and a compositor, but eventually layer manager should go. This patch blocks bug 946958, which in turn makes the rest of this bug easier to resolve (although I haven't started that yet).

Comment on attachment 8344387 [details] [diff] [review] Part 2 add mCompositor Review of attachment 8344387 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/ipc/CompositorParent.cpp @@ +728,4 @@ > #endif > } > > + if (!compositor) { We could only hit this if we got passed an invalid layers backend type, we should just assert that this doesn't happen.

https://hg.mozilla.org/integration/mozilla-inbound/rev/41839e4026bc https://hg.mozilla.org/integration/mozilla-inbound/rev/9d1a4d83eccf

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/45193a6d7994 for extreme displeasure on the part of reftest-ipc

try push: https://tbpl.mozilla.org/?tree=Try&rev=d08bbd03b851

https://hg.mozilla.org/integration/mozilla-inbound/rev/96fd6d7d2f8c https://hg.mozilla.org/integration/mozilla-inbound/rev/8927d5297df1

https://hg.mozilla.org/mozilla-central/rev/96fd6d7d2f8c https://hg.mozilla.org/mozilla-central/rev/8927d5297df1