A problem about al_destory

Quanwei Yuan

I put a bitmap pointer into this class

#SelectExpand
  1class skyGUIEditBox : public skyGUIObject{
  2public:
skyGUIEditBox(int _id,int _w,int _h,float _x,float _y,ALLEGRO_FONT * _fnt,ALLEGRO_DISPLAY* _dispaly);
~skyGUIEditBox();
void                SetText(const wchar_t* _text);
const wchar_t*        GetText(); 
  7
void                Focus(bool bFocused);
 void Render();
 void Update(double dt,ALLEGRO_EVENT * _event=0);
 11private:
void      OnKey( UINT nKey, UINT nRepCnt, UINT nFlags );
void      OnChar( UINT nChar, UINT nRepCnt, UINT nFlags );
void      InsertChar(wchar_t aChar);
void      DeleteChar(TDP aPos);
void      ClearCookie();

 18private:
int m_wdith;
int m_height;
ALLEGRO_FONT *m_fnt;
ALLEGRO_USTR  *m_str;
ALLEGRO_BITMAP *m_bg;   //I put a bitmap pointer into this class
wchar_t m_textbuff[1024];
UINT m_charpos;
bool m_posI;
bool m_gposI;
static LRESULT CALLBACK SkyEditWndProc(HWND hWnd, UINT nMsg, WPARAM wParam, LPARAM lParam);
 29};

Then, in the constructor to initialize it

#SelectExpand
  1skyGUIEditBox::skyGUIEditBox(int _id,int _w,int _h,float _x,float _y,ALLEGRO_FONT * _fnt,ALLEGRO_DISPLAY* _dispaly)
  2{
  3
id=_id;
  5
bStatic=false;
m_wdith=_w;
m_height=_h;
bVisible=true;
bEnabled=true;
rect.Set(_x, _y, _x+_w, _y+_h);
m_fnt=_fnt;
m_charpos=0;
m_gposI=false;
HWND hWnd = al_get_win_window_handle(_dispaly);
 16
if (g_LastWndProc == NULL)
{
  g_LastWndProc = (WNDPROC)::GetWindowLong(hWnd,GWL_WNDPROC);
  if(g_LastWndProc != (WNDPROC)::SetWindowLong(hWnd,GWL_WNDPROC,(LONG)SkyEditWndProc))throw NULL;
}
ClearCookie();
m_str=al_ustr_new_from_utf16((uint16_t*)m_textbuff);
 24///////////////////////Here/////////////////////////////////////
m_bg=0;
m_bg=al_create_bitmap(_w,_h);
if(!m_bg)throw NULL;
 28////////////////////////////////////////////////////////////////
 29}

use it in following function

#SelectExpand
  1void skyGUIEditBox::Render(){

al_set_target_bitmap(m_bg);
al_clear_to_color(al_map_rgba(0,0,0,0));
al_draw_ustr(m_fnt,color,1,1,0,m_str);
al_draw_rectangle(0,0,m_wdith,m_height,color,1.0f);
if(m_posI && m_gposI){
  int f_height=al_get_font_line_height(m_fnt)-4;
  int t_width =al_get_ustr_width(m_fnt,m_str);
  int x1=t_width+1;
  int y1=2;
  int x2=x1;
  int y2=y1+f_height;
  al_draw_line(x1,y1,x2,y2,color,1.0f);
}
ALLEGRO_DISPLAY *dis=al_get_current_display();
al_set_target_bitmap(al_get_backbuffer(dis));
al_draw_bitmap(m_bg,rect.x1,rect.y1,0);
 19}

but,when i destory it in destructor function,

skyGUIEditBox::~skyGUIEditBox(){
  if(m_bg){
  al_destroy_bitmap(m_bg);
  }
  ClearCookie();
  m_fnt=0;  
}

The following error occurred:

"Unhandled exception: 0xC0000005: read location 0x00000000 when an access violation occurs in the 0x645c4b16 at 地图编辑器.exe "

fallenlight12

That's easy. The reason this is happening is because it's destroying the bitmap after allegro has killed itself. At least that's what I think is happening. Something is making those bitmap pointers invalid.

I had this happen when I was first starting out too. My solution evolved to "KillAllegro()" and to careful destruction of the bitmaps so they precede the end of main().

Basically, do not destroy bitmaps in the destructor of a class. Ideally, create all your images through the same routine and store a list of pointers to them. When the application terminates, destroy them one by one.

Something (hacked together some code for you):

#SelectExpand
  1#include <list>
  2
  3typedef std::list<ALLEGRO_BITMAP*> t_imglist;
  4
  5class myimages {
  6public:
  7  myimages(){}
  8  ~myimages(){}
  9
 10  ALLEGRO_BITMAP * loadbitmap(const char *path);
 11  void killbitmaps();
 12private:
 13  std::list<ALLEGRO_BITMAP*> m_img;
 14}
 15
 16ALLEGRO_BITMAP * myimages::loadbitmap(const char *path)
 17{
 18  ALLEGRO_BITMAP * pbm=NULL;
 19  if( !(pbm = al_load_bitmap(path)) ) { 
 20    // if you have log file then log it
 21    return NULL;
 22  }
 23  // add the bitmap pointer to the list
 24  m_img.push_back(pbm);
 25  
 26  return pbm;
 27}
 28
 29void myimages::killbitmaps()
 30{
 31  t_imglist::iterator pend = m_img.end();
 32  for(t_imglist::iterator p1=m_img.begin(); p1 != pend; ++p1) {
 33    al_destroy_bitmap(*p1);
 34  }
 35  
 36  m_img.clear();
 37}

If I read what you wrote wrong, I'm sorry. I have to go to bed now. .

Kris Asick

I find it's generally not a good idea to put stuff related to Allegro into pointer destructors, since if Allegro shuts down before such a struct/class is destroyed then any calls to most Allegro functions will cause crashes.

When it comes to bitmaps, what I like to do is load all of my bitmaps into global pointer arrays, then simply copy those pointers into my class/struct objects. When I shut down my program I simply destroy the bitmaps in their global arrays before closing out Allegro, thus meaning I can just completely ignore the outdated pointers in my class/struct objects.

If you absolutely must load your bitmaps directly into your objects, (which is still a bad idea for other reasons, such as being a waste of memory if multiple objects load up identical bitmaps, or being difficult to reload if display conditions change), then what you can do is make sure all such objects are declared explicitly:

skyGUIEditBox *SGUIEB = new skyGUIEditBox;

Then, BEFORE you shut down Allegro at the end of your program:

delete SGUIEB;

Ideally though, you should load your bitmaps separate from your objects into a global array and simply copy the existing bitmap pointers into your objects. It'll save you a lot of trouble in the long run.

Quanwei Yuan

Thank you fo your answer.

Joachim Arting

I had the same problem recently.
Instead of destroying the bitmaps in a destructor, I simply made a function which I called right before return 0; in main.

Something like fallelight12 wrote, but simpler:

#SelectExpand
  1class CData
  2{
  3private:
  4    ALLEGRO_BITMAP* image[IMG_AMT];
  5    ...
  6    
  7public:
  8    ...
  9    bool loadImages();
 10    void destroyImages();
 11};
 12
 13bool CData::loadImages()
 14{
 15    image[0] = al_load_bitmap( "data\\gfx\\image1.png" );
 16    ...
 17}
 18
 19void CData::destroyImages()
 20{
 21    for ( int i = 0; i < IMG_AMT; i++ )
 22    {
 23        al_destroy_bitmap( image[i] );
 24    }
 25}
 26
 27int main()
 28{
 29    initialize etc.
 30    data.loadImages();
 31
 32    game loop
 33    ...
 34
 35    exit game loop
 36    
 37    data.destroyImages();
 38    return 0;
 39}

Not a perfect example, but you get the idea

Itachihro

I think it would be best to wrap Allegro in a C++ class to make it work better with automatic destruction. You could use unique_ptr, but I think a real wrapper may work better.

Thread #611616. Printed from Allegro.cc