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.

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 datapoints for optional depthCompare and depthWriteEnabled properties #25158

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

chrisdavidmills
Copy link
Collaborator

@chrisdavidmills chrisdavidmills commented Nov 20, 2024

Summary

In Chrome 120, the depthCompare and depthWriteEnabled properties of the createRenderPipeline/createRenderPipelineAsync descriptors have been made optional when not needed. See https://developer.chrome.com/blog/new-in-webgpu-120#changes_to_depth-stencil_state.

See gpuweb/gpuweb#4318 for spec change.

This PR adds a datapoint for this, for each method.

Test results and supporting details

Related issues

Project issue: mdn/content#36370

@github-actions github-actions bot added data:api 🐇 Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API size:m [PR only] 25-100 LoC changed labels Nov 20, 2024
Copy link
Contributor

@beaufortfrancois beaufortfrancois 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 nits

],
"support": {
"chrome": {
"version_added": "120"
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ooop, I've only got one comment from you, which says "Ditto". I'm assuming there's one missing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. I was wondering why "notes": "Currently supported on ChromeOS, macOS, and Windows only." was not present for both.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

TBH, they should be added to all datapoints and sub-data points. Added in last commit.

@caugner
Copy link
Contributor

caugner commented Nov 21, 2024

@chrisdavidmills If this is based on a spec change (I assume it is?), can you please link the spec PR in the description? 🙏

Otherwise it's hard for me as a reviewer to check the description and validate the standard_track: true part.

PS: I checked both the content issue and the Chrome blog post, and they don't seem to link there.

@chrisdavidmills
Copy link
Collaborator Author

@chrisdavidmills If this is based on a spec change (I assume it is?), can you please link the spec PR in the description? 🙏

Otherwise it's hard for me as a reviewer to check the description and validate the standard_track: true part.

PS: I checked both the content issue and the Chrome blog post, and they don't seem to link there.

Added. For future reference, you can generally find the spec change links in the "dawn" issues, for example at https://developer.chrome.com/blog/new-in-webgpu-120#changes_to_depth-stencil_state, where it says "See issue dawn:2132".

@github-actions github-actions bot added size:l [PR only] 101-1000 LoC changed and removed size:m [PR only] 25-100 LoC changed labels Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data:api 🐇 Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API size:l [PR only] 101-1000 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants