| Summary: | Refactor RenderSVG{Shape,Rect,Ellipse,Path} to be less coupled. | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Philip Rogers <pdr> | ||||
| Component: | SVG | Assignee: | Nobody <webkit-unassigned> | ||||
| Status: | NEW --- | ||||||
| Severity: | Normal | CC: | schenney, zimmermann | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Bug Depends on: | 91131, 90514, 90655, 90716 | ||||||
| Bug Blocks: | |||||||
| Attachments: |
|
||||||
Created attachment 145585 [details]
Preview patch
Preview patch for the direction I'd like to go. Looking for initial impressions.
Comment on attachment 145585 [details] Preview patch 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. |
RenderSVG{Shape,Rect,Ellipse,Path} are too tightly coupled.