Bug 43837

Summary: [Qt] PathQt should give back pointAtLength and length
Product: WebKit Reporter: Dirk Schulze <krit>
Component: WebKit QtAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ariya.hidayat, commit-queue, hausmann, jhanssen, krit, zimmermann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Proposed patch
none
Proposed patch v2
none
Proposed patch v3 none

Dirk Schulze
Reported 2010-08-11 01:21:18 PDT
PathQt should give back pointAtLength and length itself. At the moment Qt is using PathTraversalState in Path.cpp, like most other platforms. But I could be a performance win to use qreal percentAtLength ( qreal len ) const QPointF pointAtPercent for pointAtLength and qreal length () const for length().
Attachments
Proposed patch (5.14 KB, patch)
2010-09-01 22:28 PDT, Jan Erik Hanssen 		no flags
DetailsFormatted DiffDiff
Proposed patch v2 (3.88 KB, patch)
2010-09-07 23:25 PDT, Jan Erik Hanssen 		no flags
DetailsFormatted DiffDiff
Proposed patch v3 (4.21 KB, patch)
2010-09-08 00:43 PDT, Jan Erik Hanssen 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Ariya Hidayat
Comment 1 2010-08-11 06:36:36 PDT
Adding myself.
Jan Erik Hanssen
Comment 2 2010-09-01 22:28:36 PDT
Created attachment 66332 [details] Proposed patch
Jan Erik Hanssen
Comment 3 2010-09-03 12:58:45 PDT
Comment on attachment 66332 [details] Proposed patch The test case for path-totalLength is wrong, it passes for Qt but not any of the other platforms. Will upload a new patch later.
Dirk Schulze
Comment 4 2010-09-03 23:09:51 PDT
Sorry, I thougt I commented on this bug. Iam not sure about pointAtLength. IIRC we give back the last possible point if the given length is larger than the totalLength of the path. Can you check this first please?
Jan Erik Hanssen
Comment 5 2010-09-06 23:53:28 PDT
(In reply to comment #4) > Sorry, I thougt I commented on this bug. Iam not sure about pointAtLength. IIRC we give back the last possible point if the given length is larger than the totalLength of the path. Can you check this first please? I'll have a look before uploading a revised patch. Thanks.
Jan Erik Hanssen
Comment 6 2010-09-07 23:25:21 PDT
Created attachment 66847 [details] Proposed patch v2 Adding a test for Path::pointAtLength() for the case where the length argument is beyond the length of the path. Path::length() is already tested.
Dirk Schulze
Comment 7 2010-09-07 23:39:24 PDT
Why did you remove normalAngleAtLength from your first patch. Doen't http://doc.trolltech.com/4.6/qpainterpath.html#angleAtPercent make what you want?
Jan Erik Hanssen
Comment 8 2010-09-07 23:47:39 PDT
(In reply to comment #7) > Why did you remove normalAngleAtLength from your first patch. Doen't http://doc.trolltech.com/4.6/qpainterpath.html#angleAtPercent make what you want? It probably does. Path::normalAngleAtLength() not directly exposed from JS though, making it a bit more tricky to write a test case for that. Probably need to trigger a test via some SVG animation if that's possible (I haven't looked extensively at this yet).
Dirk Schulze
Comment 9 2010-09-07 23:58:50 PDT
(In reply to comment #8) > (In reply to comment #7) > > Why did you remove normalAngleAtLength from your first patch. Doen't http://doc.trolltech.com/4.6/qpainterpath.html#angleAtPercent make what you want? > > It probably does. Path::normalAngleAtLength() not directly exposed from JS though, making it a bit more tricky to write a test case for that. Probably need to trigger a test via some SVG animation if that's possible (I haven't looked extensively at this yet). It is used for text on path in SVG. So if it doesn't work you break some SVG tests. IIRC Qt has pixel tests, so you should recognize it. If you want to check it your own, visit http://dev.w3.org/SVG/profiles/1.1F2/test/harness/htmlObject/text-path-01-b.html
Jan Erik Hanssen
Comment 10 2010-09-08 00:34:35 PDT
(In reply to comment #9) > It is used for text on path in SVG. So if it doesn't work you break some SVG tests. IIRC Qt has pixel tests, so you should recognize it. If you want to check it your own, visit http://dev.w3.org/SVG/profiles/1.1F2/test/harness/htmlObject/text-path-01-b.html You're right, LayoutTests/svg/text/text-hkern.svg fails if Path::normalAngleAtLength() returns incorrect results. That should be sufficient for testing this, I'll attach a new patch.
Jan Erik Hanssen
Comment 11 2010-09-08 00:43:52 PDT
Created attachment 66854 [details] Proposed patch v3
Dirk Schulze
Comment 12 2010-09-08 01:01:32 PDT
Comment on attachment 66854 [details] Proposed patch v3 View in context: https://bugs.webkit.org/attachment.cgi?id=66854&action=prettypatch LGTM. r=me.
WebKit Commit Bot
Comment 13 2010-09-08 07:07:27 PDT
Comment on attachment 66854 [details] Proposed patch v3 Clearing flags on attachment: 66854 Committed r66979: <http://trac.webkit.org/changeset/66979>
WebKit Commit Bot
Comment 14 2010-09-08 07:07:32 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.