Tobski
commented
1 year ago - I'm just going to delete this sentence, it doesn't really add anything, and is technically incorrect for places in the spec where an enum parameter might be ignored.
- This was a deliberate choice - there's no global functions that include an enum, and none are expected to be added. We can add this in if we ever add such a function.
- Yea that was a leftover from prior to the change, this was already being removed by an unrelated change, but I'll pull it out as part of the fix for this.
- That section is borderline pointless; instead I'll add a thing to the version function stating that if it's not supported by the implementation then you've got Vulkan 1.0. I think that's probably useful for other reasons anyway.
- We should really get around to fixing #1540, but I don't want to make a point fix here in lieu of that. As for "command" we're incredibly inconsistent about this; so I'm not going to switch it right now. IIRC "command" was originally only meant to be vkCmd* calls - we should make a determination somewhere and update the spec for consistency, but not as part of this change.
- The or is deliberately omitted - it's meant to be like a standard sentence construction just broken up by bullets - i.e. "either A, B, or C". As for the correctness issue, yea I missed that this is bounded by the api version - will fix.
- I'll update to "the current instance", I don't think we need more detail here than that?
- I'll add "physical" and put gpdp2/vk1.1 ifdefs around this part. For completeness I shall extend this to say that 1.1 or gpdp2 must be supported/enabled, because technically there is a weird interaction here if you have Vulkan 1.0 and don't enable gpdp2 at instance creation.
- Incredibly unlikely (would break so much) but I'll clarify that it's objects created from a device.
- Yup, will fix.
- Will update to "current device", not planning to elaborate further.
- To be honest, I left this alone when I made the change for the rest of this, because I wasn't sure what to make of it. Might be best to just remove these two sentences to be honest.
Finally:
Most of it feels highly repetitive, and I feel the Valid Usage for Extensions and Valid Usage for Newer Core Versions sections should do the heavy lifting here. The same rules will apply for all the other non-enum entities too.
Yea I made the same observation - there's a couple reasons I didn't go down that route though. 1 is that although there's a lot of overlap with other versioned parts, there are actual differences in a handful of cases that are in the minutaie. Trying to tease that out is a pain without writing them down independently first. The other more immediately practical reason is that I didn't want to have to rewrite that entire section as part of this very deliberately more targeted change. A rewrite of that section isn't off the table, it's just not a priority for now.
I'll raise a PR for all of this so you can see what it looks like.
krOoze
1
Sentence feels confusing. Probably means "any argument". Even so the sentence feels weirdly formed; it misses verbs or something and partly redundant to the next sentence, and the "of that type" seem partly redundant to the first bullet point.
2
Rules do not cover functions without dispatchable handle, such as
vkCreateInstance. This impacts use ofVK_INSTANCE_CREATE_ENUMERATE_PORTABILITY_BIT_KHR, enumerants usable in thepNextchain, and potential new enumerants in theVkAllocationCallbacksstructure.3
The "¹" points nowhere now.
4
That is slightly incorrect. Many enumerants were added by core version 1.0, and such version cannot be queried by
vkEnumerateInstanceVersion. There seem to be a Querying Version Support chapter, so I suggest linking to that, and improving that chapter.5
This seem incorrect for
vkCreateDevice(something like #1540).BTW, I think functions in Vulkan are generally called "commands".
6
missing "or"
It seems insufficient, and contradictory to the Valid Usage for Newer Core Versions, which says:
7
Probably should be explicit which "the instance".
8
Should say "physical device".
This seems incompatible with Vulkan 1.0 without
VK_KHR_get_physical_device_properties2, which only permit use of enumerants added by instance extensions. Not sure if all extensions list these as dependency. Hypothetically, there could be an extension where use of its physical-device level commands is non-essential, therefore it does not list these dependencies.9
Brittle in case there is ever an instance-level handle added.
10
same as 6
11
Should say which device, similarly to 7. Since we have not even mentioned any device earlier, there is no "the device".
12
This is slightly confusing, since enumerants are being added all the time. It is not clear which values exactly are reserved at any given time. Based on the latest publicly released spec? Or in the patch version the driver reports (if it reports it)?
Most of it feels highly repetitive, and I feel the Valid Usage for Extensions and Valid Usage for Newer Core Versions sections should do the heavy lifting here. The same rules will apply for all the other non-enum entities too.