From 9c460507901791e7d6e1e6a1b0118f269e50f37c Mon Sep 17 00:00:00 2001 From: William Candillon Date: Sat, 18 Jul 2026 20:08:04 +0200 Subject: [PATCH 1/2] =?UTF-8?q?fix(=F0=9F=90=9B):=20Route=20surfaces=20con?= =?UTF-8?q?figured=20with=20viewFormats=20through=20the=20blit=20path?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added check for viewFormats in swapchain image usage validation.The Vulkan swapchain creates its images without VK_IMAGE_CREATE_MUTABLE_FORMAT_BIT and wraps them in a texture descriptor that omits the configuration's viewFormats, so the first GetCurrentTexture() fails the viewFormats consistency DAWN_CHECK and aborts. Any non-empty viewFormats now sets needsBlit, like an unsupported extent or usage: the user-facing texture becomes the intermediate blit texture, a regular texture created from the full descriptor, which supports reinterpretation. Metal already builds the swapchain texture from the full descriptor and is unaffected. Adds a SurfaceTests end2end case; the file previously only exercised viewFormatCount = 0, which is why this went unnoticed. Change-Id: If09df3749bb4de9c9ed011bf924141ea1fbfe15b --- src/dawn/native/vulkan/SwapChainVk.cpp | 5 ++++- src/dawn/tests/end2end/SurfaceTests.cpp | 28 +++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/dawn/native/vulkan/SwapChainVk.cpp b/src/dawn/native/vulkan/SwapChainVk.cpp index 7aeb73bc475..e2357fa3fc5 100644 --- a/src/dawn/native/vulkan/SwapChainVk.cpp +++ b/src/dawn/native/vulkan/SwapChainVk.cpp @@ -272,7 +272,10 @@ ResultOrError SwapChain::ChooseConfig( VkImageUsageFlags targetUsages = VulkanImageUsage(GetDevice(), GetUsage(), GetDevice()->GetValidInternalFormat(GetFormat())); VkImageUsageFlags supportedUsages = surfaceInfo.capabilities.supportedUsageFlags; - if (!IsSubset(targetUsages, supportedUsages)) { + // The swapchain images are also unable to satisfy viewFormats: they are + // created without VK_IMAGE_CREATE_MUTABLE_FORMAT_BIT, so they cannot be + // reinterpreted. The blit texture is a regular texture and can. + if (!IsSubset(targetUsages, supportedUsages) || !GetViewFormats().empty()) { config.needsBlit = true; } else { config.usage = targetUsages; diff --git a/src/dawn/tests/end2end/SurfaceTests.cpp b/src/dawn/tests/end2end/SurfaceTests.cpp index 0fbeeaf0249..f153bad8f0a 100644 --- a/src/dawn/tests/end2end/SurfaceTests.cpp +++ b/src/dawn/tests/end2end/SurfaceTests.cpp @@ -719,6 +719,34 @@ TEST_P(SurfaceTests, Storage) { ASSERT_EQ(wgpu::Status::Success, surface.Present()); } +// Test acquiring a texture from a surface configured with viewFormats. +TEST_P(SurfaceTests, ConfigureWithViewFormats) { + wgpu::Surface surface = CreateTestSurface(); + wgpu::SurfaceConfiguration config = GetPreferredConfiguration(surface); + + // Reinterpretation between a format and its srgb counterpart is always + // allowed; pick the counterpart of whatever the surface prefers. + wgpu::TextureFormat viewFormat; + switch (config.format) { + case wgpu::TextureFormat::BGRA8Unorm: + viewFormat = wgpu::TextureFormat::BGRA8UnormSrgb; + break; + case wgpu::TextureFormat::RGBA8Unorm: + viewFormat = wgpu::TextureFormat::RGBA8UnormSrgb; + break; + default: + GTEST_SKIP() << "Preferred surface format has no srgb counterpart"; + } + config.viewFormatCount = 1; + config.viewFormats = &viewFormat; + surface.Configure(&config); + + wgpu::SurfaceTexture surfaceTexture; + surface.GetCurrentTexture(&surfaceTexture); // aborts on Vulkan before the fix + ClearTexture(surfaceTexture.texture, {1.0, 0.0, 0.0, 1.0}); + surface.Present(); +} + // TODO(crbug.com/465183957): Implement swap chain for WebGPUBackend. DAWN_INSTANTIATE_TEST(SurfaceTests, D3D11Backend(), From 08b1d5c75c459244d46148dfe5e2aa5ba38c1749 Mon Sep 17 00:00:00 2001 From: William Candillon Date: Sun, 19 Jul 2026 14:29:59 +0200 Subject: [PATCH 2/2] Support VK_KHR_swapchain_mutable_format When the device supports VK_KHR_swapchain_mutable_format, a surface configured with viewFormats no longer needs the blit fallback: the swapchain is created with VK_SWAPCHAIN_CREATE_MUTABLE_FORMAT_BIT_KHR and a VkImageFormatListCreateInfo listing the base format and the viewFormats, and the wrapped texture descriptor carries the viewFormats so views can reinterpret the swapchain images directly. The SurfaceTests case is extended to render through the reinterpreted srgb view instead of only acquiring the texture. Change-Id: If6557d1600bb45f18c4889c794b924a8fdc49fe9 --- src/dawn/native/vulkan/SwapChainVk.cpp | 45 ++++++++++++++++----- src/dawn/native/vulkan/SwapChainVk.h | 4 ++ src/dawn/native/vulkan/VulkanExtensions.cpp | 7 ++++ src/dawn/native/vulkan/VulkanExtensions.h | 1 + src/dawn/tests/end2end/SurfaceTests.cpp | 30 +++++++++++++- 5 files changed, 77 insertions(+), 10 deletions(-) diff --git a/src/dawn/native/vulkan/SwapChainVk.cpp b/src/dawn/native/vulkan/SwapChainVk.cpp index e2357fa3fc5..6312db22b39 100644 --- a/src/dawn/native/vulkan/SwapChainVk.cpp +++ b/src/dawn/native/vulkan/SwapChainVk.cpp @@ -195,6 +195,25 @@ MaybeError SwapChain::Initialize(SwapChainBase* previousSwapChain) { createInfo.clipped = VK_FALSE; createInfo.oldSwapchain = previousVkSwapChain; + // Create the swapchain images with VK_IMAGE_CREATE_MUTABLE_FORMAT_BIT so they can be + // reinterpreted to the configuration's viewFormats. VK_KHR_swapchain_mutable_format requires + // the full list of formats to be provided, including the image format itself. + VkImageFormatListCreateInfo imageFormatListInfo; + std::vector viewFormats; + if (!mConfig.wgpuViewFormats.empty()) { + DAWN_ASSERT(device->GetDeviceInfo().HasExt(DeviceExt::SwapchainMutableFormat)); + createInfo.flags |= VK_SWAPCHAIN_CREATE_MUTABLE_FORMAT_BIT_KHR; + viewFormats.push_back(mConfig.format); + for (wgpu::TextureFormat viewFormat : mConfig.wgpuViewFormats) { + viewFormats.push_back(VulkanImageFormat(device, viewFormat)); + } + imageFormatListInfo.sType = VK_STRUCTURE_TYPE_IMAGE_FORMAT_LIST_CREATE_INFO; + imageFormatListInfo.pNext = nullptr; + imageFormatListInfo.viewFormatCount = static_cast(viewFormats.size()); + imageFormatListInfo.pViewFormats = viewFormats.data(); + createInfo.pNext = &imageFormatListInfo; + } + DAWN_TRY(CheckVkSuccess( device->fn.CreateSwapchainKHR(device->GetVkDevice(), &createInfo, nullptr, &*mSwapChain), "CreateSwapChain")); @@ -272,14 +291,20 @@ ResultOrError SwapChain::ChooseConfig( VkImageUsageFlags targetUsages = VulkanImageUsage(GetDevice(), GetUsage(), GetDevice()->GetValidInternalFormat(GetFormat())); VkImageUsageFlags supportedUsages = surfaceInfo.capabilities.supportedUsageFlags; - // The swapchain images are also unable to satisfy viewFormats: they are - // created without VK_IMAGE_CREATE_MUTABLE_FORMAT_BIT, so they cannot be - // reinterpreted. The blit texture is a regular texture and can. - if (!IsSubset(targetUsages, supportedUsages) || !GetViewFormats().empty()) { - config.needsBlit = true; - } else { + // The swapchain images support the configuration's viewFormats only if they are created + // with VK_IMAGE_CREATE_MUTABLE_FORMAT_BIT, which requires VK_KHR_swapchain_mutable_format. + // Otherwise the blit texture, a regular texture, is used to support them. + const bool viewFormatsSupported = + GetViewFormats().empty() || + ToBackend(GetDevice())->GetDeviceInfo().HasExt(DeviceExt::SwapchainMutableFormat); + if (IsSubset(targetUsages, supportedUsages) && viewFormatsSupported) { config.usage = targetUsages; config.wgpuUsage = GetUsage(); + // The swapchain will be created with VK_SWAPCHAIN_CREATE_MUTABLE_FORMAT_BIT_KHR so the + // images can be reinterpreted to these formats. + config.wgpuViewFormats = GetViewFormats(); + } else { + config.needsBlit = true; } // Only support BGRA8Unorm (and RGBA8Unorm on android) with SRGB color space for now. @@ -548,12 +573,13 @@ ResultOrError SwapChain::GetCurrentTextureInternal(bool is } lastImage.lastAcquireDoneFence = std::move(acquireFence); - // Wait on the previous fence and destroy it. + // Wrap the swapchain texture. TextureDescriptor textureDesc; textureDesc.size.width = mConfig.extent.width; textureDesc.size.height = mConfig.extent.height; textureDesc.format = mConfig.wgpuFormat; textureDesc.usage = mConfig.wgpuUsage; + textureDesc.viewFormats = mConfig.wgpuViewFormats; mTexture = SwapChainTexture::Create(device, Unpack(&textureDesc), lastImage.image); @@ -563,8 +589,9 @@ ResultOrError SwapChain::GetCurrentTextureInternal(bool is return swapChainTextureInfo; } - // 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. + // The blit texture always perfectly matches what the user requested for the swapchain, + // including the viewFormats which GetSwapChainBaseTextureDescriptor() carries over. We need + // to add the Vulkan TRANSFER_SRC flag for the vkCmdBlitImage call. TextureDescriptor desc = GetSwapChainBaseTextureDescriptor(this); DAWN_TRY_ASSIGN(mBlitTexture, InternalTexture::Create(device, Unpack(&desc), VK_IMAGE_USAGE_TRANSFER_SRC_BIT)); diff --git a/src/dawn/native/vulkan/SwapChainVk.h b/src/dawn/native/vulkan/SwapChainVk.h index 3dc1106ec76..49cfcf8a5a8 100644 --- a/src/dawn/native/vulkan/SwapChainVk.h +++ b/src/dawn/native/vulkan/SwapChainVk.h @@ -71,6 +71,10 @@ class SwapChain : public SwapChainBase { // encapsulates the native swapchain texture. wgpu::TextureUsage wgpuUsage; wgpu::TextureFormat wgpuFormat; + // When non-empty, the swapchain is created with + // VK_SWAPCHAIN_CREATE_MUTABLE_FORMAT_BIT_KHR so its images can be reinterpreted to + // these formats, and the wrapped texture exposes them as its viewFormats. + std::vector wgpuViewFormats; // Information about the blit workarounds we need to do (if any) bool needsBlit = false; diff --git a/src/dawn/native/vulkan/VulkanExtensions.cpp b/src/dawn/native/vulkan/VulkanExtensions.cpp index 8ce610dc4f4..9f232221c6c 100644 --- a/src/dawn/native/vulkan/VulkanExtensions.cpp +++ b/src/dawn/native/vulkan/VulkanExtensions.cpp @@ -149,6 +149,7 @@ static constexpr std::array sDeviceExtInfos{{ {DeviceExt::DepthClipEnable, "VK_EXT_depth_clip_enable"}, {DeviceExt::ImageDrmFormatModifier, "VK_EXT_image_drm_format_modifier"}, {DeviceExt::Swapchain, "VK_KHR_swapchain"}, + {DeviceExt::SwapchainMutableFormat, "VK_KHR_swapchain_mutable_format"}, {DeviceExt::QueueFamilyForeign, "VK_EXT_queue_family_foreign"}, {DeviceExt::Robustness2, "VK_EXT_robustness2"}, {DeviceExt::DisplayTiming, "VK_GOOGLE_display_timing"}, @@ -253,6 +254,12 @@ DeviceExtSet EnsureDependencies(const DeviceExtSet& advertisedExts, hasDependencies = instanceExts[InstanceExt::Surface]; break; + // Also requires VK_KHR_maintenance2 which is core in Vulkan 1.1. + case DeviceExt::SwapchainMutableFormat: + hasDependencies = + HasDep(DeviceExt::Swapchain) && HasDep(DeviceExt::ImageFormatList); + break; + case DeviceExt::ExternalMemoryAndroidHardwareBuffer: hasDependencies = HasDep(DeviceExt::QueueFamilyForeign); break; diff --git a/src/dawn/native/vulkan/VulkanExtensions.h b/src/dawn/native/vulkan/VulkanExtensions.h index 78d79a4c644..830782263df 100644 --- a/src/dawn/native/vulkan/VulkanExtensions.h +++ b/src/dawn/native/vulkan/VulkanExtensions.h @@ -108,6 +108,7 @@ enum class DeviceExt : uint32_t { DepthClipEnable, ImageDrmFormatModifier, Swapchain, + SwapchainMutableFormat, QueueFamilyForeign, Robustness2, DisplayTiming, diff --git a/src/dawn/tests/end2end/SurfaceTests.cpp b/src/dawn/tests/end2end/SurfaceTests.cpp index f153bad8f0a..d2b81fe5fe4 100644 --- a/src/dawn/tests/end2end/SurfaceTests.cpp +++ b/src/dawn/tests/end2end/SurfaceTests.cpp @@ -722,6 +722,8 @@ TEST_P(SurfaceTests, Storage) { // Test acquiring a texture from a surface configured with viewFormats. TEST_P(SurfaceTests, ConfigureWithViewFormats) { wgpu::Surface surface = CreateTestSurface(); + wgpu::SurfaceCapabilities caps; + surface.GetCapabilities(adapter, &caps); wgpu::SurfaceConfiguration config = GetPreferredConfiguration(surface); // Reinterpretation between a format and its srgb counterpart is always @@ -739,11 +741,37 @@ TEST_P(SurfaceTests, ConfigureWithViewFormats) { } config.viewFormatCount = 1; config.viewFormats = &viewFormat; + // When supported, also request CopySrc so the reinterpreted values can be read back. + if (caps.usages & wgpu::TextureUsage::CopySrc) { + config.usage |= wgpu::TextureUsage::CopySrc; + } surface.Configure(&config); wgpu::SurfaceTexture surfaceTexture; surface.GetCurrentTexture(&surfaceTexture); // aborts on Vulkan before the fix - ClearTexture(surfaceTexture.texture, {1.0, 0.0, 0.0, 1.0}); + + // Clear through a view using the reinterpreted format to check the texture really + // supports its viewFormats. + wgpu::TextureViewDescriptor viewDesc; + viewDesc.format = viewFormat; + utils::ComboRenderPassDescriptor renderPassDesc({surfaceTexture.texture.CreateView(&viewDesc)}); + renderPassDesc.cColorAttachments[0].loadOp = wgpu::LoadOp::Clear; + renderPassDesc.cColorAttachments[0].clearValue = {0.5, 0.5, 0.5, 1.0}; + + wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); + wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPassDesc); + pass.End(); + wgpu::CommandBuffer commands = encoder.Finish(); + queue.Submit(1, &commands); + + if (surfaceTexture.texture.GetUsage() & wgpu::TextureUsage::CopySrc) { + // The sRGB view encodes the linear 0.5 clear value to ~0.735 on store, so the raw + // non-sRGB pixels read back as ~187.5 instead of 128 if the reinterpretation took + // effect. A gray value keeps the check independent of the BGRA/RGBA channel order. + EXPECT_PIXEL_RGBA8_BETWEEN(utils::RGBA8(187, 187, 187, 255), + utils::RGBA8(188, 188, 188, 255), surfaceTexture.texture, 0, 0); + } + surface.Present(); }