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

BC7 color ramp interpolation #36

Closed
lexaknyazev opened this issue Jan 4, 2018 · 4 comments
Closed

BC7 color ramp interpolation #36

lexaknyazev opened this issue Jan 4, 2018 · 4 comments

Comments

@lexaknyazev
Copy link

Generation of color ramp uses different math than MSDN example.

Compare:
https://github.com/GPUOpen-Tools/Compressonator/blob/70d7d78f309f762b3c65fd112c2aa06e68ac06c6/Compressonator/Source/Codec/BC7/BC7_utils.cpp#L131-L142

and (from here):

UINT8 interpolate(UINT8 e0, UINT8 e1, UINT8 index, UINT8 indexprecision)
{
    if(indexprecision == 2)
        return (UINT8) (((64 - aWeights2[index])*UINT16(e0) + aWeights2[index]*UINT16(e1) + 32) >> 6);
    else if(indexprecision == 3)
        return (UINT8) (((64 - aWeights3[index])*UINT16(e0) + aWeights3[index]*UINT16(e1) + 32) >> 6);
    else // indexprecision == 4
        return (UINT8) (((64 - aWeights4[index])*UINT16(e0) + aWeights4[index]*UINT16(e1) + 32) >> 6);
}

The difference is that MSDN approach avoids floating-point calculations by using right-shift at the end.

This gives different results, e.g.:

mode 6:
color_bits = 7 + 1
ep0 = 1
ep1 = 82

index_bits = 4
index = 7 (30/64)

MSDN:
((64 - 30) * 1 + 30 * 82) >> 6 == 38

Compressonator:
1.0 * (1.0 - 0.46875) + 82.0 * 0.46875 + 0.5 == 39.46875
@NPCompress
Copy link
Contributor

Hi @lexaknyazev
Thanks for bringing this to our attention. The Ramp Decode for BC7 will be investigated to determine its accuracy with our CPU decoder implementation vs that with GPU decode results using a number of sample blocks.

Once completed the results and any changes will be posted alongside this issue report.

@xooiamd
Copy link
Contributor

xooiamd commented Feb 2, 2018

hi @lexaknyazev , using MSDN interpolation by bit shifting will affect the CPU decoder (as shown in the snapshot CPU_Decode_View image attached) and SSIM droped to 0.63. However, GPU decoder is fine ( as shown in the GPU_Decode_View image attached) and SSIM is 0.99.
gpu_decode_view
cpu_decode_view
Therefore, definition "USE_HIGH_PRECISION_INTERPOLATION_BC7" has been added to BC7_Definitions.h file in the latest commit in master branch. You can uncomment this definition to use the bit shifting MSDN defined interpolation if you are only interested in GPU Decoder. Thanks!

@lexaknyazev
Copy link
Author

Hmmm,.. That looks strange.
First of all, my example above is incorrect since it lacks + 32 operation. So, results are actually the same (after floor):

MSDN:
((64 - 30) * 1 + 30 * 82 + 32) >> 6 == 39

Compressonator:
1.0 * (1.0 - 0.46875) + 82.0 * 0.46875 + 0.5 == 39.46875

Moreover, I couldn't find any other example that shows any difference between these formulas, so it's a bit suspicious that decoded images are that different.

@xooiamd
Copy link
Contributor

xooiamd commented Feb 5, 2018

hi @lexaknyazev, the BC7 ramp calculation either by division and floor VS bit shifting produce same result. My attachment of the snapshot above are incorrect as I forgot the loop for cluster when calculating the ramp with bit shifting as shown by msdn. Both methods produce same result. Please see the latest commit 7ecb80f on the update. Thanks!

@xooiamd xooiamd closed this as completed Feb 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants