RESOLVED FIXED 80039
Update usage of LayoutUnits in RenderBox 
https://bugs.webkit.org/show_bug.cgi?id=80039
Summary Update usage of LayoutUnits in RenderBox
Levi Weintraub
Reported 2012-03-01 11:45:03 PST
Updating the usage of ints/LayoutUnits in RenderBox in preparation for subpixel positioning. Also adding some more stub methods for use with FractionalLayoutUnits when we make the switch. These will allow us to further shrink the size of the final patch that switches us over.
Attachments
Patch (15.66 KB, patch)
2012-03-01 12:15 PST, Levi Weintraub 		no flags
DetailsFormatted DiffDiff
Patch for landing (15.73 KB, patch)
2012-03-05 16:20 PST, Levi Weintraub 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Levi Weintraub
Comment 1 2012-03-01 12:15:51 PST
Created attachment 129730 [details] Patch
Levi Weintraub
Comment 2 2012-03-05 10:59:33 PST
Pinging reviewers.
Julien Chaffraix
Comment 3 2012-03-05 15:10:17 PST
Comment on attachment 129730 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=129730&action=review > Source/WebCore/rendering/RenderBox.cpp:483 > + return snapSizeToPixel(clientWidth(), clientLeft()); It's unfortunate that snapSizeToPixel requires also the location as it makes those call sites less readable and more error-prone but you can't avoid that to properly snap the size :( > Source/WebCore/rendering/RenderBox.h:131 > + // FIXME: We shouldn't be returning this as a LayoutRect, since it loses its position and won't properly pixel snap. > LayoutRect borderBoxRect() const { return LayoutRect(0, 0, width(), height()); } It took me some time to understand your comment as you can see the issue both ways: if you properly snap your width() and height() here then you should be fine. I think I agree with the gist that is that this shouldn't return LayoutRect or properly take into account the position. I wonder if we just couldn't just remove this method as it will be evil when sub-pixels are added.
Levi Weintraub
Comment 4 2012-03-05 15:13:01 PST
Comment on attachment 129730 [details] Patch Thanks for taking the time to understand and review this patch, Julien!
WebKit Review Bot
Comment 5 2012-03-05 15:33:23 PST
Comment on attachment 129730 [details] Patch Rejecting attachment 129730 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: ceeded at 1484 (offset 10 lines). Hunk #7 succeeded at 3895 (offset 11 lines). Hunk #8 succeeded at 3906 (offset 11 lines). Hunk #9 succeeded at 3927 (offset 11 lines). patching file Source/WebCore/rendering/RenderBox.h Hunk #1 FAILED at 127. 1 out of 5 hunks FAILED -- saving rejects to file Source/WebCore/rendering/RenderBox.h.rej Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Julien Cha..." exit_code: 1 cwd: /mnt/git/webkit-commit-queue/ Full output: http://queues.webkit.org/results/11836017
Levi Weintraub
Comment 6 2012-03-05 16:20:38 PST
Created attachment 130223 [details] Patch for landing
WebKit Review Bot
Comment 7 2012-03-05 18:43:15 PST
Comment on attachment 130223 [details] Patch for landing Clearing flags on attachment: 130223 Committed r109835: <http://trac.webkit.org/changeset/109835>
WebKit Review Bot
Comment 8 2012-03-05 18:43:20 PST
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.