Bug 44009 – Use SVGPathParser logic to traverse states of a Path

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

[Dirk Schulze]

Use SVGPathParser logic to traverse states of a Path.

[Dirk Schulze]

Created attachment 64417 [details]
Patch

[Eric Seidel (no email)]

Attachment 64417 [details] did not build on mac:
Build output: http://queues.webkit.org/results/3772165

[Dirk Schulze]

(In reply to comment #2)
> Attachment 64417 [details] did not build on mac:
> Build output: http://queues.webkit.org/results/3772165

Hah, just tried the debug build on Mac :-)

[Dirk Schulze]

Created attachment 64420 [details]
Patch

[Nikolas Zimmermann]

Comment on attachment 64420 [details]
Patch

Hi Dirk, good job, some comments:

WebCore/ChangeLog:8
 +          Move the getPathSegAtLength logic from SVGPathSegList to a new SVGPathParser Consumer.
Move the getPathsegAtLength logic from SVGPathSegList into a new SVGPathConsumer class: SVGPathTraversalStateBuilder.

WebCore/ChangeLog:9-10
 +          This allows us to get a SVGPathSeg at a given length of SVGPathByteStreams, as
 +          well as of SVGPathSegLists.
This allows us to get a SVGPathSeg at a given length for SVGPathByteStreams and SVGPathSegLists.

WebCore/svg/SVGPathBuilder.h:39
 +      virtual void nextPathSegment() { }
nextPathSegment sounds like it would give you the next path segment type or something.
suggestion: "pathSegmentStarted". (wrong, see below :-)
Just saw that you're calling it after the segment has been processed, so "incrementPathSegmentCount" sounds even better.


WebCore/svg/SVGPathBuilder.h:40
 +      virtual bool quitEarlier() { return false; }
I'd name it "continueConsuming" return true by default.

WebCore/svg/SVGPathElement.idl:40
 +          unsigned long getPathSegAtLength(in float distance);
Ok, just checked SVG 1.1, this is indeed correct, getPathSegAtLength doesn't raise exceptions, so fine with me.

WebCore/svg/SVGPathParserFactory.cpp:79
 +  static SVGPathTraversalStateBuilder* globalSVGPathTraversalStateBuilder(PathTraversalState* traversalState, float length)
Take a PathTraversalState& reference here.

WebCore/svg/SVGPathParserFactory.cpp:85
 +      s_builder->setCurrentTraversalState(traversalState);
Use &traversalState.

WebCore/svg/SVGPathParserFactory.cpp:275
 +      OwnPtr<PathTraversalState> traversalState = adoptPtr(new PathTraversalState(PathTraversalState::TraversalSegmentAtLength));
Don't create this on the heap - there's no gain in doing so, just create it on the stack.

WebCore/svg/SVGPathTraversalStateBuilder.cpp:70
 +      return (m_traversalState->m_totalLength >= m_traversalState->m_desiredLength);
Useless braces.

WebCore/svg/SVGPathTraversalStateBuilder.cpp:76
 +      m_traversalState->m_segmentIndex++;
++m_traversalState->m_segmentIndex.

[Dirk Schulze]

(In reply to comment #5)
> WebCore/svg/SVGPathElement.idl:40
>  +          unsigned long getPathSegAtLength(in float distance);
> Ok, just checked SVG 1.1, this is indeed correct, getPathSegAtLength doesn't raise exceptions, so fine with me.

Sure, I wouldn't change IDL's if they are not wrong :-)
Working on the other issues.

[Dirk Schulze]

Created attachment 64488 [details]
Patch

[Nikolas Zimmermann]

Comment on attachment 64488 [details]
Patch

r=me, with two small suggestions:

WebCore/ChangeLog:8
 +          Move the getPathsegAtLength logic from SVGPathSegList into a new SVGPathConsumer
s/Pathseg/PathSeg/

WebCore/svg/SVGPathTraversalStateBuilder.h:35
 +      unsigned long getSVGPathSeg();
s/getSVGPathSeg/pathSegmentIndex/

[Dirk Schulze]

Committed r65605: <http://trac.webkit.org/changeset/65605>