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

XR_MNDX_force_feedback_curl #136

Merged
merged 23 commits into from
Feb 1, 2023

Conversation

danwillm
Copy link
Contributor

@danwillm danwillm commented Sep 8, 2022

This introduces XR_MNDX_force_feedback_curl - a simple extension to provide force feedback for VR Gloves.

@danwillm danwillm force-pushed the force-feedback-ext branch 3 times, most recently from 14da518 to e9f3b48 Compare September 9, 2022 18:08
@rpavlik-bot
Copy link
Collaborator

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

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

@rpavlik-bot rpavlik-bot added the synced to gitlab A corresponding issue has been filed in the Khronos internal GitLab label Sep 21, 2022
Copy link
Contributor

@rpavlik rpavlik left a comment

Choose a reason for hiding this comment

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

FYI, some of the CI isn't set up here that we have internally. You'll want to regularly run make reflow, and also run the various "check" scripts in the specification folder plus scripts/xml_consistency.py

specification/registry/xr.xml Outdated Show resolved Hide resolved
specification/registry/xr.xml Outdated Show resolved Hide resolved
specification/registry/xr.xml Outdated Show resolved Hide resolved
specification/registry/xr.xml Outdated Show resolved Hide resolved
specification/registry/xr.xml Outdated Show resolved Hide resolved
@rpavlik
Copy link
Contributor

rpavlik commented Nov 17, 2022

Here's the current error list:


Messages for XrForceFeedbackCurlLocationMNDX

Error: /home/ryan/src/OpenXR-Docs/specification/registry/xr.xml:3180: Got an enum whose name does not match the pattern: the type is XrForceFeedbackCurlLocationMNDX which would cause values to start with XR_FORCE_FEEDBACK_CURL_LOCATION_ but got a different longest common prefix XR_FORCE_FEEDBACK_CURL_LOCATION_FINGER_

If you don't want to rephrase these we can add this to an exception list in xml_consistency.


Messages for XrForceFeedbackCurlCreateInfoMNDX

Error: /home/ryan/src/OpenXR-Docs/specification/registry/xr.xml:2468: Unknown structure type constant XR_TYPE_FORCE_FEEDBACK_CURL_CREATE_INFO_MNDX


Messages for XrApplyForceFeedbackCurlLocationsMNDX

Error: /home/ryan/src/OpenXR-Docs/specification/registry/xr.xml:2473: Type has incorrect type-member value: expected XR_TYPE_APPLY_FORCE_FEEDBACK_CURL_LOCATIONS_MNDX got XR_TYPE_FORCE_FEEDBACK_CURL_APPLY_LOCATIONS_MNDX

Got some left over and/or wrong enum values.


Messages for xrApplyForceFeedbackCurlMNDX

Error: /home/ryan/src/OpenXR-Docs/specification/registry/xr.xml:4420: Missing expected return code(s) XR_SESSION_LOSS_PENDING implied because of input of type XrHandTrackerEXT found via path xrApplyForceFeedbackCurlMNDX -> XrHandTrackerEXT

yeah just add it.

[invalid] /home/ryan/src/OpenXR-Docs/specification/registry/xr.xml
[invalid] /home/ryan/src/OpenXR-Docs/specification/registry/xr.xml failed-assert /Q{}registry[1]/Q{}types[1]/Q{}type[523]/Q{}member[4] XrApplyForceFeedbackCurlLocationsMNDX::locationCount: This count member is field 3, but at least one of the members with length bound by it appears earlier in the structure: positions are 2 for members locations respectively. Make sure the count precedes its arrays.

[invalid] /home/ryan/src/OpenXR-Docs/specification/registry/xr.xml failed-assert /Q{}registry[1]/Q{}types[1]/Q{}type[523]/Q{}member[4] XrApplyForceFeedbackCurlLocationsMNDX::locationCount: This count member is field 3, but field 4 is not an array bounded by it: All arrays bounded by locationCount should follow it immediately: locations. Either re-arrange fields to get the order right, or add the missing len attribute(s).

these are the "rearrange the count before the array".

For file /home/ryan/src/OpenXR-Docs/specification/sources/chapters/extensions/mndx/mndx_force_feedback_curl.adoc:
2 errors generated.
Checked all chapters and extensions.
sources/chapters/extensions/mndx/mndx_force_feedback_curl.adoc:22:15: error: Found reference page markup, but refpage='XrForceFeedbackLocationMNDX' type='enums' does not refer to a recognized entity (-Wrefpage_name)
If this is intentional, add the entity to EXTRA_DEFINES or EXTRA_REFPAGES in check_spec_links.py.

[open,refpage='XrForceFeedbackLocationMNDX',type='enums',desc='Describes which location to apply force feedback',xr[...]
^~~~~~~~~~~~~~~~~~~~~~~~~~~
sources/chapters/extensions/mndx/mndx_force_feedback_curl.adoc:24:4: error: Definition of link target XrForceFeedbackLocationMNDX with macro elink (used for category enums) does not exist. (-Wbad_entity)
Might be a misspelling, or, less likely, the wrong macro.

The elink:XrForceFeedbackLocationMNDX describes which location to apply
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Missing, but unreferenced, API includes/anchors - potentially not-documented entities:
Include File Anchor in lieu of include


structs/XrForceFeedbackCurlCreateInfoMNDX.txt [[XrForceFeedbackCurlCreateInfoMNDX]]

Think this is just outdated names?

@rpavlik rpavlik added waiting for reporter Need more info or a revision needs changelog Needs a changelog fragment: see README files in changes/ labels Nov 17, 2022
Copy link
Contributor

@rpavlik rpavlik left a comment

Choose a reason for hiding this comment

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

ahh I didn't realize I had a pending review still sitting in limbo here, sorry

@rpavlik rpavlik force-pushed the force-feedback-ext branch from 06e92c8 to 256e68a Compare February 1, 2023 16:56
@rpavlik rpavlik self-requested a review February 1, 2023 16:56
@rpavlik rpavlik changed the base branch from main to staging February 1, 2023 16:57
Copy link
Contributor

@rpavlik rpavlik left a comment

Choose a reason for hiding this comment

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

I rebased and force-pushed, and made some minor fixes. Just a few last items remaining.

danwillm and others added 3 commits February 1, 2023 17:11
…back_curl.adoc

Co-authored-by: Ryan A. Pavlik <ryan.pavlik@gmail.com>
…back_curl.adoc

Co-authored-by: Ryan A. Pavlik <ryan.pavlik@gmail.com>
Co-authored-by: Ryan A. Pavlik <ryan.pavlik@gmail.com>
@rpavlik rpavlik self-requested a review February 1, 2023 17:14
Copy link
Contributor

@rpavlik rpavlik left a comment

Choose a reason for hiding this comment

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

Thanks for being so responsive!

@rpavlik rpavlik merged commit bd7d617 into KhronosGroup:staging Feb 1, 2023
@rpavlik rpavlik added accepted Accepted for release, will be included in an upcoming patch release and removed waiting for reporter Need more info or a revision needs changelog Needs a changelog fragment: see README files in changes/ labels Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Accepted for release, will be included in an upcoming patch release 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.

3 participants