Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Confirm R16G16_S10_5 support #206

Closed
lexaknyazev opened this issue Feb 7, 2024 · 21 comments
Closed

Confirm R16G16_S10_5 support #206

lexaknyazev opened this issue Feb 7, 2024 · 21 comments

Comments

@lexaknyazev
Copy link
Member

The VK_NV_optical_flow extension adds a new VK_FORMAT_R16G16_S10_5_NV format:

VK_FORMAT_R16G16_S10_5_NV specifies a two-component, fixed-point format where most significant bit specifies the sign bit, next 10 bits specify the integer value and last 5 bits represent the fractional value.

It should be checked whether it's representable with DFD.

@MarkCallow
Copy link
Contributor

@fluppeteer please educate us whether this format is representable with DFD.

@fluppeteer
Copy link

Fixed-point sign-magnitude? Interesting. When I've got updates in place I'll try to make that clearer, since sign-magnitude is only really discussed under the floating point formats section.

I'd write it as this:

vendorId = 0
descriptorType = 0
versionNumber = 2
descriptorBlockSize = 88
colorModel: KHR_DF_MODEL_RGBSDA
colorPrimaries: KHR_DF_PRIMARIES_BT709 (or KHR_DF_PRIMARIES_UNSPECIFIED)
transfer = KHR_DF_TRANSFER_LINEAR (presumably)
flags = KHR_DF_FLAG_ALPHA_STRAIGHT
texelBlockDimension0: 0
texelBlockDimension1: 0
texelBlockDimension2: 0
texelBlockDimension3: 0
bytesPlane0: 2
bytesPlane1: 0
bytesPlane2: 0
bytesPlane3: 0
bytesPlane4: 0
bytesPlane5: 0
bytesPlane6: 0
bytesPlane7: 0
First sample: G mantissa
bitOffset: 0
bitLength: 14 (= "15")
channelType: GREEN
F = S = E = L = 0
samplePosition 0: 0
samplePosition 1: 0
samplePosition 2: 0
samplePosition 3: 0
sampleLower: 0
sampleUpper: 1 << 5 ("1.0" is represented as 32)
Second sample: G sign bit
bitOffset: 15
bitLength: 0 (= "1")
channelType: GREEN
F = E = L = 0
S = 1
samplePosition 0: 0
samplePosition 1: 0
samplePosition 2: 0
samplePosition 3: 0
sampleLower: 1 - 1-bit "-1"
sampleUpper: 0 - 1-bit "0"
Third sample: R mantissa
bitOffset: 16
bitLength: 14 (= "15")
channelType: RED
F = S = E = L = 0
samplePosition 0: 0
samplePosition 1: 0
samplePosition 2: 0
samplePosition 3: 0
sampleLower: 0
sampleUpper: 1 << 5 ("1.0" is represented as 32)
Second sample: R sign bit
bitOffset: 31
bitLength: 0 (= "1")
channelType: RED
F = E = L = 0
S = 1
samplePosition 0: 0
samplePosition 1: 0
samplePosition 2: 0
samplePosition 3: 0
sampleLower: 1 - 1-bit "-1"
sampleUpper: 0 - 1-bit "0"

I think that's consistent with, e.g., table 103 in the KDFS. I'm rusty, so please check it looks sensible, don't just trust me. :-)

In the above I'm assuming sign-magnitude, so "-1.0" is 0x8020". If it's fully-signed and "-1.0" is 0xFFE0, it's simpler.

I'm not a huge fan of the VK_FORMAT_R16G16_S10_5_NV naming (especially since S is used for stencil in depth-stencil format), but it's going to have to be new no matter what is done, and I don't have a firm suggestion. There are already the INT formats that are effectively the same as NORM but with a different definition of "1.0", and this feels like a variant.

@MarkCallow
Copy link
Contributor

Thanks Andrew. Looks okay to me except that bytesPlane0 should be 4. I'm not fond of the naming either.

@fluppeteer
Copy link

Thanks Andrew. Looks okay to me except that bytesPlane0 should be 4. I'm not fond of the naming either.

D'oh. Sorry, I realised I was thinking of it as a single-channel format and then corrected myself and updated the number of samples, but forgot to fix bytesPlane0.

@MarkCallow
Copy link
Contributor

@lexaknyazev how can we confirm if this is sign-magnitude or fully signed?

This format is already in the valid list in libktx's vkformat_check.c but no DFD is generated for it. If we want to support it, I'll open a bug in KTX-Software to fix the DFD generation once we know the answer about sign-magnitude. If we don't want to support it, we need to list it in the prohibited formats section.

@lexaknyazev
Copy link
Member Author

Asked internally.

@MarkCallow
Copy link
Contributor

Asked internally.

Thanks. While you are at it ask about the transfer function.

@lexaknyazev
Copy link
Member Author

Got an answer:

  • It's two's complement (like GL_FIXED)
  • You convert the int16 values to float by dividing them by 32
  • Range is [-1024 ... 1023.96875]
  • The values are linear

@fluppeteer
Copy link

Thanks. In that case it's simpler, and each channel is just one 16-bit signed integer sample with a sample_upper of 32 and sample_lower of -32, I think.

@MarkCallow
Copy link
Contributor

Thanks for the answers @lexaknyazev.

I see no reason not to support this in KTX. The DFD is straightforward. If we do, no spec. change is needed, right?

I don't think we'll want to support it in ktx create as none of the currently supported input formats seems appropriate and I don't think the image processing we have in KTX-Software supports the kind of bid-twiddling need to convert, e.g, a floating point input to this fixed point format. It could of course be added but is it worth it, given this is a vendor extension?

We'll need to update vk2dfd, dfd2vk, validate and info at a minimum.

@lexaknyazev
Copy link
Member Author

lexaknyazev commented Feb 13, 2024

no spec. change is needed, right?

Right.

Once DFD tools, ktx info, and ktx validate support this format, ktx create and ktx extract should work as well with the --raw mode.

@MarkCallow
Copy link
Contributor

I've opened KhronosGroup/KTX-Software#859 to complete support in KTX-Software. Closing this.

@MarkCallow
Copy link
Contributor

MarkCallow commented Feb 14, 2024

@fluppeteer I'm working on adding support in KTX-Software. I have vk2dfd working and I'm now making dfd2vk work. This is based around using interpretDFD. What is your recommendation for handling this in that function? I am thinking of adding an i_FIXED_POINT_FORMAT_BIT to InterpretDFDResult which I would set if the sampleUpper = -sampleLower and the value is somewhere in the middle of the range representable by the number of bits in the channel. Does that sound reasonable?

@MarkCallow
Copy link
Contributor

I am thinking of adding an i_FIXED_POINT_FORMAT_BIT to InterpretDFDResult which I would set if the sampleUpper = -sampleLower and the value is somewhere in the middle of the range representable by the number of bits in the channel. Does that sound reasonable?

ping @fluppeteer.

@fluppeteer
Copy link

Sorry Mark. Yes, I think that works for fixed-point (basically anything other than "maximum number representable-ish = NORM, 1 = INT/SCALED"). If it's really a fixed-point format and doesn't have a separate signed bit, though, I ought to push back strongly on the Vulkan format naming, because it's hopelessly misleading; I'll try to raise an internal Khronos issue.

@lexaknyazev
Copy link
Member Author

The format has been renamed to VK_FORMAT_R16G16_SFIXED5_NV in the upstream specs to better reflect its layout.

@MarkCallow
Copy link
Contributor

It is KTX-Software that needs updating. Nothing to do here. I should have closed this when I merged KTX-Software PR #864.

There is no mapping for this to formats in other APIs. @lexaknyazev are there any possible mappings? If so, please add then to formats.json. If not, please close this. Feel free to open an issue in KTX-Software to remind me to change the name there.

@MarkCallow
Copy link
Contributor

The format has been renamed to VK_FORMAT_R16G16_SFIXED5_NV in the upstream specs to better reflect its layout.

I went to start the update and found that this name is not in the vk.xml in GitLab's vulkan/vulkan/xml. It is in the vk.xml in GitHub's, KhronosGroup/Vulkan-Docs/xml. What gives @lexaknyazev? Has it been withdrawn?

@lexaknyazev
Copy link
Member Author

this name is not in the vk.xml in GitLab's vulkan/vulkan/xml

It's there, in the main branch.

@MarkCallow
Copy link
Contributor

It's there, in the main branch.

Oh! Silly me. I didn't realize my workarea didn't have main checked out.

@MarkCallow
Copy link
Contributor

The name has been updated in KTX-Software PR #921.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants