-
Notifications
You must be signed in to change notification settings - Fork 248
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
base: main
Are you sure you want to change the base?
OpenXR loader: add API layer discovery support. #413
Conversation
3821979
to
af6aeca
Compare
Hi @rpavlik, here is loader patch of API layer discovery, please help review, thanks! |
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. |
There was a problem hiding this 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.
af6aeca
to
d5150b7
Compare
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. |
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. |
434f398
to
84e07a8
Compare
fcfa422
to
fb198f9
Compare
e50ebd6
to
fcaa61a
Compare
Updates (9/27):
|
fcaa61a
to
5cb4b4b
Compare
751bd75
to
a0aed7c
Compare
a0aed7c
to
4cc3569
Compare
12/7/2023: rebase code base to OpenXR SDK 1.0.32.1 hotfix: Fix CMake build of loader on Universal Windows Platform |
563978c
to
b89a96d
Compare
3685509
to
499bef6
Compare
499bef6
to
8cbba72
Compare
Rebase to latest: 99256e9 (3/22) |
8cbba72
to
5337e74
Compare
Rebase to latest: 9e9d023ffcc67037ee244f6f0bc3785370814c96. (4/18/2024)
|
5337e74
to
16a6c58
Compare
Co-authored-by: Rylie Pavlik <rylie.pavlik@collabora.com>
De-duplicate implementations, and be more precise in naming. Co-authored-by: Rylie Pavlik <rylie.pavlik@collabora.com>
Cannot use make_unique because we are using a private constructor, but the same reasoning applies.
16a6c58
to
12433f4
Compare
OK! I was just importing this to GitLab, and I got a little carried away because I saw a few things to fix... Anyway it is now a lot cleaner of a PR, less code duplication applied, etc, and there are individual commits for small atomic changes. Additionally, there are a handful of changes that are not dependent on the layer work and can be merged on their own. I dropped the part of the change that cached the results of getting the API layers, as that seemed premature and probably unwise given that there are two authorities they could be pulled from. I did not drop the caching of the runtime chosen itself, but I did split it into a separate commit and I am inclined to drop it, as unrelated, more global state, etc. This MR works fine (after my adjustments) with the current brokers out there (as in, does not grab any layers, but doesn't fall to pieces) but is a little needlessly loud because of an over-eager exception in the installable broker. I did add a commit to require disable_sys_prop just like we require disable_environment, not sure if we want that or not. Also not sure if we want to validate the names of those props in any way, enforce a pattern or something? |
Co-authored-by: Rylie Pavlik <rylie.pavlik@collabora.com>
Co-authored-by: Rylie Pavlik <rylie.pavlik@collabora.com>
Co-authored-by: Rylie Pavlik <rylie.pavlik@collabora.com>
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
Corresponding patch of broker part can be found here: https://gitlab.freedesktop.org/monado/utilities/openxr-android-broker/-/merge_requests/18