[Glt-devel] GLT and .bmp

[Jacques G. <jac...@ho...>]

Response to discussion Re: bmp code

>Some issues with BMP code:
>- This is a C++ library, not C

I gathered as much, there should be no probs to fix that.

>- use new and delete, not malloc and free

As above

>- Granularity of bmp_C.c is too fine, too many funcs

Yeah, that is more a remnant of debugging.

>- Standard C++ headers, not ANSI C:

I agree.

>#include <stdlib.h>   -> <cstdlib>
>#include <stdio.h>    -> <cstdio>
>#include <string.h>   -> <cstring>

>- GLT has byte, int32, uint32 but no dword_t or byte_t

byte_t -> byte, word_t -> uint16, dword_t -> uint32 ??

>http://www.nigels.com/glt/doc/glt_2config_8h.html

>- No need to handle files, we deal with memory buffers
>- Endian routines already provided by misc/endian.h
>- All BMP related code should be in bmp.cpp (modularity)
>- Internal functions should be static to bmp.cpp scope
>- Generalised functions in image.h should be used where possible

The idea with the _P suffix is to imply that it is included only in the .cpp 
.
I like keeping the byte moving away from the structure of the file type, 
purely to make it easier to debug.
I suggest we keep them separate ( bmp.cpp and bmp_P.cpp ) until we are sure 
bmp_P.cpp is bug free, and then merge them. What do you think?

>- There should no need for any additional interface in image.h

Absolutely, that would break the API ! And that is a big no no.

/*!
     \brief      Decode image data from BMP
     \ingroup    Misc
     \param      width    Width of BMP
     \param      height   Height of BMP
     \param      image    Destination RGB buffer
     \param      data     Source data buffer
     \todo       Windows BMP import limited to RGB images
*/
bool decodeBMP(uint32 &width,uint32 &height,std::string &image,const 
std::string &data);

/*!
     \brief      Encode image data as RGB Windows BMP
     \ingroup    Misc
     \param      data     BMP output buffer
     \param      width    Image width
     \param      height   Image height
     \param      image    Input image buffer
     \todo       Not yet implemented
*/
bool encodeBMP(std::string &data,const uint32 width,const uint32 
height,const std::string &image);

>>   Anyway, I just commited four files to GLT. bmp_P.c/h and 
>>image_helper.c/h I just >>wanted you to check them out and see if you 
>>think the coding style and all the other >>stuff is along GLT lines.

>I think the style is still fairly far away, but we can get there over a 
>couple of iterations, if you're willing and able to migrate
>and/or extend the existing bmp.cpp interface and implementation.

Sure thing.

>>I would like to make sure the methods in image_helper follow the same 
>>method as >>you. Hope you don't mind the files being straight C? If 
>>everything is OK I will integrate >>the files with GLT.

>Convert to C++.  It's a C++ library - C++ is better than C in many ways.
>I have the patience to do tidy C++ abstractions, not the patience to deal 
>with (generally) messey C API's. :-)

Yepp.

>>   It took me a while to convert most of the functions to use buffers 
>>instead of files but >>I have been testing them and I can't find any 
>>bugs.... so far.

>Ok, but lets eliminate the FILE * based code. And keep in mind that all the 
>stuff in bmp_P.h either needs to be
>submerged in bmp.cpp or removed.

OK, I'll remove redundant code. I'll keep bmp_P.h submerged in bmp.cpp until 
we are satisfied with the quality of the code in bmp_P*, then we can just 
merge it in to the file.


Cheers, Jacques.

_________________________________________________________________
We've 100s of NEW questions! Play Millionaire online to win $$$$. Click here 
  http://sites.ninemsn.com.au/minisite/millionaire/default.asp