[ktx2] Why aren't all mip levels padded to 8 bytes?

[zeux]

texture2.c has the following code:

    // Calculate size of the image data.
    This->dataSize = 0;
    for (ktx_uint32_t i = 0; i < This->numLevels; i++) {
        This->dataSize += _KTX_PAD8(private->_levelIndex[i].byteLength);
    }

dataSize is later used to load all mip levels in bulk from the file, starting from the offset of the first (smallest) mip level. So each mip level must be padded by 8 bytes or the file will fail to load.

However, the KTX2 specification for mip level array says:

    align(8) mipPadding // No padding the last time.

Implying that the last level must not be padded. Thus files written according to spec (without the last level padded) can't be loaded. I'm not sure if it's a spec issue or not; fwiw the "don't pad the last entry" in the spec seems redundant and harder than necessary to deal with, so we could fix the spec to require padding for all elements (including kvp, although it's technically redundant there as it's subsumed by the following 8-byte padding).

[MarkCallow]

I think the code is buggy. It is definitely the intention that the last level in the file, which is level 0 should not be padded. The following should work.

    // Level 0 is last in the file so is the one without padding.
    This->dataSize = private->_levelIndex[0].byteLength) ;
    for (ktx_uint32_t i = 1; i < This->numLevels; i++) {
        This->dataSize += _KTX_PAD8(private->_levelIndex[i].byteLength);
    }

I'm not averse to changing the spec to require padding on all mip levels though the above change doesn't seem difficult. It is also the intention that the last kvp padding is subsumed by the following 8-byte padding.

I'd welcome a PR to fix the code.

[zeux]

One extra question re: spec is whether extra bytes are permitted after the contents. That is, say we change the code in libktx to remove padding from the last level when reading - are files that are being written by libktx (that have the extra padding) valid?

[zeux]

Other places where last mip is padded:

• texture.c, ktxTexture_calcDataSizeLevels (with the comment explicitly calling this out?):

        /* mipPadding. NOTE: this adds padding after the last level too. */
        if (fv == KTX_FORMAT_VERSION_TWO)
            dataSize += _KTX_PAD8(levelSize);

• writer2.c, ktxTexture2_writeToStream:

        // Write entire level.
        result = dststr->write(dststr, This->pData + srcLevelOffset,
                               levelSize, 1);
        if (result == KTX_SUCCESS) {
            align8PadLen = _KTX_PAD8_LEN(levelSize);
            if (align8PadLen != 0)
             result = dststr->write(dststr, padding, 1, align8PadLen);
        }

[zeux]

This has been addressed in the spec (thank you!), so closing.