On Oct 19, 2010, at 2:07 PM, Alex Milowski wrote:
> On Tue, Oct 19, 2010 at 11:29 AM, David Hyatt <hy...@apple.com> wrote:
>> (1) Make sure any layout methods you call do setNeedsLayout(false) at the
>> end of them.
>> (2) Look for any early returns in any of your layout methods, since maybe
>> you did an early return causing the setNeedsLayout(false) to be missed.
>> (3) Make sure you aren't dirtying a child for a re-layout without
>> immediately doing that re-layout, e.g., don't call setChildNeedsLayout(true,
>> false) on some child and then bail without doing a layout.
>
> While this is helpful, the current code (in the patch) follows these
> principles (except when RenderBlock::layout() is called last and so
> setNeedsLayout(false) is already done). The problem I have is an
> *ancestor* is marked as having a child needing layout during the
> layout process. When then MathML layout finishes, the MathML
> rendering objects do not need layout but the parent is marked with
> m_normalChildNeedsLayout set to true.
>
Ok, just speculating from eyeballing the code.... I think layoutInlineChildren
should do setNeedsLayout(false) on inlines when the end of the inline is
encountered rather than the start of it.
The iteration order is start of inline -> contents of inline -> end of inline,
and we do setNeedsLayout(false) at the start of the inline. If the contents of
the inline end up causing a dirtying up the chain, then this will not be caught
and cleared.
else if (o->isText() || (o->isRenderInline() && !endOfInline)) {
if (fullLayout || o->selfNeedsLayout())
dirtyLineBoxesForRenderer(o, fullLayout);
o->setNeedsLayout(false);
if (!o->isText())
toRenderInline(o)->invalidateVerticalPosition(); // FIXME:
Should do better here and not always invalidate everything.
}
In that code above I think !endOfInline should maybe just become endOfInline
instead.
dave
(hy...@apple.com)
_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
