RESOLVED FIXED 60578
Switch paintMask and paintMaskImages off of ints 
https://bugs.webkit.org/show_bug.cgi?id=60578
Summary Switch paintMask and paintMaskImages off of ints
Levi Weintraub
Reported 2011-05-10 13:59:58 PDT
Part of ongoing refactoring.
Attachments
Patch (17.67 KB, patch)
2011-05-12 11:02 PDT, Levi Weintraub 		eric: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Levi Weintraub
Comment 1 2011-05-11 16:47:02 PDT
Updating name.
Levi Weintraub
Comment 2 2011-05-12 11:02:41 PDT
Created attachment 93306 [details] Patch
Eric Seidel (no email)
Comment 3 2011-05-12 11:30:48 PDT
Comment on attachment 93306 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=93306&action=review > Source/WebCore/platform/graphics/IntRect.h:113 > + void expand(const IntSize& s) { m_size += s; } I would have used "size" for the argument name > Source/WebCore/rendering/RenderBox.cpp:883 > +void RenderBox::paintMask(PaintInfo& paintInfo, IntSize paintOffset) What's the offset from? Would be nice to explain that in the name.
Levi Weintraub
Comment 4 2011-05-12 11:38:28 PDT
Comment on attachment 93306 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=93306&action=review >> Source/WebCore/platform/graphics/IntRect.h:113 >> + void expand(const IntSize& s) { m_size += s; } > > I would have used "size" for the argument name Sure. >> Source/WebCore/rendering/RenderBox.cpp:883 >> +void RenderBox::paintMask(PaintInfo& paintInfo, IntSize paintOffset) > > What's the offset from? Would be nice to explain that in the name. I'm unsure of how to name this in a way that satisfies everyone. I'd go with something like offsetFromPaintingRoot but I'm worried it may not be accurate for all cases. Would love smfr's advice (he suggested an IntSize named paintOffset in IRC IIRC).
Levi Weintraub
Comment 5 2011-05-12 13:51:01 PDT
(In reply to comment #3) > (From update of attachment 93306 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=93306&action=review > > > Source/WebCore/platform/graphics/IntRect.h:113 > > + void expand(const IntSize& s) { m_size += s; } > > I would have used "size" for the argument name > > > Source/WebCore/rendering/RenderBox.cpp:883 > > +void RenderBox::paintMask(PaintInfo& paintInfo, IntSize paintOffset) > > What's the offset from? Would be nice to explain that in the name. Simon confirmed he agrees that paintOffset is the best we can do given how this currently behaves. If I switch the s to size can I get an r+?
Eric Seidel (no email)
Comment 6 2011-05-12 13:53:36 PDT
Comment on attachment 93306 [details] Patch Sure.
Levi Weintraub
Comment 7 2011-05-12 14:05:20 PDT
Committed r86377: <http://trac.webkit.org/changeset/86377>
Alexis Menard (darktears)
Comment 8 2011-05-17 06:07:25 PDT
(In reply to comment #7) > Committed r86377: <http://trac.webkit.org/changeset/86377> Committed r86670: <http://trac.webkit.org/changeset/86670> fixes a warning.
Note You need to log in before you can comment on or make changes to this bug.