Skip to content

FIX: Implement caching for InputControlPath display name#2344

Merged
josepmariapujol-unity merged 6 commits intodevelopfrom
input/fixing-cache-path-input-control-path-editor-todo
Feb 26, 2026
Merged

FIX: Implement caching for InputControlPath display name#2344
josepmariapujol-unity merged 6 commits intodevelopfrom
input/fixing-cache-path-input-control-path-editor-todo

Conversation

@josepmariapujol-unity
Copy link
Collaborator

@josepmariapujol-unity josepmariapujol-unity commented Feb 12, 2026 •
edited
Loading

Description

Purpose of this PR is to fix the TODO by caching per path value and only recompute when the string actually
changes.

ISX-2501

Testing status & QA

Check that it works as before, when inputing in the InputControlPath.

Overall Product Risks

Please rate the potential complexity and halo effect from low to high for the reviewers. Note down potential risks to specific Editor branches if any.

  • Complexity: 1
  • Halo Effect: 1

Comments to reviewers

Please describe any additional information such as what to focus on, or historical info for the 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.

@josepmariapujol-unity josepmariapujol-unity self-assigned this Feb 12, 2026
@u-pr
Copy link
Contributor

u-pr bot commented Feb 12, 2026 •
edited
Loading

PR Reviewer Guide 🔍

(Review updated until commit 8086d03)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪

The changes are localized to a single method in one file, implementing a straightforward caching pattern to optimize string generation in the editor UI.
🏅 Score: 100

The PR effectively addresses the performance issue (GC churn) by caching the display name based on the path string. The implementation correctly handles nulls, uses efficient string comparison, and follows Unity editor coding standards.
🧪 No relevant tests
🔒 No security concerns identified
⚡ No major issues detected
  • Update review

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

@u-pr
Copy link
Contributor

u-pr bot commented Feb 12, 2026 •
edited
Loading

PR Code Suggestions ✨

Latest suggestions up to 8086d03
Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Skip cache update when in manual edit mode

The display name calculation is unnecessary when m_PickerState.manualPathEditMode is
active, as the UI displays the raw path string in that mode. Add a check to skip the
cache update when manual edit mode is enabled to avoid needless GC churn and
calculations during editing.

Packages/com.unity.inputsystem/InputSystem/Editor/ControlPicker/InputControlPathEditor.cs [126-131]

 // Cache the display name per path value and only recompute when the string actually changes.
-        if (!string.Equals(path, m_CachedPath, StringComparison.Ordinal))
+        if (!m_PickerState.manualPathEditMode && !string.Equals(path, m_CachedPath, StringComparison.Ordinal))
         {
             m_CachedPath = path;
             m_CachedDisplayName = InputControlPath.ToHumanReadableString(path);
         }
Suggestion importance[1-10]: 6

__

Why: The suggestion properly optimizes the code by skipping an unnecessary string calculation when manualPathEditMode is active, as the display name is not used in that mode. This aligns with the PR's goal of reducing GC allocations, though the actual performance gain is likely minor since DelayedTextField limits the frequency of path updates.

Low
  • More suggestions

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

Previous suggestions

Suggestions up to commit 1b38eb1
CategorySuggestion                                                                                                                                    Impact
Possible issue
Restore missing displayName variable to fix compilation

The local variable displayName was removed, but it is implied that subsequent code
in the method (not visible in the diff) depends on it. This will likely cause a
compilation error. You should declare displayName and assign it the value from
m_CachedDisplayName to maintain compatibility with the rest of the method.

Packages/com.unity.inputsystem/InputSystem/Editor/ControlPicker/InputControlPathEditor.cs [126-131]

 // Cache the display name per path value and only recompute when the string actually changes.
 if (!string.Equals(path, m_CachedPath, StringComparison.Ordinal))
 {
     m_CachedPath = path;
     m_CachedDisplayName = InputControlPath.ToHumanReadableString(path);
 }
+var displayName = m_CachedDisplayName;
Suggestion importance[1-10]: 10

__

Why: The PR removes the local variable displayName which was previously defined and used. Since the subsequent code in the method (which is unchanged and not fully visible in the diff) likely relies on displayName, removing its definition will cause a compilation error.

High

@codecov-fd.fxlcf.dpdns.org
Copy link

codecov-fd.fxlcf.dpdns.org bot commented Feb 12, 2026 •
edited
Loading

Codecov Report

Attention: Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...tem/Editor/ControlPicker/InputControlPathEditor.cs 0.00% 6 Missing ⚠️ 
@@             Coverage Diff             @@
##           develop    #2344      +/-   ##
===========================================
- Coverage    77.90%   77.89%   -0.02%     
===========================================
  Files          476      476              
  Lines        97613    97616       +3     
===========================================
- Hits         76048    76040       -8     
- Misses       21565    21576      +11
Flag Coverage Δ
inputsystem_MacOS_2022.3 5.52% <0.00%> (-0.01%) ⬇️
inputsystem_MacOS_2022.3_project 75.39% <0.00%> (-0.01%) ⬇️
inputsystem_MacOS_6000.0 5.30% <0.00%> (-0.01%) ⬇️
inputsystem_MacOS_6000.0_project 77.29% <0.00%> (-0.01%) ⬇️
inputsystem_MacOS_6000.3 5.30% <0.00%> (-0.01%) ⬇️
inputsystem_MacOS_6000.3_project 77.28% <0.00%> (-0.01%) ⬇️
inputsystem_MacOS_6000.4 5.31% <0.00%> (-0.01%) ⬇️
inputsystem_MacOS_6000.4_project 77.30% <0.00%> (+<0.01%) ⬆️
inputsystem_MacOS_6000.5 5.31% <0.00%> (-0.01%) ⬇️
inputsystem_MacOS_6000.5_project 77.29% <0.00%> (-0.01%) ⬇️
inputsystem_MacOS_6000.6 5.31% <0.00%> (-0.01%) ⬇️
inputsystem_MacOS_6000.6_project 77.29% <0.00%> (-0.02%) ⬇️
inputsystem_Ubuntu_2022.3 5.53% <0.00%> (-0.01%) ⬇️
inputsystem_Ubuntu_2022.3_project 75.19% <0.00%> (-0.01%) ⬇️
inputsystem_Ubuntu_6000.0 5.31% <0.00%> (-0.01%) ⬇️
inputsystem_Ubuntu_6000.0_project 77.09% <0.00%> (-0.01%) ⬇️
inputsystem_Ubuntu_6000.3 5.31% <0.00%> (-0.01%) ⬇️
inputsystem_Ubuntu_6000.3_project 77.09% <0.00%> (-0.02%) ⬇️
inputsystem_Ubuntu_6000.4 5.32% <0.00%> (-0.01%) ⬇️
inputsystem_Ubuntu_6000.4_project 77.10% <0.00%> (-0.02%) ⬇️
inputsystem_Ubuntu_6000.5 5.32% <0.00%> (-0.01%) ⬇️
inputsystem_Ubuntu_6000.5_project 77.09% <0.00%> (-0.02%) ⬇️
inputsystem_Ubuntu_6000.6 5.32% <0.00%> (-0.01%) ⬇️
inputsystem_Ubuntu_6000.6_project 77.09% <0.00%> (-0.02%) ⬇️
inputsystem_Windows_2022.3 5.52% <0.00%> (-0.01%) ⬇️
inputsystem_Windows_2022.3_project 75.52% <0.00%> (-0.01%) ⬇️
inputsystem_Windows_6000.0 5.30% <0.00%> (-0.01%) ⬇️
inputsystem_Windows_6000.0_project 77.42% <0.00%> (-0.01%) ⬇️
inputsystem_Windows_6000.3 5.30% <0.00%> (-0.01%) ⬇️
inputsystem_Windows_6000.3_project 77.41% <0.00%> (-0.01%) ⬇️
inputsystem_Windows_6000.4 5.31% <0.00%> (-0.01%) ⬇️
inputsystem_Windows_6000.4_project 77.42% <0.00%> (-0.01%) ⬇️
inputsystem_Windows_6000.5 5.31% <0.00%> (-0.01%) ⬇️
inputsystem_Windows_6000.5_project 77.42% <0.00%> (-0.01%) ⬇️
inputsystem_Windows_6000.6 5.31% <0.00%> (-0.01%) ⬇️
inputsystem_Windows_6000.6_project 77.42% <0.00%> (-0.01%) ⬇️

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

Files with missing lines Coverage Δ
...tem/Editor/ControlPicker/InputControlPathEditor.cs 0.00% <0.00%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@josepmariapujol-unity josepmariapujol-unity marked this pull request as draft February 12, 2026 10:28
@josepmariapujol-unity josepmariapujol-unity marked this pull request as ready for review February 12, 2026 12:46
@u-pr
Copy link
Contributor

u-pr bot commented Feb 12, 2026

Persistent review updated to latest commit 8086d03

@Pauliusd01
Copy link
Collaborator

Pauliusd01 commented Feb 26, 2026

Will take a look today/tomorrow

Pauliusd01
Pauliusd01 reviewed Feb 26, 2026
View reviewed changes
@josepmariapujol-unity
Fix InputControlPath caching and UI alignment issues
f61b96f 
Fixed caching for InputControlPath display name and improved alignment of UI elements in the Input Action window.
@josepmariapujol-unity josepmariapujol-unity merged commit 55d080e into develop Feb 26, 2026
122 of 125 checks passed
@josepmariapujol-unity josepmariapujol-unity deleted the input/fixing-cache-path-input-control-path-editor-todo branch February 26, 2026 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@K-Tone K-Tone K-Tone approved these changes

@Pauliusd01 Pauliusd01 Pauliusd01 approved these changes

Assignees

@josepmariapujol-unity josepmariapujol-unity

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@josepmariapujol-unity @Pauliusd01 @K-Tone