Bug 63762 – Switch rendering tree selection code to to new layout types

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Emil A Eklund]

Convert selection handling code to new layout abstraction.

[Emil A Eklund]

Created attachment 99389 [details]
Patch

[Emil A Eklund]

No rounding methods where harmed in making this patch.

[Eric Seidel (no email)]

Comment on attachment 99389 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=99389&action=review

> Source/WebCore/rendering/InlineTextBox.cpp:118
> +    LayoutUnit sPos = max(startPos - m_start, static_cast<LayoutUnit>(0));

I don't think you'll need this cast in the float case.  Will you?

[Emil A Eklund]

(In reply to comment #3)
> (From update of attachment 99389 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=99389&action=review
> 
> > Source/WebCore/rendering/InlineTextBox.cpp:118
> > +    LayoutUnit sPos = max(startPos - m_start, static_cast<LayoutUnit>(0));
> 
> I don't think you'll need this cast in the float case.  Will you?

Sadly you do. The max method either takes int, int or float, float. In the float case without the cast this would result in a compiler error.

[Levi Weintraub]

This was one of the most frustrating things to deal with when building our proof of concept. Since we max with zero all over the code, I'm thinking it could make sense to have an inline function clapToZero which would remove our need to cast zero.

[Eric Seidel (no email)]

I thought the compiler was smart enough to treat '0' like whatever type it needed to.  I guess not.

I strongly support the addition of some sort of maxWithZero or whatever function and deploying it wherever you see fit.

[Darin Adler]

Comment on attachment 99389 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=99389&action=review

>>> Source/WebCore/rendering/InlineTextBox.cpp:118
>>> +    LayoutUnit sPos = max(startPos - m_start, static_cast<LayoutUnit>(0));
>> 
>> I don't think you'll need this cast in the float case.  Will you?
> 
> Sadly you do. The max method either takes int, int or float, float. In the float case without the cast this would result in a compiler error.

The good way to write this is:

    max<LayoutUnit>(startPos - m_start, 0);

[Darin Adler]

(In reply to comment #5)
> I'm thinking it could make sense to have an inline function clapToZero which would remove our need to cast zero.

That would be fine. I would probably not name it maxWithZero or clapToZero or even clampToZero, though.

[Emil A Eklund]

Created attachment 99843 [details]
Patch

[Emil A Eklund]

Changed max/min to use the template form rather than relying on casts. Thanks for the tip!

[Emil A Eklund]

Created attachment 100035 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 100035 [details]
Patch

LGTM.

[WebKit Review Bot]

Comment on attachment 100035 [details]
Patch

Clearing flags on attachment: 100035

Committed r90596: <http://trac.webkit.org/changeset/90596>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.