Skip to content

Conversation

@Darren-Kelly-Unity
Copy link
Collaborator

@Darren-Kelly-Unity Darren-Kelly-Unity commented Jan 8, 2026

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
Copy link
Contributor

u-pr-agent bot commented Jan 8, 2026

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
Copy link
Contributor

u-pr-agent bot commented Jan 8, 2026

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    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
Copy link

codecov-github-com bot commented Jan 8, 2026

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.

@Darren-Kelly-Unity Darren-Kelly-Unity changed the title Add a fix for the resolution issue / misaligned virtual cursor sample. Fix: Misaligned Virtual Cursor when changing resolution Jan 8, 2026
@Darren-Kelly-Unity Darren-Kelly-Unity changed the title Fix: Misaligned Virtual Cursor when changing resolution Fix: Misaligned Virtual Cursor when changing resolution (ISXB-1119) Jan 8, 2026
@Darren-Kelly-Unity Darren-Kelly-Unity changed the title Fix: Misaligned Virtual Cursor when changing resolution (ISXB-1119) FIX: Misaligned Virtual Cursor when changing resolution (ISXB-1119) Jan 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants