Bug 81921 - Update usage of LayoutUnits in RenderListMarker
Summary: Update usage of LayoutUnits in RenderListMarker
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Levi Weintraub
URL:
Keywords:
Depends on:
Blocks: 60318
  Show dependency treegraph
 
Reported: 2012-03-22 09:01 PDT by Levi Weintraub
Modified: 2012-03-27 03:09 PDT (History)
4 users (show)

See Also:


Attachments
Patch (8.70 KB, patch)
2012-03-23 07:05 PDT, Levi Weintraub
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Levi Weintraub 2012-03-22 09:01:04 PDT
Also adding pixelSnappedLocation/Size to Int/FractionalLayoutRect.
Comment 1 Levi Weintraub 2012-03-23 07:05:36 PDT
Created attachment 133476 [details]
Patch
Comment 2 Eric Seidel (no email) 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...
Comment 3 Levi Weintraub 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.
Comment 4 Eric Seidel (no email) 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. :)
Comment 5 Levi Weintraub 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!!
Comment 6 WebKit Review Bot 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>
Comment 7 WebKit Review Bot 2012-03-27 03:09:13 PDT
All reviewed patches have been landed.  Closing bug.