Implenting addArc feature

[derBobo]

Prerequisites

• [x] I have written a descriptive pull-request title
• [x] I have verified that there are no overlapping pull-requests open
• [x] I have verified that I am following matches the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules :cop:.
• [x] I have provided test coverage for my change (where applicable)

Description

This should implement the requested features from #4

[CLAassistant]

All committers have signed the CLA.

[derBobo]

Currently the transformation feature is still not completed, I think there are two ways to implement this:

1. (current code tries this) Calculate everything, annoying because for every angle for two points on the circle there are at least 2 possibilities (clockwise and anticlockwise) .
2. Every elliptical arc stores it current transformation matrix and uses it to calculate the current points

[antonfirsov]

@derBobo feel free to go with 2., we can always optimize later, the main goal is to close the feature gap.

[derBobo]

Has anyone any idea, why this code always returns the point (0|0)? It works fine without the usage of PointF.Transform

 private PointF CalculatePoint(float angle)
        {
            float x = (this.firstRadius * MathF.Sin(MathF.PI * angle / 180) * MathF.Cos(MathF.PI * this.rotation / 180)) - (this.secondRadius * MathF.Cos(MathF.PI * angle / 180) * MathF.Sin(MathF.PI * this.rotation / 180)) + this.center.X;
            float y = (this.firstRadius * MathF.Sin(MathF.PI * angle / 180) * MathF.Sin(MathF.PI * this.rotation / 180)) + (this.secondRadius * MathF.Cos(MathF.PI * angle / 180) * MathF.Cos(MathF.PI * this.rotation / 180)) + this.center.Y;
            return PointF.Transform(new PointF(x, y), this.transformation);
        }

EDIT: nvm found my mistake

[derBobo]

Should I also change the EllipsePlygon class to use the EllipticalArcLineSegment instead of CubicBezierLineSegment?

[codecov[bot]]

Codecov Report

Merging #144 (0087497) into master (d31cc33) will increase coverage by 0.13%. The diff coverage is 79.36%.

@@            Coverage Diff             @@
##           master     #144      +/-   ##
==========================================
+ Coverage   69.88%   70.02%   +0.13%     
==========================================
  Files          88       89       +1     
  Lines        5144     5207      +63     
  Branches     1052     1062      +10     
==========================================
+ Hits         3595     3646      +51     
- Misses       1338     1347       +9     
- Partials      211      214       +3     

Flag	Coverage Δ
unittests	70.02% <79.36%> (+0.13%)	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/ImageSharp.Drawing/Shapes/PathBuilder.cs	88.23% <37.50%> (-5.28%)	:arrow_down:
...geSharp.Drawing/Shapes/EllipticalArcLineSegment.cs	85.45% <85.45%> (ø)
src/ImageSharp.Drawing/Shapes/Clipper/clipper.cs	49.09% <0.00%> (+0.04%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d31cc33...0087497. Read the comment docs.

[JimBobSquarePants]

@derBobo i don’t actually know the answer to that. @tocsoft what do you think?

[tocsoft]

from memory its really an internal implemetation detail on what sort of line segment we are using so I would say unless it makes a noticable/more acurate/faster difference to the output then I personally wouldn't bother. But saying all that as long as we have sufficent test coverage then I can't imagine it would cause too much harm.

[JimBobSquarePants]

Thanks @tocsoft makes sense!

[derBobo]

does anyone have more ideas fore Xunit tests ?

[antonfirsov]

does anyone have more ideas fore Xunit tests ?

We need tests that actually draw the EllipticalArcLineSegment.

Example tests doing some drawing of IPath, you can add the new ones here: https://github.com/SixLabors/ImageSharp.Drawing/blob/master/tests/ImageSharp.Drawing.Tests/Drawing/DrawPathTests.cs

These tests are validating the output agains reference images found here: https://github.com/SixLabors/ImageSharp.Drawing/tree/master/tests/Images/ReferenceOutput/Drawing/DrawPathTests

The images are Debug-Saved under the folder /Images/ActualOutput/Drawing/DrawPathTests/. If you think the output is correct, just copy the files to the appropriate ReferenceOutput folder.

[derBobo]

I think this feature now is ready for review.

[JimBobSquarePants]

@SixLabors/core would someone have a chance to review this?

[derBobo]

You're welcome.