Fix BC6 block decoder

[ShadelessFox]

Fixes #6344.

Before	After

[radarhere]

Thanks very much for figuring this out!

[radarhere]

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?

[ShadelessFox]

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?

[radarhere]

I've created https://github.com/ShadelessFox/Pillow/pull/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?

[ShadelessFox]

Thanks, I will look into that later.

I was looking at Khronos' specs, and it says the following:

If the mode has transformed endpoints, the values from E0 are used as a base to offset all other endpoints, wrapped at the number of endpoint bits. For example, R1 = (R0 + R1) & ((1 << EPB) - 1).

Where E0 is (R0, G0, B0). The provided reference shows the operation just for a single component, and others were implied.

[ShadelessFox]

I noticed that the provided bit packing table differs from the table provided by Khronos.

You can find its textual representation here: https://registry.khronos.org/OpenGL/extensions/ARB/ARB_texture_compression_bptc.txt

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

[ShadelessFox]

I hope you don't mind this mess. I can squash commits if you want.

[radarhere]

Not a problem. It's good to be able to look back and understand where the changes came from.

[REDxEYE]

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.

[ShadelessFox]

Let me know if this PR requires more polishing before it can be merged.

[ShadelessFox]

Hi @radarhere,

Is there anything I must do before this PR can be merged?

[radarhere]

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

[radarhere]

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 bc6_gamma_correct. That doesn't seem to be mentioned in the specification?

[ShadelessFox]

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?

[radarhere]

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 https://github.com/ShadelessFox/Pillow/pull/6

[ShadelessFox]

Sorry, not washed-out but "deep-fried". If you're okay with the result, then me either.