ISX-2228: Add OrientationSensor (device orientation parity) - #2468
ISX-2228: Add OrientationSensor (device orientation parity)#2468K-Tone wants to merge 8 commits into
Conversation
Add an OrientationSensor device to the Input System package that reports the physical device orientation as a discrete value, providing feature parity with the legacy UnityEngine.Input.deviceOrientation property. - Add package-local UnityEngine.InputSystem.DeviceOrientation enum (values identical to the legacy enum, kept independent of the legacy input module). - Add OrientationControl : InputControl<DeviceOrientation>, modeled on TouchPhaseControl. - Add OrientationState (FourCC 'ORNT') and OrientationSensor : Sensor. The reported value is deliberately not compensated for screen orientation. - Register the "Orientation" control and "OrientationSensor" device layouts. - Add a mock-runtime unit test and cover OrientationSensor in Devices_CanCreateDevice. Native producers (per-platform) and docs/changelog follow in later changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a layout matcher so the native iOS orientation device (interface "iOS", deviceClass "Orientation") resolves to the OrientationSensor layout, following the same convention as the Gravity/Attitude/ LinearAcceleration sensors. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a layout matcher so the native Android orientation device (interface "Android", deviceClass "Orientation") resolves to the OrientationSensor layout. Unlike the other Android sensors it is not an ASensor, so it maps directly to the base layout rather than an AndroidSensor variant. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Surface an OrientationSensor when connected to the Unity Remote so device orientation can be tested in the Editor. - Handle the existing MessageType.DeviceOrientation message (the Remote app already sends it) and queue it to an OrientationSensor added on connect. No change to the Remote app is required. - Add a DeviceOrientationMessage struct matching HandleOrientationMessage in GenericRemote.cpp (header + int32 orientation, same enum values as ours). - Remove the device on disconnect and clear it on manual removal. - Add Remote_CanReceiveDeviceOrientationFromUnityRemote test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![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 runtime integration follows the existing sensor and Unity Remote patterns, but the PR leaves the documented migration path inaccurate and adds an unrelated project-wide dependency.
The shipped documentation still says Input.deviceOrientation has “No corresponding API yet” in Documentation~/corresponding-old-new-api.md, and Documentation~/supported-sensors-reference.md omits the new sensor. Users following the migration guide or support matrix will therefore conclude this feature is unavailable. Update those references to point to OrientationSensor.current.orientation.ReadValue() and document its supported platforms.
Reviewed commit 732016b
🤖 Helpful? 👍/👎
Review feedback: the name OrientationSensor reads ambiguously against screen orientation. Rename the public device (and its layout name) to DeviceOrientationSensor. Matchers, Unity Remote, and tests updated to match. The DeviceOrientation enum and OrientationControl are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review feedback (morgan): keep the state struct name consistent with the DeviceOrientationSensor device. Stays in sync with the native struct via the 'ORNT' FourCC. Test + Unity Remote references updated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@u-pr Can you check if all the reviewer comments have been fixed thus far? |
|
Yes, all reviewer comments have been addressed:
🤖 Helpful? 👍/👎 |
Review feedback (Morgan):
- Rename OrientationControl -> DeviceOrientationControl and its control
layout "Orientation" -> "DeviceOrientation" (registration + the state
field's layout=, kept in sync), plus the device layout displayName.
- Update the migration guide (corresponding-old-new-api.md) so
Input.deviceOrientation maps to
DeviceOrientationSensor.current.orientation.ReadValue() instead of "No
corresponding API yet", add the sensor to the supported-sensors reference,
and add a CHANGELOG entry (u-pr[bot] P2).
Note: the iOS/Android layout matchers still use .WithDeviceClass("Orientation")
- that string matches the native device descriptor type, a different
identifier from the control layout, so it is intentionally left unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…o "DeviceOrientation"
Review feedback (Morgan): tidy the internal device-class token for
consistency. The iOS/Android layout matchers now use
.WithDeviceClass("DeviceOrientation"), matching the native descriptor.type
change in the companion unity.git PR. WebGL/UWP match by the full layout
name directly and are unaffected.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@u-pr Can you check if all the reviewer comments were resolved? |
|
Yes, all reviewer comments and suggestions have been addressed and resolved:
🤖 Helpful? 👍/👎 |
ekcoh
left a comment
ekcoh
left a comment
There was a problem hiding this comment.
Looks good, lest some questions/reflections - basically happy to approve but want to also look at the native part before approval.
| /// A control reading a <see cref="DeviceOrientation"/> value. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// This is used by <see cref="DeviceOrientationSensor"/> to report the physical orientation of the device |
There was a problem hiding this comment.
Great it provides parity, but is there any kind of definition of what the reference is? Might be good to define that - if possible - since that would be the first thing I would wonder as a dev using it.
| { | ||
| public static FourCC kFormat => new FourCC('O', 'R', 'N', 'T'); | ||
|
|
||
| // Note: unlike the other sensors this value is *not* compensated for screen orientation. It reports |
There was a problem hiding this comment.
Would it make sense to provide compensated value as well in the future you think? Generally I would argue that is a binding property - but just curious on your perspective after working with this?
There was a problem hiding this comment.
Doesn't this note now contradict current native implementation?
https://github.cds.internal.unity3d.com/unity/unity/pull/118961/commits/edab6bc6bd00c2cf305443e9ce2816b25aedc87a#diff-061a8ecb697b03f7d7531a7c45406e67032416ce2687c265bb9a91978a46e663R301
| /// is a package-local enum, kept independent of the legacy input module. | ||
| /// </remarks> | ||
| /// <seealso cref="DeviceOrientationSensor"/> | ||
| public enum DeviceOrientation |
There was a problem hiding this comment.
Maybe it's just me but I find DeviceOrientation confusing since it's a quantised orientation of the screen more than anything else. Does it translate well to non-mobile sensors - I guess this mimics Input Manager president so fine by all means - just sharing a reaction since device orientation could also be seen as a Vector3 relative to some other reference Vector3 if not quantized.
I would have suspected orientation could just be derived from the Attitude sensor as part of a binding but maybe I am wrong? At least that is how I have implemented orientation based bindings previously. How is this different from quantised Attitude sensor values? Is it due to getting corresponding enum from OS?
There was a problem hiding this comment.
Not sure it will cover everything, but Tomas wanted it named that way as there is also a concept of screen orientation.
This one allow for value like device face up and down.
Does it translate well to non-mobile sensors
Currently, only mobile and web do have this. for gamepad, they are fed through device extension.
There was a problem hiding this comment.
Its fine as-is not a strong opinion, just wanted ensure we question it critically before committing to it, e.g. if a raw device orientation Vector3 is added later - it needs a matching name as well - not blocking in anyway - just a reflection
4 participants
Summary
Adds a
DeviceOrientationSensorto the Input System package, giving feature parity with the legacyUnityEngine.Input.deviceOrientation(ISX-2228, part of the IM API parity epic ISX-2108).The Input System had every raw motion sensor but no discrete physical-orientation value. This adds one, modeled on the existing sensor + enum-control patterns.
What's in this PR (managed / package side)
DeviceOrientationenum (UnityEngine.InputSystem) — package-local, values identical to the legacyUnityEngine.DeviceOrientation(Unknown=0 … FaceDown=6). Kept independent of the legacy input module so the package stays self-contained.DeviceOrientationControl : InputControl<DeviceOrientation>— enum-backed control, modeled onTouchPhaseControl.DeviceOrientationSensor : Sensor+DeviceOrientationState(FourCC'ORNT',int orientation). Reports the raw physical orientation — deliberately not run through screen-orientation compensation."DeviceOrientation"control and"DeviceOrientationSensor"device layouts.deviceClass "DeviceOrientation") toDeviceOrientationSensor. WebGL/UWP match by the full layout name directly (no matcher needed).DeviceOrientationmessage (type 4) — no Remote-app change required — and surface aDeviceOrientationSensorwhile connected.Tests
Devices_CanGetDeviceOrientationReading(mock runtime) +DeviceOrientationSensoradded toDevices_CanCreateDevice.Remote_CanReceiveDeviceOrientationFromUnityRemote.Companion PR
Native per-platform producers that feed this device live in
unity.git(trunk): ISX-2228 native device orientation producers. Both are needed end-to-end (and land together — device matching depends on both the matcher and the nativedescriptor.typeagreeing); this managed side also gets mirrored intounity.git/ShadowPackagesat publish time.Notes
DeviceOrientationSensor.current.orientation.ReadValue()(enable the device first, like any sensor).using UnityEngine;andusing UnityEngine.InputSystem;disambiguateDeviceOrientationwith ausingalias — the same established pattern asGyroscope/TouchPhase.🤖 Generated with Claude Code