Bug 81921

Summary:

Update usage of LayoutUnits in RenderListMarker

Product:

WebKit

Reporter:

Levi Weintraub <leviw>

Component:

Layout and Rendering

Assignee:

Levi Weintraub <leviw>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

eae, eric, jchaffraix, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

60318

Attachments:

Description	Flags
Patch	none

Levi Weintraub

Reported 2012-03-22 09:01:04 PDT

Also adding pixelSnappedLocation/Size to Int/FractionalLayoutRect.

Attachments
Patch (8.70 KB, patch) 2012-03-23 07:05 PDT, Levi Weintraub	no flags	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Levi Weintraub

Comment 1 2012-03-23 07:05:36 PDT

Created attachment 133476 [details] Patch

Eric Seidel (no email)

Comment 2 2012-03-26 15:36:58 PDT

Comment on attachment 133476 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=133476&action=review OK. > Source/WebCore/rendering/RenderListMarker.cpp:1124 > + marker.moveBy(roundedIntPoint(boxOrigin)); I thought we had new fancy .round() methods? > Source/WebCore/rendering/RenderListMarker.cpp:1263 > + marker.moveBy(IntPoint(roundToInt(box.x()), roundToInt(box.y() - logicalHeight()))); making a LayoutPoint and rounding that seems easier...

Levi Weintraub

Comment 3 2012-03-27 02:23:08 PDT

Comment on attachment 133476 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=133476&action=review >> Source/WebCore/rendering/RenderListMarker.cpp:1124 >> + marker.moveBy(roundedIntPoint(boxOrigin)); > > I thought we had new fancy .round() methods? We do have these on rects, but not on points. We actually don't have a .pixelSnap() method on rects either, just convenience functions to return pixel snapped values for the location, size, and edges. If you feel like that would be cleaner I'm happy to take a pass through this implementing that.

Eric Seidel (no email)

Comment 4 2012-03-27 02:26:21 PDT

I think that .round(), .floor(), ciel(), etc. are better than free funcxtions. But I also think we just need to get your branch landed and we can iterate from there. :)

Levi Weintraub

Comment 5 2012-03-27 02:33:09 PDT

(In reply to comment #4) > I think that .round(), .floor(), ciel(), etc. are better than free funcxtions. But I also think we just need to get your branch landed and we can iterate from there. :) We do have all those on LayoutUnits... the pain is that we still use integers on trunk. I think you were right that this all would have been a lot easier if we'd first moved to a IntegerLayoutUnit abstraction. Anyways, I'm definitely here to volunteer for clean-up once the switch is flipped! Thanks for your diligent reviewing!!

WebKit Review Bot

Comment 6 2012-03-27 03:09:08 PDT

Comment on attachment 133476 [details] Patch Clearing flags on attachment: 133476 Committed r112238: <http://trac.webkit.org/changeset/112238>

WebKit Review Bot

Comment 7 2012-03-27 03:09:13 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.