Fix Win32SurfaceKHR resize issues

amaiorano · amaiorano · commit 26c6c4a5eb83 · 2019-10-18T13:18:39.000Z
* Fixes crash when window is resized. * Take destination pitch into account when updating the framebuffer, so we no longer get skewed renders. * Return VK_ERROR_OUT_OF_DATE_KHR when presenting if the swapchain image dimensions do not match the window surface's. Bug: b/139372840 Change-Id: I744adbf222a1d06bc9cc879e299b11b56f6dd7cf Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/37429 Kokoro-Presubmit: kokoro <noreply+kokoro@google.com> Reviewed-by: Alexis Hétu <sugoi@google.com> Tested-by: Antonio Maiorano <amaiorano@google.com>
diff --git a/src/Vulkan/VkImage.cpp b/src/Vulkan/VkImage.cpp
@@ -733,6 +733,11 @@ void Image::blit(Image* dstImage, const VkImageBlit& region, VkFilter filter) co
 	device->getBlitter()->blit(this, dstImage, region, filter);
 }
 
+void Image::blitToBuffer(VkImageSubresourceLayers subresource, VkOffset3D offset, VkExtent3D extent, uint8_t* dst, int bufferRowPitch, int bufferSlicePitch) const
+{
+	device->getBlitter()->blitToBuffer(this, subresource, offset, extent, dst, bufferRowPitch, bufferSlicePitch);
+}
+
 void Image::resolve(Image* dstImage, const VkImageResolve& region) const
 {
 	VkImageBlit blitRegion;
diff --git a/src/Vulkan/VkImage.hpp b/src/Vulkan/VkImage.hpp
@@ -41,6 +41,7 @@ class Image : public Object<Image, VkImage>
 	void copyFrom(Buffer* srcBuffer, const VkBufferImageCopy& region);
 
 	void blit(Image* dstImage, const VkImageBlit& region, VkFilter filter) const;
+	void blitToBuffer(VkImageSubresourceLayers subresource, VkOffset3D offset, VkExtent3D extent, uint8_t* dst, int bufferRowPitch, int bufferSlicePitch) const;
 	void resolve(Image* dstImage, const VkImageResolve& region) const;
 	void clear(const VkClearValue& clearValue, const vk::Format& viewFormat, const VkRect2D& renderArea, const VkImageSubresourceRange& subresourceRange);
 	void clear(const VkClearColorValue& color, const VkImageSubresourceRange& subresourceRange);
diff --git a/src/Vulkan/VkQueue.cpp b/src/Vulkan/VkQueue.cpp
@@ -213,7 +213,7 @@ void Queue::garbageCollect()
 }
 
 #ifndef __ANDROID__
-void Queue::present(const VkPresentInfoKHR* presentInfo)
+VkResult Queue::present(const VkPresentInfoKHR* presentInfo)
 {
 	// This is a hack to deal with screen tearing for now.
 	// Need to correctly implement threading using VkSemaphore
@@ -227,8 +227,12 @@ void Queue::present(const VkPresentInfoKHR* presentInfo)
 
 	for(uint32_t i = 0; i < presentInfo->swapchainCount; i++)
 	{
-		vk::Cast(presentInfo->pSwapchains[i])->present(presentInfo->pImageIndices[i]);
+		VkResult result = vk::Cast(presentInfo->pSwapchains[i])->present(presentInfo->pImageIndices[i]);
+		if (result != VK_SUCCESS)
+			return result;
 	}
+
+	return VK_SUCCESS;
 }
 #endif
 
diff --git a/src/Vulkan/VkQueue.hpp b/src/Vulkan/VkQueue.hpp
@@ -55,7 +55,7 @@ class Queue
 	VkResult submit(uint32_t submitCount, const VkSubmitInfo* pSubmits, Fence* fence);
 	VkResult waitIdle();
 #ifndef __ANDROID__
-	void present(const VkPresentInfoKHR* presentInfo);
+	VkResult present(const VkPresentInfoKHR* presentInfo);
 #endif
 
 private:
diff --git a/src/Vulkan/libVulkan.cpp b/src/Vulkan/libVulkan.cpp
@@ -2927,9 +2927,7 @@ VKAPI_ATTR VkResult VKAPI_CALL vkQueuePresentKHR(VkQueue queue, const VkPresentI
 	TRACE("(VkQueue queue = %p, const VkPresentInfoKHR* pPresentInfo = %p)",
 			queue, pPresentInfo);
 
-	vk::Cast(queue)->present(pPresentInfo);
-
-	return VK_SUCCESS;
+	return vk::Cast(queue)->present(pPresentInfo);
 }
 
 VKAPI_ATTR VkResult VKAPI_CALL vkAcquireNextImage2KHR(VkDevice device, const VkAcquireNextImageInfoKHR *pAcquireInfo, uint32_t *pImageIndex)
diff --git a/src/WSI/MacOSSurfaceMVK.h b/src/WSI/MacOSSurfaceMVK.h
@@ -35,7 +35,7 @@ class MacOSSurfaceMVK : public SurfaceKHR, public ObjectBase<MacOSSurfaceMVK, Vk
 
     virtual void attachImage(PresentImage* image) override {}
     virtual void detachImage(PresentImage* image) override {}
-    void present(PresentImage* image) override;
+    VkResult present(PresentImage* image) override;
 
 private:
     MetalLayer* metalLayer = nullptr;
diff --git a/src/WSI/MacOSSurfaceMVK.mm b/src/WSI/MacOSSurfaceMVK.mm
@@ -128,7 +128,7 @@ VkExtent2D getExtent() const
     pSurfaceCapabilities->maxImageExtent = extent;
 }
 
-void MacOSSurfaceMVK::present(PresentImage* image)
+VkResult MacOSSurfaceMVK::present(PresentImage* image)
 {
     auto drawable = metalLayer->getNextDrawable();
     if(drawable)
@@ -140,6 +140,7 @@ VkExtent2D getExtent() const
                           bytesPerRow:image->getImage()->rowPitchBytes(VK_IMAGE_ASPECT_COLOR_BIT, 0)];
         [drawable present];
     }
+    return VK_SUCCESS;
 }
 
 }
diff --git a/src/WSI/VkSurfaceKHR.hpp b/src/WSI/VkSurfaceKHR.hpp
@@ -90,7 +90,7 @@ class SurfaceKHR
 
 	virtual void attachImage(PresentImage* image) = 0;
 	virtual void detachImage(PresentImage* image) = 0;
-	virtual void present(PresentImage* image) = 0;
+	virtual VkResult present(PresentImage* image) = 0;
 
 	void associateSwapchain(SwapchainKHR* swapchain);
 	void disassociateSwapchain();
diff --git a/src/WSI/VkSwapchainKHR.cpp b/src/WSI/VkSwapchainKHR.cpp
@@ -198,18 +198,20 @@ VkResult SwapchainKHR::getNextImage(uint64_t timeout, Semaphore* semaphore, Fenc
 	return VK_NOT_READY;
 }
 
-void SwapchainKHR::present(uint32_t index)
+VkResult SwapchainKHR::present(uint32_t index)
 {
-	auto & image = images[index];
+	auto& image = images[index];
 	image.setStatus(PRESENTING);
-	surface->present(&image);
+	VkResult result = surface->present(&image);
 	image.setStatus(AVAILABLE);
 
 	if(retired)
 	{
 		surface->detachImage(&image);
 		image.clear();
 	}
+
+	return result;
 }
 
 }
diff --git a/src/WSI/VkSwapchainKHR.hpp b/src/WSI/VkSwapchainKHR.hpp
@@ -46,7 +46,7 @@ class SwapchainKHR : public Object<SwapchainKHR, VkSwapchainKHR>
 
 	VkResult getNextImage(uint64_t timeout, Semaphore* semaphore, Fence* fence, uint32_t* pImageIndex);
 
-	void present(uint32_t index);
+	VkResult present(uint32_t index);
 	PresentImage const &getImage(uint32_t imageIndex) { return images[imageIndex]; }
 
 private:
diff --git a/src/WSI/Win32SurfaceKHR.cpp b/src/WSI/Win32SurfaceKHR.cpp
@@ -19,37 +19,35 @@
 
 #include <string.h>
 
+namespace
+{
+	VkExtent2D getWindowSize(HWND hwnd)
+	{
+		RECT clientRect = {};
+		BOOL status = GetClientRect(hwnd, &clientRect);
+		ASSERT(status != 0);
+
+		int windowWidth = clientRect.right - clientRect.left;
+		int windowHeight = clientRect.bottom - clientRect.top;
+
+		return { static_cast<uint32_t>(windowWidth), static_cast<uint32_t>(windowHeight) };
+	}
+}
+
 namespace vk {
 
 Win32SurfaceKHR::Win32SurfaceKHR(const VkWin32SurfaceCreateInfoKHR *pCreateInfo, void *mem) :
 		hwnd(pCreateInfo->hwnd)
 {
 	ASSERT(IsWindow(hwnd) == TRUE);
-
-	RECT clientRect = {};
-	BOOL status = GetClientRect(hwnd, &clientRect);
-	ASSERT(status != 0);
-
 	windowContext = GetDC(hwnd);
 	bitmapContext = CreateCompatibleDC(windowContext);
-
-	BITMAPINFO bitmapInfo = {};
-	bitmapInfo.bmiHeader.biSize = sizeof(BITMAPINFO);
-	bitmapInfo.bmiHeader.biBitCount = 32;
-	bitmapInfo.bmiHeader.biPlanes = 1;
-	bitmapInfo.bmiHeader.biHeight = clientRect.top - clientRect.bottom;
-	bitmapInfo.bmiHeader.biWidth = clientRect.right - clientRect.left;
-	bitmapInfo.bmiHeader.biCompression = BI_RGB;
-
-	bitmap = CreateDIBSection(bitmapContext, &bitmapInfo, DIB_RGB_COLORS, &framebuffer, 0, 0);
-	ASSERT(bitmap != NULL);
-	SelectObject(bitmapContext, bitmap);
+	lazyCreateFrameBuffer();
 }
 
 void Win32SurfaceKHR::destroySurface(const VkAllocationCallbacks *pAllocator)
 {
-	SelectObject(bitmapContext, NULL);
-	DeleteObject(bitmap);
+	destroyFrameBuffer();
 	ReleaseDC(hwnd, windowContext);
 	DeleteDC(bitmapContext);
 }
@@ -62,15 +60,7 @@ size_t Win32SurfaceKHR::ComputeRequiredAllocationSize(const VkWin32SurfaceCreate
 void Win32SurfaceKHR::getSurfaceCapabilities(VkSurfaceCapabilitiesKHR *pSurfaceCapabilities) const
 {
 	SurfaceKHR::getSurfaceCapabilities(pSurfaceCapabilities);
-
-	RECT clientRect = {};
-	BOOL status = GetClientRect(hwnd, &clientRect);
-	ASSERT(status != 0);
-
-	int windowWidth = clientRect.right - clientRect.left;
-	int windowHeight = clientRect.bottom - clientRect.top;
-
-	VkExtent2D extent = {static_cast<uint32_t>(windowWidth), static_cast<uint32_t>(windowHeight)};
+	VkExtent2D extent = getWindowSize(hwnd);
 	pSurfaceCapabilities->currentExtent = extent;
 	pSurfaceCapabilities->minImageExtent = extent;
 	pSurfaceCapabilities->maxImageExtent = extent;
@@ -88,20 +78,80 @@ void Win32SurfaceKHR::detachImage(PresentImage* image)
 	// present instead of associating the image with the surface.
 }
 
-void Win32SurfaceKHR::present(PresentImage* image)
+VkResult Win32SurfaceKHR::present(PresentImage* image)
 {
+	// Recreate frame buffer in case window size has changed
+	lazyCreateFrameBuffer();
+
 	if(!framebuffer)
 	{
-		return;
+		// e.g. window width or height is 0
+		return VK_SUCCESS;
 	}
 
 	VkExtent3D extent = image->getImage()->getMipLevelExtent(VK_IMAGE_ASPECT_COLOR_BIT, 0);
 
-	// FIXME(b/139184291): Assumes B8G8R8A8 surface without stride padding.
-	size_t bytes = extent.width * extent.height * 4;
-	memcpy(framebuffer, image->getImageMemory()->getOffsetPointer(0), bytes);
+	if (windowExtent.width != extent.width || windowExtent.height != extent.height)
+	{
+		return VK_ERROR_OUT_OF_DATE_KHR;
+	}
+
+	VkImageSubresourceLayers subresourceLayers{};
+	subresourceLayers.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT;
+	subresourceLayers.layerCount = 1;
+
+	image->getImage()->blitToBuffer(subresourceLayers, VkOffset3D{}, extent, reinterpret_cast<uint8_t*>(framebuffer), bitmapRowPitch, 0);
 
 	StretchBlt(windowContext, 0, 0, extent.width, extent.height, bitmapContext, 0, 0, extent.width, extent.height, SRCCOPY);
+
+	return VK_SUCCESS;
+}
+
+void Win32SurfaceKHR::lazyCreateFrameBuffer()
+{
+	auto currWindowExtent = getWindowSize(hwnd);
+	if (currWindowExtent.width == windowExtent.width && currWindowExtent.height == windowExtent.height)
+	{
+		return;
+	}
+
+	windowExtent = currWindowExtent;
+
+	if (framebuffer)
+	{
+		destroyFrameBuffer();
+	}
+
+	if (windowExtent.width == 0 || windowExtent.height == 0)
+	{
+		return;
+	}
+
+	BITMAPINFO bitmapInfo = {};
+	bitmapInfo.bmiHeader.biSize = sizeof(BITMAPINFO);
+	bitmapInfo.bmiHeader.biBitCount = 32;
+	bitmapInfo.bmiHeader.biPlanes = 1;
+	bitmapInfo.bmiHeader.biHeight = -windowExtent.height;
+	bitmapInfo.bmiHeader.biWidth = windowExtent.width;
+	bitmapInfo.bmiHeader.biCompression = BI_RGB;
+
+	bitmap = CreateDIBSection(bitmapContext, &bitmapInfo, DIB_RGB_COLORS, &framebuffer, 0, 0);
+	ASSERT(bitmap != NULL);
+	SelectObject(bitmapContext, bitmap);
+
+	BITMAP header;
+	int status = GetObject(bitmap, sizeof(BITMAP), &header);
+	ASSERT(status != 0);
+	bitmapRowPitch = static_cast<int>(header.bmWidthBytes);
+}
+
+void Win32SurfaceKHR::destroyFrameBuffer()
+{
+	SelectObject(bitmapContext, NULL);
+	DeleteObject(bitmap);
+	bitmap = {};
+	bitmapRowPitch = 0;
+	framebuffer = nullptr;
 }
 
 }
diff --git a/src/WSI/Win32SurfaceKHR.hpp b/src/WSI/Win32SurfaceKHR.hpp
@@ -41,15 +41,20 @@ class Win32SurfaceKHR : public SurfaceKHR, public ObjectBase<Win32SurfaceKHR, Vk
 
 	virtual void attachImage(PresentImage* image) override;
 	virtual void detachImage(PresentImage* image) override;
-	void present(PresentImage* image) override;
+	VkResult present(PresentImage* image) override;
 
 private:
+	void lazyCreateFrameBuffer();
+	void destroyFrameBuffer();
+
 	const HWND hwnd;
 
 	HDC windowContext = {};
 	HDC bitmapContext = {};
+	VkExtent2D windowExtent = {};
 
 	HBITMAP bitmap = {};
+	int bitmapRowPitch = 0;
 	void *framebuffer = nullptr;
 };
 
diff --git a/src/WSI/XcbSurfaceKHR.cpp b/src/WSI/XcbSurfaceKHR.cpp
@@ -139,7 +139,7 @@ void XcbSurfaceKHR::detachImage(PresentImage* image)
 	}
 }
 
-void XcbSurfaceKHR::present(PresentImage* image)
+VkResult XcbSurfaceKHR::present(PresentImage* image)
 {
 	auto it = graphicsContexts.find(image);
 	if(it != graphicsContexts.end())
@@ -167,6 +167,8 @@ void XcbSurfaceKHR::present(PresentImage* image)
 
 		libXcb->xcb_flush(connection);
 	}
+
+	return VK_SUCCESS;
 }
 
 }
diff --git a/src/WSI/XcbSurfaceKHR.hpp b/src/WSI/XcbSurfaceKHR.hpp
@@ -36,7 +36,7 @@ class XcbSurfaceKHR : public SurfaceKHR, public ObjectBase<XcbSurfaceKHR, VkSurf
 
 	virtual void attachImage(PresentImage* image) override;
 	virtual void detachImage(PresentImage* image) override;
-	void present(PresentImage* image) override;
+	VkResult present(PresentImage* image) override;
 
 private:
 	xcb_connection_t* connection;
diff --git a/src/WSI/XlibSurfaceKHR.cpp b/src/WSI/XlibSurfaceKHR.cpp
@@ -82,7 +82,7 @@ void XlibSurfaceKHR::detachImage(PresentImage* image)
 	}
 }
 
-void XlibSurfaceKHR::present(PresentImage* image)
+VkResult XlibSurfaceKHR::present(PresentImage* image)
 {
 	auto it = imageMap.find(image);
 	if(it != imageMap.end())
@@ -95,6 +95,8 @@ void XlibSurfaceKHR::present(PresentImage* image)
 			libX11->XPutImage(pDisplay, window, gc, xImage, 0, 0, 0, 0, extent.width, extent.height);
 		}
 	}
+
+	return VK_SUCCESS;
 }
 
 }
diff --git a/src/WSI/XlibSurfaceKHR.hpp b/src/WSI/XlibSurfaceKHR.hpp
@@ -36,7 +36,7 @@ class XlibSurfaceKHR : public SurfaceKHR, public ObjectBase<XlibSurfaceKHR, VkSu
 
 	virtual void attachImage(PresentImage* image) override;
 	virtual void detachImage(PresentImage* image) override;
-	void present(PresentImage* image) override;
+	VkResult present(PresentImage* image) override;
 
 private:
 	Display *const pDisplay;

Original file line number	Diff line number	Diff line change
`@@ -733,6 +733,11 @@ void Image::blit(Image* dstImage, const VkImageBlit& region, VkFilter filter) co`
`733`	`733`	`device->getBlitter()->blit(this, dstImage, region, filter);`
`734`	`734`	`}`
`735`	`735`
	`736`	`+void Image::blitToBuffer(VkImageSubresourceLayers subresource, VkOffset3D offset, VkExtent3D extent, uint8_t* dst, int bufferRowPitch, int bufferSlicePitch) const`
	`737`	`+{`
	`738`	`+ device->getBlitter()->blitToBuffer(this, subresource, offset, extent, dst, bufferRowPitch, bufferSlicePitch);`
	`739`	`+}`
	`740`	`+`
`736`	`741`	`void Image::resolve(Image* dstImage, const VkImageResolve& region) const`
`737`	`742`	`{`
`738`	`743`	`VkImageBlit blitRegion;`
Original file line number	Diff line number	Diff line change
`@@ -213,7 +213,7 @@ void Queue::garbageCollect()`
`213`	`213`	`}`
`214`	`214`
`215`	`215`	`#ifndef __ANDROID__`
`216`		`-void Queue::present(const VkPresentInfoKHR* presentInfo)`
	`216`	`+VkResult Queue::present(const VkPresentInfoKHR* presentInfo)`
`217`	`217`	`{`
`218`	`218`	`// This is a hack to deal with screen tearing for now.`
`219`	`219`	`// Need to correctly implement threading using VkSemaphore`
`@@ -227,8 +227,12 @@ void Queue::present(const VkPresentInfoKHR* presentInfo)`
`227`	`227`
`228`	`228`	`for(uint32_t i = 0; i < presentInfo->swapchainCount; i++)`
`229`	`229`	`{`
`230`		`- vk::Cast(presentInfo->pSwapchains[i])->present(presentInfo->pImageIndices[i]);`
	`230`	`+ VkResult result = vk::Cast(presentInfo->pSwapchains[i])->present(presentInfo->pImageIndices[i]);`
	`231`	`+ if (result != VK_SUCCESS)`
	`232`	`+ return result;`
`231`	`233`	`}`
	`234`	`+`
	`235`	`+ return VK_SUCCESS;`
`232`	`236`	`}`
`233`	`237`	`#endif`
`234`	`238`
Original file line number	Diff line number	Diff line change
`@@ -2927,9 +2927,7 @@ VKAPI_ATTR VkResult VKAPI_CALL vkQueuePresentKHR(VkQueue queue, const VkPresentI`
`2927`	`2927`	`TRACE("(VkQueue queue = %p, const VkPresentInfoKHR* pPresentInfo = %p)",`
`2928`	`2928`	`queue, pPresentInfo);`
`2929`	`2929`
`2930`		`- vk::Cast(queue)->present(pPresentInfo);`
`2931`		`-`
`2932`		`- return VK_SUCCESS;`
	`2930`	`+ return vk::Cast(queue)->present(pPresentInfo);`
`2933`	`2931`	`}`
`2934`	`2932`
`2935`	`2933`	`VKAPI_ATTR VkResult VKAPI_CALL vkAcquireNextImage2KHR(VkDevice device, const VkAcquireNextImageInfoKHR pAcquireInfo, uint32_t pImageIndex)`
Original file line number	Diff line number	Diff line change
`@@ -128,7 +128,7 @@ VkExtent2D getExtent() const`
`128`	`128`	`pSurfaceCapabilities->maxImageExtent = extent;`
`129`	`129`	`}`
`130`	`130`
`131`		`-void MacOSSurfaceMVK::present(PresentImage* image)`
	`131`	`+VkResult MacOSSurfaceMVK::present(PresentImage* image)`
`132`	`132`	`{`
`133`	`133`	`auto drawable = metalLayer->getNextDrawable();`
`134`	`134`	`if(drawable)`
`@@ -140,6 +140,7 @@ VkExtent2D getExtent() const`
`140`	`140`	`bytesPerRow:image->getImage()->rowPitchBytes(VK_IMAGE_ASPECT_COLOR_BIT, 0)];`
`141`	`141`	`[drawable present];`
`142`	`142`	`}`
	`143`	`+ return VK_SUCCESS;`
`143`	`144`	`}`
`144`	`145`
`145`	`146`	`}`
Original file line number	Diff line number	Diff line change
`@@ -198,18 +198,20 @@ VkResult SwapchainKHR::getNextImage(uint64_t timeout, Semaphore* semaphore, Fenc`
`198`	`198`	`return VK_NOT_READY;`
`199`	`199`	`}`
`200`	`200`
`201`		`-void SwapchainKHR::present(uint32_t index)`
	`201`	`+VkResult SwapchainKHR::present(uint32_t index)`
`202`	`202`	`{`
`203`		`- auto & image = images[index];`
	`203`	`+ auto& image = images[index];`
`204`	`204`	`image.setStatus(PRESENTING);`
`205`		`- surface->present(&image);`
	`205`	`+ VkResult result = surface->present(&image);`
`206`	`206`	`image.setStatus(AVAILABLE);`
`207`	`207`
`208`	`208`	`if(retired)`
`209`	`209`	`{`
`210`	`210`	`surface->detachImage(&image);`
`211`	`211`	`image.clear();`
`212`	`212`	`}`
	`213`	`+`
	`214`	`+ return result;`
`213`	`215`	`}`
`214`	`216`
`215`	`217`	`}`