Allow `featureLevel: "compatibility"`, which does nothing #4897

kainino0x · 2024-09-25T22:43:27Z

One of the core principles of the compatibility mode proposal is that requests for compatibility mode may be ignored. However, browsers must allow "compatibility" in order to ignore it.

This changes the current spec to allow (but otherwise ignore) featureLevel: "compatibility" in requestAdapter().

With the compatibility mode proposal, browsers would be allowed to either ignore it or impose compatibility mode validation.

Issue: #4656, #4266

github-actions · 2024-09-25T22:48:02Z

Previews, as seen when this build job started (caaf346):
WebGPU _{webgpu.idl | Explainer | Correspondence Reference}
WGSL _{grammar.js | wgsl.lalr.txt}

kainino0x · 2024-09-25T22:48:16Z

Proposing for M0 because it's important that all implementations do this, so that apps invoking compatibility mode won't break on those implementations.

toji

LGTM, with the only question being would it be worthwhile to set up an enum for the accepted feature levels now? Then the validation would be something like "must be undefined or the string value of an enum from GPUFeatureLevels" or something like that.

kainino0x · 2024-10-02T18:26:40Z

PTAL; if this seems good based on our past WG discussions, we don't need to wait for the next WG meeting.

toji · 2024-10-10T22:10:48Z

Since @kainino0x is out for a bit, @jimblandy: Are you OK with merging this now?

kainino0x · 2024-11-01T23:23:04Z

Updated per F2F decision, plus a non-normative note.

kainino0x · 2024-11-01T23:22:46Z

spec/index.bs

+
+                Note:
+                This value is reserved for future use as a way to opt into additional validation restrictions.
+                Applications should not use this value at this time.


I think this note is necessary, otherwise the spec is saying that applications can use "compatibility" now and expect to get "core" validation.

(FTR, given this, I now have no strong feeling between doing this PR and leaving the spec as it is now. Corentin's point during the F2F is valid: the spec can disallow passing any value here, and browsers not implementing compat can still go slightly ahead of the spec by allowing-and-ignoring "compatibility" knowing that it's the planned string value.)

toji

LGTM

* Briefly document featureLevel * Allow "compatibility" but it does nothing * Define "core" as well, and say "compatibility" shouldn't be used

…ng (#4897) (#4955)

* Briefly document featureLevel * Allow "compatibility" but it does nothing * Define "core" as well, and say "compatibility" shouldn't be used

Kangz · 2024-11-19T18:52:49Z

GPU Web WG 2024-10-29/30 Mountain View F2F

KN: currently spec will reject any string it does not know. To allow compatibility to be ignored later, we have to allow it now and let it do nothing for now. So we have to 1. Decide on a name, and 2. Allow its behaviour to be upgraded later.
JB: The ability to do silent upgrades has been part of the discussion of compat since the beginning, so don’t object now.
KG: What worries me is saying “feature level” here, and we might do something else later. I’d be more comfortable if it said “min feature level”, and allow it to be upgraded. That would have less risk.
Jasper: are feature levels always to be backward compatible.
KN: Yes. backward compatibility, but not necessarily always automatically upgrading.
KN: We might say FL1 is not automatically upgradable to FL2 just for compatibility reasons.
Jasper: WebGL 1 to 2 tried to be compatible, it was sometimes painful.
KN: we could call it compatibility_or_core to be explicitly super clear.
KG: D3D lets you give an array of feature levels that your application could work with, and the implementation selects. That works. Might even be priority order. So make it an array and you get something that you asked for.
CW: We’re kind of meta on this. We don’t have to set it in stone right now; we think we have good will among browsers for this. You have to reject on a string that is unknown, and included compat in that list. I’m saying we don’t need to decide firmly now. Push this back when we actually ship compat. And have the browsers implement the agreed.
KN: The risk is browsers that don’t support this string will be out there for a while. But not that long.
KR: We have all the browser vendors in the room.
JB: Firefox will get this done quickly. I think compatibility_or_core as the string addresses the upgradability story. When folks write that they know what they’re getting.
CW: We’re trying to define how compat will work before compat is defined. We don’t need to do that now. I’m here we’re uncomfortable specifying this now.
KG: we added the featureLevel member. Something that gets us past M0 is to remove that for now and push that to M1. Then we can sort it out there. Consider it a prerequisite for landing compat as a feature.
KR: Feels like we’re being too cautious. We know we need compat to get onto GLES 3.1 Android devices. We spec’d out the compat mode to work with that. I view this as relatively minor thing. It’s the only feature level we intend to auto-upgrade.
CW: uh..
KR: If “just” add this now we will get browsers out there that support it in not too long a time. It will get out there before it’s used in earnest. If we don’t put this in now it will be a longer period of WebGPU content in the web which breaks in one or two browsers because they either don’t support featureLevel member or compat as an enum value.
KG: Concrete proposal: there is a table of feature levels in the spec. It says if you say compat you get either compat or core. And if you say core you get core. That’s it for now. That feels safer than arguing upgrade paths. No implied comparability between things.
CW: If that sounds good to you then that’s workable for compat on the Chromium side.
JB: To restate, change the type from DOMString to an enum, and explain in the spec you either get compat or core when the application specifies compat.
CW: Ok with Apple, Mike?
MW: As long as we have the auto upgradability, WebKit will be happy with any reasonable approach.
KN: Do we want to add the table now? Because compat doesn’t exist.
- KG: Put the table in today.
- JB: Both entries should say the same thing.
KN: Do we give core a name?
- JB: call it core.
Jasper: Does changing to enum mean it still has the same behaviours w.r.t. Defaulting in the dictionary? [Yes]
Jasper: when i get an adapter info [object], do I read out of it what feature level is supported?
KN: Proposal for compat will deal with those details. Not yet.
CW: The TAG will be interested in this.
KN: Remember now. The reason we made it DOMString not enum is to avoid erroring out at the IDL level if the enum is not known.
KG: Can we make it DOMString | enum ?
KN: IIUC not allowed in IDL, the types overlap. Need to make it a DOMString
featureLevel member will remain a DOMString, with two strings defined in a table in the spec. core and compatibility will mean the same thing for now.

Spec PR: gpuweb/gpuweb#4897 CTS PR: gpuweb/cts#4077 Bug: 366151404 Change-Id: I451a5e66f85468cb433b9f20edf7c09644ac8cd7 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6067690 Reviewed-by: Kai Ninomiya <kainino@chromium.org> Commit-Queue: Kai Ninomiya <kainino@chromium.org> Cr-Commit-Position: refs/heads/main@{#1392500}

kainino0x added 2 commits September 25, 2024 15:39

Briefly document featureLevel

5c6b1f7

Allow "compatibility" but it does nothing

a034c2e

kainino0x added the api WebGPU API label Sep 25, 2024

kainino0x added this to the Milestone 0 milestone Sep 25, 2024

kainino0x mentioned this pull request Sep 25, 2024

Compat: describe as a featureLevel below core, upgradeable to core #4898

Closed

toji approved these changes Sep 30, 2024

View reviewed changes

kainino0x added the proposal label Sep 30, 2024

kainino0x requested review from jimblandy, kdashg and mwyrzykowski October 2, 2024 18:26

mwyrzykowski approved these changes Oct 2, 2024

View reviewed changes

Kangz added the compat WebGPU Compatibility Mode label Oct 23, 2024

kainino0x added merge-to-CR-yes and removed merge-to-CR-yes labels Nov 1, 2024

kainino0x added 2 commits November 1, 2024 16:15

Merge branch 'main' into compatibility-string

d1861ec

Define "core" as well, and say "compatibility" shouldn't be used

caaf346

kainino0x requested a review from toji November 1, 2024 23:23

kainino0x commented Nov 1, 2024

View reviewed changes

kainino0x added the api resolved Resolved - waiting for a change to the API specification label Nov 1, 2024

toji approved these changes Nov 4, 2024

View reviewed changes

kainino0x merged commit 63b719d into gpuweb:main Nov 4, 2024
4 checks passed

kainino0x deleted the compatibility-string branch November 4, 2024 18:35

kainino0x mentioned this pull request Nov 4, 2024

[M0 backport] Allow featureLevel: "compatibility", which does nothing (#4897) #4955

Merged

kainino0x added a commit that referenced this pull request Nov 4, 2024

[M0 backport] Allow featureLevel: "compatibility", which does nothi…

168d480

…ng (#4897) (#4955)

beaufortfrancois mentioned this pull request Dec 5, 2024

Add tests for GPURequestAdapterOptions featureLevel gpuweb/cts#4077

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Allow `featureLevel: "compatibility"`, which does nothing #4897

Allow `featureLevel: "compatibility"`, which does nothing #4897

Uh oh!

kainino0x commented Sep 25, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Sep 25, 2024 •

edited

Loading

Uh oh!

kainino0x commented Sep 25, 2024

Uh oh!

toji left a comment

Uh oh!

kainino0x commented Oct 2, 2024

Uh oh!

toji commented Oct 10, 2024

Uh oh!

kainino0x commented Nov 1, 2024

Uh oh!

kainino0x Nov 1, 2024

Uh oh!

kainino0x Nov 1, 2024 •

edited

Loading

Uh oh!

toji left a comment

Uh oh!

Uh oh!

Kangz commented Nov 19, 2024

Uh oh!

Uh oh!

Allow featureLevel: "compatibility", which does nothing #4897

Allow featureLevel: "compatibility", which does nothing #4897

Uh oh!

Conversation

kainino0x commented Sep 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Sep 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kainino0x commented Sep 25, 2024

Uh oh!

toji left a comment

Choose a reason for hiding this comment

Uh oh!

kainino0x commented Oct 2, 2024

Uh oh!

toji commented Oct 10, 2024

Uh oh!

kainino0x commented Nov 1, 2024

Uh oh!

kainino0x Nov 1, 2024

Choose a reason for hiding this comment

Uh oh!

kainino0x Nov 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

toji left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Kangz commented Nov 19, 2024

Uh oh!

Uh oh!

Allow `featureLevel: "compatibility"`, which does nothing #4897

Allow `featureLevel: "compatibility"`, which does nothing #4897

kainino0x commented Sep 25, 2024 •

edited

Loading

github-actions bot commented Sep 25, 2024 •

edited

Loading

kainino0x Nov 1, 2024 •

edited

Loading