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: Implement the keyboard shortcuts inhibitor protocol #5

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

NDagestad
Copy link

There are a few things still to do:

  • The inhibitor key acts like a toggle switch; you pressed it to toggle between inhibited and not inhibited
    • A behaviour like qemu or virtualbox where pressing the key only give you inputs back for 1 keypress seems more intuitive
  • There is no way of knowing if the input is inhibited or not
    • some kind of indicator like updating the window title would help in that regard
  • Making this magic key configurable is kinda important imo
  • I wanted to use a modifier with the key but it's my first time using libxkbcommon, so I haven't figured that out yet 🙃

Also, keep in mind I have not implemented a wayland protocol before, so I did my best to follow what I thought the protocol said, and as it is quite a simple one, I hope I didn't screw it up to bad.

if (inhibitor == NULL){
return;
}

Copy link
Author

@NDagestad NDagestad Apr 15, 2021

Choose a reason for hiding this comment

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

TBH I don't really like these conditions inside each other, but I didn't find a better logic to implement it... (EDIT: this was meant for the conditions above 🤦)

Copy link
Owner

Choose a reason for hiding this comment

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

If you create an "input inhibitor module", you may find that this turns out nicer. It's fine to create an abstraction for it even though it's small. You can look at how the keyboard, pointer and seat modules are structured.

@NDagestad
Copy link
Author

Here's a version where the inhibitor is put in a structure, the remaining things to do listed in my initial message are still not implemented yet, I'll see if I can get to do some of theme over the weekend

include/inhibitor.h Show resolved Hide resolved
include/inhibitor.h Show resolved Hide resolved
#include "wlr-input-inhibitor-unstable-v1.h"


struct inhibitor {
Copy link
Owner

Choose a reason for hiding this comment

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

Let's call this input_inhibitor.

src/inhibitor.c Outdated
#include "inhibitor.h"
#include <stdio.h>


Copy link
Owner

Choose a reason for hiding this comment

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

style

src/inhibitor.c Outdated
return self->inhibitor != NULL;
}


Copy link
Owner

Choose a reason for hiding this comment

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

style

src/main.c Outdated
return;
}

if (!inhibitor_is_inhibited(inhibitor)){
Copy link
Owner

Choose a reason for hiding this comment

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

Why not pass keyboard events when the input isn't inhibited?

Copy link
Author

Choose a reason for hiding this comment

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

In some cases it is not something we want. In my case, I have a shortcut that is triggered when is released, making it possible to launch rofi with a single key press, but if the client receives input when the inhibitor is not active it will receive the key press might trigger something on the client side (in this case, windows open the start menu)
The logic is still not really correct though, I fixed it in the update

src/main.c Outdated
@@ -103,6 +105,10 @@ static void registry_add(void* data, struct wl_registry* registry, uint32_t id,
xdg_wm_base = wl_registry_bind(registry, id, &xdg_wm_base_interface, 1);
} else if (strcmp(interface, "wl_shm") == 0) {
wl_shm = wl_registry_bind(registry, id, &wl_shm_interface, 1);
} else if (strcmp(interface, zwlr_input_inhibit_manager_v1_interface.name) == 0) {
struct zwlr_input_inhibit_manager_v1* manager = wl_registry_bind(registry, id, &zwlr_input_inhibit_manager_v1_interface, 1);
Copy link
Owner

Choose a reason for hiding this comment

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

The rest of the bindings here are assigned into global variables named after their respective interfaces. This should follow the same pattern.

This pattern breaks down if you want the wayland client to connect to multiple compositors, but that wouldn't really make any sense.

@NDagestad
Copy link
Author

Sorry for the long delay I did not expect to have so little time to work on this.
It turns out that this is not the right protocol to implement, I should have used the keyboard_shortcuts_inhibit_unstabe protocol instead which has the benefit of not being wlroots specific
This update is not handling seats correctly so multi seat setups will not work.
I saw you are capping the line length as 80, I tried to stay close to that but a few lines are a little longer.

@NDagestad NDagestad changed the title WIP: Implement the wlr input inhibitor protocol WIP: Implement the keyboard shortcuts inhibitor protocol Jun 9, 2021
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.

2 participants