Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Helper for robust texture content checking #1055

Merged
merged 11 commits into from
Mar 29, 2022
13 changes: 11 additions & 2 deletions src/common/framework/fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -246,9 +246,9 @@ export class Fixture {
return cond;
}

/** If the argument is an Error, fail (or warn). Otherwise, no-op. */
/** If the argument is an `Error`, fail (or warn). If it's `undefined`, no-op. */
expectOK(
error: Error | unknown,
error: Error | undefined,
{ mode = 'fail', niceStack }: { mode?: 'fail' | 'warn'; niceStack?: Error } = {}
): void {
if (error instanceof Error) {
Expand All @@ -264,4 +264,13 @@ export class Fixture {
}
}
}

eventualExpectOK(
error: Promise<Error | undefined>,
{ mode = 'fail' }: { mode?: 'fail' | 'warn' } = {}
) {
this.eventualAsyncExpectation(async niceStack => {
this.expectOK(await error, { mode, niceStack });
});
}
}
32 changes: 24 additions & 8 deletions src/unittests/conversion.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,34 @@ const cases = [
];

g.test('float16BitsToFloat32').fn(t => {
cases.forEach(value => {
// some loose check
t.expect(Math.abs(float16BitsToFloat32(value[0]) - value[1]) <= 0.00001, value[0].toString(2));
});
for (const [bits, number] of [
...cases,
[0b1_00000_0000000000, -0], // (resulting sign is not actually tested)
[0b0_00000_1111111111, 0.00006104], // subnormal f16 input
[0b1_00000_1111111111, -0.00006104],
]) {
const actual = float16BitsToFloat32(bits);
t.expect(
// some loose check
Math.abs(actual - number) <= 0.00001,
`for ${bits.toString(2)}, expected ${number}, got ${actual}`
);
}
});

g.test('float32ToFloat16Bits').fn(t => {
cases.forEach(value => {
for (const [bits, number] of [
...cases,
[0b0_00000_0000000000, 0.00001], // input that becomes subnormal in f16 is rounded to 0
[0b1_00000_0000000000, -0.00001], // and sign is preserved
]) {
// some loose check
// Does not handle clamping, underflow, overflow, or denormalized numbers.
t.expect(Math.abs(float32ToFloat16Bits(value[1]) - value[0]) <= 1, value[1].toString());
});
const actual = float32ToFloat16Bits(number);
t.expect(
Math.abs(actual - bits) <= 1,
`for ${number}, expected ${bits.toString(2)}, got ${actual.toString(2)}`
);
}
});

g.test('float32ToFloatBits_floatBitsToNumber')
Expand Down
4 changes: 2 additions & 2 deletions src/webgpu/api/operation/command_buffer/image_copy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -762,7 +762,7 @@ class ImageCopyTest extends GPUTest {
this.expectGPUBufferValuesEqual(outputBuffer, expectedData);
}

// MAINTENANCE_TODO(crbug.com/dawn/868): Revisit this when consolidating texture helpers.
// MAINTENANCE_TODO(#881): Migrate this into the texture_ok helpers.
async checkStencilTextureContent(
stencilTexture: GPUTexture,
stencilTextureSize: readonly [number, number, number],
Expand Down Expand Up @@ -984,7 +984,7 @@ class ImageCopyTest extends GPUTest {
}
}

// MAINTENANCE_TODO(crbug.com/dawn/868): Revisit this when consolidating texture helpers.
// MAINTENANCE_TODO(#881): Consider if this can be simplified/encapsulated using TexelView.
initializeDepthAspectWithRendering(
depthTexture: GPUTexture,
depthFormat: GPUTextureFormat,
Expand Down
6 changes: 6 additions & 0 deletions src/webgpu/util/check_contents.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
// MAINTENANCE_TODO: The "checkThingTrue" naming is confusing; these must be used with `expectOK`
// or the result is dropped on the floor. Rename these to things like `typedArrayIsOK`(??) to
// make it clearer.
// MAINTENANCE_TODO: Also, audit to make sure we aren't dropping any on the floor. Consider a
// no-ignored-return lint check if we can find one that we can use.

import {
assert,
ErrorWithExtra,
Expand Down
67 changes: 66 additions & 1 deletion src/webgpu/util/color_space_conversion.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { assert } from '../../common/util/util.js';
import { assert, unreachable } from '../../common/util/util.js';

import { multiplyMatrices } from './math.js';

Expand Down Expand Up @@ -194,3 +194,68 @@ export function srgbToDisplayP3(pixel: {

return pixel;
}

type InPlaceColorConversion = (rgba: {
R: number;
G: number;
B: number;
readonly A: number; // Alpha never changes during a conversion.
}) => void;

/**
* Returns a function which applies the specified colorspace/premultiplication conversion.
kainino0x marked this conversation as resolved.
Show resolved Hide resolved
* Does not clamp, so may return values outside of the `dstColorSpace` gamut, due to either
* color space conversion or alpha premultiplication.
*/
export function makeInPlaceColorConversion({
srcPremultiplied,
dstPremultiplied,
srcColorSpace = 'srgb',
dstColorSpace = 'srgb',
}: {
srcPremultiplied: boolean;
dstPremultiplied: boolean;
srcColorSpace?: PredefinedColorSpace;
dstColorSpace?: GPUPredefinedColorSpace;
}): InPlaceColorConversion {
const requireColorSpaceConversion = srcColorSpace !== dstColorSpace;
const requireUnpremultiplyAlpha =
srcPremultiplied && (requireColorSpaceConversion || srcPremultiplied !== dstPremultiplied);
const requirePremultiplyAlpha =
dstPremultiplied && (requireColorSpaceConversion || srcPremultiplied !== dstPremultiplied);

return rgba => {
assert(rgba.A >= 0.0 && rgba.A <= 1.0, 'rgba.A out of bounds');

if (requireUnpremultiplyAlpha) {
if (rgba.A !== 0.0) {
rgba.R /= rgba.A;
rgba.G /= rgba.A;
rgba.B /= rgba.A;
} else {
assert(
rgba.R === 0.0 && rgba.G === 0.0 && rgba.B === 0.0 && rgba.A === 0.0,
'Unpremultiply ops with alpha value 0.0 requires all channels equals to 0.0'
);
}
}
// It's possible RGB are now > 1.
// This technically represents colors outside the src gamut, so no clamping yet.

if (requireColorSpaceConversion) {
// WebGPU currently only supports dstColorSpace = 'srgb'.
if (srcColorSpace === 'display-p3' && dstColorSpace === 'srgb') {
rgba = displayP3ToSrgb(rgba);
} else {
unreachable();
}
}
// Now RGB may also be negative if the src gamut is larger than the dst gamut.

if (requirePremultiplyAlpha) {
rgba.R *= rgba.A;
rgba.G *= rgba.A;
rgba.B *= rgba.A;
}
};
}
36 changes: 28 additions & 8 deletions src/webgpu/util/conversion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ import { clamp } from './math.js';
*/
export function floatAsNormalizedInteger(float: number, bits: number, signed: boolean): number {
if (signed) {
assert(float >= -1 && float <= 1);
assert(float >= -1 && float <= 1, () => `${float} out of bounds of snorm`);
const max = Math.pow(2, bits - 1) - 1;
return Math.round(float * max);
} else {
assert(float >= 0 && float <= 1);
assert(float >= 0 && float <= 1, () => `${float} out of bounds of unorm`);
const max = Math.pow(2, bits) - 1;
return Math.round(float * max);
}
Expand Down Expand Up @@ -44,7 +44,10 @@ export function normalizedIntegerAsFloat(integer: number, bits: number, signed:
* sign, exponent, mantissa bits, and exponent bias.
* Returns the result as an integer-valued JS `number`.
*
* Does not handle clamping, underflow, overflow, or denormalized numbers.
* Does not handle clamping, overflow, or denormal inputs.
* On underflow (result is subnormal), rounds to (signed) zero.
*
* MAINTENANCE_TODO: Replace usages of this with numberToFloatBits.
*/
export function float32ToFloatBits(
n: number,
Expand Down Expand Up @@ -80,18 +83,24 @@ export function float32ToFloatBits(

// Convert to the new biased exponent.
const newBiasedExp = bias + exp;
assert(newBiasedExp >= 0 && newBiasedExp < 1 << exponentBits);
assert(newBiasedExp < 1 << exponentBits, () => `input number ${n} overflows target type`);

// Mask only the mantissa, and discard the lower bits.
const newMantissa = (bits & 0x7fffff) >> mantissaBitsToDiscard;
return (sign << (exponentBits + mantissaBits)) | (newBiasedExp << mantissaBits) | newMantissa;
if (newBiasedExp <= 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we throw errors or add assert for this instead of reset it to 0? I'm not sure whether this means bugs in the test cases.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the best would be to actually handle conversion from normal numbers to subnormal f16, but for the purpose of ULP checks, we don't care about subnormal - we round all subnormal numbers to 0. And generally GPUs don't guarantee results in the subnormal range anyway for a lot of operations. So I think it's unlikely to be a problem.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old code asserted, but that assertion was hit because we generated small numbers in color space conversion and then tried to convert to f16.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also recalled that on chromium implementaion, we always convert the White to D50 config (in this case D65 -> D50). This also introduce some precision difference.

// Result is subnormal or zero. Round to (signed) zero.
return sign << (exponentBits + mantissaBits);
} else {
// Mask only the mantissa, and discard the lower bits.
const newMantissa = (bits & 0x7fffff) >> mantissaBitsToDiscard;
return (sign << (exponentBits + mantissaBits)) | (newBiasedExp << mantissaBits) | newMantissa;
}
}

/**
* Encodes a JS `number` into an IEEE754 16 bit floating point number.
* Returns the result as an integer-valued JS `number`.
*
* Does not handle clamping, underflow, overflow, or denormalized numbers.
* Does not handle clamping, overflow, or denormal inputs.
* On underflow (result is subnormal), rounds to (signed) zero.
*/
export function float32ToFloat16Bits(n: number) {
return float32ToFloatBits(n, 1, 5, 10, 15);
Expand Down Expand Up @@ -146,6 +155,17 @@ export function floatBitsToNumber(bits: number, fmt: FloatFormat): number {
return numberWithWrongBias * 2 ** (kFloat32Format.bias - fmt.bias);
}

/**
* Encodes a JS `number` into an IEEE754 floating point number with the specified format.
* Returns the result as an integer-valued JS `number`.
*
* Does not handle clamping, overflow, or denormal inputs.
* On underflow (result is subnormal), rounds to (signed) zero.
*/
export function numberToFloatBits(number: number, fmt: FloatFormat): number {
return float32ToFloatBits(number, fmt.signed, fmt.exponentBits, fmt.mantissaBits, fmt.bias);
}

/**
* Given a floating point number (as an integer representing its bits), computes how many ULPs it is
* from zero.
Expand Down
Loading