Re: MagickSetImageType and gm convert: inconsistent behaviour

On Thu, 23 Sep 2010, Graham Leggett wrote:
>
> Is a grayscale image supposed to be in the RGB colourspace, and am I
> misunderstanding this?

Grayscale is a form of RGB colorspace (at least as far as 
GraphicsMagick is currently concerned).

> It turns out the reason the gm tool's "convert" function "works" is
> because the convert function cheats and hardcodes the ImageType direct
> into a private structure within the image:
>
>         if (LocaleCompare("type",option+1) == 0)
>           {
>             resource_info.image_info->type=UndefinedType;
>             if (*option == '-')
>               {
>                 i++;
>                 if (i == argc)
>
> MagickFatalError(OptionFatalError,MissingArgument,option);
>                 option=argv[i];
>                 resource_info.image_info-
> >type=StringToImageType(option);
>                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>                 if (resource_info.image_info->type == UndefinedType)
>
> MagickFatalError(OptionFatalError,UnrecognizedImageType,
>                                    option);
>               }
>             break;
>           }
>         MagickFatalError(OptionFatalError,UnrecognizedOption,option);
>
> I am now confused - which behaviour is correct, the SetImageType()
> function that does all the colourspace conversion, or the "convert"
> function that hardcodes the ImageType value into the private structure
> directly?

It is easy to see why the behavior is confusing. 
MagickSetImageType() only operates on the image (Image struct).  The 
command line tool is also setting 'type' in ImageInfo, which is used 
when a file is saved.  MagickSetImageType() is not doing that. 
Perpaps this should be added to MagickSetImageType():

   wand->image_info->type=image_type;

So that the function looks like:

WandExport unsigned int MagickSetImageType(MagickWand *wand,
   const ImageType image_type)
{
   assert(wand != (MagickWand *) NULL);
   assert(wand->signature == MagickSignature);
   if (wand->images == (Image *) NULL)
     ThrowWandException(WandError,WandContainsNoImages,wand->id);
   wand->image_info->type=image_type;
   return(SetImageType(wand->image,image_type));
}

The drawback of this is that the Image type operates immediately on 
the pixels (if required) so that it is an image processing operation 
whereas the type stored in ImageInfo is only used when saving the 
image.  These are two somewhat different functions which may or may 
not always end up with the same result.  For example, there could be a 
request to convert the image to a gray image, and then something with 
the color red is composited on top of it.  The image is then a RGB 
image but would still be saved as gray (if possible) due to also 
setting ImageInfo to the desired type.  The user would need to 
remember to set the image back to an RGB color type.

> Why would the SetImageType() function think that a grayscale image is
> already in the RGB colourspace (unless it I am misunderstanding it)?

I think that part of the logic is ok.  What is missing is to assign 
the desired 'type' in ImageInfo so that it is also applied when the 
image is saved.  This is an easy fix, but I am not 100% sure it is the 
right solution.

Bob
-- 
Bob Friesenhahn
[email protected], http://www.simplesystems.org/users/bfriesen/
GraphicsMagick Maintainer,    http://www.GraphicsMagick.org/

------------------------------------------------------------------------------
Start uncovering the many advantages of virtual appliances
and start using them to simplify application deployment and
accelerate your shift to cloud computing.
http://p.sf.net/sfu/novell-sfdev2dev