Add vkformat checks to ktxTexture2_DecodeAstc

[wasimabbas-arm]

• Adds checks for vkformat fixes ktxTexture2_DecodeAstc does not check its vkformat parameter #960
• Propogates vkformat correctly into creating astc profile

[wasimabbas-arm]

@MarkCallow I am struggling to understand the CI failures. In particular the TravisCI ones. Although one thing is consistent and good that the tests that are failing are all compare tests.

But all of the following are confusing me.

1. Its hard to tell which tests are actually ran because of the different number of tests on each platform and config. 733, 734, 741, 210 etc.

2. These all pass but 4790.13,4790.14,4790.15,4790.16 all run only 210 number of tests. While 4790.17 and 4790.18 doesn't have tests at all.

3. I test locally on a MacOs x86_64 with clang using ./scripts/build_macos.sh FEATURE_LOADTESTS=OFF gives me 741 tests and they all pass but if I run one of the tests that are failing in 4790.11 from travis like

./clitest.py --primary -e "/System/Volumes/Data/development/KTX-Software/build/macos-x86_64/Release/ktx" -d "/System/Volumes/Data/development/KTX-Software/build/macos-x86_64/Release/ktxdiff" tests/encode/compare.json --encode "{'outputTolerance': False, 'id': 'astc_0', 'flag': '--format ASTC_4x4_UNORM_BLOCK'}" --compare "{'id': 'psnr', 'flag': '--compare-psnr'}" --subcase "R8G8B8A8_UNORM_2D"

or just the whole thing

./clitest.py --primary -e "/System/Volumes/Data/development/KTX-Software/build/macos-x86_64/Release/ktx" -d "/System/Volumes/Data/development/KTX-Software/build/macos-x86_64/Release/ktxdiff" tests/encode/compare.json

it fails on my machine. How is that possible when it it runs part of the pack it succeeds but not on its own. I do see ktxToolsTest.encode.compare passing. Also if I remove the --primary it passes again however I get the warnings

WARNING: Allowed mismatch on non-primary platform between output file 'stdout' and reference file 'golden/encode/compare/astc_0_ssim_R8_UNORM_2D.out.txt'

Is that the explanation? that in the full run its not adding --primary and it passes but with warnings but I don't see them in regular run?

4. Also most of the MingW failing tests are --compare tests. Is this something to do with primary vs no primary platform?

I thought ASTC encoder has invariance accross platforms and compilers? Why are these happening any idea?
A question on the primary platform. If I build ktx with GCC on MacOs x86_64 can I regenerate the golden files then? Is that considered a primary platform?

[MarkCallow]

Thanks @wasimabbas-arm.

Do the tests for the vkformat parameter test all possible cases? If I'm reading correctly you are testing for 3 cases:

• Not an LDR format, if decoding ASTC HDR
• An sRGB format, if decoding sRGB ASTC LDR.
• Not an ASTC format.

What about, e.g. requesting a 16- or 32-bit or a FLOAT format for ASTC LDR? Or a depth-stencil format? There are many formats so it is probably better to confirm a match with the input ASTC format and the capabilities of the astcenc decoder than check for all the negative possibilities.

Please do the format checks consistently, checking This->vkFormat (via the DFD) first and then checking if vkformat is a suitable match. I got confused by the inconsistency during my review.

We definitely need unit tests for both the encode and decode functions.

[wasimabbas-arm]

Thanks @wasimabbas-arm.

Do the tests for the vkformat parameter test all possible cases? If I'm reading correctly you are testing for 3 cases:

* Not an LDR format, if decoding ASTC HDR

* An sRGB format, if decoding sRGB ASTC LDR.

* Not an ASTC format.

What about, e.g. requesting a 16- or 32-bit or a FLOAT format for ASTC LDR? Or a depth-stencil format? There are many formats so it is probably better to confirm a match with the input ASTC format and the capabilities of the astcenc decoder than check for all the negative possibilities.

Please do the format checks consistently, checking This->vkFormat (via the DFD) first and then checking if vkformat is a suitable match. I got confused by the inconsistency during my review.

@MarkCallow thanks for the review. Upon further analysis I have realised we don't really need the format argument. If we really need to decompress into 16- or 32-bit HDR formats that we can add later.

For now I have removed the argument and its corresponding checks and docs. I am now calculating the decoding format from the ASTC format. We only support 3 options. RGBA8, sRGBA8 and RGBA32.

We definitely need unit tests for both the encode and decode functions.

The compressions and decompressions are thoroughly tested part of the overal ktx encode|decode tests. What test would make sense for these specifically?

[MarkCallow]

Thanks for the quick fixes. Will review tomorrow.

We definitely need unit tests for both the encode and decode functions.

The compressions and decompressions are thoroughly tested part of the overall ktx encode|decode tests. What test would make sense for these specifically?

Yes, so I thought. Therefore the unittests should test proper handling of bad function parameters and verify that correct parameters result in the expected calls to astcenc. The latter is also covered to a large extent by the ktx encode tests. The focus here should be testing what other potential callers of the function may do. The changes in the most recent commit greatly reduce the surface area to be tested.

[MarkCallow]

The encode.compare test is failing on macOS and Linux. It appears that no output is being created, If you scroll up in the log you can find the output from clitest.py which shows the command to run the failing subcase. Run that command locally and you should be able to easily find out why no output is being produced.

I have merged the outstanding CTS update. You should update the test reference. This is unrelated to the above test failure, I think.

[wasimabbas-arm]

The encode.compare test is failing on macOS and Linux. It appears that no output is being created, If you scroll up in the log you can find the output from clitest.py which shows the command to run the failing subcase. Run that command locally and you should be able to easily find out why no output is being produced.

I can't reproduce these. All tests pass on my Mac x86_64. When you run those locally remember to change the path to ktx and ktxdiff to your local ktx and ktxdiff.

./clitest.py -e "/Users/travis/build/KhronosGroup/KTX-Software/build/Debug/ktx"` to 
./clitest.py -e "/System/Volumes/Data/development/KTX-Software/build/Release/ktx"

I have merged the outstanding CTS update. You should update the test reference. This is unrelated to the above test failure, I think.

Thank you. incoming.

[MarkCallow]

I can't reproduce these. All tests pass on my Mac x86_64. When you run those locally remember to change the path to ktx and ktxdiff to your local ktx and ktxdiff.

Neither can I. I note that the exit code shown for the failed tests is "-6". In python on Posix, this means the child process was killed by a signal 6 (SIGABRT). I don't know why. It may have something to do with macOS not finding a signed library or some other strange security issue though what could have changed on Travis I don't know. We haven't changed the macOS version requested on the runner.

I will be away the next 3 days and can do no more now. Please investigate if you can. We need to see the stdout and stderr output when the test is run. I don't recall how to make clitest.py display this. See if you can figure it out.

[MarkCallow]

Whatever the build problem is, it is to do with this PR. main builds fine.

[MarkCallow]

It does reproduce locally but only in the Debug config because the problem is:

Assertion failed: (ldr && "HDR sRGB profile not supported"), function astcProfile, file astc_codec.cpp, line 355.

This is because the sense of the test in isFormatAstcLDR at line 329 is wrong.

return (KHR_DFDSVAL(This->pDfd + 1, 0, QUALIFIERS) & KHR_DF_SAMPLE_DATATYPE_FLOAT) != 0;

is testing for HDR. Change the != to ==. Sorry for my error.

The test I am looking at, subcase 149, is providing an SRGB format input file and outputting to a file with SRGB in the name but is specifying --format ASTC_4x4_UNORM_BLOCK. If the astc encoder does legit sRGB to linear conversions, the output file name should not include SRGB. ? If it does not do conversions the --format value must be changed, unless you are intentionally testing mismatches.

[wasimabbas-arm]

It does reproduce locally but only in the Debug config because the problem is:

Woops, rookie mistake. Should have testesd both release and debug. Fixed in 0ead350

The test I am looking at, subcase 149, is providing an SRGB format input file and outputting to a file with SRGB in the name but is specifying --format ASTC_4x4_UNORM_BLOCK. If the astc encoder does legit sRGB to linear conversions, the output file name should not include SRGB. ? If it does not do conversions the --format value must be changed, unless you are intentionally testing mismatches.

Not sure what you mean by subcase 149. IIRC the test that was failing was running tests/encode/compare.json which I was running like

./clitest.py -e "ktx" -d "ktxdiff" tests/encode/compare.json --encode "{'outputTolerance': False, 'id': 'astc_0', 'flag': '--format ASTC_4x4_UNORM_BLOCK'}" --compare "{'id': 'psnr_ssim', 'flag': '--compare-psnr --compare-ssim'}"

If I just run that without any subcases. like

./clitest.py -e "ktx" -d "ktxdiff" tests/encode/compare.json

Do you mean the 149th line? [edit] found it in the ci logs.

ktx encode --testrun --threads 1 --format ASTC_4x4_UNORM_BLOCK --compare-ssim input/ktx2/valid_R8G8B8A8_SRGB_2D.ktx2 output/encode/compare/output_astc_0_ssim_R8G8B8A8_SRGB_2D.ktx2

[MarkCallow]

Do you mean the 149th line? [edit] found it in the ci logs.

No I mean the subcase as reported by clitest.py when a test fails.

    Subcase #149 FAILED:
      Expected return code '0' but got '-6'
      Cannot find output file 'output/encode/compare/output_astc_0_ssim_R8G8B8A8_SRGB_2D.ktx2'
      Run subcase with:
        clitest.py -e "/Users/mark/Projects/khronos/github/KTX-Software/build/macos/Debug/ktx" -d "/Users/mark/Projects/khronos/github/KTX-Software/build/macos/Debug/ktxdiff" tests/encode/compare.json --encode "{'outputTolerance': False, 'id': 'astc_0', 'flag': '--format ASTC_4x4_UNORM_BLOCK'}" --compare "{'id': 'ssim', 'flag': '--compare-ssim'}" --subcase "R8G8B8A8_SRGB_2D"

It is related to the array of subcases listed in compare.json.

[MarkCallow]

Is it intentional to generate UNORM ASTC images from SRGB input and compare input and output? IIRC ktx create silently decodes the SRGB input when a UNORM output format is specified. Provided the metrics tools decode sRGB to linear before running the metrics calculations this should not be a problem. I have to keep this in mind for another PR I am working on. I am thinking of stopping the automatic conversion to linear because users may not be aware of the colorspace of their input and such a conversion is likely to introduce banding to the image.

[wasimabbas-arm]

I think the sRGB topic needs a bit of walk through the whole system.

Is it intentional to generate UNORM ASTC images from SRGB input and compare input and output?

Do you mean specifically for comparison purposes? At least thats not the case in decode for ASTC. It is hardcoded to KTX_TTF_RGBA32 for basisu however. For ASTC I decode to the format of the texture ec = ktxTexture2_DecodeAstc(texture); If it was ASTC...SRGB it will be decoded to VK_FORMAT_R8G8B8A8_SRGB and for UNORM it will be VK_FORMAT_R8G8B8A8_UNORM.

IIRC ktx create silently decodes the SRGB input when a UNORM output format is specified.

This doesn't sound right. This means you can't create "UNORM" ktx files?

Provided the metrics tools decode sRGB to linear before running the metrics calculations this should not be a problem. I have to keep this in mind for another PR I am working on. I am thinking of stopping the automatic conversion to linear because users may not be aware of the colorspace of their input and such a conversion is likely to introduce banding to the image.

For encode if the input format is sRGB and the requested ASTC format also ASTC sRGB then no conversion needed.
If the requested format is ASTC UNORM however then the input data must be converted to Linear before it hits the compressor. Do we know if this is done somewhere before in the tool? I am not doing this in astc_encode.cpp. Same is true for create. Where there is input<->output mismatch it needs to be sorted before the compressor is invoked.

For metrics inside encode if the input format is sRGB then the decoded format will be VK_FORMAT_R8G8B8A8_SRGB. The decompressor doesn't even need to know about this, no conversion required. For basisu is always UNORM. Meaning for ASTC psnr comparisons are done in the asme color space while for basisu its not. Assuming

PSNR1 = psnr_basisu(t1, VK_FORMAT_R8G8B8A8_SRGB)
PSNR1 = psnr_basisu(t2, VK_FORMAT_R8G8B8A8_SRGB)
COMP1 = (PSNR1 == PSNR2)

PSNR3 = psnr_basisu(t1, VK_FORMAT_R8G8B8A8_UNORM)
PSNR4 = psnr_basisu(t2, VK_FORMAT_R8G8B8A8_UNORM)
COMP1 = (PSNR3 == PSNR4)

// AND
COMP1 == COMP2

Then it should be fine.

The test I am looking at, subcase 149, is providing an SRGB format input file and outputting to a file with SRGB in the name but is specifying --format ASTC_4x4_UNORM_BLOCK. If the astc encoder does legit sRGB to linear conversions, the output file name should not include SRGB. ?

As I said there is no conversion happening.

If it does not do conversions the --format value must be changed, unless you are intentionally testing mismatches.

In that specific test I am only interested in any astc format. The way that test is written there will always be a mismatch between the 'args:encode:flag' and 'args:subcase' formats and resultant file names.

This has however surfaced a problem, consider:

ktx encode --format ASTC_4x4_UNORM_BLOCK input_R8G8B8A8_SRGB.ktx2 output_R8G8B8A8_UNORM.ktx2

The issue is, AFAIK, there is no conversion from the input sRGB to output UNORM happening anywhere. Unless you think its happening before in the tool itself before it hits the decompressor?

[MarkCallow]

I think the sRGB topic needs a bit of walk through the whole system.

Is it intentional to generate UNORM ASTC images from SRGB input and compare input and output?

Do you mean specifically for comparison purposes?

I mean specifically for testing metrics during ASTC encoding as is being done here.

At least thats not the case in decode for ASTC. It is hardcoded to KTX_TTF_RGBA32 for basisu however. For ASTC I decode to the format of the texture ec = ktxTexture2_DecodeAstc(texture); If it was ASTC...SRGB it will be decoded to VK_FORMAT_R8G8B8A8_SRGB and for UNORM it will be VK_FORMAT_R8G8B8A8_UNORM.

--format here is the ktx create option so it decides the format of the created ktx2 file which will be UNORM. Per the above, ktxTexture2_DecodeAstc will then decode it to a UNORM format which metrics will then compare against the original sRGB input texture. It seems the same thing will happen if a BasisU texture is created from an sRGB input, if the transcoder is decoding to linear for when transcoding to KTX_TTF_RGBA32.

We'll have to investigate what the metrics computations are doing. I'll ask RasterGrid if they can provide some insight.

IIRC ktx create silently decodes the SRGB input when a UNORM output format is specified.

This doesn't sound right. This means you can't create "UNORM" ktx files?

You can create UNORM files. What I mean is the input sRGB data is silently decoded to linear before the texture is created.

For encode if the input format is sRGB and the requested ASTC format also ASTC sRGB then no conversion needed. If the requested format is ASTC UNORM however then the input data must be converted to Linear before it hits the compressor. Do we know if this is done somewhere before in the tool? I am not doing this in astc_encode.cpp. Same is true for create. Where there is input<->output mismatch it needs to be sorted before the compressor is invoked.

Yes it is done implicitly in ktx create as I've just noted. For the PR I mentioned I am considering making it so an explicit conversion request must be made..

As I said there is no conversion happening.

But there is, as I've explained above. ktx create creates a UNORM texture from converted sRGB data. That texture is decoded to a non-ASTC UNORM format and is compared against the sRGB original.

If it does not do conversions the --format value must be changed, unless you are intentionally testing mismatches.

In that specific test I am only interested in any astc format. The way that test is written there will always be a mismatch between the 'args:encode:flag' and 'args:subcase' formats and resultant file names.

Is it possible to re-write it in such a way that the --format value matches the input file format? Let's hold off on attempting this until we understand the effect of a mismatch on the metrics calculations and therefore whether it is necessary.

This has however surfaced a problem, consider:

ktx encode --format ASTC_4x4_UNORM_BLOCK input_R8G8B8A8_SRGB.ktx2 output_R8G8B8A8_UNORM.ktx2

The issue is, AFAIK, there is no conversion from the input sRGB to output UNORM happening anywhere. Unless you think its happening before in the tool itself before it hits the decompressor?

As I've already noted, conversion is happening.

[wasimabbas-arm]

As I said there is no conversion happening.

But there is, as I've explained above. ktx create creates a UNORM texture from converted sRGB data. That texture is decoded to a non-ASTC UNORM format and is compared against the sRGB original.

I mean the astc encoder/compressor/decompressor doesn't do any conversions. It only does compression/decompression.
If the compressor is provided sRGB data with an astcenc_profile=ASTCENC_PRF_LDR_SRGB then all is fine, but if its is provided sRGB data with an astcenc_profile=ASTCENC_PRF_LDR it will think the data is Linear.

[wasimabbas-arm]

Was there anything else pending on this PR?