Merge pull request #18169 from hrydgard/various-fixes-8

Better handling of shadergen failures, other minor things

Common/GPU/Vulkan/VulkanRenderManager.cpp

@@ -500,14 +500,20 @@ void VulkanRenderManager::CompileThreadFunc() {
 	}
 }
 
-void VulkanRenderManager::DrainCompileQueue() {
+void VulkanRenderManager::DrainAndBlockCompileQueue() {
 	std::unique_lock<std::mutex> lock(compileMutex_);
+	compileBlocked_ = true;
 	compileCond_.notify_all();
 	while (!compileQueue_.empty()) {
 		queueRunner_.WaitForCompileNotification();
 	}
 }
 
+void VulkanRenderManager::ReleaseCompileQueue() {
+	std::unique_lock<std::mutex> lock(compileMutex_);
+	compileBlocked_ = false;
+}
+
 void VulkanRenderManager::ThreadFunc() {
 	SetCurrentThreadName("RenderMan");
 	while (true) {
@@ -709,14 +715,12 @@ VkCommandBuffer VulkanRenderManager::GetInitCmd() {
 }
 
 VKRGraphicsPipeline *VulkanRenderManager::CreateGraphicsPipeline(VKRGraphicsPipelineDesc *desc, PipelineFlags pipelineFlags, uint32_t variantBitmask, VkSampleCountFlagBits sampleCount, bool cacheLoad, const char *tag) {
-	VKRGraphicsPipeline *pipeline = new VKRGraphicsPipeline(pipelineFlags, tag);
-
 	if (!desc->vertexShader || !desc->fragmentShader) {
 		ERROR_LOG(G3D, "Can't create graphics pipeline with missing vs/ps: %p %p", desc->vertexShader, desc->fragmentShader);
-		delete pipeline;
 		return nullptr;
 	}
 
+	VKRGraphicsPipeline *pipeline = new VKRGraphicsPipeline(pipelineFlags, tag);
 	pipeline->desc = desc;
 	pipeline->desc->AddRef();
 	if (curRenderStep_ && !cacheLoad) {
@@ -733,7 +737,11 @@ VKRGraphicsPipeline *VulkanRenderManager::CreateGraphicsPipeline(VKRGraphicsPipe
 			VKRRenderPassStoreAction::STORE, VKRRenderPassStoreAction::DONT_CARE, VKRRenderPassStoreAction::DONT_CARE,
 		};
 		VKRRenderPass *compatibleRenderPass = queueRunner_.GetRenderPass(key);
-		compileMutex_.lock();
+		std::lock_guard<std::mutex> lock(compileMutex_);
+		if (compileBlocked_) {
+			delete pipeline;
+			return nullptr;
+		}
 		bool needsCompile = false;
 		for (size_t i = 0; i < (size_t)RenderPassType::TYPE_COUNT; i++) {
 			if (!(variantBitmask & (1 << i)))
@@ -757,18 +765,19 @@ VKRGraphicsPipeline *VulkanRenderManager::CreateGraphicsPipeline(VKRGraphicsPipe
 		}
 		if (needsCompile)
 			compileCond_.notify_one();
-		compileMutex_.unlock();
 	}
 	return pipeline;
 }
 
 VKRComputePipeline *VulkanRenderManager::CreateComputePipeline(VKRComputePipelineDesc *desc) {
+	std::lock_guard<std::mutex> lock(compileMutex_);
+	if (compileBlocked_) {
+		return nullptr;
+	}
 	VKRComputePipeline *pipeline = new VKRComputePipeline();
 	pipeline->desc = desc;
-	compileMutex_.lock();
 	compileQueue_.push_back(CompileQueueEntry(pipeline));
 	compileCond_.notify_one();
-	compileMutex_.unlock();
 	return pipeline;
 }
 
@@ -814,7 +823,7 @@ void VulkanRenderManager::EndCurRenderStep() {
 	compileMutex_.lock();
 	bool needsCompile = false;
 	for (VKRGraphicsPipeline *pipeline : pipelinesToCheck_) {
-		if (!pipeline) {
+		if (!pipeline || compileBlocked_) {
 			// Not good, but let's try not to crash.
 			continue;
 		}

Common/GPU/Vulkan/VulkanRenderManager.h

@@ -455,7 +455,8 @@ class VulkanRenderManager {
 	}
 
 	void ResetStats();
-	void DrainCompileQueue();
+	void DrainAndBlockCompileQueue();
+	void ReleaseCompileQueue();
 
 private:
 	void EndCurRenderStep();
@@ -527,6 +528,7 @@ class VulkanRenderManager {
 	std::condition_variable compileCond_;
 	std::mutex compileMutex_;
 	std::vector<CompileQueueEntry> compileQueue_;
+	bool compileBlocked_ = false;
 
 	// Thread for measuring presentation delay.
 	std::thread presentWaitThread_;

Common/GPU/Vulkan/thin3d_vulkan.cpp

@@ -821,6 +821,7 @@ void VKTexture::UpdateInternal(VkCommandBuffer cmd, VulkanPushPool *pushBuffer,
 	int bpp = GetBpp(vulkanFormat);
 	int bytesPerPixel = bpp / 8;
 	TextureCopyBatch batch;
+	batch.reserve(numLevels);
 	for (i = 0; i < numLevels; i++) {
 		uint32_t offset;
 		VkBuffer buf;

GPU/Common/ShaderId.cpp

@@ -275,6 +275,21 @@ bool FragmentIdNeedsFramebufferRead(const FShaderID &id) {
 		(ReplaceBlendType)id.Bits(FS_BIT_REPLACE_BLEND, 3) == REPLACE_BLEND_READ_FRAMEBUFFER;
 }
 
+static GEBlendMode SanitizeBlendEq(GEBlendMode beq) {
+	switch (beq) {
+	case GE_BLENDMODE_MUL_AND_ADD:
+	case GE_BLENDMODE_MUL_AND_SUBTRACT:
+	case GE_BLENDMODE_MUL_AND_SUBTRACT_REVERSE:
+	case GE_BLENDMODE_MIN:
+	case GE_BLENDMODE_MAX:
+	case GE_BLENDMODE_ABSDIFF:
+		return beq;
+	default:
+		// Just return something that won't cause a shader gen failure.
+		return GE_BLENDMODE_MUL_AND_ADD;
+	}
+}
+
 // Here we must take all the bits of the gstate that determine what the fragment shader will
 // look like, and concatenate them together into an ID.
 void ComputeFragmentShaderID(FShaderID *id_out, const ComputedPipelineState &pipelineState, const Draw::Bugs &bugs) {
@@ -369,15 +384,15 @@ void ComputeFragmentShaderID(FShaderID *id_out, const ComputedPipelineState &pip
 			// 3 bits.
 			id.SetBits(FS_BIT_REPLACE_BLEND, 3, replaceBlend);
 			// 11 bits total.
-			id.SetBits(FS_BIT_BLENDEQ, 3, gstate.getBlendEq());
+			id.SetBits(FS_BIT_BLENDEQ, 3, SanitizeBlendEq(gstate.getBlendEq()));
 			id.SetBits(FS_BIT_BLENDFUNC_A, 4, gstate.getBlendFuncA());
 			id.SetBits(FS_BIT_BLENDFUNC_B, 4, gstate.getBlendFuncB());
 		}
 		id.SetBit(FS_BIT_FLATSHADE, doFlatShading);
 		id.SetBit(FS_BIT_COLOR_WRITEMASK, colorWriteMask);
 
 		// All framebuffers are array textures in Vulkan now.
-		if (gstate_c.textureIsArray && g_Config.iGPUBackend == (int)GPUBackend::VULKAN) {
+		if (gstate_c.textureIsArray && gstate_c.Use(GPU_USE_FRAMEBUFFER_ARRAYS)) {
 			id.SetBit(FS_BIT_SAMPLE_ARRAY_TEXTURE);
 		}
 

GPU/GLES/ShaderManagerGLES.cpp

@@ -748,6 +748,8 @@ void ShaderManagerGLES::DirtyLastShader() {
 	lastVShaderSame_ = false;
 }
 
+// Can only fail by failing to generate the code (bad FSID).
+// Any actual failures driver-side happens later in the render manager.
 Shader *ShaderManagerGLES::CompileFragmentShader(FShaderID FSID) {
 	uint64_t uniformMask;
 	std::string errorString;
@@ -762,6 +764,8 @@ Shader *ShaderManagerGLES::CompileFragmentShader(FShaderID FSID) {
 	return new Shader(render_, codeBuffer_, desc, params);
 }
 
+// Can only fail by failing to generate the code (bad VSID).
+// Any actual failures driver-side happens later in the render manager.
 Shader *ShaderManagerGLES::CompileVertexShader(VShaderID VSID) {
 	bool useHWTransform = VSID.Bit(VS_BIT_USE_HW_TRANSFORM);
 	uint32_t attrMask;
@@ -802,13 +806,12 @@ Shader *ShaderManagerGLES::ApplyVertexShader(bool useHWTransform, bool useHWTess
 
 	// Vertex shader not in cache. Let's compile it.
 	vs = CompileVertexShader(*VSID);
-	if (!vs || vs->Failed()) {
+	if (!vs) {
 		auto gr = GetI18NCategory(I18NCat::GRAPHICS);
 		ERROR_LOG(G3D, "Vertex shader generation failed, falling back to software transform");
 		if (!g_Config.bHideSlowWarnings) {
 			g_OSD.Show(OSDType::MESSAGE_ERROR, gr->T("hardware transform error - falling back to software"), 2.5f);
 		}
-		delete vs;
 
 		// TODO: Look for existing shader with the appropriate ID, use that instead of generating a new one - however, need to make sure
 		// that that shader ID is not used when computing the linked shader ID below, because then IDs won't match
@@ -1099,7 +1102,7 @@ bool ShaderManagerGLES::ContinuePrecompile(float sliceTime) {
 			}
 
 			Shader *vs = CompileVertexShader(id);
-			if (!vs || vs->Failed()) {
+			if (!vs) {
 				// Give up on using the cache, just bail. We can't safely create the fallback shaders here
 				// without trying to deduce the vertType from the VSID.
 				ERROR_LOG(G3D, "Failed to compile a vertex shader loading from cache. Skipping rest of shader cache.");
@@ -1121,7 +1124,16 @@ bool ShaderManagerGLES::ContinuePrecompile(float sliceTime) {
 
 		const FShaderID &id = pending.frag[i];
 		if (!fsCache_.ContainsKey(id)) {
-			fsCache_.Insert(id, CompileFragmentShader(id));
+			Shader *fs = CompileFragmentShader(id);
+			if (!fs) {
+				// Give up on using the cache - something went wrong.
+				// We'll still keep the shaders we generated so far around.
+				ERROR_LOG(G3D, "Failed to compile a fragment shader loading from cache. Skipping rest of shader cache.");
+				delete fs;
+				pending.Clear();
+				return false;
+			}
+			fsCache_.Insert(id, fs);
 		} else {
 			WARN_LOG(G3D, "Duplicate fragment shader found in GL shader cache, ignoring");
 		}

GPU/GLES/ShaderManagerGLES.h

@@ -141,7 +141,6 @@ class Shader {
 	~Shader();
 	GLRShader *shader;
 
-	bool Failed() const { return failed_; }
 	bool UseHWTransform() const { return useHWTransform_; }  // only relevant for vtx shaders
 
 	std::string GetShaderString(DebugShaderStringType type, ShaderID id) const;
@@ -152,7 +151,6 @@ class Shader {
 private:
 	GLRenderManager *render_;
 	std::string source_;
-	bool failed_ = false;
 	bool useHWTransform_;
 	bool isFragment_;
 	uint32_t attrMask_; // only used in vertex shaders

GPU/GPUState.h

@@ -491,7 +491,7 @@ enum {
 	GPU_USE_DEPTH_TEXTURE = FLAG_BIT(16),
 	GPU_USE_ACCURATE_DEPTH = FLAG_BIT(17),
 	GPU_USE_GS_CULLING = FLAG_BIT(18),  // Geometry shader
-	// Bit 19 free.
+	GPU_USE_FRAMEBUFFER_ARRAYS = FLAG_BIT(19),
 	GPU_USE_FRAMEBUFFER_FETCH = FLAG_BIT(20),
 	GPU_SCALE_DEPTH_FROM_24BIT_TO_16BIT = FLAG_BIT(21),
 	GPU_ROUND_FRAGMENT_DEPTH_TO_16BIT = FLAG_BIT(22),

GPU/Vulkan/DrawEngineVulkan.cpp

@@ -792,7 +792,7 @@ void DrawEngineVulkan::DoFlush() {
 
 			shaderManager_->GetShaders(prim, dec_, &vshader, &fshader, &gshader, pipelineState_, true, useHWTessellation_, decOptions_.expandAllWeightsToFloat, decOptions_.applySkinInDecode);
 			if (!vshader) {
-				// We're screwed.
+				// We're screwed, can't do anything.
 				return;
 			}
 			_dbg_assert_msg_(vshader->UseHWTransform(), "Bad vshader");

GPU/Vulkan/GPU_Vulkan.cpp

@@ -182,7 +182,7 @@ void GPU_Vulkan::SaveCache(const Path &filename) {
 GPU_Vulkan::~GPU_Vulkan() {
 	if (draw_) {
 		VulkanRenderManager *rm = (VulkanRenderManager *)draw_->GetNativeObject(Draw::NativeObject::RENDER_MANAGER);
-		rm->DrainCompileQueue();
+		rm->DrainAndBlockCompileQueue();
 	}
 
 	SaveCache(shaderCachePath_);
@@ -193,6 +193,11 @@ GPU_Vulkan::~GPU_Vulkan() {
 
 	delete pipelineManager_;
 	// other managers are deleted in ~GPUCommonHW.
+
+	if (draw_) {
+		VulkanRenderManager *rm = (VulkanRenderManager *)draw_->GetNativeObject(Draw::NativeObject::RENDER_MANAGER);
+		rm->ReleaseCompileQueue();
+	}
 }
 
 u32 GPU_Vulkan::CheckGPUFeatures() const {
@@ -290,6 +295,7 @@ u32 GPU_Vulkan::CheckGPUFeatures() const {
 		features &= ~GPU_USE_LIGHT_UBERSHADER;
 	}
 
+	features |= GPU_USE_FRAMEBUFFER_ARRAYS;
 	return CheckGPUFeaturesLate(features);
 }