Fixed for issue 940: HandConstraint hand tracking events not being fired (OnFirstHandDetected/OnHandActivate/OnLastHandLost/OnHandDeactivate) when SolverHandler TrackedHand is set to Right or Left #956
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
See Issue 940
HandConstraint hand tracking events are not being fired (OnFirstHandDetected/OnHandActivate/OnLastHandLost/OnHandDeactivate) when SolverHandler TrackedHand is set to Right or Left.
Affects / Reported Issue With:
Unity 2022.3.7f1
MRTK3 (4.0.0-pre.1)
Cause and Resolution
I've been looking into this. It appears the cause of the bug relates to the interplay between the SolverHandler class and the HandConstraint class and the logic they use to determine if a hand is being actively tracked.
When the SolverHandler TrackedHandedness mode is set to Both/Everything, when AttachToNewTrackedObject is called it looks to see if the preferred hand is being tracked and if not, if the other hand is being tracked - i.e. it's checking for a hand swap / change of handedness. If neither hand is now tracked (or the preferred hand stops being tracked) it sets the currentTrackedHandedness to the other hand or None. Its criteria for determining if a hand is tracked is to look at the relevant XRNode (e.g. left/right hand) and checks if HasValue is true AND crucially will also try to read the palm joint data if so. This attempt to read the palm position will fail if the hand is not actively being tracked (even though HasValue will always be true). The value of currentTrackedHandedness is used by the HandConstraint class, mapping to it's trackedNode state. When this value effectively changes, it triggers the logic in SolverUpdate in HandConstraint.cs to fire the relevant OnFirstHandDetected/OnHandActivate/OnLastHandLost/OnHandDeactivate event.
When SolverHandler TrackedHandedness is set to Left (and also presumably Right), SolverHandler.AttachToNewTrackedObject will never update the currentTrackedHandedness based on the tracked hand state (as it doesn't look for a hand swap) - so it will remain set to left or right. Therefore, when SolverUpdate in HandConstraint.cs is called, the trackedNode will (effectively) remain the same - i.e. left or right. The event firing logic would still be fine IF when the hand stops being tracked the XRNode.HasValue flag changed value to false, but it does not. If it adopted the same logic used in SolverHandler and also checked to see if it could also retrieve the palm position, then the tracking state change would be picked up and the events triggered.
Resolution
I'd recommend that HandConstraint::SolverUpdate use the same logic for determining whether a hand is tracked as SolverHandler - (i.e. as defined in IsHandTracked) and looks at both the XRNode.HasValue flag and whether it can actually retrieve the palm joint position, rather than just looking for a change of handedness or whether the current XRNode returns true for HasValue. @keveleigh what do you think? If you agree I'll make that change.
Note: I assume that XRNode.HasValue always stays true because of a quirk of how individual OpenXR providers interpret whether a hand is being tracked or not - i.e. it's reporting if hand tracking is possible, not if a hand is currently being tracked.