Skip to content

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

Open
wants to merge 19 commits into
base: trunk
Choose a base branch
from

Conversation

mehmetoguzderin
Copy link
Contributor

@mehmetoguzderin mehmetoguzderin commented Apr 19, 2025

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

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

@mehmetoguzderin mehmetoguzderin requested review from crowlKats and a team as code owners April 19, 2025 00:21
@mehmetoguzderin
Copy link
Contributor Author

Adjusted after running cargo clippy --tests --target wasm32-unknown-unknown locally. Thank you!

@ErichDonGubler ErichDonGubler self-assigned this Apr 21, 2025
@ErichDonGubler ErichDonGubler removed their assignment Apr 23, 2025
@cwfitzgerald cwfitzgerald self-assigned this Apr 23, 2025
Copy link
Member

@Wumpf Wumpf left a 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);
Copy link
Member

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.

Copy link
Contributor Author

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.
///
Copy link
Member

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.

Copy link
Contributor Author

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?

mehmetoguzderin and others added 2 commits April 27, 2025 22:49
Co-authored-by: Andreas Reich <r_andreas2@web.de>
@mehmetoguzderin
Copy link
Contributor Author

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

@mehmetoguzderin mehmetoguzderin requested a review from Wumpf April 27, 2025 20:05
@cwfitzgerald cwfitzgerald assigned Wumpf and unassigned cwfitzgerald Apr 30, 2025
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.

Sliced 3D for ASTC
5 participants