Bug 60739

Summary: Switch paintCollapsedBorder to use IntRect
Product: WebKit Reporter: Levi Weintraub <leviw>
Component: Layout and RenderingAssignee: Levi Weintraub <leviw>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, dglazkov, eae, eric, gustavo.noronha, gustavo, simon.fraser, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 60318    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Levi Weintraub
Reported 2011-05-12 17:17:41 PDT
It currently takes four ints representing a rect. This is the *last* function with this bad signature! :)
Attachments
Patch (2.55 KB, patch)
2011-05-12 17:22 PDT, Levi Weintraub 		no flags
DetailsFormatted DiffDiff
Patch (4.82 KB, patch)
2011-05-12 18:48 PDT, Levi Weintraub 		no flags
DetailsFormatted DiffDiff
Patch (4.41 KB, patch)
2011-05-13 11:05 PDT, Levi Weintraub 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Levi Weintraub
Comment 1 2011-05-12 17:22:19 PDT
Created attachment 93372 [details] Patch
Levi Weintraub
Comment 2 2011-05-12 17:23:37 PDT
Comment on attachment 93372 [details] Patch Oops, made a mistake :p
Early Warning System Bot
Comment 3 2011-05-12 17:32:12 PDT
Comment on attachment 93372 [details] Patch Attachment 93372 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/8688895
WebKit Review Bot
Comment 4 2011-05-12 17:44:46 PDT
Comment on attachment 93372 [details] Patch Attachment 93372 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/8687849
WebKit Review Bot
Comment 5 2011-05-12 17:48:51 PDT
Comment on attachment 93372 [details] Patch Attachment 93372 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/8688903
WebKit Review Bot
Comment 6 2011-05-12 18:06:41 PDT
Comment on attachment 93372 [details] Patch Attachment 93372 [details] did not pass cr-mac-ews (chromium): Output: http://queues.webkit.org/results/8687872
Collabora GTK+ EWS bot
Comment 7 2011-05-12 18:13:37 PDT
Comment on attachment 93372 [details] Patch Attachment 93372 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/8685931
Levi Weintraub
Comment 8 2011-05-12 18:48:19 PDT
Created attachment 93386 [details] Patch
Eric Seidel (no email)
Comment 9 2011-05-12 18:56:48 PDT
Comment on attachment 93386 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=93386&action=review I don't think the adjustedPaintRect change is a ncessarily a win. > Source/WebCore/rendering/RenderTableCell.cpp:931 > + IntRect adjustedPaintRect = IntRect(paintRect.x() - leftWidth / 2, You don't have to do an equal here, you can just construct directly. Webkit doesn't normally do cascading indents like this though. > Source/WebCore/rendering/RenderTableCell.cpp:952 > + borders.addBorder(topVal, BSTop, renderTop, adjustedPaintRect.x(), adjustedPaintRect.y(), adjustedPaintRect.maxX(), adjustedPaintRect.y() + topWidth, topStyle); > + borders.addBorder(bottomVal, BSBottom, renderBottom, adjustedPaintRect.x(), adjustedPaintRect.maxY() - bottomWidth, adjustedPaintRect.maxX(), adjustedPaintRect.maxY(), bottomStyle); > + borders.addBorder(leftVal, BSLeft, renderLeft, adjustedPaintRect.x(), adjustedPaintRect.y(), adjustedPaintRect.x() + leftWidth, adjustedPaintRect.maxY(), leftStyle); > + borders.addBorder(rightVal, BSRight, renderRight, adjustedPaintRect.maxX() - rightWidth, adjustedPaintRect.y(), adjustedPaintRect.maxX(), adjustedPaintRect.maxY(), rightStyle); Seems like making the locals above might make this more clear? Unleass you plan to actually use the adjustedPaintRect as an object somewhere?
Levi Weintraub
Comment 10 2011-05-13 11:05:37 PDT
Created attachment 93484 [details] Patch
Eric Seidel (no email)
Comment 11 2011-05-13 11:11:46 PDT
Comment on attachment 93484 [details] Patch LGTM.
Levi Weintraub
Comment 12 2011-05-13 11:21:15 PDT
Comment on attachment 93484 [details] Patch Clearing flags on attachment: 93484 Committed r86449: <http://trac.webkit.org/changeset/86449>
Levi Weintraub
Comment 13 2011-05-13 11:21:19 PDT
All reviewed patches have been landed. Closing bug.
Levi Weintraub
Comment 14 2011-05-13 11:22:07 PDT
(In reply to comment #11) > (From update of attachment 93484 [details]) > LGTM. Thanks for the review!
Note You need to log in before you can comment on or make changes to this bug.