When the TileController is configured to have margin tiles, repeating background images should continue to repaint in those tiles. <rdar://problem/15571300>
Created attachment 221215 [details] Patch I did a ton of manual testing. I think it would be really good to finally add LayoutTests for this stuff, but I figured I would post this patch in the meantime.
Comment on attachment 221215 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=221215&action=review > Source/WebCore/page/FrameView.h:197 > + bool hasExtendedBackground(); > + IntRect extendedBackgroundRect(); Both should be const. I would like to see extendedBackgroundRect() have a comment saying what coordinate system the rect is in. > Source/WebCore/rendering/RenderBox.cpp:1603 > + if (rectToRepaint.width() == rendererRect.width()) { > + rectToRepaint.move(extendedBackgroundRect.x(), 0); > + rectToRepaint.setWidth(extendedBackgroundRect.width()); > + } > + if (rectToRepaint.height() == rendererRect.height()) { > + rectToRepaint.move(0, extendedBackgroundRect.y()); > + rectToRepaint.setHeight(extendedBackgroundRect.height()); > + } It seems a bit arbitrary to say "if we're invalidating the width, just invalidate the extended width". What about a non-repeating animating background image that is projecting into the left margin but not the right? I think the background geometry code needs to know about extended rects.
(In reply to comment #2) > (From update of attachment 221215 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=221215&action=review > > > Source/WebCore/page/FrameView.h:197 > > + bool hasExtendedBackground(); > > + IntRect extendedBackgroundRect(); > > Both should be const. I would like to see extendedBackgroundRect() have a comment saying what coordinate system the rect is in. > Done. > > Source/WebCore/rendering/RenderBox.cpp:1603 > > + if (rectToRepaint.width() == rendererRect.width()) { > > + rectToRepaint.move(extendedBackgroundRect.x(), 0); > > + rectToRepaint.setWidth(extendedBackgroundRect.width()); > > + } > > + if (rectToRepaint.height() == rendererRect.height()) { > > + rectToRepaint.move(0, extendedBackgroundRect.y()); > > + rectToRepaint.setHeight(extendedBackgroundRect.height()); > > + } > > It seems a bit arbitrary to say "if we're invalidating the width, just invalidate the extended width". What about a non-repeating animating background image that is projecting into the left margin but not the right? > > I think the background geometry code needs to know about extended rects. I will file a follow-up bug for this. The performance penalty for over-invalidation is probably pretty small in the long run since a repeating background image like you've described is probably unlikely to be very common in the real world. But it would be nice to be fully-optimized here. Thanks for the review!
http://trac.webkit.org/changeset/162098
(In reply to comment #4) > http://trac.webkit.org/changeset/162098 It broke the WinCairo build.
(In reply to comment #5) > (In reply to comment #4) > > http://trac.webkit.org/changeset/162098 > > It broke the WinCairo build. Speculative build fix: http://trac.webkit.org/changeset/162138