Skip to content

Add TextureFormat::srgb_view_formats#9435

Open
atlv24 wants to merge 1 commit intogfx-rs:trunkfrom
atlv24:ad/texfmt-srgb
Open

Add TextureFormat::srgb_view_formats#9435
atlv24 wants to merge 1 commit intogfx-rs:trunkfrom
atlv24:ad/texfmt-srgb

Conversation

@atlv24
Copy link
Copy Markdown
Collaborator

@atlv24 atlv24 commented Apr 16, 2026

Description
Bevy needs this functionality because of TextureDescriptor::view_formats taking a &'static [TextureFormat]:

https://github.com/bevyengine/bevy/blob/2d39c4f8e4cdd021881d0bba1ab4dd174d6db0ab/crates/bevy_image/src/image.rs#L57

https://github.com/bevyengine/bevy/blob/2d39c4f8e4cdd021881d0bba1ab4dd174d6db0ab/crates/bevy_image/src/image.rs#L1267

We don't need exactly this, we could do a more straightforward TextureFormat -> &[TextureFormat] function that doesnt add srgb, and combine it with the existing add_srgb_suffix, but that ends up being 400 lines of code and not really more useful: view_formats only allows diverging from the actual texture format by srgb suffix. The only other argument in favor of it is that it would be an exhaustive implementation, which would ensure that when new TextureFormats are added the function must be updated. If this is preferred I can PR that instead. Either way, it just feels wrong for this code to live in bevy, as it is paving over a wgpu api deficiency and likely will be needed by other wgpu users.

Testing
Its been in use in bevy for a while.

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.

@andyleiserson
Copy link
Copy Markdown
Contributor

It would be nice if we could do away with this use of &'static [TextureFormat] entirely. Option<TextureFormat> would be equally capable of supporting all the current use cases, without the borrowing implications. In hypothetical future cases where there are multiple view formats, requiring &'static copies of all the values that could be constructed at runtime becomes increasingly cumbersome.

Getting rid of it in wgpu doesn't look too bad: d261c07

But it is more complicated in Bevy because the texture descriptor structure is used in various places in various ways.

@inner-daemons
Copy link
Copy Markdown
Collaborator

inner-daemons commented Apr 20, 2026

view_formats only allows diverging from the actual texture format by srgb suffix

This actually isn't true, see aspect_specific_format. Basically, multi-planar formats allow you to create a view with sub-resource formats, but those should be in this list. You actually can't even create a texture view on such a format without using a subresource format IIRC.

See also this piece of code in my tests

Copy link
Copy Markdown
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

Bleh this is annoying that we need it.

/// Returns the srgb view formats if the format has an srgb variant, otherwise returns an empty slice.
///
/// The return result covers all the results of [`TextureFormat::add_srgb_suffix`].
pub fn srgb_view_formats(&self) -> &'static [TextureFormat] {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could we just get a test to make sure this matches add_srgb_suffix

@beicause
Copy link
Copy Markdown
Contributor

Then what about non_srgb_view_formats? I realized https://github.com/bevyengine/bevy/blob/0a056c7b37a0981f903f85e4e1dfb61cef55efbe/crates/bevy_image/src/image.rs#L1267 is wrong as it doesn't consider non-srgb view.

Actually I think changing it to use smallvec on the bevy side is also good.

@beicause
Copy link
Copy Markdown
Contributor

beicause commented May 1, 2026

I proposed changes in bevy bevyengine/bevy#24056

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.

5 participants