KhronosGroup / KTX-Specification

KTX file format source
Other
69 stars 12 forks source link

Add instructions for registering vendor schemes and metadata #196

Closed MarkCallow closed 1 year ago

MarkCallow commented 1 year ago

The catalyst for this is a request from a vendor to register a supercompression scheme. As part of setting up a model for everyone to follow this moves the BasisLZ global data spec. to an appendix and moves that and the ETC1S bitstream specification appendix to external files included in the main ktxspec.adoc.

Several issues spotted during the work are included here:

  1. Fix error in description of block ordering in ETC1S slices.
  2. Clarify level data layout for BasisLz/ETC1S.
  3. Fix list of data that comprises a mip level.

1 & 2 can't be seen in a git diff because the BasisLZ/ETC1S bitstream specification has been moved to an external file so the entire file is shown in the diff. Here are the relevant diffs for those changes.

The bug fix.

diff --git a/sc-schemes/KTX/basislz.adoc b/sc-schemes/KTX/basislz.adoc
index 92bce53..88631ca 100644
--- a/sc-schemes/KTX/basislz.adoc
+++ b/sc-schemes/KTX/basislz.adoc
@@ -11,7 +11,7 @@ processed endpoint and selector data from the block-compression and
 the Huffman tables. The global data block contains this codebook.

 It also contains an array of _image descriptors_ with flags for
-each image and the offset and length within the image's mip level
+each image and the offset and length within the mip level
 of the data for that image.

 The global data structure is designed to accommodate various
@@ -386,7 +386,7 @@ Any remaining bits may be safely ignored.
 [#etc1s_slice]
 === ETC1S Slice Decoding

-ETC1S slices consist of a compressed 2D array of ETC1S blocks, always compressed in top-down/left-right raster order. For an animation sequence, the previous slice's already decoded contents may be referred to when blocks are encoded using Conditional Replenishment (also known as "`skip blocks`").
+ETC1S slices consist of a compressed 2D array of ETC1S blocks, compressed in the order indicated by <<KTXorientation>> metadata (defaults to top-down/left-right raster order). For an animation sequence, the previous slice's already decoded contents may be referred to when blocks are encoded using Conditional Replenishment (also known as "`skip blocks`").

 Each ETC1S block is encoded by using references to the color endpoint codebook and the selector codebook. The following sections describe the helper procedures used by the decoder, and how the array of ETC1S blocks is actually decoded.

The clarification.

diff --git a/sc-schemes/KTX/basislz.adoc b/sc-schemes/KTX/basislz.adoc
index 9b2056e..2b15f71 100644
--- a/sc-schemes/KTX/basislz.adoc
+++ b/sc-schemes/KTX/basislz.adoc
@@ -352,6 +352,8 @@ Any remaining bits may be safely ignored.
 [#etc1s_slice]
 === ETC1S Slice Decoding

+The data for each mip level is a tightly packed array of ETC1S slices indexed by the offsets in the `ImageDesc` array in the <<basislz_global_data_structure>>.
+
 ETC1S slices consist of a compressed 2D array of ETC1S blocks, compressed in the order indicated by <<KTXorientation>> metadata (defaults to top-down/left-right raster order). For an animation sequence, the previous slice's already decoded contents may be referred to when blocks are encoded using Conditional Replenishment (also known as "`skip blocks`").

 Each ETC1S block is encoded by using references to the color endpoint codebook and the selector codebook. The following sections describe the helper procedures used by the decoder, and how the array of ETC1S blocks is actually decoded.

@lexaknyazev please review.

MarkCallow commented 1 year ago

@lexaknyazev please review.

lexaknyazev commented 1 year ago

ETC1S slices consist of a compressed 2D array of ETC1S blocks, compressed in the order indicated by <<KTXorientation>> metadata (defaults to top-down/left-right raster order).

ETC1S block encoding is fixed, see Section C.5.3.

The data for each mip level is a tightly packed array ...

The new statement is ambiguous. The exact packing rules are provided in rgbSliceByteOffset, rgbSliceByteLength, alphaSliceByteOffset, and alphaSliceByteLength descriptions and BasisLZ Scheme notes, Section 3.8.1.

MarkCallow commented 1 year ago

ETC1S slices consist of a compressed 2D array of ETC1S blocks, compressed in the order indicated by <<KTXorientation>> metadata (defaults to top-down/left-right raster order).

ETC1S block encoding is fixed, see Section C.5.3.

But the Basis encoder has no idea of the orientation of the incoming image data. It processes the pixels in the order received merely assuming it is top-down/left-right.

The data for each mip level is a tightly packed array ...

The new statement is ambiguous. The exact packing rules are provided in rgbSliceByteOffset, rgbSliceByteLength, alphaSliceByteOffset, and alphaSliceByteLength descriptions and BasisLZ Scheme notes, Section 3.8.1.

The reason for the new statement is that Section D.5 (was C.5) is the destination of the link from the Level Data Format column of the supercompressionSchemes table and it is very confusing to end up at a section talking about the format of a slice. How about "The data for each mip level is an array of ETC1S slices indexed by the offsets in the ImageDesc array in the [BasisLZ Global Data Structure]. See Section 3.8.1."

lexaknyazev commented 1 year ago

Basis encoder has no idea of the orientation of the incoming image data.

Endpoint predictors explicitly refer to left/upper/upper-left blocks. The section should probably be rewritten to avoid these terms.

How about ...

Nit: ImageDesc is a structure name, the array is called imageDescs.

The data for each mip level is a set of ETC1S slices. The corresponding element of the imageDescs array provides slice locations within the mip level...

MarkCallow commented 1 year ago

The section should probably be rewritten to avoid these terms.

Yes. Can we leave it until post 2.0.2 or do you have time to tackle it now?

The data for each mip level is a set of ETC1S slices. The corresponding element of the imageDescs array provides slice locations within the mip level...

Good. I'll make that change.