Skip to content

Commit

Permalink
Test fixes found by Chromium automated testing
Browse files Browse the repository at this point in the history
Fixes gpuweb#230 (by moving the TODO into the repo)
Fixes gpuweb#234 (by moving the TODO into the repo)
  • Loading branch information
kainino0x committed Dec 18, 2020
1 parent 46c7b59 commit 14a5aa3
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 27 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
export const description = `
renderPass store op test that drawn quad is either stored or cleared based on storeop`;
renderPass store op test that drawn quad is either stored or cleared based on storeop
TODO: is this duplicated with api,operation,render_pass,storeOp?
`;

import { makeTestGroup } from '../../../../../common/framework/test_group.js';
import { GPUTest } from '../../../../gpu_test.js';
Expand Down
5 changes: 4 additions & 1 deletion src/webgpu/api/operation/render_pass/storeOp.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,10 @@ g.test('render_pass_store_op,color_attachment_only')
params()
.combine(poptions('colorFormat', kEncodableTextureFormats))
// Filter out any non-renderable formats
.filter(({ colorFormat }) => kEncodableTextureFormatInfo[colorFormat].renderable)
.filter(({ colorFormat }) => {
const info = kEncodableTextureFormatInfo[colorFormat];
return info.color && info.renderable;
})
.combine(poptions('storeOperation', kStoreOps))
.combine(poptions('mipLevel', kMipLevel))
.combine(poptions('arrayLayer', kArrayLayers))
Expand Down
16 changes: 15 additions & 1 deletion src/webgpu/api/validation/createBindGroup.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
kTextureBindingTypes,
kTextureBindingTypeInfo,
} from '../../capability_info.js';
import { GPUConst } from '../../constants.js';

import { ValidationTest } from './validation_test.js';

Expand Down Expand Up @@ -120,12 +121,20 @@ g.test('texture_binding_must_have_correct_usage')
size: { width: 16, height: 16, depth: 1 },
format: 'rgba8unorm' as const,
usage,
sampleCount: info.resource === 'sampledTexMS' ? 4 : 1,
};
const textureShouldError = usage === GPUTextureUsage.STORAGE && descriptor.sampleCount === 4;
let texture: GPUTexture | undefined;
t.expectValidationError(() => {
texture = t.device.createTexture(descriptor);
}, textureShouldError);
if (textureShouldError) return;
const resource = texture!.createView();

const shouldError = usage !== info.usage;
t.expectValidationError(() => {
t.device.createBindGroup({
entries: [{ binding: 0, resource: t.device.createTexture(descriptor).createView() }],
entries: [{ binding: 0, resource }],
layout: bindGroupLayout,
});
}, shouldError);
Expand Down Expand Up @@ -240,6 +249,11 @@ g.test('texture_must_have_correct_dimension').fn(async t => {
});

g.test('buffer_offset_and_size_for_bind_groups_match')
.desc(
`TODO: describe
TODO(#234): disallow zero-sized bindings`
)
.params([
{ offset: 0, size: 512, _success: true }, // offset 0 is valid
{ offset: 256, size: 256, _success: true }, // offset 256 (aligned) is valid
Expand Down
15 changes: 14 additions & 1 deletion src/webgpu/api/validation/createBindGroupLayout.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,14 @@ g.test('visibility')
.fn(async t => {
const { type, visibility } = t.params;

const info = kBindingTypeInfo[type];
const storageTextureFormat = info.resource === 'storageTex' ? 'rgba8unorm' : undefined;

const success = (visibility & ~kBindingTypeInfo[type].validStages) === 0;

t.expectValidationError(() => {
t.device.createBindGroupLayout({
entries: [{ binding: 0, visibility, type }],
entries: [{ binding: 0, visibility, type, storageTextureFormat }],
});
}, !success);
});
Expand Down Expand Up @@ -154,6 +157,11 @@ g.test('multisample_requires_2d_view_dimension')
});

g.test('number_of_dynamic_buffers_exceeds_the_maximum_value')
.desc(
`TODO: describe
TODO(#230): Update to enforce per-stage and per-pipeline-layout limits on BGLs as well.`
)
.params([
{ type: 'storage-buffer' as const, maxDynamicBufferCount: 4 },
{ type: 'uniform-buffer' as const, maxDynamicBufferCount: 8 },
Expand Down Expand Up @@ -236,6 +244,11 @@ const kCasesForMaxResourcesPerStageTests = params()
// Should never fail unless kMaxBindingsPerBindGroup is exceeded, because the validation for
// resources-of-type-per-stage is in pipeline layout creation.
g.test('max_resources_per_stage,in_bind_group_layout')
.desc(
`TODO: describe
TODO(#230): Update to enforce per-stage and per-pipeline-layout limits on BGLs as well.`
)
.params(kCasesForMaxResourcesPerStageTests)
.fn(async t => {
const { maxedType, extraType, maxedVisibility, extraVisibility } = t.params;
Expand Down
5 changes: 5 additions & 0 deletions src/webgpu/api/validation/createPipelineLayout.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ function clone<T extends GPUBindGroupLayoutDescriptor>(descriptor: T): T {
export const g = makeTestGroup(ValidationTest);

g.test('number_of_dynamic_buffers_exceeds_the_maximum_value')
.desc(
`TODO: describe
TODO(#230): Update to enforce per-stage and per-pipeline-layout limits on BGLs as well.`
)
.params(
params()
.combine(poptions('visibility', [0, 2, 4, 6]))
Expand Down
24 changes: 13 additions & 11 deletions src/webgpu/api/validation/validation_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,18 @@ export class ValidationTest extends GPUTest {
switch (state) {
case 'valid':
return this.device.createBuffer(descriptor);
case 'invalid':

case 'invalid': {
// Make the buffer invalid because of an invalid combination of usages but keep the
// descriptor passed as much as possible (for mappedAtCreation and friends).
return this.device.createBuffer({
this.device.pushErrorScope('validation');
const buffer = this.device.createBuffer({
...descriptor,
usage: descriptor.usage | GPUBufferUsage.MAP_READ | GPUBufferUsage.COPY_SRC,
});
this.device.popErrorScope();
return buffer;
}
case 'destroyed': {
const buffer = this.device.createBuffer(descriptor);
buffer.destroy();
Expand Down Expand Up @@ -94,11 +99,12 @@ export class ValidationTest extends GPUTest {
return sampler;
}

getSampledTexture(): GPUTexture {
getSampledTexture(sampleCount: number = 1): GPUTexture {
return this.device.createTexture({
size: { width: 16, height: 16, depth: 1 },
format: 'rgba8unorm',
usage: GPUTextureUsage.SAMPLED,
sampleCount,
});
}

Expand Down Expand Up @@ -145,11 +151,11 @@ export class ValidationTest extends GPUTest {
case 'compareSamp':
return this.getComparisonSampler();
case 'sampledTex':
return this.getSampledTexture().createView();
return this.getSampledTexture(1).createView();
case 'sampledTexMS':
return this.getSampledTexture(4).createView();
case 'storageTex':
return this.getStorageTexture().createView();
default:
unreachable('unknown binding resource type');
}
}

Expand Down Expand Up @@ -189,11 +195,7 @@ export class ValidationTest extends GPUTest {

createNoOpComputePipeline(): GPUComputePipeline {
const wgslCompute = `
fn main() -> void {
return;
}
entry_point compute = main;
[[stage(compute)]] fn main() -> void {}
`;

return this.device.createComputePipeline({
Expand Down
34 changes: 22 additions & 12 deletions src/webgpu/capability_info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ type ValidBindableResource =
| 'plainSamp'
| 'compareSamp'
| 'sampledTex'
| 'sampledTexMS'
| 'storageTex';
type ErrorBindableResource = 'errorBuf' | 'errorSamp' | 'errorTex';
export type BindableResource = ValidBindableResource | ErrorBindableResource;
Expand Down Expand Up @@ -315,8 +316,16 @@ export const kPerPipelineBindingLimits: {
const kBindableResource: {
readonly [k in BindableResource]: {};
} = /* prettier-ignore */ {
uniformBuf: {}, storageBuf: {}, plainSamp: {}, compareSamp: {}, sampledTex: {}, storageTex: {},
errorBuf: {}, errorSamp: {}, errorTex: {},
uniformBuf: {},
storageBuf: {},
plainSamp: {},
compareSamp: {},
sampledTex: {},
sampledTexMS: {},
storageTex: {},
errorBuf: {},
errorSamp: {},
errorTex: {},
};
export const kBindableResources = keysOf(kBindableResource);

Expand All @@ -330,12 +339,13 @@ interface BindingKindInfo {
const kBindingKind: {
readonly [k in ValidBindableResource]: BindingKindInfo;
} = /* prettier-ignore */ {
uniformBuf: { resource: 'uniformBuf', perStageLimitClass: kPerStageBindingLimits.uniformBuf, perPipelineLimitClass: kPerPipelineBindingLimits.uniformBuf, },
storageBuf: { resource: 'storageBuf', perStageLimitClass: kPerStageBindingLimits.storageBuf, perPipelineLimitClass: kPerPipelineBindingLimits.storageBuf, },
plainSamp: { resource: 'plainSamp', perStageLimitClass: kPerStageBindingLimits.sampler, perPipelineLimitClass: kPerPipelineBindingLimits.sampler, },
compareSamp: { resource: 'compareSamp', perStageLimitClass: kPerStageBindingLimits.sampler, perPipelineLimitClass: kPerPipelineBindingLimits.sampler, },
sampledTex: { resource: 'sampledTex', perStageLimitClass: kPerStageBindingLimits.sampledTex, perPipelineLimitClass: kPerPipelineBindingLimits.sampledTex, },
storageTex: { resource: 'storageTex', perStageLimitClass: kPerStageBindingLimits.storageTex, perPipelineLimitClass: kPerPipelineBindingLimits.storageTex, },
uniformBuf: { resource: 'uniformBuf', perStageLimitClass: kPerStageBindingLimits.uniformBuf, perPipelineLimitClass: kPerPipelineBindingLimits.uniformBuf, },
storageBuf: { resource: 'storageBuf', perStageLimitClass: kPerStageBindingLimits.storageBuf, perPipelineLimitClass: kPerPipelineBindingLimits.storageBuf, },
plainSamp: { resource: 'plainSamp', perStageLimitClass: kPerStageBindingLimits.sampler, perPipelineLimitClass: kPerPipelineBindingLimits.sampler, },
compareSamp: { resource: 'compareSamp', perStageLimitClass: kPerStageBindingLimits.sampler, perPipelineLimitClass: kPerPipelineBindingLimits.sampler, },
sampledTex: { resource: 'sampledTex', perStageLimitClass: kPerStageBindingLimits.sampledTex, perPipelineLimitClass: kPerPipelineBindingLimits.sampledTex, },
sampledTexMS: { resource: 'sampledTexMS', perStageLimitClass: kPerStageBindingLimits.sampledTex, perPipelineLimitClass: kPerPipelineBindingLimits.sampledTex, },
storageTex: { resource: 'storageTex', perStageLimitClass: kPerStageBindingLimits.storageTex, perPipelineLimitClass: kPerPipelineBindingLimits.storageTex, },
};

// Binding type info
Expand Down Expand Up @@ -380,10 +390,10 @@ export const kTextureBindingTypeInfo: {
// Add fields as needed
} & BindingTypeInfo;
} = /* prettier-ignore */ {
'sampled-texture': { usage: GPUConst.TextureUsage.SAMPLED, ...kBindingKind.sampledTex, ...kValidStagesAll, },
'multisampled-texture': { usage: GPUConst.TextureUsage.SAMPLED, ...kBindingKind.sampledTex, ...kValidStagesAll, },
'writeonly-storage-texture': { usage: GPUConst.TextureUsage.STORAGE, ...kBindingKind.storageTex, ...kValidStagesStorageWrite, },
'readonly-storage-texture': { usage: GPUConst.TextureUsage.STORAGE, ...kBindingKind.storageTex, ...kValidStagesAll, },
'sampled-texture': { usage: GPUConst.TextureUsage.SAMPLED, ...kBindingKind.sampledTex, ...kValidStagesAll, },
'multisampled-texture': { usage: GPUConst.TextureUsage.SAMPLED, ...kBindingKind.sampledTexMS, ...kValidStagesAll, },
'writeonly-storage-texture': { usage: GPUConst.TextureUsage.STORAGE, ...kBindingKind.storageTex, ...kValidStagesStorageWrite, },
'readonly-storage-texture': { usage: GPUConst.TextureUsage.STORAGE, ...kBindingKind.storageTex, ...kValidStagesAll, },
};
export const kTextureBindingTypes = keysOf(kTextureBindingTypeInfo);

Expand Down

0 comments on commit 14a5aa3

Please sign in to comment.