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

OpenXR loader: add API layer discovery support. #413

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

quic-dengkail
Copy link

@quic-dengkail quic-dengkail commented Jul 5, 2023

Loader currently did not support API layer disconvery yet. This patch supports this feature.

Loader will try to get all implicit/explicit API layers from broker supported by chosen active runtime and populate virtual API layer manifests from returned cursor.
Here is authority and path used to find correct cursor:

  • Loader ---> broker

    • authority:org.khronos.openxr.runtime_broker
    • path:/openxr/major_ver/abi/[abi]/api_layer/[implicit:explicit]

Corresponding patch of broker part can be found here: https://gitlab.freedesktop.org/monado/utilities/openxr-android-broker/-/merge_requests/18

@CLAassistant
Copy link

CLAassistant commented Jul 5, 2023

CLA assistant check
All committers have signed the CLA.

@quic-dengkail
Copy link
Author

Hi @rpavlik, here is loader patch of API layer discovery, please help review, thanks!

@rpavlik-bot rpavlik-bot added the synced to gitlab Synchronized to OpenXR internal GitLab label Jul 10, 2023
@rpavlik-bot
Copy link
Collaborator

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

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

Copy link

@rblenkinsopp rblenkinsopp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've taken an initial pass over this with a few minor pieces, I want to double check the layer ordering as I think runtime shipped layers should be the lowest in the stack (just above the runtime) which I believe this is currently doing correctly but I want to double check.

src/loader/android_utilities.cpp Outdated Show resolved Hide resolved
src/loader/android_utilities.cpp Outdated Show resolved Hide resolved
src/loader/manifest_file.cpp Outdated Show resolved Hide resolved
src/loader/android_utilities.cpp Outdated Show resolved Hide resolved
src/loader/runtime_interface.cpp Outdated Show resolved Hide resolved
src/loader/runtime_interface.hpp Outdated Show resolved Hide resolved
@rpavlik rpavlik force-pushed the dengkail/apilayer_discovery branch from af6aeca to d5150b7 Compare August 11, 2023 18:09
@rpavlik
Copy link
Contributor

rpavlik commented Aug 11, 2023

I did import this into OpenXR gitlab for internal discussion: https://gitlab.khronos.org/openxr/openxr/-/merge_requests/2888

If you can't access that you might need one of your colleagues to assist, since I know they don't let everyone there make a Khronos account.

@quic-dengkail
Copy link
Author

I've taken an initial pass over this with a few minor pieces, I want to double check the layer ordering as I think runtime shipped layers should be the lowest in the stack (just above the runtime) which I believe this is currently doing correctly but I want to double check.

Yes, I can confirm this is exactly this patch will do. Runtime layers will be loaded above the chosen runtime, and thanks for the suggestions, I will modify patch accordingly.

@quic-dengkail quic-dengkail force-pushed the dengkail/apilayer_discovery branch 3 times, most recently from 434f398 to 84e07a8 Compare August 14, 2023 04:45
@quic-dengkail
Copy link
Author

quic-dengkail commented Sep 27, 2023

Updates (9/27):

  1. Functions/instance_extensions query pattern should be revised, better to use similar pattern as functions query in runtime discovery.(loader/broker/runtime side) ---> done
  • Following paths will be used:

    • /openxr/[major_ver]/abi/[abi]/runtimes/active (existing)
    • /openxr/[major_ver]/abi/[abi]/runtimes/[package_name]/functions (existing)
    • /openxr/[major_ver]/abi/[abi]/api_layers/[implicit|explicit] (new in this MR - rename from api_layer )
    • /openxr/[major_ver]/abi/[abi]/api_layers/[package_name]/[layer_name]/functions (new in this MR)
    • /openxr/[major_ver]/abi/[abi]/api_layers/[package_name]/[layer_name]/instance_extensions (new in this MR)
  • The proceedure for API layers would then be:

    • Query active implict/explict layers by the /openxr/[major_ver]/abi/[abi]/api_layers/[implicit|explicit] path.

    • For each layer get it's package_name and layer_name and check has_functions and has_instance_extensions columns.

    • If has_functions is true, then query function mappings with /openxr/[major_ver]/abi/[abi]/api_layers/[package_name]/[layer_name]/functions

    • If has_instance_extensions is true, then query extensions and versions with /openxr/[major_ver]/abi/[abi]/api_layers/[package_name]/[layer_name]/instance_extensions

  1. Adapt the protocol so that the existing authorities for the brokers are used, and so that both the system broker and installable broker can report API layers.(loader/broker/runtime side) ---> done
  2. Remove ‘enableEnvironment’/‘disableEnvironment’ in API layers as they can have no effect on Android as EnvVars are not supported.(loader/broker/runtime side) ---> done
  3. Multiple cases (case A, B, D, E, F1/F2/F3) in runtime/API layer discovery are not needed or have potential security issues to achieve ‘same-origin’ policy. (mainly loader side) ---> WIP

@quic-dengkail quic-dengkail force-pushed the dengkail/apilayer_discovery branch 2 times, most recently from 751bd75 to a0aed7c Compare October 18, 2023 09:05
@quic-dengkail
Copy link
Author

@quic-dengkail quic-dengkail force-pushed the dengkail/apilayer_discovery branch 2 times, most recently from 563978c to b89a96d Compare March 4, 2024 09:32
@quic-dengkail quic-dengkail force-pushed the dengkail/apilayer_discovery branch 2 times, most recently from 3685509 to 499bef6 Compare March 13, 2024 08:33
@quic-dengkail
Copy link
Author

Rebase to latest: 99256e9 (3/22)

@quic-dengkail
Copy link
Author

Rebase to latest: 9e9d023ffcc67037ee244f6f0bc3785370814c96. (4/18/2024)

  • Rebase to loader release 1.1.36.

  • Add "disable_environment" and "enable_environment" back on Android platform.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
synced to gitlab Synchronized to OpenXR internal GitLab
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants