TextureView defaults and validation updates

Spec PR: https://github.com/gpuweb/gpuweb/pull/2687

Fixed: dawn:682
Bug: dawn:1276
Change-Id: Ifa8f94fa4c1a27fb40d0ccfb9f032ca4a28ed24e
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/84520
Reviewed-by: Loko Kung <lokokung@google.com>
Reviewed-by: Brandon Jones <bajones@chromium.org>
Commit-Queue: Austin Eng <enga@chromium.org>

src/dawn/native/Texture.cpp

@@ -49,10 +49,6 @@ namespace dawn::native {
                                             wgpu::TextureAspect aspect) {
             const Format& format = texture->GetFormat();
 
-            if (format.format == viewFormat.format) {
-                return {};
-            }
-
             if (aspect != wgpu::TextureAspect::All) {
                 wgpu::TextureFormat aspectFormat = format.GetAspectInfo(aspect).format;
                 if (viewFormat.format == aspectFormat) {
@@ -64,6 +60,10 @@ namespace dawn::native {
                 }
             }
 
+            if (format.format == viewFormat.format) {
+                return {};
+            }
+
             const FormatSet& compatibleViewFormats = texture->GetViewFormats();
             if (compatibleViewFormats[viewFormat]) {
                 // Validation of this list is done on texture creation, so we don't need to
@@ -471,8 +471,13 @@ namespace dawn::native {
         }
 
         if (desc.format == wgpu::TextureFormat::Undefined) {
-            // TODO(dawn:682): Use GetAspectInfo(aspect).
+            const Format& format = texture->GetFormat();
-            desc.format = texture->GetFormat().format;
+            Aspect aspects = SelectFormatAspects(format, desc.aspect);
+            if (HasOneBit(aspects)) {
+                desc.format = format.GetAspectInfo(aspects).format;
+            } else {
+                desc.format = format.format;
+            }
         }
         if (desc.arrayLayerCount == wgpu::kArrayLayerCountUndefined) {
             switch (desc.dimension) {

src/dawn/native/d3d12/TextureD3D12.cpp

@@ -1170,13 +1170,12 @@ namespace dawn::native::d3d12 {
         mSrvDesc.Format = D3D12TextureFormat(descriptor->format);
         mSrvDesc.Shader4ComponentMapping = D3D12_DEFAULT_SHADER_4_COMPONENT_MAPPING;
 
-        // TODO(enga): This will need to be much more nuanced when WebGPU has
-        // texture view compatibility rules.
         UINT planeSlice = 0;
-        if (GetFormat().HasDepthOrStencil()) {
+        const Format& textureFormat = texture->GetFormat();
+        if (textureFormat.HasDepthOrStencil()) {
             // Configure the SRV descriptor to reinterpret the texture allocated as
             // TYPELESS as a single-plane shader-accessible view.
-            switch (descriptor->format) {
+            switch (textureFormat.format) {
                 case wgpu::TextureFormat::Depth32Float:
                 case wgpu::TextureFormat::Depth24Plus:
                     mSrvDesc.Format = DXGI_FORMAT_R32_FLOAT;
@@ -1184,28 +1183,22 @@ namespace dawn::native::d3d12 {
                 case wgpu::TextureFormat::Depth16Unorm:
                     mSrvDesc.Format = DXGI_FORMAT_R16_UNORM;
                     break;
-                case wgpu::TextureFormat::Stencil8: {
+                case wgpu::TextureFormat::Stencil8:
-                    // Stencil8 is always backed by a DXGI_FORMAT_R24G8_TYPELESS texture in D3D12,
+                case wgpu::TextureFormat::Depth24UnormStencil8: {
-                    // so always treat it as if the StencilOnly aspect of a Depth24UnormStencil8 was
+                    Aspect aspects = SelectFormatAspects(textureFormat, descriptor->aspect);
-                    // selected.
+                    ASSERT(aspects != Aspect::None);
-                    planeSlice = 1;
+                    if (!HasZeroOrOneBits(aspects)) {
-                    mSrvDesc.Format = DXGI_FORMAT_X24_TYPELESS_G8_UINT;
+                        // A single aspect is not selected. The texture view must not be
-                    // Stencil is accessed using the .g component in the shader.
+                        // sampled.
-                    // Map it to the zeroth component to match other APIs.
+                        mSrvDesc.Format = DXGI_FORMAT_UNKNOWN;
-                    mSrvDesc.Shader4ComponentMapping = D3D12_ENCODE_SHADER_4_COMPONENT_MAPPING(
+                        break;
-                        D3D12_SHADER_COMPONENT_MAPPING_FROM_MEMORY_COMPONENT_1,
+                    }
-                        D3D12_SHADER_COMPONENT_MAPPING_FORCE_VALUE_0,
+                    switch (aspects) {
-                        D3D12_SHADER_COMPONENT_MAPPING_FORCE_VALUE_0,
+                        case Aspect::Depth:
-                        D3D12_SHADER_COMPONENT_MAPPING_FORCE_VALUE_1);
-                    break;
-                }
-                case wgpu::TextureFormat::Depth24UnormStencil8:
-                    switch (descriptor->aspect) {
-                        case wgpu::TextureAspect::DepthOnly:
                             planeSlice = 0;
                             mSrvDesc.Format = DXGI_FORMAT_R24_UNORM_X8_TYPELESS;
                             break;
-                        case wgpu::TextureAspect::StencilOnly:
+                        case Aspect::Stencil:
                             planeSlice = 1;
                             mSrvDesc.Format = DXGI_FORMAT_X24_TYPELESS_G8_UINT;
                             // Stencil is accessed using the .g component in the shader.
@@ -1217,27 +1210,28 @@ namespace dawn::native::d3d12 {
                                     D3D12_SHADER_COMPONENT_MAPPING_FORCE_VALUE_0,
                                     D3D12_SHADER_COMPONENT_MAPPING_FORCE_VALUE_1);
                             break;
-                        case wgpu::TextureAspect::All:
+                        default:
-                            // A single aspect is not selected. The texture view must not be
-                            // sampled.
-                            mSrvDesc.Format = DXGI_FORMAT_UNKNOWN;
-                            break;
-
-                        // Depth formats cannot use plane aspects.
-                        case wgpu::TextureAspect::Plane0Only:
-                        case wgpu::TextureAspect::Plane1Only:
                             UNREACHABLE();
                             break;
                     }
                     break;
+                }
                 case wgpu::TextureFormat::Depth24PlusStencil8:
-                case wgpu::TextureFormat::Depth32FloatStencil8:
+                case wgpu::TextureFormat::Depth32FloatStencil8: {
-                    switch (descriptor->aspect) {
+                    Aspect aspects = SelectFormatAspects(textureFormat, descriptor->aspect);
-                        case wgpu::TextureAspect::DepthOnly:
+                    ASSERT(aspects != Aspect::None);
+                    if (!HasZeroOrOneBits(aspects)) {
+                        // A single aspect is not selected. The texture view must not be
+                        // sampled.
+                        mSrvDesc.Format = DXGI_FORMAT_UNKNOWN;
+                        break;
+                    }
+                    switch (aspects) {
+                        case Aspect::Depth:
                             planeSlice = 0;
                             mSrvDesc.Format = DXGI_FORMAT_R32_FLOAT_X8X24_TYPELESS;
                             break;
-                        case wgpu::TextureAspect::StencilOnly:
+                        case Aspect::Stencil:
                             planeSlice = 1;
                             mSrvDesc.Format = DXGI_FORMAT_X32_TYPELESS_G8X24_UINT;
                             // Stencil is accessed using the .g component in the shader.
@@ -1249,19 +1243,12 @@ namespace dawn::native::d3d12 {
                                     D3D12_SHADER_COMPONENT_MAPPING_FORCE_VALUE_0,
                                     D3D12_SHADER_COMPONENT_MAPPING_FORCE_VALUE_1);
                             break;
-                        case wgpu::TextureAspect::All:
+                        default:
-                            // A single aspect is not selected. The texture view must not be
-                            // sampled.
-                            mSrvDesc.Format = DXGI_FORMAT_UNKNOWN;
-                            break;
-
-                        // Depth formats cannot use plane aspects.
-                        case wgpu::TextureAspect::Plane0Only:
-                        case wgpu::TextureAspect::Plane1Only:
                             UNREACHABLE();
                             break;
                     }
                     break;
+                }
                 default:
                     UNREACHABLE();
                     break;

src/dawn/native/metal/TextureMTL.mm

@@ -86,7 +86,10 @@ namespace dawn::native::metal {
 
         bool RequiresCreatingNewTextureView(const TextureBase* texture,
                                             const TextureViewDescriptor* textureViewDescriptor) {
-            if (texture->GetFormat().format != textureViewDescriptor->format) {
+            if (texture->GetFormat().format != textureViewDescriptor->format &&
+                !texture->GetFormat().HasDepthOrStencil()) {
+                // Color format reinterpretation required. Note: Depth/stencil formats don't support
+                // reinterpretation.
                 return true;
             }
 
@@ -1059,16 +1062,17 @@ namespace dawn::native::metal {
                     "Failed to create MTLTexture view for external texture.");
             }
         } else {
-            MTLPixelFormat format = MetalPixelFormat(descriptor->format);
+            MTLPixelFormat viewFormat = MetalPixelFormat(descriptor->format);
+            MTLPixelFormat textureFormat = MetalPixelFormat(GetTexture()->GetFormat().format);
             if (descriptor->aspect == wgpu::TextureAspect::StencilOnly &&
-                format != MTLPixelFormatStencil8) {
+                textureFormat != MTLPixelFormatStencil8) {
                 if (@available(macOS 10.12, iOS 10.0, *)) {
-                    if (format == MTLPixelFormatDepth32Float_Stencil8) {
+                    if (textureFormat == MTLPixelFormatDepth32Float_Stencil8) {
-                        format = MTLPixelFormatX32_Stencil8;
+                        viewFormat = MTLPixelFormatX32_Stencil8;
                     }
 #if defined(DAWN_PLATFORM_MACOS)
-                    else if (format == MTLPixelFormatDepth24Unorm_Stencil8) {
+                    else if (textureFormat == MTLPixelFormatDepth24Unorm_Stencil8) {
-                        format = MTLPixelFormatX24_Stencil8;
+                        viewFormat = MTLPixelFormatX24_Stencil8;
                     }
 #endif
                     else {
@@ -1082,6 +1086,11 @@ namespace dawn::native::metal {
                         DAWN_DEVICE_LOST_ERROR("Cannot create stencil-only texture view of "
                                                "combined depth/stencil format."));
                 }
+            } else if (GetTexture()->GetFormat().HasDepth() &&
+                       GetTexture()->GetFormat().HasStencil()) {
+                // Depth-only views for depth/stencil textures in Metal simply use the original
+                // texture's format.
+                viewFormat = textureFormat;
             }
 
             MTLTextureType textureViewType =
@@ -1091,7 +1100,7 @@ namespace dawn::native::metal {
                 NSMakeRange(descriptor->baseArrayLayer, descriptor->arrayLayerCount);
 
             mMtlTextureView =
-                AcquireNSPRef([mtlTexture newTextureViewWithPixelFormat:format
+                AcquireNSPRef([mtlTexture newTextureViewWithPixelFormat:viewFormat
                                                             textureType:textureViewType
                                                                  levels:mipLevelRange
                                                                  slices:arrayLayerRange]);

src/dawn/native/opengl/TextureGL.cpp

@@ -95,7 +95,10 @@ namespace dawn::native::opengl {
 
         bool RequiresCreatingNewTextureView(const TextureBase* texture,
                                             const TextureViewDescriptor* textureViewDescriptor) {
-            if (texture->GetFormat().format != textureViewDescriptor->format) {
+            if (texture->GetFormat().format != textureViewDescriptor->format &&
+                !texture->GetFormat().HasDepthOrStencil()) {
+                // Color format reinterpretation required. Note: Depth/stencil formats don't support
+                // reinterpretation.
                 return true;
             }
 
@@ -561,7 +564,14 @@ namespace dawn::native::opengl {
             const OpenGLFunctions& gl = ToBackend(GetDevice())->gl;
             mHandle = GenTexture(gl);
             const Texture* textureGL = ToBackend(texture);
-            const GLFormat& glFormat = ToBackend(GetDevice())->GetGLFormat(GetFormat());
+
+            const Format& textureFormat = GetTexture()->GetFormat();
+            // Depth/stencil don't support reinterpretation, and the aspect is specified at
+            // bind time. In that case, we use the base texture format.
+            const GLFormat& glFormat = textureFormat.HasDepthOrStencil()
+                                           ? ToBackend(GetDevice())->GetGLFormat(textureFormat)
+                                           : ToBackend(GetDevice())->GetGLFormat(GetFormat());
+
             gl.TextureView(mHandle, mTarget, textureGL->GetHandle(), glFormat.internalFormat,
                            descriptor->baseMipLevel, descriptor->mipLevelCount,
                            descriptor->baseArrayLayer, descriptor->arrayLayerCount);

src/dawn/native/vulkan/TextureVk.cpp

@@ -1373,7 +1373,20 @@ namespace dawn::native::vulkan {
         createInfo.flags = 0;
         createInfo.image = ToBackend(GetTexture())->GetHandle();
         createInfo.viewType = VulkanImageViewType(descriptor->dimension);
-        createInfo.format = VulkanImageFormat(device, descriptor->format);
+
+        const Format& textureFormat = GetTexture()->GetFormat();
+        if (textureFormat.HasStencil() &&
+            (textureFormat.HasDepth() || !device->IsToggleEnabled(Toggle::VulkanUseS8))) {
+            // Unlike multi-planar formats, depth-stencil formats have multiple aspects but are not
+            // created with VK_IMAGE_CREATE_MUTABLE_FORMAT_BIT.
+            // https://www.khronos.org/registry/vulkan/specs/1.3-extensions/man/html/VkImageViewCreateInfo.html#VUID-VkImageViewCreateInfo-image-01762
+            // Without, VK_IMAGE_CREATE_MUTABLE_FORMAT_BIT, the view format must match the texture
+            // format.
+            createInfo.format = VulkanImageFormat(device, textureFormat.format);
+        } else {
+            createInfo.format = VulkanImageFormat(device, descriptor->format);
+        }
+
         createInfo.components = VkComponentMapping{VK_COMPONENT_SWIZZLE_R, VK_COMPONENT_SWIZZLE_G,
                                                    VK_COMPONENT_SWIZZLE_B, VK_COMPONENT_SWIZZLE_A};
 

src/dawn/tests/unittests/validation/TextureViewValidationTests.cpp

@@ -709,6 +709,26 @@ namespace {
             ASSERT_DEVICE_ERROR(texture.CreateView(&viewDesc));
         }
 
+        // It is invalid to create a texture view with a combined depth-stencil format if only
+        // the depth aspect is selected.
+        {
+            textureDesc.format = wgpu::TextureFormat::Depth24PlusStencil8;
+            viewDesc.format = wgpu::TextureFormat::Depth24PlusStencil8;
+            viewDesc.aspect = wgpu::TextureAspect::DepthOnly;
+            wgpu::Texture texture = device.CreateTexture(&textureDesc);
+            ASSERT_DEVICE_ERROR(texture.CreateView(&viewDesc));
+        }
+
+        // It is invalid to create a texture view with a combined depth-stencil format if only
+        // the stencil aspect is selected.
+        {
+            textureDesc.format = wgpu::TextureFormat::Depth24PlusStencil8;
+            viewDesc.format = wgpu::TextureFormat::Depth24PlusStencil8;
+            viewDesc.aspect = wgpu::TextureAspect::StencilOnly;
+            wgpu::Texture texture = device.CreateTexture(&textureDesc);
+            ASSERT_DEVICE_ERROR(texture.CreateView(&viewDesc));
+        }
+
         // It is valid to create a texture view with a depth format of a depth-stencil texture
         // if the depth only aspect is selected.
         {