Fixes duplicate touch IDs building up in touches began/moved/ended events #2
Conversation
|
Hey, thanks for sending this through. I'm trying to understand the original issue before I commit. Can you describe what you mean by duplicate touch IDs building up? The code pushes received touch Android events into the three touch lists (began, moved, ended). Are you getting multiple copies of a touch event, or separate touch events sharing the same touch ID? Looking over the patch, I can't see where the new touch handlers (engine_update_*) are called from. |
|
I got rid of engine_update_touches(), which was getting called in the main app loop, and moved all of it's functionality into separate update functions that get called in the android input event handler. The reason for this is that doing the sweep for touch updates as before opened the possibility for each of the temporary touch lists to build up more than one of the same touch ID. The android event might fire 6 times in between sweeps, leading to 6 entries of the same touch ID. The moved list was the biggest culprit. I'm really just emulating the way CinderView.mm works, where it updates cinder touches at the same time as the iOS event. |
|
The patch seems to be missing the part where the new functions are attached to the touch events. When I tested the multitouch sample no touches were registered. At the moment, the event stream seen by the app should be identical to the event stream Android sends out. Won't the duplicates still turn up if you send the events directly to the handler ? From reading the native app glue code it looks like input is still handled on a separate message loop thread to other events. The reason inputs are saved and then dispatched from the message loop is to synchronize input events with the rendering (native activity) thread. See the discussion here for some background: https://groups.google.com/forum/?fromgroups=#!topic/android-developers/Oe6k1_bm38o |
|
!!!! I'm so sorry! I was working off of your regular android branch and copied that over. I forgot to update engine_handle_input() to include the new update calls (engine_update_touches_began, etc..). I will submit a new pull request. If you want to test for duplicate touch IDs right now, log the touchState->touchesMoved vector in engine_update_touches(). You'll see that the same ID might show up twice (or six times!) in some calls. The MultiTouchBasic example doesn't show any visual feed back on this, but my multitouch UI was showing lingering touch IDs. Regarding performance, I haven't noticed a lag. I assume we're fine since cinder is calling update and draw separately. The question is: if android fires six touchesMoved events in between update calls, shouldn't my cinder app know about that? Otherwise, we're left with the question of averaging the in-between touch events, or lopping off everything except the last received event. Each of those last two methods throws away information. |
|
Ah, I think there's some confusion about how touch events are handled. Touch events are never dropped. You might see multiple events with the same touch ID in an onTouch callback, but they are separate events with a unique timestamp. e.g. I/cinder (19368): XXX START LOGGING MOVED The events sent to onTouch are all the touch events that occurred since the last onTouch/update/draw cycle. If your framerate is lower than the input rate you'll get more events per frame. In the multitouch sample it was pretty hard to get multiple events because the framerate is high, might explain what you were seeing. The order that onTouch and update/draw are called is deterministic - the app receives all the touch information needed to update state in a single call. I find this easier to handle than receiving and filtering multiple touch events between update/draw calls. The app class does some basic filtering via getActiveTouches(). If you're doing your own gesture recognition you might want to implement your own touch filtering using the timestamp information. The AppImplMswBasic.cpp uses a similar implementation for Windows (see onTouch method). BTW you can update pull requests by simply pushing to your feature branch. |
|
The touch event behavior I've encountered on osx, windows, and ios builds of cinder has been this: touchesBegan, touchesMoved, and touchesEnded all fire after system touch events area realized. They only contain relavant touch IDs (touchesMoved will only have touch IDs that moved) and they never have duplicates. This logic seems to be reflected in the code. In AppImplMswBasic.cpp, privateTouchesBegan__ gets called when onTouch fires. In CinderView.mm a delegated privateTouchesBegan__ gets called when touchesBeganWithEvent fires. But in AppAndroid, temporary vectors of began/moved/ended touches are stored and then pushed to cinder level events periodically, leading to the ID buildup. Are you not seeing this same behavior as I am on other platforms? |
|
I understand the touch behaviour may not match the OS's exactly, but I've written code that works across platforms by handling multiple pointer events in my callbacks. I agree that consistent behaviour across platforms would be the ideal situation. The main issue as I understand it is that Android passes each touch event down separately, while ios and Windows group multiple events together. I group the events between frames. I'm guessing your patch sends a single touch event to the callback, which doesn't quite match the other platforms either. Anyway, send the patch through and I'll give it a whirl :) |
|
Added the patch. It's true that began and ended touch events fire one at a time on android. But I've noticed so far that moved events have 1 to N unique touch IDs. The patch insures that the life of a touch ID is in the correct run order and that the ID is always unique. |
No description provided.