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

Implement ETC2 and ASTC textures #1074

Merged
merged 2 commits into from
Dec 6, 2020
Merged

Implement ETC2 and ASTC textures #1074

merged 2 commits into from
Dec 6, 2020

Conversation

cwfitzgerald
Copy link
Member

@cwfitzgerald cwfitzgerald commented Dec 4, 2020

Connections

Closes #1070. Makes progress towards #1069.

Description

This PR has multiple functions:

  • Adds ETC and ASTC compressed textures behind features.
  • Adds three helper functions on Extent3d that help in calculating mip sizes: at_mip_level, max_mips, and physical_size.
  • Refactors various conversions into a public TextureFormat::describe function. I have used a decl macro to ease in the declaration and modifcation of an otherwise horribly verbose function.

I have tried to use clever multi-select based data copying to reduce the possibility for errors, but there's a lot of data moving around here.

Testing

Upcoming wgpu-rs pr adding wider compressed texture support to the skybox example. Helper functions were tested with doctests.

Marked as draft until I can get the wgpu-rs pr done and prove it works, it is, however, ready for review.

@cwfitzgerald cwfitzgerald requested a review from kvark December 4, 2020 23:54
Copy link
Member

@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.

Great work here! Looking forward to have all the compressed formats supported :)

wgpu-core/src/instance.rs Show resolved Hide resolved
wgpu-core/src/validation.rs Outdated Show resolved Hide resolved
wgpu-types/src/lib.rs Outdated Show resolved Hide resolved
wgpu-types/src/lib.rs Outdated Show resolved Hide resolved
wgpu-types/src/lib.rs Show resolved Hide resolved
wgpu-types/src/lib.rs Outdated Show resolved Hide resolved
Copy link
Member

@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.

Wonderful! Please rebase on #1071 and land this when ready.

@cwfitzgerald cwfitzgerald force-pushed the astc-etc branch 3 times, most recently from bafc8e2 to 5a4b3bc Compare December 5, 2020 05:46
@cwfitzgerald
Copy link
Member Author

I fixed up the validation to better deal with the difference between virtual and physical sizes in compressed texture mips. Could you review the latest commit as that's entirely new code.

/// Take `value` and round it up to the nearest alignment `alignment`.
///
/// ```text
/// (0, 3) -> 0
Copy link
Member

Choose a reason for hiding this comment

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

should this be written as Rust assertions?

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried, but because the conv module isn't visible from the outside, and doctests are technically completely different crates, I couldn't actually call the function in the doctest.

@@ -1881,8 +1881,8 @@ impl Extent3d {
let block_width = block_width as u32;
let block_height = block_height as u32;

let width = ((self.width + block_width - 1) / block_width) * block_width;
let height = ((self.height + block_height - 1) / block_height) * block_height;
let width = (((self.width + block_width) - 1) / block_width) * block_width;
Copy link
Member

Choose a reason for hiding this comment

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

what happened here?

Copy link
Member Author

Choose a reason for hiding this comment

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

... I thought I was fixing precidence but I didn't change it at all, let me give this another look

@kvark
Copy link
Member

kvark commented Dec 5, 2020

Please rebase and land this!

@cwfitzgerald
Copy link
Member Author

Thanks for the review!

bors r=kvark

@bors
Copy link
Contributor

bors bot commented Dec 6, 2020

@bors bors bot merged commit 7b886f4 into gfx-rs:master Dec 6, 2020
bors bot added a commit to gfx-rs/wgpu-rs that referenced this pull request Dec 6, 2020
664: Integrate ETC and ASTC textures in Skybox example r=kvark a=cwfitzgerald

Depends on gfx-rs/wgpu#1074

Converts skybox example to mip-mapped files in:
 - Bgra8
 - Bc1
 - ETC2
 - ASTC4x4

Looks quite good now with mipmapping and trilinear filtering even at small resolutions.

Co-authored-by: Connor Fitzgerald <[email protected]>
kvark pushed a commit to kvark/wgpu that referenced this pull request Jun 3, 2021
664: Integrate ETC and ASTC textures in Skybox example r=kvark a=cwfitzgerald

Depends on gfx-rs#1074

Converts skybox example to mip-mapped files in:
 - Bgra8
 - Bc1
 - ETC2
 - ASTC4x4

Looks quite good now with mipmapping and trilinear filtering even at small resolutions.

Co-authored-by: Connor Fitzgerald <[email protected]>
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.

ETC and ASTC compression
2 participants