Bug 62177 – Switch paint to use IntPoint

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Levi Weintraub]

Ongoing tx/ty removal. This is the "big one" for paint?

[Levi Weintraub]

Created attachment 96176 [details]
WIP

[Levi Weintraub]

Created attachment 96261 [details]
WIP that works :)

[WebKit Review Bot]

Attachment 96261 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/page/FrameView.cpp', u'Sour..." exit_code: 1

Source/WebCore/rendering/RenderEmbeddedObject.h:60:  The parameter name "paintInfo" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 73 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Levi Weintraub]

Created attachment 96269 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 96269 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=96269&action=review

LGTM.

> Source/WebCore/rendering/RenderBoxModelObject.cpp:699
> +            IntSize localOffset = isBox() ? toRenderBox(this)->locationOffset() : IntSize();

I could have sworn there is a renderBoxRect() method somewhere which does something like this.  This is fine, just there may already be a helper for this.

> Source/WebCore/rendering/RenderObject.cpp:1148
> +void RenderObject::paint(PaintInfo& /*paintInfo*/, const IntPoint& /*paintOffset*/)

I don't think these comments add much of anything.

> Source/WebCore/rendering/mathml/RenderMathMLBlock.cpp:102
> +    info.context->drawLine(IntPoint(adjustedPaintOffset.x(), adjustedPaintOffset.y() + topStart), IntPoint(adjustedPaintOffset.x() + offsetWidth(), adjustedPaintOffset.y() + topStart));

Locals or IntSize additions might make this cleaner. Can be done in later passes.

> Source/WebCore/rendering/svg/RenderSVGResourceMarker.cpp:135
> +    RenderSVGContainer::paint(info, IntPoint());

I wonder if paint() should take a default argument for this point now. :)

[Levi Weintraub]

(In reply to comment #5)
> (From update of attachment 96269 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=96269&action=review
> 
> LGTM.
> 
> > Source/WebCore/rendering/RenderBoxModelObject.cpp:699
> > +            IntSize localOffset = isBox() ? toRenderBox(this)->locationOffset() : IntSize();
> 
> I could have sworn there is a renderBoxRect() method somewhere which does something like this.  This is fine, just there may already be a helper for this.

I just looked through the classes and didn't find anything that does this for me, but that doesn't mean I didn't miss it.

> 
> > Source/WebCore/rendering/RenderObject.cpp:1148
> > +void RenderObject::paint(PaintInfo& /*paintInfo*/, const IntPoint& /*paintOffset*/)
> 
> I don't think these comments add much of anything.

I'll take them out :)

> 
> > Source/WebCore/rendering/mathml/RenderMathMLBlock.cpp:102
> > +    info.context->drawLine(IntPoint(adjustedPaintOffset.x(), adjustedPaintOffset.y() + topStart), IntPoint(adjustedPaintOffset.x() + offsetWidth(), adjustedPaintOffset.y() + topStart));
> 
> Locals or IntSize additions might make this cleaner. Can be done in later passes.

You're right. I agree.

> 
> > Source/WebCore/rendering/svg/RenderSVGResourceMarker.cpp:135
> > +    RenderSVGContainer::paint(info, IntPoint());
> 
> I wonder if paint() should take a default argument for this point now. :)

I smell another cleanup bug!

[Levi Weintraub]

Committed r88250: <http://trac.webkit.org/changeset/88250>