Bug 62177

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

Description Levi Weintraub 2011-06-06 19:11:06 PDT
Ongoing tx/ty removal. This is the "big one" for paint?
Comment 1 Levi Weintraub 2011-06-06 19:12:48 PDT
Created attachment 96176 [details]
WIP
Comment 2 Levi Weintraub 2011-06-07 10:46:04 PDT
Created attachment 96261 [details]
WIP that works :)
Comment 3 WebKit Review Bot 2011-06-07 10:48:37 PDT
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.
Comment 4 Levi Weintraub 2011-06-07 11:37:28 PDT
Created attachment 96269 [details]
Patch
Comment 5 Eric Seidel (no email) 2011-06-07 11:47:11 PDT
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. :)
Comment 6 Levi Weintraub 2011-06-07 11:54:32 PDT
(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!
Comment 7 Levi Weintraub 2011-06-07 11:56:53 PDT
Committed r88250: <http://trac.webkit.org/changeset/88250>