Bug 71249

Summary: Switch RenderObject to layout abstraction
Product: WebKit Reporter: Emil A Eklund <eae>
Component: Layout and RenderingAssignee: Emil A Eklund <eae>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, 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
Patch none

Emil A Eklund
Reported 2011-10-31 15:54:59 PDT
Switch RenderObject to layout type abstraction.
Attachments
Patch (18.35 KB, patch)
2011-10-31 15:56 PDT, Emil A Eklund 		no flags
DetailsFormatted DiffDiff
Patch (17.46 KB, patch)
2011-10-31 16:47 PDT, Emil A Eklund 		no flags
DetailsFormatted DiffDiff
Patch (16.66 KB, patch)
2011-10-31 17:32 PDT, Emil A Eklund 		no flags
DetailsFormatted DiffDiff
Patch (16.23 KB, patch)
2011-11-01 10:38 PDT, Emil A Eklund 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Emil A Eklund
Comment 1 2011-10-31 15:56:40 PDT
Created attachment 113097 [details] Patch
Darin Adler
Comment 2 2011-10-31 16:06:51 PDT
Comment on attachment 113097 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=113097&action=review > Source/WebCore/rendering/RenderObject.cpp:794 > + int width = roundf((side == BSTop || side == BSBottom ? y2 - y1 : x2 - x1)); The round function specifically takes a “float” so is not appropriate to round a LayoutUnit to an int. If LayoutUnit was a double or int it would be quite inefficient! > Source/WebCore/rendering/RenderObject.h:1052 > -inline int adjustForAbsoluteZoom(int value, RenderObject* renderer) > +inline float adjustForAbsoluteZoom(int value, RenderObject* renderer) I think this change should be landed separately with its own review and rationale.
Emil A Eklund
Comment 3 2011-10-31 16:47:53 PDT
Created attachment 113107 [details] Patch
Emil A Eklund
Comment 4 2011-10-31 16:49:12 PDT
(In reply to comment #2) > The round function specifically takes a “float” so is not appropriate to round a LayoutUnit to an int. If LayoutUnit was a double or int it would be quite inefficient! Good catch, I should have caught that. Thanks! > > > Source/WebCore/rendering/RenderObject.h:1052 > > -inline int adjustForAbsoluteZoom(int value, RenderObject* renderer) > > +inline float adjustForAbsoluteZoom(int value, RenderObject* renderer) > > I think this change should be landed separately with its own review and rationale. Reverted this change, leaving it for later. Please take another look.
Darin Adler
Comment 5 2011-10-31 16:51:14 PDT
Comment on attachment 113107 [details] Patch Patch looks good, but does not apply in the EWS.
Emil A Eklund
Comment 6 2011-10-31 17:32:53 PDT
Created attachment 113115 [details] Patch
Daniel Bates
Comment 7 2011-10-31 20:28:13 PDT
Comment on attachment 113115 [details] Patch Attachment 113115 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/10241949
Emil A Eklund
Comment 8 2011-11-01 10:38:41 PDT
Created attachment 113191 [details] Patch
Emil A Eklund
Comment 9 2011-11-01 10:41:55 PDT
Thanks Darin!
WebKit Review Bot
Comment 10 2011-11-01 16:02:37 PDT
Comment on attachment 113191 [details] Patch Clearing flags on attachment: 113191 Committed r99008: <http://trac.webkit.org/changeset/99008>
WebKit Review Bot
Comment 11 2011-11-01 16:02:43 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.