DrawPathCurveToQuadraticBezierSmooth bug

[Danack]

I believe these each of the pairs of commands below should produce the same image as their pair i.e. curveT and curveQ should be the same image, and curve2T should be the same as curveQT.

convert -size 800x500 xc: -fill "#B42AAF" -stroke Black -draw "path 'M50,200 T200,250 z'" curveT.png
convert -size 800x500 xc: -fill "#B42AAF" -stroke Black -draw "path 'M50,200 Q50,200,200,250 z'" curveQ.png

convert -size 800x500 xc: -fill "#B42AAF" -stroke Black -draw "path 'M50,200 T200,250 T400,250 z'" curve2T.png
convert -size 800x500 xc: -fill "#B42AAF" -stroke Black -draw "path 'M50,200 Q50,200,200,250 T400,250 z'" curveQT.png

They appear to be unexpectedly different, and the ones drawn with the T command look wrong.

See also - viewtopic.php?f=1&t=26652

[snibgo]

I think I have cracked it. Dirk probably knows more than I do about "draw path", so I would welcome his confirmation of this.

In draw.c, function TracePath(), for 'T' and 't' we have

Code: Select all

          if (strchr("QqTt",last_attribute) == (char *) NULL)
            {
              points[0]=points[2];
              points[1]=points[3];
            }

This sets the first two "T" points equal to the last. I believe it should, instead, set these to the current point that has been established by a previous Q or T or M or whatever:

Code: Select all

          if (strchr("QqTt",last_attribute) == (char *) NULL)
            {
              points[0]=point;
              points[1]=point;
            }

Likewise for case 'S' and 's'.

So we have a diff from v6.9.0-0:

Code: Select all

--- /home/Alan/ImageMagick-6.9.0-0/magick/draw.c	2014-12-08 18:03:23.393244500 +0000
+++ /home/Alan/ImageMagick-6.9.0-0//magick/factory/draw.c	2014-11-14 01:26:11.000000000 +0000
@@ -5540,8 +5540,8 @@
           }
           if (strchr("CcSs",last_attribute) == (char *) NULL)
             {
-              points[0]=point;
-              points[1]=point;
+              points[0]=points[2];
+              points[1]=points[3];
             }
           for (i=0; i < 4; i++)
             (q+i)->point=points[i];
@@ -5578,8 +5578,8 @@
           }
           if (strchr("QqTt",last_attribute) == (char *) NULL)
             {
-              points[0]=point;
-              points[1]=point;
+              points[0]=points[2];
+              points[1]=points[3];
             }
           for (i=0; i < 3; i++)
             (q+i)->point=points[i];

With this change, the OP examples curve2T.png and curveQT.png are identical, as are these 'S cases:

Code: Select all

convert -size 800x500 xc: -fill "#B42AAF" -stroke Black -draw "path 'M50,200 S150,200,200,250 z'" curveS.png

convert -size 800x500 xc: -fill "#B42AAF" -stroke Black -draw "path 'M50,200 S50,200,50,200,150,200,200,250 z'" curveC.png

[Danack]

Yep - that seems to fix it.

[Danack]

So, will this be merged into the SVN repo?

btw the diff above appears to be the wrong way round.

[snibgo]

btw the diff above appears to be the wrong way round.

Oops, yes, sorry, so it is. It should be:

Code: Select all

--- /home/Alan/ImageMagick-6.9.0-0/magick/factory/draw.c	2014-11-14 01:26:11.000000000 +0000
+++ /home/Alan/ImageMagick-6.9.0-0//magick/draw.c	2014-12-08 18:03:23.393244500 +0000
@@ -5540,8 +5540,8 @@
           }
           if (strchr("CcSs",last_attribute) == (char *) NULL)
             {
-              points[0]=points[2];
-              points[1]=points[3];
+              points[0]=point;
+              points[1]=point;
             }
           for (i=0; i < 4; i++)[size=50][/size]
             (q+i)->point=points[i];
@@ -5578,8 +5578,8 @@
           }
           if (strchr("QqTt",last_attribute) == (char *) NULL)
             {
-              points[0]=points[2];
-              points[1]=points[3];
+              points[0]=point;
+              points[1]=point;
             }
           for (i=0; i < 3; i++)
             (q+i)->point=points[i];

If no one has any further comments, I suggest this change is implemented.

[dlemstra]

This change has been implemented and will be available in the next release of ImageMagick (6.9.0-1)