Vulkan-Samples icon indicating copy to clipboard operation
Vulkan-Samples copied to clipboard

Implement simple cache for decoded ASTC textures

Open JoseEmilio-ARM opened this issue 8 months ago • 5 comments

Description

Introduce a simple caching mechanism for decoded ASTC textures on platforms that do not support the format. Before decoding, the system will check if there are existing binaries saved locally, and otherwise create them for future runs.

Fixes #1202

General Checklist:

Please ensure the following points are checked:

  • [x] My code follows the coding style
  • [x] I have reviewed file licenses
  • [x] I have commented any added functions (in line with Doxygen)
  • [x] I have commented any code that could be hard to understand
  • [x] My changes do not add any new compiler warnings
  • [x] My changes do not add any new validation layer errors or warnings
  • [x] I have used existing framework/helper functions where possible
  • [x] My changes do not add any regressions
  • [x] I have tested every sample to ensure everything runs correctly
  • [x] This PR describes the scope and expected impact of the changes I am making

Note: The Samples CI runs a number of checks including:

  • [x] I have updated the header Copyright to reflect the current year (CI build will fail if Copyright is out of date)
  • [x] My changes build on Windows, Linux, macOS and Android. Otherwise I have documented any exceptions

If this PR contains framework changes:

  • [x] I did a full batch run using the batch command line argument to make sure all samples still work properly

JoseEmilio-ARM avatar Apr 23 '25 16:04 JoseEmilio-ARM

First of all, to make your PR work with vkb::scene_graph::components::HPPImage, you would need to add the data_hash to that class as well. Besides that, as the caching is an ASTC-specific thing, I would have expected that the hash lives in vkb::sg::ASTC.

With this PR, you calculate the hash for each and every image, and you calculate it based on the image data itself. Wouldn't it be more efficient, to just hash the URI of that file? That would probably require to just modify the vkb::GLTFLoader.

Thanks! Prefer to leave it as is, to support cases where the image itself changes, but not its URI.

JoseEmilio-ARM avatar Apr 25 '25 19:04 JoseEmilio-ARM

This PR breaks batch mode for me. When reacing the hlsl_shaders sample (which worked fine last time I did batch with master) it now asserts in create_vk_image.

I'm also getting a lot of (silent?) exceptions when running samples that decode astc now:

image

Not sure if any of these is related, but can't approve this without being able to do a batch run.

SaschaWillems avatar May 02 '25 19:05 SaschaWillems

This PR breaks batch mode for me. When reacing the hlsl_shaders sample (which worked fine last time I did batch with master) it now asserts in create_vk_image.

I can reproduce the issue, and found a workaround in my latest commit, but cannot really explain what is wrong, any ideas @asuessenbach ?

I'm also getting a lot of (silent?) exceptions when running samples that decode astc now:

These are probably the ones I introduced when a problem occurs trying to load an image from the cache. The first time the cache does not exist, so they will be thrown, but in later runs they should not be there anymore

JoseEmilio-ARM avatar May 05 '25 13:05 JoseEmilio-ARM

Regarding all those exceptions: I don't think, it's the right approach to use exceptions for non-exceptional cases. You could instead just use std::filesystem::exists() to check if the cached file exists.

And... having that data_hash in HPPImage as well, at the right position, made the hpp-based samples run correctly. But only because there's no hpp-based sample using an ASTC texture. If we get something like that, we'd need all those hash-related functions from Image there as well. So, would be great if you could add them.

asuessenbach avatar May 06 '25 17:05 asuessenbach

Any chance of speeding this up? I'm currently doing large changes to the framework, and testing is extremely tedious as all those high-level framework samples use ASTC and every time it needs to decode them. That makes testing very slow.

SaschaWillems avatar Jun 07 '25 15:06 SaschaWillems

LGTM 👍🏻

A minor suggestion: Add the astc cache folder to the .gitignore

Done, thanks!

JoseEmilio-ARM avatar Jul 14 '25 12:07 JoseEmilio-ARM

3 approvals, merging

marty-johnson59 avatar Jul 28 '25 14:07 marty-johnson59