[Webkit-unassigned] [Bug 13487] Implement O(1) absoluteClippedOverflowRect and absoluteOutlineBox during layout for a possible speed gain

[bugzilla-daemon at webkit.org]

http://bugs.webkit.org/show_bug.cgi?id=13487





------- Comment #21 from mitz at webkit.org  2007-04-28 14:36 PDT -------
(In reply to comment #20)
> (From update of attachment 14243 [edit])
> Typo, layuot.

Oops :-)

> +    bool layoutOnlyPositionedObjects();
> 
> The verb form is "layOut" and the noun form is "layout". I know that the
> layout() function violates this, but layOutAxis() respects it. I'm not sure
> others would agree with my take on this.

It's not just layout(), it's layoutBlock(), layoutPositionedObjects(),
layout{Block,Inline}Children() and maybe a few others. I agree that layOut is
better but I would prefer not to introduce apparent inconsistency with this
patch, and perhaps rename all such functions in a separate rename-only patch.

> +    void offsetUnderRelPositionedInline(RenderObject*, int& x, int& y) const;
> 
> I'm not completely comfortable with the verb use of offset here either.

I'm not comfortable with the entire name. I think "under" can be confusing.

> Ideally
> long-term something like this would return an IntSize that you would then add
> to your existing local IntPoint variable with +=.

I might just change it to return IntSize. I'm trying to use IntSize and
IntPoint in new code. I started writing this function to return an IntSize but
then I realized that the three existing callers had separate ints and decided
not to bother with it.

> Typo here, LayuoutState.

Oops again :-}

> I think we should put braces on separate lines for multiline functions, even
> ones that are inlines in a header.

I agree. This was unintentional.

(Comments that I didn't quote and reply to I'm going to address, or at least
try to address).


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