Page 1 of 1

AffineTransform bug in PerlMagick

Posted: 2007-01-29T07:04:03-07:00
by rmabry
When giving the 6-element affine matrix in PerlMagick, the last element is not read, making ty=0 in all cases. I think each of the comparisons of length below should be decreased by one. I have 6.3.2-2 at the moment.

Code: Select all

             if (av_len(av) >= 1)
                draw_info->affine.sx=(double) SvNV(*(av_fetch(av,0,0)));
              if (av_len(av) >= 2)
                draw_info->affine.rx=(double) SvNV(*(av_fetch(av,1,0)));
              if (av_len(av) >= 3)
                draw_info->affine.ry=(double) SvNV(*(av_fetch(av,2,0)));
              if (av_len(av) >= 4)
                draw_info->affine.sy=(double) SvNV(*(av_fetch(av,3,0)));
              if (av_len(av) >= 5)
                draw_info->affine.tx=(double) SvNV(*(av_fetch(av,4,0)));
              if (av_len(av) >= 6)
                draw_info->affine.ty=(double) SvNV(*(av_fetch(av,5,0)));
Rick

Re: AffineTransform bug in PerlMagick

Posted: 2007-01-30T21:10:35-07:00
by rmabry
rmabry wrote: When giving the 6-element affine matrix in PerlMagick, the last element is not read


I should also mention that for arrays of length 1 through 6, the affine matrix is not properly formed because the last element is not read. (Giving it a bogus 7-element array will cause the first 6 elements to be read, which would be a work-around if such a thing is left unfixed.) The array is initialized to the identity array [1,0,0,1,0,0] and then the elements of the 6-member array are read in over that (with the caveat above). This can lead to some very naughty behavior, indeed! (There's a pun of size zero in there.) For instance,

$image->AffineTransform(affine => [1,1,1,2]);

actually results in the matrix [1,1,1,1,0,0] being produced because only the [1,1,1] is copied over the [1,0,0,1,0,0]. That resulting matrix produces a transform that is "singular" (its determinant is zero for those of you who recall such things) and the program hangs. (Or at least it goes off for a long time and I kill it.)

My feeling:

1. There really should be a check for a singular matrix in any case.

2. The bug itself should be fixed!

3. But instead of reading *up to* six elements as the code now attempts to do, it should only be legal to read 4 or 6 matrix elements. If 4 are read the remaining two (tx and ty) can be assumed zero, i.e., no translation.

The following seems to work for me in Magick.xs, circa line 7378:

Code: Select all

        case 75:  /* AffineTransform */
        {
          DrawInfo
            *draw_info;

          draw_info=CloneDrawInfo(info ? info->image_info : (ImageInfo *) NULL,
            (DrawInfo *) NULL);
          if (attribute_flag[0] != 0)
            {
              AV
                *av;

              av=(AV *) argument_list[0].array_reference;
/* getrid of this...
              if (av_len(av) >= 1)
                draw_info->affine.sx=(double) SvNV(*(av_fetch(av,0,0)));
              if (av_len(av) >= 2)
                draw_info->affine.rx=(double) SvNV(*(av_fetch(av,1,0)));
              if (av_len(av) >= 3)
                draw_info->affine.ry=(double) SvNV(*(av_fetch(av,2,0)));
              if (av_len(av) >= 4)
                draw_info->affine.sy=(double) SvNV(*(av_fetch(av,3,0)));
              if (av_len(av) >= 5)
                draw_info->affine.tx=(double) SvNV(*(av_fetch(av,4,0)));
              if (av_len(av) >= 6)
                draw_info->affine.ty=(double) SvNV(*(av_fetch(av,5,0)));
*/                
              if(av_len(av) != 3 && av_len(av) != 5)
              {
                    ThrowPerlException(&exception,OptionError,
                        "Affine Matrix must have 4 or 6 elements",PackageName);
                    goto PerlException;
              }

              draw_info->affine.sx=(double) SvNV(*(av_fetch(av,0,0)));
              draw_info->affine.rx=(double) SvNV(*(av_fetch(av,1,0)));
              draw_info->affine.ry=(double) SvNV(*(av_fetch(av,2,0)));
              draw_info->affine.sy=(double) SvNV(*(av_fetch(av,3,0)));

              if(fabs(draw_info->affine.sx * draw_info->affine.sy 
			- draw_info->affine.rx * draw_info->affine.ry) < MagickEpsilon)
              {
                   ThrowPerlException(&exception,OptionError,
                		"Affine Matrix is singular",PackageName);
                   goto PerlException;
              }
              if(av_len(av) == 5 )
              {
                  draw_info->affine.tx=(double) SvNV(*(av_fetch(av,4,0)));
                  draw_info->affine.ty=(double) SvNV(*(av_fetch(av,5,0)));
              }
       }
/* end changes */
       if (attribute_flag[6] != 0)
/* etc. */
Rick

Posted: 2007-01-30T21:37:51-07:00
by magick
Your patch is in ImageMagick 6.3.2-2 Beta. Thanks.