Tweak decoders, add encoder and CLI - #2
Open
jmhobbs wants to merge 6 commits into
Open
Conversation
DecodeDXT1 never implemented the 3-color + transparent-black mode: color3 always used the opaque interpolation regardless of color0 vs color1, instead of decoding to black when color0 <= color1. DecodeDXT3 and DecodeDXT5 incorrectly branched color2 on the actual color0/color1 relationship. Per spec their color block must always use the opaque encoding, treating color0 as though it were greater than color1 regardless of the real values. Replaced that branch with a new c2_opaque() used only by DXT3/DXT5.
DecodeDXT5 built its 48-bit alpha index field starting at the block's first byte, which is actually alpha0. According to spec, alpha0/alpha1 occupy bytes 0-1 and the 48-bit index field is the following 6 bytes. The existing code read its first index bits out of the alpha reference bytes and never read the block's last 2 bytes at all. testdata/dxt5.decoded was regenerated from the corrected decoder
The dxt package deals in non-premultiplied alpha bytes, which NRGBA is and RGBA is not.
Owner
|
Thanks for your contribution! I will do a more throughout review during the weekend and merge it. At first glance your PR looks great and enhances library. Special thanks for creating a CLI tool as well |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR does a couple things;
Some decisions were made around quantization and alpha on DXT1/DXT3 which could be configurable, but are not. For example DXT1 "alpha" is on/off, so we split the RGBA alpha down the middle and turn it on/off at 128.
As an example, here are the outputs of a round-trip through the CLI (encode to DXT*, decode back to PNG) for this test file: