Skip to content
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

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

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
Draft

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

wants to merge 10 commits into from

Conversation

kainino0x
Copy link
Contributor

@kainino0x kainino0x commented Apr 30, 2024 •
edited

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

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

@kainino0x kainino0x mentioned this pull request Apr 30, 2024
Merged
toji
toji reviewed May 1, 2024
View reviewed changes
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.

design/FeaturesWithLimits.md Outdated Show resolved Hide resolved
design/FeaturesWithLimits.md Outdated Show resolved Hide resolved
design/FeaturesWithLimits.md Outdated Show resolved Hide resolved
design/FeaturesWithLimits.md Outdated Show resolved Hide resolved
@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!

toji
toji approved these changes May 7, 2024
View reviewed changes
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!

design/FeaturesWithLimits.md Outdated Show resolved Hide resolved
spec/index.bs Outdated Show resolved Hide resolved
spec/index.bs Outdated Show resolved Hide resolved
spec/index.bs Outdated Show resolved Hide resolved
@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.

toji
toji approved these changes May 8, 2024
View reviewed changes
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alan-baker alan-baker alan-baker left review comments

@toji toji toji approved these changes

Assignees
No one assigned
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
Milestone
Milestone 3+
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@kainino0x @Kangz @toji @alan-baker