Skip to content

Conversation

@inner-daemons
Copy link
Collaborator

@inner-daemons inner-daemons commented Dec 4, 2025

Connections
Closes #8646

CI Failures mostly represent wgpu bugs, not issues with this PR

Description

Adds some testing for texture formats to make sure that what a device claims to support it actually supports

Testing
Is test

Squash or Rebase?
Squash

Checklist

  • Run cargo fmt.
  • Run taplo format.
  • Run cargo clippy --tests. If applicable, add:
    • --target wasm32-unknown-unknown
  • Run cargo xtask test to run tests.
  • If this contains user-facing changes, add a CHANGELOG.md entry.

@inner-daemons inner-daemons marked this pull request as ready for review December 4, 2025 19:46
/// [sRGB transfer function]: https://en.wikipedia.org/wiki/SRGB#Transfer_function_(%22gamma%22)
#[repr(C)]
#[derive(Copy, Clone, Debug, Hash, Eq, PartialEq)]
#[derive(Copy, Clone, Debug, Hash, Eq, PartialEq, strum::EnumIter)]
Copy link
Member

Choose a reason for hiding this comment

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

issue: We shouldn't expose EnumIter in our public API; we've never done so before, and I think we should discuss as a group before we actually do it. In the case of EnumIter, it's usually better to make a façade around it that returns impl Iterator.

issue: Even if we do have such an opaque API, I'm not sure if it makes sense to expose an exhaustive iterator for it anyway, except for, say, an unstable-testing feature.

Thoughts?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not at all attached to my current way of doing things, this is just quick and dirty so I could set up the tests. I do very much think that we want the iteration to be automatically generated though, so that someone can't forget to add a format down the line.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I like the idea of an extra feature here for testing, but ultimately I think the decision should be made by people more experienced with programming and with this codebase. If that makes sense to you I'm happy to do it.

Copy link
Member

Choose a reason for hiding this comment

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

This is #8438

/// ASTC RGBA channel
#[repr(C)]
#[derive(Copy, Clone, Debug, Hash, Eq, PartialEq)]
#[derive(Copy, Clone, Debug, Hash, Eq, PartialEq, Default)]
Copy link
Member

Choose a reason for hiding this comment

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

question(blocking): Why do we need to derive Default here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is used as a field in the Astc texture format enum variant. In order to enumerate each variant once strum needs this.

Copy link
Member

@ErichDonGubler ErichDonGubler left a comment

Choose a reason for hiding this comment

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

I'm inclined to Move Fast and Cover Things™ for tests, but the API changes seem like something we may not want to (stably) expose for downstreams. CC @gfx-rs/wgpu for thoughts on that specifically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Testing for texture formats

3 participants