Fix hash map collisions when setting canvases.

src/modules/graphics/Graphics.cpp

@@ -628,6 +628,7 @@ void Graphics::setCanvas(const RenderTargets &rts)
 	bool hasSRGBcanvas = firstcolorformat == PIXELFORMAT_sRGBA8;
 	int pixelw = firstcanvas->getPixelWidth(firsttarget.mipmap);
 	int pixelh = firstcanvas->getPixelHeight(firsttarget.mipmap);
+	int reqmsaa = firstcanvas->getRequestedMSAA();
 
 	for (int i = 1; i < ncanvases; i++)
 	{
@@ -648,7 +649,7 @@ void Graphics::setCanvas(const RenderTargets &rts)
 		if (!multiformatsupported && format != firstcolorformat)
 			throw love::Exception("This system doesn't support multi-canvas rendering with different canvas formats.");
 
-		if (c->getRequestedMSAA() != firstcanvas->getRequestedMSAA())
+		if (c->getRequestedMSAA() != reqmsaa)
 			throw love::Exception("All Canvases must have the same MSAA value.");
 
 		if (isPixelFormatDepthStencil(format))
@@ -684,7 +685,33 @@ void Graphics::setCanvas(const RenderTargets &rts)
 	int h = firstcanvas->getHeight(firsttarget.mipmap);
 
 	flushStreamDraws();
-	setCanvasInternal(rts, w, h, pixelw, pixelh, hasSRGBcanvas);
+
+	if (rts.depthStencil.canvas == nullptr && rts.temporaryRTFlags != 0)
+	{
+		bool wantsdepth   = (rts.temporaryRTFlags & TEMPORARY_RT_DEPTH) != 0;
+		bool wantsstencil = (rts.temporaryRTFlags & TEMPORARY_RT_STENCIL) != 0;
+
+		PixelFormat dsformat = PIXELFORMAT_STENCIL8;
+		if (wantsdepth && wantsstencil)
+			dsformat = PIXELFORMAT_DEPTH24_STENCIL8;
+		else if (wantsdepth && isCanvasFormatSupported(PIXELFORMAT_DEPTH24, false))
+			dsformat = PIXELFORMAT_DEPTH24;
+		else if (wantsdepth)
+			dsformat = PIXELFORMAT_DEPTH16;
+		else if (wantsstencil)
+			dsformat = PIXELFORMAT_STENCIL8;
+
+		// We want setCanvasInternal to have a pointer to the temporary RT, but
+		// we don't want to directly store it in the main graphics state.
+		RenderTargets realRTs = rts;
+
+		realRTs.depthStencil.canvas = getTemporaryCanvas(dsformat, pixelw, pixelh, reqmsaa);
+		realRTs.depthStencil.slice = 0;
+
+		setCanvasInternal(realRTs, w, h, pixelw, pixelh, hasSRGBcanvas);
+	}
+	else
+		setCanvasInternal(rts, w, h, pixelw, pixelh, hasSRGBcanvas);
 
 	RenderTargetsStrongRef refs;
 	refs.colors.reserve(rts.colors.size());

src/modules/graphics/Graphics.h

@@ -422,6 +422,24 @@ public:
 		{
 			return colors.empty() ? depthStencil : colors[0];
 		}
+
+		bool operator == (const RenderTargets &other) const
+		{
+			size_t ncolors = colors.size();
+			if (ncolors != other.colors.size())
+				return false;
+
+			for (size_t i = 0; i < ncolors; i++)
+			{
+				if (colors[i] != other.colors[i])
+					return false;
+			}
+
+			if (depthStencil != other.depthStencil || temporaryRTFlags != other.temporaryRTFlags)
+				return false;
+
+			return true;
+		}
 	};
 
 	struct RenderTargetsStrongRef

src/modules/graphics/opengl/Canvas.cpp

@@ -20,6 +20,7 @@
 
 #include "Canvas.h"
 #include "graphics/Graphics.h"
+#include "Graphics.h"
 
 #include <algorithm> // For min/max
 
@@ -270,6 +271,12 @@ bool Canvas::loadVolatile()
 
 void Canvas::unloadVolatile()
 {
+	// This is a bit ugly, but we need some way to destroy the cached FBO
+	// when this Canvas' texture is destroyed.
+	auto gfx = Module::getInstance<Graphics>(Module::M_GRAPHICS);
+	if (gfx != nullptr)
+		gfx->cleanupCanvas(this);
+
 	if (fbo != 0)
 		gl.deleteFramebuffer(fbo);
 

src/modules/graphics/opengl/Graphics.cpp

@@ -779,21 +779,38 @@ void Graphics::discard(OpenGL::FramebufferTarget target, const std::vector<bool>
 		glDiscardFramebufferEXT(gltarget, (GLint) attachments.size(), &attachments[0]);
 }
 
-void Graphics::bindCachedFBO(const RenderTargets &targets)
+void Graphics::cleanupCanvas(Canvas *canvas)
 {
-	RenderTarget hashtargets[MAX_COLOR_RENDER_TARGETS + 1];
-	int hashcount = 0;
+	for (auto it = framebufferObjects.begin(); it != framebufferObjects.end(); /**/)
+	{
+		bool hascanvas = false;
+		const auto &rts = it->first;
+
+		for (const RenderTarget &rt : rts.colors)
+		{
+			if (rt.canvas == canvas)
+			{
+				hascanvas = true;
+				break;
+			}
+		}
 
-	for (int i = 0; i < (int) targets.colors.size(); i++)
-		hashtargets[hashcount++] = targets.colors[i];
+		hascanvas = hascanvas || rts.depthStencil.canvas == canvas;
 
-	if (targets.depthStencil.canvas != nullptr)
-		hashtargets[hashcount++] = targets.depthStencil;
-	else if (targets.temporaryRTFlags != 0)
-		hashtargets[hashcount++] = RenderTarget(nullptr, -1, targets.temporaryRTFlags);
+		if (hascanvas)
+		{
+			if (isCreated())
+				gl.deleteFramebuffer(it->second);
+			it = framebufferObjects.erase(it);
+		}
+		else
+			++it;
+	}
+}
 
-	uint32 hash = XXH32(hashtargets, sizeof(RenderTarget) * hashcount, 0);
-	GLuint fbo = framebufferObjects[hash];
+void Graphics::bindCachedFBO(const RenderTargets &targets)
+{
+	GLuint fbo = framebufferObjects[targets];
 
 	if (fbo != 0)
 	{
@@ -801,34 +818,7 @@ void Graphics::bindCachedFBO(const RenderTargets &targets)
 	}
 	else
 	{
-		RenderTarget firstRT = targets.getFirstTarget();
-
-		int mip = firstRT.mipmap;
-		int w = firstRT.canvas->getPixelWidth(mip);
-		int h = firstRT.canvas->getPixelHeight(mip);
-		int msaa = firstRT.canvas->getMSAA();
-		int reqmsaa = firstRT.canvas->getRequestedMSAA();
-
-		RenderTarget depthstencil = targets.depthStencil;
-
-		if (depthstencil.canvas == nullptr && targets.temporaryRTFlags != 0)
-		{
-			bool wantsdepth   = (targets.temporaryRTFlags & TEMPORARY_RT_DEPTH) != 0;
-			bool wantsstencil = (targets.temporaryRTFlags & TEMPORARY_RT_STENCIL) != 0;
-
-			PixelFormat dsformat = PIXELFORMAT_STENCIL8;
-			if (wantsdepth && wantsstencil)
-				dsformat = PIXELFORMAT_DEPTH24_STENCIL8;
-			else if (wantsdepth && isCanvasFormatSupported(PIXELFORMAT_DEPTH24, false))
-				dsformat = PIXELFORMAT_DEPTH24;
-			else if (wantsdepth)
-				dsformat = PIXELFORMAT_DEPTH16;
-			else if (wantsstencil)
-				dsformat = PIXELFORMAT_STENCIL8;
-
-			depthstencil.canvas = getTemporaryCanvas(dsformat, w, h, reqmsaa);
-			depthstencil.slice = 0;
-		}
+		int msaa = targets.getFirstTarget().canvas->getMSAA();
 
 		glGenFramebuffers(1, &fbo);
 		gl.bindFramebuffer(OpenGL::FRAMEBUFFER_ALL, fbo);
@@ -873,8 +863,8 @@ void Graphics::bindCachedFBO(const RenderTargets &targets)
 		for (const auto &rt : targets.colors)
 			attachCanvas(rt);
 
-		if (depthstencil.canvas != nullptr)
-			attachCanvas(depthstencil);
+		if (targets.depthStencil.canvas != nullptr)
+			attachCanvas(targets.depthStencil);
 
 		if (ncolortargets > 1)
 			glDrawBuffers(ncolortargets, drawbuffers);
@@ -887,8 +877,8 @@ void Graphics::bindCachedFBO(const RenderTargets &targets)
 			const char *sstr = OpenGL::framebufferStatusString(status);
 			throw love::Exception("Could not create Framebuffer Object! %s", sstr);
 		}
-		
-		framebufferObjects[hash] = fbo;
+
+		framebufferObjects[targets] = fbo;
 	}
 }
 

src/modules/graphics/opengl/Graphics.h

@@ -40,6 +40,8 @@
 #include "Canvas.h"
 #include "Shader.h"
 
+#include "libraries/xxHash/xxhash.h"
+
 namespace love
 {
 
@@ -110,8 +112,30 @@ public:
 
 	Shader::Language getShaderLanguageTarget() const override;
 
+	// Internal use.
+	void cleanupCanvas(Canvas *canvas);
+
 private:
 
+	struct CachedFBOHasher
+	{
+		size_t operator() (const RenderTargets &rts) const
+		{
+			RenderTarget hashtargets[MAX_COLOR_RENDER_TARGETS + 1];
+			int hashcount = 0;
+
+			for (size_t i = 0; i < rts.colors.size(); i++)
+				hashtargets[hashcount++] = rts.colors[i];
+
+			if (rts.depthStencil.canvas != nullptr)
+				hashtargets[hashcount++] = rts.depthStencil;
+			else if (rts.temporaryRTFlags != 0)
+				hashtargets[hashcount++] = RenderTarget(nullptr, -1, rts.temporaryRTFlags);
+
+			return XXH32(hashtargets, sizeof(RenderTarget) * hashcount, 0);
+		}
+	};
+
 	love::graphics::ShaderStage *newShaderStageInternal(ShaderStage::StageType stage, const std::string &cachekey, const std::string &source, bool gles) override;
 	love::graphics::Shader *newShaderInternal(love::graphics::ShaderStage *vertex, love::graphics::ShaderStage *pixel) override;
 	love::graphics::StreamBuffer *newStreamBuffer(BufferType type, size_t size) override;
@@ -125,7 +149,7 @@ private:
 
 	void setDebug(bool enable);
 
-	std::unordered_map<uint32, GLuint> framebufferObjects;
+	std::unordered_map<RenderTargets, GLuint, CachedFBOHasher> framebufferObjects;
 	bool windowHasStencil;
 	GLuint mainVAO;