Guard against deleted bitmaps

Ruby is not guaranteed to delete bitmaps after any windows, sprites, etc that they are attached to. In the event that the attached object is not deleted before the next call to Graphics.update, this will most likely result in a segfault, as isDisposed() is not guaranteed to return true for a deleted bitmap.

Bitmap::invalid was added in an attempt to guard against this for sprites, but since the bitmap in question is deleted it's not guaranteed that the pointer actually points to null, which made the fix unreliable.

src/display/bitmap.cpp

@@ -2523,24 +2523,6 @@ int Bitmap::maxSize(){
     return glState.caps.maxTexSize;
 }
 
-// This might look ridiculous, but apparently, it is possible
-// to encounter seemingly empty bitmaps during Graphics::update,
-// or specifically, during a Sprite's prepare function.
-
-// I have no idea why it happens, but it seems like just skipping
-// them makes it okay, so... that's what this function is for, at
-// least unless the actual source of the problem gets found, at
-// which point I'd get rid of it.
-
-// I get it to happen by trying to beat the first rival fight in
-// Pokemon Flux, on macOS. I don't think I've seen anyone bring up
-// something like this happening anywhere else, so... I dunno.
-// If a game suddenly explodes during Graphics.update, maybe try
-// breakpointing this?
-bool Bitmap::invalid() const {
-    return p == 0;
-}
-
 void Bitmap::assumeRubyGC()
 {
     p->assumingRubyGC = true;

src/display/bitmap.h

@@ -166,8 +166,6 @@ public:
 	sigslot::signal<> modified;
 
 	static int maxSize();
-    
-    bool invalid() const;
 
     void assumeRubyGC();
 

src/display/plane.cpp

@@ -46,6 +46,8 @@ struct PlanePrivate
 {
 	Bitmap *bitmap;
 
+	sigslot::connection bitmapDispCon;
+
 	NormValue opacity;
 	BlendType blendType;
 	Color *color;
@@ -83,6 +85,14 @@ struct PlanePrivate
 	~PlanePrivate()
 	{
 		prepareCon.disconnect();
+		
+		bitmapDisposal();
+	}
+
+	void bitmapDisposal()
+	{
+		bitmap = 0;
+		bitmapDispCon.disconnect();
 	}
 
 	void updateQuadSource()
@@ -138,6 +148,9 @@ struct PlanePrivate
 
 	void prepare()
 	{
+		if (nullOrDisposed(bitmap))
+			return;
+		
 		if (quadSourceDirty)
 		{
 			updateQuadSource();
@@ -176,8 +189,15 @@ void Plane::setBitmap(Bitmap *value)
 
 	p->bitmap = value;
 
-	if (!value)
+	p->bitmapDispCon.disconnect();
+
+	if (nullOrDisposed(value))
+	{
+		p->bitmap = 0;
 		return;
+	}
+
+	p->bitmapDispCon = value->wasDisposed.connect(&PlanePrivate::bitmapDisposal, p);
 
 	value->ensureNonMega();
 }

src/display/sprite.cpp

@@ -48,6 +48,8 @@ struct SpritePrivate
 {
     Bitmap *bitmap;
     
+    sigslot::connection bitmapDispCon;
+    
     Quad quad;
     Transform trans;
     
@@ -137,8 +139,16 @@ struct SpritePrivate
     {
         srcRectCon.disconnect();
         prepareCon.disconnect();
+        
+        bitmapDisposal();
     }
     
+    void bitmapDisposal()
+    {
+        bitmap = 0;
+        bitmapDispCon.disconnect();
+    }
+
     void recomputeBushDepth()
     {
         if (nullOrDisposed(bitmap))
@@ -189,9 +199,6 @@ struct SpritePrivate
         if (nullOrDisposed(bitmap))
             return;
         
-        if (bitmap->invalid())
-            return;
-        
         if (!opacity)
             return;
         
@@ -372,8 +379,15 @@ void Sprite::setBitmap(Bitmap *bitmap)
     
     p->bitmap = bitmap;
     
+    p->bitmapDispCon.disconnect();
+    
     if (nullOrDisposed(bitmap))
+    {
+        p->bitmap = 0;
         return;
+    }
+    
+    p->bitmapDispCon = bitmap->wasDisposed.connect(&SpritePrivate::bitmapDisposal, p);
     
     bitmap->ensureNonMega();
     

src/display/tilemap.cpp

@@ -331,6 +331,8 @@ struct TilemapPrivate
 	/* Dispose watches */
 	sigslot::connection autotilesDispCon[autotileCount];
 
+	sigslot::connection tilesetDispCon;
+
 	/* Draw prepare call */
 	sigslot::connection prepareCon;
 
@@ -403,6 +405,7 @@ struct TilemapPrivate
 
 		/* Disconnect signal handlers */
 		tilesetCon.disconnect();
+		tilesetDispCon.disconnect();
 		for (int i = 0; i < autotileCount; ++i)
 		{
 			autotilesCon[i].disconnect();
@@ -490,6 +493,20 @@ struct TilemapPrivate
 		atlasDirty = true;
 	}
 
+	void atlasContentsDisposal(int i)
+	{
+		// Guard against deleted bitmaps
+		autotiles[i] = 0;
+		
+		invalidateAtlasContents();
+	}
+
+	void tilesetDisposal()
+	{
+		tileset = 0;
+		tilesetDispCon.disconnect();
+	}
+
 	void invalidateBuffers()
 	{
 		buffersDirty = true;
@@ -1189,12 +1206,18 @@ void Tilemap::Autotiles::set(int i, Bitmap *bitmap)
 	p->invalidateAtlasContents();
 
 	p->autotilesCon[i].disconnect();
+	p->autotilesDispCon[i].disconnect();
+
+	if (nullOrDisposed(bitmap))
+	{
+		p->autotiles[i] = 0;
+		return;
+	}
+
 	p->autotilesCon[i] = bitmap->modified.connect
 	        (&TilemapPrivate::invalidateAtlasContents, p);
 
-	p->autotilesDispCon[i].disconnect();
+	p->autotilesDispCon[i] = bitmap->wasDisposed.connect( [i, this] { p->atlasContentsDisposal(i); } );
-	p->autotilesDispCon[i] = bitmap->wasDisposed.connect
-	        (&TilemapPrivate::invalidateAtlasContents, p);
 
 	p->updateAutotileInfo();
 }
@@ -1269,14 +1292,23 @@ void Tilemap::setTileset(Bitmap *value)
 
 	p->tileset = value;
 
-	if (!value)
+	p->tilesetDispCon.disconnect();
+	p->tilesetCon.disconnect();
+
+	if (nullOrDisposed(value))
+	{
+		p->tileset = 0;
 		return;
+	}
 
 	p->invalidateAtlasSize();
-	p->tilesetCon.disconnect();
+
 	p->tilesetCon = value->modified.connect
 	        (&TilemapPrivate::invalidateAtlasSize, p);
 
+	p->tilesetDispCon = value->wasDisposed.connect
+	        (&TilemapPrivate::tilesetDisposal, p);
+
 	p->updateAtlasInfo();
 }
 

src/display/tilemapvx.cpp

@@ -182,6 +182,14 @@ struct TilemapVXPrivate : public ViewportElement, TileAtlasVX::Reader
 		atlasDirty = true;
 	}
 
+	void atlasDisposal(int i)
+	{
+		// Guard against deleted bitmaps
+		bitmaps[i] = 0;
+		
+		invalidateAtlas();
+	}
+
 	void invalidateBuffers()
 	{
 		buffersDirty = true;
@@ -415,12 +423,18 @@ void TilemapVX::BitmapArray::set(int i, Bitmap *bitmap)
 	p->atlasDirty = true;
 
 	p->bmChangedCons[i].disconnect();
+	p->bmDisposedCons[i].disconnect();
+
+	if (nullOrDisposed(bitmap))
+	{
+		p->bitmaps[i] = 0;
+		return;
+	}
+
 	p->bmChangedCons[i] = bitmap->modified.connect
         (&TilemapVXPrivate::invalidateAtlas, p);
 
-	p->bmDisposedCons[i].disconnect();
+	p->bmDisposedCons[i] = bitmap->wasDisposed.connect( [i, this] { p->atlasDisposal(i); } );
-	p->bmDisposedCons[i] = bitmap->wasDisposed.connect
-		(&TilemapVXPrivate::invalidateAtlas, p);
 }
 
 Bitmap *TilemapVX::BitmapArray::get(int i) const

src/display/window.cpp

@@ -172,6 +172,9 @@ struct WindowPrivate
 
 	Bitmap *contents;
 
+	sigslot::connection windowskinDispCon;
+	sigslot::connection contentsDispCon;
+
 	bool bgStretch;
 	Rect *cursorRect;
 	bool active;
@@ -282,6 +285,21 @@ struct WindowPrivate
 		shState->texPool().release(baseTex);
 		cursorRectCon.disconnect();
 		prepareCon.disconnect();
+
+		windowskinDisposal();
+		contentsDisposal();
+	}
+
+	void windowskinDisposal()
+	{
+		windowskin = 0;
+		windowskinDispCon.disconnect();
+	}
+
+	void contentsDisposal()
+	{
+		contents = 0;
+		contentsDispCon.disconnect();
 	}
 
 	void markControlVertDirty()
@@ -720,10 +738,17 @@ void Window::setWindowskin(Bitmap *value)
 
 	p->windowskin = value;
 
+	p->windowskinDispCon.disconnect();
+
 	if (nullOrDisposed(value))
+	{
+		p->windowskin = 0;
 		return;
+	}
 
 	value->ensureNonMega();
+	
+	p->windowskinDispCon = value->wasDisposed.connect(&WindowPrivate::windowskinDisposal, p);
 }
 
 void Window::setContents(Bitmap *value)
@@ -736,10 +761,18 @@ void Window::setContents(Bitmap *value)
 	p->contents = value;
 	p->controlsVertDirty = true;
 
+	p->contentsDispCon.disconnect();
+
 	if (nullOrDisposed(value))
+	{
+		p->contents = 0;
 		return;
+	}
+
+	p->contentsDispCon = value->wasDisposed.connect(&WindowPrivate::contentsDisposal, p);
 
 	value->ensureNonMega();
+
 	p->contentsQuad.setTexPosRect(value->rect(), value->rect());
 }
 

src/display/windowvx.cpp

@@ -153,6 +153,9 @@ struct WindowVXPrivate
 	Bitmap *windowskin;
 
 	Bitmap *contents;
+	
+	sigslot::connection windowskinDispCon;
+	sigslot::connection contentsDispCon;
 
 	Rect *cursorRect;
 	bool active;
@@ -278,6 +281,21 @@ struct WindowVXPrivate
 		cursorRectCon.disconnect();
 		toneCon.disconnect();
 		prepareCon.disconnect();
+
+		windowskinDisposal();
+		contentsDisposal();
+	}
+
+	void windowskinDisposal()
+	{
+		windowskin = 0;
+		windowskinDispCon.disconnect();
+	}
+
+	void contentsDisposal()
+	{
+		contents = 0;
+		contentsDispCon.disconnect();
 	}
 
 	void invalidateCursorVert()
@@ -908,6 +926,16 @@ void WindowVX::setWindowskin(Bitmap *value)
 
 	p->windowskin = value;
 	p->base.texDirty = true;
+
+	p->windowskinDispCon.disconnect();
+
+	if (nullOrDisposed(value))
+	{
+		p->windowskin = 0;
+		return;
+	}
+
+	p->windowskinDispCon = value->wasDisposed.connect(&WindowVXPrivate::windowskinDisposal, p);
 }
 
 void WindowVX::setContents(Bitmap *value)
@@ -919,8 +947,15 @@ void WindowVX::setContents(Bitmap *value)
 
 	p->contents = value;
 
+	p->contentsDispCon.disconnect();
+
 	if (nullOrDisposed(value))
+	{
+		p->contents = 0;
 		return;
+	}
+
+	p->contentsDispCon = value->wasDisposed.connect(&WindowVXPrivate::contentsDisposal, p);
 
 	FloatRect rect = p->contents->rect();
 	p->contentsQuad.setTexPosRect(rect, rect);