KTX2 Updates: ETC1s/BasisLZ, ASTC HDR, and faster Zstd#18411
KTX2 Updates: ETC1s/BasisLZ, ASTC HDR, and faster Zstd#18411brianreavis wants to merge 48 commits intobevyengine:mainfrom
Conversation
|
You added a new feature but didn't add a description for it. Please update the root Cargo.toml file. |
brianreavis
changed the title
brianreavis
changed the title
|
Will try to get a KTX2 release out. |
kristoff3r
added
A-Rendering
|
The generated |
brianreavis
added
C-Performance
…" builds) (#18538) # Objective - Fixes #17891 - Cherry-picked from #18411 ## Solution The `name` argument could either be made permanent (by removing the `#[cfg(...)]` condition) or eliminated entirely. I opted to remove it, as debugging a specific DDS texture edge case in GLTF files doesn't seem necessary, and there isn't any other foreseeable need to have it. ## Migration Guide - `Image::from_buffer()` no longer has a `name` argument that's only present in debug builds when the `"dds"` feature is enabled. If you happen to pass a name, remove it.
|
There's some good stuff in there, but I think this PR should be broken up a bit. Like the KTX2 update could be one PR, the zstd_native/rust split could be another. I'm sure there are others too. Also, adding a bunch of binary assets isn't ideal if we want to keep the size of the repo in check. |
I can see the case for breaking out the zstd changes to its own PR. I'm not so sure about others being plausible and worthwhile – but am open to feedback. I'm going to hold off on the breakouts until this gets added to a milestone.
Any suggestions on this front? |
# Objective - add support for alternate zstd backend through `zstd` for faster decompression ## Solution - make existing `zstd` feature only specify that support is required, disambiguate which backend to use via two other features `zstd_native` and `zstd_rust`. - Similar to the approach taken by #18411, but we keep current behavior by defaulting to the rust implementation because its safer, and isolate this change. NOTE: the default feature-set may seem to not currently require `zstd`, but it does, it is enabled transitively by the `tonemapping_luts` feature, which is a default feature. Thus this does not add default features. ## Testing - Cargo clippy on both feature combinations
# Objective - Fix bug in dds channel order transcode - taken from #18411 ## Solution - fix it - improve docs a bit ## Testing - example in linked pr
# Objective - avoid several internal vec copies while collecting all the level data in ktx2 load - merge another little piece of #18411 (benchmarks there found this to be a significant win) ## Solution - reserve and extend ## Testing - ran a few examples that load ktx2 images, like ssr. looks fine ## Future work - fast path logic to skip the reading into different vecs and just read it all in one go into the final buffer instead - as above, but directly into gpu staging buffer perhaps
|
I think this is a very useful update, unfortunately the upstream basis-universal-rs does not seem to be under active maintenance, I would greatly appreciate it if bevy could maintain a fork.
I would suggest reducing the test combinations, e.g. remove other ASTC textures except 4x4 linear, 4x4 srgb, 6x6 hdr, only adding one ktx2 zstd variant, and remove uastc rdo textures. |
|
This PR should be remade at this point: a lot of related work has been merged, and resolving the diff and comments will be quite confusing. |
alice-i-cecile
added
S-Adopt-Me
7 participants
Objective
This PR expands Bevy’s supported KTX2 texture formats and transcoding capabilities (and .basis capabilities, as well). It also features some bugfixes and optimizations to improve speed and memory use. For more details on the changes, jump to the Changelog section below.
Updated Dependencies
ktx2to 0.4.0 for ASTC HDR constants (thanks @cwfitzgerald & @expenses)basis-universalto a forked version with updates:"encoding"and"transcoding"(for lighter builds)Faster Zstd Decompression
Users can now choose the backend for Zstd supercompressed files:
zstd(faster native bindings, new default) via the"zstd_native"feature.ruzstd(safer, previous Bevy default) via the"zstd_rust"feature.In benchmarks, reading a 284kb supercompressed KTX2 was 44% faster with
zstdthanruzstd.Showcase
cargo run --example texture_support --features="ktx2 basis-universal bmp gif exr ico jpeg pnm qoi tga tiff webp dds"To test behavior when a user has a GPU that does not support compressed textures, run with the
--no-compressed-texturesargument:cargo run --example texture_support --features="ktx2 basis-universal bmp gif exr ico jpeg pnm qoi tga tiff webp dds" -- --no-compressed-texturesFor Maintainers / Reviewers
Test by running the example above. When reviewing the changes, make sure you look for the "Load diff" links on ktx2.rs and texture_support.rs (they're easy to miss).
Important
Before merging, unless/until the basis-universal updates are submitted and merged upstream, I suggest forking my fork of basis-universal-rs and pointing to it. Also, fork the basis_universal submodule within it.
Related Issues
bevy_render#16859Migration Guide
"tonemapping_luts"or"ktx2"), specify the zstd implementation you want to use using one of the following feature flags:"zstd_native"(faster) or"zstd_rust"(safer)"compressed_image_saver"feature (this is no longer enabled by default for all builds)Changelog
Rgb32Float,Rgb32Sint,Rgb32Uint"zstd_native"feature).This mode is notably faster – performing 44% faster for a 284kb file.
R32Float,Rg32Float,Rgba32Float