SvgPath.Path() behavior changed when ISvgRenderer argument is null

[pablojmp]

Description

When traversing the SVG document in order to translate it to a custom vector format, I was using the curves contained in the GraphicsPath returned by Path() to construct the corresponding class in the output vector format. After version 3.0.102 the GraphicsPath contained ellipses not found in previous versions.

Example data

This is the specific code that breaks on the new version:

private Figure ExtractFigure( SvgVisualElement aVisualElement )
{
  var lGraphicsPath = aVisualElement.Path( null );
  var lIterator     = new GraphicsPathIterator( lGraphicsPath );
  var lMultiRegion  = PathToCurve.Proceed( lIterator );
  lMultiRegion?.Transform( Transformation.MirrorY() );

  return BuildShape( AddStyle( lMultiRegion, aVisualElement ) );
}

When aVisualElement is of type SvgPath, the changes introduced in #500 add ellipses to correct for bounds calculations when the renderer is null, but that isn't the actual path represented in the SVG path data. I can work around this by using a renderer that does nothing, but i think that the GraphicsPath returned should match the svg path data, regardless of how the the bounds are computed.

One of my test cases (sample.zip) was a 5 point closed SvgPath that returned a GraphicsPath with 32 points when passing null as the renderer. When i used the output vector format and rendered that into a PNG i got the circles in the output:

I think a good way of correcting the bounds and leaving the GraphicsPath as it was is to do the following changes:

public override GraphicsPath Path(ISvgRenderer renderer)
{
  if (_path == null || IsPathDirty)
  {
    _path = new GraphicsPath();

    if (PathData != null && PathData.Count > 0 && PathData.First is SvgMoveToSegment)
    {
      foreach (var segment in PathData)
        segment.AddToPath(_path);

      if (_path.PointCount == 0)
      {
        if (PathData.Count > 0)
        {
          // special case with one move command only, see #223
          var segment = PathData.Last;
          _path.AddLine(segment.End, segment.End);
          Fill = SvgPaintServer.None;
        }
        else
          _path = null;
      }
    }
    IsPathDirty = false;
  }
  return _path;
}

public override RectangleF Bounds
{
  get
  {
    if (IsPathDirty)
    {
      var boundsPath = (GraphicsPath) Path(null)?.Clone();
      if (boundsPath != null)
      {
        var radius = StrokeWidth * 2;
        var bounds = boundsPath.GetBounds();
        boundsPath.AddEllipse(bounds.Left - radius, bounds.Top - radius, 2 * radius, 2 * radius);
        boundsPath.AddEllipse(bounds.Right - radius, bounds.Bottom - radius, 2 * radius, 2 * radius);
        _bounds = boundsPath.GetBounds();
      }
      else
      {
        _bounds = RectangleF.Empty;
      }
        
    }
    return _bounds;
  }
}

private RectangleF _bounds;

Used Versions

This can be reproduced on Windows 10 1909 x64, .NET Framework 4.7, Svg nuget version 3.0.102

[pablojmp]

@H1Gdev regarding #500

[H1Gdev]

@pablojmp

Interface specification of Path method is here. https://github.com/vvvv/SVG/pull/500#discussion_r297482241

SvgPath violated this spec. and was fixed in PR # 500. Changes to that specification will affect all derived classes of SvgVisualElement.

[pablojmp]

@H1Gdev forgive my ignorance, where are the docs for that spec? or are the comments to that PR the specs?

Also, if the PR is following what's done in SvgLine, i think the code should read

var radius = StrokeWidth / 2;

I don't get why Path should have extra figures only to get the bounds... if rendering is using GDI+, GraphicsPath.GetBounds(Matrix, Pen) gives the same result than the current implementation and keeps _path tidy.

After some testing, if we use the old Path(null) definition and do

Path(null).GetBounds(
new Matrix(),
new Pen(System.Drawing.Color.Transparent, lPath.StrokeWidth / 2) {
  StartCap = LineCap.Round, EndCap = LineCap.Round,
  LineJoin = LineJoin.Round });

returns the same RectangleF as the #500 implementation (after the ellipse radius correction)

[H1Gdev]

forgive my ignorance, where are the docs for that spec? or are the comments to that PR the specs?

As stated in https://github.com/vvvv/SVG/pull/500#discussion_r297482241, I think this was made to fix some bugs. (I don't know why this was done before I joined.)

I have also tried to remove this specification, but it was not easy. Path method is called in various situations.

Right now I prioritize improving code quality.