Vulkan simplification: in PipelineCacheKey, use shader IDs instead of pointers to shaders.

GPU/Vulkan/DrawEngineVulkan.cpp

@@ -301,12 +301,12 @@ void DrawEngineVulkan::Flush() {
 				ConvertStateToVulkanKey(*framebufferManager_, shaderManager_, prim, pipelineKey_, dynState_);
 			}
 
-			VulkanVertexShader *vshader = nullptr;
+			VShaderID vshaderID;
-			VulkanFragmentShader *fshader = nullptr;
+			FShaderID fshaderID;
 
-			shaderManager_->GetShaders(prim, dec_->VertexType(), &vshader, &fshader, pipelineState_, true, decOptions_.expandAllWeightsToFloat, applySkinInDecode_, clipInfoFlags_);
+			shaderManager_->GetShaderIDs(prim, dec_->VertexType(), &vshaderID, &fshaderID, pipelineState_, true, decOptions_.expandAllWeightsToFloat, applySkinInDecode_, clipInfoFlags_);
-			_dbg_assert_msg_(vshader->UseHWTransform(), "Bad vshader");
+			_dbg_assert_msg_(vshaderID.Bit(VS_BIT_USE_HW_TRANSFORM) == true, "Bad vshader ID");
-			VulkanPipeline *pipeline = pipelineManager_->GetOrCreatePipeline(renderManager, pipelineLayout_, pipelineKey_, &dec_->decFmt, vshader, fshader, true, 0, framebufferManager_->GetMSAALevel(), false);
+			VulkanPipeline *pipeline = pipelineManager_->GetOrCreatePipeline(renderManager, shaderManager_, pipelineLayout_, pipelineKey_, &dec_->decFmt, vshaderID, fshaderID, true, 0, framebufferManager_->GetMSAALevel(), false);
 			if (!pipeline || !pipeline->pipeline) {
 				// Already logged, let's bail out.
 				ResetAfterSkippedDraw();
@@ -459,12 +459,12 @@ void DrawEngineVulkan::Flush() {
 					ConvertStateToVulkanKey(*framebufferManager_, shaderManager_, prim, pipelineKey_, dynState_);
 				}
 
-				VulkanVertexShader *vshader = nullptr;
+				VShaderID vshaderID;
-				VulkanFragmentShader *fshader = nullptr;
+				FShaderID fshaderID;
 
-				shaderManager_->GetShaders(prim, swDec->VertexType(), &vshader, &fshader, pipelineState_, false, decOptions_.expandAllWeightsToFloat, true, clipInfoFlags_);
+				shaderManager_->GetShaderIDs(prim, swDec->VertexType(), &vshaderID, &fshaderID, pipelineState_, false, decOptions_.expandAllWeightsToFloat, true, clipInfoFlags_);
-				_dbg_assert_msg_(!vshader->UseHWTransform(), "Bad vshader");
+				_dbg_assert_msg_(vshaderID.Bit(VS_BIT_USE_HW_TRANSFORM) == false, "Bad vshader ID");
-				VulkanPipeline *pipeline = pipelineManager_->GetOrCreatePipeline(renderManager, pipelineLayout_, pipelineKey_, &swDec->decFmt, vshader, fshader, false, 0, framebufferManager_->GetMSAALevel(), false);
+				VulkanPipeline *pipeline = pipelineManager_->GetOrCreatePipeline(renderManager, shaderManager_, pipelineLayout_, pipelineKey_, &swDec->decFmt, vshaderID, fshaderID, false, 0, framebufferManager_->GetMSAALevel(), false);
 				if (!pipeline || !pipeline->pipeline) {
 					// Already logged, let's bail out.
 					ResetAfterSkippedDraw();

GPU/Vulkan/DrawEngineVulkan.h

@@ -25,7 +25,6 @@
 // * binding 3: Base Uniform Buffer (includes fragment state)
 // * binding 4: Light uniform buffer
 // * binding 5: Bone uniform buffer
-// * binding 6: Tess data storage buffer
 //
 // All shaders conform to this layout, so they are all compatible with the same descriptor set.
 // The format of the various uniform buffers may vary though - vertex shaders that don't skin

GPU/Vulkan/PipelineManagerVulkan.cpp

@@ -185,7 +185,7 @@ static std::string CutFromMain(const std::string &str) {
 
 static VulkanPipeline *CreateVulkanPipeline(VulkanRenderManager *renderManager, VkPipelineCache pipelineCache,
 	VKRPipelineLayout *layout, PipelineFlags pipelineFlags, VkSampleCountFlagBits sampleCount, const VulkanPipelineRasterStateKey &key,
-	const DecVtxFormat *decFmt, VulkanVertexShader *vs, VulkanFragmentShader *fs, bool useHwTransform, u32 variantBitmask, bool cacheLoad) {
+	const DecVtxFormat *decFmt, const VulkanVertexShader *vs, const VulkanFragmentShader *fs, bool useHwTransform, u32 variantBitmask, bool cacheLoad) {
 	_assert_(fs && vs);
 
 	if (!fs || !fs->GetModule()) {
@@ -342,7 +342,7 @@ static VulkanPipeline *CreateVulkanPipeline(VulkanRenderManager *renderManager,
 	return vulkanPipeline;
 }
 
-VulkanPipeline *PipelineManagerVulkan::GetOrCreatePipeline(VulkanRenderManager *renderManager, VKRPipelineLayout *layout, const VulkanPipelineRasterStateKey &rasterKey, const DecVtxFormat *decFmt, VulkanVertexShader *vs, VulkanFragmentShader *fs, bool useHwTransform, u32 variantBitmask, int multiSampleLevel, bool cacheLoad) {
+VulkanPipeline *PipelineManagerVulkan::GetOrCreatePipeline(VulkanRenderManager *renderManager, ShaderManagerVulkan *shaderManager, VKRPipelineLayout *layout, const VulkanPipelineRasterStateKey &rasterKey, const DecVtxFormat *decFmt, VShaderID vid, FShaderID fid, bool useHwTransform, u32 variantBitmask, int multiSampleLevel, bool cacheLoad) {
 	if (!pipelineCache_) {
 		VkPipelineCacheCreateInfo pc{ VK_STRUCTURE_TYPE_PIPELINE_CACHE_CREATE_INFO };
 		VkResult res = vkCreatePipelineCache(vulkan_->GetDevice(), &pc, nullptr, &pipelineCache_);
@@ -353,8 +353,8 @@ VulkanPipeline *PipelineManagerVulkan::GetOrCreatePipeline(VulkanRenderManager *
 
 	key.raster = rasterKey;
 	key.useHWTransform = useHwTransform;
-	key.vShader = vs->GetModule();
+	key.vid = vid;
-	key.fShader = fs->GetModule();
+	key.fid = fid;
 	key.vtxFmtId = useHwTransform ? decFmt->id : 0;
 
 	VulkanPipeline *pipeline;
@@ -362,6 +362,9 @@ VulkanPipeline *PipelineManagerVulkan::GetOrCreatePipeline(VulkanRenderManager *
 		return pipeline;
 	}
 
+	const VulkanFragmentShader *fs = shaderManager->GetFragmentShaderFromID(fid);
+	const VulkanVertexShader *vs = shaderManager->GetVertexShaderFromID(vid);
+
 	PipelineFlags pipelineFlags = (PipelineFlags)0;
 	if (fs->Flags() & FragmentShaderFlags::USES_DISCARD) {
 		pipelineFlags |= PipelineFlags::USES_DISCARD;
@@ -575,13 +578,9 @@ std::string VulkanPipelineKey::GetDescription(DebugShaderStringType stringType,
 
 	case SHADER_STRING_SOURCE_CODE:
 	{
-		// More detailed description of all the parts of the pipeline.
-		VkShaderModule fsModule = this->fShader->BlockUntilReady();
-		VkShaderModule vsModule = this->vShader->BlockUntilReady();
-
 		std::string str;
-		str += "VS: " + shaderManager->GetVertexShaderFromModule(vsModule)->GetID().Description() + "\n";
+		str += "VS: " + vid.Description() + "\n";
-		str += "FS: " + shaderManager->GetFragmentShaderFromModule(fsModule)->GetID().Description() + "\n";
+		str += "FS: " + fid.Description() + "\n";
 		str += GetRasterStateDesc(true);
 		return str;
 	}
@@ -653,8 +652,8 @@ void PipelineManagerVulkan::SavePipelineCache(FILE *file, bool saveRawPipelineCa
 	pipelines_.Iterate([&](const VulkanPipelineKey &pkey, VulkanPipeline *value) {
 		if (failed)
 			return;
-		VulkanVertexShader *vshader = shaderManager->GetVertexShaderFromModule(pkey.vShader->BlockUntilReady());
+		const VulkanVertexShader *vshader = shaderManager->GetVertexShaderFromID(pkey.vid);
-		VulkanFragmentShader *fshader = shaderManager->GetFragmentShaderFromModule(pkey.fShader->BlockUntilReady());
+		const VulkanFragmentShader *fshader = shaderManager->GetFragmentShaderFromID(pkey.fid);
 		if (!vshader || !fshader) {
 			failed = true;
 			return;
@@ -778,9 +777,6 @@ bool PipelineManagerVulkan::LoadPipelineCache(FILE *file, bool loadRawPipelineCa
 			continue;
 		}
 
-		VulkanVertexShader *vs = shaderManager->GetVertexShaderFromID(key.vShaderID);
-		VulkanFragmentShader *fs = shaderManager->GetFragmentShaderFromID(key.fShaderID);
-
 		// Avoid creating multisampled shaders if it's not enabled, as that results in an invalid combination.
 		// Note that variantsToBuild is NOT directly a RenderPassType! instead, it's a collection of (1 << RenderPassType).
 		u32 variantsToBuild = key.variants;
@@ -795,7 +791,7 @@ bool PipelineManagerVulkan::LoadPipelineCache(FILE *file, bool loadRawPipelineCa
 		DecVtxFormat fmt;
 		fmt.InitializeFromID(key.vtxFmtId);
 		VulkanPipeline *pipeline = GetOrCreatePipeline(
-			rm, layout, key.raster, key.useHWTransform ? &fmt : 0, vs, fs, key.useHWTransform, variantsToBuild, multiSampleLevel, true);
+			rm, shaderManager, layout, key.raster, key.useHWTransform ? &fmt : 0, key.vShaderID, key.fShaderID, key.useHWTransform, variantsToBuild, multiSampleLevel, true);
 		if (!pipeline) {
 			pipelineCreateFailCount += 1;
 		}

GPU/Vulkan/PipelineManagerVulkan.h

@@ -43,9 +43,8 @@ class DrawEngineCommon;
 
 struct VulkanPipelineKey {
 	VulkanPipelineRasterStateKey raster;  // prim is included here
-	VKRRenderPass *renderPass;
+	VShaderID vid;
-	Promise<VkShaderModule> *vShader;
+	FShaderID fid;
-	Promise<VkShaderModule> *fShader;
 	uint32_t vtxFmtId;
 	bool useHWTransform;
 
@@ -87,8 +86,8 @@ public:
 	~PipelineManagerVulkan();
 
 	// variantMask is only used when loading pipelines from cache.
-	VulkanPipeline *GetOrCreatePipeline(VulkanRenderManager *renderManager, VKRPipelineLayout *layout, const VulkanPipelineRasterStateKey &rasterKey, const DecVtxFormat *decFmt,
+	VulkanPipeline *GetOrCreatePipeline(VulkanRenderManager *renderManager, ShaderManagerVulkan *shaderManager, VKRPipelineLayout *layout, const VulkanPipelineRasterStateKey &rasterKey, const DecVtxFormat *decFmt,
-		VulkanVertexShader *vs, VulkanFragmentShader *fs, bool useHwTransform, u32 variantMask, int multiSampleLevel, bool cacheLoad);
+		VShaderID vid, FShaderID fid, bool useHwTransform, u32 variantMask, int multiSampleLevel, bool cacheLoad);
 	int GetNumPipelines() const { return (int)pipelines_.size(); }
 
 	void Clear();

GPU/Vulkan/ShaderManagerVulkan.cpp

@@ -208,8 +208,6 @@ void ShaderManagerVulkan::Clear() {
 	vsCache_.Clear();
 	lastFSID_.set_invalid();
 	lastVSID_.set_invalid();
-	lastVShader_ = nullptr;
-	lastFShader_ = nullptr;
 	gstate_c.Dirty(DIRTY_VERTEXSHADER_STATE | DIRTY_FRAGMENTSHADER_STATE);
 }
 
@@ -223,8 +221,6 @@ void ShaderManagerVulkan::DirtyLastShader() {
 	// Forget the last shader ID
 	lastFSID_.set_invalid();
 	lastVSID_.set_invalid();
-	lastVShader_ = nullptr;
-	lastFShader_ = nullptr;
 	gstate_c.Dirty(DIRTY_VERTEXSHADER_STATE | DIRTY_FRAGMENTSHADER_STATE);
 }
 
@@ -242,70 +238,75 @@ uint64_t ShaderManagerVulkan::UpdateUniforms(bool useBufferedRendering) {
 	return dirty;
 }
 
-void ShaderManagerVulkan::GetShaders(int prim, u32 vertexType, VulkanVertexShader **vshader, VulkanFragmentShader **fshader, const ComputedPipelineState &pipelineState, bool useHWTransform, bool weightsAsFloat, bool useSkinInDecode, ClipInfoFlags clipInfoFlags) {
+const VulkanVertexShader *ShaderManagerVulkan::GetVertexShaderFromID(VShaderID VSID) {
+	VulkanVertexShader *vs = vsCache_.GetOrNull(VSID);
+	if (vs) {
+		return vs;
+	}
+	VulkanContext *vulkan = (VulkanContext *)draw_->GetNativeObject(Draw::NativeObject::CONTEXT);
+	// Vertex shader not in cache. Let's compile it.
+	std::string genErrorString;
+	uint64_t uniformMask = 0;  // Not used
+	uint32_t attributeMask = 0;  // Not used
+	VertexShaderFlags flags{};
+	bool success = GenerateVertexShader(VSID, codeBuffer_, compat_, draw_->GetBugs(), &attributeMask, &uniformMask, &flags, &genErrorString);
+	_assert_msg_(success, "VS gen error: %s", genErrorString.c_str());
+	_assert_msg_(strlen(codeBuffer_) < CODE_BUFFER_SIZE, "VS length error: %d", (int)strlen(codeBuffer_));
+
+	const bool useHWTransform = VSID.Bit(VS_BIT_USE_HW_TRANSFORM);
+	vs = new VulkanVertexShader(vulkan, VSID, flags, codeBuffer_, useHWTransform);
+	vsCache_.Insert(VSID, vs);
+	return vs;
+}
+
+const VulkanFragmentShader *ShaderManagerVulkan::GetFragmentShaderFromID(FShaderID FSID) {
+	VulkanFragmentShader *fs = fsCache_.GetOrNull(FSID);
+	if (fs) {
+		return fs;
+	}
+	VulkanContext *vulkan = (VulkanContext *)draw_->GetNativeObject(Draw::NativeObject::CONTEXT);
+	// Fragment shader not in cache. Let's compile it.
+	std::string genErrorString;
+	uint64_t uniformMask = 0;  // Not used
+	FragmentShaderFlags flags{};
+	bool success = GenerateFragmentShader(FSID, codeBuffer_, compat_, draw_->GetBugs(), &uniformMask, &flags, &genErrorString);
+	_assert_msg_(success, "FS gen error: %s", genErrorString.c_str());
+	_assert_msg_(strlen(codeBuffer_) < CODE_BUFFER_SIZE, "FS length error: %d", (int)strlen(codeBuffer_));
+
+	fs = new VulkanFragmentShader(vulkan, FSID, flags, codeBuffer_);
+	fsCache_.Insert(FSID, fs);
+	return fs;
+}
+
+void ShaderManagerVulkan::GetShaderIDs(int prim, u32 vertexType, VShaderID *vshader, FShaderID *fshader, const ComputedPipelineState &pipelineState, bool useHWTransform, bool weightsAsFloat, bool useSkinInDecode, ClipInfoFlags clipInfoFlags) {
 	VulkanContext *vulkan = (VulkanContext *)draw_->GetNativeObject(Draw::NativeObject::CONTEXT);
 
-	VShaderID VSID;
-	VulkanVertexShader *vs = nullptr;
 	bool recomputedVS = false;
-	bool recomputedFS = false;
+	VShaderID VSID;
 	if (gstate_c.IsDirty(DIRTY_VERTEXSHADER_STATE)) {
 		gstate_c.Clean(DIRTY_VERTEXSHADER_STATE);
-		recomputedVS = true;
 		ComputeVertexShaderID(&VSID, vertexType, useHWTransform, weightsAsFloat, useSkinInDecode, clipInfoFlags);
-		if (VSID == lastVSID_) {
-			_dbg_assert_(lastVShader_ != nullptr);
-			vs = lastVShader_;
-		} else if (!vsCache_.Get(VSID, &vs)) {
-			// Vertex shader not in cache. Let's compile it.
-			std::string genErrorString;
-			uint64_t uniformMask = 0;  // Not used
-			uint32_t attributeMask = 0;  // Not used
-			VertexShaderFlags flags{};
-			bool success = GenerateVertexShader(VSID, codeBuffer_, compat_, draw_->GetBugs(), &attributeMask, &uniformMask, &flags, &genErrorString);
-			_assert_msg_(success, "VS gen error: %s", genErrorString.c_str());
-			_assert_msg_(strlen(codeBuffer_) < CODE_BUFFER_SIZE, "VS length error: %d", (int)strlen(codeBuffer_));
-
-			// Don't need to re-lookup anymore, now that we lock wider.
-			vs = new VulkanVertexShader(vulkan, VSID, flags, codeBuffer_, useHWTransform);
-			vsCache_.Insert(VSID, vs);
-		}
-		lastVShader_ = vs;
 		lastVSID_ = VSID;
+		*vshader = VSID;
+		recomputedVS = true;
 	} else {
 		VSID = lastVSID_;
-		vs = lastVShader_;
+		*vshader = lastVSID_;
 	}
-	*vshader = vs;
 
 	FShaderID FSID;
 	VulkanFragmentShader *fs = nullptr;
+	bool recomputedFS = false;
 	if (gstate_c.IsDirty(DIRTY_FRAGMENTSHADER_STATE)) {
 		gstate_c.Clean(DIRTY_FRAGMENTSHADER_STATE);
 		ComputeFragmentShaderID(&FSID, pipelineState, draw_->GetBugs(), clipInfoFlags);
-		recomputedFS = true;
-		if (FSID == lastFSID_) {
-			_dbg_assert_(lastFShader_ != nullptr);
-			fs = lastFShader_;
-		} else if (!fsCache_.Get(FSID, &fs)) {
-			// Fragment shader not in cache. Let's compile it.
-			std::string genErrorString;
-			uint64_t uniformMask = 0;  // Not used
-			FragmentShaderFlags flags{};
-			bool success = GenerateFragmentShader(FSID, codeBuffer_, compat_, draw_->GetBugs(), &uniformMask, &flags, &genErrorString);
-			_assert_msg_(success, "FS gen error: %s", genErrorString.c_str());
-			_assert_msg_(strlen(codeBuffer_) < CODE_BUFFER_SIZE, "FS length error: %d", (int)strlen(codeBuffer_));
-
-			fs = new VulkanFragmentShader(vulkan, FSID, flags, codeBuffer_);
-			fsCache_.Insert(FSID, fs);
-		}
-		lastFShader_ = fs;
 		lastFSID_ = FSID;
+		*fshader = FSID;
+		recomputedFS = true;
 	} else {
 		FSID = lastFSID_;
-		fs = lastFShader_;
+		*fshader = lastFSID_;
 	}
-	*fshader = fs;
 
 	// If you hit these, look at recomputedVS and recomputedFS to determine if it's a dirty-flag problem
 	// or an ID generation problem (if any of them are false, it's a dirty-flag problem).
@@ -314,7 +315,7 @@ void ShaderManagerVulkan::GetShaders(int prim, u32 vertexType, VulkanVertexShade
 	_dbg_assert_(FSID.Bit(FS_BIT_MINMAX_DISCARD) == VSID.Bit(VS_BIT_FS_MINMAX_DISCARD));
 	_dbg_assert_(FSID.Bit(FS_BIT_DEPTH_CLAMP) == VSID.Bit(VS_BIT_FS_DEPTH_CLAMP));
 
-	_dbg_assert_msg_((*vshader)->UseHWTransform() == useHWTransform, "Bad vshader was computed");
+	_dbg_assert_msg_(VSID.Bit(VS_BIT_USE_HW_TRANSFORM) == useHWTransform, "Bad vshader ID was computed");
 }
 
 std::vector<std::string> ShaderManagerVulkan::DebugGetShaderIDs(DebugShaderType type) {
@@ -364,28 +365,6 @@ std::string ShaderManagerVulkan::DebugGetShaderString(std::string id, DebugShade
 	}
 }
 
-VulkanVertexShader *ShaderManagerVulkan::GetVertexShaderFromModule(VkShaderModule module) {
-	VulkanVertexShader *vs = nullptr;
-	vsCache_.Iterate([&](const VShaderID &id, VulkanVertexShader *shader) {
-		Promise<VkShaderModule> *p = shader->GetModule();
-		VkShaderModule m = p->BlockUntilReady();
-		if (m == module)
-			vs = shader;
-	});
-	return vs;
-}
-
-VulkanFragmentShader *ShaderManagerVulkan::GetFragmentShaderFromModule(VkShaderModule module) {
-	VulkanFragmentShader *fs = nullptr;
-	fsCache_.Iterate([&](const FShaderID &id, VulkanFragmentShader *shader) {
-		Promise<VkShaderModule> *p = shader->GetModule();
-		VkShaderModule m = p->BlockUntilReady();
-		if (m == module)
-			fs = shader;
-	});
-	return fs;
-}
-
 // Shader cache.
 //
 // We simply store the IDs of the shaders used during gameplay. On next startup of

GPU/Vulkan/ShaderManagerVulkan.h

@@ -46,7 +46,7 @@ public:
 	const std::string &source() const { return source_; }
 
 	std::string GetShaderString(DebugShaderStringType type) const;
-	Promise<VkShaderModule> *GetModule() { return module_; }
+	Promise<VkShaderModule> *GetModule() const { return module_; }
 	const FShaderID &GetID() const { return id_; }
 
 	FragmentShaderFlags Flags() const { return flags_;  }
@@ -72,7 +72,7 @@ public:
 	VertexShaderFlags Flags() const { return flags_; }
 
 	std::string GetShaderString(DebugShaderStringType type) const;
-	Promise<VkShaderModule> *GetModule() { return module_; }
+	Promise<VkShaderModule> *GetModule() const { return module_; }
 	const VShaderID &GetID() const { return id_; }
 
 protected:
@@ -102,19 +102,15 @@ public:
 	void DeviceLost() override;
 	void DeviceRestore(Draw::DrawContext *draw) override;
 
-	void GetShaders(int prim, u32 vertexType, VulkanVertexShader **vshader, VulkanFragmentShader **fshader, const ComputedPipelineState &pipelineState, bool useHWTransform, bool weightsAsFloat, bool useSkinInDecode, ClipInfoFlags clipInfoFlags);
+	void GetShaderIDs(int prim, u32 vertexType, VShaderID *vshader, FShaderID *fshader, const ComputedPipelineState &pipelineState, bool useHWTransform, bool weightsAsFloat, bool useSkinInDecode, ClipInfoFlags clipInfoFlags);
 	void ClearShaders() override;
 	void DirtyLastShader() override;
 
 	int GetNumVertexShaders() const { return (int)vsCache_.size(); }
 	int GetNumFragmentShaders() const { return (int)fsCache_.size(); }
 
-	// Used for saving/loading the cache. Don't need to be particularly fast.
+	const VulkanVertexShader *GetVertexShaderFromID(VShaderID VSID);
-	VulkanVertexShader *GetVertexShaderFromID(VShaderID id) { return vsCache_.GetOrNull(id); }
+	const VulkanFragmentShader *GetFragmentShaderFromID(FShaderID FSID);
-	VulkanFragmentShader *GetFragmentShaderFromID(FShaderID id) { return fsCache_.GetOrNull(id); }
-
-	VulkanVertexShader *GetVertexShaderFromModule(VkShaderModule module);
-	VulkanFragmentShader *GetFragmentShaderFromModule(VkShaderModule module);
 
 	std::vector<std::string> DebugGetShaderIDs(DebugShaderType type) override;
 	std::string DebugGetShaderString(std::string id, DebugShaderType type, DebugShaderStringType stringType) override;
@@ -158,10 +154,6 @@ private:
 	uint64_t uboAlignment_;
 
 	Uniforms *uniforms_;
-
-	VulkanFragmentShader *lastFShader_ = nullptr;
-	VulkanVertexShader *lastVShader_ = nullptr;
-
 	FShaderID lastFSID_;
 	VShaderID lastVSID_;
 };