[webkit-dev] InlineBox::m_isSVG

[Ojan Vafai]

One simple chunk you could break off into it's own patch is the
hasSpecialHeight refactor. It would be straightforward to review and could
be committed without blocking on the ruby side.

 On Thu, Jun 25, 2009 at 2:44 AM, Eric Seidel <eric at webkit.org> wrote:
>
> 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)
> >>
> >
> >
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20090625/3055d972/attachment.html>