-
Notifications
You must be signed in to change notification settings - Fork 1k
Support Sliced 3D for ASTC #7577
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
base: trunk
Are you sure you want to change the base?
Support Sliced 3D for ASTC #7577
Conversation
Adjusted after running |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks! Looks mostly good to me, some changes proposed in comments.
Would be great to have a test for this that generates astc textures, but iirc we don't have that for other compressed formats either :/
@@ -531,6 +531,7 @@ impl super::Adapter { | |||
.compressed_texture_astc_supports_ldr_profile() | |||
{ | |||
features.insert(wgt::Features::TEXTURE_COMPRESSION_ASTC); | |||
features.insert(wgt::Features::TEXTURE_COMPRESSION_ASTC_SLICED_3D); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit confused about the extension zoo here tbh. So apparently [WEBGL_compressed_texture_astc
])https://registry.khronos.org/webgl/extensions/WEBGL_compressed_texture_astc/) implies all of KHR_texture_compression_astc_hdr
, which in turn says It includes support for 2D and slice-based 3D textures, with low and high dynamic range, at bitrates from below 1 bit/pixel up to 8 bits/pixel in fine steps.
Which kinda implies that for webgl, either profile should imply sliced 3d.
The else
block (native gl) should then instead check for GL_KHR_texture_compression_astc_sliced_3d
to activate the feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Wumpf good one, I missed the native GL block. I tried to address in latest commit! PLMK if it helps sufficiently.
@@ -1301,6 +1304,19 @@ bitflags_array! { | |||
/// This is a web and native feature. | |||
const TEXTURE_COMPRESSION_ASTC = WEBGPU_FEATURE_TEXTURE_COMPRESSION_ASTC; | |||
|
|||
|
|||
/// Allows the 3d dimension for textures with ASTC compressed formats. | |||
/// |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would be nice to mention on the above feature that volume textures aren't included unless TEXTURE_COMPRESSION_ASTC_SLICED_3D
is enabled.
Wondering what to do with the non-webgpu _HDR
variant, seems a bit underspeced 🤔. I'm fine with ignoring that for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Wumpf makes sense, does the new commit look sufficient?
Co-authored-by: Andreas Reich <r_andreas2@web.de>
@Wumpf Thank you very much for the review. Besides accepting the suggestion, I made a commit to address the other ones. For testing in general for compression maybe a tracking issue can be created however webgpu-samples already include an astc-compressed example data (and CTS covers ASTC Sliced 3D), or I can try to help in a consecutive PR to have it generate directly inside repo. Appreciated. Hope the commits help. |
Connections
Fixes #7576
Description
This PR aims to enable
"texture-compression-astc-sliced-3d"
for backends that support ASTC (such as excluding D3D12).If any aspects could be improved to make it more suitable for the project, please let me know, and I will adjust it. Thank you very much in advance!
Testing
Mainly
cargo xtask test
.Squash or Rebase?
Squash
Checklist
cargo fmt
.taplo format
.cargo clippy --tests
. If applicable, add:--target wasm32-unknown-unknown
cargo xtask test
to run tests.CHANGELOG.md
entry.