Summary: | Update usage of LayoutUnits in RenderBox | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Levi Weintraub <leviw> | ||||||
Component: | Layout and Rendering | Assignee: | Levi Weintraub <leviw> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | darin, eae, eric, jchaffraix, simon.fraser, 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-01 11:45:03 PST
Created attachment 129730 [details]
Patch
Pinging reviewers. 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. Comment on attachment 129730 [details]
Patch
Thanks for taking the time to understand and review this patch, Julien!
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 Created attachment 130223 [details]
Patch for landing
Comment on attachment 130223 [details] Patch for landing Clearing flags on attachment: 130223 Committed r109835: <http://trac.webkit.org/changeset/109835> All reviewed patches have been landed. Closing bug. |