diff --git a/src/dawn_native/vulkan/SwapChainVk.cpp b/src/dawn_native/vulkan/SwapChainVk.cpp index b91f0e3df4..ad88c9d6a3 100644 --- a/src/dawn_native/vulkan/SwapChainVk.cpp +++ b/src/dawn_native/vulkan/SwapChainVk.cpp @@ -183,6 +183,8 @@ namespace dawn_native { namespace vulkan { DetachFromSurface(); } + // Note that when we need to re-create the swapchain because it is out of date, + // previousSwapChain can be set to `this`. MaybeError SwapChain::Initialize(NewSwapChainBase* previousSwapChain) { Device* device = ToBackend(GetDevice()); Adapter* adapter = ToBackend(GetDevice()->GetAdapter()); @@ -217,7 +219,9 @@ namespace dawn_native { namespace vulkan { std::swap(previousVulkanSwapChain->mSwapChain, oldVkSwapChain); device->GetFencedDeleter()->DeleteWhenUnused(oldVkSwapChain); - previousSwapChain->DetachFromSurface(); + if (previousSwapChain != this) { + previousSwapChain->DetachFromSurface(); + } } if (mVkSurface == VK_NULL_HANDLE) { @@ -236,12 +240,11 @@ namespace dawn_native { namespace vulkan { createInfo.flags = 0; createInfo.surface = mVkSurface; createInfo.minImageCount = 3; // TODO - createInfo.imageFormat = VK_FORMAT_B8G8R8A8_UNORM; // TODO - createInfo.imageColorSpace = VK_COLOR_SPACE_SRGB_NONLINEAR_KHR; // TODO? - createInfo.imageExtent.width = GetWidth(); // TODO - createInfo.imageExtent.height = GetHeight(); // TODO + createInfo.imageFormat = mConfig.format; + createInfo.imageColorSpace = mConfig.colorSpace; + createInfo.imageExtent = mConfig.extent; createInfo.imageArrayLayers = 1; - createInfo.imageUsage = VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT; + createInfo.imageUsage = mConfig.usage; createInfo.imageSharingMode = VK_SHARING_MODE_EXCLUSIVE; createInfo.queueFamilyIndexCount = 0; createInfo.pQueueFamilyIndices = nullptr; @@ -310,19 +313,131 @@ namespace dawn_native { namespace vulkan { config.presentMode = kPresentModeFallbacks[modeIndex]; } + // Choose the target width or do a blit. + if (GetWidth() < surfaceInfo.capabilities.minImageExtent.width || + GetWidth() > surfaceInfo.capabilities.maxImageExtent.width || + GetHeight() < surfaceInfo.capabilities.minImageExtent.height || + GetHeight() > surfaceInfo.capabilities.maxImageExtent.height) { + config.needsBlit = true; + } else { + config.extent.width = GetWidth(); + config.extent.height = GetHeight(); + } + + // Choose the target usage or do a blit. + VkImageUsageFlags targetUsages = + VulkanImageUsage(GetUsage(), GetDevice()->GetValidInternalFormat(GetFormat())); + VkImageUsageFlags supportedUsages = surfaceInfo.capabilities.supportedUsageFlags; + if ((supportedUsages & targetUsages) != targetUsages) { + config.needsBlit = true; + } else { + config.usage = targetUsages; + config.wgpuUsage = GetUsage(); + } + + // Only support BGRA8Unorm with SRGB color space for now. + bool hasBGRA8Unorm = false; + for (const VkSurfaceFormatKHR& format : surfaceInfo.formats) { + if (format.format == VK_FORMAT_B8G8R8A8_UNORM && + format.colorSpace == VK_COLOR_SPACE_SRGB_NONLINEAR_KHR) { + hasBGRA8Unorm = true; + break; + } + } + if (!hasBGRA8Unorm) { + return DAWN_INTERNAL_ERROR( + "Vulkan swapchain must support BGRA8Unorm with SRGB colorspace"); + } + config.format = VK_FORMAT_B8G8R8A8_UNORM; + config.colorSpace = VK_COLOR_SPACE_SRGB_NONLINEAR_KHR; + config.wgpuFormat = wgpu::TextureFormat::BGRA8Unorm; + + // Choose a valid config for the swapchain texture that will receive the blit. + if (config.needsBlit) { + // Vulkan has provisions to have surfaces that adapt to the swapchain size. If that's + // the case it is very likely that the target extent works, but clamp it just in case. + // Using the target extent for the blit is better when possible so that texels don't + // get stretched. This case is exposed by having the special "-1" value in both + // dimensions of the extent. + constexpr uint32_t kSpecialValue = 0xFFFF'FFFF; + if (surfaceInfo.capabilities.currentExtent.width == kSpecialValue && + surfaceInfo.capabilities.currentExtent.height == kSpecialValue) { + // extent = clamp(targetExtent, minExtent, maxExtent) + config.extent.width = GetWidth(); + config.extent.width = + std::min(config.extent.width, surfaceInfo.capabilities.maxImageExtent.width); + config.extent.width = + std::max(config.extent.width, surfaceInfo.capabilities.minImageExtent.width); + + config.extent.height = GetHeight(); + config.extent.height = + std::min(config.extent.height, surfaceInfo.capabilities.maxImageExtent.height); + config.extent.height = + std::max(config.extent.height, surfaceInfo.capabilities.minImageExtent.height); + } else { + // If it is not an adaptable swapchain, just use the current extent for the blit + // texture. + config.extent = surfaceInfo.capabilities.currentExtent; + } + + // TODO(cwallez@chromium.org): If the swapchain image doesn't support TRANSFER_DST + // then we'll need to have a second fallback that uses a blit shader :( + if ((supportedUsages & VK_IMAGE_USAGE_TRANSFER_DST_BIT) == 0) { + return DAWN_INTERNAL_ERROR( + "Swapchain cannot fallback to a blit because of a missing " + "VK_IMAGE_USAGE_TRANSFER_DST_BIT"); + } + config.usage = VK_IMAGE_USAGE_TRANSFER_DST_BIT; + config.wgpuUsage = wgpu::TextureUsage::CopyDst; + } + return config; } MaybeError SwapChain::PresentImpl() { Device* device = ToBackend(GetDevice()); - // Transition the texture to the present usage. + CommandRecordingContext* recordingContext = device->GetPendingRecordingContext(); + + if (mConfig.needsBlit) { + // TODO ditto same as present below: eagerly transition the blit texture to CopySrc. + mBlitTexture->TransitionUsageNow(recordingContext, wgpu::TextureUsage::CopySrc, + mBlitTexture->GetAllSubresources()); + mTexture->TransitionUsageNow(recordingContext, wgpu::TextureUsage::CopyDst, + mTexture->GetAllSubresources()); + + VkImageBlit region; + region.srcSubresource.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT; + region.srcSubresource.mipLevel = 0; + region.srcSubresource.baseArrayLayer = 0; + region.srcSubresource.layerCount = 1; + region.srcOffsets[0] = {0, 0, 0}; + region.srcOffsets[1] = {static_cast(mBlitTexture->GetWidth()), + static_cast(mBlitTexture->GetHeight()), 1}; + + region.dstSubresource = region.srcSubresource; + region.dstOffsets[0] = {0, 0, 0}; + region.dstOffsets[1] = {static_cast(mTexture->GetWidth()), + static_cast(mTexture->GetHeight()), 1}; + + device->fn.CmdBlitImage(recordingContext->commandBuffer, mBlitTexture->GetHandle(), + mBlitTexture->GetCurrentLayoutForSwapChain(), + mTexture->GetHandle(), mTexture->GetCurrentLayoutForSwapChain(), + 1, ®ion, VK_FILTER_LINEAR); + + // TODO(cwallez@chromium.org): Find a way to reuse the blit texture between frames + // instead of creating a new one every time. This will involve "un-destroying" the + // texture or making the blit texture "external". + mBlitTexture->Destroy(); + mBlitTexture = nullptr; + } + // TODO(cwallez@chromium.org): Remove the need for this by eagerly transitioning the // presentable texture to present at the end of submits that use them and ideally even // folding that in the free layout transition at the end of render passes. - CommandRecordingContext* recordingContext = device->GetPendingRecordingContext(); mTexture->TransitionUsageNow(recordingContext, kPresentTextureUsage, mTexture->GetAllSubresources()); + DAWN_TRY(device->SubmitPendingCommands()); // Assuming that the present queue is the same as the graphics queue, the proper @@ -345,18 +460,28 @@ namespace dawn_native { namespace vulkan { VkResult result = VkResult::WrapUnsafe(device->fn.QueuePresentKHR(device->GetQueue(), &presentInfo)); - if (result == VK_ERROR_OUT_OF_DATE_KHR) { - // TODO reinitialize? - } else if (result == VK_ERROR_SURFACE_LOST_KHR) { - // TODO IDK what to do here, just lose the device? - } else { - DAWN_TRY(CheckVkSuccess(::VkResult(result), "QueuePresent")); - } - return {}; + switch (result) { + case VK_SUCCESS: + return {}; + + case VK_ERROR_OUT_OF_DATE_KHR: + // This present cannot be recovered. Re-initialize the VkSwapchain so that future + // presents work.. + return Initialize(this); + + // TODO(cwallez@chromium.org): Allow losing the surface at Dawn's API level? + case VK_ERROR_SURFACE_LOST_KHR: + default: + return CheckVkSuccess(::VkResult(result), "QueuePresent"); + } } ResultOrError SwapChain::GetCurrentTextureViewImpl() { + return GetCurrentTextureViewInternal(); + } + + ResultOrError SwapChain::GetCurrentTextureViewInternal(bool isReentrant) { Device* device = ToBackend(GetDevice()); // Transiently create a semaphore that will be signaled when the presentation engine is done @@ -371,36 +496,80 @@ namespace dawn_native { namespace vulkan { device->fn.CreateSemaphore(device->GetVkDevice(), &createInfo, nullptr, &*semaphore), "CreateSemaphore")); - // TODO(cwallez@chromium.org) put the semaphore on the texture so it is waited on when used - // instead of directly on the recording context? - device->GetPendingRecordingContext()->waitSemaphores.push_back(semaphore); - VkResult result = VkResult::WrapUnsafe(device->fn.AcquireNextImageKHR( device->GetVkDevice(), mSwapChain, std::numeric_limits::max(), semaphore, VkFence{}, &mLastImageIndex)); - if (result == VK_SUBOPTIMAL_KHR) { - // TODO reinitialize? - } else if (result == VK_ERROR_OUT_OF_DATE_KHR) { - // TODO reinitialize? - } else if (result == VK_ERROR_SURFACE_LOST_KHR) { - // TODO IDK what to do here, just lose the device? + + if (result == VK_SUCCESS) { + // TODO(cwallez@chromium.org) put the semaphore on the texture so it is waited on when + // used instead of directly on the recording context? + device->GetPendingRecordingContext()->waitSemaphores.push_back(semaphore); } else { - DAWN_TRY(CheckVkSuccess(::VkResult(result), "AcquireNextImage")); + // The semaphore wasn't actually used (? this is unclear in the spec). Delete it when + // we get a chance. + ToBackend(GetDevice())->GetFencedDeleter()->DeleteWhenUnused(semaphore); } - VkImage currentImage = mSwapChainImages[mLastImageIndex]; + switch (result) { + // TODO(cwallez@chromium.org): Introduce a mechanism to notify the application that + // the swapchain is in a suboptimal state? + case VK_SUBOPTIMAL_KHR: + case VK_SUCCESS: + break; - TextureDescriptor textureDesc = GetSwapChainBaseTextureDescriptor(this); + case VK_ERROR_OUT_OF_DATE_KHR: { + // Prevent infinite recursive calls to GetCurrentTextureViewInternal when the + // swapchains always return that they are out of date. + if (isReentrant) { + // TODO(cwallez@chromium.org): Allow losing the surface instead? + return DAWN_INTERNAL_ERROR( + "Wasn't able to recuperate the surface after a VK_ERROR_OUT_OF_DATE_KHR"); + } + + // Re-initialize the VkSwapchain and try getting the texture again. + DAWN_TRY(Initialize(this)); + return GetCurrentTextureViewInternal(true); + } + + // TODO(cwallez@chromium.org): Allow losing the surface at Dawn's API level? + case VK_ERROR_SURFACE_LOST_KHR: + default: + DAWN_TRY(CheckVkSuccess(::VkResult(result), "AcquireNextImage")); + } + + TextureDescriptor textureDesc; + textureDesc.size.width = mConfig.extent.width; + textureDesc.size.height = mConfig.extent.height; + textureDesc.format = mConfig.wgpuFormat; + textureDesc.usage = mConfig.wgpuUsage; + + VkImage currentImage = mSwapChainImages[mLastImageIndex]; mTexture = Texture::CreateForSwapChain(device, &textureDesc, currentImage); - return mTexture->CreateView(nullptr); + + // In the happy path we can use the swapchain image directly. + if (!mConfig.needsBlit) { + return mTexture->CreateView(nullptr); + } + + // The blit texture always perfectly matches what the user requested for the swapchain. + // We need to add the Vulkan TRANSFER_SRC flag for the vkCmdBlitImage call. + TextureDescriptor desc = GetSwapChainBaseTextureDescriptor(this); + DAWN_TRY_ASSIGN(mBlitTexture, + Texture::Create(device, &desc, VK_IMAGE_USAGE_TRANSFER_SRC_BIT)); + return mBlitTexture->CreateView(nullptr); } void SwapChain::DetachFromSurfaceImpl() { - if (mTexture.Get() != nullptr) { + if (mTexture) { mTexture->Destroy(); mTexture = nullptr; } + if (mBlitTexture) { + mBlitTexture->Destroy(); + mBlitTexture = nullptr; + } + // The swapchain images are destroyed with the swapchain. if (mSwapChain != VK_NULL_HANDLE) { ToBackend(GetDevice())->GetFencedDeleter()->DeleteWhenUnused(mSwapChain); diff --git a/src/dawn_native/vulkan/SwapChainVk.h b/src/dawn_native/vulkan/SwapChainVk.h index 009c14cf13..21b65b9ea1 100644 --- a/src/dawn_native/vulkan/SwapChainVk.h +++ b/src/dawn_native/vulkan/SwapChainVk.h @@ -55,13 +55,23 @@ namespace dawn_native { namespace vulkan { MaybeError Initialize(NewSwapChainBase* previousSwapChain); struct Config { - // Information that's passed to swapchain creation. + // Information that's passed to vulkan swapchain creation. VkPresentModeKHR presentMode; + VkExtent2D extent; + VkImageUsageFlags usage; + VkFormat format; + VkColorSpaceKHR colorSpace; - // TODO more information used to create the swapchain. - // TODO information about the blit that needs to happen. + // Redundant information but as WebGPU enums to create the wgpu::Texture that + // encapsulates the native swapchain texture. + wgpu::TextureUsage wgpuUsage; + wgpu::TextureFormat wgpuFormat; + + // Information about the blit workarounds we need to do (if any) + bool needsBlit = false; }; ResultOrError ChooseConfig(const VulkanSurfaceInfo& surfaceInfo) const; + ResultOrError GetCurrentTextureViewInternal(bool isReentrant = false); // NewSwapChainBase implementation MaybeError PresentImpl() override; @@ -75,6 +85,7 @@ namespace dawn_native { namespace vulkan { std::vector mSwapChainImages; uint32_t mLastImageIndex = 0; + Ref mBlitTexture; Ref mTexture; }; diff --git a/src/dawn_native/vulkan/TextureVk.cpp b/src/dawn_native/vulkan/TextureVk.cpp index 8de770aedd..db510f301c 100644 --- a/src/dawn_native/vulkan/TextureVk.cpp +++ b/src/dawn_native/vulkan/TextureVk.cpp @@ -455,11 +455,12 @@ namespace dawn_native { namespace vulkan { } // static - ResultOrError> Texture::Create(Device* device, - const TextureDescriptor* descriptor) { + ResultOrError> Texture::Create(Device* device, + const TextureDescriptor* descriptor, + VkImageUsageFlags extraUsages) { Ref texture = AcquireRef(new Texture(device, descriptor, TextureState::OwnedInternal)); - DAWN_TRY(texture->InitializeAsInternalTexture()); + DAWN_TRY(texture->InitializeAsInternalTexture(extraUsages)); return std::move(texture); } @@ -485,7 +486,7 @@ namespace dawn_native { namespace vulkan { return std::move(texture); } - MaybeError Texture::InitializeAsInternalTexture() { + MaybeError Texture::InitializeAsInternalTexture(VkImageUsageFlags extraUsages) { Device* device = ToBackend(GetDevice()); // Create the Vulkan image "container". We don't need to check that the format supports the @@ -499,7 +500,7 @@ namespace dawn_native { namespace vulkan { createInfo.flags = 0; createInfo.format = VulkanImageFormat(device, GetFormat().format); createInfo.tiling = VK_IMAGE_TILING_OPTIMAL; - createInfo.usage = VulkanImageUsage(GetUsage(), GetFormat()); + createInfo.usage = VulkanImageUsage(GetUsage(), GetFormat()) | extraUsages; createInfo.sharingMode = VK_SHARING_MODE_EXCLUSIVE; createInfo.queueFamilyIndexCount = 0; createInfo.pQueueFamilyIndices = nullptr; @@ -1094,6 +1095,11 @@ namespace dawn_native { namespace vulkan { } } + VkImageLayout Texture::GetCurrentLayoutForSwapChain() const { + ASSERT(mSubresourceLastUsages.size() == 1); + return VulkanImageLayout(mSubresourceLastUsages[0], GetFormat()); + } + // static ResultOrError TextureView::Create(TextureBase* texture, const TextureViewDescriptor* descriptor) { diff --git a/src/dawn_native/vulkan/TextureVk.h b/src/dawn_native/vulkan/TextureVk.h index 1eafb0e3a8..801eebcaee 100644 --- a/src/dawn_native/vulkan/TextureVk.h +++ b/src/dawn_native/vulkan/TextureVk.h @@ -41,8 +41,9 @@ namespace dawn_native { namespace vulkan { class Texture final : public TextureBase { public: // Used to create a regular texture from a descriptor. - static ResultOrError> Create(Device* device, - const TextureDescriptor* descriptor); + static ResultOrError> Create(Device* device, + const TextureDescriptor* descriptor, + VkImageUsageFlags extraUsages = 0); // Creates a texture and initializes it with a VkImage that references an external memory // object. Before the texture can be used, the VkDeviceMemory associated with the external @@ -84,6 +85,8 @@ namespace dawn_native { namespace vulkan { void EnsureSubresourceContentInitialized(CommandRecordingContext* recordingContext, const SubresourceRange& range); + VkImageLayout GetCurrentLayoutForSwapChain() const; + // Binds externally allocated memory to the VkImage and on success, takes ownership of // semaphores. MaybeError BindExternalMemory(const ExternalImageDescriptorVk* descriptor, @@ -100,7 +103,7 @@ namespace dawn_native { namespace vulkan { ~Texture() override; using TextureBase::TextureBase; - MaybeError InitializeAsInternalTexture(); + MaybeError InitializeAsInternalTexture(VkImageUsageFlags extraUsages); MaybeError InitializeFromExternal(const ExternalImageDescriptorVk* descriptor, external_memory::Service* externalMemoryService); void InitializeForSwapChain(VkImage nativeImage); diff --git a/src/tests/end2end/SwapChainTests.cpp b/src/tests/end2end/SwapChainTests.cpp index bfa10a1ca6..c81307a03a 100644 --- a/src/tests/end2end/SwapChainTests.cpp +++ b/src/tests/end2end/SwapChainTests.cpp @@ -30,7 +30,12 @@ class SwapChainTests : public DawnTest { glfwSetErrorCallback([](int code, const char* message) { dawn::ErrorLog() << "GLFW error " << code << " " << message; }); - glfwInit(); + + // GLFW can fail to start in headless environments, in which SwapChainTests are + // inapplicable. Skip this cases without producing a test failure. + if (glfwInit() == GLFW_FALSE) { + GTEST_SKIP(); + } // The SwapChainTests don't create OpenGL contexts so we don't need to call // SetupGLFWWindowHintsForBackend. Set GLFW_NO_API anyway to avoid GLFW bringing up a GL @@ -131,6 +136,10 @@ TEST_P(SwapChainTests, DestroySurfaceAfterGet) { // Test switching between present modes. TEST_P(SwapChainTests, SwitchPresentMode) { + // For unclear reasons recreating the swapchain produces a debug report warning on NVIDIA and + // makes the test fail. + DAWN_SKIP_TEST_IF(IsVulkan() && IsNvidia()); + constexpr wgpu::PresentMode kAllPresentModes[] = { wgpu::PresentMode::Immediate, wgpu::PresentMode::Fifo, @@ -156,6 +165,10 @@ TEST_P(SwapChainTests, SwitchPresentMode) { // Test resizing the swapchain and without resizing the window. TEST_P(SwapChainTests, ResizingSwapChainOnly) { + // For unclear reasons recreating the swapchain produces a debug report warning on NVIDIA and + // makes the test fail. + DAWN_SKIP_TEST_IF(IsVulkan() && IsNvidia()); + for (int i = 0; i < 10; i++) { wgpu::SwapChainDescriptor desc = baseDescriptor; desc.width += i * 10; @@ -182,6 +195,10 @@ TEST_P(SwapChainTests, ResizingWindowOnly) { // Test resizing both the window and the swapchain at the same time. TEST_P(SwapChainTests, ResizingWindowAndSwapChain) { + // For unclear reasons recreating the swapchain produces a debug report warning on NVIDIA and + // makes the test fail. + DAWN_SKIP_TEST_IF(IsVulkan() && IsNvidia()); + for (int i = 0; i < 10; i++) { glfwSetWindowSize(window, 400 - 10 * i, 400 + 10 * i); glfwPollEvents(); @@ -202,7 +219,11 @@ TEST_P(SwapChainTests, ResizingWindowAndSwapChain) { // Test switching devices on the same adapter. TEST_P(SwapChainTests, SwitchingDevice) { - wgpu::Device device2 = GetAdapter().CreateDevice(); + // For unclear reasons recreating the swapchain produces a debug report warning on NVIDIA and + // makes the test fail. + DAWN_SKIP_TEST_IF(IsVulkan() && IsNvidia()); + + wgpu::Device device2 = wgpu::Device::Acquire(GetAdapter().CreateDevice()); for (int i = 0; i < 3; i++) { wgpu::Device deviceToUse; @@ -218,4 +239,4 @@ TEST_P(SwapChainTests, SwitchingDevice) { } } -DAWN_INSTANTIATE_TEST(SwapChainTests, MetalBackend()); +DAWN_INSTANTIATE_TEST(SwapChainTests, MetalBackend(), VulkanBackend());