Skip to content

Add tests for GPUDevice.adapterInfo #4023

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

Merged
merged 9 commits into from
Nov 6, 2024
Merged
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 41 additions & 2 deletions src/webgpu/api/operation/adapter/info.spec.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
export const description = `
Tests GPUAdapter.info members formatting.
Tests for GPUAdapterInfo.
`;

import { Fixture } from '../../../../common/framework/fixture.js';
import { makeTestGroup } from '../../../../common/framework/test_group.js';
import { getGPU } from '../../../../common/util/navigator_gpu.js';
import { assert } from '../../../../common/util/util.js';
import { assert, objectEquals } from '../../../../common/util/util.js';

export const g = makeTestGroup(Fixture);

Expand Down Expand Up @@ -39,3 +39,42 @@ g.test('adapter_info')
`adapterInfo.device should be a normalized identifier. But it's '${adapterInfo.device}'`
);
});

g.test('same_object')
.desc(
`
GPUAdapter.info provides the same object each time it's accessed`
)
.fn(async t => {
const gpu = getGPU(t.rec);
const adapter = await gpu.requestAdapter();
assert(adapter !== null);

const adapterInfo1 = adapter.info;
const adapterInfo2 = adapter.info;
t.expect(adapterInfo1 === adapterInfo2);
});

g.test('device_matches_adapter')
.desc(
`
Test that GPUDevice.adapterInfo matches GPUAdapter.info`
)
.fn(async t => {
const gpu = getGPU(t.rec);
const adapter = await gpu.requestAdapter();
assert(adapter !== null);

const device = await t.requestDeviceTracked(adapter);
assert(device !== null);

assert(device.adapterInfo instanceof GPUAdapterInfo);
assert(adapter.info instanceof GPUAdapterInfo);

for (const k of Object.keys(GPUAdapterInfo.prototype)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Landed the spec; didn't make adapter and device use the same object, so we should test that adapter.info !== device.adapterInfo.

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 spec doesn't say they have to be different object, so I wouldn't test that adapter.info !== device.adapterInfo. Note that Chromium experimental implementation actually returns the same object: https://chromium-review.googlesource.com/c/chromium/src/+/5980752/3/third_party/blink/renderer/modules/webgpu/gpu_device.cc

Copy link
Collaborator

Choose a reason for hiding this comment

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

It does implicitly require them to be different objects, because a new one is created the first time each of them is accessed, there's no opportunity to return the same one.

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've updated this PR.
Do you know by any chance how to return a copy of adapter_->info() in Chromium?

Copy link
Collaborator

@kainino0x kainino0x Nov 5, 2024

Choose a reason for hiding this comment

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

We should construct the adapterInfo when we construct GPUDevice and store it in a field of GPUDevice, the same way we do for info when we construct GPUAdapter. The initialization will be the same so we should take this code, move it into a GPUAdapter method called something like CreateAdapterInfoForAdapter(), have it return the GPUAdapterInfo object, and then call it from both GPUAdapter::GPUAdapter and GPUDevice::GPUDevice.
https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/modules/webgpu/gpu_adapter.cc;l=198-210;drc=0afc9ac9afcaab79fc54299039f4d27abf3a086d

t.expect(
objectEquals(device.adapterInfo[k], adapter.info[k]),
`device.adapterInfo.${k} is "${device.adapterInfo[k]}". Expected "${adapter.info[k]}"`
);
}
});
Loading