[webkit-reviews] review requested: [Bug 107618] Fix painting phases for composited scrolling : [Attachment 191745] Patch

Wed Mar 6 06:48:18 PST 2013

vollick at chromium.org has asked	for review:
Bug 107618: Fix painting phases for composited scrolling
https://bugs.webkit.org/show_bug.cgi?id=107618

Attachment 191745: Patch
https://bugs.webkit.org/attachment.cgi?id=191745&action=review

------- Additional Comments from vollick at chromium.org
(In reply to comment #14)
> (From update of attachment 188625 [details])
> View in context:
https://bugs.webkit.org/attachment.cgi?id=188625&action=review
>
> > Source/WebCore/rendering/RenderLayer.cpp:3770
> > -	     // Now walk the sorted list of children with negative z-indices.
> > -	     paintList(negZOrderList(), context, localPaintingInfo,
localPaintFlags);
> > +	     if (!usesCompositedScrolling()) {
> > +		 // Now walk the sorted list of children with negative
z-indices.
> > +		 paintList(negZOrderList(), context, localPaintingInfo,
localPaintFlags);
> > +	     }
> > +
> > +	     // When using composited scrolling, outlines paint into the
background.
> > +	     if (usesCompositedScrolling() && shouldPaintOutline &&
!outlineRect.isEmpty()) {
> > +		 // Paint our own outline
> > +		 PaintInfo paintInfo(context,
pixelSnappedIntRect(outlineRect.rect()), PaintPhaseSelfOutline, paintBehavior,
paintingRootForRenderer, localPaintingInfo.region);
> > +		 clipToRect(localPaintingInfo.rootLayer, context,
localPaintingInfo.paintDirtyRect, outlineRect,
DoNotIncludeSelfForBorderRadius);
> > +		 renderer()->paint(paintInfo, paintOffset);
> > +		 restoreClip(context, localPaintingInfo.paintDirtyRect,
outlineRect);
> > +	     }
> >	 }
> >
> >	 if (localPaintFlags & PaintLayerPaintingCompositingForegroundPhase) {
> > +	     if (usesCompositedScrolling()) {
> > +		 // Now walk the sorted list of children with negative
z-indices.
> > +		 paintList(negZOrderList(), context, localPaintingInfo,
localPaintFlags);
> > +	     }
>
> Seems wrong to do it like this. RenderLayer shouldn't really care about what
compositing layers exist; the behavior should be expressible via paint phases.
Done.
>
> > LayoutTests/ChangeLog:32
> > +	     *
compositing/overflow/do-not-paint-outline-into-composited-scrolling-contents.ht
ml: Added.
> > +	     *
compositing/overflow/paint-neg-z-order-descendants-into-scrolling-contents-laye
r.html: Added.
> > +	     * platform/chromium/TestExpectations:
> > +	     * platform/mac-wk2/TestExpectations:
>
> I'd like to see a test with layerTeeAsText output here.
Thanks, this turned out to be very helpful. In order to build a
layerTreeAsText-based test, I was forced to update the paintingPhase of the
graphics layers rather than doing last minute paint phase hacks in
RenderLayerBacking::paintIntoLayer. This simplified logic everywhere and allows
for a new kind of text-based testing.
>
> >
LayoutTests/compositing/overflow/do-not-paint-outline-into-composited-scrolling
-contents.html:3
> > +<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"
> > +	"http://www.w3.org/TR/html4/loose.dtd">
> > +<html lang="en">
>
> Please use an HTML5-style DOCTYPE.
Done.