Improve Android mouse input and restructure Android input handlers #4971

Susko3 · 2021-12-27T15:55:23Z

This brings us one step closer to resolving #4373.

In addition to separating mouse from touch events, AndroidMouseHandler enables the following:

support for all mouse buttons (including forward and back)
support for scrolling
starting point for implementing pointer capture.

Tested on Realme 6, Android 11 (API 30) with a bluetooth mouse.
On-screen keyboard input works as expected.

Test works with:

touchpad (not required, but would be nice to test)

Test existing behaviour isn't broken for:

physical keyboard
stylus

`ConfigChanges.Navigation` is needed so the game doesn't crash when connecting additional devices (eg. a bluetooth mouse). `ConfigChanges.Touchscreen` added just in case.

…od overrides This makes it easier to get the desired behaviour for the upcoming AndroidMouseHandler. Also add some comments to the events and moves them to a separate region.

In addition to separating mouse from touch events, AndroidMouseHandler enables the following: - Support for all mouse buttons (including forward and back). - Support for scrolling. Additional changes include: - Changes AndroidGameView to invoke events based on the input source. - Adds new events specifically for the mouse and touchpad. - Exposes framework internals to Android project so that FrameStatistics for MouseEvents can be incremented.

bdach

seemingly correct but i have no actual means to test

osu.Framework.Android/AndroidGameView.cs

osu.Framework.Android/Input/AndroidMouseHandler.cs

The events were getting out of hand, and future additions (eg. controller input) would increase the complexity even more. So checking the event source was moved to InputHandler-side and standardized with AndroidInputHandler.

osu.Framework.Android/Input/AndroidKeyboardHandler.cs

osu.Framework.Android/Input/AndroidInputHandler.cs

AndroidInputHandler will now strictly enforce the event types and sources specified. Input handlers now override the desired methods to handle those events.

Susko3 · 2021-12-28T13:22:32Z

osu.Framework.Android/Input/AndroidTouchHandler.cs

-        public override bool Initialize(GameHost host) => true;
-
-        private void handleTouch(object sender, View.TouchEventArgs e)
+        protected override void OnTouch(MotionEvent touchEvent)
        {


Using MotionEvent because e.Event is nullable and I didn't want to have a null check.

…ant method was not overriden

bdach · 2021-12-29T09:27:28Z

osu.Framework.Android/Input/AndroidInputHandler.cs

+        /// The <see cref="InputEventType"/>s that this <see cref="AndroidInputHandler"/> will handle.
+        /// </summary>
+        /// <remarks>For each <see cref="InputEventType"/> specified here, this <see cref="AndroidInputHandler"/> should override the appropriate method.</remarks>
+        protected abstract IEnumerable<InputEventType> HandledEventTypes { get; }


Not super sure on this setup. Rather than having to declare event types the handler is interested in and then having to override the correct methods (lest you get NotSupportedException at runtime), you could just subscribe to all events but leave the handlers empty. That'd get rid of some of the boilerplate.

I don't hate this setup though, will wait for more opinions.

I can agree with this. Seems like too much boilerplate for an edge case usage as it's currently written.

peppy

PR in general seems pretty fine, but would like to see it with manual event binding at each usage as proposed.

As requested in code review. `Handle*` methods are now subscribed to in each handler separately. Too much boilerplate with everything in AndroidInputHandler.

peppy · 2022-01-04T03:04:30Z

Code looks good now. I'll try and give it a test today against touch and stylus.

peppy · 2022-01-04T10:45:16Z

Stylus and touch work as expected here 👍

bdach · 2022-01-04T20:37:42Z

osu.Framework.Android/Input/AndroidInputHandler.cs

+        /// Subscribe <see cref="HandleCapturedPointer"/> to <see cref="View"/>.<see cref="AndroidGameView.CapturedPointer"/> to receive events here.
+        /// </remarks>
+        protected virtual void OnCapturedPointer(MotionEvent capturedPointerEvent)
+        {
+            throw new NotSupportedException($"{nameof(HandleCapturedPointer)} subscribed to {nameof(View.CapturedPointer)} but the relevant method was not overriden.");


This isn't quite what I had in mind with my last review but I suppose it works...?

To clarify, I was proposing subscribing to all input events which are available on the view at the AndroidInputHandler level, and calling the OnEvent methods on them there - as long as they match eventSourceBitmask - but leaving the input handlers blank to be overridden by inheritors. That way the only thing that needs to be done to handle an event in an implementation of this is overriding the correct handler, while this solution also requires manually hooking the events up in OnInitialize().

I tried doing it how peppy suggested, and figured it was less wasteful than your proposal. So I went with that.

Not sure any overhead from these events is ever going to matter but I don't feel like splitting hairs over this.

osu.Framework.Android/Input/AndroidKeyboardHandler.cs

bdach

code seems fine, haven't tested, relying on @peppy's testing as i could only test the same part of this PR at most anyway (don't have required hardware to test mouse)

Susko3 added 3 commits December 27, 2021 15:33

Update config changes to allow connecting additional devices

f050c2a

`ConfigChanges.Navigation` is needed so the game doesn't crash when connecting additional devices (eg. a bluetooth mouse). `ConfigChanges.Touchscreen` added just in case.

Change AndroidGameView Hover and Touch events to be based on meth…

4dd39c0

…od overrides This makes it easier to get the desired behaviour for the upcoming AndroidMouseHandler. Also add some comments to the events and moves them to a separate region.

pull-request-size bot added the size/L label Dec 27, 2021

bdach added area:input platform:android labels Dec 27, 2021

bdach reviewed Dec 27, 2021

View reviewed changes

osu.Framework.Android/AndroidGameView.cs Outdated Show resolved Hide resolved

osu.Framework.Android/Input/AndroidMouseHandler.cs Outdated Show resolved Hide resolved

Add AndroidInputHandler and simplify AndroidGameView events

ce83b61

The events were getting out of hand, and future additions (eg. controller input) would increase the complexity even more. So checking the event source was moved to InputHandler-side and standardized with AndroidInputHandler.

bdach reviewed Dec 28, 2021

View reviewed changes

osu.Framework.Android/Input/AndroidKeyboardHandler.cs Outdated Show resolved Hide resolved

osu.Framework.Android/Input/AndroidInputHandler.cs Outdated Show resolved Hide resolved

Susko3 added 2 commits December 28, 2021 14:15

Move subscribe and source check logic for events to AndroidInputHandler

40c3e35

AndroidInputHandler will now strictly enforce the event types and sources specified. Input handlers now override the desired methods to handle those events.

Move resharper explainer comment to new line

a30ccee

pull-request-size bot added size/XL and removed size/L labels Dec 28, 2021

Susko3 changed the title ~~Add AndroidMouseHandler and improve Android mouse input~~ Improve Android mouse input and restructure Android input handlers Dec 28, 2021

Susko3 commented Dec 28, 2021

View reviewed changes

Throw if event type is specified in HandledEventTypes but the relev…

3b0e79a

…ant method was not overriden

bdach reviewed Dec 29, 2021

View reviewed changes

peppy requested changes Jan 3, 2022

View reviewed changes

Susko3 added 2 commits January 3, 2022 22:33

Subscribe to events at each usage

8a3a6cd

As requested in code review. `Handle*` methods are now subscribed to in each handler separately. Too much boilerplate with everything in AndroidInputHandler.

Rename inputSource -> eventSource to match HandledEventSources

addbcb1

bdach reviewed Jan 4, 2022

View reviewed changes

bdach approved these changes Jan 4, 2022

View reviewed changes

Merge branch 'master' into android-mouse

3faefab

peppy approved these changes Jan 5, 2022

View reviewed changes

peppy merged commit e4f3389 into ppy:master Jan 5, 2022

Susko3 deleted the android-mouse branch January 5, 2022 06:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve Android mouse input and restructure Android input handlers #4971

Improve Android mouse input and restructure Android input handlers #4971

Susko3 commented Dec 27, 2021 •

edited by peppy

Loading

bdach left a comment

Susko3 Dec 28, 2021

bdach Dec 29, 2021

peppy Jan 3, 2022

peppy left a comment

peppy commented Jan 4, 2022

peppy commented Jan 4, 2022

bdach Jan 4, 2022

Susko3 Jan 4, 2022

bdach Jan 4, 2022

bdach left a comment •

edited

Loading

Improve Android mouse input and restructure Android input handlers #4971

Improve Android mouse input and restructure Android input handlers #4971

Conversation

Susko3 commented Dec 27, 2021 • edited by peppy Loading

bdach left a comment

Choose a reason for hiding this comment

Susko3 Dec 28, 2021

Choose a reason for hiding this comment

bdach Dec 29, 2021

Choose a reason for hiding this comment

peppy Jan 3, 2022

Choose a reason for hiding this comment

peppy left a comment

Choose a reason for hiding this comment

peppy commented Jan 4, 2022

peppy commented Jan 4, 2022

bdach Jan 4, 2022

Choose a reason for hiding this comment

Susko3 Jan 4, 2022

Choose a reason for hiding this comment

bdach Jan 4, 2022

Choose a reason for hiding this comment

bdach left a comment • edited Loading

Choose a reason for hiding this comment

Susko3 commented Dec 27, 2021 •

edited by peppy

Loading

bdach left a comment •

edited

Loading