Skip to content

Conversation

@bmalrat
Copy link
Collaborator

@bmalrat bmalrat commented Oct 1, 2024

Description

When reading the Display Index of touch event is add 2048 to the value.
https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1101
It uses the inherited pointer's ushort definition but for touchscreen it's a byte.

Changes made

Fixed the mapping of the Display Index field that doesn't match for touch event.

Testing

local testing on multiple screen

Risk

Please describe the potential risks of your changes 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.

After merge:

  • Create forward/backward port if needed. If you are blocked from creating a forward port now please add a task to ISX-1444.

@bmalrat bmalrat changed the title FIx: Fixed wrong Display Index value for touchscreen events (ISXB-1101) FIX: Fixed wrong Display Index value for touchscreen events (ISXB-1101) Oct 1, 2024
@Pauliusd01 Pauliusd01 self-requested a review October 2, 2024 06:55
[InputControl(name = "pressure", useStateFrom = "primaryTouch/pressure")]
[InputControl(name = "radius", useStateFrom = "primaryTouch/radius")]
[InputControl(name = "press", useStateFrom = "primaryTouch/phase", layout = "TouchPress", synthetic = true, usages = new string[0])]
[InputControl(name = "displayIndex", useStateFrom = "primaryTouch/displayIndex", format = "BYTE")] // added format to override the Pointer's USHT value
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a good fix. Thanks for adding the comment to say why format is added to make sure its not removed in future refactorings.

base.FinishSetup();

primaryTouch = GetChildControl<TouchControl>("primaryTouch");
displayIndex = primaryTouch.displayIndex;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this line removed ?

I'd suggest Hakan review this PR since he added this code:
#1623

Copy link
Collaborator Author

@bmalrat bmalrat Oct 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not needed any more since I added the other attributes which override display index and use the state from the primary touch.
Like other overridden values like pressure Wich are already mapped in the base Pointer class.

@lyndon-unity lyndon-unity requested a review from ekcoh October 2, 2024 10:05
@Pauliusd01 Pauliusd01 requested a review from stefanunity October 4, 2024 10:47
Copy link
Collaborator

@stefanunity stefanunity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-works with two displays (1 a touch) showing correct displayIndex
-touch works as per expectations from the user now

@Pauliusd01 Pauliusd01 removed their request for review October 8, 2024 07:46
Copy link
Collaborator

@ekcoh ekcoh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for fixing this

@bmalrat bmalrat merged commit 5f6e2f5 into develop Oct 10, 2024
77 checks passed
@bmalrat bmalrat deleted the isxb-1101-fix-for-invalid-touch-displayindex branch October 10, 2024 14:43
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.

4 participants