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

Correspondence Reference: Limits #3088

Merged
merged 7 commits into from
Jul 6, 2022
Merged

Correspondence Reference: Limits #3088

merged 7 commits into from
Jul 6, 2022

Conversation

kainino0x
Copy link
Contributor

@kainino0x kainino0x commented Jun 23, 2022 •
edited
Loading

For rendered version, see preview link below

Documents how to compute limits on each platform. Subsumes Limits.md.

Fixes #3027

@kainino0x kainino0x force-pushed the limits-docs branch 4 times, most recently from 51167b6 to 81bb6bf Compare June 24, 2022 00:39
@kainino0x kainino0x changed the title Add "Correspondence Reference" with limits documentation Correspondence Reference: Limits Jun 24, 2022
@kainino0x kainino0x force-pushed the limits-docs branch 2 times, most recently from c37fc0f to 494dcdd Compare June 27, 2022 23:01
@kainino0x kainino0x requested a review from toji June 27, 2022 23:02
@kainino0x kainino0x marked this pull request as ready for review June 27, 2022 23:17
@kainino0x kainino0x requested review from Kangz, litherum and toji and removed request for toji June 27, 2022 23:18
kainino0x added 2 commits June 27, 2022 16:24
@kainino0x
Correspondence Reference: Limits
ed88c25 
Documents how to compute limits on each platform. Hopefully will be
expanded with more API and shading language correspondence tables in the
future.

Subsumes Limits.md.

Fixes gpuweb#3027
@kainino0x
update Vulkan maxColorAttachments
4a8e5a3
@github-actions

This comment was marked as outdated.

Sign in to view
@kainino0x
Copy link
Contributor Author

@cwfitzgerald you may also be interested in this

@github-actions

This comment was marked as outdated.

Sign in to view
@cwfitzgerald

This comment was marked as resolved.

Sign in to view
@github-actions

This comment was marked as outdated.

Sign in to view
@cwfitzgerald
Copy link
Contributor

Good stuff. Do note that FF has had trouble with buffer sizes above 2GB: gfx-rs/wgpu#2796

@github-actions
Copy link
Contributor

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

toji
toji approved these changes Jun 30, 2022
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, thanks for putting this together! Two minor style nits, though I don't consider either of them to be blocking.

correspondence/index.bs Outdated Show resolved Hide resolved
correspondence/index.bs
<th>`maxStorageTexturesPerShaderStage`
<td>[#409](https://github.com/gpuweb/gpuweb/issues/409)
<td>`maxPerStageDescriptorStorageImages`
<td rowspan=2>*Strategy-dependent.* Allocate `Maximum number of Unordered Access Views in all descriptor tables across all stages` across these two limits.
Copy link
Member

Choose a reason for hiding this comment

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

This rowspan, when paired with two offset rowspans in the Metal column, is a little awkward to visually to parse in the preview. I don't really have a suggestion for resolving it, but felt like it was worth pointing out anyway.

Screen Shot 2022-06-30 at 3 24 38 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

An alternative would be to do these as footnotes, no rowspans, put "[1]"/"[2]"/"[3]" in each cell and then put the text at the bottom.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 1, 2022

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

@toji
Copy link
Member

toji commented Jul 6, 2022

The issues certainly read better now, thanks! Not sure how critical I think it is to change the rowspans, they're parse-able it just take a moment to line things up visually.

Still LGTM.

@kainino0x kainino0x merged commit be10d23 into gpuweb:main Jul 6, 2022
@kainino0x kainino0x deleted the limits-docs branch July 6, 2022 22:37
jdarpinian pushed a commit to jdarpinian/gpuweb that referenced this pull request Aug 12, 2022
@kainino0x @jdarpinian
Correspondence Reference: Limits (gpuweb#3088)
67f2d3d 
* Correspondence Reference: Limits

Documents how to compute limits on each platform. Hopefully will be
expanded with more API and shading language correspondence tables in the
future.

Subsumes Limits.md.

Fixes gpuweb#3027

* update Vulkan maxColorAttachments

* fix for d3d12 minStorageBufferOffsetAlignment

* fix dark mode

* add some missing links

* td.issue -> td p.issue

* 4GiB or 2GiB
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@toji toji toji approved these changes

@Kangz Kangz Awaiting requested review from Kangz

@litherum litherum Awaiting requested review from litherum

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update Limits.md documentation
3 participants
@kainino0x @cwfitzgerald @toji