Skip to content

Commit

Permalink
Merge pull request Ancurio#191 from WaywardHeart/bitmap-memory-leaks
Browse files Browse the repository at this point in the history 
Fix a few memory leaks involving Bitmaps - ONE MAJOR
  • Loading branch information
@Splendide-Imaginarius
Splendide-Imaginarius authored Jul 2, 2024
2 parents 8fcd6dd + 9221cc7 commit 67b9853
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 37 deletions.
9 changes: 8 additions & 1 deletion binding/bitmap-binding.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,22 @@ 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);

// Leave property as default nil if hasHires() is false.
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) {
Expand Down
6 changes: 0 additions & 6 deletions binding/disposable-binding.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
86 changes: 59 additions & 27 deletions src/display/bitmap.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -542,6 +550,10 @@ Bitmap::Bitmap(const char *filename)
delete handler.gif;
delete handler.gif_data;

delete p;
if (hiresBitmap)
delete hiresBitmap;

throw e;
}

Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}

Expand All @@ -770,10 +791,15 @@ Bitmap::Bitmap(TEXFBO &other)
}

p = new BitmapPrivate(this);
p->selfHires = hiresBitmap;

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;
}

p->selfHires = hiresBitmap;
if (p->selfHires != nullptr) {
p->gl.selfHires = &p->selfHires->getGLTypes();
}
Expand Down Expand Up @@ -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;
}
Expand All @@ -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());
Expand Down Expand Up @@ -2192,10 +2222,12 @@ void Bitmap::setFont(Font &value)
void Bitmap::setInitFont(Font *value)
{
if (hasHires()) {
Font *hiresFont = new Font(*value);
// Disable the illegal font size check when creating a high-res font.
hiresFont->setSize(hiresFont->getSize() * p->selfHires->width() / width(), false);
p->selfHires->setInitFont(hiresFont);
Font *hiresFont = p->selfHires->p->font;
if (hiresFont && hiresFont != &shState->defaultFont())
{
// Disable the illegal font size check when creating a high-res font.
hiresFont->setSize(hiresFont->getSize() * p->selfHires->width() / width(), false);
}
}

p->font = value;
Expand Down
13 changes: 10 additions & 3 deletions src/util/disposable.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,16 @@ class Disposable
if (disposed)
return;

releaseResources();
disposed = true;
wasDisposed();
GFX_LOCK;
try {
releaseResources();
disposed = true;
wasDisposed();
} catch (Exception &e) {
GFX_UNLOCK;
throw e;
}
GFX_UNLOCK;
}

bool isDisposed() const
Expand Down

0 comments on commit 67b9853

Please sign in to comment.