The latest input code features api improvements that make bug 890186 easier to implement.

Got this running on JB. Now need to get it working on ICS.

Working on ICS. Now need to test back on JB..

Works as expected.

This is for reference during review.

Comment on attachment 795698 [details] [diff] [review] Our changes to the Android files Review of attachment 795698 [details] [diff] [review]: ----------------------------------------------------------------- Most of these changes involve changing what headers are included. A bunch of them fix the code to point to local things. The logging change was a bit more intrusive in order to force the code to load our own logging header. There's also a bunch of #ifdef'ing in the SpriteController code to disable that code - IIRC, it tries to use surfaceflinger to draw the cursor. ::: widget/gonk/libui/InputReader.h @@ +498,4 @@ > virtual ~InputReaderThread(); > > private: > + uint32_t mFoo; This hack is also present in the current libui. Something was overwriting mReader, and we didn't have time to figure out the exact issue last year. Putting this dummy field in prevented mReader from getting corrupted.

Comment on attachment 795696 [details] [diff] [review] Update libui, v2 Review of attachment 795696 [details] [diff] [review]: ----------------------------------------------------------------- Most of this updating libui. There's a few files outside libui which are also updated: nsAppShell.cpp moz.build Makefile.in A large number of files were removed from libui since they were only necessary for building on Gingerbread. ::: widget/gonk/Makefile.in @@ +34,4 @@ > LOCAL_INCLUDES += \ > -I$(ANDROID_SOURCE)/hardware/libhardware/include \ > -I$(ANDROID_SOURCE)/hardware/libhardware_legacy/include \ > + -I$(ANDROID_SOURCE)/frameworks/native/opengl/include \ libui now pulls in headers that look for EGL headers, so I added this in.

Comment on attachment 795696 [details] [diff] [review] Update libui, v2 m1, would be nice to get your feedback on whether the headphone/mic insertion event stuff still works after this patch.

Comment on attachment 795698 [details] [diff] [review] Our changes to the Android files Review of attachment 795698 [details] [diff] [review]: ----------------------------------------------------------------- ::: widget/gonk/libui/EventHub.cpp @@ +49,4 @@ > #include <sys/epoll.h> > #include <sys/ioctl.h> > #include <sys/limits.h> > +#include <sha1.h> This lets EventHub.cpp find bionic's implementation of sha1.h.

Comment on attachment 795696 [details] [diff] [review] Update libui, v2 Ben, do you mind reviewing this? The last reviewer I had for this code is not available (cjones), and it seems like you might be familiar with this code.

Comment on attachment 795696 [details] [diff] [review] Update libui, v2 Looks like headset detection is broken by this patch. Here's what the event stream looks like on my device: ---- $ adb shell getevent -l /dev/input/event4 <Insert headset> /dev/input/event4: EV_SW SW_HEADPHONE_INSERT 00000001 /dev/input/event4: EV_SW SW_MICROPHONE_INSERT 00000001 /dev/input/event4: EV_SYN SYN_REPORT 00000000 <Remove headset> /dev/input/event4: EV_SW SW_HEADPHONE_INSERT 00000000 /dev/input/event4: EV_SW SW_MICROPHONE_INSERT 00000000 /dev/input/event4: EV_SYN SYN_REPORT 00000000 ----

Thanks. I'll review the headset detection code a bit more closely. Also, it looks like updating this code will also fix bug 904544 .

Tried fixing the jack detection with this, but it doesn't seem to work and I'm too tired to figure it out tonight.

Comment on attachment 795696 [details] [diff] [review] Update libui, v2 Michael, I've traced only part of key event flow and ain't familiar with libui code. Sorry can't help it.

Comment on attachment 796373 [details] [diff] [review] Fix headphone detection logic I don't see what's wrong with this. Maybe it works better on the devices that require it?

Comment on attachment 795696 [details] [diff] [review] Update libui, v2 m1, would you be interested in reviewing this? We're low on reviewers familiar with the input code. This is mostly updating to the latest input code from Android so it shouldn't be too complicated, though I've made comments on the tricky bits. I don't intend to check this in until we fix the jack detection, so I think we can treat that part as a follow up within this bug. (generating these patches was a pain..)

Comment on attachment 795696 [details] [diff] [review] Update libui, v2 r+ based more on attachment 795698 [details] [diff] [review] than this.

Comment on attachment 796373 [details] [diff] [review] Fix headphone detection logic Does this fix N4? On my device I see SW_HEADPHONE_INSERT at |getevents| but it doesn't make it up to GeckoInputDispatcher::notifySwitch(). SW_MICROPHONE_INSERT makes it all the way up though.

(In reply to Michael Vines [:m1] [:evilmachines] from comment #17) > Comment on attachment 796373 [details] [diff] [review] > Fix headphone detection logic > > Does this fix N4? > > On my device I see SW_HEADPHONE_INSERT at |getevents| but it doesn't make it > up to GeckoInputDispatcher::notifySwitch(). SW_MICROPHONE_INSERT makes it > all the way up though. Doesn't work on N4, so I was wondering if it worked on your device. Guess I'm still missing something then.

This seems to work for me. Just had to treat the thing as a bitmask.

Unbitrotted

This caused a regression reported on bug 914776. There are multiple regressions reported on that bug; the specific one I'm talking about is that in the Gallery app, in landscape mode, the 'camera' button (which should allow switching back to the camera mode to take new pictures) does nothing.

It looks like the mXScale and mYScale is not correct in landscape mode, it will cause the touch event mapping incorrect. I think problem comes from the mSurfaceWidth and mSurfaceHeight. // The surface orientation, width and height set by configureSurface(). // The width and height are derived from the viewport but are specified // in the natural orientation. int32_t mSurfaceWidth; int32_t mSurfaceHeight; Take N4 for example, it should be 768(width)*1280(height) no matter it's landscape or portrait. But we have 768*1280 in portrait and 1280*768 in landscape mode now. Since we always setDisplayInfo with gScreenBounds.width and gScreenBounds.height in nsAppShell, I'm thinking we should used it directly as the mSurfacewidth and mSurfaceHeight in TouchInputMapper::configureSurface without considering the orientation. Please feel free let me know if you want me to provide the patch. Thank you.

Thanks for debugging! Do you mind providing a patch? We don't have enough people working on this code, which is a problem when I need to get a review in this code.

Hi Michael, I'm not so sure if it's the best way to fix this since we do rotation calculating in different functions. Thank you for any of your suggestion or feedback!

Can't you remove all rotation handling in GeckoPointerController::getBounds ? We shouldn't touch libui unless we need to.

Also, this follow up should be in a new bug.