[Webkit-unassigned] [Bug 88231] Refactor RenderSVG{Shape, Rect, Ellipse, Path} to be less coupled.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=88231





--- Comment #2 from Stephen Chenney <schenney at chromium.org>  2012-06-04 08:07:31 PST ---
(From update of attachment 145585)
View in context: https://bugs.webkit.org/attachment.cgi?id=145585&action=review

> Source/WebCore/rendering/svg/RenderSVGEllipse.h:59
> +    bool usePathStrokeContains;

I think that m_shouldUsePathStrokeContains would better fit WebKit style.

> Source/WebCore/rendering/svg/RenderSVGPath.h:41
> +    virtual void applyNonScalingStrokeTransform(const AffineTransform*, bool);

Should any or all of the new methods be OVERRIDE?

> Source/WebCore/rendering/svg/RenderSVGRect.h:59
> +    bool usePathStrokeContains;

m_shouldUsePathStrokeContains

> Source/WebCore/rendering/svg/RenderSVGShape.cpp:190
> +void RenderSVGShape::applyNonScalingStrokeTransform(const AffineTransform* strokeTransform, bool inverse)

I'm not entirely happy with this. I can see that you want to cache the pre transform path and also take care of clearing the path, but it seems like there is a cleaner way to do this. I would most like to get rid of the "new" in the unapply case.

> Source/WebCore/rendering/svg/RenderSVGShape.h:89
> +    virtual FloatRect strokeBoundingBox() const;

If your adding this, it would be good to add the OVERRIDE to all of these methods.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.