这是indexloc提供的服务,不要输入任何密码
Skip to content

Define conventions/rules for features that add new limits #4613

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

Closed
wants to merge 4 commits into from

Conversation

kainino0x
Copy link
Contributor

@kainino0x kainino0x commented Apr 30, 2024

See the included design doc for the high-level explanation and worked case studies.

Came from this discussion on clip-distances: #4588 (comment)

Copy link
Member

@toji toji left a comment

Choose a reason for hiding this comment

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

Looking pretty good! I appreciate the mapping of example inputs to results. Left a few thoughts.

@kainino0x kainino0x marked this pull request as ready for review May 7, 2024 02:20
@kainino0x
Copy link
Contributor Author

Revised and ready for review, I think!

Copy link
Member

@toji toji left a comment

Choose a reason for hiding this comment

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

LGTM with a couple of non-blocking, mostly formatting nits. Described behavior sounds good!

@kainino0x kainino0x added the needs-cts-issue This change requires tests (or would need tests if accepted), but may not have a CTS issue filed yet label May 7, 2024
@kainino0x
Copy link
Contributor Author

@toji I did some rewriting, please take another look. Sorry it was messier than I realized before.

Copy link
Member

@toji toji left a comment

Choose a reason for hiding this comment

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

Updates LGTM, Thanks!

@kainino0x kainino0x marked this pull request as draft May 8, 2024 23:18
@kainino0x kainino0x added this to the Milestone 3+ milestone May 8, 2024
@Kangz
Copy link
Contributor

Kangz commented May 14, 2024

GPU Web WG 2024-05-08 Pacific-time
  • KG: With the consensus for hardcoded limit on Add optional feature clip-distances #4588, we probably don’t immediately need this?
  • KN: Yes, but we can still take a look at this. If we don’t have people who really are excited to work on this early, we can just defer this for now.
  • KG: We could look but I think Mozilla would rather defer.
  • Consensus: Milestone 3 for now.

@kainino0x kainino0x added the api WebGPU API label May 21, 2024
kainino0x added a commit to kainino0x/gpuweb that referenced this pull request Jul 24, 2024
This is useful for application code like this:

`{ requiredLimits: { someLimit: adapter.limits.someLimit } }`

in several situations:

- A limit was removed (gpuweb#4688)
- A limit exists in one browser, but another browser doesn't know about it
- Possibly, depending on future designs:
  a limit is part of a feature, and the feature isn't available (gpuweb#4613)
@kainino0x kainino0x force-pushed the features-with-limits branch from 3ed27ff to a904b81 Compare July 24, 2024 06:39
Copy link
Contributor

github-actions bot commented Jul 24, 2024

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

@kainino0x kainino0x force-pushed the features-with-limits branch 2 times, most recently from 7a795db to 1599744 Compare July 24, 2024 23:52
kainino0x added a commit to kainino0x/gpuweb that referenced this pull request Jul 25, 2024
This is useful for application code like this:

`{ requiredLimits: { someLimit: adapter.limits.someLimit } }`

in several situations:

- A limit was removed (gpuweb#4688)
- A limit exists in one browser, but another browser doesn't know about it
  (though it provides very limited protection against this)
- Possibly, depending on future designs:
  a limit is part of a feature, and the feature isn't available (gpuweb#4613)
kainino0x added a commit to kainino0x/gpuweb that referenced this pull request Jul 25, 2024
This is useful for application code like this:

`{ requiredLimits: { someLimit: adapter.limits.someLimit } }`

in several situations:

- A limit was removed (gpuweb#4688)
- A limit exists in one browser, but another browser doesn't know about it
  (though it provides very limited protection against this)
- Possibly, depending on future designs:
  a limit is part of a feature, and the feature isn't available (gpuweb#4613)
@kainino0x kainino0x force-pushed the features-with-limits branch from 1599744 to 82c2394 Compare July 25, 2024 00:12
kainino0x added a commit to kainino0x/gpuweb that referenced this pull request Jul 27, 2024
This is useful for application code like this:

`{ requiredLimits: { someLimit: adapter.limits.someLimit } }`

in several situations:

- A limit was removed (gpuweb#4688)
- A limit exists in one browser, but another browser doesn't know about it
  (though it provides very limited protection against this)
- Possibly, depending on future designs:
  a limit is part of a feature, and the feature isn't available (gpuweb#4613)
@greggman
Copy link
Contributor

how is this going? ATM chrome reports 4294967295 for limits that are unavailable. It would be good to fix this to whatever the standard is before pages start writing code that checks for 4294967295

For example:

device = await (await navigator.gpu.requestAdapter()).requestDevice();
console.log(device.limits.maxSubgroupSize); // prints 4294967295

Like, if it reported zero then you could write code like

adapter = await navigator.gpu.requestAdapter();
const requiredFeatures = [];
if (adapter.features.has('subgroups')) {
  requiredFeatures.push('subgroups');
}
device = adapter.requestDevice({requiredFeatures});

...

if (device.limits.maxSubgroupSize > 0) {
  // use subgroups
}

@beaufortfrancois
Copy link
Contributor

how is this going? ATM chrome reports 4294967295 for limits that are unavailable.

For info, #4963 suggests we're adding new properties for adapter info, not limits for minSubgroupSize and maxSubgroupSize.

I agree though it would be nice to solve this issue regardless of subgroups.

@alan-baker
Copy link
Contributor

how is this going? ATM chrome reports 4294967295 for limits that are unavailable.

For info, #4963 suggests we're adding new properties for adapter info, not limits for minSubgroupSize and maxSubgroupSize.

I agree though it would be nice to solve this issue regardless of subgroups.

This question came up on #4963. I think there are two main possibilities when unsupported: undefined or default values. For subgroups, the default values make a lot of sense. I could envision that each feature might have a different answer though. So maybe the requirement is just that the getAdapterInfo() algorithm describes how the fields are populated when the feature is unsupported.

@kainino0x
Copy link
Contributor Author

For limits, IIRC this proposal is still what I want to do. However, we shouldn't try to figure this out until we actually have a feature that adds limits; subgroups will not. We should discuss what subgroups does with adapter properties on #4963 (sorry I have put off looking at it)

@kainino0x
Copy link
Contributor Author

Closing, no need for this right now.

@kainino0x kainino0x closed this Mar 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api WebGPU API needs-cts-issue This change requires tests (or would need tests if accepted), but may not have a CTS issue filed yet proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants