Re: [AD] palette change for load_bitmap

[Elias Pschernig <elias@xxxxxxxxxx>]

On Tue, 2002-11-05 at 11:30, Eric Botcazou wrote:
> > Ok, in this version, there's no extra parameter.
> 
> Fine. But I think we should properly document the behaviour of 
> _fixup_loaded_bitmap in the source file, something like:
> 
> /* _fixup_loaded_bitmap:
>  *  Helper function for adjusting the color depth of a loaded image.
>  *  Converts the bitmap BMP to the color depth BPP. If BMP is a 8-bit
>  *  bitmap, PAL must be the palette attached to the bitmap. If BPP is
>  *  equal to 8, the conversion is performed either by building a palette
>  *  optimized for the bitmap if PAL is not NULL (in which case PAL gets
>  *  filled in with this palette) or by using the current palette if PAL
>  *  is NULL. In any other cases, PAL is unused.
>  */
> 
> And I've now realized that the patch changes the behaviour of load_bitmap(), 
> load_bmp(), load_pcx() and load_tga() when loading truecolor images, the 
> color depth is 8 and PAL is NULL. That's ok, since passing NULL as the PAL 
> argument was never documented. But it must now be documented.

Yes, that was the purpose. My reasoning was, the optimized_palette is
"useless", as it is never returned - and therefore I couldn't see why
anyone would use load_bitmap in this way. In the attached patch I try to
describe the new behavior.

> > > - can't we get rid of the 332 palette too ?
> >
> > It's removed now when no palette argument is given. The other case -
> > returning a palette although a true-color bitmap is returned - is
> > probably useless, but I see no reason to change this behavior now.
> 
> I agree with you, but it looks like your patch doesn't: when the destination 
> color depth is not the same as the source color depth, but both are higher 
> than 8-bit, the palette is unconditionally not set, isn't it ?
> 

Ah, true. I had pasted this into load_{bmp,tga,pcx}:

  if (dest_depth != bpp) {
     if (nopal && bpp != 8)
        pal = NULL;
     bmp = _fixup_loaded_bitmap(bmp, pal, dest_depth);
  }
  else if (!nopal && bpp != 8)
     generate_332_palette (pal);

No idea how I came up with that 'else' line :) Replacing it with:

  if (!nopal && bpp != 8 && dest_depth != 8)

should make it behave right.

--
Elias Pschernig

--- cvs/allegro/docs/src/allegro._tx	2002-10-31 18:38:46.000000000 +0100
+++ ./allegro/docs/src/allegro._tx	2002-11-05 12:22:51.000000000 +0100
@@ -3179,13 +3187,24 @@
 @@BITMAP *@load_bitmap(const char *filename, RGB *pal);
 @xref load_bmp, load_lbm, load_pcx, load_tga, destroy_bitmap, save_bitmap
 @xref register_bitmap_file_type, set_color_depth, set_color_conversion
+@xref generate_optimized_palette, generate_332_palette
    Loads a bitmap from a file, returning a pointer to a bitmap and storing 
    the palette data in the specified location, which should be an array of 
    256 RGB structures. You are responsible for destroying the bitmap when 
    you are finished with it. Returns NULL on error. At present this function 
    supports BMP, LBM, PCX, and TGA files, determining the type from the file 
-   extension. If the file contains a truecolor image, you must set the video 
-   mode or call set_color_conversion() before loading it.
+   extension.
+
+   If the file contains a truecolor image, you must set the video mode or
+   call set_color_conversion() before loading it. In this case, if the
+   destination color depth is 8bit, a palette will be generated by calling
+   generate_optimized_palette on the bitmap. Otherwise, the returned
+   palette will be generated with generate_332_palette.
+
+   The pal argument may be NULL. In this case, the palette data are not
+   returned. Additionally, if the file is a truecolor bitmap, but is loaded
+   into an 8bit destination depth, it will use the current palette instead
+   of generating an optimized one from the bitmap.
 
 @@BITMAP *@load_bmp(const char *filename, RGB *pal);
 @xref load_bitmap