Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Map mouse input to new / moved touches instead of only first #3688

Merged
merged 13 commits into from Jul 8, 2020

Conversation

frenzibyte
Copy link
Member

@frenzibyte frenzibyte commented Jul 5, 2020

As discussed and considered, mapping mouse input to any new / moved touches instead of first touch is more better as a good starting point.

Huge note though is that now with this new logic, there will be "false drags" raised when still holding a touch and pressing a new one (mouse moves from one to other while left still pressed, thus a false drag raised), discussed about that with the result of opening an issue post-merge as it doesn't seem easy to resolve.

@peppy
Copy link
Sponsor Member

peppy commented Jul 5, 2020

secondary touches should not trigger clicks, so that sounds correct?

@frenzibyte
Copy link
Member Author

frenzibyte commented Jul 5, 2020

secondary touches should not trigger clicks, so that sounds correct?

Is there a reason for them not to do so? I find it kinda incorrect they don't trigger clicks, holding a touch and then tapping with another touch.

To expand a bit on this, multi-touch events are mostly meant to be handled for when you want to receive more than one touch and start performing certain actions on them (pinch, rotate, etc...).


Either way I actually forgot to mention that also with the false-drag issue, secondary touches won't even raise OnMouseDown(...) to the drawable they're touching, so this change seems to be useless without the issue fixed...

@bdach
Copy link
Collaborator

bdach commented Jul 5, 2020

I don't think it is weird that secondary touches don't click. Just from a super quick test on Android, if you start a first touch and then hold it, the second touch doesn't cause clicks.

Overall it seems fine to me that the first touch "takes ownership of being the mouse". It's just the false drags upon touch transfer that are undesirable.

@frenzibyte
Copy link
Member Author

We already had that discussion, if we're testing to that, then in iOS second touches do cause clicks (hold one finger in empty area and tap an app in another finger).

And from what I'm seeing, currently it seems that checking against Touch1 is actually sometimes not even correct, as it's not really the first touch, new touches would claim that source if it's not attached to any, and fixing that by mapping mouse to only the first touch of the current gesture wouldn't really make sense to have, so latest began still sounds best to me.

@bdach
Copy link
Collaborator

bdach commented Jul 5, 2020

I remember the order discussion but it appears I misunderstood what you meant by "clicking not working at all with secondary touches".

@peppy
Copy link
Sponsor Member

peppy commented Jul 5, 2020

ios does not trigger "clicks" / primary action on subsequent touches. it causes positional updates. not clicks. it should not cause clicks.

@frenzibyte
Copy link
Member Author

OK but still as I said:

Either way I actually forgot to mention that also with the false-drag issue, secondary touches won't even raise OnMouseDown(...) to the drawable they're touching, so this change seems to be useless without the issue fixed...

If OnMouseDown shouldn't be triggered to drawables in secondary touches as well, then I really don't see a point with this change.

@bdach
Copy link
Collaborator

bdach commented Jul 5, 2020

Why would it though? I'm not sure where the problem is. Secondary inputs should be secondary inputs and not perform primary actions, which in our case is equivalent to being translated to a mouse event (if the drawable doesn't do manual touch handling).

I thought the goal of this change was to go from the android-y way of "primary touch is first" to the iOS-y way of "primary touch is last".

I really fail to see what the issue is. I think if you gave a real example of why you think secondary touches should fire mouse events, we could come to an understanding.

@frenzibyte
Copy link
Member Author

frenzibyte commented Jul 6, 2020

I really fail to see what the issue is.

Yeah figured that would be the case.

The current mouse-from-touch behaviour in master and its inappropriacy

Currently in master, any touch input from Touch1 source (aka "primary source") would produce mouse input for compatibility with drawables that do not handle multi-touching events.

This behaviour does not seem good enough as a starting point from discussion, for the case of holding 2 touches, then releasing the first ("primary") one and start moving around with the secondary one, mouse input would not be produced for the currently single secondary touch, that's the reason this may not be good enough.

The purpose of this PR and the new mouse-from-touch behaviour

This PR is meant to improve the inappropriacy in the above behaviour by marking the latest began touch as the "primary" instead of the first, so the behaviour this PR as aiming to have is that each new touch would transfer mouse input control from previous one to it, (e.g. holding a drawable with one touch then holding another drawable with another touch would transfer mouse control by raising an OnMouseUp to the previous drawable, and raising an OnMouseDown to the current drawable.)

The above behaviour sounds better to me as drawables are meant to handle multi-touch events (TouchEvent) when:

  • They want to handler certain gestures that requires more than one touch (pinch, rotate, etc...), which a mouse isn't capable of doing.
  • Or in the case of rulesets, receive one touch and listen from it without interruption from another touch (which is what relying on a mouse would cause).

The false-dragging issue in this PR (transferring mouse input control)

The only objection from achieving the mentioned behaviour is that "transferring the mouse input control" would require introducing the "input-cancellation" concept to button states, such that mouse clicks don't get raised to old touch-held drawables, so in the 2 drawables example above, the previous drawable would receive an OnMouseUp but should never receive an OnClick, I can work through implementing this but have been told to keep that issue as-is for now, and open an issue thread about it.

So with that issue staying as-is, this change doesn't seem to be helpful in anything, if the behaviour above sounds more correct, then I would take the effort to introduce input cancellation and fix this issue to have the proposed behaviour working as it is meant to. (shouldn't really be hard)


Hope that clears up everything!

@peppy
Copy link
Sponsor Member

peppy commented Jul 6, 2020

Reading the essay above changes nothing.

A second touch should not trigger a mousedown, if the "mouse" is already down.

The point of this change is to make the second touch continue positional tracking, rather than being completely ignored.

@frenzibyte
Copy link
Member Author

I fail to see a reason why tapping with a second touch wouldn't raise mouse downs to the drawable they're tapping, but continue positional tracking, and where would that behaviour be useful for?

@peppy
Copy link
Sponsor Member

peppy commented Jul 6, 2020

The goal is to match existing behaviours. Please try using a mobile device and notice that on no device does tapping a second finger (in a scenario where multitouch is not specifically supported) ever trigger a "mouse down" event. This is not only what users are used to, it makes logical sense.

This is how we want it implemented, without question.

@frenzibyte
Copy link
Member Author

OK yeah the only problem I was having here is where to actually test this, but if that's how you want it then sure wouldn't mind that.

So if secondary touches are not meant to raise mouse down events to what they're tapping, but we still want them to continue positional tracking, would it be fine to just release mouse left when a secondary touch has began to fix the false-dragging issue and call it a day?

@peppy
Copy link
Sponsor Member

peppy commented Jul 6, 2020

You can test at a home screen, settings, anywhere.

It should not be released, no. Leave the delta in place for now and it can be re-addressed in a future PR if deemed necessary.

@frenzibyte
Copy link
Member Author

@peppy I'm actually quite concerned about how the behaviour should go, when holding a touch on a drawable and drag a bit, then pressing another touch, would the dragged drawable receive an OnDragEnd, and would it also receive an OnMouseUp? otherwise when would those events be raised?

@peppy
Copy link
Sponsor Member

peppy commented Jul 7, 2020

as long as a finger is down, no up or end events.

@frenzibyte
Copy link
Member Author

frenzibyte commented Jul 7, 2020

Because that would mean nullifying the delta is kinda impossible / not making sense with the events raised as-is, but I'm just gonna remove the commented assertions for now.

@peppy
Copy link
Sponsor Member

peppy commented Jul 7, 2020

See comment before. Leaving delta is the proposed path forward for this PR.

@peppy
Copy link
Sponsor Member

peppy commented Jul 7, 2020

What you have right now isn't too bad, but let's go with the following (iOS standard) for now:

  • Put two fingers on screen
  • Move second, cursor should follow it
  • Move first, cursor should follow it <- currently doesn't work

@frenzibyte
Copy link
Member Author

frenzibyte commented Jul 7, 2020

So basically let mouse follow any touch position change, would it be fine for mouse to also move to the position of any released touch? talking for simplicity matter. (as the user would most likely nudge before releasing)

@peppy
Copy link
Sponsor Member

peppy commented Jul 7, 2020

No, only move (and release of last touch). If anything, a release should make the position update to any remaining touch, but doing nothing until movement is also fine.

@frenzibyte frenzibyte changed the title Map mouse input to the latest began touch instead of first Map mouse input to new / moved touches instead of only first Jul 7, 2020
@peppy
Copy link
Sponsor Member

peppy commented Jul 8, 2020

Behaviour looks correct now 👍

@peppy peppy merged commit db02f6a into ppy:master Jul 8, 2020
@frenzibyte frenzibyte deleted the map-mouse-input-to-latest-touch branch July 8, 2020 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants