-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
DDS BC5_SNORM images are decoded incorrectly #7386
Comments
It'll take some more investigation to figure out the problem, but I can at least say that this shouldn't be the case. Here is the ATI2 code in Python, Pillow/src/PIL/DdsImagePlugin.py Lines 176 to 179 in b8b78ed
and then where your image is handled. Pillow/src/PIL/DdsImagePlugin.py Lines 193 to 196 in b8b78ed
If you're interested, almost all of the decoding logic for this lives in our C code. Pillow/src/libImaging/BcnDecode.c Lines 852 to 864 in b8b78ed
|
In that case, the issue is most likely in Pillow/src/libImaging/BcnDecode.c Line 114 in b8b78ed
For reference: this is how DirectXTex does it. |
What program did you use to convert the image to your "expected" PNG? |
I've created PR #7401. With it, this is the result. What do you think? |
Sorry for the late response @radarhere and thank you for fixing this! I used texconv to decode the DDS file, but other programs (e.g. PhotoShop) give the same result (no idea what they are doing/using under the hood). So I'm not sure whether the new output is 100% correct. BC5 doesn't define a B channel (I think), so arguably the new output is technically correct, but I think the output of texconv and co makes more logical sense. These programs say that the B channel is 0. For BC5_UNORM, we actually get B=0 in the output, but for BC5_SNORM, these programs output B=128. This is because 0 (SNORM) is mapped to 128 (UNORM). |
I find making changes not outlined by a specification to be awkward, but the fact that texconv is developed by Microsoft, who wrote the specification, is a pretty good argument for mimicking that behaviour. |
I've created #7413 to resolve this. |
That looks perfect. Thank you @radarhere! |
What did you do?
I loaded a BC5 SNORM images.
What did you expect to happen?
For it to be decoded correctly.
What actually happened?
Pillow incorrectly decoded the image. It added artifacts and washed out the "colors" of the stored normals.
This is speculation on my part (I haven't read PIL's source code), but it seems like PIL uses the same algorithm for BC5_UNORM (ATI2) and BC5_SNORM.
What are your OS, Python and Pillow versions?
The DDS file in question: cables_normal.zip
The text was updated successfully, but these errors were encountered: