Merge pull request #15848 from hrydgard/refcount-shader-modules

Refcount shader modules properly in thin3d

Common/GPU/D3D9/thin3d_d3d9.cpp

@@ -259,17 +259,23 @@ class D3D9Pipeline : public Pipeline {
 public:
 	D3D9Pipeline() {}
 	~D3D9Pipeline() {
+		if (vshader) {
+			vshader->Release();
+		}
+		if (pshader) {
+			pshader->Release();
+		}
 	}
 
-	D3D9ShaderModule *vshader;
-	D3D9ShaderModule *pshader;
+	D3D9ShaderModule *vshader = nullptr;
+	D3D9ShaderModule *pshader = nullptr;
 
-	D3DPRIMITIVETYPE prim;
+	D3DPRIMITIVETYPE prim{};
 	AutoRef<D3D9InputLayout> inputLayout;
 	AutoRef<D3D9DepthStencilState> depthStencil;
 	AutoRef<D3D9BlendState> blend;
 	AutoRef<D3D9RasterState> raster;
-	UniformBufferDesc dynamicUniforms;
+	UniformBufferDesc dynamicUniforms{};
 
 	void Apply(LPDIRECT3DDEVICE9 device, uint8_t stencilRef, uint8_t stencilWriteMask, uint8_t stencilCompareMask);
 };
@@ -713,9 +719,11 @@ Pipeline *D3D9Context::CreateGraphicsPipeline(const PipelineDesc &desc) {
 		}
 		if (iter->GetStage() == ShaderStage::Fragment) {
 			pipeline->pshader = static_cast<D3D9ShaderModule *>(iter);
+			pipeline->pshader->AddRef();
 		}
 		else if (iter->GetStage() == ShaderStage::Vertex) {
 			pipeline->vshader = static_cast<D3D9ShaderModule *>(iter);
+			pipeline->vshader->AddRef();
 		}
 	}
 	pipeline->prim = primToD3D9[(int)desc.prim];

Common/GPU/thin3d.cpp

@@ -66,10 +66,15 @@ bool DataFormatIsDepthStencil(DataFormat fmt) {
 	}
 }
 
+RefCountedObject::~RefCountedObject() {
+	_dbg_assert_(refcount_ == 0xDEDEDE);
+}
 
 bool RefCountedObject::Release() {
 	if (refcount_ > 0 && refcount_ < 10000) {
 		if (--refcount_ == 0) {
+			// Make it very obvious if we try to free this again.
+			refcount_ = 0xDEDEDE;
 			delete this;
 			return true;
 		}

Common/GPU/thin3d.h

@@ -341,7 +341,9 @@ class RefCountedObject {
 	RefCountedObject() {
 		refcount_ = 1;
 	}
-	virtual ~RefCountedObject() {}
+	RefCountedObject(const RefCountedObject &other) = delete;
+	RefCountedObject& operator=(RefCountedObject const&) = delete;
+	virtual ~RefCountedObject();
 
 	void AddRef() { refcount_++; }
 	bool Release();

GPU/Common/Draw2D.cpp

@@ -110,19 +110,20 @@ void FramebufferManagerCommon::DrawStrip2D(Draw::Texture *tex, Draw2DVertex *ver
 
 	if (!draw2DPipelineColor_) {
 		char *fsCode = new char[4000];
-		char *fsDepthCode = new char[4000];
 		GenerateDraw2DFs(fsCode, shaderLanguageDesc);
-		GenerateDraw2DDepthFs(fsDepthCode, shaderLanguageDesc);
 
-		draw2DFs_ = draw_->CreateShaderModule(ShaderStage::Fragment, shaderLanguageDesc.shaderLanguage, (const uint8_t *)fsCode, strlen(fsCode), "draw2d_fs");
+		ShaderModule *draw2DFs = draw_->CreateShaderModule(ShaderStage::Fragment, shaderLanguageDesc.shaderLanguage, (const uint8_t *)fsCode, strlen(fsCode), "draw2d_fs");
+		delete[] fsCode;
 
-		_assert_(draw2DFs_);
+		_assert_(draw2DFs);
 
+		ShaderModule *draw2DFsDepth = nullptr;
 		if (draw_->GetDeviceCaps().fragmentShaderDepthWriteSupported) {
-			draw2DFsDepth_ = draw_->CreateShaderModule(ShaderStage::Fragment, shaderLanguageDesc.shaderLanguage, (const uint8_t *)fsDepthCode, strlen(fsDepthCode), "draw2d_depth_fs");
-			_assert_(draw2DFsDepth_);
-		} else {
-			draw2DFsDepth_ = nullptr;
+			char *fsDepthCode = new char[4000];
+			GenerateDraw2DDepthFs(fsDepthCode, shaderLanguageDesc);
+			draw2DFsDepth = draw_->CreateShaderModule(ShaderStage::Fragment, shaderLanguageDesc.shaderLanguage, (const uint8_t *)fsDepthCode, strlen(fsDepthCode), "draw2d_depth_fs");
+			delete[] fsDepthCode;
+			_assert_(draw2DFsDepth);
 		}
 
 		// verts have positions in 2D clip coordinates.
@@ -151,15 +152,15 @@ void FramebufferManagerCommon::DrawStrip2D(Draw::Texture *tex, Draw2DVertex *ver
 
 		PipelineDesc draw2DColorPipelineDesc{
 			Primitive::TRIANGLE_STRIP,
-			{ draw2DVs_, draw2DFs_ },
+			{ draw2DVs_, draw2DFs },
 			inputLayout, noDepthStencil, blendOff, rasterNoCull, nullptr,
 		};
 
 		draw2DPipelineColor_ = draw_->CreateGraphicsPipeline(draw2DColorPipelineDesc);
 
 		PipelineDesc draw2DDepthPipelineDesc{
 			Primitive::TRIANGLE_STRIP,
-			{ draw2DVs_, draw2DFsDepth_ },
+			{ draw2DVs_, draw2DFsDepth },
 			inputLayout, depthWriteAlways, blendDiscard, rasterNoCull, nullptr,
 		};
 
@@ -170,7 +171,10 @@ void FramebufferManagerCommon::DrawStrip2D(Draw::Texture *tex, Draw2DVertex *ver
 			draw2DPipelineDepth_ = nullptr;
 		}
 
-		delete[] fsCode;
+		draw2DFs->Release();
+		if (draw2DFsDepth) {
+			draw2DFsDepth->Release();
+		}
 
 		rasterNoCull->Release();
 		blendOff->Release();

GPU/Common/FramebufferManagerCommon.cpp

@@ -2351,6 +2351,9 @@ static void DoRelease(T *&obj) {
 
 void FramebufferManagerCommon::DeviceLost() {
 	DestroyAllFBOs();
+
+	presentation_->DeviceLost();
+
 	for (int i = 0; i < 3; i++) {
 		for (int j = 0; j < 3; j++) {
 			DoRelease(reinterpretFromTo_[i][j]);
@@ -2359,18 +2362,14 @@ void FramebufferManagerCommon::DeviceLost() {
 	DoRelease(reinterpretVBuf_);
 	DoRelease(reinterpretSampler_);
 	DoRelease(reinterpretVS_);
-	DoRelease(stencilUploadFs_);
-	DoRelease(stencilUploadVs_);
 	DoRelease(stencilUploadSampler_);
 	DoRelease(stencilUploadPipeline_);
-	DoRelease(draw2DPipelineColor_);
-	DoRelease(draw2DPipelineDepth_);
 	DoRelease(draw2DSamplerNearest_);
 	DoRelease(draw2DSamplerLinear_);
 	DoRelease(draw2DVs_);
-	DoRelease(draw2DFs_);
-	DoRelease(draw2DFsDepth_);
-	presentation_->DeviceLost();
+	DoRelease(draw2DPipelineColor_);
+	DoRelease(draw2DPipelineDepth_);
+
 	draw_ = nullptr;
 }
 

GPU/Common/FramebufferManagerCommon.h

@@ -482,8 +482,6 @@ class FramebufferManagerCommon {
 	// Common implementation of stencil buffer upload. Also not 100% optimal, but not perforamnce
 	// critical either.
 	Draw::Pipeline *stencilUploadPipeline_ = nullptr;
-	Draw::ShaderModule *stencilUploadVs_ = nullptr;
-	Draw::ShaderModule *stencilUploadFs_ = nullptr;
 	Draw::SamplerState *stencilUploadSampler_ = nullptr;
 
 	// Draw2D pipelines
@@ -492,6 +490,5 @@ class FramebufferManagerCommon {
 	Draw::SamplerState *draw2DSamplerLinear_ = nullptr;
 	Draw::SamplerState *draw2DSamplerNearest_ = nullptr;
 	Draw::ShaderModule *draw2DVs_ = nullptr;
-	Draw::ShaderModule *draw2DFs_ = nullptr;
-	Draw::ShaderModule *draw2DFsDepth_ = nullptr;
+	// The fragment shaders are "owned" by the pipelines since they're 1:1.
 };

GPU/Common/ReinterpretFramebuffer.cpp

@@ -150,15 +150,13 @@ void FramebufferManagerCommon::ReinterpretFramebuffer(VirtualFramebuffer *vfb, G
 		return;
 	}
 
-	char *vsCode = nullptr;
-	char *fsCode = nullptr;
-
 	if (!reinterpretVS_) {
-		vsCode = new char[4000];
+		char *vsCode = new char[4000];
 		const ShaderLanguageDesc &shaderLanguageDesc = draw_->GetShaderLanguageDesc();
 		GenerateReinterpretVertexShader(vsCode, shaderLanguageDesc);
 		reinterpretVS_ = draw_->CreateShaderModule(ShaderStage::Vertex, shaderLanguageDesc.shaderLanguage, (const uint8_t *)vsCode, strlen(vsCode), "reinterpret_vs");
 		_assert_(reinterpretVS_);
+		delete[] vsCode;
 	}
 
 	if (!reinterpretSampler_) {
@@ -176,11 +174,12 @@ void FramebufferManagerCommon::ReinterpretFramebuffer(VirtualFramebuffer *vfb, G
 
 	Draw::Pipeline *pipeline = reinterpretFromTo_[(int)oldFormat][(int)newFormat];
 	if (!pipeline) {
-		fsCode = new char[4000];
+		char *fsCode = new char[4000];
 		const ShaderLanguageDesc &shaderLanguageDesc = draw_->GetShaderLanguageDesc();
 		GenerateReinterpretFragmentShader(fsCode, oldFormat, newFormat, shaderLanguageDesc);
 		Draw::ShaderModule *reinterpretFS = draw_->CreateShaderModule(ShaderStage::Fragment, shaderLanguageDesc.shaderLanguage, (const uint8_t *)fsCode, strlen(fsCode), "reinterpret_fs");
 		_assert_(reinterpretFS);
+		delete[] fsCode;
 
 		std::vector<Draw::ShaderModule *> shaders;
 		shaders.push_back(reinterpretVS_);
@@ -239,6 +238,4 @@ void FramebufferManagerCommon::ReinterpretFramebuffer(VirtualFramebuffer *vfb, G
 		// In case ReinterpretFramebuffer was called from the texture manager.
 		draw_->BindFramebufferAsRenderTarget(currentRenderVfb_->fbo, { Draw::RPAction::KEEP, Draw::RPAction::KEEP, Draw::RPAction::KEEP }, "After reinterpret");
 	}
-	delete[] vsCode;
-	delete[] fsCode;
 }

GPU/Common/StencilCommon.cpp

@@ -207,10 +207,10 @@ bool FramebufferManagerCommon::PerformStencilUpload(u32 addr, int size, StencilU
 		GenerateStencilFs(fsCode, shaderLanguageDesc, draw_->GetBugs());
 		GenerateStencilVs(vsCode, shaderLanguageDesc);
 
-		stencilUploadFs_ = draw_->CreateShaderModule(ShaderStage::Fragment, shaderLanguageDesc.shaderLanguage, (const uint8_t *)fsCode, strlen(fsCode), "stencil_fs");
-		stencilUploadVs_ = draw_->CreateShaderModule(ShaderStage::Vertex, shaderLanguageDesc.shaderLanguage, (const uint8_t *)vsCode, strlen(vsCode), "stencil_vs");
+		ShaderModule *stencilUploadFs = draw_->CreateShaderModule(ShaderStage::Fragment, shaderLanguageDesc.shaderLanguage, (const uint8_t *)fsCode, strlen(fsCode), "stencil_fs");
+		ShaderModule *stencilUploadVs = draw_->CreateShaderModule(ShaderStage::Vertex, shaderLanguageDesc.shaderLanguage, (const uint8_t *)vsCode, strlen(vsCode), "stencil_vs");
 
-		_assert_(stencilUploadFs_ && stencilUploadVs_);
+		_assert_(stencilUploadFs && stencilUploadVs);
 
 		InputLayoutDesc desc = {
 			{
@@ -234,7 +234,7 @@ bool FramebufferManagerCommon::PerformStencilUpload(u32 addr, int size, StencilU
 
 		PipelineDesc stencilWriteDesc{
 			Primitive::TRIANGLE_LIST,
-			{ stencilUploadVs_, stencilUploadFs_ },
+			{ stencilUploadVs, stencilUploadFs },
 			inputLayout, stencilWrite, blendOff, rasterNoCull, &stencilUBDesc,
 		};
 		stencilUploadPipeline_ = draw_->CreateGraphicsPipeline(stencilWriteDesc);
@@ -248,6 +248,9 @@ bool FramebufferManagerCommon::PerformStencilUpload(u32 addr, int size, StencilU
 		stencilWrite->Release();
 		inputLayout->Release();
 
+		stencilUploadFs->Release();
+		stencilUploadVs->Release();
+
 		SamplerStateDesc descNearest{};
 		stencilUploadSampler_ = draw_->CreateSamplerState(descNearest);
 	}