Bug 81038 - Update LayoutUnit usage in RenderListBox
Summary: Update LayoutUnit usage in RenderListBox
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 tree / graph
 
Reported: 2012-03-13 14:49 PDT by Levi Weintraub
Modified: 2012-03-16 12:07 PDT (History)
5 users (show)

See Also:

Attachments
Patch (3.33 KB, patch)
2012-03-13 15:33 PDT, Levi Weintraub 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Levi Weintraub 2012-03-13 14:49:00 PDT 
Rounding paint offsets, snapping painted rects, and rounding mouse coordinates, as laid out in https://trac.webkit.org/wiki/LayoutUnit.
Comment 1 Levi Weintraub 2012-03-13 15:33:15 PDT 
Created attachment 131727 [details]
Patch
Comment 2 Julien Chaffraix 2012-03-16 07:57:09 PDT 
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 3 Levi Weintraub 2012-03-16 11:20:15 PDT 
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 4 WebKit Review Bot 2012-03-16 12:07:42 PDT 
Comment on attachment 131727 [details]
Patch

Clearing flags on attachment: 131727

Committed r111040: <http://trac.webkit.org/changeset/111040>
Comment 5 WebKit Review Bot 2012-03-16 12:07:47 PDT 
All reviewed patches have been landed.  Closing bug.