Hand Tracking change requests #476

StephenHodgson · 2020-03-29T19:07:29Z

Refactored the hand controller data provider, as well as the simulated devices
- Added a new ISimulatedControllerDataProvider interface to help with the dependency problems
Removed references to profiles in the implementations themselves (they should only ever be used in the inspectors, never cached.
Updated all the profile inspectors to make sure they functioned properly.
Removed HandTrackingProfile in favor of a per platform profile option.

# Conflicts: # XRTK-Core/Packages/com.xrtk.core/Inspectors/Profiles/InputSystem/MixedRealityInputSystemProfileInspector.cs # XRTK-Core/Packages/com.xrtk.core/Inspectors/Profiles/MixedRealityServiceProviderProfileInspector.cs

StephenHodgson · 2020-03-30T02:31:58Z

...s/com.xrtk.core/Definitions/Controllers/Simulation/SimulatedControllerDataProviderProfile.cs

-        /// <summary>
-        /// The currently registered controller data providers for simulation.
-        /// </summary>
-        public ControllerDataProviderConfiguration[] RegisteredControllerDataProviders => registeredControllerDataProviders;


These are registered in the ControllerDataProviderProfile with the rest of the controllers

...ckages/com.xrtk.core/Definitions/Controllers/BaseMixedRealityHandDataProviderProfile.cs.meta

...re/Packages/com.xrtk.core/Inspectors/Profiles/MixedRealityServiceProviderProfileInspector.cs

added icon for the data provider as well

SimonDarksideJ · 2020-03-30T07:27:17Z

....core/Definitions/Controllers/Simulation/Hands/SimulatedHandControllerDataProviderProfile.cs

@@ -8,7 +8,7 @@
 namespace XRTK.Definitions.Controllers.Simulation.Hands
 {
    [CreateAssetMenu(menuName = "Mixed Reality Toolkit/Input System/Controller Data Providers/Simulated Hand Controller Data Provider Profile", fileName = "SimulatedHandControllerDataProviderProfile", order = (int)CreateProfileMenuItemIndices.Input)]
-    public class SimulatedHandControllerDataProviderProfile : BaseMixedRealityControllerDataProviderProfile
+    public class SimulatedHandControllerDataProviderProfile : SimulatedControllerDataProviderProfile


Any reason for a specific rather than generic base? Concerned this separates this from the standard implementation

SimonDarksideJ · 2020-03-30T07:32:41Z

I think the changes in this PR are fine. Although for the main hands PR, I'm concerned with the need for multiple profiles for multiple platforms to support hands.
Ideally, this should be worked into the existing platform data providers. With Oculus specifically, there is a risk that having both controllers and hands may be detrimental

FejZa

Approved but I'll have to verify all of these changes whether things still work as expected.

...re/Packages/com.xrtk.core/Inspectors/Profiles/MixedRealityServiceProviderProfileInspector.cs

FejZa · 2020-03-30T10:41:37Z

...m.xrtk.core/Interfaces/InputSystem/Controllers/Hands/ISimulatedHandControllerDataProvider.cs

@@ -6,7 +6,7 @@

 namespace XRTK.Interfaces.InputSystem.Controllers.Hands
 {
-    public interface ISimulatedHandControllerDataProvider : IMixedRealityHandControllerDataProvider
+    public interface ISimulatedHandControllerDataProvider : ISimulatedControllerDataProvider


This looks wrong. ISimulatedControllerDataProvider defines properties that apply to simulation in generel, like Depth Mulitplier etc. The simulated hand data provider doesn't need to redefine those.

FejZa · 2020-03-30T10:42:23Z

XRTK-Core/Packages/com.xrtk.core/Interfaces/Providers/Controllers/IMixedRealityController.cs

+        /// <summary>
+        /// 
+        /// </summary>
+        Vector3 AngularVelocity { get; }


This is weird, pretty sure I already has these two added to IMixedRealityController. Docs are missing. But I can add those.

FejZa · 2020-03-30T10:45:57Z

...kages/com.xrtk.core/Definitions/Controllers/Hands/BaseMixedRealityHandDataProviderProfile.cs

+    /// <summary>
+    /// Provides additional configuration options for hand controller data providers.
+    /// </summary>
+    public abstract class BaseMixedRealityHandDataProviderProfile : BaseMixedRealityControllerDataProviderProfile


This is LITERALLY what I had a while ago when discussion started and it was suggested to add a HandTrackingProfile to the input system profile to instead. I even provided screenshots and everything to make 100% sure that's what' asked for. Basically you are here just reversing those changes I did after being asked to do them here and restoring the state I had before that.

FejZa · 2020-03-30T10:48:42Z

XRTK-Core/Packages/com.xrtk.core/Definitions/InputSystem/MixedRealityHandTrackingProfile.cs

+//    /// Configuration profile settings for setting up and consuming gesture based input actions.
+//    /// </summary>
+//    [CreateAssetMenu(menuName = "Mixed Reality Toolkit/Input System/Hand Tracking Profile", fileName = "MixedRealityHandTrackingProfile", order = (int)CreateProfileMenuItemIndices.HandTracking)]
+//    public class MixedRealityHandTrackingProfile : BaseMixedRealityProfile


See my comment above about this change. Also this seems only commented out, I'd rather have code removed that's not needed anymore.

FejZa · 2020-03-30T10:48:59Z

XRTK-Core/Packages/com.xrtk.core/Definitions/InputSystem/MixedRealityInputSystemProfile.cs

-            internal set => handTrackingProfile = value;
-        }
+        //[SerializeField]
+        //[Tooltip("Profile for platform agnostic hand tracking configuration.")]


Again see comment from above.

FejZa · 2020-03-30T10:55:57Z

...e/Packages/com.xrtk.core/Providers/Controllers/Simulation/SimulatedControllerDataProvider.cs

@@ -17,7 +16,7 @@ namespace XRTK.Providers.Controllers.Simulation
    /// The simulated controller data provider is mainly responsible for managing
    /// active simulated controllers, such as hand controllers.
    /// </summary>
-    public class SimulatedControllerDataProvider : BaseControllerDataProvider
+    public class SimulatedControllerDataProvider : BaseControllerDataProvider, ISimulatedControllerDataProvider


I can't really wrap my head around what you did to simulation. Going to have to check this out to understand. To me it currently looks like with your changes the SimulatedControllerDataProvider is now useless.

FejZa · 2020-03-30T10:56:25Z

...ges/com.xrtk.core/Inspectors/Profiles/InputSystem/MixedRealityInputSystemProfileInspector.cs

@@ -36,7 +35,6 @@ protected override void OnEnable()
            controllerVisualizationProfile = serializedObject.FindProperty("controllerVisualizationProfile");


Again see comment from above.

* Update profile platform utility * Add and implement TryGetBounds(...) for hand controllers * Update submodule * Move hand visualizers to SDK * Remove mesh and joint visualizer setting * Update SDK submodule * Resolve merge issue * Update inspector namespace * Fix simulation created asset path and name * Fix UpdateController() not in basecontroleller interface * Base hand data provider cleanup * Remove all occurences of default hand controller * Moving state update to hand controller * Pipe hand data update though input system * Fix namespaces and assign asset icons * Remove obsolete hand specific visualization profile * Update submodules * Remove IMixedRealityHandControllerDataProvider since obsolete * Separate simulation and hands * Remove SimulationInput (using MixedRealityPose instead) * Add simulated controller type setting * Update SDK checkout * Cleanup simulated dp * Undo newline file changes * Update submoduules * Remove profile property from data provider and expose settings as properties * Merge SimulatedHandControllerState into SimulatedHandController * Move constructors to top * Rename HandUtils to HandUtilities * Remove HandRay class (returning in separate PR) * Cleanup utils and make extensions * Some more extnsion conversions * Remove not needed setter * Remove dead event definition * Remove setters in hand data snapshot * Add setter to hand data mesh property * Remove cached camera rerefence * Update SDK * Add hand mesh data constructors * Update Oculus submodule * Revert some meta files not related to hands changes * Revert not related change * Remove simulated controller controls from profile * Update interactions in base * Use interaction mapping for movement * Reset editor window options * remove hand joitn pose dictionary * Update SDK checkout * Pull update controller * Update submodules * Remove redundant joint count definition * Fix broken JointCount references * Minor BaseHandController cleanup * Fix copy paste error in windows api checker * Update submodules * Fix missing UpdateController definition issue * Fix controller list management * Update submodules * Update SDK checkout * Update SDK checkout * Update SDK * Define agnostic hand controller update * Fix namespaces and create mr hand controller mapping profile * Fix texture path for hand mapping profile * Update SDK checkout * Update WMR checkout * Update submodules * Update submodules * Update submodules * Upate simulated hand controller implementation to new interface * Make joint poses private * Add comment for simulation specifc update * Update submodules * Update sdk checkout * Update submodules * Move interaction updates to base hand * Remove default interactions defintions * Cleanup using * Update SDK checkout * Remove hand triangles shader * Update submodules * Update SDK checkout * Implement Gizmo utlity to draw wire capsule * Update SDK * Implement GetOrAddComponent extension * Implement bounds calculation for each TrackedHandBounds type * Update SDK checkout * Update SDK * Introduce shared hand profile configuraiton * Introduce base hand controller data provide to consume shared profile * Update simulation to make use of new shared hand data provider settings * Add label to Simulation specific in inspector * Update submodules * Update Oculus checkout * Introduce HandMeshingEnabled profile setting * Raise input source events in base hand controller * Update SDK * Delete now redundant GizmoUtility * Introduce "Hand Tracking System Profile" * Implement hand ray configuration * Update SDK * Update SDK * Update SDK * Update SDK and Oculus checkouts * Update Lumin / Oculus checkout * Update SDK * Remove SimulatedHandController button mappings from BaseHandController * Update simulation input actions in shc * Hand Tracking change requests (#476) * latest changes from dev plus change requests * pulled out new interface into it's own file * updated base controller data provider profile namespace added icon for the data provider as well * updated sdk checkout * Update submodule checkouts * Refactor simulation data provider relationship * Update SDK checkout * updated simulated hand data profile with required fields for IMixedRealityHandControllerDataProvider (#480) * removed ignored meta file * Fixed merge issues * Update copyright headers * Update checkouts * Return of the global hand tracking profile * Update SDK * Fix hands not showing up issue * Simplify ISimulatedHandControllerDataProvider retrieval * Remove BaseMixedRealityHandDataProviderProfile * Remove BaseHandControllerDataProvider * Add hand settings overrides to simulation profile * bumped minor version * cleaned up base hand controller (#496) * cleaned up base hand controller * missed a few * Add docs to explaoin what OnInspectorAdditionalGUI does in simulation * Update submodules * updated oculus * Update submodule * Remove hand ray type setting * Change requests/cleanup definitions (#497) * updated simulated hand controller pose type from class to struct removed allocators each frame when lerping between two poses misc formatting * updated sdk checkout * asset organization * updated wmr checkout * updated lumin commit * Moved stopwatch utility * updated submodules * Added BaseHandDatProvider and BaseHandDataProviderProfile * Add controller pose field to base hand * Fix simulation * Fix naming * Fix UWP running in editor when not configured to * Update submodules * moved constructor and changed access modifier for the constructor * updated sdk checkout * moved numerics extension and updated wmr checkout * updated wmr checkout * updated wmr checkout * updated Lumin checkout * updated Lumin checkout * fixed hand data provider inspectors * Update Lumin,SDK * Update Lumin * updated sdk checkout * Fix platform checks for iOS, Android and WebGL * apply global override if different from platform setting * updated lumin SDK * updated lumin checkout * updated oculus checkout * Update submodules * updated lumin checkout * change-request/hand-tracking (#501) * Added IMixedRealityControllerDataProvider to IMixedRealityController updated all the controller constructors to take a data provider parameter * a bit more cleanup and ensured controllers we being added to the data provider active controller registry * updated wmr checkout * Fix simulated hand controller instantiation * Fix UseSourcePose fallback * Update checkouts * Update SDK checkout * Fix missing data provider parameter for Unity Joystick controller * Rename OpenVR Oculus Touch profile and inspector to avoid confusion * Update checkouts * Update WMR checkout * reverting Oculus Touch rename * Revert visualizer.UseSourcePoseData change * updated sdk checkout * Update SDK * misc formatting * fixed constructor arg exception * updated sdk checkout * fixed merge conflict * fixed documentation * updated simulated hand controller to use data provider reference * Fix pose change raised twice per update * Fix some docs in visualization profile * Update SDK submodule * updated submodules Co-authored-by: Stephen Hodgson <[email protected]> Co-authored-by: Simon (Darkside) Jackson <[email protected]> Co-authored-by: Stephen Hodgson <[email protected]>

latest changes from dev plus change requests

dea7abd

StephenHodgson requested a review from FejZa March 29, 2020 19:07

StephenHodgson added the Ready for review PR finished primary development, open for review label Mar 29, 2020

StephenHodgson added 2 commits March 29, 2020 15:21

pulled out new interface into it's own file

2a7e9cb

StephenHodgson changed the title ~~latest changes from dev plus change requests~~ Hand Tracking change requests Mar 30, 2020

StephenHodgson commented Mar 30, 2020

View reviewed changes

StephenHodgson assigned FejZa Mar 30, 2020

StephenHodgson requested a review from SimonDarksideJ March 30, 2020 02:37

StephenHodgson assigned SimonDarksideJ and StephenHodgson Mar 30, 2020

StephenHodgson commented Mar 30, 2020

View reviewed changes

...ckages/com.xrtk.core/Definitions/Controllers/BaseMixedRealityHandDataProviderProfile.cs.meta Outdated Show resolved Hide resolved

StephenHodgson commented Mar 30, 2020

View reviewed changes

...re/Packages/com.xrtk.core/Inspectors/Profiles/MixedRealityServiceProviderProfileInspector.cs Show resolved Hide resolved

StephenHodgson added 2 commits March 29, 2020 23:29

updated base controller data provider profile namespace

9d23764

added icon for the data provider as well

updated sdk checkout

40d0ff9

SimonDarksideJ reviewed Mar 30, 2020

View reviewed changes

FejZa approved these changes Mar 30, 2020

View reviewed changes

FejZa merged commit 42f03be into feature/hand-tracking Mar 30, 2020

FejZa deleted the change-requests/hand-tracking branch March 30, 2020 12:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hand Tracking change requests #476

Hand Tracking change requests #476

StephenHodgson commented Mar 29, 2020 •

edited

Loading

StephenHodgson Mar 30, 2020

SimonDarksideJ Mar 30, 2020

SimonDarksideJ commented Mar 30, 2020

FejZa left a comment

FejZa Mar 30, 2020

FejZa Mar 30, 2020

FejZa Mar 30, 2020

FejZa Mar 30, 2020

FejZa Mar 30, 2020

FejZa Mar 30, 2020

FejZa Mar 30, 2020

		@@ -36,7 +35,6 @@ protected override void OnEnable()
		controllerVisualizationProfile = serializedObject.FindProperty("controllerVisualizationProfile");

Hand Tracking change requests #476

Hand Tracking change requests #476

Conversation

StephenHodgson commented Mar 29, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SimonDarksideJ commented Mar 30, 2020

FejZa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

StephenHodgson commented Mar 29, 2020 •

edited

Loading