[Webkit-unassigned] [Bug 41410] SVG CleanUp of SVGPathData parsing

[bugzilla-daemon at webkit.org]

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


Nikolas Zimmermann <zimmermann at kde.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #61881|review?                     |review-
               Flag|                            |




--- Comment #16 from Nikolas Zimmermann <zimmermann at kde.org>  2010-07-17 03:19:14 PST ---
(From update of attachment 61881)
Ok, I tried hard, that's all I could find: :-)

JavaScriptCore/wtf/MathExtras.h:58
 +  const double piOverTwoDouble = 1.57079632679489661923;
Needs a ChangeLog.

WebCore/ChangeLog:5
 +          SVG CleanUp of SVGPathData parsing
Please provide a _detailed_ ChangeLog what happens, so others know it too :-)

WebCore/WebCore.vcproj/WebCore.vcproj:2197
 +  
Still newlines here, please remove.

WebCore/svg/SVGPathConsumer.h:60
 +  
A newline too much.

WebCore/svg/SVGPathParser.cpp:65
 +          if (m_mode == RelativeCoordinates)
This still looks weird. How about:

if (m_mode == RelativeCoordinates)
    m_currentPoint += toPoint;
else
    m_currentPoint = toPoint;

m_subPathPoint = m_currentPoint;

WebCore/svg/SVGPathParser.cpp:86
 +              m_currentPoint = m_currentPoint + toPoint;
m_currentPoint += toPoint;

WebCore/svg/SVGPathParser.cpp:175
 +      if (!(m_lastCommand == 'c'
How about reversing this check:
if (m_lastCommand != 'c' && m_...)
Boolean algebra simplifications :-)

WebCore/svg/SVGPathParser.cpp:243
 +      if (!(m_lastCommand == 'q'
Ditto.

WebCore/svg/SVGPathParser.cpp:274
 +      bool largeArc, sweep;
Two lines needed unfortuantely.

WebCore/svg/SVGPathParser.cpp:290
 +      // Spec: radii are nonnegative numbers
Make this a whole sentence please with a trailing period.

WebCore/svg/SVGPathParser.cpp:402
 +              if (command == 'M')
I wish this was commented :(

WebCore/svg/SVGPathParser.cpp:404
 +              else if (command == 'm')
Ditto. But maybe you don't even know why it's needed? Given that it's old legacy code...

WebCore/svg/SVGPathParser.cpp:454
 +      if (scaleFactorSquared < 0)
Can you use float scaleFactorSquared = std::min(1 / d - 0.25f, 0)?

WebCore/svg/SVGPathParser.cpp:484
 +          if (!isfinite(t))
This seems wrong, isfinite returns a non-zero value if it's finite! So you return here, if the value is finite.

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