QueryMagickColor bug

Post any defects you find in the released or beta versions of the ImageMagick software here. Include the ImageMagick version, OS, and any command-line required to reproduce the problem. Got a patch for a bug? Post it here.
Post Reply
el_supremo
Posts: 1015
Joined: 2005-03-21T21:16:57-07:00

QueryMagickColor bug

Post by el_supremo »

[edit] I forgot to mention this is in version 6.2.6 03/18/06 Q8

There's a bug in the way that QueryMagickColor (in color.c) handles hex colour specifications.
This code:

Code: Select all

PixelSetColor(c_wand,"rgb(0,0,0)");
MessageBox(NULL,PixelGetColorAsString(c_wand),"",MB_OK);
will print 0,0,0 as it should but the equivalent colour in hex:

Code: Select all

PixelSetColor(c_wand,"#000000");
MessageBox(NULL,PixelGetColorAsString(c_wand),"",MB_OK);
will print 0,0,0,0 which indicates that matte has been set.
In QueryMagickColor, having detected that it is dealing with a hex code, there are these statements:

Code: Select all

if ((n == 3) || (n == 6) || (n == 9) || (n == 12) || (n == 24))
   {
     n/=3;
If the hex code is #000000 then once this if statement is done, n will be 2.
In the "else" statement tied to this, it does:

Code: Select all

else
  {
    n/=4;
This would be executed if the code were #00000000, in which case n will also be 2 after the if statement.
But outside the if statement it continues:

Code: Select all

n<<=2;
quantum_range=(1UL << n)-1;
if (n == 32)
  quantum_range=4294967295UL;
color->colorspace=RGBColorspace;
color->matte=MagickFalse;
color->red=(MagickRealType) ScaleAnyToQuantum(pixel.red,quantum_range);
color->green=(MagickRealType) ScaleAnyToQuantum(pixel.green,
  quantum_range);
color->blue=(MagickRealType) ScaleAnyToQuantum(pixel.blue,quantum_range);
color->opacity=(MagickRealType) OpaqueOpacity;
if ((n != 3) && (n != 6) && (n != 9) && (n != 12) && (n != 24))
{
  color->matte=MagickTrue;
  color->opacity=(MagickRealType)


    ScaleAnyToQuantum(pixel.opacity,quantum_range);
}
The n<<2 multiplies n by 4 which will compensate for the divide by 4 in the else clause but will not be correct in the case where n was divided by three in the initial if clause.
This will mean that any hex spec with six digits, which should imply no matte, will actually set the matte and opacity because n will be 8 after "n<<2".

One way to fix this would be to change this:

Code: Select all

if ((n == 3) || (n == 6) || (n == 9) || (n == 12) || (n == 24))
  {
    n/=3;
    do
    {
      pixel.red=pixel.green;
      pixel.green=pixel.blue;
      pixel.blue=0;
      for (i=(long) n-1; i >= 0; i--)
      {
        c=(*name++);
        pixel.blue<<=4;
        if ((c >= '0') && (c <= '9'))
          pixel.blue|=(int) (c-'0');
        else
          if ((c >= 'A') && (c <= 'F'))
            pixel.blue|=(int) c-((int) 'A'-10);
          else
            if ((c >= 'a') && (c <= 'f'))
              pixel.blue|=(int) c-((int) 'a'-10);
            else
              return(MagickFalse);
      }
    } while (isxdigit((int) ((unsigned char) *name)) != MagickFalse);
  }
else
  if ((n != 4) && (n != 8) && (n != 16) && (n != 32))
    {
      (void) ThrowMagickException(exception,GetMagickModule(),
        OptionWarning,"UnrecognizedColor","`%s'",name);
      return(MagickFalse);
    }
  else
    {
      n/=4;
      do
      {
        pixel.red=pixel.green;
        pixel.green=pixel.blue;
        pixel.blue=pixel.opacity;
        pixel.opacity=0;
        for (i=(long) n-1; i >= 0; i--)
        {
          c=(*name++);
          pixel.opacity<<=4;
          if ((c >= '0') && (c <= '9'))
            pixel.opacity|=(int) (c-'0');
          else
            if ((c >= 'A') && (c <= 'F'))
              pixel.opacity|=(int) c-((int) 'A'-10);
            else
              if ((c >= 'a') && (c <= 'f'))
                pixel.opacity|=(int) c-((int) 'a'-10);
              else
                return(MagickFalse);
        }
      } while (isxdigit((int) ((unsigned char) *name)) != MagickFalse);
    }
n<<=2;
to this:

Code: Select all

if ((n == 3) || (n == 6) || (n == 9) || (n == 12) || (n == 24))
  {
    n/=3;
    do
    {
      pixel.red=pixel.green;
      pixel.green=pixel.blue;
      pixel.blue=0;
      for (i=(long) n-1; i >= 0; i--)
      {
        c=(*name++);
        pixel.blue<<=4;
        if ((c >= '0') && (c <= '9'))
          pixel.blue|=(int) (c-'0');
        else
          if ((c >= 'A') && (c <= 'F'))
            pixel.blue|=(int) c-((int) 'A'-10);
          else
            if ((c >= 'a') && (c <= 'f'))
              pixel.blue|=(int) c-((int) 'a'-10);
            else
              return(MagickFalse);
      }
    } while (isxdigit((int) ((unsigned char) *name)) != MagickFalse);
    n *= 3;
  }
else
  if ((n != 4) && (n != 8) && (n != 16) && (n != 32))
    {
      (void) ThrowMagickException(exception,GetMagickModule(),
        OptionWarning,"UnrecognizedColor","`%s'",name);
      return(MagickFalse);
    }
  else
    {
      n/=4;
      do
      {
        pixel.red=pixel.green;
        pixel.green=pixel.blue;
        pixel.blue=pixel.opacity;
        pixel.opacity=0;
        for (i=(long) n-1; i >= 0; i--)
        {
          c=(*name++);
          pixel.opacity<<=4;
          if ((c >= '0') && (c <= '9'))
            pixel.opacity|=(int) (c-'0');
          else
            if ((c >= 'A') && (c <= 'F'))
              pixel.opacity|=(int) c-((int) 'A'-10);
            else
              if ((c >= 'a') && (c <= 'f'))
                pixel.opacity|=(int) c-((int) 'a'-10);
              else
                return(MagickFalse);
        }
      } while (isxdigit((int) ((unsigned char) *name)) != MagickFalse);
      n *= 4;
    }
so that each clause fixes up n (note that the change removes "n<<2;" from the end).

Pete
User avatar
magick
Site Admin
Posts: 11064
Joined: 2003-05-31T11:32:55-07:00

Post by magick »

We will get your patch into ImageMagick within the next few days. Thanks for the problem report and patch.
Post Reply