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

Implement the session lock protocol, take 2 #2237

Merged
merged 6 commits into from
Mar 24, 2024

Conversation

lcolitti
Copy link
Contributor

Re-applies #2162 (which was reverted) plus addresses more review comments.

Fixes #1494
Fixes #1358

Copy link
Member

@ammen99 ammen99 left a comment

Choose a reason for hiding this comment

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

I've tested this and it seems to work. There is one smaller issue regarding the backup node, otherwise LGTM.

Also please update the API/ABI version in plugin.hpp - it should be updated every time we change something in the API headers.


class backup_node : public simple_text_node_t
{
public:
Copy link
Member

Choose a reason for hiding this comment

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

This node doesn't have keyboard or pointer interaction set - but it should have it, otherwise, it is not really 'locking' the desktop.

Also on my machine there are a few thin semi-transparent strips along some of the emoji's sides, I am not sure why that happens - I'm mentioning this because through them, I could see that I am clicking on the terminal behind the crashed swaylock :)

Copy link
Contributor Author

@lcolitti lcolitti Mar 24, 2024

Choose a reason for hiding this comment

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

Urg, yes. If I make the backup nodes transparent, it's pretty clear that they're not actually preventing keyboard or mouse input. Fixed in latest patch series (I think) by overriding find_node_at. Not sure if this is correct or whether it will fix touch as well (I don't have a way to test touch).

Copy link
Member

Choose a reason for hiding this comment

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

Urg, yes. If I make the backup nodes transparent, it's pretty clear that they're not actually preventing keyboard or mouse input. Fixed in latest patch series (I think) by overriding find_node_at. Not sure if this is correct or whether it will fix touch as well (I don't have a way to test touch).

Yes, this is enough to fix touch, tablet and pointer. For the keyboard you'd also have to implement the refocus_keyboard() function very similarly to the lock surface node type.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, you already have this, nice! I'll test on multi-monitor as well to make sure the logic doesn't break somewhere and merge.

@lcolitti lcolitti force-pushed the support-session-lock branch 3 times, most recently from 5b0aefb to a1f6665 Compare March 24, 2024 06:14
These classes are useful and needed by any code that deals with
surfaces, because such code usually needs to deal with
subsurfaces as well.

Move the include files into api/wayfire/unstable so they can be
used by code all over the tree, and rename them to reflect the
utility classes that they contain.
This will allow lockscreens to be implemented in plugins.
This adds a simple node that can display text.
This will be used by the upcoming implementation of the
ext-session-lock protocol.
Currently implemented:

- Locking and unlocking.
- Display above all desktop layers, including OVERLAY (but below
  DWIDGET).
- Waiting for the client to display locked surfaces before
  declaring the screen to be locked. If the client does not
  display all surfaces, locks anyway after a timeout.
- Multiple outputs, including outputs being added/removed while
  locking or locked.
- Leaving the screen locked if the client crashes (but allows a
  new client to come in and lock the screen).

TODO:
- Deal with outputs being resized.
- Properly display text if the screenlocker crashes. Currently it
  just displays two a big "explosion" emoji.
- Use a view instead of a plain wlr_surface_node?

Tested with swaylock.

Fixes WayfireWM#1494
Fixes WayfireWM#1358
Copy link
Member

@ammen99 ammen99 left a comment

Choose a reason for hiding this comment

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

Looks great, I'll do one last round of testing and merge if nothing comes up. Thanks a lot!

@ammen99
Copy link
Member

ammen99 commented Mar 24, 2024

@lcolitti Unfortunately I found one bug which seems quite important to fix, namely start Wayfire with two outputs, start swaylock, disconnect one of them => system is not responsive. Weirdly enough this does not happen if I use nested Wayfire outputs. So far I haven't been able to see what causes this though .. And it does not seem to happen when I use nested wayland backend outputs. Nevermind, this was an issue related to my setup. The only bug I found is that the screen is not properly damaged on unlock, which was easy to fix, I pushed a commit which makes things work. Thanks again for your work :)

@ammen99 ammen99 merged commit 8c93bb2 into WayfireWM:master Mar 24, 2024
4 checks passed
@lcolitti lcolitti deleted the support-session-lock branch April 3, 2024 13:19
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.

Support ext-session-lock mako notifications cannot be made above fullscreen views but below lockscreens
2 participants