KhronosGroup / KTX-Software

KTX (Khronos Texture) Library and Tools
Other
874 stars 229 forks source link

KTX breaks block alignment of compressed texture mip data with length field #161

Closed alecazam closed 4 years ago

alecazam commented 4 years ago

There seems to be a flaw with alignment in the KTX spec, so I thought I'd raise it here to make sure this is addressed in KTX2 (or some revision of KTX). The assumption is that 4 byte alignment is sufficient, but that breaks the ability the mmap and upload the mip levels on systems that have larger alignment requirements.

APIs like Metal expect that their blit encoder to receive mip levels that are a multiple of the block size. So with compressed textures like BC (8/16 byte block) or ASTC (16 byte block) blocks, these cannot be blit due to the imageSize that is written out. Compressed formats would all be completely aligned if it weren't for those 4 bytes on each mip level of the KTX file.

The following seems sufficient to address this, and the first sizePadding would also account for any non-block size multiple from the name-value pairs. This could also address block alignment of 64- and 128-bit explicit formats.

<- header already 64-bytes <- name-value pairs

for each mipmap_level in numberOfMipmapLevels UInt32 imageSize; <- imageSizePadding = (blockSize-1) - ((totalDataSize + (blockSize-1)) % blockSize; imageData...

MarkCallow commented 4 years ago

I don't think there is a problem in KTX1. mipPadding (the 4-byte alignment) is there to ensure the 32-bit imageSize values are correctly aligned. Since all known block-compressed formats have block-sizes that are a multiple of 4, mipPadding should always be zero for those formats. The length given by imageSize will be that of an integral number of blocks since that is what the formats require.

That said, as I look at it the KTX2 spec now, there doesn't seem to be any purpose to mipPadding as size information is no longer interleaved between between mip levels. Perhaps we should remove it. mip-levels will naturally align.

Do we need to ensure that the very first mip-level in the file is aligned on a block-size boundary? Currently that is align(8).

alecazam commented 4 years ago

The mip padding wasn't the issue in KTX1, although it wasn't enough to realign things.

  1. the header - already 64-bytes, so that's great, and much smaller and actually aligned compared to DDS
  2. the name-value pairs (these are 4 byte aligned)
  3. imageSize - 4 bytes
  4. data

so 2 and 3 both throw off the block alignment, but if we assume no name-value pairs, then just the imageSize throws off alignment. That's why either removing this, or adding padding after 3 is needed.

Do we need to ensure that the very first mip-level in the file is aligned on a block-size boundary? Currently that is align(8).

Yes, that wouldn't work for 16-byte aligned formats (most of BC and ASTC).

Sounds like mips are tight packed in KTX2, and so I think the only necessary padding would be after header and or name-value pairs, to get the start of the mip data back to block-aligned (8 or 16 bytes for compressed). 32, 48, 64, 96, 128 for 16f/32f data.

I do really like KTX1, but I was surprised at some of the alignment restrictions. With GL, you could mostly set it to 4, but r/rg/rgb8 often needed to be set to 1 if you had odd size mips. I could see the mip padding being useful there, just not on compressed or 4 component formats.

alecazam commented 4 years ago

Also with the exception of basis, any compression defeats mmap-ing the ktx/ktx2 directly anyways, but it would be good to have a padding option to the toktx (-blockAlignMips).

MarkCallow commented 4 years ago

I've opened https://github.com/KhronosGroup/KTX-Specification/issues/116 which is the right place for this discussion and will close this. @alecazam please subscribe to that issue, verify what it suggests is okay and offer any ideas you may have on how to express this in the spec.