FIX: [XRA-675] PoseControl isTracked returns false via non-optimized read paths#2443
FIX: [XRA-675] PoseControl isTracked returns false via non-optimized read paths#2443kevin-zakszewski wants to merge 8 commits into
Conversation
|
|
kevin-zakszewski
changed the title
![u-pr[bot]](https://avatars.githubusercontent.com/in/1201017?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Harness Review
The fix is focused, but the new Pose fast-path gate now accepts isTracked configurations that the old check deliberately excluded, which can make the parent control bypass child-control semantics in overridden layouts.
Reviewed commit 1fdc5db4
🤖 Helpful? 👍/👎
Codecov ReportAll modified and coverable lines are covered by tests ✅ @@ Coverage Diff @@
## develop #2443 +/- ##
============================================
+ Coverage 58.58% 79.02% +20.44%
============================================
Files 738 766 +28
Lines 135831 140451 +4620
============================================
+ Hits 79570 110995 +31425
+ Misses 56261 29456 -26805 Flags with carried forward coverage won't be shown. Click here to find out more.
|
MorganHoarau
left a comment
MorganHoarau
left a comment
There was a problem hiding this comment.
Optimization guard update is correct, and the test covers the regression in both execution path. Would be nice to add proper optimization path for 1-bit in ButtonControl, but I can do that on a different PR if preferred.
FYI @chris-massie
Also updates the mouse test that I believe would be impacted by the change.
…e-bug # Conflicts: # Packages/com.unity.inputsystem/CHANGELOG.md
chris-massie
left a comment
chris-massie
left a comment
There was a problem hiding this comment.
I tested with a scene that displayed the Is Tracked status of controller poses from the Hand Interaction Profile. Obscuring the controller under the desk and waiting eventually caused the device to report false for Is Tracked, and returning above the desk in view of the HMD made the device report true. I also verified the Debugger Window shows 1 instead of 0.003921569, and reading Assert.AreEqual(device.devicePose.isTracked.value, device.isTracked.value) now properly passes with the OculusTouchControllerProfile.OculusTouchController after this fix.
MorganHoarau
left a comment
MorganHoarau
left a comment
There was a problem hiding this comment.
Thanks for the changes and updating the relevant test!
PR description's Comments to reviewers needs to be updated.
I updated the description to reflect your suggestion being applied. |
3 participants
Description
XRA-675
Fix
PoseControl.isTrackedalways returning false via non-optimized read pathsPoseState.isTrackedwas declared withsizeInBits = 8to enable thekFormatPoseoptimization. However, this caused InputStateBlock.ReadFloat() to treat the bool byte value as an 8-bit normalized quantity(1/255 ≈ 0.004)instead of a discrete 0.0 or 1.0. Since0.004falls below the0.5button press threshold,isTrackedalways returned false when read through non-optimized code paths, including the Input Debugger.Summary of Changes
PoseControl.cs — two changes:
sizeInBits = 8tosizeInBits = 1. This makesInputStateBlock.ReadFloat()use theReadSingleBitpath, which correctly returns0.0for1.0f. The struct layout is unaffected -StructLayout(LayoutKind.Explicit)withFieldOffsetattributes controls the memory layout independently ofsizeInBits.isTrackedvalidation fromisTracked.optimizedControlDataType == InputStateBlock.kFormatByteto checkingisTracked.optimizedControlDataType == InputStateBlock.kFormatBit. This preserves thekFormatPosefast path (direct*(PoseState*)ptrstruct cast), which remains valid because the C# bool at byte 0 is always stored as a full byte (0 or 1) regardless of the declared bit width.Testing status & QA
Added the test
Controls_PoseControl_IsTracked_ReadsCorrectly- parameterized with[TestCase(true)]and[TestCase(false)]to run with optimized controls both enabled and disabled. For each case it:poseControl.isTracked.isPressedreturns trueposeControl.isTracked.ReadValue()returns 1.0fposeControl.ReadValue().isTrackedreturns trueisTracked = 0and verifies all three return false/0.0fOverall Product Risks
Comments to reviewers
With this fix,
isTrackedgains a ButtonControl-level optimization — it previously gotkFormatByte (via sizeInBits = 8), and now allows a fast path for 1-bit controls. ThePoseControlwas updated for this new optimized control data type forisTrackedto allow it to use thekFormatPoseoptimization.I looked into adding a
kFormatBitoptimization toAxisControlso that 1-bitButtonControlswould inherit a directReadSingleBitfast path. This would restore the optimization forisTracked, put thePoseControlcheck back on the standardoptimizedControlDataTypepipeline, and benefit other 1-bit buttons or axes in the system (mouse, keyboard, gamepad, etc.). The main concern is the wider blast radius — it touchesAxisControl, which affects every device type, so it would need a proper audit and test pass.Checklist
Before review:
Changed,Fixed,Addedsections.Area_CanDoX,Area_CanDoX_EvenIfYIsTheCase,Area_WhenIDoX_AndYHappens_ThisIsTheResult.During merge:
NEW: ___.FIX: ___.DOCS: ___.CHANGE: ___.RELEASE: 1.1.0-preview.3.