[Webkit-unassigned] [Bug 58362] Bundle w and tmpW in findNextLineBreak together as a class

[bugzilla-daemon at webkit.org]

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


Dave Hyatt <hyatt at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #89268|review?                     |review+
               Flag|                            |




--- Comment #3 from Dave Hyatt <hyatt at apple.com>  2011-04-12 14:10:44 PST ---
(From update of attachment 89268)
View in context: https://bugs.webkit.org/attachment.cgi?id=89268&action=review

This could also be done as a followup, but availableWidth should move into LineWidth as well.  Then you could ask questions like "remainingWidth()" to get availableWidth - currentWidth etc., and lots of these comparisons would get cleaner.

r=me

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:1639
> +inline LineWidth::LineWidth()
> +    : m_uncommittedWidth(0)
> +    , m_committedWidth(0)
> +{
> +}

I don't much like this style when the class is so tiny.  I'd just put this right inside the class declaration.

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:1645
> +inline void LineWidth::commit()
> +{
> +    m_committedWidth += m_uncommittedWidth;
> +    m_uncommittedWidth = 0;
> +}

Same here.  Just put it up where you first declared the function.

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:1660
> +    float availableWidth = lineOffsets.width();
> +    LineWidth width;

Any reason not to just merge LineWidth with LineOffsets? They're close enough to one another that I'd just make them a single struct. I guess that could be done in a future patch though.

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