From c8b26707eedd1769dafda2edef8f5f1cb7776b4d Mon Sep 17 00:00:00 2001 From: Chris Jones Date: Tue, 14 Sep 2010 00:23:08 -0500 Subject: [PATCH] Bug 570625, part 9: When updating thebes layers, swap out back/front buffers and invalidate the newly-painted region on the old front buffer. r=roc sr=shaver --- gfx/layers/basic/BasicLayers.cpp | 147 +++++++++++++++++--------- gfx/layers/ipc/PLayers.ipdl | 12 +-- gfx/layers/ipc/ShadowLayers.cpp | 8 +- gfx/layers/ipc/ShadowLayers.h | 32 ++---- gfx/layers/ipc/ShadowLayersParent.cpp | 24 +++-- 5 files changed, 133 insertions(+), 90 deletions(-) diff --git a/gfx/layers/basic/BasicLayers.cpp b/gfx/layers/basic/BasicLayers.cpp index 7d13ce30cb5..44f307e811c 100644 --- a/gfx/layers/basic/BasicLayers.cpp +++ b/gfx/layers/basic/BasicLayers.cpp @@ -262,19 +262,17 @@ public: CreateBuffer(ContentType aType, const nsIntSize& aSize); /** - * Swap out the old backing buffer for |aBuffer|. - * - * CAVEAT EMPTOR: |aBuffer| must have the same dimensions and pixels - * as the previous buffer. If not, rendering glitches will occur. - * This is a rather dangerous and low-level interface added in bug - * 570625 as an intermediate step to a better interface. + * Swap out the old backing buffer for |aBuffer| and attributes. */ - void SetBackingBuffer(gfxASurface* aBuffer) + void SetBackingBuffer(gfxASurface* aBuffer, + const nsIntRect& aRect, const nsIntPoint& aRotation) { gfxIntSize prevSize = gfxIntSize(BufferDims().width, BufferDims().height); - NS_ABORT_IF_FALSE(aBuffer->GetSize() == prevSize, + gfxIntSize newSize = aBuffer->GetSize(); + NS_ABORT_IF_FALSE(newSize == prevSize, "Swapped-in buffer size doesn't match old buffer's!"); - SetBuffer(aBuffer, BufferDims(), BufferRect(), BufferRotation()); + SetBuffer(aBuffer, + nsIntSize(newSize.width, newSize.height), aRect, aRotation); } private: @@ -1220,6 +1218,7 @@ BasicLayerManager::CreateCanvasLayer() #ifdef MOZ_IPC +class BasicShadowableThebesLayer; class BasicShadowableLayer : public ShadowableLayer { public: @@ -1262,6 +1261,8 @@ public: // shutdown anyway). mShadow = nsnull; } + + virtual BasicShadowableThebesLayer* AsThebes() { return nsnull; } }; static ShadowableLayer* @@ -1370,11 +1371,17 @@ public: virtual PRBool SupportsSurfaceDescriptor() const { return PR_TRUE; } - virtual void SetBackBuffer(const SurfaceDescriptor& aBuffer) + void SetBackBufferAndAttrs(const ThebesBuffer& aBuffer, + const nsIntRegion& aValidRegion, + float aXResolution, float aYResolution) { - mBackBuffer = aBuffer; + mBackBuffer = aBuffer.buffer(); + mValidRegion = aValidRegion; + mXResolution = aXResolution; + mYResolution = aYResolution; + nsRefPtr backBuffer = BasicManager()->OpenDescriptor(mBackBuffer); - mBuffer.SetBackingBuffer(backBuffer); + mBuffer.SetBackingBuffer(backBuffer, aBuffer.rect(), aBuffer.rotation()); } virtual void Disconnect() @@ -1383,6 +1390,8 @@ public: BasicShadowableLayer::Disconnect(); } + virtual BasicShadowableThebesLayer* AsThebes() { return this; } + private: BasicShadowLayerManager* BasicManager() { @@ -1421,6 +1430,7 @@ BasicShadowableThebesLayer::PaintBuffer(gfxContext* aContext, "should have a back buffer by now"); BasicManager()->PaintedThebesBuffer(BasicManager()->Hold(this), + aRegionToDraw, mBuffer.BufferRect(), mBuffer.BufferRotation(), mBackBuffer); @@ -1695,24 +1705,20 @@ public: MOZ_COUNT_DTOR(ShadowThebesLayerBuffer); } - already_AddRefed - Swap(gfxASurface* aNewFrontBuffer, const nsIntSize& aBufferDims, - const nsIntRect& aBufferRect, - const nsIntPoint& aRotation=nsIntPoint(0, 0)) + void Swap(gfxASurface* aNewBuffer, + const nsIntRect& aNewRect, const nsIntPoint& aNewRotation, + gfxASurface** aOldBuffer, + nsIntRect* aOldRect, nsIntPoint* aOldRotation) { - nsRefPtr newBackBuffer = SetBuffer(aNewFrontBuffer, - aBufferDims, - aBufferRect, aRotation); - if (newBackBuffer && aNewFrontBuffer) { - // Copy the new pixels in the new front buffer to our previous - // front buffer. This is intended to be optimized! Many - // factors are involved. - nsRefPtr tmpCtx = new gfxContext(newBackBuffer); - tmpCtx->SetOperator(gfxContext::OPERATOR_SOURCE); - tmpCtx->DrawSurface(aNewFrontBuffer, - gfxIntSize(aBufferDims.width, aBufferDims.height)); - } - return newBackBuffer.forget(); + *aOldRect = BufferRect(); + *aOldRotation = BufferRotation(); + + gfxIntSize newSize = aNewBuffer->GetSize(); + nsRefPtr oldBuffer; + oldBuffer = SetBuffer(aNewBuffer, + nsIntSize(newSize.width, newSize.height), + aNewRect, aNewRotation); + oldBuffer.forget(aOldBuffer); } protected: @@ -1724,10 +1730,13 @@ protected: } }; + class BasicShadowThebesLayer : public ShadowThebesLayer, BasicImplData { public: - BasicShadowThebesLayer(BasicShadowLayerManager* aLayerManager) : - ShadowThebesLayer(aLayerManager, static_cast(this)) + BasicShadowThebesLayer(BasicShadowLayerManager* aLayerManager) + : ShadowThebesLayer(aLayerManager, static_cast(this)) + , mOldXResolution(1.0) + , mOldYResolution(1.0) { MOZ_COUNT_CTOR(BasicShadowThebesLayer); } @@ -1739,21 +1748,37 @@ public: MOZ_COUNT_DTOR(BasicShadowThebesLayer); } + virtual void SetValidRegion(const nsIntRegion& aRegion) + { + mOldValidRegion = mValidRegion; + ShadowThebesLayer::SetValidRegion(aRegion); + } + + virtual void SetResolution(float aXResolution, float aYResolution) + { + mOldXResolution = mXResolution; + mOldYResolution = mYResolution; + ShadowThebesLayer::SetResolution(aXResolution, aYResolution); + } + virtual void Disconnect() { DestroyFrontBuffer(); ShadowThebesLayer::Disconnect(); } - virtual SurfaceDescriptor - Swap(const SurfaceDescriptor& aNewFront, - const nsIntRect& aBufferRect, - const nsIntPoint& aRotation); + virtual void + Swap(const ThebesBuffer& aNewFront, const nsIntRegion& aUpdatedRegion, + ThebesBuffer* aNewBack, nsIntRegion* aNewBackValidRegion, + float* aNewXResolution, float* aNewYResolution); virtual void DestroyFrontBuffer() { - nsRefPtr unused = - mFrontBuffer.Swap(nsnull, nsIntSize(), nsIntRect()); + mFrontBuffer.Clear(); + mOldValidRegion.SetEmpty(); + mOldXResolution = 1.0; + mOldYResolution = 1.0; + if (IsSurfaceDescriptorValid(mFrontBufferDescriptor)) { BasicManager()->ShadowLayerManager::DestroySharedSurface(&mFrontBufferDescriptor); } @@ -1773,24 +1798,38 @@ private: ShadowThebesLayerBuffer mFrontBuffer; // Describes the gfxASurface we hand out to |mFrontBuffer|. SurfaceDescriptor mFrontBufferDescriptor; + // When we receive an update from our remote partner, we stow away + // our previous parameters that described our previous front buffer. + // Then when we Swap() back/front buffers, we can return these + // parameters to our partner (adjusted as needed). + nsIntRegion mOldValidRegion; + float mOldXResolution; + float mOldYResolution; }; -SurfaceDescriptor -BasicShadowThebesLayer::Swap(const SurfaceDescriptor& aNewFront, - const nsIntRect& aBufferRect, - const nsIntPoint& aRotation) +void +BasicShadowThebesLayer::Swap(const ThebesBuffer& aNewFront, + const nsIntRegion& aUpdatedRegion, + ThebesBuffer* aNewBack, + nsIntRegion* aNewBackValidRegion, + float* aNewXResolution, float* aNewYResolution) { - SurfaceDescriptor newBackBuffer = mFrontBufferDescriptor; + aNewBack->buffer() = mFrontBufferDescriptor; + // We have to invalidate the pixels painted into the new buffer. + // They might overlap with our old pixels. + aNewBackValidRegion->Sub(mOldValidRegion, aUpdatedRegion); + *aNewXResolution = mOldXResolution; + *aNewYResolution = mOldYResolution; + nsRefPtr newFrontBuffer = - BasicManager()->OpenDescriptor(aNewFront); - gfxIntSize size = newFrontBuffer->GetSize(); + BasicManager()->OpenDescriptor(aNewFront.buffer()); - nsRefPtr unused = - mFrontBuffer.Swap(newFrontBuffer, nsIntSize(size.width, size.height), - aBufferRect, aRotation); - mFrontBufferDescriptor = aNewFront; + nsRefPtr unused; + mFrontBuffer.Swap( + newFrontBuffer, aNewFront.rect(), aNewFront.rotation(), + getter_AddRefs(unused), &aNewBack->rect(), &aNewBack->rotation()); - return newBackBuffer; + mFrontBufferDescriptor = aNewFront.buffer(); } void @@ -2149,6 +2188,16 @@ BasicShadowLayerManager::EndTransaction(DrawThebesLayerCallback aCallback, const EditReply& reply = replies[i]; switch (reply.type()) { + case EditReply::TOpThebesBufferSwap: { + MOZ_LAYERS_LOG(("[LayersForwarder] ThebesBufferSwap")); + + const OpThebesBufferSwap& obs = reply.get_OpThebesBufferSwap(); + BasicShadowableThebesLayer* thebes = GetBasicShadowable(obs)->AsThebes(); + thebes->SetBackBufferAndAttrs( + obs.newBackBuffer(), + obs.newValidRegion(), obs.newXResolution(), obs.newYResolution()); + break; + } case EditReply::TOpBufferSwap: { MOZ_LAYERS_LOG(("[LayersForwarder] BufferSwap")); diff --git a/gfx/layers/ipc/PLayers.ipdl b/gfx/layers/ipc/PLayers.ipdl index e83973d99c3..1c35025a05d 100644 --- a/gfx/layers/ipc/PLayers.ipdl +++ b/gfx/layers/ipc/PLayers.ipdl @@ -160,6 +160,7 @@ struct ThebesBuffer { struct OpPaintThebesBuffer { PLayer layer; ThebesBuffer newFrontBuffer; + nsIntRegion updatedRegion; }; struct OpPaintCanvas { @@ -204,22 +205,19 @@ union Edit { // Replies to operations struct OpBufferSwap { PLayer layer; SurfaceDescriptor newBackBuffer; }; -/* - * XXX: if we choose *not* to do a new-front-to-new-back fill in the - * parent process, then we'll need to send back the old buffer - * attributes so that it can be filled anew. - * struct OpThebesBufferSwap { PLayer layer; ThebesBuffer newBackBuffer; + nsIntRegion newValidRegion; + float newXResolution; + float newYResolution; }; -*/ // Unit of a "changeset reply". This is a weird abstraction, probably // only to be used for buffer swapping. union EditReply { OpBufferSwap; -// OpThebesBufferSwap; + OpThebesBufferSwap; }; diff --git a/gfx/layers/ipc/ShadowLayers.cpp b/gfx/layers/ipc/ShadowLayers.cpp index 3f312a940fe..d39a4a874cf 100644 --- a/gfx/layers/ipc/ShadowLayers.cpp +++ b/gfx/layers/ipc/ShadowLayers.cpp @@ -253,14 +253,16 @@ ShadowLayerForwarder::RemoveChild(ShadowableLayer* aContainer, void ShadowLayerForwarder::PaintedThebesBuffer(ShadowableLayer* aThebes, - nsIntRect aBufferRect, - nsIntPoint aBufferRotation, + const nsIntRegion& aUpdatedRegion, + const nsIntRect& aBufferRect, + const nsIntPoint& aBufferRotation, const SurfaceDescriptor& aNewFrontBuffer) { mTxn->AddEdit(OpPaintThebesBuffer(NULL, Shadow(aThebes), ThebesBuffer(aNewFrontBuffer, aBufferRect, - aBufferRotation))); + aBufferRotation), + aUpdatedRegion)); } void ShadowLayerForwarder::PaintedImage(ShadowableLayer* aImage, diff --git a/gfx/layers/ipc/ShadowLayers.h b/gfx/layers/ipc/ShadowLayers.h index 7e8afc972ba..0e5aaa61670 100644 --- a/gfx/layers/ipc/ShadowLayers.h +++ b/gfx/layers/ipc/ShadowLayers.h @@ -61,6 +61,7 @@ class ShadowThebesLayer; class ShadowImageLayer; class ShadowCanvasLayer; class SurfaceDescriptor; +class ThebesBuffer; class Transaction; /** @@ -207,8 +208,9 @@ public: * is buffer's rotation, if any. */ void PaintedThebesBuffer(ShadowableLayer* aThebes, - nsIntRect aBufferRect, - nsIntPoint aBufferRotation, + const nsIntRegion& aUpdatedRegion, + const nsIntRect& aBufferRect, + const nsIntPoint& aBufferRotation, const SurfaceDescriptor& aNewFrontBuffer); /** * NB: this initial implementation only forwards RGBA data for @@ -399,7 +401,7 @@ public: /** * CONSTRUCTION PHASE ONLY */ - void SetValidRegion(const nsIntRegion& aRegion) + virtual void SetValidRegion(const nsIntRegion& aRegion) { mValidRegion = aRegion; Mutated(); @@ -408,7 +410,7 @@ public: /** * CONSTRUCTION PHASE ONLY */ - void SetResolution(float aXResolution, float aYResolution) + virtual void SetResolution(float aXResolution, float aYResolution) { mXResolution = aXResolution; mYResolution = aYResolution; @@ -421,25 +423,11 @@ public: * Publish the remote layer's back ThebesLayerBuffer to this shadow, * swapping out the old front ThebesLayerBuffer (the new back buffer * for the remote layer). - * - * XXX should the receiving process blit updates from the new front - * buffer to the previous front buffer (new back buffer) while it has - * access to the new front buffer? Or is it better to fill the - * updates bits in anew on the new back buffer? - * - * Seems like memcpy()s from new-front to new-back would have to - * always be no slower than any kind of fill from content, so one - * would expect the former to win in terms of total throughput. - * However, that puts the blit on the critical path of - * publishing-process-blocking-on-receiving-process, so - * responsiveness might suffer, pointing to the latter. Experience - * will tell! (Maybe possible to choose between both depending on - * size of blit vs. expense of re-fill?) */ - virtual SurfaceDescriptor - Swap(const SurfaceDescriptor& aNewFront, - const nsIntRect& aBufferRect, - const nsIntPoint& aRotation) = 0; + virtual void + Swap(const ThebesBuffer& aNewFront, const nsIntRegion& aUpdatedRegion, + ThebesBuffer* aNewBack, nsIntRegion* aNewBackValidRegion, + float* aNewXResolution, float* aNewYResolution) = 0; /** * CONSTRUCTION PHASE ONLY diff --git a/gfx/layers/ipc/ShadowLayersParent.cpp b/gfx/layers/ipc/ShadowLayersParent.cpp index 4911db063bc..32787b93afd 100644 --- a/gfx/layers/ipc/ShadowLayersParent.cpp +++ b/gfx/layers/ipc/ShadowLayersParent.cpp @@ -189,9 +189,12 @@ ShadowLayersParent::RecvUpdate(const nsTArray& cset, ShadowThebesLayer* thebes = static_cast( AsShadowLayer(otb)->AsLayer()); - unused << thebes->Swap(otb.initialFront(), - otb.bufferRect(), - nsIntPoint(0, 0)); + ThebesBuffer unusedBuffer; + nsIntRegion unusedRegion; float unusedXRes, unusedYRes; + thebes->Swap( + ThebesBuffer(otb.initialFront(), otb.bufferRect(), nsIntPoint(0, 0)), + unusedRegion, + &unusedBuffer, &unusedRegion, &unusedXRes, &unusedYRes); break; } @@ -368,12 +371,15 @@ ShadowLayersParent::RecvUpdate(const nsTArray& cset, static_cast(shadow->AsLayer()); const ThebesBuffer& newFront = op.newFrontBuffer(); - SurfaceDescriptor newBack = thebes->Swap(newFront.buffer(), - newFront.rect(), - newFront.rotation()); - - // XXX figure me out - replyv.push_back(OpBufferSwap(shadow, NULL, newBack)); + ThebesBuffer newBack; + nsIntRegion newValidRegion; + float newXResolution, newYResolution; + thebes->Swap(newFront, op.updatedRegion(), + &newBack, &newValidRegion, &newXResolution, &newYResolution); + replyv.push_back( + OpThebesBufferSwap( + shadow, NULL, + newBack, newValidRegion, newXResolution, newYResolution)); break; } case Edit::TOpPaintCanvas: {