Skip to content
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

Create should verify ASTC options and the format provided #808

Closed
abbaswasim opened this issue Nov 30, 2023 · 5 comments · Fixed by #809
Closed

Create should verify ASTC options and the format provided #808

abbaswasim opened this issue Nov 30, 2023 · 5 comments · Fixed by #809

Comments

@abbaswasim
Copy link
Contributor

If an ASTC encode is intended by providing any of the astc encode options to "ktx create" the options should be verified for compatibility along side the VK_FORMAT provided.

@MarkCallow
Copy link
Collaborator

MarkCallow commented Nov 30, 2023

Currently, per the documentation, astc encoder arguments are ignored if create's --format argument is not an ASTC format. It seems like users will easily be caught out by this. I agree with @abbaswasim that an error should be flagged in this case. @lexaknyazev, @aqnuep what do you think?

@lexaknyazev
Copy link
Member

Makes sense to reject invalid commands.

@aqnuep
Copy link
Collaborator

aqnuep commented Nov 30, 2023

Can you clarify which argument combinations are you talking about? It is unclear from the description.

@wasimabbas-arm
Copy link
Contributor

wasimabbas-arm commented Nov 30, 2023

I tried --format R8G8B8A8_SRGB --astc-quality fastest --astc-perceptual and apparently these are incompatible but not reported.

I guess generally any format that is not ASTC format should be flagged if used with those ASTC options.

@aqnuep
Copy link
Collaborator

aqnuep commented Nov 30, 2023

I see, so the problem is that the ASTC options are accepted to be specified even when the format is not ASTC. Understood.

MarkCallow pushed a commit that referenced this issue Dec 7, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Fixes #808.
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 a pull request may close this issue.

5 participants