Dex touchpad fix: orientation, click, gesture, etc.

[knyipab]

In short, it should fix #419. The suggested changes should have no impact on existing functionality. If you need to review the code changes and features added, I hope below explanation helps and won't be too long.

Root Causes of #419

• Unlike touchscreen, the tocuchpad reports coordinates in its own (physical) orientation, does not flip along with display rotation.
• Also, the comment at line 185 "Regular touchpads and Dex touchpad ... should be handled as touchscreens with trackpad mode" is not implemented as expected. Indeed, mTouchpadHandler.handleTouchEvent() will handle pointer inputs as hardware mouse, calling mHMListener.onTouch because of the "if condition" (|| (event.getPointerCount() == 1 && mTouchpadHandler == null && (event.getSource() & InputDevice.SOURCE_TOUCHPAD) == InputDevice.SOURCE_TOUCHPAD)) being positive.

My solution

• Remove the "if condition" so that touchpad event can invoke TrackpadInputStrategy. Note that we could not simply create another variable to be invoked because the code logics test mInputStrategy at multiple places. I found it easiest to just override mInputStrategy in DeX (SamsungDexUtils.checkDeXEnabled(mContext)). This makes sense because (1) finger input in DeX can only be touchpad not touchscreen, and (2) mouse input handler in DeX is also overrided.
• The flipped axis issue is tackled by introducing another option for captured pointer transformation, namely "Automatic (for touchpad)". Under this option, the rotation of display ID 1 (primary display I believe) will be checked and motion input will be handled accordingly. Hopefully this could handle both Pad and DeX touchpad issue.
• The 3-finger down gesture to "toggle EK bar" is change to "release pointer capture" under DeX mode. This is because (1) EK bar is useless and cannot be pressed in captured pointer mode, and (2) if user enters DeX using solely soft touchpad and soft keyboard, there should be a way to exit the capture mode and the more-than-2-finger gesture is just right play that role (3-finger as shortcut to release capture plus minimize window, 4-finger to only release capture). At the end, the current way for soft touchpad & soft keyboard under DeX to use EK bar is perhaps to first release pointer capture and use volume key to toggle EK bar.

[knyipab]

Force pushed fix to crash in "Automatic (for touchpad)" mode mentioned in #419.

[knyipab]

@twaik perhaps we should rehthink the gesture, though I do not have a concrete idea in mind. After some time testing this PR with soft touchpad in DeX, I found that there are not enough gesture/button to accomplish all four features below.

feature	default gesture/button (with this PR)	comment
escape pointer capture	3+ finger swipe down	necessary cuz soft keyboard often has no "ESC" key
activate soft keyboard	3+ finger swipe up	necessary cuz soft touchpad users often rely on soft keyboard
toggle EK bar	vol down	highly preferred feature as EK bar is useful for DeX + soft keyboard users
volume up / down	None	when preference set to not intercept physical vol+/- buttons, it will work adjust volume, but sacrifices the function of toggling EK bar

[twaik]

Remove the "if condition" so that touchpad event can invoke TrackpadInputStrategy. Note that we could not simply create another variable to be invoked because the code logics test mInputStrategy at multiple places. I found it easiest to just override mInputStrategy in DeX (SamsungDexUtils.checkDeXEnabled(mContext)). This makes sense because (1) finger input in DeX can only be touchpad not touchscreen, and (2) mouse input handler in DeX is also overrided.

The touchpad handling code is created for both Dex and real touchpad cases. And there may be physical touchpad, mice and touchpad connected to the same device at the same time. That is a reason I am invoking the code handling touchpads and Dex in separate TouchInputHandler instance with forced trackpad mode. mInputStrategy overriding will break this case.

[knyipab]

I am indeed well aware of the fact that touchpad + mouse should be fine, but in touchpad + mouse + touchscreen (in DeX) or perhaps simply touchscreen in DeX, the touchscreen mode would be completely overridden. Besides, I am not sure how touchscreen can behave under onscreen DeX mode (as a touchpad? or mouse? does Termux:X11 touchscreen mode work?) which is only available to Tablet user and I don't own one.

It is just that

the code logics test mInputStrategy at multiple places

So it will require some more changes to your code base (probably a real mTouchpadHandler resembling touchpad InputStrategy). I was afraid to do such big change and unsure about my expertise.

It's up to you. Want me to implement it (though not any time soon) or you change it directly? Or is it still acceptable as is?

[twaik]

Ok, probably it is a good time to make a rebase.

So it will require some more changes to your code base

In the case if you detect event came from touchpad you should redirect it to mTouchpadHandler immediately (in TouchInputHandler::onTouch).

About

                if (capturedPointerTransformation == CapturedPointerTransformation.AUTO) {
                    for (Display display : mDisplayManager.getDisplays()) {
                        if (display.getState() == Display.STATE_ON) {
                            transform = display.getRotation() % 4;
                            break;
                        }
                    }
                }

I still think it is a good idea to use DisplayManager::getDisplay(Display.DEFAULT_DISPLAY) which should return phone's primary display.

About

                case MotionEvent.ACTION_BUTTON_PRESS:
                case MotionEvent.ACTION_BUTTON_RELEASE:
                    // For hardware touchpad in DeX (captured mode), handle physical click buttons
                    if ((event.getSource() & InputDevice.SOURCE_TOUCHPAD) == InputDevice.SOURCE_TOUCHPAD) {
                        currentBS = event.getButtonState();
                        for (int[] button: buttons)
                            if (isMouseButtonChanged(button[0]))
                                mInjector.sendMouseEvent(null, button[1], mouseButtonDown(button[0]), true);
                        savedBS = currentBS;
                        break;
                    }

I still think it is a good idea to move it out from switch. There are some weird firmwares with custom actions hardcoded. Sending event will not be triggered in the case if button state is not changed so it should be safe.

Also be aware of new preferences making possible to configure three-finger swipes, back button, volume keys actions. Also there is new option to toggle soft keyboard from additional keyboard bar (enabled by default) so the changes overriding default actions in dex mode should be removed.

[knyipab]

Rebased and addressed all your comments.

In the case if you detect event came from touchpad you should redirect it to mTouchpadHandler immediately (in TouchInputHandler::onTouch).

Sounds smart and requires minimal changes. I frontload the mTouchpadHandler in handleTouchEvent in a5a9147. Now mInputStrategy overriding only takes place when it is mTouchpadHandler. Perhaps a thorough test is probably having a tablet owner to use physiccal touchpad and touchscreen in non-trackpad mode together.

I still think it is a good idea to use DisplayManager::getDisplay(Display.DEFAULT_DISPLAY) which should return phone's primary display.

Thanks. While mActivity.getLorieView().getDisplay() does not work well for my foldable device soft touchpad under DeX, DisplayManager::getDisplay(Display.DEFAULT_DISPLAY) works perfectly.

I still think it is a good idea to move it out from switch. There are some weird firmwares with custom actions hardcoded. Sending event will not be triggered in the case if button state is not changed so it should be safe.

I moved it out of the switch statement. See if you may want to invite other users to conduct a final UAT cuz I don't own a tablet.

Also be aware of new preferences making possible to configure three-finger swipes, back button, volume keys actions. Also there is new option to toggle soft keyboard from additional keyboard bar (enabled by default) so the changes overriding default actions in dex mode should be removed.

I am fine with new preferences and now this PR leaves the three-finger swipe gestures untouched. Cuz I found that under DeX soft touchpad mode, I can escape the pointer capture by swipe up at the bottom of phone screen. That will bring up the bottom "navigation gesture bar" (not sure the correct name) and escape pointer capture. Nonetheless, it assumes new Termux:X11 user to know such trick, and perhaps some first time user would be panic and ends up unplugging the HDMI cable to escape.

Well... perhaps this app has already made quite some assumptions about new users when we don't have a starter guide LOL. For example, I discovered preferences page can be triggered from notification bar after few weeks of use, or perhaps I am stupid haha.

[knyipab]

By the way,

1. shall we make "transform captured pointer movements" by default to be "Automatic (for touchpad)"? It seems harmless and perhaps enhane user experience
2. would it be better to show the current chosen option right below "transform captured pointer movements"? I couldn't quite find where to implement unless dive deeper in your code.

[twaik]

Well... perhaps this app has already made quite some assumptions about new users when we don't have a starter guide LOL. For example, I discovered preferences page can be triggered from notification bar after few weeks of use, or perhaps I am stupid haha.

Not stupid. I have some troubles with writing user manuals.
Also yesterday I pushed a commit which adds preferences and toggle soft keyboard actions to the additional key bar.

shall we make "transform captured pointer movements" by default to be "Automatic (for touchpad)"? It seems harmless and perhaps enhance user experience

I already checked it when tried to fix touchpad by myself. It breaks touchpads on other devices.

would it be better to show the current chosen option right below "transform captured pointer movements"? I couldn't quite find where to implement unless dive deeper in your code.

I think it will be enough to change the summary of preference when it changes. I'll do that a bit later.

[twaik]

Probably polling for display rotation with mDisplayManager.getDisplay(Display.DEFAULT_DISPLAY).getRotation() on every single event is not very wise. You can get more than 200 input events in a sec and invoking IPC for every single event seems to be wasteful. Probably it would be better to create self-calling lambda like checkXEvents which invokes MainActivity.handler.postDelayed to call itself every 300 ms which will save current rotations somewhere.

[twaik]

@AlphaBs @hansm629 can you please test latest CI artifact of this PR?

[hansm629]

@AlphaBs @hansm629 can you please test latest CI artifact of this PR?

@twaik
I'll test it out after work today

[AlphaBs]

I tested this build on Galaxy Tab S8+ bookcover keyboard, with trackpad, capturing external pointer, and automatic point movements mode.

Touch gestures (tap, two-finger tap to right click, two-finger scroll, click buttons, long tap to drag, tap-to-move), scroll axis, pointer movement axis, pointer speed are all normal and work perfectly. Thanks for a great job!

[knyipab]

@twaik just pushed a commit to update rotation using DisplayManager#registerDisplayListener() which works on my phone. See if you may have concern about firmware/vendor-specific issue and prefer MainActivity.handler.postDelayed.