Skip to content

Conversation

@johnkearney
Copy link

OpenXR API Layers may need to query other layers or the runtime before the instance has been created - add an explicit test for this case.

This is an external mirror of https://gitlab.khronos.org/openxr/openxr/-/merge_requests/3415

OpenXR API Layers may need to query other layers or the runtime before
the instance has been created - add an explicit test for this case.
@CLAassistant
Copy link

CLAassistant commented Aug 6, 2024

CLA assistant check
All committers have signed the CLA.

@johnkearney
Copy link
Author

@fredemmott I've copied the CTS test that I've been working on in the working group gitlab here in case you happen to have views on how it should be implemented.

I see your existing loader spec change: KhronosGroup/OpenXR-SDK-Source#490 and OpenXR spec change: KhronosGroup/OpenXR-Docs#173 which seem to be connected to this; and the recent bug in Meta PC v68 release.

@fredemmott
Copy link
Contributor

Thanks; I’ll take a look through when I can. Those PRs are really the easy half of the problem: I see them as only clarifying behavior that is already required by various parts of the spec.

I think the behavior that the loader spec requires for xrEnumerateApiLayerProperties is incorrect, and needs to be similar to instance extensions ; I just wanted to put up the less controversial PRs first

@fredemmott
Copy link
Contributor

It would be useful to check xrEnumerateInstanceExtensionProperties both before and after creating an instance: if this layer is above other layers adding extensions (e.g. HTC, Ultraleap), the list may change.

Given the values can change at runtime according to the spec, there isn't a correct before-and-after set of values that applies generically, but it would be good to test it works.

It would be good to preemptively add a similar test for xrEnumerateApiLayerProperties - there have been several similar bugs in hardware vendor's API layers, but not - yet - in a runtime.

The spec issue for xrEnumerateApiLayerProperties is:

"xrEnumerateApiLayerProperties must: return only its own API layer properties."

Say you have game -> layer A -> layer B -> layer C -> runtime; by a strict reading:

  1. layer A can see that layer B exists
  2. layer B can see that layer C exists
  3. layer A can not see that layer C exists, because B must 'only return its own API layer properties'

I believe that 3 is undesirable and surprising behavior; instead, layer A should be able to see both layer B and layer C. The spec should be changed (and I'm happy to create PRs) to be more similar to the xrEnumerateInstanceExtensionProperties - though, more specifically, layer B should be required to prepend its' own information:

  • unlike with extensions, order matters for layers
  • if it is prepended, while this would be a spec change adding more requirements, I do not believe there is a conformant way for a caller to depend on the old behavior: callers must allocate a buffer of the correct size, they must not assume it only has 0 or 1 entries.

@rpavlik
Copy link
Contributor

rpavlik commented Oct 31, 2024

A revised version of this PR has been merged internally.

@rpavlik rpavlik closed this Nov 27, 2024
@rpavlik
Copy link
Contributor

rpavlik commented Nov 27, 2024

That revised version is now released.

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

Successfully merging this pull request may close these issues.

4 participants