[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