praeclarum / NGraphics

NGraphics is a cross platform library for rendering vector graphics on .NET. It provides a unified API for both immediate and retained mode graphics using high quality native renderers.
MIT License
711 stars 133 forks source link

Incorrect decoding of coordinates in SVG path parameters #77

Closed franzdevel closed 5 years ago

franzdevel commented 7 years ago

The following SVG image is not processed correctly by the SvgReader:

<svg xmlns="http://www.w3.org/2000/svg" width="48" height="48" viewBox="0 0 48 48">
    <path d="M13.25 21.59c2.88 5.66 7.51 10.29 13.18 13.17l4.4-4.41c.55-.55 1.34-.71 2.03-.49C35.1 30.6 37.51 31 40 31c1.11 0 2 .89 2 2v7c0 1.11-.89 2-2 2C21.22 42 6 26.78 6 8c0-1.11.9-2 2-2h7c1.11 0 2 .89 2 2 0 2.49.4 4.9 1.14 7.14.22.69.06 1.48-.49 2.03l-4.4 4.42z" fill="#000000" fill-opacity="0.54" />
</svg>

The issue is with one of the 'c' (curveto) commands in the path: c0-1.11.9-2 2-2. More concretely, the reader cannot correctly decode the block -1.11.9 which includes multiple coordinates not separated by wsp.

StenPetrov commented 7 years ago

Is the "c0-1.11.9-2 2-2" token expected to be decoded correctly? Is it C 0 1.1 1.9 -2 2 -2 or C 0 1.11 .9 -2 2 -2?

franzdevel commented 7 years ago

I think it is the second version c 0 1.11 .9 -2 2 -2 (with the small 'c' for relative curveto). In the standard at https://www.w3.org/TR/SVG/paths.html#PathData, the end of the section reads:

The processing of the BNF must consume as much of a given BNF production as possible, stopping at the point when a character is encountered which no longer satisfies the production. Thus, in the string "M 100-200", the first coordinate for the "moveto" consumes the characters "100" and stops upon encountering the minus sign because the minus sign cannot follow a digit in the production of a "coordinate". The result is that the first coordinate will be "100" and the second coordinate will be "-200".

Similarly, for the string "M 0.6.5", the first coordinate of the "moveto" consumes the characters "0.6" and stops upon encountering the second decimal point because the production of a "coordinate" only allows one decimal point. The result is that the first coordinate will be "0.6" and the second coordinate will be ".5".

In our example, decoding the first coordinate in -1.11.9 cannot stop at -1.1 because -1.11 is still a valid number. So we take -1.11 as the first coordinate and .9 as the second one.

I'm working on a corresponding fix for SvgReader.ReadPath().

franzdevel commented 7 years ago

Here comes the fix for SvgReader.ReadPath().

(this version also includes the fix for #75)

Please check solution for overall correctness.

static Regex pathRegex = new Regex(@"[MLHVCSQTAZmlhvcsqtaz][^MLHVCSQTAZmlhvcsqtaz]*", RegexOptions.Singleline);
static Regex negativeNumberRe = new Regex("(?<=[0-9])-");
static Regex floatingPointRe = new Regex("(?<=\\.[0-9]+)\\.");

void ReadPath (Path p, string pathDescriptor)
{
    Match m = pathRegex.Match(pathDescriptor);
    Point previousPoint = new Point();
    while(m.Success)
    {
        var match = m.Value.TrimStart ();
        var op = match[0];

        if (op == 'z' || op == 'Z') {
            p.Close ();
        } else {
            // ensure all coordinates are split by WSC
            match = negativeNumberRe.Replace(match.Substring(1), " -");
            match = floatingPointRe.Replace(match, " .");
            var args = match.Split(WSC, StringSplitOptions.RemoveEmptyEntries);

            int index = 0;
            while(index < args.Length)
            {
                if ((op == 'M' || op == 'm') && args.Length >= index+2) {
                    var point = new Point (ReadNumber (args [index]), ReadNumber (args [index+1]));
                    if (op == 'm')
                        point += previousPoint;
                    p.MoveTo (point);
                    index += 2;
                } else if ((op == 'L' || op == 'l') && args.Length >= index+2) {
                    var point = new Point (ReadNumber (args [index]), ReadNumber (args [index+1]));
                    if (op == 'l')
                        point += previousPoint;
                    p.LineTo (point);
                    index += 2;
                } else if ((op == 'C' || op == 'c') && args.Length >= index+6) {
                    var c1 = new Point (ReadNumber (args [index]), ReadNumber (args [index+1]));
                    var c2 = new Point (ReadNumber (args [index+2]), ReadNumber (args [index+3]));
                    var pt = new Point (ReadNumber (args [index+4]), ReadNumber (args [index+5]));
                    if (op == 'c')
                    {
                        c1 += previousPoint;
                        c2 += previousPoint;
                        pt += previousPoint;
                    }
                    p.CurveTo (c1, c2, pt);
                    index += 6;
                } else if ((op == 'S' || op == 's') && args.Length >= index+4) {
                    var c  = new Point (ReadNumber (args [index]), ReadNumber (args [index+1]));
                    var pt = new Point (ReadNumber (args [index+2]), ReadNumber (args [index+3]));
                    if (op == 's')
                    {
                        c += previousPoint;
                        pt += previousPoint;
                    }
                    p.ContinueCurveTo (c, pt);
                    index += 4;
                } else if ((op == 'A' || op == 'a') && args.Length >= index+7) {
                    var r = new Size (ReadNumber (args [index]), ReadNumber (args [index+1]));
                    var laf = ReadNumber (args [index+3]) != 0;
                    var swf = ReadNumber (args [index+4]) != 0;
                    var pt = new Point (ReadNumber (args [index+5]), ReadNumber (args [index+6]));
                    if (op == 'a')
                        pt += previousPoint;
                    p.ArcTo (r, laf, swf, pt);
                    index += 7;
                } else if ((op == 'V' || op == 'v') && args.Length >= index+1 && p.Operations.Count > 0) {
                    var previousX = previousPoint.X;
                    var y = ReadNumber(args[index]);
                    if (op == 'v')
                        y += previousPoint.Y;
                    var point = new Point(previousX, y);
                    p.LineTo(point);
                    index += 1;
                } else if ((op == 'H' || op == 'h') && args.Length >= index+1 && p.Operations.Count > 0) {
                    var previousY = previousPoint.Y;
                    var x = ReadNumber(args[index]);
                    if (op == 'h')
                        x += previousPoint.X;
                    var point = new Point(x, previousY);
                    p.LineTo(point);
                    index += 1;
                } else {
                    throw new NotSupportedException ("Cannot decode path operation " + op + match);
                }
                previousPoint = p.Operations.Last().EndPoint;
            }
        }
        m = m.NextMatch();
    }
}
justintemplar commented 7 years ago

Is this fix in a current build?

franzdevel commented 7 years ago

No, I don't think I have the rights to actually submit code to the project.

praeclarum commented 5 years ago

Hello, thank you for fixing this. I have merged it in.