[PATCH]Android port doesn't work well with multi-touch input

[xyz]

It doesn't respect AMOTION_EVENT_ACTION_MASK and therefore EventAction could be set to a wrong value when there are multiple pointers. It also doesn't respect AMOTION_EVENT_ACTION_POINTER_DOWN and AMOTION_EVENT_ACTION_POINTER_UP which means that there're no events fired when a second pointer appears.

I think the MultiTouchInput should also contain the ID field (which can be retrieved using AMotionEvent_getPointerId). This also should be used to implement PrevX and PrevY which are marked with TODO.

Here's a helpful header file: http://mobilepearls.com/labs/native-and ... id/input.h

And docs: http://developer.android.com/reference/ ... Event.html

[xyz]

UPD: fixed a bug with previous position being wrongly initialized
UPD2: added number of pointers to the event

Patch suggestion:

Code: Select all

 
Index: CIrrDeviceAndroid.cpp
===================================================================
--- CIrrDeviceAndroid.cpp   (revision 4618)
+++ CIrrDeviceAndroid.cpp   (working copy)
@@ -37,6 +37,7 @@
 #ifdef _DEBUG
    setDebugName("CIrrDeviceAndroid");
 #endif
+   previousMotionData = new core::map<s32, irr::core::vector2d<s32> >;
 
    // Get the interface to the native Android activity.
    Android = (android_app*)(param.PrivateData);
@@ -260,7 +261,9 @@
    {
        SEvent Event;
        s32 PointerCount = AMotionEvent_getPointerCount(androidEvent);
-       s32 EventAction = AMotionEvent_getAction(androidEvent);
+       s32 AndroidEventAction = AMotionEvent_getAction(androidEvent);
+       s32 EventAction =  AndroidEventAction & AMOTION_EVENT_ACTION_MASK;
+       s32 ChangedPointerID = (AndroidEventAction & AMOTION_EVENT_ACTION_POINTER_INDEX_MASK) >> AMOTION_EVENT_ACTION_POINTER_INDEX_SHIFT;
 
        bool MultiTouchEvent = true;
        bool Touched = false;
@@ -268,6 +271,7 @@
        switch (EventAction)
        {
        case AMOTION_EVENT_ACTION_DOWN:
+       case AMOTION_EVENT_ACTION_POINTER_DOWN:
            Event.MultiTouchInput.Event = EMTIE_PRESSED_DOWN;
            Touched = true;
            break;
@@ -276,6 +280,7 @@
            Touched = true;
            break;
        case AMOTION_EVENT_ACTION_UP:
+       case AMOTION_EVENT_ACTION_POINTER_UP:
            Event.MultiTouchInput.Event = EMTIE_LEFT_UP;
            break;
        default:
@@ -287,25 +292,294 @@
        {
            Event.EventType = EET_MULTI_TOUCH_EVENT;
            Event.MultiTouchInput.clear();
+           Event.MultiTouchInput.PointerCount = PointerCount;
 
+           core::map<s32, irr::core::vector2d<s32> > *newMotionData = new core::map<s32, irr::core::vector2d<s32> >;
+
            for (s32 i = 0; i < PointerCount; ++i)
            {
                if (i >= NUMBER_OF_MULTI_TOUCHES)
                    break;
 
-               Event.MultiTouchInput.PrevX[i] = 0; // TODO
-               Event.MultiTouchInput.PrevY[i] = 0; // TODO
-               Event.MultiTouchInput.X[i] = AMotionEvent_getX(androidEvent, i);
-               Event.MultiTouchInput.Y[i] = AMotionEvent_getY(androidEvent, i);
-               Event.MultiTouchInput.Touched[i] = Touched;
+               s32 x = AMotionEvent_getX(androidEvent, i);
+               s32 y = AMotionEvent_getY(androidEvent, i);
+               Event.MultiTouchInput.X[i] = x;
+               Event.MultiTouchInput.Y[i] = y;
+
+               s32 id = AMotionEvent_getPointerId(androidEvent, i);
+               Event.MultiTouchInput.ID[i] = id;
+
+               core::map<s32, irr::core::vector2d<s32> >::Node *previousMotion;
+               if ((previousMotion = Device->previousMotionData->find(id))) {
+                   Event.MultiTouchInput.PrevX[i] = previousMotion->getValue().X;
+                   Event.MultiTouchInput.PrevY[i] = previousMotion->getValue().Y;
+               } else {
+                   Event.MultiTouchInput.PrevX[i] = x;
+                   Event.MultiTouchInput.PrevY[i] = y;
+               }
+
+               if ((Event.MultiTouchInput.Touched[i] = Touched || (ChangedPointerID != id)))
+                   (*newMotionData)[id] = core::vector2d<s32>(x, y);
            }
+           delete Device->previousMotionData;
+           Device->previousMotionData = newMotionData;
 
            Device->postEventFromUser(Event);
 

Code: Select all

 
Index: IEventReceiver.h
===================================================================
--- IEventReceiver.h    (revision 4618)
+++ IEventReceiver.h    (working copy)
@@ -382,6 +382,7 @@
         //! Reset variables.
        void clear()
        {
+           PointerCount = 0;
            for (u16 i = 0; i < NUMBER_OF_MULTI_TOUCHES; ++i)
             {
                Touched[i] = 0;
@@ -389,9 +390,12 @@
                Y[i] = 0;
                PrevX[i] = 0;
                PrevY[i] = 0;
+               ID[i] = 0;
            }
        }
-        
+       // Number of pointers
+        u8 PointerCount;
+
         // Status of simple touch.
         u8 Touched[NUMBER_OF_MULTI_TOUCHES];
         
@@ -406,6 +410,8 @@
         
         // Previous Y position of simple touch.
        s32 PrevY[NUMBER_OF_MULTI_TOUCHES];
+
+       s32 ID[NUMBER_OF_MULTI_TOUCHES];
         
        //! Type of multi touch event
        EMULTI_TOUCH_INPUT_EVENT Event;
 

This makes multitouch events work with multiple pointers down/up events, fixes Touched, adds ID field and implements PrevX and PrevY.

[Nadro]

Thanks for it, I'll check this patch

[Lysenko]

Good work, thanks.
I have a question: why do we need Touched flag?
I saw it used in touchedCount(), but here you store number of pointers in PointerCount.
Is Touched used for something else?

[Lysenko]

One thing.

s32 ChangedPointerID = (AndroidEventAction & AMOTION_EVENT_ACTION_POINTER_INDEX_MASK) >> AMOTION_EVENT_ACTION_POINTER_INDEX_SHIFT;

ChangedPointerID is not a pointer ID, it is a pointer index.
Then you compare it with a pointer id, which is wrong, isn't it?

[zerochen]

hi,

doesnt look to deep into your code but...

Code: Select all

 #ifdef _DEBUG
    setDebugName("CIrrDeviceAndroid");
 #endif
+   previousMotionData = new core::map<s32, irr::core::vector2d<s32> >;

i think previousMotionData is not declared in this scope? and why it is on the heap?
also isnt it better to change the code so that core::map<s32, irr::core::vector2d<s32> can be an array?
isnt here a = missing?

Code: Select all

 if ((Event.MultiTouchInput.Touched[i] =[b]=[/b]Touched || (ChangedPointerID != id)))

regards
zerochen

[xyz]

Good work, thanks.
I have a question: why do we need Touched flag?
I saw it used in touchedCount(), but here you store number of pointers in PointerCount.
Is Touched used for something else?

You can use it to determine which pointer goes up.

One thing.

s32 ChangedPointerID = (AndroidEventAction & AMOTION_EVENT_ACTION_POINTER_INDEX_MASK) >> AMOTION_EVENT_ACTION_POINTER_INDEX_SHIFT;

ChangedPointerID is not a pointer ID, it is a pointer index.
Then you compare it with a pointer id, which is wrong, isn't it?

This is true, thanks for noticing! I'll test the fix and update the first post.

hi,

doesnt look to deep into your code but...

Code: Select all

 #ifdef _DEBUG
    setDebugName("CIrrDeviceAndroid");
 #endif
+   previousMotionData = new core::map<s32, irr::core::vector2d<s32> >;

i think previousMotionData is not declared in this scope? and why it is on the heap?

Yeah, you're right. I forgot to attach the header file diff.

also isnt it better to change the code so that core::map<s32, irr::core::vector2d<s32> can be an array?

Android docs don't specify the range of values getPointerId can return.

isnt here a = missing?

Code: Select all

 if ((Event.MultiTouchInput.Touched[i] =[b]=[/b]Touched || (ChangedPointerID != id)))

No, it assigns to Touched and then returns Touched so that the condition works. I don't know why did I write it this way (perhaps it was a bad idea to code at night) but I'll change it.

[xyz]

Sorry for not replying for so long. Anyway, maintaining my .patch file has become a bit tedious so fixes are available from the following github project.