.
Created attachment 456500 [details] Patch
Created attachment 456504 [details] Patch
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?!
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?! 🤦🏻♂️
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(...)`
Created attachment 456533 [details] Patch
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;
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 };
Committed r292552 (249390@trunk): <https://commits.webkit.org/249390@trunk>
<rdar://problem/91437290>