Skip to content

Add GPURequestAdapterOptions.allowSoftware #1634

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.

Sign up for GitHub

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

Jump to bottom
Closed
kainino0x wants to merge 2 commits into from
Closed

Add GPURequestAdapterOptions.allowSoftware #1634

kainino0x wants to merge 2 commits into from

Conversation

@kainino0x
Copy link
Contributor

@kainino0x kainino0x commented Apr 13, 2021 •
edited by pr-preview bot
Loading

Proposal for the direction I think we agreed upon; alternative to #1302, related to #1439

💥 Error: 500 Internal Server Error 💥

PR Preview failed to build. (Last tried on Apr 14, 2021, 6:52 PM UTC).

More

PR Preview relies on a number of web services to run. There seems to be an issue with the following one:

🚨 HTML Diff Service - The HTML Diff Service is used to create HTML diffs of the spec changes suggested in a pull request.

🔗 Related URL

<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
<html><head>
<title>500 Internal Server Error</title>
</head><body>
<h1>Internal Server Error</h1>
<p>The server encountered an internal error or
misconfiguration and was unable to complete
your request.</p>
<p>Please contact the server administrator at 
 [email protected] to inform them of the time this error occurred,
 and the actions you performed just before this error.</p>
<p>More information about this error may be available
in the server error log.</p>
</body></html>

If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.

kvark
kvark requested changes Apr 14, 2021
View reviewed changes
spec/index.bs
</script>

<script type=idl>
enum GPUAllowSoftwareOption {
Copy link
Contributor

@kvark kvark Apr 14, 2021

Choose a reason for hiding this comment

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

Why can't this just be a boolean? It's up to the UA to figure out when this should be preferred, anyway.

Copy link
Contributor Author

@kainino0x kainino0x Apr 14, 2021

Choose a reason for hiding this comment

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

For extensibility, see [1]

Copy link
Contributor

@kvark kvark Apr 14, 2021

Choose a reason for hiding this comment

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

I think, if you want to have rich controls like this, then the "allow" part becomes confusing. I.e. allowSoftware = "always" doesn't read to me like a thing that would prefer software over hardware adapters.

Copy link
Contributor Author

@kainino0x kainino0x Apr 14, 2021

Choose a reason for hiding this comment

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

with plural requestAdapters, I envisioned "always" would still list software adapters last, effectively keeping the same preference order but not hiding software adapters.

kainino0x
kainino0x commented Apr 14, 2021
View reviewed changes
spec/index.bs
</script>

<script type=idl>
enum GPUAllowSoftwareOption {
Copy link
Contributor Author

@kainino0x kainino0x Apr 14, 2021

Choose a reason for hiding this comment

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

For extensibility, see [1]

spec/index.bs
Issue:
Additional possible options here include `"always"`, if requestAdapter is pluralized,
and `"only"`, if we want to allow apps to force software
(e.g. as a driver bug workaround, or for automated testing).
Copy link
Contributor Author

@kainino0x kainino0x Apr 14, 2021

Choose a reason for hiding this comment

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

[1]

@kainino0x
Copy link
Contributor Author

kainino0x commented Apr 22, 2021 •
edited
Loading

Editors meeting resolution: This solution is not ideal as it doesn't actually solve all cases - for example, it doesn't allow an application to implement logic to blocklist known buggy GPUs and fall back to software (or to another GPU in the system).
It is also rather hamfisted with the 3-4 possible "allowSoftware" modes.

Plural requestAdapters(), plus a GPUAdapter.isSoftware flag, would solve this.
We can expose both hardware and software adapters to a page and allow them to choose.
Additionally, it would resolve @jdashg's concern that we make it too easy for applications to disallow software implementations 'just because it seems nice'.

@toji is looking at a proposal that incorporates plural requestAdapters() + .isSoftware together.
This doesn't have to expose multiple hardware adapters (and in fact we could require, say, a Permissions API permission in order to see multiple hardware adapters).

@toji toji mentioned this pull request Apr 22, 2021
Closed
@kainino0x
Copy link
Contributor Author

kainino0x commented Jun 1, 2021

Closing in favor of #1734

@kainino0x kainino0x closed this Jun 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kvark kvark kvark requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@kainino0x @kvark