Now we have Azure everywhere, we can rip out a whole bunch of code from TLB. May as well rename it at the same time as step 1 towards not using 'Thebes' to mean 'content'. Then, rejoice.

Just to be clear: there is currently code that is completely not being used anywhere by any other code, and it can be removed. Right? What's getting renamed?

(In reply to Milan Sreckovic [:milan] from comment #1) > Just to be clear: there is currently code that is completely not being used > anywhere by any other code, and it can be removed. Right? Right. Needs some untangling too, and a little bit of Thebes->Azure conversion, but basically dead code removal. > What's getting renamed? ThebesLayerBuffer -> ContentLayerBuffer because it will no longer use Thebes, and to be more in line with ContentClient/Host.

We have an order in which we want to do the remaining -> Moz2D work. If possible, I'd rather not skip steps or take them out of order, and we should have separate bugs for all of them. This bug encompasses way too much inside of it, and it isn't clear what exactly will be going on. Really, a meta bug. For Moz2D, the order is: Remove asking Moz2D for Thebes (helper functions) Remove Moz2D Thebes interop / stop using gfxContext where equivalent Moz2D API exists Turn on D2D 1.1 Clear manual creation of gfxContext Simplify/de-branch gfxContext ... What you're mentioning in here fits into that picture, or did we miss something?

(In reply to Milan Sreckovic [:milan] from comment #3) > We have an order in which we want to do the remaining -> Moz2D work. If > possible, I'd rather not skip steps or take them out of order, and we should > have separate bugs for all of them. This bug encompasses way too much > inside of it, and it isn't clear what exactly will be going on. Really, a > meta bug. > > For Moz2D, the order is: > > Remove asking Moz2D for Thebes (helper functions) > Remove Moz2D Thebes interop / stop using gfxContext where equivalent Moz2D > API exists > Turn on D2D 1.1 > Clear manual creation of gfxContext > Simplify/de-branch gfxContext > ... > > What you're mentioning in here fits into that picture, or did we miss > something? I think this is separate from that lot. It is looking only at one class - ThebesLayerBuffer - which currently has separate code paths for Thebes and Moz2D and removing the Thebes one. In terms of order it is neither blocked by nor blocks any of those steps.

OK, we missed one. Bas, where should we insert this? I understand there are no dependencies, but we didn't sort the list just on dependencies, but on other benefits as well.

Not for landing, just for if anyone wants to apply this patch queue and build, you will need it. For landing we have to block on removing LayerManagerOGL.

note to self: need to land this in the Moz2D repo too.

Comment on attachment 825644 [details] [diff] [review] patch 1: remove the thebes paths from TLB Review of attachment 825644 [details] [diff] [review]: ----------------------------------------------------------------- We also probably need a patch to remove the azure.content.enabled pref, and also ensure that all platforms have a fallback azure backend that can't be removed with pref changes.

Comment on attachment 825653 [details] [diff] [review] patch 5: rename TLB Review of attachment 825653 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/RotatedBuffer.h @@ +77,5 @@ > const gfx::Matrix* aMaskTransform = nullptr) const; > > /** > * |BufferRect()| is the rect of device pixels that this > + * RotatedContentBuffer covers. That is what DrawBufferWithRotation() RotatedBuffer

(In reply to Milan Sreckovic [:milan] from comment #5) > OK, we missed one. Bas, where should we insert this? I understand there > are no dependencies, but we didn't sort the list just on dependencies, but > on other benefits as well. I think this is mainly a cleanup operation. In some place we added branches because we were using Moz2D and Thebes in parallel, I think a separate point would probably be: 'Remove existing branches to accommodate Thebes/Moz2D parallel usage' And it can probably go fairly high up as it will increase our ability to detect places accidentally still using a thebes gfxContext, as well as provide a large amount of code cleanup.

So looks like I was a bit optimistic - patch 3 doesn't work because we can pass in gfxContexts from layout/SVG to basic layers which aren't backed by a DrawTarget, and these find their way to TLB. I will try to land patch 1 and rebased versions of 4 and 5. I'll move patches 2 and 3 to another bug, although I bet by the time they can be applied, they will be rotten beyond use.

(In reply to Nick Cameron [:nrc] from comment #15) > So looks like I was a bit optimistic - patch 3 doesn't work because we can > pass in gfxContexts from layout/SVG to basic layers which aren't backed by a > DrawTarget, and these find their way to TLB. I will try to land patch 1 and > rebased versions of 4 and 5. I'll move patches 2 and 3 to another bug, > although I bet by the time they can be applied, they will be rotten beyond > use. Filed bug 934276 for this followup work.

Looks like I was still too optimistic and that even patch 1 fails due to getting non-DrawTarget gfxContexts from layout. So. Much. Sadness.

Try push for patch 1 only: https://tbpl.mozilla.org/?tree=Try&rev=9289fa217685

(In reply to Nick Cameron [:nrc] from comment #17) > Looks like I was still too optimistic and that even patch 1 fails due to > getting non-DrawTarget gfxContexts from layout. So. Much. Sadness. Actually, maybe not too optimistic - the test failures involve perspective, so maybe the gfxContext is due to an intermediate surface, not layout.

This one, I bet: http://mxr.mozilla.org/mozilla-central/source/gfx/layers/basic/BasicLayerManager.cpp#982

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

Comment on attachment 827062 [details] [diff] [review] patch 1.5 more azurification for patch 1 to work with 3d transforms Review of attachment 827062 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/basic/BasicLayerManager.cpp @@ +794,5 @@ > // Create a surface the size of the transformed object. > nsRefPtr<gfxASurface> dest = aDest->CurrentSurface(); > + nsRefPtr<gfxImageSurface> destImage = new gfxImageSurface(gfxIntSize(aDestRect.width, > + aDestRect.height), > + gfxImageFormatARGB32); Use Factory::CreateDataSourceSurface() here instead, and make the return value a TemporaryRef<SourceSurface> You'll need to add a gfxContext::SetSource overload that just takes a SourceSurface, but that should be easy.

Patches 2,3,4 have been reorganised a bit and moved to bug 934276.

https://hg.mozilla.org/integration/mozilla-inbound/rev/95f3ce0b7c96 https://hg.mozilla.org/integration/mozilla-inbound/rev/c38b37bee015 https://hg.mozilla.org/integration/mozilla-inbound/rev/d0ac87065f1a https://hg.mozilla.org/integration/mozilla-inbound/rev/35014d18ff1f

https://hg.mozilla.org/mozilla-central/rev/95f3ce0b7c96 https://hg.mozilla.org/mozilla-central/rev/c38b37bee015 https://hg.mozilla.org/mozilla-central/rev/d0ac87065f1a https://hg.mozilla.org/mozilla-central/rev/35014d18ff1f