Bug 52107 – SVG rendering clean up according to the webkit style rules

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Dirk Schulze]

SVG rendering clean up according to the webkit style rules

[Dirk Schulze]

Created attachment 78307 [details]
Patch

[Adam Barth]

Comment on attachment 78307 [details]
Patch

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

> WebCore/rendering/RenderSVGModelObject.h:70
> -    bool nodeAtPoint(const HitTestRequest& request, HitTestResult& result, int xInContainer, int yInContainer, int dxParentToContainer, int dyParentToContainer, HitTestAction hitTestAction);
> +    bool nodeAtPoint(const HitTestRequest&, HitTestResult&, int xInContainer, int yInContainer, int dxParentToContainer, int dyParentToContainer, HitTestAction);

This change doesn't seem valuable.  How else will we know which one is the request and which one is the result?

> WebCore/rendering/RenderSVGResourceFilterPrimitive.h:47
> -    static FloatRect determineFilterPrimitiveSubregion(FilterEffect* effect, SVGFilter* filter);
> +    static FloatRect determineFilterPrimitiveSubregion(FilterEffect*, SVGFilter*);

Same thing here.

> WebCore/rendering/RenderSVGRoot.cpp:324
> -bool RenderSVGRoot::nodeAtPoint(const HitTestRequest& request, HitTestResult& result, int _x, int _y, int _tx, int _ty, HitTestAction hitTestAction)
> +bool RenderSVGRoot::nodeAtPoint(const HitTestRequest& request, HitTestResult& result, int x, int y, int tx, int ty, HitTestAction hitTestAction)
>  {
> -    IntPoint pointInContainer(_x, _y);
> -    IntSize containerToParentOffset(_tx, _ty);
> +    IntPoint pointInContainer(x, y);
> +    IntSize containerToParentOffset(tx, ty);

I don't understand this code well enough to know whether this is a good change.  I think _x might mean something in the rendering code.

[Dirk Schulze]

We talked about this on IRC, just for the bug history:

(In reply to comment #2)
> (From update of attachment 78307 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=78307&action=review
> 
> > WebCore/rendering/RenderSVGModelObject.h:70
> > -    bool nodeAtPoint(const HitTestRequest& request, HitTestResult& result, int xInContainer, int yInContainer, int dxParentToContainer, int dyParentToContainer, HitTestAction hitTestAction);
> > +    bool nodeAtPoint(const HitTestRequest&, HitTestResult&, int xInContainer, int yInContainer, int dxParentToContainer, int dyParentToContainer, HitTestAction);
> 
> This change doesn't seem valuable.  How else will we know which one is the request and which one is the result?
The object type tells what is the request and what is the result (HitTestResult, HitTestRequest).

> 
> > WebCore/rendering/RenderSVGResourceFilterPrimitive.h:47
> > -    static FloatRect determineFilterPrimitiveSubregion(FilterEffect* effect, SVGFilter* filter);
> > +    static FloatRect determineFilterPrimitiveSubregion(FilterEffect*, SVGFilter*);
> 
> Same thing here.
Ditto. FilterEffect and SVGFilter.

> 
> > WebCore/rendering/RenderSVGRoot.cpp:324
> > -bool RenderSVGRoot::nodeAtPoint(const HitTestRequest& request, HitTestResult& result, int _x, int _y, int _tx, int _ty, HitTestAction hitTestAction)
> > +bool RenderSVGRoot::nodeAtPoint(const HitTestRequest& request, HitTestResult& result, int x, int y, int tx, int ty, HitTestAction hitTestAction)
> >  {
> > -    IntPoint pointInContainer(_x, _y);
> > -    IntSize containerToParentOffset(_tx, _ty);
> > +    IntPoint pointInContainer(x, y);
> > +    IntSize containerToParentOffset(tx, ty);
> 
> I don't understand this code well enough to know whether this is a good change.  I think _x might mean something in the rendering code.
Double check it before landing.

[Dirk Schulze]

Committed r75324: <http://trac.webkit.org/changeset/75324>