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

WIP: refactor(types)!: SampleCount newtype #5915

Draft
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

ErichDonGubler
Copy link
Member

@ErichDonGubler ErichDonGubler commented Jul 5, 2024

Connections

N/A

Description

A mostly refactoring (but breaking) change I thought might be interesting. I'm not significantly invested in it. Each commit is atomic, and so the suggested review experience is commit-by-commit.

Testing

I believe I'm preserving

Checklist

  • Run cargo fmt.
  • Run cargo clippy. If applicable, add:
    • --target wasm32-unknown-unknown
    • --target wasm32-unknown-emscripten
  • Run cargo xtask test to run tests.
  • Add change to CHANGELOG.md. See simple instructions inside file.

@ErichDonGubler ErichDonGubler added the kind: refactor Making existing function faster or nicer label Jul 5, 2024
@ErichDonGubler ErichDonGubler self-assigned this Jul 5, 2024
@ErichDonGubler ErichDonGubler force-pushed the push-npqkorssmrwx branch 5 times, most recently from 4d67733 to 5e308bf Compare July 5, 2024 05:06
@teoxoy
Copy link
Member

teoxoy commented Jul 5, 2024

We shouldn't have this type in wgpu-core's public API as the spec allows 0 to be passed for sample counts. If a user passes a 0, they will get a validation error.

We can still have this type at the wgpu level if users find it helpful, it could also check that the given sample count is a power of 2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: refactor Making existing function faster or nicer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants