Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

FIX: [droid] fix multitouch #2482

Open
wants to merge 1 commit into from

5 participants

@koying
Collaborator

If I'm not mistaken, there was a confusion between the pointer index of the event and the absolute pointer id.

@theuni theuni was assigned
@Montellese Montellese was assigned
@Montellese
Owner

Oh I didn't know about AMotionEvent_getPointerId(). With that it might even be possible to get rid of the whole UpdateTouchPointer() stuff because I only added that because I couldn't figure out how to determine to which pointer an event belonged. The pointer index was always 0 for touch move events. But this change might also mess up some of the gesture recognition logic. I'll have to take a closer look.

Did you find some case where multitouch currently doesn't work (because you label this PR with "FIX")?

@koying
Collaborator

Don't know if this fixes a functionality. I was trying to get "2-fingers-left-swipe=back" working and saw that one before noticing generic multi-touch swipe wasn't supported ;)

Obvious Technical fix, as we are mixing apple & pears between the pointer indexes in the event and the pointer id.

I agree the gesture recognition would probably need rework. IMO, it is flawed because it works on pointer indexes only valid within a specific event.
It is actually working on Android (only platform it is actually used, right?) because the pointer indexes happen to be the same as pointer id in most cases...

@koying
Collaborator

Thinking about it, we should probably just have an array of pointer id's and passing indexes to this table further down.
That would prevent gesture recognition refactoring. I bit silly for Android as indexes will be = id's but that would be generic.
What do you think?

@Montellese
Owner

I've played around with multiple touches a bit and looked at the indexes and IDs of the touch events provided by android and I'm not sure what the best solution is. With your changes (i.e. passing the pointer ID instead of the pointer index into CGenericTouchInputHandler) there will be problems when the user e.g. first touches down pointer 0 (ID = 0), then pointer 1 (ID = 1) but then first lifts pointer 0 (ID = 0, but now the index of pointer 1 becomes 0 whereas the ID remains 1) and then pointer 1 (ID = 1 but index = 0) because CGenericTouchInputHandler already does that switching internally i.e. if the primary pointer is lifted, it replaces it with the next available pointer.

The general questions are

  • where should this be handled? In the OS-specific raw event handling or in CGenericTouchInputHandler?
  • should we work with indexes or with IDs? If we want to work with IDs we probably need to move from an array to a map because we can not assume and be certain that the IDs are in a specific order.

Another problem on android is that touch move events are always sent for the pointer with index 0, we don't receive events for any of the other pointers (even if we don't move the pointer with index 0) so we can't get rid of that loop calling CGenericTouchInputHandler::UpdateTouchPointer() which I kind of dislike.

@koying
Collaborator

Re ID vs. index, not sure we actually have a choice. The index we receive in the events are really just indexes in that specific event. I don't think there is a proper way to make those global other than making wild assumptions...

I'll dig further re Move event. There must be a way to get the postion of individual pointers...

@Montellese
Owner

Well they don't have to be global, we only really care about events from the first touch down to the last touch up. After that, we don't care about anything until the next (first) touch down. In between we neither care about index nor ID.

@koying
Collaborator

Re
Another problem on android is that touch move events are always sent for the pointer with index 0,
this is, I think, a non issue.
If I understand the MotionEvent doc correctly:
Motion events contain information about all of the pointers that are currently active even if some of them have not moved since the last event was delivered.,
you always have the latest position of all pointers in all motion events via AMotionEvent_getX/getY

@Montellese
Owner

Yeah it's not really a problem, it's just ugly IMO that we get the event with an index that may not actually have moved. That's why I added the UpdateTouchPointer() method so that we can update all touch pointers in a touch move event.

@Memphiz
Owner

is this still valid? @Montellese ?

@Montellese
Owner

The best solution would be to re-write the whole android-specific touch event handling because when I wrote it I didn't realize the meaning of ID vs. index.

@da-anda
Collaborator

is anybody going to rewrite the android touch implementation in the near future or should we better merge this PR for the time being?

@Montellese
Owner

I'm planning to adjust this when I get an android tablet. It's just too cumbersome on my phone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 22, 2013
  1. @koying

    FIX: [droid] fix multitouch

    koying authored
This page is out of date. Refresh to see the latest.
Showing with 6 additions and 5 deletions.
  1. +6 −5 xbmc/android/activity/AndroidTouch.cpp
View
11 xbmc/android/activity/AndroidTouch.cpp
@@ -50,7 +50,8 @@ bool CAndroidTouch::onTouchEvent(AInputEvent* event)
int32_t eventAction = AMotionEvent_getAction(event);
int8_t touchAction = eventAction & AMOTION_EVENT_ACTION_MASK;
- size_t touchPointer = eventAction >> AMOTION_EVENT_ACTION_POINTER_INDEX_SHIFT;
+ size_t touchPointerIdx = eventAction >> AMOTION_EVENT_ACTION_POINTER_INDEX_SHIFT;
+ int32_t touchPointerID = AMotionEvent_getPointerId(event, touchPointerIdx);
TouchInput touchEvent = TouchInputAbort;
switch (touchAction)
@@ -75,18 +76,18 @@ bool CAndroidTouch::onTouchEvent(AInputEvent* event)
break;
}
- float x = AMotionEvent_getX(event, touchPointer);
- float y = AMotionEvent_getY(event, touchPointer);
+ float x = AMotionEvent_getX(event, touchPointerIdx);
+ float y = AMotionEvent_getY(event, touchPointerIdx);
float size = m_dpi / 16.0f;
int64_t time = AMotionEvent_getEventTime(event);
// first update all touch pointers
for (unsigned int pointer = 0; pointer < numPointers; pointer++)
- CGenericTouchInputHandler::Get().UpdateTouchPointer(pointer, AMotionEvent_getX(event, pointer), AMotionEvent_getY(event, pointer),
+ CGenericTouchInputHandler::Get().UpdateTouchPointer(AMotionEvent_getPointerId(event, pointer), AMotionEvent_getX(event, pointer), AMotionEvent_getY(event, pointer),
AMotionEvent_getEventTime(event), m_dpi / 16.0f);
// now send the event
- return CGenericTouchInputHandler::Get().HandleTouchInput(touchEvent, x, y, time, touchPointer, size);
+ return CGenericTouchInputHandler::Get().HandleTouchInput(touchEvent, x, y, time, touchPointerID, size);
}
void CAndroidTouch::setDPI(uint32_t dpi)
Something went wrong with that request. Please try again.