Summary: | Switch paint to use IntPoint | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Levi Weintraub <leviw> | ||||||||
Component: | Layout and Rendering | Assignee: | 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
Levi Weintraub
2011-06-06 19:11:06 PDT
Created attachment 96176 [details]
WIP
Created attachment 96261 [details]
WIP that works :)
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.
Created attachment 96269 [details]
Patch
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. :) (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! Committed r88250: <http://trac.webkit.org/changeset/88250> |