Data format descriptor by superdump · Pull Request #4 · BVE-Reborn/ktx2

superdump · 2022-01-23T17:53:35Z

Checklist

cargo clippy reports no issues
cargo doc reports no issues
cargo deny issues have been fixed or added to deny.toml
cargo test shows all tests passing
human-readable change descriptions added to the changelog under the "Unreleased" heading.
- If the change does not affect the user (or is a process change), preface the change with "Internal:"
- Add credit to yourself for each change: Added new functionality @githubname.

Description

Add support for parsing Data Format Descriptor sections from KTX2 files.

This is functionally the same but perhaps an Unsupported variant should be added to the enums for the catch all?

superdump · 2022-01-23T17:59:36Z

To appease clippy I had to change some 0 | _ => SomeType::Unspecified to just _ => as it rightly points out that the _ matches anything else anyway. I had left it as 0 | _ as some kind of documentation. 0 is explicitly in the spec as an 'unspecified' type, presumably so it can be used for custom data without breaking things for other people as ids get standardised. However, as ids get standardised, they won't be in the enums. It feels wrong that a constructor function would return a Result so perhaps adding an Unsupported to the enums and mapping 0 to Unspecified as per the spec, and anything that the crate doesn't know about to Unsupported. However, it would be a shame to break/block user code just because it is an unknown type by the crate. Maybe instead adding an Unknown(pub u32) or whatever is a better option...? Mmm, I like that last option.

Ralith

Thanks for working on this! Out of curiosity, what's your use case for this data?

src/lib.rs

superdump · 2022-01-23T20:26:44Z

Thanks for working on this! Out of curiosity, what's your use case for this data?

tl;dr so I can load supercompressed and compressed textures from KTX2 files.

I pulled in the ktx2 crate to bevy to try to add support for some compressed texture formats (also ddsfile). As I was hooking things up and trying to get something working, I noticed that the sample I had was supercompressed and had no format. I looked at the spec and it seemed like parsing the Data Format Descriptor was necessary to figure out the format of the contained data. Indeed after implementing this I was able to understand that the textures contained in the sample that I had were UASTC format, which I think need additional transcoding to formats that can be directly sampled by a GPU like BC*/ASTC/ETC2.

So, my goal is support for KTX2 as a container primarily for compressed textures, with or without mipmaps, and perhaps also supporting cubemaps, even if long-term bevy has a different and custom asset metadata format to be able to configure whatever the engine needs.

The asset pipeline is the next priority in bevy I think, but it’s good to have good support for glTF 2.0, and KTX2/DDS out of the box.

Ralith · 2022-01-23T22:57:44Z

I noticed that the sample I had was supercompressed and had no format

Interesting, didn't realize that was at all common. That makes this a pretty key feature!

…ns an Iterator

Note that while 0 means Unspecified, and ironically is defined in the specification with a particular meaning, because pseudo_enum uses NonZeroU32, it cannot represent 0. As such, I chose to make the members Option<T> and have None mean Unspecified.

src/lib.rs

…ator

src/lib.rs

Fallible parsing returning ParseError Separate out DataFormatDescriptorHeader. Remove size member. Derive Eq + PartialEq. Add constant for DFD header for Basic variant. Remove data_format_descriptor_type method.

src/lib.rs

Ralith

Thanks, this is looking good! Just a couple nits left.

src/lib.rs

Ralith

LGTM, thanks for all your work!

superdump · 2022-02-03T20:54:50Z

LGTM, thanks for all your work!

Thanks for all the review! :) It’s for sure a better PR because of it.

superdump added 3 commits January 23, 2022 18:45

Initial shot at parsing the Data Format Descriptor sections

cdf928c

Appease clippy

b023701

This is functionally the same but perhaps an Unsupported variant should be added to the enums for the catch all?

Add credit to changelog

b15478a

superdump mentioned this pull request Jan 23, 2022

Compressed texture support bevyengine/bevy#3608

Closed

2 tasks

Add Unknown(u32) for standardized enum values unknown by the crate

2544aff

Ralith requested changes Jan 23, 2022

View reviewed changes

src/lib.rs Outdated Show resolved Hide resolved

src/lib.rs Outdated Show resolved Hide resolved

src/lib.rs Outdated Show resolved Hide resolved

src/lib.rs Outdated Show resolved Hide resolved

src/lib.rs Show resolved Hide resolved

superdump added 3 commits January 23, 2022 22:38

Remove explicit #[inline]

80d3bbc

Remove UNINITIALIZED and NONE flags

9767eaf

Make data_format_descriptors() return an Iterator

f23ada2

superdump added 2 commits January 25, 2022 18:40

Add DataFormatDesctiptor::sample_informations(data: &[u8]) that retur…

dfff3d2

…ns an Iterator

superdump force-pushed the data-format-descriptor branch from 0f7a3b4 to 4ff2308 Compare January 25, 2022 18:06

cargo fmt

f3641a4

Ralith requested changes Jan 25, 2022

View reviewed changes

src/lib.rs Outdated Show resolved Hide resolved

Pipe data through BasicDataFormatDescriptor for SampleInformationIter…

74bb216

…ator

Ralith requested changes Jan 29, 2022

View reviewed changes

src/lib.rs Outdated Show resolved Hide resolved

src/lib.rs Outdated Show resolved Hide resolved

src/lib.rs Outdated Show resolved Hide resolved

src/lib.rs Outdated Show resolved Hide resolved

Address some review feedback

3f0b978

Ralith requested changes Jan 29, 2022

View reviewed changes

src/lib.rs Outdated Show resolved Hide resolved

src/lib.rs Outdated Show resolved Hide resolved

src/lib.rs Show resolved Hide resolved

src/lib.rs Outdated Show resolved Hide resolved

src/lib.rs Outdated Show resolved Hide resolved

src/lib.rs Outdated Show resolved Hide resolved

Iterate on review feedback

ba1b558

Fallible parsing returning ParseError Separate out DataFormatDescriptorHeader. Remove size member. Derive Eq + PartialEq. Add constant for DFD header for Basic variant. Remove data_format_descriptor_type method.

Ralith requested changes Feb 1, 2022

View reviewed changes

src/lib.rs Outdated Show resolved Hide resolved

src/lib.rs Outdated Show resolved Hide resolved

superdump added 2 commits February 2, 2022 22:46

Validate data format descriptor in Reader::new()

37260ed

Fix data format descriptor iterator data slice start

e2c4ad1

Ralith requested changes Feb 3, 2022

View reviewed changes

src/lib.rs Outdated Show resolved Hide resolved

src/lib.rs Outdated Show resolved Hide resolved

superdump added 2 commits February 3, 2022 21:33

Only check if there is enough data for the data format descriptor block

8dca93d

Re-slice &[u8] instead of maintaining a start offset

cd14b46

Ralith approved these changes Feb 3, 2022

View reviewed changes

Ralith merged commit b7ab91d into BVE-Reborn:trunk Feb 3, 2022

Conversation

superdump commented Jan 23, 2022

Checklist

Description

Uh oh!

superdump commented Jan 23, 2022

Uh oh!

Ralith left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

superdump commented Jan 23, 2022

Uh oh!

Ralith commented Jan 23, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Ralith left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Ralith left a comment

Choose a reason for hiding this comment

Uh oh!

superdump commented Feb 3, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants