Merge pull request #11747 from hrydgard/vk-validation-fixes

Vk validation fixes, plus PowerVR swapchain hack

Common/FileUtil.cpp

@@ -652,7 +652,6 @@ bool DeleteDirRecursively(const std::string &directory)
 
 	if (hFind == INVALID_HANDLE_VALUE)
 	{
-		FindClose(hFind);
 		return false;
 	}
 
@@ -671,7 +670,6 @@ bool DeleteDirRecursively(const std::string &directory)
 	{
 		const std::string virtualName = result->d_name;
 #endif
-
 		// check for "." and ".."
 		if (((virtualName[0] == '.') && (virtualName[1] == '\0')) ||
 			((virtualName[0] == '.') && (virtualName[1] == '.') && 
@@ -683,21 +681,23 @@ bool DeleteDirRecursively(const std::string &directory)
 		{
 			if (!DeleteDirRecursively(newPath))
 			{
-				#ifndef _WIN32
+#ifndef _WIN32
 				closedir(dirp);
-				#endif
-
+#else
+				FindClose(hFind);
+#endif
 				return false;
 			}
 		}
 		else
 		{
 			if (!File::Delete(newPath))
 			{
-				#ifndef _WIN32
+#ifndef _WIN32
 				closedir(dirp);
-				#endif
-
+#else
+				FindClose(hFind);
+#endif
 				return false;
 			}
 		}
@@ -709,8 +709,7 @@ bool DeleteDirRecursively(const std::string &directory)
 	}
 	closedir(dirp);
 #endif
-	File::DeleteDir(directory);
-	return true;
+	return File::DeleteDir(directory);
 #endif
 }
 

Common/Vulkan/VulkanContext.cpp

@@ -505,15 +505,18 @@ void VulkanContext::ChooseDevice(int physical_device) {
 		ELOG("Could not find a usable depth stencil format.");
 	}
 
-	// This is as good a place as any to do this
+	// This is as good a place as any to do this.
 	vkGetPhysicalDeviceMemoryProperties(physical_devices_[physical_device_], &memory_properties);
 	ILOG("Memory Types (%d):", memory_properties.memoryTypeCount);
-	for (int i = 0; i < memory_properties.memoryTypeCount; i++) {
+	for (int i = 0; i < (int)memory_properties.memoryTypeCount; i++) {
+		// Don't bother printing dummy memory types.
+		if (!memory_properties.memoryTypes[i].propertyFlags)
+			continue;
 		ILOG("  %d: Heap %d; Flags: %s%s%s%s  ", i, memory_properties.memoryTypes[i].heapIndex,
-			(memory_properties.memoryTypes[i].propertyFlags & VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT) ? "DEVICE_LOCAL_BIT" : "",
-			(memory_properties.memoryTypes[i].propertyFlags & VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT) ? "HOST_VISIBLE_BIT" : "",
-			(memory_properties.memoryTypes[i].propertyFlags & VK_MEMORY_PROPERTY_HOST_CACHED_BIT) ? "HOST_CACHED_BIT" : "",
-			(memory_properties.memoryTypes[i].propertyFlags & VK_MEMORY_PROPERTY_HOST_COHERENT_BIT) ? "HOST_COHERENT_BIT" : "");
+			(memory_properties.memoryTypes[i].propertyFlags & VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT) ? "DEVICE_LOCAL " : "",
+			(memory_properties.memoryTypes[i].propertyFlags & VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT) ? "HOST_VISIBLE " : "",
+			(memory_properties.memoryTypes[i].propertyFlags & VK_MEMORY_PROPERTY_HOST_CACHED_BIT) ? "HOST_CACHED " : "",
+			(memory_properties.memoryTypes[i].propertyFlags & VK_MEMORY_PROPERTY_HOST_COHERENT_BIT) ? "HOST_COHERENT " : "");
 	}
 
 	// Optional features
@@ -585,7 +588,9 @@ VkResult VulkanContext::CreateDevice() {
 	}
 	assert(found);
 
-	deviceExtensionsLookup_.DEDICATED_ALLOCATION = EnableDeviceExtension(VK_KHR_DEDICATED_ALLOCATION_EXTENSION_NAME);
+	if (EnableDeviceExtension(VK_KHR_GET_MEMORY_REQUIREMENTS_2_EXTENSION_NAME)) {
+		deviceExtensionsLookup_.DEDICATED_ALLOCATION = EnableDeviceExtension(VK_KHR_DEDICATED_ALLOCATION_EXTENSION_NAME);
+	}
 
 	VkDeviceCreateInfo device_info{ VK_STRUCTURE_TYPE_DEVICE_CREATE_INFO };
 	device_info.queueCreateInfoCount = 1;
@@ -642,43 +647,30 @@ void VulkanContext::DestroyDebugMsgCallback() {
 	}
 }
 
-void VulkanContext::InitSurface(WindowSystem winsys, void *data1, void *data2, int width, int height) {
+VkResult VulkanContext::InitSurface(WindowSystem winsys, void *data1, void *data2) {
 	winsys_ = winsys;
 	winsysData1_ = data1;
 	winsysData2_ = data2;
-	ReinitSurface(width, height);
+	return ReinitSurface();
 }
 
-void VulkanContext::ReinitSurface(int width, int height) {
+VkResult VulkanContext::ReinitSurface() {
 	if (surface_ != VK_NULL_HANDLE) {
-		ILOG("Destroying Vulkan surface (%d, %d)", width_, height_);
+		ILOG("Destroying Vulkan surface (%d, %d)", swapChainExtent_.width, swapChainExtent_.height);
 		vkDestroySurfaceKHR(instance_, surface_, nullptr);
 		surface_ = VK_NULL_HANDLE;
 	}
 
-	ILOG("Creating Vulkan surface (%d, %d)", width, height);
+	ILOG("Creating Vulkan surface for window");
 	switch (winsys_) {
 #ifdef _WIN32
 	case WINDOWSYSTEM_WIN32:
 	{
-		HINSTANCE connection = (HINSTANCE)winsysData1_;
-		HWND window = (HWND)winsysData2_;
-
-		if (width < 0 || height < 0)
-		{
-			RECT rc;
-			GetClientRect(window, &rc);
-			width = rc.right - rc.left;
-			height = rc.bottom - rc.top;
-		}
-
 		VkWin32SurfaceCreateInfoKHR win32{ VK_STRUCTURE_TYPE_WIN32_SURFACE_CREATE_INFO_KHR };
 		win32.flags = 0;
-		win32.hwnd = window;
-		win32.hinstance = connection;
-		VkResult res = vkCreateWin32SurfaceKHR(instance_, &win32, nullptr, &surface_);
-		assert(res == VK_SUCCESS);
-		break;
+		win32.hwnd = (HWND)winsysData2_;
+		win32.hinstance = (HINSTANCE)winsysData1_;
+		return vkCreateWin32SurfaceKHR(instance_, &win32, nullptr, &surface_);
 	}
 #endif
 #if defined(__ANDROID__)
@@ -688,9 +680,7 @@ void VulkanContext::ReinitSurface(int width, int height) {
 		VkAndroidSurfaceCreateInfoKHR android{ VK_STRUCTURE_TYPE_ANDROID_SURFACE_CREATE_INFO_KHR };
 		android.flags = 0;
 		android.window = wnd;
-		VkResult res = vkCreateAndroidSurfaceKHR(instance_, &android, nullptr, &surface_);
-		assert(res == VK_SUCCESS);
-		break;
+		return vkCreateAndroidSurfaceKHR(instance_, &android, nullptr, &surface_);
 	}
 #endif
 #if defined(VK_USE_PLATFORM_XLIB_KHR)
@@ -700,9 +690,7 @@ void VulkanContext::ReinitSurface(int width, int height) {
 		xlib.flags = 0;
 		xlib.dpy = (Display *)winsysData1_;
 		xlib.window = (Window)winsysData2_;
-		VkResult res = vkCreateXlibSurfaceKHR(instance_, &xlib, nullptr, &surface_);
-		assert(res == VK_SUCCESS);
-		break;
+		return vkCreateXlibSurfaceKHR(instance_, &xlib, nullptr, &surface_);
 	}
 #endif
 #if defined(VK_USE_PLATFORM_XCB_KHR)
@@ -712,9 +700,7 @@ void VulkanContext::ReinitSurface(int width, int height) {
 		xcb.flags = 0;
 		xcb.connection = (Connection *)winsysData1_;
 		xcb.window = (Window)(uintptr_t)winsysData2_;
-		VkResult res = vkCreateXcbSurfaceKHR(instance_, &xcb, nullptr, &surface_);
-		assert(res == VK_SUCCESS);
-		break;
+		return vkCreateXcbSurfaceKHR(instance_, &xcb, nullptr, &surface_);
 	}
 #endif
 #if defined(VK_USE_PLATFORM_WAYLAND_KHR)
@@ -724,18 +710,14 @@ void VulkanContext::ReinitSurface(int width, int height) {
 		wayland.flags = 0;
 		wayland.display = (wl_display *)winsysData1_;
 		wayland.surface = (wl_surface *)winsysData2_;
-		VkResult res = vkCreateWaylandSurfaceKHR(instance_, &wayland, nullptr, &surface_);
-		assert(res == VK_SUCCESS);
-		break;
+		return vkCreateWaylandSurfaceKHR(instance_, &wayland, nullptr, &surface_);
 	}
 #endif
 
 	default:
 		_assert_msg_(G3D, false, "Vulkan support for chosen window system not implemented");
-		break;
+		return VK_ERROR_INITIALIZATION_FAILED;
 	}
-	width_ = width;
-	height_ = height;
 }
 
 bool VulkanContext::InitQueue() {
@@ -826,6 +808,14 @@ bool VulkanContext::InitQueue() {
 	return true;
 }
 
+int clamp(int x, int a, int b) {
+	if (x < a)
+		return a;
+	if (x > b)
+		return b;
+	return x;
+}
+
 bool VulkanContext::InitSwapchain() {
 	VkResult res = vkGetPhysicalDeviceSurfaceCapabilitiesKHR(physical_devices_[physical_device_], surface_, &surfCapabilities_);
 	assert(res == VK_SUCCESS);
@@ -837,19 +827,20 @@ bool VulkanContext::InitSwapchain() {
 	res = vkGetPhysicalDeviceSurfacePresentModesKHR(physical_devices_[physical_device_], surface_, &presentModeCount, presentModes);
 	assert(res == VK_SUCCESS);
 
-	VkExtent2D swapChainExtent;
-	// width and height are either both -1, or both not -1.
-	if (surfCapabilities_.currentExtent.width == (uint32_t)-1) {
-		// If the surface size is undefined, the size is set to
-		// the size of the images requested.
-		ILOG("initSwapchain: %dx%d", width_, height_);
-		swapChainExtent.width = width_;
-		swapChainExtent.height = height_;
-	} else {
-		// If the surface size is defined, the swap chain size must match
-		swapChainExtent = surfCapabilities_.currentExtent;
+	ILOG("surfCapabilities_.currentExtent: %dx%d", surfCapabilities_.currentExtent.width, surfCapabilities_.currentExtent.height);
+	ILOG("surfCapabilities_.minImageExtent: %dx%d", surfCapabilities_.minImageExtent.width, surfCapabilities_.minImageExtent.height);
+	ILOG("surfCapabilities_.maxImageExtent: %dx%d", surfCapabilities_.maxImageExtent.width, surfCapabilities_.maxImageExtent.height);
+
+	swapChainExtent_.width = clamp(surfCapabilities_.currentExtent.width, surfCapabilities_.minImageExtent.width, surfCapabilities_.maxImageExtent.width);
+	swapChainExtent_.height = clamp(surfCapabilities_.currentExtent.height, surfCapabilities_.minImageExtent.height, surfCapabilities_.maxImageExtent.height);
+
+	if (physicalDeviceProperties_[physical_device_].vendorID == VULKAN_VENDOR_IMGTEC) {
+		// Swap chain width hack to avoid issue #11743 (PowerVR driver bug).
+		swapChainExtent_.width &= ~31;
 	}
 
+	ILOG("swapChainExtent: %dx%d", swapChainExtent_.width, swapChainExtent_.height);
+
 	// TODO: Find a better way to specify the prioritized present mode while being able
 	// to fall back in a sensible way.
 	VkPresentModeKHR swapchainPresentMode = VK_PRESENT_MODE_MAX_ENUM_KHR;
@@ -904,8 +895,8 @@ bool VulkanContext::InitSwapchain() {
 	swap_chain_info.minImageCount = desiredNumberOfSwapChainImages;
 	swap_chain_info.imageFormat = swapchainFormat_;
 	swap_chain_info.imageColorSpace = VK_COLORSPACE_SRGB_NONLINEAR_KHR;
-	swap_chain_info.imageExtent.width = swapChainExtent.width;
-	swap_chain_info.imageExtent.height = swapChainExtent.height;
+	swap_chain_info.imageExtent.width = swapChainExtent_.width;
+	swap_chain_info.imageExtent.height = swapChainExtent_.height;
 	swap_chain_info.preTransform = preTransform;
 	swap_chain_info.imageArrayLayers = 1;
 	swap_chain_info.presentMode = swapchainPresentMode;

Common/Vulkan/VulkanContext.h

@@ -142,8 +142,9 @@ class VulkanContext {
 	VulkanDeleteList &Delete() { return globalDeleteList_; }
 
 	// The parameters are whatever the chosen window system wants.
-	void InitSurface(WindowSystem winsys, void *data1, void *data2, int width = -1, int height = -1);
-	void ReinitSurface(int width = -1, int height = -1);
+	// The extents will be automatically determined.
+	VkResult InitSurface(WindowSystem winsys, void *data1, void *data2);
+	VkResult ReinitSurface();
 
 	bool InitQueue();
 	bool InitObjects();
@@ -160,8 +161,8 @@ class VulkanContext {
 	VkFence CreateFence(bool presignalled);
 	bool CreateShaderModule(const std::vector<uint32_t> &spirv, VkShaderModule *shaderModule);
 
-	int GetBackbufferWidth() { return width_; }
-	int GetBackbufferHeight() { return height_; }
+	int GetBackbufferWidth() { return (int)swapChainExtent_.width; }
+	int GetBackbufferHeight() { return (int)swapChainExtent_.height; }
 
 	void BeginFrame();
 	void EndFrame();
@@ -295,9 +296,9 @@ class VulkanContext {
 	// Custom collection of things that are good to know
 	VulkanPhysicalDeviceInfo deviceInfo_{};
 
-	// Swap chain
-	int width_ = 0;
-	int height_ = 0;
+	// Swap chain extent
+	VkExtent2D swapChainExtent_{};
+
 	int flags_ = 0;
 
 	int inflightFrames_ = MAX_INFLIGHT_FRAMES;

Common/Vulkan/VulkanDebug.cpp

@@ -79,8 +79,6 @@ VkBool32 VKAPI_CALL Vulkan_Dbg(VkDebugReportFlagsEXT msgFlags, VkDebugReportObje
 		return false;
 	if (msgCode == 5)
 		return false;  // Not exactly a false positive, see https://github.com/KhronosGroup/glslang/issues/1418
-	if (msgCode == 0 && strstr(pMsg, "vertexPipelineStoresAndAtomics"))  // https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/73
-		return false;
 #ifdef _WIN32
 	std::string msg = message.str();
 	OutputDebugStringA(msg.c_str());

Common/Vulkan/VulkanImage.cpp

@@ -76,11 +76,8 @@ bool VulkanTexture::CreateDirect(VkCommandBuffer cmd, int w, int h, int numMips,
 
 	if (allocator_) {
 		offset_ = allocator_->Allocate(mem_reqs, &mem_, Tag());
-		if (offset_ == VulkanDeviceAllocator::ALLOCATE_FAILED) {
-			vkDestroyImage(vulkan_->GetDevice(), image_, nullptr);
-			ELOG("Image memory allocation failed (mem_reqs.size = %d)", (int)mem_reqs.size);
-			return false;
-		}
+		// Destructor will take care of the image.
+		return false;
 	} else {
 		VkMemoryAllocateInfo mem_alloc{ VK_STRUCTURE_TYPE_MEMORY_ALLOCATE_INFO };
 		mem_alloc.memoryTypeIndex = 0;
@@ -95,8 +92,6 @@ bool VulkanTexture::CreateDirect(VkCommandBuffer cmd, int w, int h, int numMips,
 			ELOG("vkAllocateMemory failed: %s", VulkanResultToString(res));
 			_assert_msg_(G3D, res != VK_ERROR_TOO_MANY_OBJECTS, "Too many Vulkan memory objects!");
 			_assert_(res == VK_ERROR_OUT_OF_HOST_MEMORY || res == VK_ERROR_OUT_OF_DEVICE_MEMORY || res == VK_ERROR_TOO_MANY_OBJECTS);
-			vkDestroyImage(vulkan_->GetDevice(), image_, nullptr);
-			image_ = VK_NULL_HANDLE;
 			return false;
 		}
 

GPU/Vulkan/PipelineManagerVulkan.cpp

@@ -112,12 +112,14 @@ static int SetupVertexAttribs(VkVertexInputAttributeDescription attrs[], const D
 	return count;
 }
 
-static int SetupVertexAttribsPretransformed(VkVertexInputAttributeDescription attrs[]) {
+static int SetupVertexAttribsPretransformed(VkVertexInputAttributeDescription attrs[], bool needsColor1) {
 	int count = 0;
 	VertexAttribSetup(&attrs[count++], DEC_FLOAT_4, 0, PspAttributeLocation::POSITION);
 	VertexAttribSetup(&attrs[count++], DEC_FLOAT_3, 16, PspAttributeLocation::TEXCOORD);
 	VertexAttribSetup(&attrs[count++], DEC_U8_4, 28, PspAttributeLocation::COLOR0);
-	VertexAttribSetup(&attrs[count++], DEC_U8_4, 32, PspAttributeLocation::COLOR1);
+	if (needsColor1) {
+		VertexAttribSetup(&attrs[count++], DEC_U8_4, 32, PspAttributeLocation::COLOR1);
+	}
 	return count;
 }
 
@@ -243,7 +245,8 @@ static VulkanPipeline *CreateVulkanPipeline(VkDevice device, VkPipelineCache pip
 		attributeCount = SetupVertexAttribs(attrs, *decFmt);
 		vertexStride = decFmt->stride;
 	} else {
-		attributeCount = SetupVertexAttribsPretransformed(attrs);
+		bool needsColor1 = vs->GetID().Bit(FS_BIT_LMODE);
+		attributeCount = SetupVertexAttribsPretransformed(attrs, needsColor1);
 		vertexStride = 36;
 	}
 

SDL/SDLVulkanGraphicsContext.cpp

@@ -63,16 +63,15 @@ bool SDLVulkanGraphicsContext::Init(SDL_Window *&window, int x, int y, int mode,
 	case SDL_SYSWM_X11:
 #if defined(VK_USE_PLATFORM_XLIB_KHR)
 		vulkan_->InitSurface(WINDOWSYSTEM_XLIB, (void*)sys_info.info.x11.display,
-				(void *)(intptr_t)sys_info.info.x11.window, pixel_xres, pixel_yres);
+				(void *)(intptr_t)sys_info.info.x11.window);
 #elif defined(VK_USE_PLATFORM_XCB_KHR)
 		vulkan_->InitSurface(WINDOWSYSTEM_XCB, (void*)XGetXCBConnection(sys_info.info.x11.display),
-				(void *)(intptr_t)sys_info.info.x11.window, pixel_xres, pixel_yres);
+				(void *)(intptr_t)sys_info.info.x11.window);
 #endif
 		break;
 #if defined(VK_USE_PLATFORM_WAYLAND_KHR)
 	case SDL_SYSWM_WAYLAND:
-		vulkan_->InitSurface(WINDOWSYSTEM_WAYLAND, (void*)sys_info.info.wl.display,
-				(void *)sys_info.info.wl.surface, pixel_xres, pixel_yres);
+		vulkan_->InitSurface(WINDOWSYSTEM_WAYLAND, (void*)sys_info.info.wl.display, (void *)sys_info.info.wl.surface);
 		break;
 #endif
 	default:

android/jni/AndroidVulkanContext.cpp

@@ -135,14 +135,8 @@ bool AndroidVulkanContext::InitAPI() {
 }
 
 bool AndroidVulkanContext::InitFromRenderThread(ANativeWindow *wnd, int desiredBackbufferSizeX, int desiredBackbufferSizeY, int backbufferFormat, int androidVersion) {
-	int width = desiredBackbufferSizeX;
-	int height = desiredBackbufferSizeY;
-	if (!width || !height) {
-		width = pixel_xres;
-		height = pixel_yres;
-	}
-	ILOG("InitSurfaceAndroid: width=%d height=%d", width, height);
-	g_Vulkan->InitSurface(WINDOWSYSTEM_ANDROID, (void *)wnd, nullptr, width, height);
+	ILOG("InitSurfaceAndroid: desiredwidth=%d desiredheight=%d", desiredBackbufferSizeX, desiredBackbufferSizeY);
+	g_Vulkan->InitSurface(WINDOWSYSTEM_ANDROID, (void *)wnd, nullptr);
 	if (g_validate_) {
 		int bits = VK_DEBUG_REPORT_ERROR_BIT_EXT | VK_DEBUG_REPORT_WARNING_BIT_EXT | VK_DEBUG_REPORT_PERFORMANCE_WARNING_BIT_EXT;
 		g_Vulkan->InitDebugMsgCallback(&Vulkan_Dbg, bits, &g_LogOptions);
@@ -205,7 +199,7 @@ void AndroidVulkanContext::Resize() {
 	g_Vulkan->DestroyObjects();
 
 	// backbufferResize updated these values.	TODO: Notify another way?
-	g_Vulkan->ReinitSurface(pixel_xres, pixel_yres);
+	g_Vulkan->ReinitSurface();
 	g_Vulkan->InitObjects();
 	draw_->HandleEvent(Draw::Event::GOT_BACKBUFFER, g_Vulkan->GetBackbufferWidth(), g_Vulkan->GetBackbufferHeight());
 	ILOG("AndroidVulkanContext::Resize end (%d, %d)", g_Vulkan->GetBackbufferWidth(), g_Vulkan->GetBackbufferHeight());