SDL_image: bug, fix, and enhancement(s)

SDL Development
1

under MSVC on win32 (likely other platforms as well).
there is a bug with SDL detecting an image type.
all the images i load are with RW objects, so there
is no filename to help the ‘guess’. (perhaps this
is why the bug hasn’t cropped up to often??)

anyways, the “supported” array needs a specific
final NULL element in it. i noticed a comma after the
last format (png). i am guessing with gcc it will
automatically create a final all-zero element?
with MSVC there is definitely NO final NULL elements,
and the seaching routines tromp right past the end
of the list when a format isn’t found. here is the
fixed array

… supported[] = {
/* keep magicless formats first (denoted by is==NULL) */
{ “TGA”, NULL, IMG_LoadTGA_RW },
{ “BMP”, IMG_isBMP, IMG_LoadBMP_RW },
{ “PPM”, IMG_isPPM, IMG_LoadPPM_RW },
{ “PCX”, IMG_isPCX, IMG_LoadPCX_RW },
{ “GIF”, IMG_isGIF, IMG_LoadGIF_RW },
{ “JPG”, IMG_isJPG, IMG_LoadJPG_RW },
{ “TIF”, IMG_isTIF, IMG_LoadTIF_RW },
{ “PNG”, IMG_isPNG, IMG_LoadPNG_RW },
{ NULL }
};

note, at first i just had “{ }” as the trailing element
in the list, MSVC gave me an error on that one (strange?)
so the single NULL is required.

while i was at it i wrote a quick feature i’ve been wanting.
what better time to add it into SDL_image?

/* Detect the type of image from a given RW */
const char *IMG_GetType_RW(SDL_RWops *src)
{
int i, start;

/* Make sure there is something to do… */
if ( src == NULL ) {
return(NULL);
}

/* See whether or not this data source can handle seeking */
if ( SDL_RWseek(src, 0, SEEK_CUR) < 0 ) {
    IMG_SetError("Can't seek in this data source");
    return(NULL);
}

/* Detect the type of image being loaded */
start = SDL_RWtell(src);
for ( i=0; supported[i].type; ++i ) {
    if( (supported[i].is
        && (SDL_RWseek(src, start, SEEK_SET),
        supported[i].is(src)))) {

        SDL_RWseek(src, start, SEEK_SET);
        return supported[i].type;
    }
}
return(NULL);

}

some final notes…
this is merely a shameless trim version of IMG_LoadTyped_RW.
my only question about this code (and LoadTyped) is should
we rewind the RW object to ‘start’ on the failure cases?
i may be misreading the super-logic of the “if()” statement,
but it doesn’t look right to me.

last note. would it be worthwhile having a “is” function
for TGA? while there’s no magic bytes to check, it would
be easy enough to write a sanity checker. it would work
similar to LoadTGA, just returning 0 where LoadTGA calls
"unsupported()" and not actually loading any image data.
i just know that by only using RW objects i’ll never be
able to load a TGA unless i call IMG_LoadTyped_RW and
specifically tell it to use TGA. just a thought, i could
take care of that if there were interest.

2

anyways, the “supported” array needs a specific
final NULL element in it. i noticed a comma after the
last format (png). i am guessing with gcc it will
automatically create a final all-zero element?

No it won’t, it’s gcc convenience syntax. It’s a bug, thank you.
(I’ll replace it with an element count; I prefer that to a null terminator.)

while i was at it i wrote a quick feature i’ve been wanting.
what better time to add it into SDL_image?

const char *IMG_GetType_RW(SDL_RWops *src)

Well maybe we can add it. May I ask why you need it?

this is merely a shameless trim version of IMG_LoadTyped_RW.
my only question about this code (and LoadTyped) is should
we rewind the RW object to ‘start’ on the failure cases?

last note. would it be worthwhile having a “is” function
for TGA? while there’s no magic bytes to check, it would
be easy enough to write a sanity checker.

No, it wouldn’t be worthwhile. Any such predicate would be imperfect by
design. Either you know it’s a TGA, and use IMG_LoadTyped_RW, or you don’t
but have the file name and use IMG_Load. If you have no file name and
don’t know its type, you should not try to load it as a TGA.