-
Notifications
You must be signed in to change notification settings - Fork 89
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
Add AdapterLimitsGPUTest that sets all limits to the adapter's #3466
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. This makes more sense than the thing that tried to get a device twice.
Note: I'm not sure how I refactored device_pool.ts is good but, the old code created one initial device with no descriptor the first time in acquire. It then created another of the type actually wanted by the user. That no longer works becauase the adapter is passed in and so if the first requestDevice succeeds, then the adapter has been used and can't be used for the second one. Also, assuming this change is approved, I can (and should?) probably refactor LimitsTest to use this vs how it's doing it now. That would end up using cached devices.
This change seems to be breaking a bunch of things with the node based test runner. Test that use
Are now causing exceptions on devices that don't support I also get the impression that once a device is lost, all subsequent tests fail to reacquire a new device. |
sorry about that. I'll fix assp |
This reverts commits: * f71a834. * 0a68bf7. Revert "Add AdapterLimitsGPUTest that sets all limits to the adapter's (gpuweb#3466)" This reverts commit 0a68bf7.
This reverts commits: * f71a834. * 0a68bf7. Revert "Add AdapterLimitsGPUTest that sets all limits to the adapter's (gpuweb#3466)" This reverts commit 0a68bf7.
Note: I'm not sure how I refactored device_pool.ts is good but, the old code created one initial device with no descriptor the first time in acquire. It then created another of the type actually wanted by the user. That no longer works becauase the adapter is passed in and so if the first requestDevice succeeds, then the adapter has been used and can't be used for the second one.
Also, assuming this change is approved, I can (and should?) probably refactor LimitsTest to use this vs how it's doing it now. That would end up using cached devices.
Issue: #3363
Requirements for PR author:
.unimplemented()
./** documented */
and new helper files are found inhelper_index.txt
.Requirements for reviewer sign-off:
When landing this PR, be sure to make any necessary issue status updates.