Bug 1187619 - Only optmimize FrameLayerBuilder visibility calculations if correct. r=mattwoodrow

Bug 1176077 introduced the parameter aDirtyRegion to
DrawPaintedLayerCallback, which allows the callback to recompute the
visibility of all items to be painted in that transaction in a single
go. However, this parameter can not always be determined correctly
when using RotatedBuffer, and using an incorrect value was causing
graphical glitches.

Make the parameter optional, and on null values do not perform the
optimisation. Pass null from ClientPaintedLayer, which uses
RotatedBuffer and was causing problems, but continue to pass the
correct value from other Layer implementations. This optimisation was
most important for tiled layers using progressive paint, so this is
okay.
This commit is contained in:
Jamie Nicol 2015-08-03 04:07:00 -04:00
parent e365bb83c0
commit d10ee0730d
10 changed files with 31 additions and 26 deletions

View File

@ -1686,7 +1686,7 @@ static void
DrawPaintedLayer(PaintedLayer* aLayer,
gfxContext* aContext,
const nsIntRegion& aRegionToDraw,
const nsIntRegion& aDirtyRegion,
const nsIntRegion* aDirtyRegion,
DrawRegionClip aClip,
const nsIntRegion& aRegionToInvalidate,
void* aCallbackData)

View File

@ -258,11 +258,11 @@ public:
* The callee must draw all of aRegionToDraw.
* This region is relative to 0,0 in the PaintedLayer.
*
* aDirtyRegion contains the region that is due to be painted at some point,
* even though only aRegionToDraw should be drawn during this call.
* They will often be equal, but aDirtyRegion may be larger, for example
* when painting progressively. aRegionToDraw must be entirely contained
* within aDirtyRegion.
* aDirtyRegion, if non-null, contains the total region that is due to be
* painted during the transaction, even though only aRegionToDraw should
* be drawn during this call. The sum of every aRegionToDraw over the
* course of the transaction must equal aDirtyRegion. aDirtyRegion can be
* null if the total dirty region is unknown.
*
* aRegionToInvalidate contains a region whose contents have been
* changed by the layer manager and which must therefore be invalidated.
@ -284,7 +284,7 @@ public:
typedef void (* DrawPaintedLayerCallback)(PaintedLayer* aLayer,
gfxContext* aContext,
const nsIntRegion& aRegionToDraw,
const nsIntRegion& aDirtyRegion,
const nsIntRegion* aDirtyRegion,
DrawRegionClip aClip,
const nsIntRegion& aRegionToInvalidate,
void* aCallbackData);

View File

@ -735,14 +735,6 @@ RotatedContentBuffer::BorrowDrawTargetForPainting(PaintState& aPaintState,
return nullptr;
}
if (result->GetBackendType() == BackendType::DIRECT2D ||
result->GetBackendType() == BackendType::DIRECT2D1_1) {
// Simplify the draw region to avoid hitting expensive drawing paths for
// complex regions. Must be applied to the entire draw region so that it
// remains a superset of the iterator's draw region.
aPaintState.mRegionToDraw.SimplifyOutwardByArea(100 * 100);
}
nsIntRegion* drawPtr = &aPaintState.mRegionToDraw;
if (aIter) {
// The iterators draw region currently only contains the bounds of the region,
@ -750,6 +742,12 @@ RotatedContentBuffer::BorrowDrawTargetForPainting(PaintState& aPaintState,
aIter->mDrawRegion.And(aIter->mDrawRegion, aPaintState.mRegionToDraw);
drawPtr = &aIter->mDrawRegion;
}
if (result->GetBackendType() == BackendType::DIRECT2D ||
result->GetBackendType() == BackendType::DIRECT2D1_1) {
// Simplify the draw region to avoid hitting expensive drawing paths
// for complex regions.
drawPtr->SimplifyOutwardByArea(100 * 100);
}
if (aPaintState.mMode == SurfaceMode::SURFACE_COMPONENT_ALPHA) {
if (!mDTBuffer || !mDTBufferOnWhite) {

View File

@ -91,7 +91,7 @@ BasicPaintedLayer::PaintThebes(gfxContext* aContext,
groupContext = aContext;
}
SetAntialiasingFlags(this, groupContext->GetDrawTarget());
aCallback(this, groupContext, toDraw, toDraw,
aCallback(this, groupContext, toDraw, &toDraw,
DrawRegionClip::NONE, nsIntRegion(), aCallbackData);
if (needsGroup) {
aContext->PopGroupToSource();

View File

@ -112,7 +112,7 @@ protected:
BasicManager()->SetTransactionIncomplete();
return;
}
aCallback(this, aContext, aExtendedRegionToDraw, aExtendedRegionToDraw,
aCallback(this, aContext, aExtendedRegionToDraw, &aExtendedRegionToDraw,
aClip, aRegionToInvalidate, aCallbackData);
// Everything that's visible has been validated. Do this instead of just
// OR-ing with aRegionToDraw, since that can lead to a very complex region

View File

@ -87,7 +87,7 @@ ClientPaintedLayer::PaintThebes()
ClientManager()->GetPaintedLayerCallback()(this,
ctx,
iter.mDrawRegion,
state.mRegionToDraw,
nullptr,
state.mClip,
state.mRegionToInvalidate,
ClientManager()->GetPaintedLayerCallbackData());

View File

@ -181,7 +181,7 @@ ClientSingleTiledLayerBuffer::PaintThebes(const nsIntRegion& aNewValidRegion,
nsRefPtr<gfxContext> ctx = new gfxContext(dt);
ctx->SetMatrix(ctx->CurrentMatrix().Translate(-mTilingOrigin.x, -mTilingOrigin.y));
aCallback(mPaintedLayer, ctx, paintRegion, paintRegion, DrawRegionClip::DRAW, nsIntRegion(), aCallbackData);
aCallback(mPaintedLayer, ctx, paintRegion, &paintRegion, DrawRegionClip::DRAW, nsIntRegion(), aCallbackData);
}
// Mark the area we just drew into the back buffer as invalid in the front buffer as they're

View File

@ -964,7 +964,7 @@ ClientMultiTiledLayerBuffer::PaintThebes(const nsIntRegion& aNewValidRegion,
PROFILER_LABEL("ClientMultiTiledLayerBuffer", "PaintThebesSingleBufferDraw",
js::ProfileEntry::Category::GRAPHICS);
mCallback(mPaintedLayer, ctxt, aPaintRegion, aDirtyRegion,
mCallback(mPaintedLayer, ctxt, aPaintRegion, &aDirtyRegion,
DrawRegionClip::NONE, nsIntRegion(), mCallbackData);
}
@ -1171,7 +1171,7 @@ void ClientMultiTiledLayerBuffer::Update(const nsIntRegion& newValidRegion,
ctx->SetMatrix(
ctx->CurrentMatrix().Scale(mResolution, mResolution).Translate(ThebesPoint(-mTilingOrigin)));
mCallback(mPaintedLayer, ctx, aPaintRegion, aDirtyRegion,
mCallback(mPaintedLayer, ctx, aPaintRegion, &aDirtyRegion,
DrawRegionClip::DRAW, nsIntRegion(), mCallbackData);
mMoz2DTiles.clear();
// Reset:

View File

@ -5547,7 +5547,7 @@ private:
FrameLayerBuilder::DrawPaintedLayer(PaintedLayer* aLayer,
gfxContext* aContext,
const nsIntRegion& aRegionToDraw,
const nsIntRegion& aDirtyRegion,
const nsIntRegion* aDirtyRegion,
DrawRegionClip aClip,
const nsIntRegion& aRegionToInvalidate,
void* aCallbackData)
@ -5605,11 +5605,16 @@ FrameLayerBuilder::DrawPaintedLayer(PaintedLayer* aLayer,
// Recompute visibility of items in our PaintedLayer. Note that this
// recomputes visibility for all descendants of our display items too,
// so there's no need to do this for the items in inactive PaintedLayers.
// If aDirtyRegion is non-null then recompute the visibility of the entire
// aDirtyRegion at once, rather of aRegionToDraw separately on each call.
int32_t appUnitsPerDevPixel = presContext->AppUnitsPerDevPixel();
RecomputeVisibilityForItems(entry->mItems, builder, aDirtyRegion,
RecomputeVisibilityForItems(entry->mItems, builder,
aDirtyRegion ? *aDirtyRegion : aRegionToDraw,
offset, appUnitsPerDevPixel,
userData->mXScale, userData->mYScale);
userData->mNeedsRecomputeVisibility = false;
if (aDirtyRegion) {
userData->mNeedsRecomputeVisibility = false;
}
}
nsRenderingContext rc(aContext);

View File

@ -282,12 +282,14 @@ public:
* must be the nsDisplayListBuilder containing this FrameLayerBuilder.
* This function can be called multiple times in a row to draw
* different regions. This will occur when, for example, progressive paint is
* enabled, in which case aRegionToDraw will be a subregion of aDirtyRegion.
* enabled. In these cases aDirtyRegion can optionally be used to specify the
* total region that will be drawn during the transaction, possibly allowing
* the callback to make optimizations.
*/
static void DrawPaintedLayer(PaintedLayer* aLayer,
gfxContext* aContext,
const nsIntRegion& aRegionToDraw,
const nsIntRegion& aDirtyRegion,
const nsIntRegion* aDirtyRegion,
mozilla::layers::DrawRegionClip aClip,
const nsIntRegion& aRegionToInvalidate,
void* aCallbackData);