Once we get rid of the Thebes paths from ThebesLayerBuffer (soon to be RotatedContentBuffer) we should replace mContext in ThebesLayerBuffer::PaintState with a draw target. This is a little bit complicated because we will have to pass around a transform (just a translation) for the offset into the buffer due to rotation. We then have to apply and remove this whenever we use the DT (or wrap it in a gfxContext to pass to layout). The WIP patch is basically missing that (the transform in the above para) so rendering is pretty broken (may also be broken for other reasons).

Hmm, so it looks like this would be pretty useful actually. To improve perf (esp. on Windows) we would like to keep hold of the DrawTarget underlying the TLB and have more control over flushing and locking. Nical - just checking you've not started this too.

(In reply to Nick Cameron [:nrc] from comment #1) > Nical - just checking you've not started this too. I haven't touched ContentClient stuff so far

Blocking bug 946567 because I want to reduce the amount of locking and flushing we do, and this makes it possible.

Comment on attachment 8346307 [details] [diff] [review] Change PaintState to use a DrawTarget instead of a gfxContext Review of attachment 8346307 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/RotatedBuffer.h @@ +206,2 @@ > DrawRegionClip mClip; > + bool mDidSelfCopy; Whitespace! @@ +356,5 @@ > */ > + TemporaryRef<gfx::DrawTarget> > + GetTargetForQuadrantUpdate(const nsIntRect& aBounds, > + ContextSource aSource, > + gfx::IntPoint* aTopLeft); This is a bit worrying. We're passing the top-left as an outparam and expecting any callers to set (and then restore) the appropriate transform. At minimum we need a comment here explaining what you need to use aTopLeft for, that you need to restore matrix change, and that you need to do so before trying to do anything else with this RotatedBuffer. Ideally I guess we'd want a different API that only gives temporary access to the DT, and can handle cleanup itself. ::: gfx/layers/client/ContentClient.cpp @@ +649,5 @@ > + } > + > + void Translate(const IntPoint& aOffset) > + { > + mOldTransform.Translate(-aOffset.x, -aOffset.y); Having this translate by the inverse of aOffset is weird. How about swapping this, and inverting at the caller.

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

Quite a lot has changed, so requesting re-review.

Comment on attachment 8348322 [details] [diff] [review] Change PaintState to use a DrawTarget instead of a gfxContextTLB_thebes.patch Review of attachment 8348322 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/RotatedBuffer.h @@ +222,2 @@ > DrawRegionClip mClip; > + bool mDidSelfCopy; Trailing whitespace

https://hg.mozilla.org/integration/mozilla-inbound/rev/950dfd7a4ead https://hg.mozilla.org/integration/mozilla-inbound/rev/5b8fa7087936

and out again: https://hg.mozilla.org/integration/mozilla-inbound/rev/4ef057726724

https://hg.mozilla.org/integration/mozilla-inbound/rev/aa070938b018 https://hg.mozilla.org/integration/mozilla-inbound/rev/acb5541fdea6

and a fixup: https://hg.mozilla.org/integration/mozilla-inbound/rev/b668ebb1d700

https://hg.mozilla.org/mozilla-central/rev/aa070938b018 https://hg.mozilla.org/mozilla-central/rev/acb5541fdea6 https://hg.mozilla.org/mozilla-central/rev/b668ebb1d700