Container of surface pointers

Started by
5 comments, last by Rombethor 16 years, 9 months ago
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, SCREEN );
                  }
                  SDL_Flip( SCREEN );
                  SDL_FillRect( SCREEN, &amp;SCREEN-&gt;clip_rect, SDL_MapRGB( SCREEN-&gt;format, <span class="cpp-number">0</span>, <span class="cpp-number">0</span>, <span class="cpp-number">0</span> ) );
             }
}

<span class="cpp-comment">//in the main program….</span>
main()
{
    Game* g();
    SDL_Surface* Jak = Load_Image(<span class="cpp-literal">"Jak"</span>); <span class="cpp-comment">//Load_Image works and just loads an image to the surface</span>
    <span class="cpp-keyword">int</span> done;
    done = <span class="cpp-number">0</span>;
    <span class="cpp-keyword">while</span>(!done)
    {
        g-&gt;Apply_Surface( <span class="cpp-number">0</span>, <span class="cpp-number">0</span>, Jak, <span class="cpp-number">3</span> );
        g-&gt;Flip();
    }
}

</pre></div><!–ENDSCRIPT–>

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?
Advertisement
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*/)// orGame 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.

C++: A Dialog | C++0x Features: Part1 (lambdas, auto, static_assert) , Part 2 (rvalue references) , Part 3 (decltype) | Write Games | Fix Your Timestep!

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. :)
Create-ivity - a game development blog Mouseover for more information.
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, screen );<br>                  }<br>                  SDL_Flip( screen );<br>                  SDL_FillRect( screen , NULL, SDL_MapRGB( screen-&gt;format, <span class="cpp-number">0</span>, <span class="cpp-number">0</span>, <span class="cpp-number">0</span> ) );<br>             }<br>};<br><br></pre></div><!–ENDSCRIPT–><br><br>You may need to include code to clear the layers too, depending &#111;n what you are doing.
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

I would love to change the world, but they won’t give me the source code.

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.
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?

This topic is closed to new replies.

Advertisement