-
-
Notifications
You must be signed in to change notification settings - Fork 264
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dex touchpad fix: orientation, click, gesture, etc. #617
base: master
Are you sure you want to change the base?
Conversation
6fbd8c8
to
1a94082
Compare
Force pushed fix to crash in "Automatic (for touchpad)" mode mentioned in #419. |
lorieView.releasePointerCapture(); | ||
lorieParent.releasePointerCapture(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to ask for pointer release twice. Pointer capture is activity-wide.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just copied from somewhere else without much thinking about it. Feel free to amend if you think appropriate
toggleKeyboardVisibility(MainActivity.this); | ||
Log.d("MainActivity", "Toggling keyboard visibility"); | ||
if(inputMethodManager != null) { | ||
inputMethodManager.showSoftInput(lorieView, InputMethodManager.SHOW_FORCED); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason to trigger toggling IME twice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
During my previous test, toggleKeyboardVisibility
did not work. However, I cannot reproduce it now, so I think one toggleKeyboardVisibility
should just work.
I was mistaken in previous test perhaps because the keyboard toggling feature disappear sometimes when the app switches from phone screen to external display or vice versa. It isn't too annoying so now I simply close and re-open everytime.
if (capturedPointerTransformation == CapturedPointerTransformation.AUTO) { | ||
for (Display display : mDisplayManager.getDisplays()) { | ||
if (display.getState() == Display.STATE_ON) { | ||
transform = display.getRotation() % 4; | ||
break; | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any specific reason to obtain display via DisplayManager
and not using mActivity.getLorieView().getDisplay()
? This definitely will break in multi-display mode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cuz in smarphone HDMI external display DeX mode (using phone as software touchpad), the transformation should depend on phone orientation (=soft touchpad) but mActivity.getLorieView().getDisplay()
return HDMI display which is useless.
This enumeration works well for case of both external display DeX + software touchpad and , tablet hardware touchpad.
I do reckon the risk of not working well in some more complex setup, such as external display DeX + hardware touchpad, or Tablet in external display DeX + hardware touchpad, and perhaps more etc.
A better approach might be depends on input device information. But I don't have both devices so I used that workaround. FYR. soft touchpad might be "sec_touchpad" but I am not so sure. In case you want to implement:
@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.
|
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
mTouchpadHandler.handleTouchEvent()
will handle pointer inputs as hardware mouse, callingmHMListener.onTouch
because of the "if condition" (|| (event.getPointerCount() == 1 && mTouchpadHandler == null && (event.getSource() & InputDevice.SOURCE_TOUCHPAD) == InputDevice.SOURCE_TOUCHPAD)
) being positive.My solution
TrackpadInputStrategy
. Note that we could not simply create another variable to be invoked because the code logics testmInputStrategy
at multiple places. I found it easiest to just overridemInputStrategy
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.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.