Merge pull request #191 from WaywardHeart/bitmap-memory-leaks

Fix a few memory leaks involving Bitmaps - ONE MAJOR

binding/bitmap-binding.cpp

@@ -46,7 +46,6 @@ void bitmapInitProps(Bitmap *b, VALUE self) {
     rb_obj_call_init(fontObj, 0, 0);
     
     Font *font = getPrivateData<Font>(fontObj);
-    b->setInitFont(font);
     
     rb_iv_set(self, "font", fontObj);
 
@@ -54,7 +53,15 @@ void bitmapInitProps(Bitmap *b, VALUE self) {
     if (b->hasHires()) {
         b->assumeRubyGC();
         wrapProperty(self, b->getHires(), "hires", BitmapType);
+        
+        VALUE hiresFontObj = rb_obj_alloc(fontKlass);
+        rb_obj_call_init(hiresFontObj, 0, 0);
+        Font *hiresFont = getPrivateData<Font>(hiresFontObj);
+        rb_iv_set(rb_iv_get(self, "hires"), "font", hiresFontObj);
+        b->getHires()->setInitFont(hiresFont);
+        
     }
+    b->setInitFont(font);
 }
 
 RB_METHOD(bitmapInitialize) {

binding/disposable-binding.h

@@ -36,13 +36,7 @@ RB_METHOD(disposableDispose)
 	if (!d)
 		return Qnil;
 
-	/* Nothing to do if already disposed */
-	if (d->isDisposed())
-		return Qnil;
-
-    GFX_LOCK;
 	d->dispose();
-    GFX_UNLOCK;
 
 	return Qnil;
 }

src/display/bitmap.cpp

@@ -509,28 +509,36 @@ Bitmap::Bitmap(const char *filename)
     }
 
     BitmapOpenHandler handler;
-    shState->fileSystem().openRead(handler, filename);
+    try {
-    
+        shState->fileSystem().openRead(handler, filename);
-    if (!handler.error.empty()) {
+        
-        // Not loaded with SDL, but I want it to be caught with the same exception type
+        if (!handler.error.empty()) {
-        throw Exception(Exception::SDLError, "Error loading image '%s': %s", filename, handler.error.c_str());
+            // Not loaded with SDL, but I want it to be caught with the same exception type
-    }
+            throw Exception(Exception::SDLError, "Error loading image '%s': %s", filename, handler.error.c_str());
-    else if (!handler.gif && !handler.surface) {
+        }
-        throw Exception(Exception::SDLError, "Error loading image '%s': %s",
+        else if (!handler.gif && !handler.surface) {
-                        filename, SDL_GetError());
+            throw Exception(Exception::SDLError, "Error loading image '%s': %s",
+                            filename, SDL_GetError());
+        }
+    } catch (const Exception &e) {
+        if (hiresBitmap)
+            delete hiresBitmap;
+        throw e;
     }
     
     if (handler.gif) {
-        p = new BitmapPrivate(this);
-
-        p->selfHires = hiresBitmap;
-        
         if (handler.gif->width >= (uint32_t)glState.caps.maxTexSize || handler.gif->height > (uint32_t)glState.caps.maxTexSize)
         {
+            if (hiresBitmap)
+                delete hiresBitmap;
             throw new Exception(Exception::MKXPError, "Animation too large (%ix%i, max %ix%i)",
                                 handler.gif->width, handler.gif->height, glState.caps.maxTexSize, glState.caps.maxTexSize);
         }
         
+        p = new BitmapPrivate(this);
+        
+        p->selfHires = hiresBitmap;
+        
         if (handler.gif->frame_count == 1) {
             TEXFBO texfbo;
             try {
@@ -542,6 +550,10 @@ Bitmap::Bitmap(const char *filename)
                 delete handler.gif;
                 delete handler.gif_data;
                 
+                delete p;
+                if (hiresBitmap)
+                    delete hiresBitmap;
+                
                 throw e;
             }
             
@@ -579,8 +591,7 @@ Bitmap::Bitmap(const char *filename)
             if (i > 0) {
                 int status = gif_decode_frame(handler.gif, i);
                 if (status != GIF_OK && status != GIF_WORKING) {
-                    for (TEXFBO &frame : p->animation.frames)
+                    releaseResources();
-                        shState->texPool().release(frame);
                     
                     gif_finalise(handler.gif);
                     delete handler.gif;
@@ -597,8 +608,7 @@ Bitmap::Bitmap(const char *filename)
             }
             catch (const Exception &e)
             {
-                for (TEXFBO &frame : p->animation.frames)
+                releaseResources();
-                    shState->texPool().release(frame);
                 
                 gif_finalise(handler.gif);
                 delete handler.gif;
@@ -640,7 +650,14 @@ Bitmap::Bitmap(int width, int height, bool isHires)
         hiresBitmap->setLores(this);
     }
 
-    TEXFBO tex = shState->texPool().request(width, height);
+    TEXFBO tex;
+    try {
+        tex = shState->texPool().request(width, height);
+    } catch (const Exception &e) {
+        if (hiresBitmap)
+            delete hiresBitmap;
+        throw e;
+    }
     
     p = new BitmapPrivate(this);
     p->gl = tex;
@@ -713,7 +730,12 @@ Bitmap::Bitmap(const Bitmap &other, int frame)
     
     // TODO: Clean me up
     if (!other.isAnimated() || frame >= -1) {
-        p->gl = shState->texPool().request(other.width(), other.height());
+        try {
+            p->gl = shState->texPool().request(other.width(), other.height());
+        } catch (const Exception &e) {
+            delete p;
+            throw e;
+        }
         
         GLMeta::blitBegin(p->gl);
         // Blit just the current frame of the other animated bitmap
@@ -742,8 +764,7 @@ Bitmap::Bitmap(const Bitmap &other, int frame)
             try {
                 newframe = shState->texPool().request(p->animation.width, p->animation.height);
             } catch(const Exception &e) {
-                for (TEXFBO &f : p->animation.frames)
+                releaseResources();
-                    shState->texPool().release(f);
                 throw e;
             }
             
@@ -770,10 +791,15 @@ Bitmap::Bitmap(TEXFBO &other)
     }
 
     p = new BitmapPrivate(this);
-
-    p->gl = shState->texPool().request(other.width, other.height);
-
     p->selfHires = hiresBitmap;
+
+    try {
+        p->gl = shState->texPool().request(other.width, other.height);
+    } catch (const Exception &e) {
+        delete p;
+        throw e;
+    }
+
     if (p->selfHires != nullptr) {
         p->gl.selfHires = &p->selfHires->getGLTypes();
     }
@@ -831,6 +857,8 @@ void Bitmap::initFromSurface(SDL_Surface *imgSurf, Bitmap *hiresBitmap, bool for
         }
         catch (const Exception &e)
         {
+            if (hiresBitmap)
+                delete hiresBitmap;
             SDL_FreeSurface(imgSurf);
             throw e;
         }
@@ -844,6 +872,8 @@ void Bitmap::initFromSurface(SDL_Surface *imgSurf, Bitmap *hiresBitmap, bool for
         
         TEX::bind(p->gl.tex);
         TEX::uploadImage(p->gl.width, p->gl.height, imgSurf->pixels, GL_RGBA);
+        
+        SDL_FreeSurface(imgSurf);
     }
     
     p->addTaintedArea(rect());
@@ -2192,10 +2222,12 @@ void Bitmap::setFont(Font &value)
 void Bitmap::setInitFont(Font *value)
 {
     if (hasHires()) {
-        Font *hiresFont = new Font(*value);
+        Font *hiresFont = p->selfHires->p->font;
-        // Disable the illegal font size check when creating a high-res font.
+        if (hiresFont && hiresFont != &shState->defaultFont())
-        hiresFont->setSize(hiresFont->getSize() * p->selfHires->width() / width(), false);
+        {
-        p->selfHires->setInitFont(hiresFont);
+            // Disable the illegal font size check when creating a high-res font.
+            hiresFont->setSize(hiresFont->getSize() * p->selfHires->width() / width(), false);
+        }
     }
 
     p->font = value;

src/util/disposable.h

@@ -50,9 +50,16 @@ public:
 		if (disposed)
 			return;
 
-		releaseResources();
+		GFX_LOCK;
-		disposed = true;
+		try {
-		wasDisposed();
+			releaseResources();
+			disposed = true;
+			wasDisposed();
+		} catch (Exception &e) {
+			GFX_UNLOCK;
+			throw e;
+		}
+		GFX_UNLOCK;
 	}
 
 	bool isDisposed() const