Page 1 of 1

QueryMagickColor bug

Posted: 2006-03-31T16:55:54-07:00
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

Posted: 2006-03-31T20:38:12-07:00
by magick
We will get your patch into ImageMagick within the next few days. Thanks for the problem report and patch.