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

[WIP] Touch support #143

Draft
wants to merge 3 commits into
base: wayland
Choose a base branch
from
Draft

[WIP] Touch support #143

wants to merge 3 commits into from

Conversation

chayleaf
Copy link

@chayleaf chayleaf commented Aug 24, 2024

See #122

This implements touch support in the following way:

  • Holding the screen and moving your finger up/down scrolls down/up. Scroll speed is hardcoded, and the mouse position is set to the initial tap position.
  • A short tap emulates a return button press.
  • A long tap that didn't move far enough to scroll emulates an escape button press (ideally, -click-to-exit would work, then this would be unnecessary)
    This is obviously not ideal, so I'd like some feedback on how a proper implementation should look like

Maybe programmable gesture support should be added? this would be useful for e.g. mode switching. I have no idea how the config for that should look though.

Copy link
Owner

@lbonn lbonn left a comment

Choose a reason for hiding this comment

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

I cannot really test this right now but it seems fine to have it merged as a first iteration :)

Thanks!

@@ -526,6 +526,100 @@ static const struct wl_keyboard_listener wayland_keyboard_listener = {
.repeat_info = wayland_keyboard_repeat_info,
};

static void wayland_touch_down(void *data, struct wl_touch *wl_touch,
Copy link
Owner

Choose a reason for hiding this comment

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

The indentation is a bit inconsistent with the rest of the file.

Could you please run clang-format on the lines you've added?

return;
}
int key = KEY_ENTER;
if (time - self->touches[id].start_time > 200) {
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe define this 200 as a constant with a proper name at the top of this function?

}

static int32_t y_offset_to_line_offset(int32_t y_offset) {
static const int32_t line_height = 20;
Copy link
Owner

Choose a reason for hiding this comment

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

So, this "20" is some kind of hard-coded sensitivity?

It is fine for now but maybe add a TODO comment like "TODO: make sensitivity parametrizable"

in0ni added a commit to in0ni/rofi-wayland that referenced this pull request Oct 27, 2024
@in0ni
Copy link

in0ni commented Oct 27, 2024

Moving discussion from #156 here, and will close according to authors responses.

@chayleaf, do you plan continuing work on this? I figured your PR was exactly that, a request to pull this into code base, and @lbonn has already expressed he would merge as an initial iteration.

Though it was not clear from this thread that this does not work as you state in 156... yes it's not perfect, but from my initial testing on two devices suffices to avoid having to replace rofi. There are some minor bugs, but at least it's functional and for me it greatly helps. Better this than having to look for another launcher.

If you can provide some feedback, I'm happy to help, or close the other PR -- just let me know :)

PS: duly noted regarding history/authorship, will do, thanks.

@chayleaf
Copy link
Author

chayleaf commented Oct 27, 2024

do you plan continuing work on this?

I am daily driving this branch on my phone, which is why I know it's not ready to merge yet. I do want to fix the issues when I have the time.

I figured your PR was exactly that, a request to pull this into code base

There is a reason this is a draft PR, draft PRs aren't ready for merging

For now, I recommend you to apply these patches locally.

if (time - self->touches[id].start_time > 200) {
key = KEY_ESC;
}
nk_bindings_seat_handle_key(wayland->bindings_seat, NULL,
Copy link
Author

Choose a reason for hiding this comment

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

on some rofi launches, this doesn't work (i.e. if it doesn't work, it won't magically fix itself if you click multiple times). TODO figure out why

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants