session-lock: lock the screen even if the client provides no lock surface #2550
Conversation
plugins/protocols/session-lock.cpp
Outdated
| @@ -318,7 +318,7 @@ class wf_session_lock_plugin : public wf::plugin_interface_t | |||
| if (state == ZOMBIE) | |||
| { | |||
| output->set_inhibited(true); | |||
| output_states[output]->crashed_node->display(); | |||
| output_states[output]->crashed_node->display("💥"); | |||
There was a problem hiding this comment.
Maybe we can define this emoji somewhere as a constant in order to not repeat it? :)
There was a problem hiding this comment.
Yes, makes sense -- I've added a separate function for it
dkondor
force-pushed
the
lock_no_surface
branch
from
3da867f to
a644c23
Compare
|
I've also realized after some re-testing that this does not work if a screenlocker crashes without ever displaying a surface. The last commit takes care of this (let me know if you prefer a different approach though) |
plugins/protocols/session-lock.cpp
Outdated
| auto layer_node = output->node_for_layer(wf::scene::layer::LOCK); | ||
| wf::scene::add_back(layer_node, shared_from_this()); | ||
| if (!is_displayed) |
There was a problem hiding this comment.
Do we need an additional bool variable for this? AFAICT all you want to know is whether the node has alraedy been added to the scenegraph. You could do that by checking whether its parent is NULL.
There was a problem hiding this comment.
No need, I didn't realize parent() can be used for this :)
| { | ||
| output_state->surface_node->display(); | ||
| } else | ||
| { | ||
| // if the surface node has not yet been displayed we show an empty surface |
There was a problem hiding this comment.
Should we hide the crashed node once the actual lock node is there? I think for example if one wanted to implement a blurred lockscreen, or animate the lockscreen surface, this would prevent it from working.
There was a problem hiding this comment.
Potentially, however, after doing some tests with the current implementation, I'm a bit confused how this should happen:
- with Hyprlock: things already seem to "work": the crashed node disappears, and fade-in starts from the unlocked desktop
- with shaderlock: the unlocked screen flashes briefly (which is weird, since I assumed that crashed node should be there), but then the crashed lock screen comes back and is the basis of further animations
This is especially strange given that both projects seem to use the wlr-screencopy protocol to get a screenshot of the current desktop contents. Do you happen to know of another lockscreen app that does not make a screenshot, but just simply displays a (partially) transparent surface? That would allow me to test this more cleanly.
On a second thought, actually it could even make sense to not show the desktop after the previous lockscreen has already crashed. Contrary to when originally locking the screen, it could be a security / privacy issue if the screen contents are revealed when restarting a crashed lockscreen (however briefly). But then it is very weird why the screen shows up with shaderlock, I'll investigate that separately a bit more.
There was a problem hiding this comment.
As a guess, maybe they take the screenshot at different times?
There was a problem hiding this comment.
Yes, that can be.
Now I realized what we have here does not actually matter, since a new instance is created when a new lock client connects:
wayfire/plugins/protocols/session-lock.cpp
Line 460 in 4dc41b8
So that would also explain why the desktop becomes visible even with shaderlock (I assume it takes the screenshot before creating the new lock then). Maybe it would make sense to refactor this so that the crashed node is kept until the new client creates a lock surface and thus the screen cannot be revealed accidentally. However, that would be a bigger change and IMO unrelated to the current PR -- I can create a separate issue and try writing a patch for it if you think this can be an issue. Note that I've only seen this with shaderlock, probably because it is a bit slow to display its initial lock surface (e.g. swaylock appears fast enough). In any case, this is likely not a common issue, since most lockscreens do not crash, and even in that case, the problem manifests only if there is some automated way to restart them which could trigger when an "unauthorized" user is present.
There was a problem hiding this comment.
Note that I've only seen this with shaderlock, probably because it is a bit slow to display its initial lock surface (e.g. swaylock appears fast enough).
Actually, I've just tried gtklock and it is also slow enough that the unlocked desktop flashes, but in this case really briefly (barely possible to notice it).
Minimal changes to hide the screen contents in this case as well.
Simple test client: https://github.com/dkondor/simple_lock