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

Reserve and vendor extension proposal for OPPO #144

Closed
wants to merge 0 commits into from

Conversation

hjiang36
Copy link
Contributor

@hjiang36 hjiang36 commented Mar 1, 2023

No description provided.

@hjiang36
Copy link
Contributor Author

hjiang36 commented Mar 6, 2023

@rpavlik Could you help review this request? Let us know if there is anything we should update. Thanks a lot.

@rpavlik
Copy link
Contributor

rpavlik commented Mar 7, 2023

Nope this looks fine to me, I'll just have to realign it when I go to merge to avoid conflicts. Did you want to reserve more than one or is one all you need?

@hjiang36
Copy link
Contributor Author

hjiang36 commented Mar 7, 2023

Thanks @rpavlik. One is good enough for foreseeable future.
I guess we could submit another request to reserve more when we need them.

Thanks

@hjiang36 hjiang36 changed the title Reserve vendor extension for OPPO Reserve and vendor extension proposal for OPPO Mar 14, 2023
@hjiang36
Copy link
Contributor Author

@rpavlik We added the actual proposal for XR_OPPO_controller_interaction on top of the original pull request to reserve placeholder ID. Could you help check again if there is something missing / wrong in the proposal?
Feel free to let us know if you prefer us to wait until the reserve ID pull request gets merged first before adding the actual proposal. Thanks a lot.

@hjiang36
Copy link
Contributor Author

@rpavlik Sorry to bother you again. I'm wondering if we also need to create pull request to https://github.com/KhronosGroup/OpenXR-SDK/blob/main/include/openxr/openxr.h or that file would auto-generate changes related to this interaction profile extension?

@rpavlik-bot rpavlik-bot added the synced to gitlab A corresponding issue has been filed in the Khronos internal GitLab label Mar 20, 2023
@rpavlik-bot
Copy link
Collaborator

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

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

** subpathname:/input/y/click
** subpathname:/input/y/touch
** subpathname:/input/menu/click
** subpathname:/input/heartrate/value
Copy link
Contributor

Choose a reason for hiding this comment

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

The heartrate path needs to be decorated with heartrate_oppo as per https://registry.khronos.org/OpenXR/specs/1.0/html/xrspec.html#_adding_input_sources_via_extensions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Wallbraker Thanks for reviewing. I updated the path with _oppo decoration per your suggestion in a new commit in this pull request.
Feel free to let me know if there is anything else that needs to be updated.

Thanks

@rpavlik
Copy link
Contributor

rpavlik commented Mar 20, 2023

The headers auto-generate from the XML. I'll pull in the reservation first because there's a review process for the extensions.

@rpavlik
Copy link
Contributor

rpavlik commented Mar 20, 2023

OK, I merged the reservation in #145 , please rebase on the staging branch (nothing is merged to the main branch in this repo except at releases)

@hjiang36 hjiang36 changed the base branch from main to staging March 20, 2023 21:43
@hjiang36
Copy link
Contributor Author

Thanks @rpavlik for merge the reserve commit.
I just changed the base of the pull request to staging branch and resolved conflict.
I used the newly assigned extension ID as well.

If you or @Wallbraker could help review it again, it would be greatly appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
synced to gitlab A corresponding issue has been filed in the Khronos internal GitLab
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants