Skip to content
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

Expand digitizer feature. #15917

Closed
wants to merge 1 commit into from
Closed

Conversation

rcrowell
Copy link

Expands on the awesome digitizer work done by @a-chol in #12851.

This is my first time contributing to qmk or writing anything substantial in c, so feedback is very welcome.

Description

Fleshes out the recent digitizer feature:

  • configurable usage id (linux seems to ignore the default generic usage 0x01)
  • support additional stylus buttons, like barrel buttons and the eraser
  • add "digitizer keys"; position the cursor and set stylus buttons from keymap
  • various optional operating system work-arounds via fuzzy positioning:
    1. generating an additional nearby touch event (sometimes repeat identical touch events are ignored)
    2. jiggling the mouse slightly after touching (sometimes osx hides the cursor after a touch event... not sure exactly when or why)

Breaking changes:

  • switch from relative x/y floats to absolute ints, with DIG_REL2ABS_X(float) and DIG_REL2ABS_Y(float) macros to get back the old behavior
  • remove tipswitch member from digitizer_t in favor of a buttons bitmask

I have also added an example keymap using these features under keyboards/handwired/digitizer_demo/.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

  • None as far as I know.

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

* switch to absolute x/y

* switch to buttons bitmask

* add digitizer keys for keymaps

* configurable usage id

* fuzzy x/y os workarounds
@@ -0,0 +1,48 @@
#pragma once
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs a license header.

But also, this may be better as a keymap for handwired/onekey

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, good call. I just wired up a promicro onekey board to test with, so I can make this suggested change.

@zvecr zvecr changed the base branch from master to develop January 18, 2022 14:37
@a-chol
Copy link
Contributor

a-chol commented Jan 18, 2022

I read through the changes and they look good.

There is a sample fw in handwired/onekey that you can use to test and demonstrate usage of what you added. It probably should be updated with your changes at least.

I also had troubles with the cursor disappearing on windows. My first approach was to move the mouse afterwards like you did but after digging, I found that windows could be configured to avoid hiding the cursor + setting the usage Id to generic digitizer helped. I couldn't test on macOS so that's a good thing you could.

@drashna
Copy link
Member

drashna commented Jan 22, 2022

I also had troubles with the cursor disappearing on windows. My first approach was to move the mouse afterwards like you did but after digging, I found that windows could be configured to avoid hiding the cursor + setting the usage Id to generic digitizer helped. I couldn't test on macOS so that's a good thing you could.

Would it be worth adding a define so that this behavior can be configurable? Because I can see how both cases could be useful.

@rcrowell
Copy link
Author

Would it be worth adding a define so that this behavior can be configurable? Because I can see how both cases could be useful.

There actually are several defines to tweak how this feature works now. You can enable either (or both) of the 'fuzzy' strategies that I implemented; by default, both are disabled:

// enable all fuzzy strategies
#define DIGITIZER_FUZZ
// enable fuzzy InRange strategy, which generates multiple touch events
#define DIGITIZER_FUZZ_INRANGE
// enable fuzzy mouse strategy, which jiggles the mouse after a touch event
#define DIGITIZER_FUZZ_MOUSE

I think the function name digitizer_fuzz_xy could be tweaked, since it will send a touch event even if all the fuzzy strategies are disabled. Probably something simple like digitizer_point_xy would be better.

@stale
Copy link

stale bot commented Apr 16, 2022

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@yorickpeterse
Copy link

yorickpeterse commented May 16, 2022

I've been toying around with this MR (it requires some changes to not cause conflicts in master). One thing I noticed is that under Wayland/libinput, the digitizer device gets ignored with the following warning:

event4  - Bastard Keyboards Skeletyl Touchscreen: libinput bug: missing tablet capabilities:
 resolution. Ignoring this device.

This is from libinput list-devices. Applying steps similar to Evidlo/remarkable_mouse#18 (comment) (but obviously using the correct IDs per my local keyboard) seems to do something, but not much: I have a key that's supposed to move the mouse to the top-left middle of the screen. Pressing the button results in the cursor sort of "blinking" then vanishing until I move it again. Unfortunately, the cursor doesn't actually move to the desired position.

Fuzzing is something I couldn't get to work, as master now requires a pointing device driver when POINTING_DEVICE_ENABLE is enabled, and I'm not sure what driver value one would use here (since there technically is no pointing device). Seems this is only needed when enabling all strategies.

@a-chol
Copy link
Contributor

a-chol commented May 18, 2022

I checked the HID usage table again and couldn't find a clear usage to define the resolution. If you can find it or an example of a hid report that works, that could be helpful.
In the meantime, maybe you can try to change the current report for the digitizer part to declare a different usage : the current is the Pen usage (usb_descriptor.c:174, 0x20). You could try a different usage that may work with the same values in the report, for instance Touch Screen (replace 0x20 with 0x04), Touch Pad (0x05), Multipoint digitizer (0x0C), Finger (9x22).

@yorickpeterse
Copy link

@a-chol IIRC I tried it by reporting it as either a touch screen or pen. In both cases the cursor is duplicated, and apparently this is by design in Wayland/GNOME: https://gitlab.gnome.org/GNOME/mutter/-/issues/1916. I've put this on the side for now, but I'd be happy to further test this MR once it's ready (i.e. takes into account the latest master changes).

@github-actions
Copy link

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with bug, awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@github-actions github-actions bot added the stale Issues or pull requests that have become inactive without resolution. label Jul 13, 2022
@github-actions
Copy link

Thank you for your contribution!
This pull request has been automatically closed because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, or re-open when it's ready.
// [stale-action-closed]

@github-actions github-actions bot closed this Aug 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting changes core documentation keyboard keymap stale Issues or pull requests that have become inactive without resolution.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants