OpenXR-Docs icon indicating copy to clipboard operation
OpenXR-Docs copied to clipboard

Can length annotations for the output sizes of returned arrays be added?

Open expipiplus1 opened this issue 5 years ago • 4 comments

For example the spec says here that the length of displayRefreshRates is displayRefreshRateCapacityInput, but this doesn't tell the whole story after the function is called! Is it possible to expand the spec with information about how many values in the array are valid (i.e. displayRefreshRateCountOutput) after the call returns?

      <command successcodes="XR_SUCCESS,XR_SESSION_LOSS_PENDING" errorcodes="XR_ERROR_INSTANCE_LOST,XR_ERROR_SESSION_LOST,XR_ERROR_RUNTIME_FAILURE,XR_ERROR_HANDLE_INVALID,XR_ERROR_SIZE_INSUFFICIENT,XR_ERROR_FUNCTION_UNSUPPORTED,XR_ERROR_VALIDATION_FAILURE">
        <proto><type>XrResult</type> <name>xrEnumerateDisplayRefreshRatesFB</name></proto>
        <param><type>XrSession</type> <name>session</name></param>
        <param optional="true"><type>uint32_t</type> <name>displayRefreshRateCapacityInput</name></param>
        <param><type>uint32_t</type>* <name>displayRefreshRateCountOutput</name></param>
        <param optional="true" len="displayRefreshRateCapacityInput"><type>float</type>* <name>displayRefreshRates</name></param>
      </command>

FWIW the Vulkan spec uses the same parameter for input and output so doesn't fall into this problem, although it's probably too late to change things now!

expipiplus1 avatar Dec 12 '20 06:12 expipiplus1

I notice that the length attribute for the returned value is used some functions, such as xrGetVulkanDeviceExtensionsKHR which has <param optional="true" len="bufferCapacityInput,null-terminated"><type>char</type>* <name>buffer</name></param> meaning bufferCapacityInput size on the way in and null-terminated on the way out. I found this quite surprising as the semantics of len in the Vulkan spec are quite different (subsequent comma-separated values in this attribute are used to indicate the length of values at different levels of indirection).

expipiplus1 avatar Dec 13 '20 05:12 expipiplus1

An issue (number 1513) has been filed to correspond to this issue in the internal Khronos GitLab (Khronos members only: KHR:openxr/openxr#1513 ), to facilitate working group processes.

This GitHub issue will continue to be the main site of discussion.

rpavlik-bot avatar Feb 04 '21 22:02 rpavlik-bot

Hmm, I didn't realize the vulkan extension one was like that, I thought it was an array of strings. Our comma-separated sizes usually do refer to different levels of indirection as well, we share about 90% of the spec toolchain with Vulkan. I think some of our verification/generation scripts treat null-terminated as a special exception, probably, so that the implicit valid usage comes out OK.

What would be your use case for the additional metadata? FWIW, because we're pretty strict about naming, we have tools that detect these "two-call idiom" calls (we call them that internally, but it might not actually be in the spec text) and use things based on the name. This script is in our CI, so you can be assured that it will be obeyed in future releases. (If we needed to break that for some reason, it would at least be called out separately as a script change in addition to a spec change, but I don't foresee any new items being added that would need to break this pattern.) This logic verifies our style guide rules for two-call idiom: https://github.com/KhronosGroup/OpenXR-Docs/blob/957b138ce697067c577a92aa06b8504abab504c7/specification/scripts/xml_consistency.py#L271-L407

rpavlik avatar Mar 04 '21 18:03 rpavlik

OK, so upon review, that was just a bug in the vulkan extension (#77) - the ,null-terminated there was an error that will be fixed in the next release. My other questions are still open, though.

rpavlik avatar Mar 04 '21 21:03 rpavlik