remove pixel size and try to calculate texture sizes more correctly by hymm · Pull Request #6788 · bevyengine/bevy

hymm · 2022-11-28T21:27:15Z

Objective

Rather than maintain a big match statement get the info from wgpu instead. wgpu already maintains a list of this info https://docs.rs/wgpu-types/0.14.0/src/wgpu_types/lib.rs.html#2359

Solution

more complicated alternative to [Merged by Bors] - get pixel size from wgpu #6820
this mostly be considered a rebase of Texture formats #4124. Though some of the details are modified.
remove the TextureFormatPixelInfo trait. It is inaccurate to use pixel_size to calculate bytes when a texture is compressed
create a trait to calculate the usages of pixel_size directly. These are bytes_per_row, total_bytes, and rows_per_image.
use format.describe().block_size directly in situations where it is obvious that the format is uncompressed or where surrounding code assumes the texture is uncompressed.
add some debug asserts to functions that assume the texture is uncompressed.
note that all the internal uses of TextureFormatPixelInfo were on uncompressed textures. A smaller change would be to panic if in the pixel_info function if the texture is compressed and return block_size for type_size otherwise.

Breaking Changes

TextureFormatPixelInfo has been removed. PixelInfo::components can be gotten from texture_format.describe().components. PixelInfo::type_size can be gotten from texture_format.describe().block_size/ texture_format.describe().components. But note this can yield incorrect results for some texture types like Rg11b10Float. You might prefer to use the new functions on texture_descriptor instead. These include bytes_per_row, total_bytes and rows_per_image.

hymm · 2022-11-28T21:42:04Z

I think block_size =! type_size, so need to investigate more on what the equivalence is.

edit: I think it's block_size = type_size * components, but will go through the whole table and check

hymm · 2022-11-29T03:21:50Z

So there's one mismatch and that is on Depth24Plus. The size on that one before this PR is 3 and after this PR is 4. There is a FIXME on that line, so I'm inclined to trust wgpu on this one. Ideally we'd test it somehow, but I'm not sure how at the moment.

hymm · 2022-11-30T03:20:15Z

I now see there's an old PR that tried to do something similar. #4124. It seems like there's some issues around compressed textures that this PR might not be dealing with correctly.

Co-authored-by: Kurt Kühnert <kurt@kuhnert.dev>

superdump · 2022-11-30T08:02:37Z

I now see there's an old PR that tried to do something similar. #4124. It seems like there's some issues around compressed textures that this PR might not be dealing with correctly.

I knew I’d seen a PR for this before.

hymm · 2022-11-30T18:50:04Z

crates/bevy_render/src/texture/hdr_texture_loader.rs

    ) -> BoxedFuture<'a, Result<()>> {
        Box::pin(async move {
            let format = TextureFormat::Rgba32Float;
-            debug_assert_eq!(


this debug assert is sort of meaningless since we're hard coded to Rgba32Float

alice-i-cecile · 2022-12-11T18:12:57Z

Closing now that #6820 is getting merged. We can add the extra step in a new PR.

# Objective - Get rid of giant match statement to get PixelInfo. - This will allow for supporting any texture that is uncompressed, instead of people needing to PR in any textures that are supported in wgpu, but not bevy. ## Solution - More conservative alternative to #6788, where we don't try to make some of the calculations correct for compressed types. - Delete `PixelInfo` and get the pixel_size directly from wgpu. Data from wgpu is here: https://docs.rs/wgpu-types/0.14.0/src/wgpu_types/lib.rs.html#2359 - Panic if the texture is a compressed type. An integer byte size of a pixel is no longer a valid concept when talking about compressed textures. - All internal usages use `pixel_size` and not `pixel_info` and are on uncompressed formats. Most of these usages are on either explicit texture formats or slightly indirectly through `TextureFormat::bevy_default()`. The other uses are in `TextureAtlas` and have other calculations that assumes the texture is uncompressed. ## Changelog - remove `PixelInfo` and get `pixel_size` from wgpu ## Migration Guide `PixelInfo` has been removed. `PixelInfo::components` is equivalent to `texture_format.describe().components`. `PixelInfo::type_size` can be gotten from `texture_format.describe().block_size/ texture_format.describe().components`. But note this can yield incorrect results for some texture types like Rg11b10Float.

# Objective - Get rid of giant match statement to get PixelInfo. - This will allow for supporting any texture that is uncompressed, instead of people needing to PR in any textures that are supported in wgpu, but not bevy. ## Solution - More conservative alternative to bevyengine#6788, where we don't try to make some of the calculations correct for compressed types. - Delete `PixelInfo` and get the pixel_size directly from wgpu. Data from wgpu is here: https://docs.rs/wgpu-types/0.14.0/src/wgpu_types/lib.rs.html#2359 - Panic if the texture is a compressed type. An integer byte size of a pixel is no longer a valid concept when talking about compressed textures. - All internal usages use `pixel_size` and not `pixel_info` and are on uncompressed formats. Most of these usages are on either explicit texture formats or slightly indirectly through `TextureFormat::bevy_default()`. The other uses are in `TextureAtlas` and have other calculations that assumes the texture is uncompressed. ## Changelog - remove `PixelInfo` and get `pixel_size` from wgpu ## Migration Guide `PixelInfo` has been removed. `PixelInfo::components` is equivalent to `texture_format.describe().components`. `PixelInfo::type_size` can be gotten from `texture_format.describe().block_size/ texture_format.describe().components`. But note this can yield incorrect results for some texture types like Rg11b10Float.

get pixel info from wgpu

af44e98

james7132 added A-Rendering Drawing game state to the screen C-Code-Quality A section of code that is hard to understand or change labels Nov 28, 2022

james7132 approved these changes Nov 28, 2022

View reviewed changes

delete PixelInfo and just provide pixel_size

9781f12

hymm force-pushed the get-texture-info-from-wgpu branch from 2846343 to 9781f12 Compare November 28, 2022 22:09

james7132 added the M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label Nov 28, 2022

hymm changed the title ~~get pixel info from wgpu~~ get pixel size from wgpu Nov 29, 2022

remove pixel size usages

9f8295a

Co-authored-by: Kurt Kühnert <kurt@kuhnert.dev>

hymm commented Nov 30, 2022

View reviewed changes

hymm changed the title ~~get pixel size from wgpu~~ remove pixel size and try to calculate sizes more correctlier Dec 1, 2022

hymm changed the title ~~remove pixel size and try to calculate sizes more correctlier~~ remove pixel size and try to calculate sizes more correctly Dec 1, 2022

combine traits into one trait

fe17713

hymm force-pushed the get-texture-info-from-wgpu branch from 5d9b8dd to fe17713 Compare December 1, 2022 03:53

hymm changed the title ~~remove pixel size and try to calculate sizes more correctly~~ remove pixel size and try to calculate texture sizes more correctly Dec 2, 2022

hymm added 2 commits December 1, 2022 19:06

move trait onto texture descriptor for fewer input parameters

8afa805

clean up docs

42463cb

hymm mentioned this pull request Dec 2, 2022

[Merged by Bors] - get pixel size from wgpu #6820

Closed

alice-i-cecile closed this Dec 11, 2022

hymm deleted the get-texture-info-from-wgpu branch October 5, 2023 16:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

remove pixel size and try to calculate texture sizes more correctly#6788

remove pixel size and try to calculate texture sizes more correctly#6788
hymm wants to merge 6 commits intobevyengine:mainfrom
hymm:get-texture-info-from-wgpu

hymm commented Nov 28, 2022 •

edited

Loading

Uh oh!

hymm commented Nov 28, 2022 •

edited

Loading

Uh oh!

hymm commented Nov 29, 2022

Uh oh!

hymm commented Nov 30, 2022

Uh oh!

superdump commented Nov 30, 2022

Uh oh!

hymm Nov 30, 2022

Uh oh!

alice-i-cecile commented Dec 11, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

hymm commented Nov 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Objective

Solution

Breaking Changes

Uh oh!

hymm commented Nov 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hymm commented Nov 29, 2022

Uh oh!

hymm commented Nov 30, 2022

Uh oh!

superdump commented Nov 30, 2022

Uh oh!

hymm Nov 30, 2022

Choose a reason for hiding this comment

Uh oh!

alice-i-cecile commented Dec 11, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hymm commented Nov 28, 2022 •

edited

Loading

hymm commented Nov 28, 2022 •

edited

Loading