Merge pull request #16683 from hrydgard/minor-fixes

Assorted fixes after looking at crash data

Author: hrydgard

Common/GPU/Vulkan/VulkanContext.cpp

@@ -719,7 +719,9 @@ VkResult VulkanContext::CreateDevice() {
 	allocatorInfo.physicalDevice = physical_devices_[physical_device_];
 	allocatorInfo.device = device_;
 	allocatorInfo.instance = instance_;
-	vmaCreateAllocator(&allocatorInfo, &allocator_);
+	VkResult result = vmaCreateAllocator(&allocatorInfo, &allocator_);
+	_assert_(result == VK_SUCCESS);
+	_assert_(allocator_ != VK_NULL_HANDLE);
 
 	// Examine the physical device to figure out super rough performance grade.
 	// Basically all we want to do is to identify low performance mobile devices

Common/GPU/Vulkan/VulkanRenderManager.cpp

@@ -37,20 +37,28 @@ bool VKRGraphicsPipeline::Create(VulkanContext *vulkan, VkRenderPass compatibleR
 		}
 	}
 
+	// Sanity check.
+	// Seen in crash reports from PowerVR GE8320, presumably we failed creating some shader modules.
+	if (!desc->vertexShader || !desc->fragmentShader) {
+		ERROR_LOG(G3D, "Failed creating graphics pipeline - missing vs/fs shader module pointers!");
+		pipeline[(size_t)rpType]->Post(VK_NULL_HANDLE);
+		return false;
+	}
+
 	// Fill in the last part of the desc since now it's time to block.
 	VkShaderModule vs = desc->vertexShader->BlockUntilReady();
 	VkShaderModule fs = desc->fragmentShader->BlockUntilReady();
 	VkShaderModule gs = desc->geometryShader ? desc->geometryShader->BlockUntilReady() : VK_NULL_HANDLE;
 
 	if (!vs || !fs || (!gs && desc->geometryShader)) {
 		ERROR_LOG(G3D, "Failed creating graphics pipeline - missing shader modules");
-		// We're kinda screwed here?
+		pipeline[(size_t)rpType]->Post(VK_NULL_HANDLE);
 		return false;
 	}
 
 	if (!compatibleRenderPass) {
-		ERROR_LOG(G3D, "Failed creating graphics pipeline - compatible render pass was null");
-		// We're kinda screwed here?
+		ERROR_LOG(G3D, "Failed creating graphics pipeline - compatible render pass was nullptr");
+		pipeline[(size_t)rpType]->Post(VK_NULL_HANDLE);
 		return false;
 	}
 
@@ -527,8 +535,12 @@ VkCommandBuffer VulkanRenderManager::GetInitCmd() {
 
 VKRGraphicsPipeline *VulkanRenderManager::CreateGraphicsPipeline(VKRGraphicsPipelineDesc *desc, PipelineFlags pipelineFlags, uint32_t variantBitmask, VkSampleCountFlagBits sampleCount, const char *tag) {
 	VKRGraphicsPipeline *pipeline = new VKRGraphicsPipeline(pipelineFlags, tag);
-	_dbg_assert_(desc->vertexShader);
-	_dbg_assert_(desc->fragmentShader);
+
+	if (!desc->vertexShader || !desc->fragmentShader) {
+		ERROR_LOG(G3D, "Can't create graphics pipeline with missing vs/ps: %p %p", desc->vertexShader, desc->fragmentShader);
+		return nullptr;
+	}
+
 	pipeline->desc = desc;
 	pipeline->desc->AddRef();
 	if (curRenderStep_) {
@@ -634,6 +646,10 @@ void VulkanRenderManager::EndCurRenderStep() {
 	compileMutex_.lock();
 	bool needsCompile = false;
 	for (VKRGraphicsPipeline *pipeline : pipelinesToCheck_) {
+		if (!pipeline) {
+			// Not good, but let's try not to crash.
+			continue;
+		}
 		if (!pipeline->pipeline[(size_t)rpType]) {
 			pipeline->pipeline[(size_t)rpType] = Promise<VkPipeline>::CreateEmpty();
 			_assert_(renderPass);

GPU/Vulkan/PipelineManagerVulkan.cpp

@@ -180,11 +180,25 @@ static std::string CutFromMain(std::string str) {
 static VulkanPipeline *CreateVulkanPipeline(VulkanRenderManager *renderManager, VkPipelineCache pipelineCache,
 	VkPipelineLayout layout, PipelineFlags pipelineFlags, VkSampleCountFlagBits sampleCount, const VulkanPipelineRasterStateKey &key,
 	const DecVtxFormat *decFmt, VulkanVertexShader *vs, VulkanFragmentShader *fs, VulkanGeometryShader *gs, bool useHwTransform, u32 variantBitmask) {
+
+	if (!fs->GetModule()) {
+		ERROR_LOG(G3D, "Fragment shader missing in CreateVulkanPipeline");
+		return nullptr;
+	}
+	if (!vs->GetModule()) {
+		ERROR_LOG(G3D, "Vertex shader missing in CreateVulkanPipeline");
+		return nullptr;
+	}
+
 	VulkanPipeline *vulkanPipeline = new VulkanPipeline();
 	vulkanPipeline->desc = new VKRGraphicsPipelineDesc();
 	VKRGraphicsPipelineDesc *desc = vulkanPipeline->desc;
 	desc->pipelineCache = pipelineCache;
 
+	desc->fragmentShader = fs->GetModule();
+	desc->vertexShader = vs->GetModule();
+	desc->geometryShader = gs ? gs->GetModule() : nullptr;
+
 	PROFILE_THIS_SCOPE("pipelinebuild");
 	bool useBlendConstant = false;
 
@@ -257,9 +271,6 @@ static VulkanPipeline *CreateVulkanPipeline(VulkanRenderManager *renderManager,
 	rs.polygonMode = VK_POLYGON_MODE_FILL;
 	rs.depthClampEnable = key.depthClampEnable;
 
-	desc->fragmentShader = fs->GetModule();
-	desc->vertexShader = vs->GetModule();
-	desc->geometryShader = gs ? gs->GetModule() : nullptr;
 	desc->fragmentShaderSource = fs->GetShaderString(SHADER_STRING_SOURCE_CODE);
 	desc->vertexShaderSource = vs->GetShaderString(SHADER_STRING_SOURCE_CODE);
 	if (gs) {
@@ -360,6 +371,8 @@ VulkanPipeline *PipelineManagerVulkan::GetOrCreatePipeline(VulkanRenderManager *
 	VulkanPipeline *pipeline = CreateVulkanPipeline(
 		renderManager, pipelineCache_, layout, pipelineFlags, sampleCount,
 		rasterKey, decFmt, vs, fs, gs, useHwTransform, variantBitmask);
+
+	// If the above failed, we got a null pipeline. We still insert it to keep track.
 	pipelines_.Insert(key, pipeline);
 
 	// Don't return placeholder null pipelines.

GPU/Vulkan/ShaderManagerVulkan.cpp

@@ -119,7 +119,9 @@ VulkanFragmentShader::VulkanFragmentShader(VulkanContext *vulkan, FShaderID id,
 VulkanFragmentShader::~VulkanFragmentShader() {
 	if (module_) {
 		VkShaderModule shaderModule = module_->BlockUntilReady();
-		vulkan_->Delete().QueueDeleteShaderModule(shaderModule);
+		if (shaderModule) {
+			vulkan_->Delete().QueueDeleteShaderModule(shaderModule);
+		}
 		vulkan_->Delete().QueueCallback([](void *m) {
 			auto module = (Promise<VkShaderModule> *)m;
 			delete module;
@@ -152,7 +154,9 @@ VulkanVertexShader::VulkanVertexShader(VulkanContext *vulkan, VShaderID id, Vert
 VulkanVertexShader::~VulkanVertexShader() {
 	if (module_) {
 		VkShaderModule shaderModule = module_->BlockUntilReady();
-		vulkan_->Delete().QueueDeleteShaderModule(shaderModule);
+		if (shaderModule) {
+			vulkan_->Delete().QueueDeleteShaderModule(shaderModule);
+		}
 		vulkan_->Delete().QueueCallback([](void *m) {
 			auto module = (Promise<VkShaderModule> *)m;
 			delete module;
@@ -185,7 +189,9 @@ VulkanGeometryShader::VulkanGeometryShader(VulkanContext *vulkan, GShaderID id,
 VulkanGeometryShader::~VulkanGeometryShader() {
 	if (module_) {
 		VkShaderModule shaderModule = module_->BlockUntilReady();
-		vulkan_->Delete().QueueDeleteShaderModule(shaderModule);
+		if (shaderModule) {
+			vulkan_->Delete().QueueDeleteShaderModule(shaderModule);
+		}
 		vulkan_->Delete().QueueCallback([](void *m) {
 			auto module = (Promise<VkShaderModule> *)m;
 			delete module;

UI/NativeApp.cpp

@@ -985,7 +985,9 @@ bool CreateGlobalPipelines() {
 void NativeShutdownGraphics() {
 	INFO_LOG(SYSTEM, "NativeShutdownGraphics");
 
-	screenManager->deviceLost();
+	if (screenManager) {
+		screenManager->deviceLost();
+	}
 
 	if (gpu)
 		gpu->DeviceLost();