microsoft / MixedReality-GraphicsTools-Unity

Graphics tools and components for developing Mixed Reality applications in Unity.
MIT License
182 stars 42 forks source link

Update render order when using XRI #182

Closed shaynie closed 1 year ago

shaynie commented 1 year ago

Overview

Update render order when using XRI. Created by Kurtis. See his explanation below:

The change in https://github.com/microsoft/MixedRealityToolkit-Unity/pull/11575 is causing some execution order issues with the cadence XRI updates its data and visuals. It aligns the cursor with the proximity light, but at the expense of misaligning the cursor with the pointing ray (as well as not using the very latest platform data used for visualization). If possible, a different fix would be to update the proximity lights instead, to update as late as possible to ensure all other objects are in their expected locations for rendering.

XRI updates its various components (in this case, controllers, interactors, and visuals) in specific orders. You can see all the values in https://docs.unity3d.com/Packages/com.unity.xr.interaction.toolkit@2.4/api/UnityEngine.XR.Interaction.Toolkit.XRInteractionUpdateOrder.html, though it's a little confusing as they're alphabetical...looking at the raw script in the XRI package is easier, imo.

As far as the components we care about in this scenario, those are:

k_Controllers = -29990 MRTKRayInteractor updates itself at k_Controllers + 1 The other interactors update themselves in ProcessInteractions (called by the IM below in step 3). There's a comment about doing this to account for the UIInputModule, but I'm not sure what that means for all the other interactors...maybe UIInputModule only supports ray interactors, but I'm unsure at this point There's a chance not updating the interactor's position in onBeforeRender as well helps exacerbate this issue. Interesting... k_InteractionManager = -105 k_BeforeRenderOrder = 100 k_BeforeRenderLineVisual = 101

There's a very specific dance these all play.

The controllers (XRControllers, not necessarily hardware controllers) are updated to match the device/hand pose. Then, MRTKRayInteractor updates itself out of band, I guess for UIInputModule Then, the interaction manager updates all interactors. MRTK's interactors do things a little differently here than XRI interactors, where the MRTK interactors (other than the ray interactor, mentioned above) move themselves to match their corresponding poses (pointer pose, poke pose, etc). The data from these two updates is then used for targeting/input/interaction. In onBeforeRender callbacks, XRI then updates XRControllers into their visual poses. It queries runtime data as late as possible so the rendering matches reality as close as possible. This has the effect of shifting all the interactors, as children of the controller. It also processes interactors again, though it looks like all the MRTK interactors ignore this event. As I mentioned above, I wonder if there's a visual offset introduced because of this...either way though, I think aligning the ray visuals, cursor, and proximity light is probably more important than a potential offset between the controller and pointing ray, but it's something to think about. Finally, after everything above has been updated, the ray visuals and reticle visuals update themselves with the absolute latest and last data from the runtime. This is the piece that was removed in the above PR. In the current state of the repo, the cursor is now updated somewhere between steps 3 and 4, which means that the final controller pose update isn't accounted for. This means that the cursor visuals will shift based on the difference in the two controller poses, which causes a misalignment with the controller and ray and becomes very noticeable if there's some noise in the controller poses.

Changes

Verification

This optional section is a place where you can detail the specific type of verification you want from reviewers. For example, if you want reviewers to checkout the PR locally and validate the functionality of specific scenarios, provide instructions on the specific scenarios and what you want verified.

If there are specific areas of concern or question feel free to highlight them here so that reviewers can watch out for those issues.

As a reviewer, it is possible to check out this change locally by using the following commands (substituting {PR_ID} with the ID of this pull request):

git fetch origin pull/{PR_ID}/head:name_of_local_branch

git checkout name_of_local_branch