Bug 90716

Summary:

Move zero-length-subpaths from RenderSVGShape to RenderSVGPath

Product:

WebKit

Reporter:

Philip Rogers <pdr>

Component:

SVG

Assignee:

Philip Rogers <pdr>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

eric, webkit.review.bot, zimmermann

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

88231

Attachments:

Description	Flags
Preliminary patch	none
Move zero-length-subpaths from RenderSVGShape to RenderSVGPath	none
Update per reviewer comments	zimmermann: review+, zimmermann: commit-queue-
Update per reviewer comments (2)	none

Philip Rogers

Reported 2012-07-06 19:05:22 PDT

The SVG spec only allows zero-length-subpaths in paths but our current codebase has the code in RenderSVGShape, the superclass of RenderSVGRect and RenderSVGEllipse. Neither rects nor ellipses can contain zero-length-subpaths so we should move the zero-length-subpath code into RenderSVGPath for better code clarity and rendering performance.

Attachments
Preliminary patch (25.08 KB, patch) 2012-07-06 19:35 PDT, Philip Rogers	no flags	Details Formatted Diff Diff
Move zero-length-subpaths from RenderSVGShape to RenderSVGPath (20.71 KB, patch) 2012-07-12 12:33 PDT, Philip Rogers	no flags	Details Formatted Diff Diff
Update per reviewer comments (21.16 KB, patch) 2012-07-12 13:38 PDT, Philip Rogers	zimmermann: review+ zimmermann: commit-queue-	Details Formatted Diff Diff
Update per reviewer comments (2) (21.20 KB, patch) 2012-07-17 13:56 PDT, Philip Rogers	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Philip Rogers

Comment 1 2012-07-06 19:35:47 PDT

Created attachment 151129 [details] Preliminary patch This patch depends on both WK90514 and WK90655 so it's just a preview until those land.

Philip Rogers

Comment 2 2012-07-12 12:33:05 PDT

Created attachment 152024 [details] Move zero-length-subpaths from RenderSVGShape to RenderSVGPath

Stephen Chenney

Comment 3 2012-07-12 13:16:25 PDT

Comment on attachment 152024 [details] Move zero-length-subpaths from RenderSVGShape to RenderSVGPath View in context: https://bugs.webkit.org/attachment.cgi?id=152024&action=review Looks good to me. > Source/WebCore/ChangeLog:10 > + to RenderSVGPath. I would suggest adding what you gain by this.

Philip Rogers

Comment 4 2012-07-12 13:38:56 PDT

Created attachment 152046 [details] Update per reviewer comments Updated ChangeLog to summarize why this is awesome. Also added OVERRIDEs to RenderSVGPath per schenney's comments in wkb.ug/88231

Philip Rogers

Comment 5 2012-07-16 14:08:33 PDT

Nikolas, I think you may have the most context for a review here. Friendly ping for a review :)

Nikolas Zimmermann

Comment 6 2012-07-16 23:46:51 PDT

Comment on attachment 152046 [details] Update per reviewer comments View in context: https://bugs.webkit.org/attachment.cgi?id=152046&action=review Looks great, r=me with a minor comment. > Source/WebCore/rendering/svg/RenderSVGPath.cpp:56 > +FloatRect RenderSVGPath::updateStrokeBoundingBox() const I think updateStrokeBoundingBox is a misleading name, as it returns a new strokeBoundingBox, instead of updating the m_strokeBoundingBox. Maybe you can come up with a better name not involving update... > Source/WebCore/rendering/svg/RenderSVGPath.cpp:96 > + context->save(); Use GraphicsContextStateSaver stateSaver(*context, true) here.

Philip Rogers

Comment 7 2012-07-17 13:56:40 PDT

Created attachment 152821 [details] Update per reviewer comments (2)

WebKit Review Bot

Comment 8 2012-07-17 14:43:15 PDT

Comment on attachment 152821 [details] Update per reviewer comments (2) Clearing flags on attachment: 152821 Committed r122875: <http://trac.webkit.org/changeset/122875>

WebKit Review Bot

Comment 9 2012-07-17 14:43:20 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.