Skip to content
This repository has been archived by the owner on Dec 29, 2022. It is now read-only.

iffy looking scaling code in LogicalASTCBlock::ColorAt() #9

Open
richgel999 opened this issue Aug 20, 2019 · 2 comments
Open

iffy looking scaling code in LogicalASTCBlock::ColorAt() #9

richgel999 opened this issue Aug 20, 2019 · 2 comments

Comments

@richgel999
Copy link

Hi,
The code to compute 8-bit non-sRGB output doesn't seem to follow the method outlined in the ASTC spec:

const int c = (c0 * (64 - weight) + c1 * weight + 32) / 64;
// TODO(google): Handle conversion to sRGB or FP16 per C.2.19.
const int quantized = ((c * 255) + 32767) / 65536;
assert(quantized < 256);

The spec says "If sRGB conversion is not enabled and the decoding mode is decode_unorm8, then the top 8 bits of the interpolation result for the R, G, B and A channels are used as the final result.":

Section 23.19

The difference will be very slight, and I'm not sure if this actually causes any issues at all because the scale factors are so similar. I'm pointing it out here because there's a comment, and because I noticed that the implementation deviates from the spec in a way that could break bit-exact decoding.

@richgel999
Copy link
Author

I have verified that the code which is checked in (that scales and divs by 65536) is definitely not producing correct results, by comparing this decoder's output vs. another. The fix is simple:

int quantized = c >> 8;

@richgel999
Copy link
Author

Also, this decoder needs an "sRGB" flag that the user can pass in, because it impacts how the 8-bit endpoints are scaled to 16-bits before the interpolation. Without this you can't correctly validate the codec when sRGB is enabled:

https://www.khronos.org/registry/DataFormat/specs/1.2/dataformat.1.2.html#astc_weight_application

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant