Bug 44197

Summary: getPointAtLength returns incorrect values for paths closed with the closepath command
Product: WebKit Reporter: Eric Waller <eric>
Component: SVGAssignee: Dirk Schulze <krit>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, eric, krit, webkit.review.bot, zimmermann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac (Intel)   
OS: OS X 10.5   
URL: http://ericwaller.com/getPointAtLength-bug/index.html
Bug Depends on: 44756    
Bug Blocks:    
Attachments:
Description Flags
Test case for getPointAtLength bug
none
Reduced test case
none
Patch
none
Patch none

Eric Waller
Reported 2010-08-18 13:08:24 PDT
Created attachment 64759 [details] Test case for getPointAtLength bug For paths closed with the closepath ("z") command, getPointAtLength returns incorrect values for points along the line segment which closes the path. I.e. for a path like "M x0,y0 x1,y1 x2,y2 x3,y3 z", getPointAtLength will return incorrect values for the implied line from (x3,y3) to (x0,y0). Explicitly closed paths, like "M x0,y0 x1,y1 x2,y2 x3,y3 x0,y0", are handled correctly.
Attachments
Test case for getPointAtLength bug (30.40 KB, application/zip)
2010-08-18 13:08 PDT, Eric Waller 		no flags
Details
Reduced test case (886 bytes, text/html)
2010-08-19 00:27 PDT, Dirk Schulze 		no flags
Details
Patch (4.49 KB, patch)
2010-08-26 13:12 PDT, Dirk Schulze 		no flags
DetailsFormatted DiffDiff
Patch (4.40 KB, patch)
2010-08-27 04:12 PDT, Dirk Schulze 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Dirk Schulze
Comment 1 2010-08-19 00:27:27 PDT
Created attachment 64814 [details] Reduced test case I would say, that we set the points in http://trac.webkit.org/browser/trunk/WebCore/platform/graphics/PathTraversalState.cpp#L163 wrong. The current point should be the start point of the path: m_current = m_control1 = m_control2 = m_start;
Dirk Schulze
Comment 2 2010-08-25 05:31:20 PDT
This also influences animations on animateTransform for the z-segment. The animated object has a wrong position for this segment.
Dirk Schulze
Comment 3 2010-08-26 13:12:24 PDT
Created attachment 65606 [details] Patch
Nikolas Zimmermann
Comment 4 2010-08-27 01:06:47 PDT
Comment on attachment 65606 [details] Patch Great fix, r=me.
Dirk Schulze
Comment 5 2010-08-27 01:20:16 PDT
Comment on attachment 65606 [details] Patch Clearing flags on attachment: 65606 Committed r66188: <http://trac.webkit.org/changeset/66188>
Dirk Schulze
Comment 6 2010-08-27 01:20:25 PDT
All reviewed patches have been landed. Closing bug.
WebKit Review Bot
Comment 7 2010-08-27 01:54:47 PDT
http://trac.webkit.org/changeset/66188 might have broken Qt Linux Release
Dirk Schulze
Comment 8 2010-08-27 03:07:40 PDT
QtPainterPath makes several optimizations like closing paths, eliminate redundant paths. Thats why DRT results for Paths differ from the other platforms that much. And it's the reason, why this patch fails on Qt. Qt fails anyway, with or without this patch. I'll upload the patch again, with better results for the other bots, but a long term goal is to do all the calculation without the graphic libraries but with the SVGPathByteStream. That way we have common results across all platforms.
Dirk Schulze
Comment 9 2010-08-27 04:12:19 PDT
Created attachment 65697 [details] Patch
Nikolas Zimmermann
Comment 10 2010-08-27 04:20:35 PDT
Comment on attachment 65697 [details] Patch r=me. Needs updates on most platforms...
Dirk Schulze
Comment 11 2010-08-27 05:09:26 PDT
Comment on attachment 65697 [details] Patch Clearing flags on attachment: 65697 Committed r66208: <http://trac.webkit.org/changeset/66208>
Dirk Schulze
Comment 12 2010-08-27 05:09:36 PDT
All reviewed patches have been landed. Closing bug.
Dirk Schulze
Comment 13 2010-08-27 06:09:14 PDT
Committed r66214: <http://trac.webkit.org/changeset/66214>
WebKit Review Bot
Comment 14 2010-08-27 06:42:53 PDT
http://trac.webkit.org/changeset/66214 might have broken Leopard Intel Release (Tests)
Note You need to log in before you can comment on or make changes to this bug.