Tighten Vulkan / GL enums about ktx-specification HOT 39 CLOSED

A few points in no particular order.

It is not my intent to force loaders to attempt to load any format on any implementation of anything. GL loaders should refuse to load a texture if glInternalFormat is GL_FORMAT_UNKNOWN (or whatever we end up with for this) and Vulkan loaders should refuse to load a texture if vkFormat is VK_FORMAT_UNKNOWN. I have written "should" here. Obvious spec language would be "must not" but I do not want to rule out using KTX files to store multi-sample textures for which there are no formats in either API. So I will most like use "may not".

If both glInternalFormat and vkFormat are set to known values, the values must match otherwise the file is invalid. I don't think there is any language today that states this. I need suggestions for how to specify what "match" means.

For GL loaders glInternalFormat is the important field. glType and glFormat are there to make it easier for loaders. We can add a table like Table 8.2 in OpenGL ES that specifies valid combinations of format, type and internalformat.

Re the Vulkan "scaled" formats most Vulkan implementations do not support using these formats as textures so I have no problem disallowing them in KTX2. There is an issue in the spec. regarding these.

That leaves the BGRA format issue. Note that the values of GL_BGRA_EXT in the apple extension and GL_BGRA in glcorearb.h are the same so from the p.o.v. of KTX2 they're identical. It looks like OpenGL converts those to whatever internalformat is specified. OpenGL ES < 3 converts them to GL_RGBA. I need to look further into what happens on OpenGL ES >= 3. One way to solve this is to have in the valid combination table format = GL_BGRA & internalformat = GL_BGRA8_EXT. Loaders can make best effort to load into versions of OpenGL that don't support GL_RGBA8_EXT and failing with unsupported format is acceptable. Or we could just forbid use of BGRA with OpenGL.

from ktx-specification.

It is not my intent to force loaders to attempt to load any format on any implementation of anything. GL loaders should refuse to load a texture if glInternalFormat is GL_FORMAT_UNKNOWN (or whatever we end up with for this) and Vulkan loaders should refuse to load a texture if vkFormat is VK_FORMAT_UNKNOWN.

Looking at the current API usage trends (like ANGLE and VkPortability), it seems that the format should be as platform-agnostic as possible yet avoiding edge cases that require runtime conversions.

For GL loaders glInternalFormat is the important field.

Does it mean that KTX2 follows OpenGL ES notion of glInternalFormat?

glType and glFormat are there to make it easier for loaders.

Sorry, I don't follow. Loader will need to validate three fields (glInternalFormat, glFormat, and glType) before pushing them to OpenGL (since we cannot always trust drivers) instead of deriving glFormat and glType from pre-made correct lookup table.

from ktx-specification.

Does it mean that KTX2 follows OpenGL ES notion of glInternalFormat?

Yes it should. We don't want to encourage data conversions so glFormat and glType should match glinternalFormat.

Sorry, I don't follow. Loader will need to validate three fields

Yes you are right. I was thinking too narrowly of just the GL loader. It can pass the fields to OpenGL {,ES} and let the API do the validation. But as you have pointed out, it is not appropriate to rely on OpenGL {,ES} because of the variability between them and across implementations.

I'm coming round to your idea of having just the vkFormat field. I was worried about the size of the switch statement needed to map to equivalent GL formats, which is why I left the GL fields in, but the validation code is probably as large or larger.

To ensure consistency we could add a table to the spec. giving the required mapping, if there is any ambiguity, maybe in an appendix as it will be a long table.

from ktx-specification.

If we drop the GL fields, what should we do about glTypeSize. This gives the size of data type and is used when byte swapping is necessary. Do we keep it, renamed to just typeSize or do we have readers divine it from the value of vkFormat?

Part of me rebels against the idea of thousands of applications having these large tables vs. a handful of KTX2 writers. That is why the GL fields are still there. If we ditch typeSize readers will continue to need a big switch statement to divine the typeSize even if not supporting OpenGL.

from ktx-specification.

I would definitely prefer the option is to have a canonical mapping from a vkFormat to the required gl enums for loading using GL or other legacy APIs vs having multiple fields in. KTX is somewhat notorious for having lots of bad, buggy implementations, and having multiple fields which represent the same information but in different way, and which can become unsychronized with each other is one source of those bugs. Library and tool authors naturally ask themselves why a given piece of information is represented in multiple ways and sometimes come to the conclusion that there is something unexpected they're supposed to do with one of the representations.

If a large switch statement is required to divine all the appropriate GL enums required, that's because of the failings of the GL API, and shouldn't be propagated to KTX2.

from ktx-specification.

@jherico, thanks for the comment. Even readers not supporting OpenGL will need a large switch statement or table to map from a VkFormat enum to the size of its type, if we remove typeSize from KTX2.

For readers supporting GL loading, the same table with added data can be used to map vkFormat to the proper GL format, internalformat and type. The size of the table is purely down to the number of formats in Vulkan.

from ktx-specification.

glTypeSize description is a bit confusing to me. There're several types of texture formats:

• integers of 8/16/32/64 bit per channel (I guess these correspond to 1/2/4/8)
• floats of 16/32/64 bit per channel (should glTypeSize be 1?)
• packed formats like 565 or 5551, i.e., 8/16/32 bits per pixel (should glTypeSize be 1?)
• block-compressed formats with 64 or 128 bits per block (the spec says that glTypeSize should be 1)
• recently added video-plane formats (some of them are packed, some are aligned)

If endianness conversion is expected to affect only the first group, the "switch case" with vkFormat is very small because VkFormat enum values are pre-sorted by type.

Size 2:

• [70, 75]
• [77, 82]
• [84, 89]
• [91, 96]

Size 4:

• [98, 99]
• [101, 102]
• [104, 105]
• [107, 108]

Size 8:

• [110, 111]
• [113, 114]
• [116, 117]
• [119, 120]

All ranges are inclusive.

Also, the spec should have some language about whether depth/stencil formats are supported (this may affect ranges above).

from ktx-specification.

First-glance implementation seems to be quite compact (definitely not the biggest part of loader):

int getTypeSizeFromVkFormat(int vkFormat) {
    if (vkFormat >= 70 && vkFormat <= 96 && 
        vkFormat != 76 && vkFormat != 83 && vkFormat != 90) {
        return 2;
    }

    if (vkFormat >= 98 && vkFormat <= 108 && 
        vkFormat != 100 && vkFormat != 103 && vkFormat != 106) {
        return 4;
    }

    if (vkFormat >= 110 && vkFormat <= 120 && 
        vkFormat != 112 && vkFormat != 115 && vkFormat != 118) {
        return 8;
    }

    return 1;
}

from ktx-specification.

There're several types of texture formats:

• integers of 8/16/32/64 bit per channel (I guess these correspond to 1/2/4/8)

Yes.

• floats of 16/32/64 bit per channel (should glTypeSize be 1?)

No. Floating point values typically need endianness conversion as well so glTypesize should be set accordingly to 2, 4 or 8.

• packed formats like 565 or 5551, i.e., 8/16/32 bits per pixel (should glTypeSize be 1?)

No. glTypesize is set according to the size of the type the components are packed in to. For 565 & 5551 that is a uint16_t so typesize is 2.

• block-compressed formats with 64 or 128 bits per block (the spec says that glTypeSize should be 1)

These are not byte oriented so no endiannness conversion is needed. glTypeSize is set to 1 to indicate that.

• recently added video-plane formats (some of them are packed, some are aligned)

See above re packed. Not sure what you mean by "aligned". Probably we need to consult Andrew on these.

Please make suggestions how I can improve the explanation of glTypeSizein KTX 1 to avoid he confusion you felt. If we remove glTypeSize from KTX2 we will still need a section on endianness conversion that explains what readers must do in the event of a mismatch with the writer.

from ktx-specification.

Thanks for the clarification re float and packed formats.

Formats added in Vulkan 1.1 are quite unique and may need some additional treatment re planes layout from KTX (unless we want to disallow them). A few examples from the Vulkan 1.1 spec (there're 34 of them with different layouts):

Updated code that covers all Vulkan 1.0 formats (integers, floats, packed, compressed, depth, stencil)

int getTypeSizeFromVkFormat(int vkFormat) {
    if ((vkFormat >= 2 && vkFormat <= 8) || // pack 16
        (vkFormat >= 70 && vkFormat <= 97) || // regular 16
        vkFormat == 124) { // D16_UNORM
        return 2;
    }

    if (vkFormat == 128) return 3; // D16_UNORM_S8_UINT

    if ((vkFormat >= 51 && vkFormat <= 69) || // pack 32
        (vkFormat >= 98 && vkFormat <= 109) || // regular 32
        vkFormat == 122 || // B10G11R11_UFLOAT_PACK32
        vkFormat == 123 || // E5B9G9R9_UFLOAT_PACK32
        vkFormat == 125 || // X8_D24_UNORM_PACK32
        vkFormat == 126 || // D32_SFLOAT
        vkFormat == 129) { // D24_UNORM_S8_UINT
        return 4;
    }

    if (vkFormat == 130) return 5; // D32_SFLOAT_S8_UINT

    if (vkFormat >= 110 && vkFormat <= 121) { // regular 64
        return 8;
    }

    return 1;
}

from ktx-specification.

@lexaknyazev You need to account for edge conditions and formats added by extensions. Presumably that would be easiest to do by changing return 1 to if (vkFormat > VK_FORMAT_UNDEFINED && vkFormat <= VK_FORMAT_ASTC_12x12_SRGB_BLOCK) { return 1; } else { return -1; }. That way a subsequent extension that adds a new format doesn't automatically get 1 as a type size.

from ktx-specification.

Sure. I'm just evaluating how much code is needed. Updated snippet with your suggestion and my first-impression understanding (which might be incorrect) of new Vulkan 1.1 formats (some range checks may be optimized by using bitmasks, let's focus on high-level question first):

int getTypeSizeFromVkFormat(int vkFormat) {
    if ((vkFormat >= 2 && vkFormat <= 8) || // pack 16
        (vkFormat >= 70 && vkFormat <= 97) || // regular 16
        vkFormat == 124 || // D16_UNORM
        vkFormat == 1000156007 || // R10X6_UNORM_PACK16
        vkFormat == 1000156017 || // R12X4_UNORM_PACK16
        (vkFormat >= 1000156027 && vkFormat <= 1000156033)) { // multi-planar & 2x1 16 bpc
        return 2;
    }

    if (vkFormat == 128) return 3; // D16_UNORM_S8_UINT

    if ((vkFormat >= 51 && vkFormat <= 69) || // pack 32
        (vkFormat >= 98 && vkFormat <= 109) || // regular 32
        vkFormat == 122 || // B10G11R11_UFLOAT_PACK32
        vkFormat == 123 || // E5B9G9R9_UFLOAT_PACK32
        vkFormat == 125 || // X8_D24_UNORM_PACK32
        vkFormat == 126 || // D32_SFLOAT
        vkFormat == 129 || // D24_UNORM_S8_UINT
        vkFormat == 1000156000 || // G8B8G8R8_422_UNORM
        vkFormat == 1000156001 || // B8G8R8G8_422_UNORM
        vkFormat == 1000156008 || // R10X6G10X6_UNORM_2PACK16
        vkFormat == 1000156018) { // R12X4G12X4_UNORM_2PACK16
        return 4;
    }

    if (vkFormat == 130) return 5; // D32_SFLOAT_S8_UINT

    if ((vkFormat >= 1000156012 && vkFormat <= 1000156016) || // multi-planar 10 bpc
        (vkFormat >= 1000156022 && vkFormat <= 1000156026)) { // multi-planar 12 bpc
        return 6;
    }

    if ((vkFormat >= 110 && vkFormat <= 121) || // regular 64
        (vkFormat >= 1000156009 && vkFormat <= 1000156011) || // RGBA10 & 2x1 10 bpc
        (vkFormat >= 1000156019 && vkFormat <= 1000156021)) { // RGBA12 & 2x1 12 bpc
        return 8;
    }

    if ((vkFormat > 0 && vkFormat <= 184) || // all other known core formats
        (vkFormat >= 1000054000 && vkFormat <= 1000054007) || // PVRTC
        (vkFormat >= 1000156002 && vkFormat <= 1000156006)) { // multi-planar 8 bpc
        return 1;
    }
    
    return -1;
}

This is arguably more complex than the initial version above, so I'm interested what do people think re deriving typeSize from vkFormat on load vs reading both values from the file.
Part of me screams "never trust the input" so having two fields won't keep KTX2 loader from validation which would require roughly the same procedure as deriving typeSize at runtime.

from ktx-specification.

Thanks for the code @lexaknyazev. Is it possible to use the enum names rather than values?

For the 2 example Vulkan 1.1 formats you added, for G8B8G8R8_422_UNORM the spec states it is 32-bit so typesize would be 4, as you have in your code. G8_B8_R8 is just bytes so typesize would be 1.

One reason for preferring having typesize in the file is because it is more future-proof. A Vulkan loader for KTX does not need to be aware of newly added types. It simply does the endian conversion indicated by typesize then uses the VkFormat value for the upload without needing any awareness of what it actually means.

from ktx-specification.

We can probably get the type size from the data format descriptor. I'll check with Andrew.

from ktx-specification.

Is it possible to use the enum names rather than values?

Sure. I used values just to showcase ranges and spare some screen space. An actual program would likely use names.

We can probably get the type size from the data format descriptor.

Isn't DFD optional?

Future-proof and input-proof implementation would be like:

// read vkFormat and typeSize from file

int knownTypeSize = getTypeSizeFromVkFormat(vkFormat);

if (typeSize == knownTypeSize) {
  // OK
} else if (knownTypeSize == -1) {
  // assume file is correct, use typeSize
} else {
  // file is surely invalid
}

from ktx-specification.

DFD is not optional.

I have just published a new draft spec with the status reverted to not ready for implementation while we resolve this issue.

from ktx-specification.

Getting the typesize from a DFD is not straightforward because a DFD has no concept of primitive types. So we'll either need to keep the field or provide example code like we've been discussing above.

My asking Andrew about this has led to a lengthy discussion between us about endianness support and making vkFormat and the DFD play well together. I still do not have a clear idea how to resolve these issues. I will open other issues about these topics.

from ktx-specification.

There are 2 things holding up resolution of this issue:

1. I want KTX2 to be usable with Vulkan and OpenGL formats that may not exist yet, i.e. may be introduced by extensions. Having both gl* and vkFormat values enables this. Removing the gl* values means a mapping to GL is required in the loader and that mapping will be unaware of newly introduced Vulkan formats. I do not yet see an obvious way to strictly define combinations while allowing use of new formats from extensions.

2. We need to resolve whether to keep a typesize field. This is related to no. 1 above, as it is needed to avoid hardwired mappings, and to Issue #14. If we figure out a resolution to 1, I can of course keep the typesize field until #14 is also resolved.

from ktx-specification.

On (1).

So far, the only formats missing from Vulkan are palette and legacy compressed formats (ASTC 3D will obviously be in Vulkan). I doubt that we will ever see new GPU formats introduced exclusively in OpenGL future versions or extensions.

And even if that's the case, depending on the underlying platform, an application may still need to know about such extension's existence to properly create and initialize GL context; so some kind of in-app hard-coded switch-case seems to be inevitable for today-unknown formats.

Also, values of row_of_blocks, block_of_pixels, and format-specific-number-of-bytes are format-dependent, so a generic KTX loader may be unable to unpack unknown format.

from ktx-specification.

As discussed in today's TSG meeting, we we will fix this by

1. Removing the gl header fields, except glTypeSize which will be handled as part of issue #14.
2. Specifying that vkFormat can take any of the values from the VkFormat enumeration in the Vulkan 1.1 spec, except the SCALED types and the PACK32 types. It can also take any values added to that enum by extensions.
3. Add metadata, ktxRequiredVkExtension (and ktxRequiredGLExtension if there is a GL equivalent) which will give the names of extensions required when loading a format added by an extension.
4. Adding a table specifying mappings from VkFormats to GL internalformat, format & type and, as a bonus, DX formats.

@lexaknyazev to draft the table referred to in no. 4.

from ktx-specification.

@MarkCallow Why PACK32 types are disallowed? Some of them are widely supported (like E5B9G9R9).

from ktx-specification.

It is not all PACK32 types, sorry. My shorthand was a bit too brief. Here's the explanation I've written in the spec. draft I'm working on.

The A8B8G8R8*PACK32 formats are prohibited because the end result
is the same regardless of whether the data is treated as packed into
32-bits or as the equivalent R8G8B8A8 format, i.e. as an array of
4 bytes, and because the Data Format Descriptor cannot distinguish
between these cases.

from ktx-specification.

re ktxRequiredGLExtension:

What GL version will be considered as "baseline"? The specs make many assumptions regarding compressed formats that may or may not be true on a given platform. Since we decided to not include GL enums, I doubt that there's much value in including GL extensions names.

from ktx-specification.

Here's the first draft of a full list of Vulkan formats with their OpenGL, Metal, and DX counterparts (excluding SCALED and A8B8G8R8*PACK32 variants).

Keep in mind that different APIs might have different conventions on naming enums, so some matching formats may appear to a reader as if they have different channels order. For the same reason, some packed formats that do not have corresponding native VkFormat value are not in the list.

A few random notes:

• RGB formats are less portable than RGBA.
• sRGB transformation isn't widely available for R and RG formats.
• BGRA variants are less portable than RGBA.
• OpenGL's (desktop) way of describing image data (glFormat + glType describing source and glInternalFormat describing target) is way too flexible to be used for the file format unless we're ready to invest a lot into conformance/validation suite.
• Alpha-only and Luminance formats (missing in Vulkan) can be stored as R or RG with additional swizzling in KTXswizzle or DFD.

TODO:

• Check with Vulkan WG and/or HW vendors which extended range and video formats can be natively used with other APIs (DX reference).
• There's no ASTC 3D enums outside of OpenGL. We need to push for having them in Vulkan.

from ktx-specification.

Great. Thanks. I haven't attempted to check it for correctness. It would be great if we could put this data in some form that it can be used to generate the table for the spec. and a switch statement or other code for run time mapping. Then we'd only need to update one place.

Keep in mind that different APIs might have different conventions

The spec needs to include this paragraph and several of the random notes.

Regarding OpenGL being too flexible, OpenGL ES 3.2 has table 8.2 which lists valid combinations of format, internalformat and type. The mapping should be based on that. If we were retaining the gl* fields we could have referenced that table to limit the combinations.

Yes we should raise the ASTC 3D format enums with the Vulkan WG. That should be on the TODO list.

Do we need to explicitly support alpha-only and luminance formats, i.e, define the necessary swizzles?

from ktx-specification.

Do we need to explicitly support alpha-only and luminance formats, i.e, define the necessary swizzles?

Mapping is straightforward:

Alpha8
VkFormat: VK_FORMAT_R8_UNORM
KTXswizzle: 000r

Luminance8
VkFormat: VK_FORMAT_R8_UNORM
KTXswizzle: rrr1

Luminance8Alpha8
VkFormat: VK_FORMAT_R8G8_UNORM
KTXswizzle: rrrg

Clients may opt to detect the first case and use API-provided enums like MTLPixelFormatA8Unorm or DXGI_FORMAT_A8_UNORM.

from ktx-specification.

Okay. Let's do it.

from ktx-specification.

KTXswizzle: rrra

Is this one right? Shouldn't it be "rrrg"?

from ktx-specification.

Oops, you're right. Fixed the comment above.

I'll add these definitions to the KTX spec. The table won't change, though.

from ktx-specification.

I'm working on this. As I write the description of KTXrequiredVulkanExtension the following question has arisen. What do we say about formats which are added in new core versions of Vulkan without there having been extensions? As far as I can tell there were no extensions for any of the formats added in Vulkan 1.1. Do we need a KTXrequiredVulkanVersion as well?

from ktx-specification.

What would be expected use cases for these fields? Unlike OpenGL (that has all enums in global scope), all Vulkan formats exist within VkFormat scope, so values unknown to an application will not cause any interference. Do we expect that apps will be able to "blindly" enable extensions based on their names in a KTX file?

So far, only one image format extension exists - VK_IMG_format_pvrtc. I think it's safe to assume that apps running on PowerVR GPUs will have it enabled anyway.

from ktx-specification.

I can't find any actual text for VK_IMG_format_pvrtc. If it is the case that format extensions simply add formats to VkFormat and apps can use the standard query to check if the format is supported then this metadata is unnecessary.

I was under the impression that all Vulkan extensions had to be enabled before use. The metadata was to provide the name of the extension. Apps would not "blindly" enable a listed extension. They would check if it exists in the implementation and if so enable it. If not the should raise an appropriate error such as "unsupported format".

from ktx-specification.

There is also a possibility that an extension may add to the textureCompression* items in VkPhysicalDeviceFeatures. As for the existing items (textureCompressionETC2, textureCompressionBC and textureCompressionASTC_LDR, should we add metadata to indicate if the format needs one of these features to be enabled?

from ktx-specification.

Several hours after writing the above comments I realized that extensions and device features have to be enabled at device creation, i.e. long before any texture file is read. I expect that is what @lexaknyazev was referring to by "blindly enable extensions".

The metadata is therefore probably not useful. How about a recommendation that apps handling unknown textures should enable all texture formats available in the implementation at device creation and use vkGetPhysicalDeviceImageFormatProperties to determine if the format is supported at time of loading a texture.

from ktx-specification.

See KhronosGroup/Vulkan-Docs#512 re VK_IMG_format_pvrtc.

+1 for apps querying runtime about format support.
Putting pipeline tools aside, what are "apps handling unknown textures"?

from ktx-specification.

Thanks for the pointer to your Vulkan-Docs issue. Is the information, given in the answer there, in your mapping table in Google Docs?

An example of "apps handling unknown textures" is my test program for the Vulkan loader portion of libktx. I enable all compressed texture features available in the implementation. Now that there are extensions that add formats I will also have to enable those extensions. Elsewhere in the program I have a list of texture tests with many duplicated using different texture formats. The program loads and runs those using formats that runtime queries show are supported. The tests run on iOS, linux, macOS & Windows.

Enabling all available features up front has 2 advantages: it avoids a rats nest of platform ifdefs in the code enabling formats and in the test list and I can add new tests to the list without worrying about whether its format has been enabled.

from ktx-specification.

Please review the PR including the latest commit to it about handling ASTC HDR & 3D in the absence of Vulkan extensions.

from ktx-specification.

I've added some more commits to the PR linked above. The change is now complete except for the table mapping Vulkan formats to other API formats. Please review. I want to merge this so we can make progress on the other issues.

You can also review the data from which the mapping table will be completed

from ktx-specification.

Closed by PR #40. Don't know why it didn't happen automatically.

from ktx-specification.