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

Fix exporting R8 files as PNG using libpng #503

Merged
merged 5 commits into from
Sep 3, 2024
Merged

Conversation

brunoanc
Copy link
Contributor

@brunoanc brunoanc commented Sep 2, 2024

Fixes an incorrect stride value for R8 images being exported to PNG on the experimental libpng SaveToPNGFile implementation.

Also updated to make it clear interlaced images are not supported.

Fixes an incorrect stride value for R8 images being exported to PNG on
the experimental libpng SaveToPNGFile implementation.
@walbourn walbourn self-assigned this Sep 2, 2024
@walbourn walbourn added bug wsl Related to Windows Subsystem for Linux support labels Sep 2, 2024
@walbourn
Copy link
Member

walbourn commented Sep 2, 2024

@luncliff Do you agree with this fix?

When testing this I found that GetMetadataFromFile wasn't fully initializing the metadata struct.
@walbourn
Copy link
Member

walbourn commented Sep 3, 2024

I add a formal test for this code, and verified this is the right fix.

Copy link
Contributor

@luncliff luncliff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mostly worked with reading/writing colored images.
Nice fix!

R8_UNORM shouldn't use TEX_ALPHA_MODE flags.
added error since interlacing fails
Trim whitespace
@walbourn
Copy link
Member

walbourn commented Sep 3, 2024

The CLA was triggered due to my edits.

@microsoft-github-policy-service agree company="Microsoft"

@walbourn
Copy link
Member

walbourn commented Sep 3, 2024

Updating test suite to cover this code explicitly walbourn/directxtextest#46

@walbourn walbourn self-requested a review September 3, 2024 02:45
Copy link
Member

@walbourn walbourn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix. Added a few more after testing.

@walbourn walbourn merged commit c401f2f into microsoft:main Sep 3, 2024
53 of 54 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug wsl Related to Windows Subsystem for Linux support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants