[Webkit-unassigned] [Bug 43618] Generalize SVGPathParser to allow more than just strings as input source

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Aug 6 07:35:16 PDT 2010


https://bugs.webkit.org/show_bug.cgi?id=43618





--- Comment #3 from Nikolas Zimmermann <zimmermann at kde.org>  2010-08-06 07:35:17 PST ---
(From update of attachment 63719)
Looks like you're almost there, a small design problem remains:

The SVGPathSource passed to parsePathDataFromSource, is stored in an OwnPtr in SVGPathParser. As SVGPathParser is a static object in SVGPathParserFactory, no one will delete the SVGPathSource.
How about changing the design a little, by storing a SVGPathSource* pointer in SVGPathParser, just like it's done for SVGPathConsumer, plus a new setCurrentSource(SVGPathSource*) method.

Then you'd only need to modify SVGPathParserFactory, from:
bool ok = parser->parsePathDataFromSource(SVGPathStringSource::create(d), NormalizedParsing);

to:
OwnPtr<SVGPathSource> source = SVGPathStringSource::create(d);
bool ok = parser->parsePathDataFromSource(source, NormalizedParsing);
...

This way it's guaranteed that source is going to die, after the build* methods are finished.

Some const issues:
WebCore/svg/SVGPathSource.h:32
 +      virtual bool hasMoreData() = 0;
This should be const.

WebCore/svg/SVGPathSource.h:35
 +      virtual bool moveToNextToken() = 0;
Ditto.

WebCore/svg/SVGPathSource.h:36
 +      virtual SVGPathSegType parseSVGSegmentType() = 0;
for consistency, it should be:
virtual bool parseSVGSegmentType(SVGPathSegType& result) = 0;

WebCore/svg/SVGPathSource.h:37
 +      virtual SVGPathSegType nextCommand(SVGPathSegType) = 0;
maybe write 'previousCommand' as parameter name?

WebCore/svg/SVGPathStringSource.cpp:111
 +      if ((*m_current == '+' || *m_current == '-' || *m_current == '.' || (*m_current >= '0' && *m_current <= '9'))
Useless outer brace.

Other than these small issues, it looks great! Looking forward to the bytestream patches :-)
Not setting r- officially, as you wanted me to wait for EWS results.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list