Sign in to follow this  
Rombethor

Container of surface pointers

Recommended Posts

Ok, i'm using SDL in dev-cpp, and i'm trying to make a container class hold a list of SDL_Surface*, these will act as layers so i can apply images from different classes to the screen in the right order. But it's either giving me a MAV (memory access violation) or not showing the images. (in a nutshell:)
#include "stdafx.h"
#include "Image.h"

//this is my main game class that handles the screen
class Game{
      private:
             int layer_count;
             map<int, SDL_Surface*> Screen_Layers;
      public:
             Game();

             void startup(){
             /* includes startup stuff for SDL */
                 layer_count = 5;
                 for( int l = 0; l < layer_count; l++ )
                 {
                      Screen_Layers[l] = (SDL_Surface*) malloc(sizeof(SDL_Surface));
                 }
             }

             void Game::Apply_Surface(int x, int y, SDL_Surface* Source, int layer, SDL_Rect* clip )
             {
                 Surface_Apply( x, y, Source, Screen_Layers[layer], clip );
             }

             void Flip()
             {
                  for(unsigned int s = 0; s < Screen_Layers.size(); s++)
                  {
                      Surface_Apply( 0, 0, Screen_Layers[s], SCREEN );
                  }
                  SDL_Flip( SCREEN );
                  SDL_FillRect( SCREEN, &SCREEN->clip_rect, SDL_MapRGB( SCREEN->format, 0, 0, 0 ) );
             }
}

//in the main program....
main()
{
    Game* g();
    SDL_Surface* Jak = Load_Image("Jak"); //Load_Image works and just loads an image to the surface
    int done;
    done = 0;
    while(!done)
    {
        g->Apply_Surface( 0, 0, Jak, 3 );
        g->Flip();
    }
}

I cut the code short because there's lots of fiddly stuff and i've been playing around alot. But basically i think the problem is to do with the putting pointers in the map. Any idea please?

Share this post


Link to post
Share on other sites
Game* g();

This line doesn't mean what you think it means. What you think it does: create a variable g that is a pointer to a game object. What it really means: create a function decleration for g that takes no parameters and returns a pointer to a game object. The following two examples should be more correct:

Game * g = new Game;
g->ApplySurface(/*arguments*/)
// or
Game g;
g.ApplySurface(/*arguments*/)


Someone feel free to correct me if I'm wrong

edit: I also hope you actually initialize SDL somewhere in your full code.

Share this post


Link to post
Share on other sites
Notice the difference between:

SDL_Surface* Jak = Load_Image("Jak");

and:

Screen_Layers[l] = (SDL_Surface*) malloc(sizeof(SDL_Surface));


Why do you use 'malloc', instead of Load_Image? Why do you use 'malloc' at all, instead of the C++ operator 'new'? 'malloc' only allocates memory, it does not initialize a valid object at that place. You'll have to do that yourself. The 'new' operator does initialize an object, as it calls the constructor of that object after allocating memory for it. So no casting is needed, you immediatly get a pointer to the type you needed. Load_Image probably already does that under the hood, I think.

Also, and that has nothing to do with your problem, you don't free the memory you allocated. Use 'free' if you use 'malloc', use 'delete' if you use 'new'. Otherwise, you'll end up with memory leaks.

Anyway, I don't know about dev-cpp's debugging capabilities, but it surely pays off to learn how to use your debugger. :)

Share this post


Link to post
Share on other sites
SDL_Surfaces are complex objects and should not be malloced into existance. Use SDL_CreateRGBSurface() [other function include RGBASurface and SurfaceFrom]. instead.

All caps identifiers are usually reserved for preprocessor defines or macros. Don't make a global screen surface, SDL has one already for you, SDL_GetVideoSurface().

std::map is the wrong data structure for this. Instead have an array (or a std::vector) of SDL_Surfaces:


class Game{
private:
const int layer_count = 5;
SDL_Surface* Screen_Layers[layer_count];
public:
// dont have a seperate startup() method
Game() {
for( int l = 0; l < layer_count; l++ )
{
Screen_Layers[l] = SDL_CreateRGBSurface(/*look at SDL docs for params*/ );
}
}

void Game::Apply_Surface(int x, int y, SDL_Surface* Source, int layer, SDL_Rect* clip )
{
assert(layer < layer_count);
Surface_Apply( x, y, Source, Screen_Layers[layer], clip );
}

void Flip()
{
SDL_Surface *screen = SDL_GetVideoSurface();
for(unsigned int s = 0; s < layer_count ; s++)
{
Surface_Apply( 0, 0, Screen_Layers[s], screen );
}
SDL_Flip( screen );
SDL_FillRect( screen , NULL, SDL_MapRGB( screen->format, 0, 0, 0 ) );
}
};



You may need to include code to clear the layers too, depending on what you are doing.

Share this post


Link to post
Share on other sites
Nah rip-off I don't agree, yes you right map is worse for this but array of sdl_surfaces is worse too. The only options as you already said is using std::vector

Share this post


Link to post
Share on other sites
SDL_Surface* be ugly-fugly C-compatible non-C++ API! SDL_Surface* deserving of RAII wrapper magic!

A simple (untested, but globally correct) wrapper, using boost::shared_ptr:

#ifndef INCLUDE_SDL_SURFACE_WRAPPER_HPP
#define INCLUDE_SDL_SURFACE_WRAPPER_HPP

#include <exception>
#include <string>
#include <cassert>
#include <boost/shared_ptr.hpp>
#include <sdl.h>

namespace SDL
{
class Surface
{
// Store an SDL_Surface internally using a shared pointer.
// The SDL_Surface automatically is destroyed when the
// last SDL::Surface that references it is destroyed.
boost::shared_ptr<SDL_Surface> surf;

// Prevent creation of surfaces without underlying SDL_Surface,
// except in controlled environments
Surface() {}

// Set up the pointer and its destruction function, which
// is called when the object is not referenced anymore
Surface (SDL_Surface *s) : surf(s,SDL_FreeSurface) { assert(s); }

// A function which does nothing. Useful for "destroying" the
// video surface.
static void nil(SDL_Surface *) {}

public:

// Be able to use an SDL::Surface as if it were an SDL_Surface *
operator SDL_Surface *() const { return &*surf; }
SDL_Surface *operator ->() const { return &*surf; }

// Create a surface from a file
static Surface LoadBMP(const std::string & s)
{
SDL_Surface *loaded = SDL_LoadBMP(s.c_str());
if (loaded) return Surface(loaded);
throw std::exception(
(std::string("SDL: Could not load file ") + s).c_str());
}

// Creates a surface from dimensions
static Surface CreateRGB(
unsigned flags, int width, int height, int depth,
unsigned Rmask, unsigned Gmask, unsigned Bmask, unsigned Amask,
void *pixels = 0)
{
SDL_Surface *created =
pixels
? SDL_CreateRGBSurface( flags, width, height, depth,
Rmask, Gmask, Bmask, Amask)
: SDL_CreateRGBSurfaceFrom( pixels,
flags, width, height, depth,
Rmask, Gmask, Bmask, Amask);

if (created) return Surface(created);
throw std::exception("Could not create surface");
}

// Get the currentvideo surface
static Surface GetVideoSurface()
{
SDL_Surface *video = SDL_GetVideoSurface();
if (!video)
throw std::exception("There is no video surface");

Surface s; // Use a special destruction object
s.surf.reset(s,Surface::nil);
return s;
}
};
}

#endif




Then, you can std::map<int,SDL::Surface> safely and without additional bookkeeping.

Share this post


Link to post
Share on other sites
Thanks for the replies so far! By the way, i've got Game* g = new Game but forgot to put it in the post! I'm sorry about that. I think i worded the original post badly...
The SCREEN surface = SDL_SetVideoMode();
Well when If i replace the malloc with SDL_CreateRGBSurface() it only shows the image when it is applied to the last surface in the container. I was originally using std::vector but i changed to see if it would be any better... I guess i'll change back :p But why can i only use the last SDL_Surface in the vector? Any help please?

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this