From 3b92a37e780a90b7adee635598e3424b0263a947 Mon Sep 17 00:00:00 2001 From: Kai Ninomiya Date: Tue, 7 Jan 2025 13:13:07 -0800 Subject: [PATCH 1/2] Update tests using .isCompatibilityMode to understand .featureLevel And update all comments about removing `compatibilityMode: boolean`. --- src/common/runtime/cmdline.ts | 2 +- src/common/runtime/helper/utils_worker.ts | 2 +- src/common/runtime/server.ts | 2 +- src/common/runtime/standalone.ts | 2 +- .../operation/adapter/requestDevice.spec.ts | 34 +++++++++++++------ src/webgpu/capability_info.ts | 12 ++++--- 6 files changed, 36 insertions(+), 18 deletions(-) diff --git a/src/common/runtime/cmdline.ts b/src/common/runtime/cmdline.ts index b2d220ec8ef9..2fd9e588154e 100644 --- a/src/common/runtime/cmdline.ts +++ b/src/common/runtime/cmdline.ts @@ -121,7 +121,7 @@ for (let i = 0; i < sys.args.length; ++i) { let codeCoverage: CodeCoverageProvider | undefined = undefined; if (globalTestConfig.compatibility || globalTestConfig.forceFallbackAdapter) { - // MAINTENANCE_TODO: remove the cast once compatibilityMode is officially added + // MAINTENANCE_TODO: remove compatibilityMode (and the typecast) once no longer needed. setDefaultRequestAdapterOptions({ compatibilityMode: globalTestConfig.compatibility, featureLevel: globalTestConfig.compatibility ? 'compatibility' : 'core', diff --git a/src/common/runtime/helper/utils_worker.ts b/src/common/runtime/helper/utils_worker.ts index 5886839f6c33..d4f589625f46 100644 --- a/src/common/runtime/helper/utils_worker.ts +++ b/src/common/runtime/helper/utils_worker.ts @@ -26,7 +26,7 @@ export function setupWorkerEnvironment(ctsOptions: CTSOptions): Logger { if (powerPreference || compatibility) { setDefaultRequestAdapterOptions({ ...(powerPreference && { powerPreference }), - // MAINTENANCE_TODO: Change this to whatever the option ends up being + // MAINTENANCE_TODO: remove compatibilityMode once no longer needed. ...(compatibility && { compatibilityMode: true, featureLevel: 'compatibility' }), }); } diff --git a/src/common/runtime/server.ts b/src/common/runtime/server.ts index d908ce89ba7a..f675a7f60e28 100644 --- a/src/common/runtime/server.ts +++ b/src/common/runtime/server.ts @@ -120,7 +120,7 @@ for (let i = 0; i < sys.args.length; ++i) { let codeCoverage: CodeCoverageProvider | undefined = undefined; if (globalTestConfig.compatibility || globalTestConfig.forceFallbackAdapter) { - // MAINTENANCE_TODO: remove the cast once compatibilityMode is officially added + // MAINTENANCE_TODO: remove compatibilityMode (and the typecast) once no longer needed. setDefaultRequestAdapterOptions({ compatibilityMode: globalTestConfig.compatibility, featureLevel: globalTestConfig.compatibility ? 'compatibility' : 'core', diff --git a/src/common/runtime/standalone.ts b/src/common/runtime/standalone.ts index a079ac28dd98..4fa4d2e3c455 100644 --- a/src/common/runtime/standalone.ts +++ b/src/common/runtime/standalone.ts @@ -84,7 +84,7 @@ stopButtonElem.addEventListener('click', () => { if (powerPreference || compatibility || forceFallbackAdapter) { setDefaultRequestAdapterOptions({ ...(powerPreference && { powerPreference }), - // MAINTENANCE_TODO: Change this to whatever the option ends up being + // MAINTENANCE_TODO: remove compatibilityMode once no longer needed. ...(compatibility && { compatibilityMode: true, featureLevel: 'compatibility' }), ...(forceFallbackAdapter && { forceFallbackAdapter: true }), }); diff --git a/src/webgpu/api/operation/adapter/requestDevice.spec.ts b/src/webgpu/api/operation/adapter/requestDevice.spec.ts index 42701660386c..c502166a5c72 100644 --- a/src/webgpu/api/operation/adapter/requestDevice.spec.ts +++ b/src/webgpu/api/operation/adapter/requestDevice.spec.ts @@ -491,7 +491,7 @@ g.test('always_returns_device') Note: This is a regression test for a Chrome bug crbug.com/349062459 Checking that a requestDevice always return a device is checked in other tests above - but those tests have 'compatibilityMode: true' set for them by the API that getGPU + but those tests have 'featureLevel: "compatibility"' set for them by the API that getGPU returns when the test suite is run in compatibility mode. This test tries to force both compat and core separately so both code paths are @@ -502,19 +502,33 @@ g.test('always_returns_device') .fn(async t => { const { compatibilityMode } = t.params; const gpu = getGPU(t.rec); - // MAINTENANCE_TODO: Remove this cast compatibilityMode is added. - const adapter = await gpu.requestAdapter({ compatibilityMode } as GPURequestAdapterOptions); + // MAINTENANCE_TODO: Remove compatibilityMode and the cast once compatibilityMode is no longer + // used (mainly in `setDefaultRequestAdapterOptions`). + const adapter = await gpu.requestAdapter({ + compatibilityMode, + featureLevel: compatibilityMode ? 'compatibility' : 'core', + } as GPURequestAdapterOptions); if (adapter) { + const device = await t.requestDeviceTracked(adapter); + assert(device instanceof GPUDevice, 'requestDevice must return a device or throw'); + if (!compatibilityMode) { - // This check is to make sure something lower-level is not forcing compatibility mode - // MAINTENANCE_TODO: Remove this cast compatibilityMode is added. + // This check is to make sure something lower-level is not forcing compatibility mode. + + // MAINTENANCE_TODO: Simplify this check (and typecast) once we standardize how to do this. + const adapterExtensions = adapter as unknown as { + isCompatibilityMode?: boolean; + featureLevel?: string; + }; t.expect( - !(adapter as unknown as { isCompatibilityMode?: boolean }).isCompatibilityMode, - 'must not be compatibility mode' + // Old version of Compat design. + !adapterExtensions.isCompatibilityMode && + // Current version of Compat design, as of this writing. + adapterExtensions.featureLevel !== 'compatibility' && + // An unlanded proposed change to the Compat design, but it doesn't hurt to just check. + !device.features.has('webgpu-core'), + 'must not get a Compatibility adapter if not requested' ); } - const device = await t.requestDeviceTracked(adapter); - t.expect(device instanceof GPUDevice, 'requestDevice must return a device or throw'); - device.destroy(); } }); diff --git a/src/webgpu/capability_info.ts b/src/webgpu/capability_info.ts index 661812cd4beb..00f6fddc1cc6 100644 --- a/src/webgpu/capability_info.ts +++ b/src/webgpu/capability_info.ts @@ -802,11 +802,15 @@ export function getDefaultLimits(featureLevel: FeatureLevel) { export function getDefaultLimitsForAdapter(adapter: GPUAdapter) { // MAINTENANCE_TODO: Remove casts when GPUAdapter IDL has isCompatibilityMode. - return getDefaultLimits( - (adapter as unknown as { isCompatibilityMode: boolean }).isCompatibilityMode + const adapterExtensions = adapter as unknown as { + isCompatibilityMode?: boolean; + featureLevel?: string; + }; + const featureLevel = + adapterExtensions.featureLevel === 'compatibility' || adapterExtensions.isCompatibilityMode ? 'compatibility' - : 'core' - ); + : 'core'; + return getDefaultLimits(featureLevel); } const kEachStage = [ From 4e590cfcb4e1405b70f4c989b4ece6216bdf2cac Mon Sep 17 00:00:00 2001 From: Kai Ninomiya Date: Wed, 8 Jan 2025 15:30:12 -0800 Subject: [PATCH 2/2] address comments --- src/webgpu/api/operation/adapter/requestDevice.spec.ts | 3 ++- src/webgpu/capability_info.ts | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/webgpu/api/operation/adapter/requestDevice.spec.ts b/src/webgpu/api/operation/adapter/requestDevice.spec.ts index c502166a5c72..421fc991f402 100644 --- a/src/webgpu/api/operation/adapter/requestDevice.spec.ts +++ b/src/webgpu/api/operation/adapter/requestDevice.spec.ts @@ -525,7 +525,8 @@ g.test('always_returns_device') !adapterExtensions.isCompatibilityMode && // Current version of Compat design, as of this writing. adapterExtensions.featureLevel !== 'compatibility' && - // An unlanded proposed change to the Compat design, but it doesn't hurt to just check. + // An as-yet-unlanded proposed change to the Compat design, but for now it doesn't hurt + // to just check. Unlanded PR: https://github.com/gpuweb/gpuweb/pull/5036 !device.features.has('webgpu-core'), 'must not get a Compatibility adapter if not requested' ); diff --git a/src/webgpu/capability_info.ts b/src/webgpu/capability_info.ts index 00f6fddc1cc6..4da99bad66e8 100644 --- a/src/webgpu/capability_info.ts +++ b/src/webgpu/capability_info.ts @@ -801,7 +801,8 @@ export function getDefaultLimits(featureLevel: FeatureLevel) { } export function getDefaultLimitsForAdapter(adapter: GPUAdapter) { - // MAINTENANCE_TODO: Remove casts when GPUAdapter IDL has isCompatibilityMode. + // MAINTENANCE_TODO: Remove casts once we have a standardized way to do this + // (see https://github.com/gpuweb/gpuweb/pull/5037#issuecomment-2576110161). const adapterExtensions = adapter as unknown as { isCompatibilityMode?: boolean; featureLevel?: string;