-
-
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
Added DDS BC6H reading #6449
Added DDS BC6H reading #6449
Conversation
f6bf186
to
37f766b
Compare
Thanks very much for figuring this out! |
Our habit is to add tests with PRs. This helps prevent regressions in the future. If you're someone who works with BC6 images, do you happen to have an image that we can add to our test suite, and distribute under the Pillow license? |
I have several images that can be used as a reference. I'm unsure about their origin. If that's okay, I can attach them. I originally wanted to add tests, but there are no python bindings for BC6H. Are there C API tests of some kind? |
I've created ShadelessFox#1 to add Python bindings. Just making sure there is an explanation of the change here - you looked at the code, and as per https://docs.microsoft.com/en-us/windows/win32/direct3d11/bc6h-format#transform-inversion-for-endpoint-values, saw that not all of the deltas were being applied, yes? Previously, the B0 delta was incorrectly being applied to each of the RGB channels, right? |
Thanks, I will look into that later. I was looking at Khronos' specs, and it says the following:
Where |
Decoding error were caused by additional sign extend call after endpoint transform, according to khronos documentation, you only suppose to sign extend endpoints only once, further calls to sign extend mangles endpoint data.
I noticed that the provided bit packing table differs from the table provided by Khronos. You can find its textual representation here: The following python script can be used to transform it into a suitable format: def transform(data: str) -> str:
result = []
for elem in data.split(','):
split = elem.index('[')
name = elem[0:split]
indices = elem[split + 1:len(elem) - 1]
if ':' in indices:
a, b = map(int, indices.split(':'))
if a > b:
indices = [x for x in range(b, a + 1, +1)]
else:
indices = [x for x in range(b, a - 1, -1)]
else:
indices = [int(indices)]
for index in indices:
result.append(endpoints[name] << 4 | index)
return ', '.join(map(str, result)) Update: wrong spec link |
Restored unimplemented DXGI format test
I hope you don't mind this mess. I can squash commits if you want. |
Not a problem. It's good to be able to look back and understand where the changes came from. |
All this started from an idea of previewing textures in a java application, both I and @ShadelessFox ended up implementing BC1-BC7 decoders, to see who can get it working first. @ShadelessFox found an issue with delta decoding and I found an issue with the signed format. |
Let me know if this PR requires more polishing before it can be merged. |
Rename format to BC6H and BC6HS
Hi @radarhere, Is there anything I must do before this PR can be merged? |
I'm not aware of any problems at the moment, and you've added tests, so it's just a matter of myself or someone else finding the time to review. The Valgrind failure happening here isn't due to your code. I've raised a separate issue for it - #6468 |
Interestingly, I found that Table 58 at https://registry.khronos.org/DataFormat/specs/1.1/dataformat.1.1.html#_bc6h has some mistakes in it (e.g. Mode 1, Bit 4 should be G35 as per Table 57). Everything in this PR makes sense to me, except for |
Updated BC6H test images
It's not the part of the specification. I had to perform gamma correction so the output image looks fine without washed-out colors due to the latter float16 -> int8 conversion required by Pillow. AFAIK Pillow does not support floating point storage? |
There is an open issue for multichannel floating point images - #1888. I'm not following why that should lead to washed-out colors though? If I remove the gamma correction, then the output matches what I get when saving the DDS image as a PNG from macOS Preview. I've created ShadelessFox#6 |
Sorry, not washed-out but "deep-fried". If you're okay with the result, then me either. |
Removed gamma correction
Fixes #6344