We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 3958 - Android, some issue with InputDevice checks
Summary: Android, some issue with InputDevice checks
Status: NEW
Alias: None
Product: SDL
Classification: Unclassified
Component: *don't know* (show other bugs)
Version: don't know
Hardware: All Android (All)
: P2 normal
Assignee: Ryan C. Gordon
QA Contact: Sam Lantinga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-11-13 20:41 UTC by Sylvain
Modified: 2019-01-17 12:43 UTC (History)
0 users

See Also:


Attachments
patch (13.59 KB, patch)
2017-11-13 20:41 UTC, Sylvain
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sylvain 2017-11-13 20:41:50 UTC
Created attachment 3086 [details]
patch

There are some strange bitwise in SDL2 android port.

In android java class InputDevice, there are constants named SOURCE_CLASS_something and SOURCE_something.
It describres input devices that can be connected like touchscreen, mouse, joystick and so on.
See https://developer.android.com/reference/android/view/InputDevice.html


A "SOURCE_something" can belong to a "SOURCE_CLASS_something". 
(it can be 0 class, like SOURCE_TOUCH_NAVIGATION or SOURCE_ROTARY_ENCODE who have no class, all others are 1. But we even could consider a source can belong to N class).
Anyway, per CLASS, they are potentially several SOURCE_something.

A CLASS is bitfield with only 1 bit set.
A SOURCE is a 1 bit or'ed with (0/1/N?) CLASS.

Then, you can have some value to match with. Those comes from event.getSource() and device.getSources(). 
Note the S of getSources(). It provides a value where sources are OR'ed ! So "sources" is a set of several source(s) and several classes.


Here's come the strangeness in SDLActivity and SDLControllerManager: 

1)
No bug, but no need to check that much to filter by CLASS:
  if ((sources & InputDevice.SOURCE_CLASS_JOYSTICK) == InputDevice.SOURCE_CLASS_JOYSTICK) {
Because it's 1 bit, it's enough to do:
  if ((sources & InputDevice.SOURCE_CLASS_JOYSTICK) != 0) {

2)
When it's written:
  if ((event.getSource() & InputDevice.SOURCE_KEYBOARD) != 0) {
Here you don't check that you have exactly a SOURCE_KEYBOARD as input. In fact, you check that you have a Source that belongs to the classe(s) of the Keyboard (which is CLASS_BUTTON).
For instance, SOURCE_GAMEPAD or SOURCE_DPAD, which also belong to SOURCE_CLASS_BUTTON, would also match the previous condition.

In fact, it's equivalent to:
  if ((event.getSource() & InputDevice.SOURCE_CLASS_BUTTON) != 0) {
If you really want to check that you have source_keyboard, you should write:
  if (event.getSource() == InputDevice.SOURCE_KEYBOARD) {

3)
Same for SOURCE_MOUSE:
  if ((event.getSource() & InputDevice.SOURCE_MOUSE) != 0) {
Is in fact:
  if ((event.getSource() & InputDevice.SOURCE_CLASS_POINTER) != 0) {

4)
Then, there is SDL java inputGetInputDeviceIds() which is called to register touch devices.
It used to be needed to detect joystick and was generic for this purpose. 
But now, sources is hardcoded in native code to "InputDevice.SOURCE_TOUCHSCREEN".

Moreover, when you do: 
  if ((device != null) && ((device.getSources() & sources) != 0)) {
you allow anything of the same class as SOURCE_TOUCHSCREEN (which is SOURCE_CLASS_POINTER).
So you allow : SOURCE_MOUSE, SOURCE_STYLUS.
For instance, an USB Mouse device get added as SDL_Touch device, which is wrong.
(But maybe a SOURCE_STYLUS should be added ?)


Here's a patch that correct previous things. I gave try to see that it was working. 
I modified the condition so that it's written the way it's currently running.

Exception for the inputGetInputDeviceIds(), where I only add the SOURCE_TOUCHSCREEN.
(anyway, any nativeOnTouch would add any touch device underneath)

I also made the inputGetInputDeviceIds() simplier (and it adds the device name).

At the beginning of the patch, there is also a debug function you may want to use, but not to commit.
Comment 1 Sylvain 2019-01-10 20:44:42 UTC
In https://hg.libsdl.org/SDL/rev/24bbe1496ef7

Part 4 "add name for Touch devices and simplification" is added,

But not 

> Moreover, when you do: 
>   if ((device != null) && ((device.getSources() & sources) != 0)) {
> you allow anything of the same class as SOURCE_TOUCHSCREEN (which is > SOURCE_CLASS_POINTER).
> So you allow : SOURCE_MOUSE, SOURCE_STYLUS.
> For instance, an USB Mouse device get added as SDL_Touch device, which is wrong.
> (But maybe a SOURCE_STYLUS should be added ?)
Comment 2 Sylvain 2019-01-17 12:43:56 UTC
The part 1 is in:
https://hg.libsdl.org/SDL/rev/8ca43abe9870