Switch paintMask and paintMaskImages off of ints

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Levi Weintraub]

Part of ongoing refactoring.

[Levi Weintraub]

Updating name.

[Levi Weintraub]

Created attachment 93306 [details]
Patch

[Eric Seidel (no email)]

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 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]

(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 on attachment 93306 [details]
Patch

Sure.

[Levi Weintraub]

Committed r86377: <http://trac.webkit.org/changeset/86377>

[Alexis Menard (darktears)]

(In reply to comment #7)
> Committed r86377: <http://trac.webkit.org/changeset/86377>

Committed r86670: <http://trac.webkit.org/changeset/86670> fixes a warning.