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

Add ktx deflate command #896

Merged
merged 15 commits into from
Apr 18, 2024
Merged

Add ktx deflate command #896

merged 15 commits into from
Apr 18, 2024

Conversation

MarkCallow
Copy link
Collaborator

@MarkCallow MarkCallow commented Apr 17, 2024

For deflating an existing ktx 2.0 file.

Fixes part of #747. A complete fix requires either ktx encode support ASTC or a new ktx encode-astc command.

@MarkCallow
Copy link
Collaborator Author

Questions for reviewers:

  1. The command raises an error if neither --zlib or --zstd is specified. CompressOptions does not check for this since it is fine to not specify a scheme in either create or encode. Should I move the check to CompressOptions with some kind of option to enable it or is it fine in deflate's processOptions.
  2. The command preserves previously applied options in KTXwriterScParams by either maintaining the original KTXwriter, if it was a member of the ktx suite, or appending the original writer and params to the end of the new KTXwriterScParams. See comment at
    // ======= KTXwriter and KTXwriterScParams metadata handling =======
    for complete details.

@MarkCallow MarkCallow requested a review from aqnuep April 17, 2024 09:52
@MarkCallow
Copy link
Collaborator Author

@lexaknyazev please review. For some reason your id doesn't show up in the potential reviewers list.

@MarkCallow MarkCallow marked this pull request as draft April 17, 2024 10:04
@MarkCallow MarkCallow marked this pull request as ready for review April 18, 2024 11:32
@MarkCallow MarkCallow merged commit da97911 into main Apr 18, 2024
18 checks passed
@MarkCallow MarkCallow deleted the deflate branch April 18, 2024 21:56
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.

2 participants