OpenXR-SDK-Source icon indicating copy to clipboard operation
OpenXR-SDK-Source copied to clipboard

API layers can be marked as enabled in two places

Open rpavlik opened this issue 4 years ago • 6 comments

Follow up from https://github.com/KhronosGroup/OpenXR-SDK-Source/pull/155#issuecomment-577783292

cc @sn0w75

rpavlik avatar Jan 23 '20 17:01 rpavlik

An issue (number 1319) has been filed to correspond to this issue in the internal Khronos GitLab.

If you have a Khronos account, you can access that issue at KHR:openxr/openxr#1319.

rpavlik-bot avatar Mar 23 '20 17:03 rpavlik-bot

Is reproducing this as simple as "XR_ENABLE_API_LAYERS=XR_APILAYER_LUNARG_core_validation;XR_APILAYER_LUNARG_core_validation"? It's not clear from https://github.com/KhronosGroup/OpenXR-SDK-Source/pull/155 what precipitates the issue.

It's conceivable that someone could do, e.g. "XR_ENABLE_API_LAYERS=XR_APILAYER_LUNARG_api_dump;SOME_WEIRD_LAYER;XR_APILAYER_LUNARG_api_dump" (i.e. layer enabled twice on purpose to see what the difference is between what the app issued and what SOME_WEIRD_LAYER did on behalf of the app. So not only should this work but the layer should probably be loaded and two instances created. (Is that even legal in the current loader spec?)

bradgrantham-lunarg avatar Mar 04 '21 18:03 bradgrantham-lunarg

I think it was enabling validation both from the environment and the application. However as you point out, there's a use case at least for apidump being more than once applied, though configuring it would be tricky.

rpavlik avatar Mar 05 '21 00:03 rpavlik

Yeah, I'm not sure it is a valid use case after thinking about it. They both read the same environment variable for the type and destination of the output - how would one break out the output of one layer versus the other? I doubt coreval and apidump will handle being instantiated twice either, but I haven't tried it.

In any case, the loader appendicies.adoc says this about the API layer list:

The loader will remove any duplicate API layers that appear in both this list by using the first occurrence of any API layer.

That seems pretty clear. The section is also pretty clear (if I read it correctly) that the order is

  1. implicit layers
  2. layers in XR_ENABLE_API_LAYERS
  3. layers in CreateInstance list

And "remove any duplicate API layers [...] by using the first occurrence" disambiguates which ones are kept and removed as well.

Will do more work on this bug later, but it seems to say that an API layer being "marked as enabled in two places" is not a bug, but any behavior which is not enabling the "first" instance of the API layer and disregarding the rest is a bug.

bradgrantham-lunarg avatar Mar 05 '21 21:03 bradgrantham-lunarg

Layers appear to be enabled in the order they are discovered in manifests. That is to say, if there are no implicit layers, and then XR_API_LAYER_PATH=location-of-layer-B-manifest;location-of-layer-A-manifest and XR_ENABLE_API_LAYERS=layer-A;layer-B, layerB is still initialized first. (Tested with overlays layer and api_dump, which have manifests in different directories, and breakpoints in the layers, and crossing fingers I did that right.) This also occurs if the layers are explicit, i.e. listed in CreateInstance and XR_ENABLE_API_LAYERS is unset. I'll dig into this a little more to be sure.

So that appears to be a bug in that it doesn't match the docs. Without looking in the loader, I'll guess that the layer list is populated in manifest order and then the layer enable just sets booleans in those layers.

The way forward is probably to close this bug "as designed" (since "API layers can be marked as enabled in two places" is documented and does appear to work now, but then open another bug "layers are opened in the wrong order." @rpavlik do you agree? (Or we could rename this bug.)

bradgrantham-lunarg avatar Mar 09 '21 21:03 bradgrantham-lunarg

An issue (number 1531) has been filed in the internal Khronos GitLab, to correspond to the new issue relating to order of API layer load and initialization.

If you have a Khronos account, you can access that issue at KHR:openxr/openxr#1531.

bradgrantham-lunarg avatar Mar 09 '21 22:03 bradgrantham-lunarg