WGSL builtin textureSample compressed formats #3760

greggman · 2024-05-23T06:43:10Z

I'm not sure these are needed or not but given we were testing so many other formats it seems like we should be testing compressed formats?

It does this by filling a compressed texture with random data. For compressed textures of type unorm or snorm this is fine. For float types this would fail since they random values might be large, Infinity, or NaN. We aren't converting from values to a compressed format. We're filling a buffer with binary random data and assuming the values that come out are in range.

After that, read that texture back to the GPU using a compute shader. This way we get WebGPU to decode the compressed texture to an uncompressed format.

Some places I got lost. I first put in the derivatives test and it passed. Then I put in non-derivatives test and it failed on srgb formats. This makes some sense I think because I was using only rgba32float as the format for pulling the data out of the texture.

That meant, when doing the software interpolation it was probably doing it wrong. I switched to using one of rgba8unorm, rgba8snorm or rgba8unorm-srgb as the format the texture is read back to TexelView as. I think ideally if I could have an rgba32float-srgb that would have been best but that format doesn't exist and I think hacking it into TexelView would have made TexelView's list of formats not a subset of WebGPU's

One thing I'm wondering is if I should combine the compressed and uncompressed tests. They're very similar and it wouldn't take too much work to add some small amount of
if compressed {x} else {y} to consolidate them.

Requirements for PR author:

All missing test coverage is tracked with "TODO" or .unimplemented().
New helpers are /** documented */ and new helper files are found in helper_index.txt.
Test behaves as expected in a WebGPU implementation. (If not passing, explain above.)
Test have be tested with compatibility mode validation enabled and behave as expected. (If not passing, explain above.)

Requirements for reviewer sign-off:

Tests are properly located in the test tree.
Test descriptions allow a reader to "read only the test plans and evaluate coverage completeness", and accurately reflect the test code.
Tests provide complete coverage (including validation control cases). Missing coverage MUST be covered by TODOs.
Helpers and types promote readability and maintainability.

When landing this PR, be sure to make any necessary issue status updates.

kainino0x

One thing I'm wondering is if I should combine the compressed and uncompressed tests. They're very similar and it wouldn't take too much work to add some small amount of
if compressed {x} else {y} to consolidate them.

No strong opinion, but this does sound a bit simpler

src/webgpu/listing_meta.json

src/webgpu/shader/execution/expression/call/builtin/textureSample.spec.ts

src/webgpu/shader/execution/expression/call/builtin/texture_utils.ts

I'm not sure these are needed or not but given we were testing so many other formats it seems like we should be testing compressed formats? It does this by filling a compressed texture with random data. For compressed textures of type unorm or snorm this is fine. For float types this would fail since they random values might be large, Infinity, or NaN. We aren't converting from values to a compressed format. We're filling a buffer with binary random data and assuming the values that come out are in range. After that, read that texture back to the GPU using a compute shader. This way we get WebGPU to decode the compressed texture to an uncompressed format. Some places I got lost. I first put in the derivatives test and it passed. Then I put in non-derivatives test and it failed on srgb formats. This makes some sense I think because I was using only `rgba32float` as the format for pulling the data out of the texture. That meant, when doing the software interpolation it was probably doing it wrong. I switched to using one of `rgba8unorm`, `rgba8snorm` or `rgba8unorm-srgb` as the format the texture is read back to `TexelView` as. I think ideally if I could have an `rgba32float-srgb` that would have been best but that format doesn't exist and I think hacking it into `TexelView` would have made `TexelView`'s list of formats not a subset of WebGPU's Also, I'm pretty worried about the number of tests. Just these 4 tests take 42 seconds to run on my M1 Mac and there's still 22 tests to go for this single built in alone and then all the rest of the texture builtins.

greggman · 2024-05-28T07:21:56Z

note: only android is failing and it's failing on astc unorm formats with

no ulp representation for infinity/nan
Error: no ulp representation for infinity/nan

AFAICT this a bug on Android. The same tests pass on all other devices. If I understand correctly there are no invalid values for astc unorm formats. Further, the code is supposed to be predictable. The values put into the textures for testing should be the same across platforms. So it seems like this is either a GPU bug on those devices, a Dawn bug, or a JS bug (somehow it's producing NaN when it shouldn't)

I'm committing this for now. Revert if you have to. I'll also file a chrome bug

kainino0x

AFAICT this a bug on Android. The same tests pass on all other devices. If I understand correctly there are no invalid values for astc unorm formats. Further, the code is supposed to be predictable. The values put into the textures for testing should be the same across platforms. So it seems like this is either a GPU bug on those devices, a Dawn bug, or a JS bug (somehow it's producing NaN when it shouldn't)

Could be a filtering issue (see comment) or it could be that some bit patterns in ASTC don't actually have defined outputs. Trying to look up the GL spec I was reminded that there are both LDR and HDR versions of ASTC. We only support LDR, but it seems from the GL extension spec that uploading HDR data into an LDR texture has defined results, see C.2.5 LDR and HDR Modes.
But there are also mentions of "invalid" inputs like "Results from invalid void extents are undefined."

kainino0x · 2024-05-28T22:52:14Z

src/webgpu/shader/execution/expression/call/builtin/texture_utils.ts

+ * We need the software rendering to do the same interpolation as the hardware
+ * rendered so for -srgb formats we set the TexelView to an -srgb format as
+ * TexelView handles this case. Note: It might be nice to add rgba32float-srgb
+ * or something similar to TexelView.


Hm, this comment made me remember, IIRC it's undefined whether filtering happens before or after gamma conversion. I think that could cause some issues with the tests?

I read that in the spec too. At the moment all the srgb tests are passing on all devices.

greggman · 2024-05-29T01:43:37Z

You're right, there are invalid inputs to astc. I can add a PR to filter those out. We'll have to either make an astc aware filler or leave them untested.

greggman requested a review from kainino0x May 23, 2024 06:43

kainino0x approved these changes May 24, 2024

View reviewed changes

greggman force-pushed the texSample-compressed branch from 6cacb03 to bf3d149 Compare May 27, 2024 02:28

increase tolerances

bbec2b9

greggman merged commit f7b3c49 into gpuweb:main May 28, 2024
1 check passed

greggman deleted the texSample-compressed branch May 28, 2024 07:22

kainino0x mentioned this pull request May 28, 2024

Test texture-compression-bc-sliced-3d #3761

Open

kainino0x reviewed May 28, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WGSL builtin textureSample compressed formats #3760

WGSL builtin textureSample compressed formats #3760

greggman commented May 23, 2024 •

edited by kainino0x

Loading

kainino0x left a comment

greggman commented May 28, 2024 •

edited

Loading

kainino0x left a comment

kainino0x May 28, 2024

greggman May 29, 2024

greggman commented May 29, 2024

WGSL builtin textureSample compressed formats #3760

WGSL builtin textureSample compressed formats #3760

Conversation

greggman commented May 23, 2024 • edited by kainino0x Loading

kainino0x left a comment

Choose a reason for hiding this comment

greggman commented May 28, 2024 • edited Loading

kainino0x left a comment

Choose a reason for hiding this comment

kainino0x May 28, 2024

Choose a reason for hiding this comment

greggman May 29, 2024

Choose a reason for hiding this comment

greggman commented May 29, 2024

greggman commented May 23, 2024 •

edited by kainino0x

Loading

greggman commented May 28, 2024 •

edited

Loading