From 65aab17dba4cf996af4a794999e9ed9ffa36a34e Mon Sep 17 00:00:00 2001
From: Wayward Heart <91356680+WaywardHeart@users.noreply.github.com>
Date: Sun, 19 May 2024 03:10:01 -0500
Subject: [PATCH] Release resources if we error in the Bitmap constructor.

---
 src/display/bitmap.cpp | 76 +++++++++++++++++++++++++++++-------------
 1 file changed, 52 insertions(+), 24 deletions(-)

diff --git a/src/display/bitmap.cpp b/src/display/bitmap.cpp
index 67bab50..bec7a2e 100644
--- a/src/display/bitmap.cpp
+++ b/src/display/bitmap.cpp
@@ -509,28 +509,36 @@ Bitmap::Bitmap(const char *filename)
     }
 
     BitmapOpenHandler handler;
-    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
-        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",
-                        filename, SDL_GetError());
+    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
+            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",
+                            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)
-                        shState->texPool().release(frame);
+                    releaseResources();
                     
                     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)
-                    shState->texPool().release(frame);
+                releaseResources();
                 
                 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)
-                    shState->texPool().release(f);
+                releaseResources();
                 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;
         }