| Summary: | Update LayoutUnit usage in RenderListBox | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Levi Weintraub <leviw> | ||||
| Component: | Layout and Rendering | Assignee: | Levi Weintraub <leviw> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | eae, eric, jchaffraix, tkent, webkit.review.bot | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Bug Depends on: | |||||||
| Bug Blocks: | 60318 | ||||||
| Attachments: |
|
||||||
|
Description
Levi Weintraub
2012-03-13 14:49:00 PDT
Created attachment 131727 [details]
Patch
Comment on attachment 131727 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=131727&action=review > Source/WebCore/ChangeLog:19 > + (WebCore::RenderListBox::panScroll): EventHandler::currentMousePosition is an IntPoint, > + so it doesn't require additional rounding in RenderListBox. It would be nice to make such conversion fail at compile time when you change the type of LayoutUnit to catch such unneeded copying. > Source/WebCore/rendering/RenderListBox.cpp:425 > + paintInfo.context->drawBidiText(itemFont, textRun, roundedIntPoint(r.location())); I was going to ask about why rounding and then I saw that's what pixelSnappedIntRect does. I wonder if we could not make it more obvious that pixelSnappedIntRect is doing a roundedIntPoint on the location... Comment on attachment 131727 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=131727&action=review >> Source/WebCore/ChangeLog:19 >> + so it doesn't require additional rounding in RenderListBox. > > It would be nice to make such conversion fail at compile time when you change the type of LayoutUnit to catch such unneeded copying. Making IntPoint->LayoutPoint an explicit conversion would be a good idea, you're right. >> Source/WebCore/rendering/RenderListBox.cpp:425 >> + paintInfo.context->drawBidiText(itemFont, textRun, roundedIntPoint(r.location())); > > I was going to ask about why rounding and then I saw that's what pixelSnappedIntRect does. I wonder if we could not make it more obvious that pixelSnappedIntRect is doing a roundedIntPoint on the location... We could call it pixelSnappedIntPoint, but I feel like that doesn't actually make things clearer... Comment on attachment 131727 [details] Patch Clearing flags on attachment: 131727 Committed r111040: <http://trac.webkit.org/changeset/111040> All reviewed patches have been landed. Closing bug. |