FIX: Misaligned Virtual Cursor when changing resolution (ISXB-1119)

[Darren-Kelly-Unity]

Description

When changing resolution in the virtual cursor sample the virtual cursor ended up misaligned in different resolutions.

Testing status & QA

I have tested in multiple resolutions & swapping resolutions at runtime to be sure this is working.

Overall Product Risks

• Complexity: 1
• Halo Effect: 1

Comments to reviewers

Checklist

Before review:

• Changelog entry added.
  • Explains the change in Changed, Fixed, Added sections.
  • For API change contains an example snippet and/or migration example.
  • JIRA ticket linked, example (case %%). If it is a private issue, just add the case ID without a link.
  • Jira port for the next release set as "Resolved".
• Tests added/changed, if applicable.
  • Functional tests Area_CanDoX, Area_CanDoX_EvenIfYIsTheCase, Area_WhenIDoX_AndYHappens_ThisIsTheResult.
  • Performance tests.
  • Integration tests.
• Docs for new/changed API's.
  • Xmldoc cross references are set correctly.
  • Added explanation how the API works.
  • Usage code examples added.
  • The manual is updated, if needed.

During merge:

• Commit message for squash-merge is prefixed with one of the list:
  • NEW: ___.
  • FIX: ___.
  • DOCS: ___.
  • CHANGE: ___.
  • RELEASE: 1.1.0-preview.3.

[u-pr-agent]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪ The change is localized to virtual cursor motion scaling and UI positioning with a small amount of new logic to validate across a few UI configurations.

🏅 Score: 78 The fix is directionally correct but introduces some null-safety and edge-case risks around `Canvas`/`CanvasScaler` availability and scaling math.

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Null Safety m_CanvasScaler is checked for null but m_Canvas is still dereferenced when computing scales; if no Canvas is found (or TryFindCanvas fails due to hierarchy changes), accessing m_Canvas.pixelRect can throw and break virtual cursor motion. var resolutionXScale = 1f; var resolutionYScale = 1f; if (m_CanvasScaler != null) { resolutionXScale = m_Canvas.pixelRect.xMax / m_CanvasScaler.referenceResolution.x; resolutionYScale = m_Canvas.pixelRect.yMax / m_CanvasScaler.referenceResolution.y; } Divide By Zero The resolution scale divides by m_CanvasScaler.referenceResolution.x/y; if either component is zero (misconfigured scaler), this will produce infinities and propagate into delta and anchored positioning. if (m_CanvasScaler != null) { resolutionXScale = m_Canvas.pixelRect.xMax / m_CanvasScaler.referenceResolution.x; resolutionYScale = m_Canvas.pixelRect.yMax / m_CanvasScaler.referenceResolution.y; } var currentTime = InputState.currentTime; if (Mathf.Approximately(0, m_LastStickValue.x) && Mathf.Approximately(0, m_LastStickValue.y)) { // Motion has started. m_LastTime = currentTime; } // Compute delta. var deltaTime = (float)(currentTime - m_LastTime); var delta = new Vector2(m_CursorSpeed * resolutionXScale * stickValue.x * deltaTime, m_CursorSpeed * resolutionYScale * stickValue.y * deltaTime); Coordinate Space Writing anchoredPosition from newPosition assumes compatible coordinate spaces; when using different Canvas render modes or non-default scaling modes, pixelRect space and RectTransform.anchoredPosition space may not map 1:1, so the cursor could still drift/misalign. var pixelRect = m_Canvas.pixelRect; newPosition.x = Mathf.Clamp(newPosition.x, pixelRect.xMin, pixelRect.xMax); newPosition.y = Mathf.Clamp(newPosition.y, pixelRect.yMin, pixelRect.yMax); } ////REVIEW: the fact we have no events on these means that actions won't have an event ID to go by; problem? InputState.Change(m_VirtualMouse.position, newPosition); InputState.Change(m_VirtualMouse.delta, delta); // Update software cursor transform, if any. if (m_CursorTransform != null && (m_CursorMode == CursorMode.SoftwareCursor || (m_CursorMode == CursorMode.HardwareCursorIfAvailable && m_SystemMouse == null))) { m_CursorTransform.anchoredPosition = m_CanvasScaler == null ? newPosition : new Vector2(newPosition.x / resolutionXScale, newPosition.y / resolutionYScale); }

Update review

---

🤖 Helpful? Please react with 👍/👎 | Questions❓Please reach out in Slack #ask-u-pr-agent

[u-pr-agent]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix scaling computation and null-safety Guard against m_Canvas being null when m_CanvasScaler is found, otherwise m_Canvas.pixelRect can throw at runtime. Also avoid using xMax/yMax as “size”; use pixelRect.width/height (or size) to compute scale correctly even if the rect origin is not at (0,0). Packages/com.unity.inputsystem/InputSystem/Plugins/UI/VirtualMouseInput.cs [426-432] var resolutionXScale = 1f; var resolutionYScale = 1f; -if (m_CanvasScaler != null) +if (m_CanvasScaler != null && m_Canvas != null) { - resolutionXScale = m_Canvas.pixelRect.xMax / m_CanvasScaler.referenceResolution.x; - resolutionYScale = m_Canvas.pixelRect.yMax / m_CanvasScaler.referenceResolution.y; + var pixelRect = m_Canvas.pixelRect; + resolutionXScale = pixelRect.width / m_CanvasScaler.referenceResolution.x; + resolutionYScale = pixelRect.height / m_CanvasScaler.referenceResolution.y; } Suggestion importance[1-10]: 7 __ Why: As written, m_Canvas.pixelRect can throw if m_Canvas is null, and using pixelRect.xMax/yMax is incorrect for size when the rect origin is not (0,0); width/height is the right measure. This is a real runtime correctness fix in UpdateMotion.	Medium
	Ensure scaler matches the canvas Prefer finding the CanvasScaler from the same Canvas hierarchy you already resolved to avoid mismatches when nested canvases exist. This prevents using a scaler from a different parent canvas than the one providing pixelRect, which would skew scaling. Packages/com.unity.inputsystem/InputSystem/Plugins/UI/VirtualMouseInput.cs [371-375] private void TryFindCanvas() { m_Canvas = m_CursorGraphic?.GetComponentInParent<Canvas>(); - m_CanvasScaler = m_CursorGraphic?.GetComponentInParent<CanvasScaler>(); + m_CanvasScaler = m_Canvas != null ? m_Canvas.GetComponent<CanvasScaler>() : null; } Suggestion importance[1-10]: 6 __ Why: Resolving CanvasScaler independently from m_CursorGraphic can pick a scaler from a different parent than m_Canvas, causing inconsistent scaling against m_Canvas.pixelRect. Tying m_CanvasScaler lookup to the resolved m_Canvas improves correctness with nested canvases.	Low
More suggestions

---

🤖 Helpful? Please react with 👍/👎 | Questions❓Please reach out in Slack #ask-u-pr-agent

[codecov-github-com]

Codecov Report

Attention: Patch coverage is 73.33333% with 4 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...system/InputSystem/Plugins/UI/VirtualMouseInput.cs	73.33%	4 Missing ⚠️

@@           Coverage Diff            @@
##           develop    #2315   +/-   ##
========================================
  Coverage    77.95%   77.95%           
========================================
  Files          477      476    -1     
  Lines        97419    97448   +29     
========================================
+ Hits         75943    75966   +23     
- Misses       21476    21482    +6     

Flag	Coverage Δ
inputsystem_MacOS_2022.3	5.54% <ø> (-0.01%)	⬇️
inputsystem_MacOS_2022.3_project	75.47% <73.33%> (-0.03%)	⬇️
inputsystem_MacOS_6000.0	5.32% <ø> (-0.01%)	⬇️
inputsystem_MacOS_6000.0_project	77.36% <73.33%> (-0.05%)	⬇️
inputsystem_MacOS_6000.2	5.32% <ø> (-0.01%)	⬇️
inputsystem_MacOS_6000.2_project	77.36% <73.33%> (-0.06%)	⬇️
inputsystem_MacOS_6000.3	5.32% <ø> (-0.01%)	⬇️
inputsystem_MacOS_6000.3_project	77.36% <73.33%> (-0.06%)	⬇️
inputsystem_MacOS_6000.4	5.32% <ø> (-0.01%)	⬇️
inputsystem_MacOS_6000.4_project	77.37% <73.33%> (+<0.01%)	⬆️
inputsystem_MacOS_6000.5	5.32% <ø> (-0.01%)	⬇️
inputsystem_MacOS_6000.5_project	77.37% <73.33%> (-0.05%)	⬇️
inputsystem_Ubuntu_2022.3	5.54% <ø> (-0.01%)	⬇️
inputsystem_Ubuntu_2022.3_project	75.27% <73.33%> (-0.03%)	⬇️
inputsystem_Ubuntu_6000.0	5.32% <ø> (-0.01%)	⬇️
inputsystem_Ubuntu_6000.0_project	77.16% <73.33%> (-0.06%)	⬇️
inputsystem_Ubuntu_6000.2	5.32% <ø> (-0.01%)	⬇️
inputsystem_Ubuntu_6000.2_project	77.16% <73.33%> (-0.06%)	⬇️
inputsystem_Ubuntu_6000.3	5.32% <ø> (-0.01%)	⬇️
inputsystem_Ubuntu_6000.3_project	77.17% <73.33%> (-0.05%)	⬇️
inputsystem_Ubuntu_6000.4	5.33% <ø> (-0.01%)	⬇️
inputsystem_Ubuntu_6000.4_project	77.17% <73.33%> (-0.06%)	⬇️
inputsystem_Ubuntu_6000.5	5.33% <ø> (-0.01%)	⬇️
inputsystem_Ubuntu_6000.5_project	77.18% <73.33%> (-0.05%)	⬇️
inputsystem_Windows_2022.3	5.54% <ø> (-0.01%)	⬇️
inputsystem_Windows_2022.3_project	75.60% <73.33%> (-0.03%)	⬇️
inputsystem_Windows_6000.0	5.32% <ø> (-0.01%)	⬇️
inputsystem_Windows_6000.0_project	77.49% <73.33%> (-0.05%)	⬇️
inputsystem_Windows_6000.2	5.32% <ø> (-0.01%)	⬇️
inputsystem_Windows_6000.2_project	77.49% <73.33%> (-0.05%)	⬇️
inputsystem_Windows_6000.3	5.32% <ø> (-0.01%)	⬇️
inputsystem_Windows_6000.3_project	77.49% <73.33%> (-0.05%)	⬇️
inputsystem_Windows_6000.4	5.32% <ø> (-0.01%)	⬇️
inputsystem_Windows_6000.4_project	77.49% <73.33%> (-0.05%)	⬇️
inputsystem_Windows_6000.5	5.32% <ø> (-0.01%)	⬇️
inputsystem_Windows_6000.5_project	77.49% <73.33%> (-0.05%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...system/InputSystem/Plugins/UI/VirtualMouseInput.cs	69.11% <73.33%> (+1.16%)	⬆️

... and 8 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[ekcoh]

I only looked at code changes, but fixing this TODO looks legit, I would recommend adding a CHANGELOG entry though and linking the associated public issue tracker from the changelog.

[ekcoh]

Needs CHANGELOG entry.