Bug 125263 – Bad repaints on Twitter when the tile cache has a margin

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Beth Dakin]

With the new backgroundShouldExtendBeyondPage() Setting, there are painting artifacts on Twitter and other webpages that grow their contents size.

<rdar://problem/15576884>

[Beth Dakin]

Created attachment 218468 [details]
Patch

[Tim Horton]

Comment on attachment 218468 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=218468&action=review

> Source/WebCore/platform/graphics/ca/mac/TileController.mm:755
> +        // If there are magin tiles and the bounds have grown taller or wider, then the tiles that used to

margin

> Source/WebCore/platform/graphics/ca/mac/TileController.mm:761
> +                IntRect boundsWithoutMargin = IntRect(IntPoint(), expandedIntSize(m_tileCacheLayer->bounds().size()));
> +                IntRect oldBoundsWithoutMargin = IntRect(IntPoint(), m_boundsAtLastRevalidate.size());
> +                oldBoundsWithoutMargin.contract(IntSize(leftMarginWidth() + rightMarginWidth(), topMarginHeight() + bottomMarginHeight()));

it might be nice if these things were factored out, especially the things that are done elsewhere

> Source/WebCore/platform/graphics/ca/mac/TileController.mm:763
> +                if (bounds.height() > m_boundsAtLastRevalidate.height()) {

smfr mentioned when things get shorter/skinnier, and you said that it didn't seem to be an issue, but I wonder why? what is invalidating the layer in that case? why doesn't it have stale content? would be nice to have a clear understanding.

[Beth Dakin]

(In reply to comment #2)
> (From update of attachment 218468 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=218468&action=review
> 
> > Source/WebCore/platform/graphics/ca/mac/TileController.mm:755
> > +        // If there are magin tiles and the bounds have grown taller or wider, then the tiles that used to
> 
> margin
> 

Hehe

> > Source/WebCore/platform/graphics/ca/mac/TileController.mm:761
> > +                IntRect boundsWithoutMargin = IntRect(IntPoint(), expandedIntSize(m_tileCacheLayer->bounds().size()));
> > +                IntRect oldBoundsWithoutMargin = IntRect(IntPoint(), m_boundsAtLastRevalidate.size());
> > +                oldBoundsWithoutMargin.contract(IntSize(leftMarginWidth() + rightMarginWidth(), topMarginHeight() + bottomMarginHeight()));
> 
> it might be nice if these things were factored out, especially the things that are done elsewhere
> 

Will do.

> > Source/WebCore/platform/graphics/ca/mac/TileController.mm:763
> > +                if (bounds.height() > m_boundsAtLastRevalidate.height()) {
> 
> smfr mentioned when things get shorter/skinnier, and you said that it didn't seem to be an issue, but I wonder why? what is invalidating the layer in that case? why doesn't it have stale content? would be nice to have a clear understanding.

I did some debugging here and Tim and I talked about it in person. Basically it seems like the cached repaint rects in the GraphicsLayerCA class always seem to have a repaint rect equivalent to the "old" size in changing-content-size test cases. So when the content goes from small to large, there is a small repaint rect that doesn't cover everything. Normal content ends up being fine due to other normal-content invalidations. When the page goes from large to small there is a large repaint rect that is big enough to cover the margin.

[Beth Dakin]

Thanks Tim! http://trac.webkit.org/changeset/160179