Skip to content

Commit

Permalink
Revert gpuweb#3469 and gpuweb#3466
Browse files Browse the repository at this point in the history 
This reverts commits:
* f71a834.
* 0a68bf7.

These changes have been idenitified as causing a large collection of CTS failures with 'webgpu:web_platform,copyToTexture,ImageBitmap' (and possibly others).
  • Loading branch information
@ben-clayton
ben-clayton committed Mar 18, 2024
1 parent addbf81 commit 4d407e4
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 99 deletions.
4 changes: 2 additions & 2 deletions src/webgpu/api/operation/rendering/draw.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ import {
TypedArrayBufferView,
TypedArrayBufferViewConstructor,
} from '../../../../common/util/util.js';
import { AdapterLimitsGPUTest, TextureTestMixin } from '../../../gpu_test.js';
import { GPUTest, TextureTestMixin } from '../../../gpu_test.js';
import { PerPixelComparison } from '../../../util/texture/texture_ok.js';

class DrawTest extends TextureTestMixin(AdapterLimitsGPUTest) {
class DrawTest extends TextureTestMixin(GPUTest) {
checkTriangleDraw(opts: {
firstIndex: number | undefined;
count: number;
Expand Down
90 changes: 8 additions & 82 deletions src/webgpu/gpu_test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,47 +93,11 @@ export function initUncanonicalizedDeviceDescriptor(
}
}

/**
* Gets the adapter limits as a standard JavaScript object.
*/
function getAdapterLimitsAsDeviceRequiredLimits(adapter: GPUAdapter) {
const requiredLimits: Record<string, GPUSize64> = {};
const adapterLimits = adapter.limits as unknown as Record<string, GPUSize64>;
for (const key in adapter.limits) {
requiredLimits[key] = adapterLimits[key];
}
return requiredLimits;
}

/**
* Conditionally applies adapter limits to device descriptor
* but does not overwrite existing requested limits.
*/
function conditionallyApplyAdapterLimitsToDeviceDescriptor(
adapter: GPUAdapter,
useAdapterLimits: boolean,
descriptor: UncanonicalizedDeviceDescriptor | undefined
): UncanonicalizedDeviceDescriptor {
return {
...(descriptor || {}),
requiredLimits: {
...(useAdapterLimits && getAdapterLimitsAsDeviceRequiredLimits(adapter)),
...(descriptor?.requiredLimits || {}),
},
};
}

export class GPUTestSubcaseBatchState extends SubcaseBatchState {
/** Provider for default device. */
private provider: Promise<DeviceProvider> | undefined;
/** Provider for mismatched device. */
private mismatchedProvider: Promise<DeviceProvider> | undefined;
/** True if device should be created with adapter limits */
private useAdapterLimits = false;

constructor(recorder: TestCaseRecorder, params: TestParams) {
super(recorder, params);
}

override async postInit(): Promise<void> {
// Skip all subcases if there's no device.
Expand All @@ -159,14 +123,6 @@ export class GPUTestSubcaseBatchState extends SubcaseBatchState {
return this.provider;
}

useAdapterLimitsForDevice() {
assert(
this.provider === undefined,
'useAdapterLimits must be called before getting the device'
);
this.useAdapterLimits = true;
}

get isCompatibility() {
return globalTestConfig.compatibility;
}
Expand All @@ -184,18 +140,10 @@ export class GPUTestSubcaseBatchState extends SubcaseBatchState {
*/
selectDeviceOrSkipTestCase(descriptor: DeviceSelectionDescriptor): void {
assert(this.provider === undefined, "Can't selectDeviceOrSkipTestCase() multiple times");
this.provider = devicePool
.requestAdapter(this.recorder)
.then(adapter =>
devicePool.acquire(
adapter,
conditionallyApplyAdapterLimitsToDeviceDescriptor(
adapter,
this.useAdapterLimits,
initUncanonicalizedDeviceDescriptor(descriptor)
)
)
);
this.provider = devicePool.acquire(
this.recorder,
initUncanonicalizedDeviceDescriptor(descriptor)
);
// Suppress uncaught promise rejection (we'll catch it later).
this.provider.catch(() => {});
}
Expand Down Expand Up @@ -253,18 +201,10 @@ export class GPUTestSubcaseBatchState extends SubcaseBatchState {
"Can't selectMismatchedDeviceOrSkipTestCase() multiple times"
);

this.mismatchedProvider = mismatchedDevicePool
.requestAdapter(this.recorder)
.then(adapter =>
mismatchedDevicePool.acquire(
adapter,
conditionallyApplyAdapterLimitsToDeviceDescriptor(
adapter,
this.useAdapterLimits,
initUncanonicalizedDeviceDescriptor(descriptor)
)
)
);
this.mismatchedProvider = mismatchedDevicePool.acquire(
this.recorder,
initUncanonicalizedDeviceDescriptor(descriptor)
);
// Suppress uncaught promise rejection (we'll catch it later).
this.mismatchedProvider.catch(() => {});
}
Expand Down Expand Up @@ -1263,20 +1203,6 @@ export class GPUTest extends GPUTestBase {
}
}

/**
* A version of GPUTest that requires the adapter limits.
*/
export class AdapterLimitsGPUTest extends GPUTest {
public static override MakeSharedState(
recorder: TestCaseRecorder,
params: TestParams
): GPUTestSubcaseBatchState {
const state = new GPUTestSubcaseBatchState(recorder, params);
state.useAdapterLimitsForDevice();
return state;
}
}

/**
* Texture expectation mixin can be applied on top of GPUTest to add texture
* related expectation helpers.
Expand Down
43 changes: 28 additions & 15 deletions src/webgpu/util/device_pool.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,21 +22,33 @@ class FeaturesNotSupported extends Error {}
export class TestOOMedShouldAttemptGC extends Error {}

export class DevicePool {
private holders = new DescriptorToHolderMap();

async requestAdapter(recorder: TestCaseRecorder) {
const gpu = getGPU(recorder);
const adapter = await gpu.requestAdapter();
assert(adapter !== null, 'requestAdapter returned null');
return adapter;
}
private holders: 'uninitialized' | 'failed' | DescriptorToHolderMap = 'uninitialized';

/** Acquire a device from the pool and begin the error scopes. */
async acquire(
adapter: GPUAdapter,
recorder: TestCaseRecorder,
descriptor?: UncanonicalizedDeviceDescriptor
): Promise<DeviceProvider> {
const holder = await this.holders.getOrCreate(adapter, descriptor);
let errorMessage = '';
if (this.holders === 'uninitialized') {
this.holders = new DescriptorToHolderMap();
try {
await this.holders.getOrCreate(recorder, undefined);
} catch (ex) {
this.holders = 'failed';
if (ex instanceof Error) {
errorMessage = ` with ${ex.name} "${ex.message}"`;
}
}
}

assert(
this.holders !== 'failed',
`WebGPU device failed to initialize${errorMessage}; not retrying`
);

const holder = await this.holders.getOrCreate(recorder, descriptor);

assert(holder.state === 'free', 'Device was in use on DevicePool.acquire');
holder.state = 'acquired';
holder.beginTestScope();
Expand Down Expand Up @@ -126,7 +138,7 @@ class DescriptorToHolderMap {
* Throws SkipTestCase if devices with this descriptor are unsupported.
*/
async getOrCreate(
adapter: GPUAdapter,
recorder: TestCaseRecorder,
uncanonicalizedDescriptor: UncanonicalizedDeviceDescriptor | undefined
): Promise<DeviceHolder> {
const [descriptor, key] = canonicalizeDescriptor(uncanonicalizedDescriptor);
Expand All @@ -151,7 +163,7 @@ class DescriptorToHolderMap {
// No existing item was found; add a new one.
let value;
try {
value = await DeviceHolder.create(adapter, descriptor);
value = await DeviceHolder.create(recorder, descriptor);
} catch (ex) {
if (ex instanceof FeaturesNotSupported) {
this.unsupported.add(key);
Expand Down Expand Up @@ -286,14 +298,15 @@ class DeviceHolder implements DeviceProvider {
// Gets a device and creates a DeviceHolder.
// If the device is lost, DeviceHolder.lost gets set.
static async create(
adapter: GPUAdapter,
recorder: TestCaseRecorder,
descriptor: CanonicalDeviceDescriptor | undefined
): Promise<DeviceHolder> {
assert(adapter !== null, 'requestAdapter is null');
const gpu = getGPU(recorder);
const adapter = await gpu.requestAdapter();
assert(adapter !== null, 'requestAdapter returned null');
if (!supportsFeature(adapter, descriptor)) {
throw new FeaturesNotSupported('One or more features are not supported');
}

const device = await adapter.requestDevice(descriptor);
assert(device !== null, 'requestDevice returned null');

Expand Down

0 comments on commit 4d407e4

Please sign in to comment.