Skip to content

Commit

Permalink
Bug 783368 - Fix progressive tile update coherency issues. r=bgirard
Browse files Browse the repository at this point in the history 
Fix some progressive tile updating coherency issues caused by aborting at
inopportune times and tile draw ordering.
  • Loading branch information
@Cwiiis
Cwiiis committed Nov 21, 2012
1 parent 22d1eef commit 7fce49e
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 30 deletions.
57 changes: 39 additions & 18 deletions gfx/layers/basic/BasicTiledThebesLayer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ BasicTiledThebesLayer::ComputeProgressiveUpdateRegion(BasicTiledLayerBuffer& aTi
const nsIntRegion& aOldValidRegion,
nsIntRegion& aRegionToPaint,
const gfx3DMatrix& aTransform,
const nsIntRect& aCompositionBounds,
const gfx::Point& aScrollOffset,
const gfxSize& aResolution,
bool aIsRepeated)
Expand All @@ -281,18 +282,16 @@ BasicTiledThebesLayer::ComputeProgressiveUpdateRegion(BasicTiledLayerBuffer& aTi
bool drawingLowPrecision = aTiledBuffer.IsLowPrecision();

// Find out if we have any non-stale content to update.
nsIntRegion freshRegion;
if (!mFirstPaint) {
freshRegion.And(aInvalidRegion, aOldValidRegion);
freshRegion.Sub(aInvalidRegion, freshRegion);
}
nsIntRegion staleRegion;
staleRegion.And(aInvalidRegion, aOldValidRegion);

// Find out the current view transform to determine which tiles to draw
// first, and see if we should just abort this paint. Aborting is usually
// caused by there being an incoming, more relevant paint.
gfx::Rect viewport;
float scaleX, scaleY;
if (BasicManager()->ProgressiveUpdateCallback(!freshRegion.IsEmpty(), viewport,
if (BasicManager()->ProgressiveUpdateCallback(!staleRegion.Contains(aInvalidRegion),
viewport,
scaleX, scaleY, !drawingLowPrecision)) {
SAMPLE_MARKER("Abort painting");
aRegionToPaint.SetEmpty();
Expand All @@ -304,10 +303,16 @@ BasicTiledThebesLayer::ComputeProgressiveUpdateRegion(BasicTiledLayerBuffer& aTi
RoundedTransformViewportBounds(viewport, aScrollOffset, aResolution,
scaleX, scaleY, aTransform);

// Paint tiles that have no content before tiles that only have stale content.
bool drawingStale = freshRegion.IsEmpty();
// Paint tiles that have stale content or that intersected with the screen
// at the time of issuing the draw command in a single transaction first.
// This is to avoid rendering glitches on animated page content, and when
// layers change size/shape.
nsIntRect criticalViewportRect = roundedTransformedViewport.Intersect(aCompositionBounds);
aRegionToPaint.And(aInvalidRegion, criticalViewportRect);
aRegionToPaint.Or(aRegionToPaint, staleRegion);
bool drawingStale = !aRegionToPaint.IsEmpty();
if (!drawingStale) {
aRegionToPaint = freshRegion;
aRegionToPaint = aInvalidRegion;
}

// Prioritise tiles that are currently visible on the screen.
Expand All @@ -317,6 +322,10 @@ BasicTiledThebesLayer::ComputeProgressiveUpdateRegion(BasicTiledLayerBuffer& aTi
paintVisible = true;
}

// Paint area that's visible and overlaps previously valid content to avoid
// visible glitches in animated elements, such as gifs.
bool paintInSingleTransaction = paintVisible && (drawingStale || mFirstPaint);

// The following code decides what order to draw tiles in, based on the
// current scroll direction of the primary scrollable layer.
NS_ASSERTION(!aRegionToPaint.IsEmpty(), "Unexpectedly empty paint region!");
Expand Down Expand Up @@ -364,11 +373,12 @@ BasicTiledThebesLayer::ComputeProgressiveUpdateRegion(BasicTiledLayerBuffer& aTi
// The region needed to paint is larger then our progressive chunk size
// therefore update what we want to paint and ask for a new paint transaction.

// If we're drawing stale, visible content, make sure that it happens
// in one go by repeating this work without calling the painted
// callback. The remaining content is then drawn tile-by-tile in
// multiple transactions.
if (!drawingLowPrecision && paintVisible && drawingStale) {
// If we need to draw more than one tile to maintain coherency, make
// sure it happens in the same transaction by requesting this work be
// repeated immediately.
// If this is unnecessary, the remaining work will be done tile-by-tile in
// subsequent transactions.
if (!drawingLowPrecision && paintInSingleTransaction) {
repeatImmediately = true;
} else {
BasicManager()->SetRepeatTransaction();
Expand All @@ -384,6 +394,7 @@ BasicTiledThebesLayer::ProgressiveUpdate(BasicTiledLayerBuffer& aTiledBuffer,
nsIntRegion& aInvalidRegion,
const nsIntRegion& aOldValidRegion,
const gfx3DMatrix& aTransform,
const nsIntRect& aCompositionBounds,
const gfx::Point& aScrollOffset,
const gfxSize& aResolution,
LayerManager::DrawThebesLayerCallback aCallback,
Expand All @@ -399,6 +410,7 @@ BasicTiledThebesLayer::ProgressiveUpdate(BasicTiledLayerBuffer& aTiledBuffer,
aOldValidRegion,
regionToPaint,
aTransform,
aCompositionBounds,
aScrollOffset,
aResolution,
repeat);
Expand Down Expand Up @@ -497,12 +509,20 @@ BasicTiledThebesLayer::PaintThebes(gfxContext* aContext,
resolution.height *= metrics.mResolution.height;
}

// Calculate the scroll offset since the last transaction.
// Calculate the scroll offset since the last transaction, and the
// composition bounds.
nsIntRect compositionBounds;
gfx::Point scrollOffset(0, 0);
Layer* primaryScrollable = BasicManager()->GetPrimaryScrollableLayer();
if (primaryScrollable) {
const FrameMetrics& metrics = primaryScrollable->AsContainerLayer()->GetFrameMetrics();
scrollOffset = metrics.mScrollOffset;
gfxRect transformedViewport = transform.TransformBounds(
gfxRect(metrics.mCompositionBounds.x, metrics.mCompositionBounds.y,
metrics.mCompositionBounds.width, metrics.mCompositionBounds.height));
transformedViewport.RoundOut();
compositionBounds = nsIntRect(transformedViewport.x, transformedViewport.y,
transformedViewport.width, transformedViewport.height);
}

// Only draw progressively when the resolution is unchanged.
Expand All @@ -528,8 +548,8 @@ BasicTiledThebesLayer::PaintThebes(gfxContext* aContext,
}

if (!ProgressiveUpdate(mTiledBuffer, mValidRegion, invalidRegion,
oldValidRegion, transform, scrollOffset, resolution,
aCallback, aCallbackData))
oldValidRegion, transform, compositionBounds,
scrollOffset, resolution, aCallback, aCallbackData))
return;
} else {
mTiledBuffer.SetFrameResolution(resolution);
Expand Down Expand Up @@ -594,7 +614,8 @@ BasicTiledThebesLayer::PaintThebes(gfxContext* aContext,
updatedLowPrecision =
ProgressiveUpdate(mLowPrecisionTiledBuffer, mLowPrecisionValidRegion,
lowPrecisionInvalidRegion, oldValidRegion, transform,
scrollOffset, resolution, aCallback, aCallbackData);
compositionBounds, scrollOffset, resolution, aCallback,
aCallbackData);
}

// Re-add the high-precision valid region intersection so that we can
Expand Down
5 changes: 5 additions & 0 deletions gfx/layers/basic/BasicTiledThebesLayer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,8 @@ class BasicTiledThebesLayer : public ThebesLayer,
* which indicates that there is no more work to do.
* aTransform is the transform required to convert from screen-space to
* layer-space.
* aCompositionBounds is the composition bounds from the primary scrollable
* layer, transformed into layer coordinates.
* aScrollOffset is the current scroll offset of the primary scrollable layer.
* aResolution is the render resolution of the layer.
* aIsRepeated should be true if this function has already been called during
Expand All @@ -243,18 +245,21 @@ class BasicTiledThebesLayer : public ThebesLayer,
const nsIntRegion& aOldValidRegion,
nsIntRegion& aRegionToPaint,
const gfx3DMatrix& aTransform,
const nsIntRect& aCompositionBounds,
const gfx::Point& aScrollOffset,
const gfxSize& aResolution,
bool aIsRepeated);

/**
* Performs a progressive update of a given tiled buffer.
* See ComputeProgressiveUpdateRegion above for parameter documentation.
*/
bool ProgressiveUpdate(BasicTiledLayerBuffer& aTiledBuffer,
nsIntRegion& aValidRegion,
nsIntRegion& aInvalidRegion,
const nsIntRegion& aOldValidRegion,
const gfx3DMatrix& aTransform,
const nsIntRect& aCompositionBounds,
const gfx::Point& aScrollOffset,
const gfxSize& aResolution,
LayerManager::DrawThebesLayerCallback aCallback,
Expand Down
12 changes: 0 additions & 12 deletions mobile/android/base/gfx/GeckoLayerClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -412,18 +412,6 @@ public ProgressiveUpdateData progressiveUpdateCallback(boolean aHasPendingNewThe
return mProgressiveUpdateData;
}

// There's no new content (where new content is considered to be an
// update in a region that wasn't previously visible), and we've sent a
// more recent display-port.
// Aborting in this situation helps us recover more quickly when the
// user starts scrolling on a page that contains animated content that
// is slow to draw.
if (!aHasPendingNewThebesContent) {
Log.d(LOGTAG, "Aborting update due to more relevant display-port in event queue");
mProgressiveUpdateData.abort = true;
return mProgressiveUpdateData;
}

return mProgressiveUpdateData;
}

Expand Down

0 comments on commit 7fce49e

Please sign in to comment.