Skip to content

Commit b619004

Browse files
authored
Fix ktxTexture2_DecodeAstc error returns and document them. (#961)
This started due to a warning when building documentation that `astc_encode.cpp` was not being found. After fixing that I noticed issues with the documentation for `ktxTexture2_DecodeAstc` which I fixed in c16537c. That commit also fixed an issue, where the function would fail with `KTX_INVALID_OPERATION` if a zlib or zstd supercompressed file had been opened without specifying the LOAD_DATA flag, by removing the check for `KTX_SS_NONE`. Subsequently I realized there still some supercompression scheme situations that need to be guarded against and I realized the function documentation was not listing the possible return values. While fixing these I spotted writes to stdout that this PR comments out. Libraries potentially used by GUI apps should not write to stdout. We could potentially add a flag parameter to request the ouput. Lastly all `astcenc` errors were being returned as KTX_INVALID_OPERATION. This maps astcenc errors to equivalent KTX_ errors with asserts on errors that are the fault of libktx or the way libktx has been compiled. Adding unit tests for the codec functions to texturetests would be a good thing.
1 parent c16537c commit b619004

File tree

1 file changed

+99
-17
lines changed

1 file changed

+99
-17
lines changed

lib/astc_codec.cpp

Lines changed: 99 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -530,6 +530,7 @@ struct LaunchDesc {
530530

531531
/**
532532
* @internal
533+
* @~English
533534
* @brief Helper function to translate thread entry points.
534535
*
535536
* Convert a (void*) thread entry to an (int, void*) thread entry, where the
@@ -573,6 +574,52 @@ launchThreads(int threadCount, void (*func)(int, int, void*), void *payload) {
573574
delete[] threadDescs;
574575
}
575576

577+
/**
578+
* @internal
579+
* @~English
580+
* @brief Map astcenc error code to KTX error code
581+
*
582+
* Asserts are fired on errors reflecting bad parameters passed by libktx
583+
* or astcenc compilation settings that do not permit correct operation.
584+
*
585+
* @param astc_error The error code to be mapped.
586+
* @return An equivalent KTX error code.
587+
*/
588+
static ktx_error_code_e
589+
mapAstcError(astcenc_error astc_error) {
590+
switch (astc_error) {
591+
case ASTCENC_SUCCESS:
592+
return KTX_SUCCESS;
593+
case ASTCENC_ERR_OUT_OF_MEM:
594+
return KTX_OUT_OF_MEMORY;
595+
case ASTCENC_ERR_BAD_BLOCK_SIZE: //[[fallthrough]];
596+
case ASTCENC_ERR_BAD_DECODE_MODE: //[[fallthrough]];
597+
case ASTCENC_ERR_BAD_FLAGS: //[[fallthrough]];
598+
case ASTCENC_ERR_BAD_PARAM: //[[fallthrough]];
599+
case ASTCENC_ERR_BAD_PROFILE: //[[fallthrough]];
600+
case ASTCENC_ERR_BAD_QUALITY: //[[fallthrough]];
601+
case ASTCENC_ERR_BAD_SWIZZLE:
602+
assert(false && "libktx passing bad parameter to astcenc");
603+
return KTX_INVALID_VALUE;
604+
case ASTCENC_ERR_BAD_CONTEXT:
605+
assert(false && "libktx has set up astcenc context incorrectly");
606+
return KTX_INVALID_OPERATION;
607+
case ASTCENC_ERR_BAD_CPU_FLOAT:
608+
assert(false && "Code compiled such that float operations do not meet codec's assumptions.");
609+
// Most likely compiled with fast math enabled.
610+
return KTX_INVALID_OPERATION;
611+
case ASTCENC_ERR_NOT_IMPLEMENTED:
612+
assert(false && "ASTCENC_BLOCK_MAX_TEXELS not enough for specified block size");
613+
return KTX_UNSUPPORTED_FEATURE;
614+
// gcc fails to detect that the switch handles all astcenc_error
615+
// enumerators and raises a return-type error, "control reaches end of
616+
// non-void function", hence this
617+
default:
618+
assert(false && "Unhandled astcenc error");
619+
return KTX_INVALID_OPERATION;
620+
}
621+
}
622+
576623
/**
577624
* @memberof ktxTexture2
578625
* @ingroup writer
@@ -605,10 +652,16 @@ launchThreads(int threadCount, void (*func)(int, int, void*), void *payload) {
605652
* The texture's images are 1D. Only 2D images can
606653
* be supercompressed.
607654
* @exception KTX_INVALID_OPERATION
608-
* ASTC compressor failed to compress image.
655+
* ASTC encoder failed to compress image.
656+
* Possibly due to incorrect floating point
657+
* compilation settings. Should not happen
658+
* in release package.
609659
* @exception KTX_INVALID_OPERATION
610660
* This->generateMipmaps is set.
611661
* @exception KTX_OUT_OF_MEMORY Not enough memory to carry out compression.
662+
* @exception KTX_UNSUPPORTED_FEATURE ASTC encoder not compiled with enough
663+
* capacity for requested block size. Should
664+
* not happen in release package.
612665
*/
613666
extern "C" KTX_error_code
614667
ktxTexture2_CompressAstcEx(ktxTexture2* This, ktxAstcParams* params) {
@@ -717,13 +770,13 @@ ktxTexture2_CompressAstcEx(ktxTexture2* This, ktxAstcParams* params) {
717770
&astc_config);
718771

719772
if (astc_error != ASTCENC_SUCCESS)
720-
return KTX_INVALID_OPERATION;
773+
return mapAstcError(astc_error);
721774

722775
astc_error = astcenc_context_alloc(&astc_config, threadCount,
723776
&astc_context);
724777

725778
if (astc_error != ASTCENC_SUCCESS)
726-
return KTX_INVALID_OPERATION;
779+
return mapAstcError(astc_error);
727780

728781
// Walk in reverse on levels so we don't have to do this later
729782
assert(prototype->dataSize && "Prototype texture size not initialized.\n");
@@ -785,11 +838,11 @@ ktxTexture2_CompressAstcEx(ktxTexture2* This, ktxAstcParams* params) {
785838
imageFree(input_image);
786839

787840
if (work.error != ASTCENC_SUCCESS) {
788-
std::cout << "ASTC compressor failed\n" <<
789-
astcenc_get_error_string(work.error) << std::endl;
841+
//std::cout << "ASTC compressor failed\n" <<
842+
// astcenc_get_error_string(work.error) << std::endl;
790843

791844
astcenc_context_free(astc_context);
792-
return KTX_INVALID_OPERATION;
845+
return mapAstcError(work.error);
793846
}
794847
}
795848
}
@@ -857,7 +910,7 @@ ktxTexture2_CompressAstcEx(ktxTexture2* This, ktxAstcParams* params) {
857910
* @return KTX_SUCCESS on success, other KTX_* enum values on error.
858911
*
859912
* @exception KTX_INVALID_OPERATION
860-
* The texture is already supercompressed.
913+
* The texture's images are supercompressed.
861914
* @exception KTX_INVALID_OPERATION
862915
* The texture's image are in a block compressed
863916
* format.
@@ -933,24 +986,53 @@ static void decompression_workload_runner(int thread_count, int thread_id, void*
933986
*
934987
* @param This The texture to decode
935988
* @param vkformat The decoding format to use
989+
*
990+
* @return KTX_SUCCESS on success, other KTX_* enum values on error.
991+
*
992+
* @exception KTX_FILE_DATA_ERROR
993+
* DFD is incorrect: supercompression scheme or
994+
* sample's channelId do not match ASTC colorModel.
995+
* @exception KTX_INVALID_OPERATION
996+
* The texture's images are not in ASTC format.
997+
* @exception KTX_INVALID_OPERATION
998+
* The texture object does not contain any data.
999+
* @exception KTX_INVALID_OPERATION
1000+
* ASTC decoder failed to decompress image.
1001+
* Possibly due to incorrect floating point
1002+
* compilation settings. Should not happen
1003+
* in release package.
1004+
* @exception KTX_OUT_OF_MEMORY Not enough memory to carry out decoding.
1005+
* @exception KTX_UNSUPPORTED_FEATURE
1006+
* The texture's images are supercompressed with an
1007+
* unsupported scheme.
1008+
* @exception KTX_UNSUPPORTED_FEATURE
1009+
* ASTC encoder not compiled with enough
1010+
* capacity for requested block size. Should
1011+
* not happen in release package.
9361012
*/
9371013
KTX_error_code
9381014
ktxTexture2_DecodeAstc(ktxTexture2 *This, ktx_uint32_t vkformat) {
9391015
// Decompress This using astc-decoder
9401016
uint32_t* BDB = This->pDfd + 1;
9411017
khr_df_model_e colorModel = (khr_df_model_e)KHR_DFDVAL(BDB, MODEL);
9421018
if (colorModel != KHR_DF_MODEL_ASTC) {
943-
return KTX_INVALID_OPERATION; // Not in valid astc decodable format
1019+
return KTX_INVALID_OPERATION; // Not in astc decodable format
1020+
}
1021+
if (This->supercompressionScheme == KTX_SS_BASIS_LZ) {
1022+
return KTX_FILE_DATA_ERROR; // Not a valid file.
9441023
}
1024+
// Safety check.
1025+
if (This->supercompressionScheme > KTX_SS_END_RANGE) {
1026+
return KTX_UNSUPPORTED_FEATURE; // Unsupported scheme.
1027+
}
1028+
// Other schemes are decoded in ktxTexture2_LoadImageData.
9451029

9461030
DECLARE_PRIVATE(priv, This);
9471031

9481032
uint32_t channelId = KHR_DFDSVAL(BDB, 0, CHANNELID);
949-
if (channelId == KHR_DF_CHANNEL_ASTC_DATA) {
950-
// Found astc data
951-
}
952-
else
1033+
if (channelId != KHR_DF_CHANNEL_ASTC_DATA) {
9531034
return KTX_FILE_DATA_ERROR;
1035+
}
9541036

9551037
// Create a prototype texture to use for calculating sizes in the target
9561038
// format and, as useful side effects, provide us with a properly sized
@@ -988,7 +1070,7 @@ ktxTexture2_DecodeAstc(ktxTexture2 *This, ktx_uint32_t vkformat) {
9881070
return result;
9891071
}
9901072
} else {
991-
// No data to transcode.
1073+
// No data to decode.
9921074
ktxTexture2_Destroy(prototype);
9931075
return KTX_INVALID_OPERATION;
9941076
}
@@ -1021,12 +1103,12 @@ ktxTexture2_DecodeAstc(ktxTexture2 *This, ktx_uint32_t vkformat) {
10211103
&astc_config);
10221104

10231105
if (astc_error != ASTCENC_SUCCESS)
1024-
return KTX_INVALID_OPERATION;
1106+
return mapAstcError(astc_error);
10251107

10261108
astc_error = astcenc_context_alloc(&astc_config, threadCount, &astc_context);
10271109

10281110
if (astc_error != ASTCENC_SUCCESS)
1029-
return KTX_INVALID_OPERATION;
1111+
return mapAstcError(astc_error);
10301112

10311113
decompression_workload work;
10321114
work.context = astc_context;
@@ -1077,10 +1159,10 @@ ktxTexture2_DecodeAstc(ktxTexture2 *This, ktx_uint32_t vkformat) {
10771159
astcenc_decompress_reset(astc_context);
10781160

10791161
if (work.error != ASTCENC_SUCCESS) {
1080-
std::cout << "ASTC decompressor failed\n" << astcenc_get_error_string(work.error) << std::endl;
1162+
//std::cout << "ASTC decompressor failed\n" << astcenc_get_error_string(work.error) << std::endl;
10811163

10821164
astcenc_context_free(astc_context);
1083-
return KTX_INVALID_OPERATION;
1165+
return mapAstcError(work.error);
10841166
}
10851167
}
10861168
}

0 commit comments

Comments
 (0)