-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Fix for eye gaze pinch controls and fix pointer naming bug #9364
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
Fix for eye gaze pinch controls and fix pointer naming bug #9364
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
FYI @sostel |
cre8ivepark
left a comment
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.
Tested on the device and works well! Thanks!
| PointerName = $"{Handedness}_{gameObject.name}"; | ||
|
|
||
| // We've been destroyed during the await. | ||
| if (this == null) |
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.
We may want to move this below the if (this == null) check. If this == null, then gameObject will likely be null too and gameObject.name will null ref
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.
The this == null check is meant to guard against the case where a hand is detected and lost within a single frame or a couple frames (either way, in fewer frames than it took for await EnsureInputSystemValid(); to return). In that case, the pointer might be cleaned up and gone when we get here
|
|
||
| if (base.Controller != null && this != null) | ||
| { | ||
| PointerName = $"{Handedness}_{gameObject.name}"; |
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 think moving this out of the Controller setter might cause unexpected pointer names in the case where they're recycled and reused.
MixedRealityToolkit-Unity/Assets/MRTK/Core/Providers/BaseInputDeviceManager.cs
Lines 200 to 220 in 965f566
| if (EnablePointerCache) | |
| { | |
| var pointerCache = pointerConfigurations[i].cache; | |
| while (pointerCache.Count > 0) | |
| { | |
| var p = pointerCache.Pop(); | |
| if (p.TryGetMonoBehaviour(out MonoBehaviour pointerComponent)) | |
| { | |
| pointerComponent.gameObject.SetActive(true); | |
| // We got pointer from cache, continue to next pointer option to review | |
| requestedPointer = p; | |
| DebugUtilities.LogVerboseFormat("RequestPointers: Reusing a cached pointer {0} for controller type {1} and handedness {2}", | |
| requestedPointer, | |
| controllerType, | |
| controllingHand); | |
| break; | |
| } | |
| } | |
| } |
in that case, we'll reuse a pointer GameObject which will retain its previous name, which might have been for a differently-handed controller
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.
Currently, with the name update in the setter, the name will update when a reused pointer is updated at, for example:
MixedRealityToolkit-Unity/Assets/MRTK/Providers/XRSDK/XRSDKDeviceManager.cs
Lines 205 to 208 in 2948006
| for (int i = 0; i < detectedController.InputSource?.Pointers?.Length; i++) | |
| { | |
| detectedController.InputSource.Pointers[i].Controller = detectedController; | |
| } |
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.
Unfortunately this led to the repeating handedness bug we observed earlier. I suppose the best way to go about addressing this is to have another variable that denotes the pointerTypeName? Though, I'm not super enthuthiastic about setting that initially and then never touching it again. I also wasn't super thrilled that the naming methodology had started to get fragmented across the many different pointers.
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.
Yeah, that's definitely fair! I wonder if we could split on _ and replace the first value with our new handedness or something?
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 also wasn't super thrilled that the naming methodology had started to get fragmented across the many different pointers.
Also this though, yeah :(
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.
Yeah, that's definitely fair! I wonder if we could split on
_and replace the first value with our new handedness or something?
I worry that we start relying too much on the naming structure of the object that's getting passed in, and the extra variable approach is slightly more appealing to me in this case...
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.
Followup PR here #9371
* Clarify where packages are being published * Move NuGet steps out of general CI * pr feedback * feedback: simplification * pr feedback * Move feed publishing from common-nonUnity to ci-packaging-internal * Fix paths to templates * Revert UPM pack on non-Unity machines versionmetadata.yml depends on the project being opened in Unity to generate meta files for the files it adds. Therefore, we have to run UPM pack after that point (which happens when we pack the .unitypackages) * Also revert when NuGet pack happens, since it apparently has to be before tasks/pack-unitypackages.yml I couldn't get to the bottom of this...so I'm reverting it (and better sectioning off the NuGet area for future removal * Updated text label 'BoundingBox.cs to BoundsControl.cs'. Updated Slate prefab: Checked title bar's ObjectManipulator Near Smoothing. (it was unchecked, made the movement jitter) * Add Microsoft.MixedReality.Toolkit.PlaneFinding to exclusion list * Update Authors list * Initial commit for docs binary generation Co-Authored-By: Kurtis <3580640+keveleigh@users.noreply.github.com> * Update settings.yml Co-Authored-By: Kurtis <3580640+keveleigh@users.noreply.github.com> * Update createbinariesfordocs.ps1 * Update docs-binaries.yml * Change GUID references in asmdefs to name references * Update createbinariesfordocs.ps1 * Update to be compatible with multiple OpenXR package versions FrameTime was graduated from the Preview namespace * Update scripts/packaging/createbinariesfordocs.ps1 Co-authored-by: Kurtis <kurtie@microsoft.com> * Update createbinariesfordocs.ps1 * Update MRTK.OpenXR.asmdef * Finger cursor and pointer naming rework (#9280) * initial commit for finger cursor rework * renamed gaze cursor prefab to something more appropriate, labeled gaze cursor prefab field * cursors now decoupled from their parent pointers, cursors and pointers are now named appropriately based on their handedness in the heirarchy * minor unit test updates * addressed PR comments, provided named constants * fixed code validation issue, provided variable clarity * Update Assets/MRTK/SDK/Features/UX/Scripts/Pointers/SpherePointer.cs Co-authored-by: Kurtis <kurtie@microsoft.com> * Update Assets/MRTK/SDK/Features/UX/Scripts/Pointers/BaseControllerPointer.cs Co-authored-by: Kurtis <kurtie@microsoft.com> * added comments concerning assumptions made about the object hierarchy Co-authored-by: Kurtis <kurtie@microsoft.com> * Typo fixes in tooltips (#9297) * Update URLs in the source code * Remove en-us from URLs * Update ElasticsManager.cs * Actually cache handJointService * Re-add missing using and clean-up usings * Finger cursor and pointer naming rework (#9280) * initial commit for finger cursor rework * renamed gaze cursor prefab to something more appropriate, labeled gaze cursor prefab field * cursors now decoupled from their parent pointers, cursors and pointers are now named appropriately based on their handedness in the heirarchy * minor unit test updates * addressed PR comments, provided named constants * fixed code validation issue, provided variable clarity * Update Assets/MRTK/SDK/Features/UX/Scripts/Pointers/SpherePointer.cs Co-authored-by: Kurtis <kurtie@microsoft.com> * Update Assets/MRTK/SDK/Features/UX/Scripts/Pointers/BaseControllerPointer.cs Co-authored-by: Kurtis <kurtie@microsoft.com> * added comments concerning assumptions made about the object hierarchy Co-authored-by: Kurtis <kurtie@microsoft.com> * Fix Unity 2018 docs generation issues * replace wmr config checker with asmdef version defines * restore configuration checker * Create objects only for the correct loader (#9249) * Check if the loader is the correct one * Fix for creating the correct controllers for the running loader (e.g. when the loaded/running loader is after the ones that are present but not running). If Oculus and WindowsMR are both checked and WindowsMR is running, MRTK should load the WindowsMR controllers, not Oculus ones. * Change condition on WMR_ENABLED from MRTK.WMR. * Formatting and clean-up Co-authored-by: Kurtis <kurtie@microsoft.com> * Shorten State Visualizer file paths * Update UnityProjectInfo.cs * Update createbinariesfordocs.ps1 * enable handphysics asm on any platform * hand physics: set default supported platforms to all * pr feedback: update profile * Update backing fields to private and setters to protected * Formatting and region clarifications * Add base loader checkers * Update Oculus and WMR with IsActiveLoader pattern * Improve enabled state checking for XR SDK * Add active loader checking for OpenXR * Rearrange one more region * Properly wrap with WMR_ENABLED * Fix #9335 - Added 'Auto Follow Trigger' event to the FollwMeToggle script. Toggle Pin button using this event properly. * Updating text input filed example in the Slate UGUI example. * Reserialize ReadingModeDemo.unity in Unity 2018 with correct scripts * Update ProjectVersion.txt * Remove extraneous components from SceneDescriptionPanel * Applied feedback. * Reserialize MRTK changes * Re-enabling shader inspector for later properties. * Spell check * Clean up #pragma warning disable by marking as Obsolete * Remove extra pragmas * Fix translation handle rotation * Add the NearInteractionGrabbable component to the Slate prefab * Fix Bounding Box material for Slate prefab * Adding updated 'grabbed' material for the slate as well. * Adjusting the content center so that rotation pivot is located at the center of the slate. * Unchecking rotation handle visibility for Z axis. Adjusting collider size for the rotation handle. * Replace Slate prefab in the example scene. * Fixed for eye gaze pinch controls and fix pointer naming bug * Fix BoundsControl incorrectly calculates world space center of bounds * Addressing remaining feedback on #9364 (#9371) * fixed naming with controllers and eye gaze examples * Addressing remaining feedback on the pointer naming PR * addressing last traces of feedback * Debug -> UnityEngine.Debug * Fix wrong field reference * Gate retargeting to non-abstract UnityEngine.Components * Update comments * Update coding conventions * Primary Cursor and InputAction's Profile fixes (#9379) * Added workaround for primary cursor scene, fixed input actions scene profile * Made ggvpointer highlightable * Update Assets/MRTK/Examples/Demos/Input/Scenes/PrimaryPointer/PrimaryPointerHandlerExample.cs Co-authored-by: Kurtis <kurtie@microsoft.com> * reverted profile back to Hololens1 style gestures Co-authored-by: Kurtis <kurtie@microsoft.com> * Add ability to write dictionary to file during pipeline * Rewrite ProcessYamlFile to allow for rewriting same file * Implement ability to retarget to script IDs * Update to use VS2019 * Update BuildSource.proj to take in an arbitrary VS version * Update some logs * Change mapping file lookup to name * Update pipeline to write file to correct spot * Revert VS2019 as default * Expand search to UnityEngine.Object * Fix edge case where pipeline depends on all files being written in this step * Exclude rsp files from packing * Add txt file and meta * Update where the dict is written to And add copy step to the NuGet folder. This ensure the file will be in the UPM and UnityPackage distributions as well. * Update dictionary reading to be more robust to malformed txt files * Don't publish to feeds except for CI This allows for manual runs of mrtk_ci without extraneous packages in the feeds * Add progress bar to GUID remapping * Include the MSBuild binary in the nuspec Since this assembly isn't under .Tools for some reason...even though it's in the Tools folder * Refactor remapping method and add popup for selection of mapping file * Fix issues associated with the new SU package * Only update renderViewPortScale if an XR device is present * update remoting documentation * Ensure we don't set 0 to renderViewportScale * Update Interactive Element Docs * Remove Unity 2018 support for scene understanding * Update SceneUnderstandingExample.unity * Add comments * add versioning to the shader import logic * Update ReleaseNotes.md * add versioning comment to shader source files * Update SceneUnderstanding.md * add documentation on updating shaders * add doc comments * undo changes to project prefs * Update release notes * Fix links * Update TOC * Update Assets/MRTK/StandardAssets/EditorUtilities/OnLoadUtilities.cs Co-authored-by: Kurtis <kurtie@microsoft.com> * update TMP version * Explicitly set IsEnabled to false when the SU package is not present Co-Authored-By: Kurtis <3580640+keveleigh@users.noreply.github.com> * Update SceneUnderstanding.md * Add null check for XR not initialized * Update legacy and XR SDK implementation * Fixed articulated hands being classified as HP Motion Controllers (#9410) * Fixed articulated hands being classified as HP Motion Controllers * small modification to ensure we only name interaction sources of kind controller * small adjustment to the naming schemes * implement ignore support for shader update * update shader update documentation and images * fix code validation error (missing doc comment contents) * remove unneeded debug.log messages * Add Scene Understanding to the ignore list * fix test discovered error * Update README_Dialog.md The dialog has been moved to a different folder. * Add Pulse Shader Graduation to Release Notes * Remove dotnetwinrt dependency in Scene Understanding * Branch synchronization: prerelease/2.6.0_stabilization --> mrtk_development (#9416) * Fix issues associated with the new SU package * Update Interactive Element Docs * Remove Unity 2018 support for scene understanding * Update SceneUnderstandingExample.unity * Add comments * add versioning to the shader import logic * Update ReleaseNotes.md * add versioning comment to shader source files * Update SceneUnderstanding.md * add documentation on updating shaders * add doc comments * undo changes to project prefs * Update release notes * Fix links * Update TOC * Update Assets/MRTK/StandardAssets/EditorUtilities/OnLoadUtilities.cs Co-authored-by: Kurtis <kurtie@microsoft.com> * update TMP version * Explicitly set IsEnabled to false when the SU package is not present Co-Authored-By: Kurtis <3580640+keveleigh@users.noreply.github.com> * Update SceneUnderstanding.md * Add null check for XR not initialized * Update legacy and XR SDK implementation * Fixed articulated hands being classified as HP Motion Controllers (#9410) * Fixed articulated hands being classified as HP Motion Controllers * small modification to ensure we only name interaction sources of kind controller * small adjustment to the naming schemes * implement ignore support for shader update * update shader update documentation and images * fix code validation error (missing doc comment contents) * remove unneeded debug.log messages * Add Scene Understanding to the ignore list * fix test discovered error * Add Pulse Shader Graduation to Release Notes Co-authored-by: MaxWang-MS <68253937+MaxWang-MS@users.noreply.github.com> Co-authored-by: Catherine Diaz <cadia@microsoft.com> Co-authored-by: davidkline-ms <david.kline@microsoft.com> Co-authored-by: David Kline <davidkl@microsoft.com> Co-authored-by: Kurtis <kurtie@microsoft.com> Co-authored-by: Kurtis <3580640+keveleigh@users.noreply.github.com> Co-authored-by: RogPodge <39840334+RogPodge@users.noreply.github.com> Co-authored-by: CDiaz-MS <53493796+CDiaz-MS@users.noreply.github.com> * Update SceneUnderstandingExample.unity * Fix link to installation guide * Formatting * Update Profiles.md * Add additional XR SDK profile info * Add additional general profile info * Update SceneUnderstandingExample.unity * Formatting fix * Add SU to the exclusion list * Update docs-binaries.yml * Update settings.yml * Update docs-binaries.yml * Clean up inconsistent whitespace Removed trailing whitespace, updating brace spacing, etc * Remove and sort usings * Turn off auto mergetool (for now) * On UWP, the type is OpenXRLoaderNoPreInit * Actually, reference OpenXRLoaderBase Some platforms have different types, but so far they all have the same base class * Add a we've moved banner to doc site * Update scripts/docs/templates/mrtk/partials/navbar.tmpl.partial Co-authored-by: RogPodge <39840334+RogPodge@users.noreply.github.com> * Add we've moved to README * Update README.md * Update README.md Co-authored-by: Kurtis <kurtie@microsoft.com> * Ensure async operations on progress indicators don't wait on animations if the object is hidden * Clipping primitive now removes OnPreRender event upon deletion (#9449) * Clipping primitive now removes OnPreRender event upon deletion * fixed so the root cause of multiple events getting added to the handler was addressed * added unit tests for cases where scrolling object collection is destroyed * Avoid calling Physics.ClosestPoint on non-convex collider prevents log spam, and possibly some unexpected side effects of Physics.ClosestPoint failing. * Reduce log spam from HandConstraintPalmUp check that the controller position is available before expecting that the joints have been added to the controller. (I'm also VERY curious why line 208 returns true) * Raise Source Pose Changed on Interaction Update * Add test that fails before this fix, passes with the fix * Merge pull request #9430 from keveleigh/fix-openxr-uwp Fix OpenXR on UWP * Clipping primitive now removes OnPreRender event upon deletion (#9449) * Clipping primitive now removes OnPreRender event upon deletion * fixed so the root cause of multiple events getting added to the handler was addressed * added unit tests for cases where scrolling object collection is destroyed * Typo fixes in tooltips (#9297) * Decouple the scene understanding sample from the spatial awareness one * Update the Scene Understanding sample scene to show SU package is available * Fix the profile issue in the dwell sample scene * Fixed a bug where the grab action was classified incorrectly (#9484) * Fixed a bug where the grab action was classified incorrectly * added more adjustments to accomodate for GripPress reassignment * OpenXR Controller MappingProfile updated for 2020 * Removed the ability for users to set a TypeReferenceProperty to None (#9466) * Removed the ability for users to set a TypeReferenceProperty to None, as it breaks most scenarios * Update Assets/MRTK/Core/Inspectors/PropertyDrawers/TypeReferencePropertyDrawer.cs Co-authored-by: Kurtis <kurtie@microsoft.com> Co-authored-by: Kurtis <kurtie@microsoft.com> * DeviceManager dropdown only appears if there are profiles for the service. (#9414) * made the devicemanager profile dropdown only appear if there are valid profiles to choose from, otherwise the user cannot specify a profile * fixed missing paren * deviceManager profiles now supports multiple types * Users can no longer select (None) for types which have an attribute indicating that they require profiles * added function summaries and addressed PR comments * addressed edge case where there are no profiles available for a service with the requiresProfile attribute * GetProfileTypeForService now looks at the loaded assemblies for determining if an appropriate profile type exists, rather than looking at all available profile assets * modifying behavior to be performant on Unity 2019+, Unity 2018 does not have this improvement, so using the old behavior * adjusted the GetTypes() check in code validation * Update Assets/MRTK/Core/Inspectors/Utilities/MixedRealityProfileUtility.cs Co-authored-by: Kurtis <kurtie@microsoft.com> Co-authored-by: Kurtis <kurtie@microsoft.com> * Add description to the HandInteractionRecordArticulatedHandPose scene * Removed unused arkit and arcore dependencies due to incompatible Gradle versions (#9502) * Fixed typos * Fix for missing gaze extension (Continuation of #9125) (#9438) * Fix for errors occuring when no GazeProvider is selected. * added tests for reinitializing the input system after the gaze provider gets destoryed * fixed formatting issue * added IsNull() check * added IsNull() check again Co-authored-by: Andreas Herbig <andreas.herbig@3spin.de> * Added checks for null for XRGeneralSettings.Instance and XRGeneralSettings.Instance.Manager (#9521) * added checks for null for .Instance and .Manager * explicit null checks Co-authored-by: Kurtis <kurtie@microsoft.com> Co-authored-by: davidkline-ms <david.kline@microsoft.com> Co-authored-by: David Kline <davidkl@microsoft.com> Co-authored-by: Yoon Park <cre8ive.park@gmail.com> Co-authored-by: MaxWang-MS <68253937+MaxWang-MS@users.noreply.github.com> Co-authored-by: Kurtis <3580640+keveleigh@users.noreply.github.com> Co-authored-by: Philipp <allbecomesgood@gmail.com> Co-authored-by: Eusebiu Marcu <marcueusebiu@gmail.com> Co-authored-by: Catherine Diaz <cadia@microsoft.com> Co-authored-by: Cameron Micka <thmicka@microsoft.com> Co-authored-by: CDiaz-MS <53493796+CDiaz-MS@users.noreply.github.com> Co-authored-by: cihankurt98 <31470675+cihankurt98@users.noreply.github.com> Co-authored-by: Mixed Reality Toolkit <54139085+mrtk-bld@users.noreply.github.com> Co-authored-by: Kevin Semple <30964480+polar-kev@users.noreply.github.com> Co-authored-by: Roberto Sonnino <robertos@microsoft.com> Co-authored-by: Patrick Cook <pcook@microsoft.com> Co-authored-by: Andreas Herbig <andreas.herbig@3spin.de>
Overview
Previously, the eye gaze controls were broken and would not let the users pinch to manipulate objects. This PR fixes this by removing the hard-coded dependency on controller names, and it also fixes a bug introduced with pointer naming
old naming:

new naming:

Changes
Verification