Bug 71249

Summary:

Switch RenderObject to layout abstraction

Product:

WebKit

Reporter:

Emil A Eklund <eae>

Component:

Layout and Rendering

Assignee:

Emil A Eklund <eae>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

darin, webkit.review.bot

Priority:

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

Description Emil A Eklund 2011-10-31 15:54:59 PDT

Switch RenderObject to layout type abstraction.

Comment 1 Emil A Eklund 2011-10-31 15:56:40 PDT

Created attachment 113097 [details]
Patch

Comment 2 Darin Adler 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.

Comment 3 Emil A Eklund 2011-10-31 16:47:53 PDT

Created attachment 113107 [details]
Patch

Comment 4 Emil A Eklund 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.

Comment 5 Darin Adler 2011-10-31 16:51:14 PDT

Comment on attachment 113107 [details]
Patch

Patch looks good, but does not apply in the EWS.

Comment 6 Emil A Eklund 2011-10-31 17:32:53 PDT

Created attachment 113115 [details]
Patch

Comment 7 Daniel Bates 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

Comment 8 Emil A Eklund 2011-11-01 10:38:41 PDT

Created attachment 113191 [details]
Patch

Comment 9 Emil A Eklund 2011-11-01 10:41:55 PDT

Thanks Darin!

Comment 10 WebKit Review Bot 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>

Comment 11 WebKit Review Bot 2011-11-01 16:02:43 PDT

All reviewed patches have been landed.  Closing bug.