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

Tweak requiredBytesInCopy, make bytesPerRow/rowsPerImage optional #1014

Merged
merged 2 commits into from
Aug 24, 2020

Conversation

kainino0x
Copy link
Contributor

@kainino0x kainino0x commented Aug 19, 2020

bytesPerRow and rowsPerImage are optional, but required if used.
Both values always have to make sense if they're provided (regardless of
whether they have an effect).

This shuffles the contents of this algorithm around again, to:

  • Build requiredBytesInCopy incrementally.
  • Remove the special-case for width==0 || height==0 || depth==0.
    Notably this changes the validation so that, e.g., for a 0x2x0 copy,
    bytesPerRow * (2 - 1) bytes are still required.

First proposed here: #1006 (comment)

Fixes #984


Preview | Diff

bytesPerRow and rowsPerImage are optional, but required if used.
Both values always have to make sense if they're provided (regardless of
whether they have an effect).

This shuffles the contents of this algorithm around again, to:
- Build requiredBytesInCopy incrementally.
- Remove the special-case for `width==0 || height==0 || depth==0`.
  Notably this changes the validation so that, e.g., for a `0x2x0` copy,
  `bytesPerRow * (2 - 1)` bytes are still required.

Fixes gpuweb#984
@kainino0x kainino0x requested a review from kvark August 19, 2020 19:02
Copy link
Contributor

@kvark kvark left a comment

Choose a reason for hiding this comment

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

nice!

@kainino0x kainino0x requested a review from Kangz August 19, 2020 20:29
@kainino0x kainino0x added this to Needs Discussion in Main Aug 19, 2020
@kainino0x
Copy link
Contributor Author

Let's air this one at the meeting.

spec/index.bs Show resolved Hide resolved
@grorg
Copy link
Contributor

grorg commented Aug 24, 2020

Discussed at the 2020-08-24 meeting.

@kainino0x
Copy link
Contributor Author

Resolution: merge

@kainino0x kainino0x merged commit 60a6b7f into gpuweb:main Aug 24, 2020
@kainino0x kainino0x deleted the b984 branch August 24, 2020 19:30
@Kangz Kangz moved this from Needs Discussion to Needs Testing in Main Sep 11, 2020
kainino0x added a commit to kainino0x/gpuweb that referenced this pull request Oct 3, 2020
I previously updated this in gpuweb#1014. This PR fixes some cases where
requiredBytesInCopy was too large:

- 2x0x0: previously 2 * block size, now 0.
- 2x2x0: previously 4 * block size, now 0.
kvark pushed a commit that referenced this pull request Oct 5, 2020
I previously updated this in #1014. This PR fixes some cases where
requiredBytesInCopy was too large:

- 2x0x0: previously 2 * block size, now 0.
- 2x2x0: previously 4 * block size, now 0.
@kainino0x kainino0x moved this from Needs CTS Issue to Specification Done in Main Jan 15, 2022
ben-clayton pushed a commit to ben-clayton/gpuweb that referenced this pull request Sep 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Main
Specification Done
Development

Successfully merging this pull request may close these issues.

bytesPerRow and rowsPerImage disagree on validation between 0 and min
5 participants