[Webkit-unassigned] [Bug 103510] Reduce the children repaints when moved multiple times during the layout

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=103510





--- Comment #8 from Carlos Garcia Campos <cgarcia at igalia.com>  2012-12-03 03:24:37 PST ---
(In reply to comment #7)
> (From update of attachment 176952 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=176952&action=review

Thanks for the review.

> > LayoutTests/css3/flexbox/repaint-column-reverse.html:23
> > +    if (window.testRunner) {
> > +        testRunner.waitUntilDone();
> > +        testRunner.display();
> 
> Please add testRunner.dumpAsText(true) so we get the pixel result, but no render tree dump which isn't important for this test.

Sure.

> > LayoutTests/css3/flexbox/repaint-column-reverse.html:29
> > +        document.body.appendChild(document.createTextNode(
> > +            "This test checks that for flex items that are moved multiple times during the layout "
> > +            + "only the initial and final positions are repainted. Only the blue flex item should be repainted "
> > +            + "after changing its position. If the other flex items are repainted, this test fails."));
> > +    }
> 
> I would always include this text. It makes it easier for gardeners.

Ok, I was trying to avoid adding text to a pixel test, I won't be able to add the ref image indeed, because my system renders font slightly different.

> > Source/WebCore/rendering/RenderFlexibleBox.cpp:395
> > +        childIndex++;
> 
> Nit: ++childIndex

Ok.

> > Source/WebCore/rendering/RenderFlexibleBox.cpp:735
> > +    iterator.first();
> 
> Ah right, previously the constructor would do this for us. Can we also remove the call to first() in the OrderIterator constructor?

I'll check.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.