[webkit-dev] InlineBox::m_isSVG

[Eric Seidel]

400k is too large of a patch for anyone to review.

I would suggest you start by splitting out the layout test changes
from the rest of the patch.  I would also suggest that you try to post
the code changes in smaller chunks.  Ideal patch review size is <20k,
but that's not always possible for feature patches of course. :)

-eric

On Thu, Jun 25, 2009 at 2:32 AM, Roland Steiner<rolandsteiner at google.com> wrote:
> Hi Dave,
>
> thanks again for the feedback! I've now submitted a patch to bug #3749 with
> a basic ruby implementation with all the changes discussed on the list.
> (including the flag). Would be great if you could take time to review the
> patch whenever you can spare the time.
>
> Cheers,
>
> Roland
>
> On Tue, Jun 23, 2009 at 2:12 AM, David Hyatt <hyatt at apple.com> wrote:
>>
>> On Jun 21, 2009, at 11:18 PM, Roland Steiner wrote:
>>
>>> Hi Dave,
>>>
>>> as I will probably need to special-case height() for ruby InlineBox
>>> objects in the same way as is done for SVG boxes (still ironing out the
>>> details, though), making height() virtual was exactly my intent. I would
>>> have thought that the performance cost of a virtual call to height() would
>>> be offset by being able to remove the isSVG() condition inside (and later a
>>> potential isRuby() condition as well).
>>>
>>> Now if there are actual performance reasons for that bit and/or for
>>> having height() be non-virtual, then I may need to find another solution.
>>>
>>> Thanks,
>>>
>>> Roland
>>
>> You could probably just rename the m_isSVG bit to be something like
>> m_calculatesHeight, and then the virtual method that height() calls when
>> that is true could be renamed to be more general.
>>
>> dave
>> (hyatt at apple.com)
>>
>
>