Assorted helpers used for texture checking #1068

kainino0x · 2022-03-16T01:42:34Z

These are some assorted helpers that are used in the texture checking helpers in #1055 (WIP).
The commits are separate changes and may be reviewed separately if desired.

Issue: #881

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.)

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.

github-actions · 2022-03-16T01:49:01Z

Previews, as seen when this build job started (ca02348):
Run tests | View tsdoc

kainino0x · 2022-03-16T01:52:33Z

@shaoboyan PTAL at this first round of helpers.

The commits are separate changes and may be reviewed separately if desired.

I'm going to add some tests for floatBitsToNumber since that's difficult to be sure is correct.

github-actions · 2022-03-16T02:01:04Z

Previews, as seen when this build job started (a3418d9):
Run tests | View tsdoc

github-actions · 2022-03-16T02:11:52Z

Previews, as seen when this build job started (75e6df4):
Run tests | View tsdoc

src/webgpu/util/conversion.ts

shaoboyan · 2022-03-16T05:50:28Z

src/webgpu/util/conversion.ts

+ * Subnormal values are flushed to 0.
+ * Positive and negative 0 are both considered to be 0 ULPs from 0.
+ */
+export function floatBitsToNormalULPFromZero(bits: number, fmt: FloatFormat): number {


For copy_to_texture 16-bit float(and 32-bit float) result comparasion. With this helper function, it seems that we could check the result by:

// Assume expect is larger than actual floatBitsToNormalULPFromZero(Uint16(expected) - Uint16(actual), kFloat16Format) < constant?

Am I right?

If this is the direction, I'm a bit worry about the case running time.
As you may know, current compare logic is simple and hack
But it still took longer time because it requires a buffer view reinterpretation.
If we add an extra ops floatBitsToNormalULPFromZero, I think it took longer time.

So maybe a bit hack but do you think it is possible that we took everything as Uint8 as input and do the bit ops? It will save the buffer view reinterpretation.

And another option is to save time on the other place rather than the float compare.

Performance is definitely a potential issue and I haven't investigated it enough yet, thanks for highlighting it.
It's even worse than your example code, because it's more like floatBitsToNormalULPFromZero(expected) - floatBitsToNormalULPFromZero(actual).

We have a diffULP helper already for directly determining the ULPs between two values without computing them relative to zero. I'll investigate the performance and see what can be done.

As a point of comparison,
webgpu:web_platform,copyToTexture,ImageBitmap:from_ImageData:alpha="none";orientation="none";srcDoFlipYDuringCopy=true;dstColorFormat="rgba16float";*
before: 2500ms each
after: 4300ms each

Not quite as bad as I expected, but could probably be better

Looking at 7481681, I'm guessing it was float16BitsToFloat32/float16BitsToFloat32. Which is probably a little more expensive than floatBitsToNormalULPFromZero though I wouldn't expect it to be that much worse.

Looking at 7481681, I'm guessing it was float16BitsToFloat32/float16BitsToFloat32. Which is probably a little more expensive than floatBitsToNormalULPFromZero though I wouldn't expect it to be that much worse.

Yes, removing these two helper functions help accelerated the tests a lot but it is still worse than the Uint8 comparation a lot (on my machine) but the same performance as float32 comparation. So I suspect this is due to the reinterpretation (But I think it shouldn't took long time).

webgpu:web_platform,copyToTexture,ImageBitmap:from_ImageData:alpha="none";orientation="none";srcDoFlipYDuringCopy=true;dstColorFormat="rgba16float";*
before: 2500ms each
after: 4300ms each

Thanks for testing! I understand that 4300ms is the time that applying diffULP, right?

I dug down into the performance of the ImageBitmap:from_ImageData test and found that it was a simple matter of implementing this optimization I had left for myself (in #1055):

// MAINTENANCE_TODO: Could be faster to actually implement numberToBits directly. numberToBits: (components: PerTexelComponent<number>) => ret.unpackBits(new Uint8Array(ret.pack(encode(components)))),

before: 2500ms
draft: 4300ms
after: 2030ms!

For another test case:
https://gpuweb.github.io/cts/standalone/?runnow=1&worker=0&debug=1&q=webgpu:web_platform,copyToTexture,canvas:copy_contents_from_gpu_context_canvas:canvasType=%22onscreen%22;srcAndDstInSameGPUDevice=true;dstColorFormat=%22rgba16float%22;srcPremultiplied=true;dstPremultiplied=true;srcDoFlipYDuringCopy=true

before: 1490ms
after: 1410ms

src/webgpu/util/math.ts

shaoboyan

I've reviewed float helpers and getTextureSubCopyLayout implementations. Looks great!

github-actions · 2022-03-16T23:44:17Z

Previews, as seen when this build job started (2c7eea0):
Run tests | View tsdoc

kainino0x · 2022-03-17T01:10:01Z

A few more perf numbers:
Optimizing floatBitsToNumber brought one testcase's total runtime from 4250ms to 3500ms. This was before I did other optimizations. The performance was extremely close to the original float16BitsToFloat32 even though that was specialized to float16, so I didn't add back the specialization.

github-actions · 2022-03-17T01:14:41Z

Previews, as seen when this build job started (3382eae):
Run tests | View tsdoc

shaoboyan · 2022-03-17T01:37:36Z

src/webgpu/util/conversion.ts

+const workingDataU32 = new Uint32Array(workingData);
+const workingDataF32 = new Float32Array(workingData);
+export function float32BitsToNumber(bits: number): number {
+  workingDataU32[0] = bits;


I think the take away here is don't create temporary TypedArrayBuffer for reinterpretation.

~~I measured the performance of this briefly while testing a bunch of other things. It didn't have a very large effect, but it was enough that it seemed worth using.~~

However I tried measuring it against just now (by just moving workingData* inside these functions) and I wasn't able to measure a difference... ~2020ms either way. Maybe it got optimized better somehow when written this way?

Oh, the test case I was using is no longer bottlenecked on this function. I tested a different test case (rgba32float) which is, and the results are good.
webgpu:web_platform,copyToTexture,ImageBitmap:from_ImageData:alpha="premultiply";orientation="flipY";srcDoFlipYDuringCopy=false;dstColorFormat="rgba32float";dstPremultiplied=true

preallocated (this PR): 1640ms
late allocated (same but workingData moved inside the function): 2130ms
array-initialized (new Float32Array(new Uint32Array([bits]).buffer)[0]): 2260ms

incidentally I realized one of these functions is implemented wrong, so fixing that.

Ok, so it seems that the take away is still correct! Thanks for resolving this performance issue!

shaoboyan

LGTM, thanks for the iteration!

austinEng · 2022-03-17T17:35:23Z

@kainino0x after this PR, webgpu:api,operation,command_buffer,image_copy:mip_levels: tests are hitting an assert

1 | + This is a testharness.js-based test.
2 | + FAIL ;dimension="2d" assert_unreached:
3 | + - INFO: subcase: copySizeInBlocks={"width":5,"height":4,"depthOrArrayLayers":1};originInBlocks={"x":3,"y":2,"z":0};mipLevel=1;textureSize=[64,48,1]
4 | + - INFO: subcase: copySizeInBlocks={"width":5,"height":4,"depthOrArrayLayers":1};originInBlocks={"x":3,"y":2,"z":0};mipLevel=1;textureSize=[60,48,1]
5 | + - EXCEPTION: copySize must be a multiple of the block size
6 | + at assert (https://web-platform.test:8444/gen/third_party/webgpu-cts/src/common/util/util.js:21:15)
7 | + at getTextureSubCopyLayout (https://web-platform.test:8444/gen/third_party/webgpu-cts/src/webgpu/util/texture/layout.js:32:5)
8 | + at getTextureCopyLayout (https://web-platform.test:8444/gen/third_party/webgpu-cts/src/webgpu/util/texture/layout.js:19:20)
9 | + at ImageCopyTest.uploadTextureAndVerifyCopy (https://web-platform.test:8444/gen/third_party/webgpu-cts/src/webgpu/api/operation/command_buffer/image_copy.spec.js:298:47)
10 | + at RunCaseSpecific.fn (https://web-platform.test:8444/gen/third_party/webgpu-cts/src/webgpu/api/operation/command_buffer/image_copy.spec.js:1095:7)

austinEng · 2022-03-17T18:01:05Z

Example https://gpuweb.github.io/cts/standalone/?runnow=1&worker=0&debug=0&q=webgpu:api,operation,command_buffer,image_copy:mip_levels:initMethod=%22WriteTexture%22;checkMethod=%22FullCopyT2B%22;format=%22bc1-rgba-unorm%22;dimension=%222d%22

kainino0x · 2022-03-17T18:35:45Z

shoot, guess I should have dry-run these changes as well. Thanks for reporting.

kainino0x · 2022-03-17T23:08:35Z

Fix for at least that bug in #1077

Add getSubTextureCopyLayout helper

09e8277

kainino0x mentioned this pull request Mar 16, 2022

Helper for robust texture content checking #1055

Merged

7 tasks

kainino0x requested a review from shaoboyan March 16, 2022 01:44

kainino0x force-pushed the pre-texture-checking branch from ca02348 to a3418d9 Compare March 16, 2022 01:53

shaoboyan reviewed Mar 16, 2022

View reviewed changes

src/webgpu/util/conversion.ts Outdated Show resolved Hide resolved

shaoboyan reviewed Mar 16, 2022

View reviewed changes

src/webgpu/util/math.ts Show resolved Hide resolved

shaoboyan reviewed Mar 16, 2022

View reviewed changes

kainino0x force-pushed the pre-texture-checking branch from a4403b9 to 3382eae Compare March 17, 2022 01:07

shaoboyan self-requested a review March 17, 2022 01:32

shaoboyan reviewed Mar 17, 2022

View reviewed changes

shaoboyan approved these changes Mar 17, 2022

View reviewed changes

kainino0x added 3 commits March 16, 2022 19:10

floatBitsToNumber, floatBitsToNormalULPFromZero, signExtend

db24bb2

move generatePrettyTable to its own file (no changes)

7492ae5

reifyOrigin3D

0adaa5d

kainino0x force-pushed the pre-texture-checking branch from 3382eae to 2d74be6 Compare March 17, 2022 02:11

float32BitsToNumber/numberToFloat32Bits

0e8bf3d

kainino0x force-pushed the pre-texture-checking branch from 2d74be6 to 0e8bf3d Compare March 17, 2022 02:14

kainino0x enabled auto-merge (rebase) March 17, 2022 02:18

kainino0x merged commit 14b988a into gpuweb:main Mar 17, 2022

kainino0x deleted the pre-texture-checking branch March 17, 2022 02:19

kainino0x added a commit to kainino0x/cts that referenced this pull request Mar 17, 2022

Bugfix for getTextureCopyLayout introduced in gpuweb#1068

c9748ae

kainino0x mentioned this pull request Mar 17, 2022

Bugfix for getTextureCopyLayout #1077

Merged

7 tasks

kainino0x added a commit that referenced this pull request Mar 17, 2022

Bugfix for getTextureCopyLayout introduced in #1068 (#1077)

87e74a9

kainino0x added a commit that referenced this pull request Mar 18, 2022

Bugfix for getTextureCopyLayout introduced in #1068 (#1077)

19f28b9

kainino0x mentioned this pull request Mar 18, 2022

Fix readSinglePixelFrom2DTexture helper #1088

Merged

7 tasks

kainino0x added a commit that referenced this pull request Mar 18, 2022

Bugfix for getTextureCopyLayout introduced in #1068 (#1077)

25d05d4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Assorted helpers used for texture checking #1068

Assorted helpers used for texture checking #1068

kainino0x commented Mar 16, 2022 •

edited

Loading

github-actions bot commented Mar 16, 2022

kainino0x commented Mar 16, 2022

github-actions bot commented Mar 16, 2022

github-actions bot commented Mar 16, 2022

shaoboyan Mar 16, 2022

shaoboyan Mar 16, 2022

shaoboyan Mar 16, 2022

kainino0x Mar 16, 2022

kainino0x Mar 16, 2022

kainino0x Mar 16, 2022

shaoboyan Mar 17, 2022

kainino0x Mar 17, 2022

kainino0x Mar 17, 2022

shaoboyan Mar 17, 2022

shaoboyan left a comment

github-actions bot commented Mar 16, 2022

kainino0x commented Mar 17, 2022

github-actions bot commented Mar 17, 2022

shaoboyan Mar 17, 2022

kainino0x Mar 17, 2022 •

edited

Loading

kainino0x Mar 17, 2022

shaoboyan Mar 17, 2022 •

edited

Loading

shaoboyan left a comment

austinEng commented Mar 17, 2022

austinEng commented Mar 17, 2022

kainino0x commented Mar 17, 2022

kainino0x commented Mar 17, 2022

Assorted helpers used for texture checking #1068

Assorted helpers used for texture checking #1068

Conversation

kainino0x commented Mar 16, 2022 • edited Loading

github-actions bot commented Mar 16, 2022

kainino0x commented Mar 16, 2022

github-actions bot commented Mar 16, 2022

github-actions bot commented Mar 16, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shaoboyan left a comment

Choose a reason for hiding this comment

github-actions bot commented Mar 16, 2022

kainino0x commented Mar 17, 2022

github-actions bot commented Mar 17, 2022

Choose a reason for hiding this comment

kainino0x Mar 17, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shaoboyan Mar 17, 2022 • edited Loading

Choose a reason for hiding this comment

shaoboyan left a comment

Choose a reason for hiding this comment

austinEng commented Mar 17, 2022

austinEng commented Mar 17, 2022

kainino0x commented Mar 17, 2022

kainino0x commented Mar 17, 2022

kainino0x commented Mar 16, 2022 •

edited

Loading

kainino0x Mar 17, 2022 •

edited

Loading

shaoboyan Mar 17, 2022 •

edited

Loading