[C] Texture management / pointer question

Question

I'm working on a texture management and animation solution for a small side project of mine. Although the project uses Allegro for rendering and input, my question mostly revolves around C and memory management. I wanted to post it here to get thoughts and insight into the approach, as I'm terrible when it comes to pointers.

Essentially what I'm trying to do is load all of my texture resources into a central manager (textureManager) - which is essentially an array of structs containing ALLEGRO_BITMAP objects. The textures stored within the textureManager are mostly full sprite sheets.

From there, I have an anim(ation) struct, which contains animation-specific information (along with a pointer to the corresponding texture within the textureManager).

To give you an idea, here's how I setup and play the players 'walk' animation:

createAnimation(&player.animations[0], "media/characters/player/walk.png", player.w, player.h);
playAnimation(&player.animations[0], 10);

Rendering the animations current frame is just a case of blitting a specific region of the sprite sheet stored in textureManager.

For reference, here's the code for anim.h and anim.c. I'm sure what I'm doing here is probably a terrible approach for a number of reasons. I'd like to hear about them! Am I opening myself to any pitfalls? Will this work as I'm hoping?

anim.h

#ifndef ANIM_H
#define ANIM_H

#define ANIM_MAX_FRAMES 10
#define MAX_TEXTURES 50

struct texture {
    bool active;
    ALLEGRO_BITMAP *bmp;
};
struct texture textureManager[MAX_TEXTURES];

typedef struct tAnim {
    ALLEGRO_BITMAP **sprite;
    int w, h;
    int curFrame, numFrames, frameCount;
    float delay;
} anim;

void setupTextureManager(void);
int addTexture(char *filename);

int createAnimation(anim *a, char *filename, int w, int h);
void playAnimation(anim *a, float delay);
void updateAnimation(anim *a);

#endif

anim.c

void setupTextureManager() {
    int i = 0;
    for(i = 0; i < MAX_TEXTURES; i++) {
        textureManager[i].active = false;
    }
}
int addTextureToManager(char *filename) {
    int i = 0;
    for(i = 0; i < MAX_TEXTURES; i++) {
        if(!textureManager[i].active) {
            textureManager[i].bmp = al_load_bitmap(filename);
            textureManager[i].active = true;
            if(!textureManager[i].bmp) {
                printf("Error loading texture: %s", filename);
                return -1;
            }
            return i;
        }
    }

    return -1;
}

int createAnimation(anim *a, char *filename, int w, int h) {
    int textureId = addTextureToManager(filename);

    if(textureId > -1) {
        a->sprite = textureManager[textureId].bmp;
        a->w = w;
        a->h = h;       
        a->numFrames = al_get_bitmap_width(a->sprite) / w;

        printf("Animation loaded with %i frames, given resource id: %i\n", a->numFrames, textureId);
    } else {
        printf("Texture manager full\n");
        return 1;
    }

    return 0;
}
void playAnimation(anim *a, float delay) {
    a->curFrame = 0;
    a->frameCount = 0;
    a->delay = delay;
}
void updateAnimation(anim *a) {
    a->frameCount ++;
    if(a->frameCount >= a->delay) {
        a->frameCount = 0;
        a->curFrame ++;
        if(a->curFrame >= a->numFrames) {
            a->curFrame = 0;
        }
    }
}

Answer 1

Are you sure you need a pointer to pointer for ALLEGRO_BITMAP **sprite; in anim?

IIRC Allegro BITMAP-handles are pointers already, so there is no need double-reference them, since you seem to only want to store one Bitmap per animation.

You ought to use ALLEGRO_BITMAP *sprite; in anim.

I do not see any other problems with your code.

Answer 2

You may want to consider a more flexible Animation structure that contains an array of Frame structures. Each frame structure could contain the frame delay, an x/y hotspot offset, etc. This way different frames of the same animation could be different sizes and delays. But if you don't need those features, then what you're doing is fine.

I assume you'll be running the logic at a fixed frame rate (constant # of logical frames per second)? If so, then the delay parameters should work out well.

A quick comment regarding your code:

textureManager[i].active = true;

You probably shouldn't mark it as active until after you've checked if the bitmap loaded.

Also note that Allegro 4.9/5.0 is fully backed by OpenGL or D3D textures and, as such, large bitmaps will fail to load on some video cards! This could be a problem if you are generating large sprite sheets. As of the current version, you have to work around it yourself.

You could do something like:

al_set_new_bitmap_flags(ALLEGRO_MEMORY_BITMAP);
ALLEGRO_BITMAP *sprite_sheet = al_load_bitmap("sprites.png");
al_set_new_bitmap_flags(0);
if (!sprite_sheet) return -1; // error

// loop over sprite sheet, creating new video bitmaps for each frame
for (i = 0; i < num_sprites; ++i)
{
  animation.frame[i].bmp = al_create_bitmap( ... );
  al_set_target_bitmap(animation.frame[i].bmp);
  al_draw_bitmap_region( sprite_sheet, ... );
}

al_destroy_bitmap(sprite_sheet);

al_set_target_bitmap(al_get_backbuffer());

To be clear: this is a video card limitation. So a large sprite sheet may work on your computer but fail to load on another. The above approach loads the sprite sheet into a memory bitmap (essentially guaranteed to succeed) and then creates a new, smaller hardware accelerated video bitmap per frame.