Bug 63762

Summary: Switch rendering tree selection code to to new layout types
Product: WebKit Reporter: Emil A Eklund <eae>
Component: Layout and RenderingAssignee: Emil A Eklund <eae>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, eric, leviw, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 63567    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Emil A Eklund
Reported 2011-06-30 15:51:28 PDT
Convert selection handling code to new layout abstraction.
Attachments
Patch (39.31 KB, patch)
2011-06-30 16:09 PDT, Emil A Eklund 		no flags
DetailsFormatted DiffDiff
Patch (35.89 KB, patch)
2011-07-06 09:40 PDT, Emil A Eklund 		no flags
DetailsFormatted DiffDiff
Patch (38.56 KB, patch)
2011-07-07 14:45 PDT, Emil A Eklund 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Emil A Eklund
Comment 1 2011-06-30 16:09:33 PDT
Created attachment 99389 [details] Patch
Emil A Eklund
Comment 2 2011-06-30 16:11:27 PDT
No rounding methods where harmed in making this patch.
Eric Seidel (no email)
Comment 3 2011-06-30 16:43:23 PDT
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
Comment 4 2011-06-30 16:46:18 PDT
(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
Comment 5 2011-07-01 12:06:27 PDT
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)
Comment 6 2011-07-01 12:11:44 PDT
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 7 2011-07-01 16:28:25 PDT
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
Comment 8 2011-07-01 16:29:16 PDT
(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
Comment 9 2011-07-06 09:40:07 PDT
Created attachment 99843 [details] Patch
Emil A Eklund
Comment 10 2011-07-06 09:44:37 PDT
Changed max/min to use the template form rather than relying on casts. Thanks for the tip!
Emil A Eklund
Comment 11 2011-07-07 14:45:39 PDT
Created attachment 100035 [details] Patch
Eric Seidel (no email)
Comment 12 2011-07-07 14:56:08 PDT
Comment on attachment 100035 [details] Patch LGTM.
WebKit Review Bot
Comment 13 2011-07-07 15:38:10 PDT
Comment on attachment 100035 [details] Patch Clearing flags on attachment: 100035 Committed r90596: <http://trac.webkit.org/changeset/90596>
WebKit Review Bot
Comment 14 2011-07-07 15:38:15 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.