Bug 238710 – [WebGPU] Implement Texture view format compatibility

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGPU

[Myles C. Maxfield]

.

[Myles C. Maxfield]

Created attachment 456500 [details]
Patch

[Myles C. Maxfield]

Created attachment 456504 [details]
Patch

[Dan Glastonbury]

Comment on attachment 456504 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=456504&action=review

> Source/WebGPU/WebGPU/Texture.mm:1119
> +        return std::nullopt;

There appears to be an inconsistency with the logic in this function. If `WGPUTextureFormat_ASTC4x4Unorm` returns `WGPUTextureFormat_ASTC4x4Unorm`, surely these should return themselves? What am I missing?

> Source/WebGPU/WebGPU/Texture.mm:1243
> +    if (canonicalizedFormat1 != canonicalizedFormat1)

Comparison to self?!

[Myles C. Maxfield]

Comment on attachment 456504 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=456504&action=review

>> Source/WebGPU/WebGPU/Texture.mm:1119
>> +        return std::nullopt;
> 
> There appears to be an inconsistency with the logic in this function. If `WGPUTextureFormat_ASTC4x4Unorm` returns `WGPUTextureFormat_ASTC4x4Unorm`, surely these should return themselves? What am I missing?

Yeah, I see what you mean.

Originally, this function was intended to create equivalence classes via normalization; e.g. RGBA8Unorm and RGBA8UnormSrgb should both be members of the same equivalence class, and this function returns a representative member of that class (sort of like how union set works). ASTC4x4Unorm and ASTC4x4UnormSrgb are in the same equivalence class. I was originally thinking that formats like R8Unorm wouldn't be part of any equivalence class, so they would return nullopt here, but I can also understand where you're coming from about saying R8Unorm should be in an equivalence class of 1. I'll change the implementation to do that.

>> Source/WebGPU/WebGPU/Texture.mm:1243
>> +    if (canonicalizedFormat1 != canonicalizedFormat1)
> 
> Comparison to self?!

🤦🏻‍♂️

[Dan Glastonbury]

Comment on attachment 456504 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=456504&action=review

>>> Source/WebGPU/WebGPU/Texture.mm:1119
>>> +        return std::nullopt;
>> 
>> There appears to be an inconsistency with the logic in this function. If `WGPUTextureFormat_ASTC4x4Unorm` returns `WGPUTextureFormat_ASTC4x4Unorm`, surely these should return themselves? What am I missing?
> 
> Yeah, I see what you mean.
> 
> Originally, this function was intended to create equivalence classes via normalization; e.g. RGBA8Unorm and RGBA8UnormSrgb should both be members of the same equivalence class, and this function returns a representative member of that class (sort of like how union set works). ASTC4x4Unorm and ASTC4x4UnormSrgb are in the same equivalence class. I was originally thinking that formats like R8Unorm wouldn't be part of any equivalence class, so they would return nullopt here, but I can also understand where you're coming from about saying R8Unorm should be in an equivalence class of 1. I'll change the implementation to do that.

And with that change this function can be `WGPUTextureFormat Text::removeSRGBSuffix(...)`

[Myles C. Maxfield]

Created attachment 456533 [details]
Patch

[Kimmo Kinnunen]

Comment on attachment 456533 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=456533&action=review

> Source/WebGPU/WebGPU/Texture.mm:2255
> +    bool foundFormat = descriptor.format == m_descriptor.format;

if (descriptor.format != m_descriptor.format && !m_viewFormats.contains(descriptor.format)
     return false;

[Kimmo Kinnunen]

Comment on attachment 456533 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=456533&action=review

> Source/WebGPU/WebGPU/Texture.mm:2017
> +        viewFormats.reserveInitialCapacity(descriptorViewFormats.viewFormatsCount);

viewFormats = Vector { descriptorViewFormats.viewFormats, descriptorViewFormats.viewFormatsCount };

[Myles C. Maxfield]

Committed r292552 (249390@trunk): <https://commits.webkit.org/249390@trunk>

[Radar WebKit Bug Importer]

<rdar://problem/91437290>