| Summary: | SDL-Android project: Classification of touch events | ||
|---|---|---|---|
| Product: | SDL | Reporter: | wboe <w.boeke> |
| Component: | *don't know* | Assignee: | Gabriel Jacobo <gabomdq> |
| Status: | RESOLVED WONTFIX | QA Contact: | Sam Lantinga <slouken> |
| Severity: | normal | ||
| Priority: | P2 | CC: | gabomdq, philipp.wiesemann |
| Version: | HG 2.0 | Keywords: | target-2.0.0 |
| Hardware: | All | ||
| OS: | Android (All) | ||
| Attachments: | Patch file for SDL-for-Android project | ||
If getActionMasked() [1] or getActionIndex() [2] is used this part will not work on Android 2.1 or older. See Bug 1563. If pointerCount == 1 then pointerCount > 1 will be false. The supplied line numbers seem not to match the file in repository. The leftFingerDown variable is used for mouse emulation. This is needed because the "left" mouse button can only go down once but on touch screen there may be multiple fingers down. This would fire multiple mouse down events (which would be bad emulation :). The two constants are really deprecated on Java side [3] because there are new ones with the same value. How they are called on C side does not really matter because there is no semantic link to the Java side. [1] http://developer.android.com/reference/android/view/MotionEvent.html#getActionMasked%28%29 [2] http://developer.android.com/reference/android/view/MotionEvent.html#getActionIndex%28%29 [3] http://developer.android.com/reference/android/view/MotionEvent.html#ACTION_POINTER_1_DOWN (In reply to comment #1) > > If pointerCount == 1 then pointerCount > 1 will be false. The supplied line > numbers seem not to match the file in repository. > To clarify: if the action == ACTION_MOVE and pointerCount == 1, then pointerFingerId will have the value event.getPointerId(actionPointerIndex), which is the initial value which is only valid for a DOWN or an UP event. Sorry for the wrong line numbers. > > The two constants are really deprecated on Java side [3] because there are new > ones with the same value. How they are called on C side does not really matter > because there is no semantic link to the Java side. > Indeed, this was more like a cosmetic issue. Still I think that also the comments inside source code should be not misleading. If you can update the patch without getActionMasked and getActionIndex I'll take a look at it to incorporate it. Thanks! (In reply to comment #3) > If you can update the patch without getActionMasked and getActionIndex I'll > take a look at it to incorporate it. Thanks! Dear Gabriel, What's the point of sticking to old Android API's because someone might be still using them? I think you underestimate the intellect of the average dev! Anyhow, please keep the getActionMasked and getActionIndex, they make the code cleaner and better understandable. You are missing the point, it's not a matter of developer intellect. The lowest API level SDL requires, the more devices it can run in. To force an increase in the minimum API level required, there needs to be some substantial gain with no way around it, in this case there's none that I can see. (In reply to comment #5) I'm feeling very stupid, but I can't recover the original, patched SDLActivity.java. Please could you do it manually? old: final int action = event.getActionMasked(); new: final int action = event.getAction() & MotionEvent.ACTION_MASK; old: int actionPointerIndex = event.getActionIndex(); new: int actionPointerIndex = (event.getAction() & MotionEvent.ACTION_POINTER_ID_MASK) >> MotionEvent.ACTION_POINTER_ID_SHIFT; Regards, Wouter (In reply to comment #6) Maybe the whole issue about getActionMasked() and getActionIndex() is moot, because of a statement in the Android documentation at http://developer.android.com/tools/sdk/ndk/index.html. There is stated: If you use this NDK to create a native library that uses the API to access Android Bitmap pixel buffers or utilizes native activities, the application containing the library can be deployed only to devices running Android 2.2 (API level 8) or higher. To ensure compatibility, make sure that your application declares <uses-sdk android:minSdkVersion="8" /> attribute value in its manifest. This means that all that is available from version 8 can be used, including getActionMasked and getActionIndex FYI, we're currently requiring SDK revision 10 for SDL 2.0. I don't remember what bumped us to that, but that's where we're currently sitting. Gabriel, at some point it's worth doing a review of the Android code and marking which revision we need and why, and upgrading code workarounds for older SDK versions we no longer need. I've added a few comments on the README.android file stating the minimum API level required, and a short paragraph explaining why (I doubled checked it by trying to build against API level 8, there's no egl.h available in the NDK). http://hg.libsdl.org/SDL/rev/6ce8edb5577b (In reply to comment #9) > I've added a few comments on the README.android file stating the minimum API > level required, and a short paragraph explaining why (I doubled checked it > by trying to build against API level 8, there's no egl.h available in the > NDK). > > http://hg.libsdl.org/SDL/rev/6ce8edb5577b According to the NDK version history [1] support for OpenGL ES was added in September 2009. API 9 was released in December 2010. Therefore API 9 could be no requirement for using it. Also the documentation [1] states that OpenGL ES 1.1 may be used with API level 4. Which means parts of SDL may be used with older Android versions. The reason for API 10 in SDLs AndroidManifest.xml seems to have nothing to do with the source code itself. [2] Also there may be a misunderstanding about "minSdkVersion" and "targetSdkVersion". [3] I think "targetSdkVersion" should be "10" or higher while "minSdkVersion" currently may be "8" or lower. Last time I checked "7" seems to be working (in emulator). The function "getExternalFilesDir" is used on C side which requires API 8 but if it is not used it will not matter. SDL's README.Platforms states a different Android version. Maybe this could be adapted to fit "minSdkVersion". [1] http://developer.android.com/tools/sdk/ndk/index.html [2] http://hg.libsdl.org/SDL/rev/47ab7ba21530 [3] http://developer.android.com/guide/topics/manifest/uses-sdk-element.html Did you look at the actual NDK files? Try compiling SDL for Android using the "android-8" platform from the NDK and see what happens. (In reply to comment #11) > Did you look at the actual NDK files? Try compiling SDL for Android using > the "android-8" platform from the NDK and see what happens. I compile with "targetSdkVersion" set to "10" and "android-10". Therefore it works or me. The compiled objects still can be used on older Android devices down to the "minSdkVersion". Things like newer headers only exist at compile time. Just take a look at the documentation I linked in my last post or try it with an emulator. I've read the documentation quite a few times, thanks for the suggestion though. As it is now, SDL's code requires (among other things, as you mentioned yourself) the egl.h header, which is present on the android-9 NDK platform, which puts us on an API level 10 requirement to build the library. Could the binaries generated with the android-9 platform work on lower API level devices? Maybe. Did you try it on a good number of API level 8 devices to be comfortable doing that recommendation to everyone? I sure did not, and I don't think we should recommend it. Android is already a minefield of vendors and device idiosyncrasies, I don't think hinting to users that they could run this on API level 8 or lower has much benefit, nor is the idea of recommending "you can run on API level X if you remove Y feature" from the library, specially because API level 10 now comprises 90% of the market. (In reply to comment #13) > I've read the documentation quite a few times, thanks for the suggestion > though. Your changeset added (in my option) wrong information about OpenGL ES to the README.android. That is what I wanted to point out some posts ago and that is why I referenced the documentation (because the dates are in there) in the first place. (In reply to comment #10) > (In reply to comment #9) > > I've added a few comments on the README.android file stating the minimum API > > level required, and a short paragraph explaining why (I doubled checked it > > by trying to build against API level 8, there's no egl.h available in the > > NDK). > > > > http://hg.libsdl.org/SDL/rev/6ce8edb5577b > > According to the NDK version history [1] support for OpenGL ES was added in > September 2009. API 9 was released in December 2010. Therefore API 9 could > be no requirement for using it. > Also the documentation [1] states that OpenGL ES 1.1 may be used with API > level 4. Which means parts of SDL may be used with older Android versions. > > [...] > > SDL's README.Platforms states a different Android version. Maybe this could > be adapted to fit "minSdkVersion". > > [...] I created bug #1748 to propose a patch which makes the text (in my opinion) more correct. I also searched for errors in other README files to make the patch more worthwhile. (Sorry if you get a lot of copies of this email, we're touching dozens of bug reports right now.) Tagging a bunch of bugs as target-2.0.0, Priority 2. This means we're in the final stretch for an official SDL 2.0.0 release! These are the bugs we really want to fix before shipping if humanly possible. That being said, we don't promise to fix them because of this tag, we just want to make sure we don't forget to deal with them before we bless a final 2.0.0 release, and generally be organized about what we're aiming to ship. Hopefully you'll hear more about this bug soon. If you have more information (including "this got fixed at some point, nevermind"), we would love to have you come add more information to the bug report when you have a moment. Thanks! --ryan. Gabriel, is there anything left that needs to be done to close this bug out? It looks like a bunch of things were discussed. The patch as is doesn't apply, and reviewing the discussion I don't think it's valid (as Philipp stated, if pointerCount == 1 then pointerCount is not > 1). Marking as WONTFIX but feel free to update the patch and re open if there's something of value that I'm missing. |
Created attachment 1012 [details] Patch file for SDL-for-Android project Hi, IMHO in SDLActivity.java there is a glitch in the classification of touch events. After line 624: if (action == MotionEvent.ACTION_MOVE && pointerCount > 1) { in case pointerCount == 1, the statement pointerFingerId = event.getPointerId(i) will be executed which is not correct. I attached a patch file, that is also more modern and straightforward. There is one more issue. In src/video/android/SDL_androidtouch.c there is a boolean leftFingerDown that probably is meant to repair some extern bug, I don't know what. Anyhow I see no use for that. Maybe it's a good idea to remove it plus all the code that would be executed if its value was true. In the same file there is: // The following two are deprecated but it seems they are still emitted (instead the corresponding ACTION_UP/DOWN) as of Android 3.2 #define ACTION_POINTER_1_DOWN 5 #define ACTION_POINTER_1_UP 6 Actually this is not deprecated at all, but the names should be ACTION_POINTER_DOWN and ACTION_POINTER_UP. They are used if one finger goes up or down while there are other fingers still down. Kind regards, Wouter Boeke